January 2023 - flashrom-gerrit

[S] Change in flashrom[master]: tests/chip.c: Set MOCK_CHIP_CONTENT non-ambiguously
by Anastasia Klimchuk (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71826 ) Change subject: tests/chip.c: Set MOCK_CHIP_CONTENT non-ambiguously ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71826 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I15905180141aee54c166ff1c0275d1a7dfde0a46 Gerrit-Change-Number: 71826 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 12 Jan 2023 00:47:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: programmer: Add bitbanging programmer driver for Linux libgpiod
by Anastasia Klimchuk (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Thomas Heijligen, Angel Pons, Steve Markgraf. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71801 ) Change subject: programmer: Add bitbanging programmer driver for Linux libgpiod ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: Hello Steve, Thank you so much for your contribution! While this will need to go through all reviews from other people as well, I have a question to you. As you are an original author of the programmer, would you agree to add yourself into MAINTAINERS file for this programmer? This would mean you will be automatically added as reviewer if someone sends a patch which touches code for the programmer. Also since MAINTAINERS file is a bit of documentation, you will be the person who "knows something about this programmer" and people might ask you questions :) It would be great if you agree! As I said, the patch needs to go through all reviews, but I decided to ask a question now so you can think about it. Thank you! File linux_gpiod.c: https://review.coreboot.org/c/flashrom/+/71801/comment/2e5df448_ff6592ac PS1, Line 148: if (data) { : if (gpiod_line_bulk_num_lines(&data->bulk) > 0) : gpiod_line_release_bulk(&data->bulk); : : free(data); : } : : if (chip) : gpiod_chip_close(chip); I have few questions about the error path exit. 1) It looks like it is doing the same as shutdown function (which of course makes sense). Is it possible to try re-write a bit, so that just call shutdown function? If any changes are needed later for shutdown process, only one place would need to be changed. 2) The first two callsites of `goto err_exit` , where there is no data and chip is not opened yet: does anything need to be done there? could that be `return 1` (tell me if I am wrong)? I am thinking that maybe those two first callsites could be `return 1` and then err_exit could just call shutdown function? -- To view, visit https://review.coreboot.org/c/flashrom/+/71801 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icad3eb7764f28feaea51bda3a7893da724c86d06 Gerrit-Change-Number: 71801 Gerrit-PatchSet: 1 Gerrit-Owner: Steve Markgraf <steve(a)steve-m.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Steve Markgraf <steve(a)steve-m.de> Gerrit-Comment-Date: Thu, 12 Jan 2023 00:46:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests/: Add non-aligned write within a region unit-test
by Evan Benn (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Sam McNally, Edward O'Callaghan, Anastasia Klimchuk. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71659 ) Change subject: tests/: Add non-aligned write within a region unit-test ...................................................................... Patch Set 4: Code-Review+1 (8 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/71659/comment/e022e2b5_a5c37f5c PS2, Line 7: tests/: Add subregion alignment unit-test > Done What does non-aligned mean? The commit message can match the test name File tests/chip.c: https://review.coreboot.org/c/flashrom/+/71659/comment/866dc56d_59aa0572 PS2, Line 469: * {MOCK_CHIP_SUBREGION_CONTENTS} [..] {MOCK_CHIP_SUBREGION_CONTENTS}. > Done I dont understand this: `{MOCK_CHIP_SUBREGION_CONTENTS} [..] {MOCK_CHIP_SUBREGION_CONTENTS}` File tests/chip.c: https://review.coreboot.org/c/flashrom/+/71659/comment/4bfa7968_2a006efd PS4, Line 443: /* FIXME: MOCK_CHIP_CONTENT is buggy within setup_chip, it should also should this comment be here? if its buggy does the test work? what bug? https://review.coreboot.org/c/flashrom/+/71659/comment/5887cb65_e01c274f PS4, Line 447: #define MOCK_CHIP_SUBREGION_CONTENTS 0xCC CC is now the default setting of the chip, probably doesnt matter https://review.coreboot.org/c/flashrom/+/71659/comment/ee2c82e2_129af283 PS4, Line 457: /* Expect to verify the write completed successfuly within the region. */ is the presence of these flags important for catching the bug? If so, document why. If not, remove them. https://review.coreboot.org/c/flashrom/+/71659/comment/bd3c251f_82049b0f PS4, Line 469: * Create subregion smaller than erase granularity of chip. subregion -> region whole file replace please https://review.coreboot.org/c/flashrom/+/71659/comment/417ed310_671617d6 PS4, Line 498: /* .. */ remove https://review.coreboot.org/c/flashrom/+/71659/comment/aa83260a_d1be3a51 PS4, Line 500: /* Expect a verification pass that content within the region however this doesnt make sense -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 12 Jan 2023 00:40:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Remove --output log redirect option
by Peter Marheine (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Edward O'Callaghan, Evan Benn. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71773 ) Change subject: flashrom_tester: Remove --output log redirect option ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/71773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5eab8169644a16ba31b203e8607853c459f92978 Gerrit-Change-Number: 71773 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 00:28:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests/chip.c: Set MOCK_CHIP_CONTENT non-ambiguously
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71826 ) Change subject: tests/chip.c: Set MOCK_CHIP_CONTENT non-ambiguously ...................................................................... tests/chip.c: Set MOCK_CHIP_CONTENT non-ambiguously A chip content setup as 0x00 is ambiguous from a zero'ed heap or some erased chips and 0xFF is ambiguous from an erased chip. Change-Id: I15905180141aee54c166ff1c0275d1a7dfde0a46 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/chip.c 1 file changed, 14 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/71826/1 diff --git a/tests/chip.c b/tests/chip.c index 1d9cdc9..580f4ea 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -37,7 +37,7 @@ #include "programmer.h" #define MOCK_CHIP_SIZE (8*MiB) -#define MOCK_CHIP_CONTENT 0xff +#define MOCK_CHIP_CONTENT 0xCC /* 0x00 is a zeroed heap and 0xFF is an erased chip. */ static struct { unsigned int unlock_calls; /* how many times unlock function was called */ -- To view, visit https://review.coreboot.org/c/flashrom/+/71826 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I15905180141aee54c166ff1c0275d1a7dfde0a46 Gerrit-Change-Number: 71826 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: flashrom_tester: Remove --output log redirect option
by Nikolai Artemiev (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Edward O'Callaghan, Evan Benn, Peter Marheine. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71773 ) Change subject: flashrom_tester: Remove --output log redirect option ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: I support removing this, we get weird diffs with the `--output` option. `log()` calls are included in both stdout and the log file, but `println()!` is only included in stdout. Unix style redirect is more reliable for now. -- To view, visit https://review.coreboot.org/c/flashrom/+/71773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5eab8169644a16ba31b203e8607853c459f92978 Gerrit-Change-Number: 71773 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 12 Jan 2023 00:11:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: dirtyjtag: Add DirtyJTAG programmer
by Anastasia Klimchuk (Code Review) 11 Jan '23

11 Jan '23

Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878 ) Change subject: dirtyjtag: Add DirtyJTAG programmer ...................................................................... Patch Set 17: (1 comment) Patchset: PS17: Hello Jean, Many thanks for your contribution! Sorry for adding this follow-up comment much later, but I missed the patch. For the question I want to ask you, it's definitely not too late! :) Also I don't know whether you are on irc #flashrom, so will try here. We have MAINTAINERS file on flashrom, which documents people who have knowledge and could code reviews for a particular area of flashrom. An area could be just one specific programmer (and there are already entries in the file, for specific programmers). Would you agree to add yourself into MAINTAINERS file for DirtyJTAG programmer? You are original author of the programmer, it would be so great if you agree! I really hope for that. That would mean: if someone sends a patch which changes the programmer, you will be automatically added as reviewer. Also, MAINTAINERS serves as a piece of documentation, to find people "who knows something about X?" If you agree, you can just send a patch. As a plan B, I can send a patch myself and you would need to approve it. The first option is ideal. Thank you! Hope you will read this message :) -- To view, visit https://review.coreboot.org/c/flashrom/+/67878 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic43e9a014ed7d04e429e73b30c9dcfdde1a78913 Gerrit-Change-Number: 67878 Gerrit-PatchSet: 17 Gerrit-Owner: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Jan 2023 23:13:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: parallel: Drop explicit fallback_chip_X boilerplate
by Edward O'Callaghan (Code Review) 11 Jan '23

11 Jan '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71745 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: parallel: Drop explicit fallback_chip_X boilerplate ...................................................................... parallel: Drop explicit fallback_chip_X boilerplate A NULL func pointer is necessary and sufficient for the condition `NULL func pointer => fallback_chip_X' as to not need this explicit specification. Therefore drop the explicit need to specify these fallback callback function pointer in the par_master struct. This is a reasonable default for every driver in the tree. Furthermore, move the 'fallback_chip_X()' func from the generic programmer.c register logic into its relevant home of parallel.c and make static local to clean up link-time symbol space. This simplifies the code and driver development. Change-Id: If25c0048a07057aa72be6ffa8d8ad7f0a568dcf7 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/71745 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M atahpt.c M atapromise.c M atavia.c M drkaiser.c M gfxnvidia.c M include/programmer.h M internal.c M it8212.c M nic3com.c M nicintel.c M nicnatsemi.c M nicrealtek.c M parallel.c M programmer.c M satamv.c M satasii.c M serprog.c 17 files changed, 98 insertions(+), 144 deletions(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved diff --git a/atahpt.c b/atahpt.c index 89b44c2..71fd4d5 100644 --- a/atahpt.c +++ b/atahpt.c @@ -74,13 +74,7 @@ static const struct par_master par_master_atahpt = { .chip_readb = atahpt_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = atahpt_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = atahpt_shutdown, }; diff --git a/atapromise.c b/atapromise.c index e1b8638..9beebf1 100644 --- a/atapromise.c +++ b/atapromise.c @@ -113,13 +113,7 @@ static const struct par_master par_master_atapromise = { .chip_readb = atapromise_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = atapromise_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = atapromise_shutdown, }; diff --git a/atavia.c b/atavia.c index 8af52d6..f35771a 100644 --- a/atavia.c +++ b/atavia.c @@ -135,13 +135,7 @@ static const struct par_master lpc_master_atavia = { .map_flash_region = atavia_map, .chip_readb = atavia_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = atavia_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, }; static int atavia_init(const struct programmer_cfg *cfg) diff --git a/drkaiser.c b/drkaiser.c index 96f2c99..ebf5119 100644 --- a/drkaiser.c +++ b/drkaiser.c @@ -71,13 +71,7 @@ static const struct par_master par_master_drkaiser = { .chip_readb = drkaiser_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = drkaiser_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = drkaiser_shutdown, }; diff --git a/gfxnvidia.c b/gfxnvidia.c index 1d484ea..b420975 100644 --- a/gfxnvidia.c +++ b/gfxnvidia.c @@ -96,13 +96,7 @@ static const struct par_master par_master_gfxnvidia = { .chip_readb = gfxnvidia_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = gfxnvidia_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = gfxnvidia_shutdown, }; diff --git a/include/programmer.h b/include/programmer.h index 67e7b4a..281a4f6 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -442,12 +442,6 @@ int register_par_master(const struct par_master *mst, const enum chipbustype buses, void *data); /* programmer.c */ -void fallback_chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr); -void fallback_chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr); -void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len); -uint16_t fallback_chip_readw(const struct flashctx *flash, const chipaddr addr); -uint32_t fallback_chip_readl(const struct flashctx *flash, const chipaddr addr); -void fallback_chip_readn(const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len); struct registered_master { enum chipbustype buses_supported; struct { diff --git a/internal.c b/internal.c index 9b80f37..f21c4fc 100644 --- a/internal.c +++ b/internal.c @@ -116,7 +116,6 @@ .chip_writeb = internal_chip_writeb, .chip_writew = internal_chip_writew, .chip_writel = internal_chip_writel, - .chip_writen = fallback_chip_writen, }; static int get_params(const struct programmer_cfg *cfg, diff --git a/it8212.c b/it8212.c index 8fe2b59..3c1161d 100644 --- a/it8212.c +++ b/it8212.c @@ -64,13 +64,7 @@ static const struct par_master par_master_it8212 = { .chip_readb = it8212_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = it8212_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = it8212_shutdown, }; diff --git a/nic3com.c b/nic3com.c index efba979..a578d48 100644 --- a/nic3com.c +++ b/nic3com.c @@ -91,13 +91,7 @@ static const struct par_master par_master_nic3com = { .chip_readb = nic3com_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = nic3com_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = nic3com_shutdown, }; diff --git a/nicintel.c b/nicintel.c index 47edbb7..feb07b6 100644 --- a/nicintel.c +++ b/nicintel.c @@ -67,13 +67,7 @@ static const struct par_master par_master_nicintel = { .chip_readb = nicintel_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = nicintel_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = nicintel_shutdown, }; diff --git a/nicnatsemi.c b/nicnatsemi.c index efa879a..65377dc 100644 --- a/nicnatsemi.c +++ b/nicnatsemi.c @@ -78,13 +78,7 @@ static const struct par_master par_master_nicnatsemi = { .chip_readb = nicnatsemi_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = nicnatsemi_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = nicnatsemi_shutdown, }; diff --git a/nicrealtek.c b/nicrealtek.c index 103ea9e..5937e35 100644 --- a/nicrealtek.c +++ b/nicrealtek.c @@ -87,13 +87,7 @@ static const struct par_master par_master_nicrealtek = { .chip_readb = nicrealtek_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = nicrealtek_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = nicrealtek_shutdown, }; diff --git a/parallel.c b/parallel.c index cea9e7a..b193723 100644 --- a/parallel.c +++ b/parallel.c @@ -27,19 +27,49 @@ flash->mst->par.chip_writeb(flash, val, addr); } +/* Little-endian fallback for drivers not supporting 16 bit accesses */ +static void fallback_chip_writew(const struct flashctx *flash, uint16_t val, + chipaddr addr) +{ + chip_writeb(flash, val & 0xff, addr); + chip_writeb(flash, (val >> 8) & 0xff, addr + 1); +} + void chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr) { - flash->mst->par.chip_writew(flash, val, addr); + if (flash->mst->par.chip_writew) + flash->mst->par.chip_writew(flash, val, addr); + fallback_chip_writew(flash, val, addr); +} + +/* Little-endian fallback for drivers not supporting 32 bit accesses */ +static void fallback_chip_writel(const struct flashctx *flash, uint32_t val, + chipaddr addr) +{ + chip_writew(flash, val & 0xffff, addr); + chip_writew(flash, (val >> 16) & 0xffff, addr + 2); } void chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr) { - flash->mst->par.chip_writel(flash, val, addr); + if (flash->mst->par.chip_writel) + flash->mst->par.chip_writel(flash, val, addr); + fallback_chip_writel(flash, val, addr); +} + +static void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len) +{ + size_t i; + for (i = 0; i < len; i++) + chip_writeb(flash, buf[i], addr + i); + return; } void chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len) { - flash->mst->par.chip_writen(flash, buf, addr, len); + if (flash->mst->par.chip_writen) + flash->mst->par.chip_writen(flash, buf, addr, len); + fallback_chip_writen(flash, buf, addr, len); } uint8_t chip_readb(const struct flashctx *flash, const chipaddr addr) @@ -47,20 +77,53 @@ return flash->mst->par.chip_readb(flash, addr); } +/* Little-endian fallback for drivers not supporting 16 bit accesses */ +static uint16_t fallback_chip_readw(const struct flashctx *flash, const chipaddr addr) +{ + uint16_t val; + val = chip_readb(flash, addr); + val |= chip_readb(flash, addr + 1) << 8; + return val; +} + uint16_t chip_readw(const struct flashctx *flash, const chipaddr addr) { - return flash->mst->par.chip_readw(flash, addr); + if (flash->mst->par.chip_readw) + return flash->mst->par.chip_readw(flash, addr); + return fallback_chip_readw(flash, addr); +} + +/* Little-endian fallback for drivers not supporting 32 bit accesses */ +static uint32_t fallback_chip_readl(const struct flashctx *flash, const chipaddr addr) +{ + uint32_t val; + val = chip_readw(flash, addr); + val |= chip_readw(flash, addr + 2) << 16; + return val; } uint32_t chip_readl(const struct flashctx *flash, const chipaddr addr) { - return flash->mst->par.chip_readl(flash, addr); + if (flash->mst->par.chip_readl) + return flash->mst->par.chip_readl(flash, addr); + return fallback_chip_readl(flash, addr); +} + +static void fallback_chip_readn(const struct flashctx *flash, uint8_t *buf, + chipaddr addr, size_t len) +{ + size_t i; + for (i = 0; i < len; i++) + buf[i] = chip_readb(flash, addr + i); + return; } void chip_readn(const struct flashctx *flash, uint8_t *buf, chipaddr addr, size_t len) { - flash->mst->par.chip_readn(flash, buf, addr, len); + if (flash->mst->par.chip_readn) + flash->mst->par.chip_readn(flash, buf, addr, len); + fallback_chip_readn(flash, buf, addr, len); } int register_par_master(const struct par_master *mst, @@ -87,9 +150,7 @@ return ERROR_FLASHROM_BUG; } - if (!mst->chip_writeb || !mst->chip_writew || !mst->chip_writel || - !mst->chip_writen || !mst->chip_readb || !mst->chip_readw || - !mst->chip_readl || !mst->chip_readn) { + if (!mst->chip_writeb || !mst->chip_readb) { msg_perr("%s called with incomplete master definition. " "Please report a bug at flashrom(a)flashrom.org\n", __func__); diff --git a/programmer.c b/programmer.c index d4f15a1..15fd1b8 100644 --- a/programmer.c +++ b/programmer.c @@ -17,57 +17,6 @@ #include "flash.h" #include "programmer.h" -/* Little-endian fallback for drivers not supporting 16 bit accesses */ -void fallback_chip_writew(const struct flashctx *flash, uint16_t val, - chipaddr addr) -{ - chip_writeb(flash, val & 0xff, addr); - chip_writeb(flash, (val >> 8) & 0xff, addr + 1); -} - -/* Little-endian fallback for drivers not supporting 16 bit accesses */ -uint16_t fallback_chip_readw(const struct flashctx *flash, const chipaddr addr) -{ - uint16_t val; - val = chip_readb(flash, addr); - val |= chip_readb(flash, addr + 1) << 8; - return val; -} - -/* Little-endian fallback for drivers not supporting 32 bit accesses */ -void fallback_chip_writel(const struct flashctx *flash, uint32_t val, - chipaddr addr) -{ - chip_writew(flash, val & 0xffff, addr); - chip_writew(flash, (val >> 16) & 0xffff, addr + 2); -} - -/* Little-endian fallback for drivers not supporting 32 bit accesses */ -uint32_t fallback_chip_readl(const struct flashctx *flash, const chipaddr addr) -{ - uint32_t val; - val = chip_readw(flash, addr); - val |= chip_readw(flash, addr + 2) << 16; - return val; -} - -void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len) -{ - size_t i; - for (i = 0; i < len; i++) - chip_writeb(flash, buf[i], addr + i); - return; -} - -void fallback_chip_readn(const struct flashctx *flash, uint8_t *buf, - chipaddr addr, size_t len) -{ - size_t i; - for (i = 0; i < len; i++) - buf[i] = chip_readb(flash, addr + i); - return; -} - /* The limit of 4 is totally arbitrary. */ #define MASTERS_MAX 4 struct registered_master registered_masters[MASTERS_MAX]; diff --git a/satamv.c b/satamv.c index dcd53c7..fea4c2b 100644 --- a/satamv.c +++ b/satamv.c @@ -88,13 +88,7 @@ static const struct par_master par_master_satamv = { .chip_readb = satamv_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = satamv_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = satamv_shutdown, }; diff --git a/satasii.c b/satasii.c index 011562e..e881f96 100644 --- a/satasii.c +++ b/satasii.c @@ -95,13 +95,7 @@ static const struct par_master par_master_satasii = { .chip_readb = satasii_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = satasii_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = satasii_shutdown, }; diff --git a/serprog.c b/serprog.c index 4e131e5..5ddbe3d 100644 --- a/serprog.c +++ b/serprog.c @@ -594,13 +594,8 @@ static const struct par_master par_master_serprog = { .map_flash_region = serprog_map, .chip_readb = serprog_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, .chip_readn = serprog_chip_readn, .chip_writeb = serprog_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .delay = serprog_delay, }; -- To view, visit https://review.coreboot.org/c/flashrom/+/71745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If25c0048a07057aa72be6ffa8d8ad7f0a568dcf7 Gerrit-Change-Number: 71745 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flashrom_tester: Remove --output log redirect option
by Evan Benn (Code Review) 11 Jan '23

11 Jan '23

Attention is currently required from: Edward O'Callaghan, Peter Marheine. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71773 ) Change subject: flashrom_tester: Remove --output log redirect option ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > This has been a pretty useful facility during AVL is `flashrom_tester > log_file` sufficiently useful? -- To view, visit https://review.coreboot.org/c/flashrom/+/71773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5eab8169644a16ba31b203e8607853c459f92978 Gerrit-Change-Number: 71773 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Jan 2023 22:46:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: sb600spi.c: Move promontory code into a mmap_read=yes mode
by Arthur Heymans (Code Review) 11 Jan '23

11 Jan '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71565 ) Change subject: sb600spi.c: Move promontory code into a mmap_read=yes mode ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/71565/comment/56fb5635_e0a7e86f PS2, Line 10: This is not : hardware specific > I believe it is hardware specific to controllers after a certain generation. i.e., supported on SPI100 controllers (Zen etc) however not supported on pre-SPI100 controllers was my understanding. I think the parts controlling the speed and mode might not apply, but it is memory mapped for sure. -- To view, visit https://review.coreboot.org/c/flashrom/+/71565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13eb5a646d3569170b3911ae7b3127cd3e6022aa Gerrit-Change-Number: 71565 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Jan 2023 14:58:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023