- flashrom-gerrit - mail.coreboot.org

[XS] Change in flashrom[main]: doc: Link useful section of manpage into Supported programmers index
by Anastasia Klimchuk (Code Review) 25 May '24

25 May '24

Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/82646?usp=email ) Change subject: doc: Link useful section of manpage into Supported programmers index ...................................................................... doc: Link useful section of manpage into Supported programmers index Change-Id: I0c8a761626784f31a71a47c2cebff2579ebbe416 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/classic_cli_manpage.rst M doc/supported_hw/supported_prog/index.rst 2 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/82646/1 diff --git a/doc/classic_cli_manpage.rst b/doc/classic_cli_manpage.rst index beafd82..19c804a 100644 --- a/doc/classic_cli_manpage.rst +++ b/doc/classic_cli_manpage.rst @@ -329,6 +329,7 @@ **-R, --version** Show version information and exit. +.. _programmer-specific information: PROGRAMMER-SPECIFIC INFORMATION ------------------------------- diff --git a/doc/supported_hw/supported_prog/index.rst b/doc/supported_hw/supported_prog/index.rst index 3f05c25..130ac20 100644 --- a/doc/supported_hw/supported_prog/index.rst +++ b/doc/supported_hw/supported_prog/index.rst @@ -4,6 +4,8 @@ flashrom supports many different programmers, for the full list you can look into `programmer_table.c <https://github.com/flashrom/flashrom/blob/main/programmer_table.c>`_ in the source tree. Some of the programmers have their own documentation pages, see below. +Please check the :ref:`programmer-specific information` section of the :doc:`/classic_cli_manpage` for more details about programmers and their usage. + If you can run flashrom locally, the command ``flashrom -L`` prints all devices supported per programmer (see :doc:`/classic_cli_manpage` for more details on command line options). The output of this command is long, so you might want to save it to file or grep. -- To view, visit https://review.coreboot.org/c/flashrom/+/82646?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I0c8a761626784f31a71a47c2cebff2579ebbe416 Gerrit-Change-Number: 82646 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>

1 0

[S] Change in flashrom[main]: doc: Add doc for supported flash chips
by Anastasia Klimchuk (Code Review) 25 May '24

25 May '24

Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen, Victor Lim. Hello Alexander Goncharov, Peter Marheine, Thomas Heijligen, Victor Lim, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82197?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: doc: Add doc for supported flash chips ...................................................................... doc: Add doc for supported flash chips Change-Id: I05fb60a4caf2cfb30586fa482687b10638996395 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/supported_hw/index.rst A doc/supported_hw/supported_flashchips.rst 2 files changed, 27 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/82197/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/82197?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I05fb60a4caf2cfb30586fa482687b10638996395 Gerrit-Change-Number: 82197 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Victor Lim <vlim(a)gigadevice.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <vlim(a)gigadevice.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips: add support for MX77U51250F chip
by Anastasia Klimchuk (Code Review) 25 May '24

25 May '24

Attention is currently required from: Bora Guvendik, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Bora Guvendik. ( https://review.coreboot.org/c/flashrom/+/82626?usp=email ) Change subject: flashchips: add support for MX77U51250F chip ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Bora, thank you for the patch! Could you please give a link to the datasheet for the chip? Also if you could tell which programmer are you using, and maybe you can give logs, for example you can upload them with https://paste.flashrom.org/ Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/82626?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I2c2e94f01dc63f60cf636bc6afe1f033e2a6f83c Gerrit-Change-Number: 82626 Gerrit-PatchSet: 3 Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 25 May 2024 08:56:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: flashchips.c: Add support for XM25RU256C
by Anastasia Klimchuk (Code Review) 25 May '24

25 May '24

Attention is currently required from: Georg Gottleuber, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Georg Gottleuber. ( https://review.coreboot.org/c/flashrom/+/82101?usp=email ) Change subject: flashchips.c: Add support for XM25RU256C ...................................................................... Patch Set 2: (2 comments) Patchset: PS1: > I have personally asked for the datasheet, but I don't know if it is okay to publish it. […] Sorry for delay in replying, I was thinking about your patch on the background. I usually ask for datasheet to check the values, but after all, you (the contributor) are the person who has the chip and running flashrom on it, so you did the actual testing, not me. The trickiest part of chip definition is write-protect bits, but you are not adding that support anyway. I think we can go ahead with the patch, I only have one small comment about indentation. If you double-check the values with datasheet, that would be really nice! Also if there were some of the fields that you were not entirely sure what to put there, please tell me. Thank you! File flashchips.c: https://review.coreboot.org/c/flashrom/+/82101/comment/c74866a0_214296b7?us… : PS2, Line 21729: | FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ : | FEATURE_4BA_WRITE, Sorry I didn't notice last time: please intend these two lines so that they align with the first feature line (align with FEATURE_WRSR_WREN). We keep all the values on the right-side of `=` signs, it's easier to read. -- To view, visit https://review.coreboot.org/c/flashrom/+/82101?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I431474a662304d09438e274706d3fc9cfbbe0bd6 Gerrit-Change-Number: 82101 Gerrit-PatchSet: 2 Gerrit-Owner: Georg Gottleuber <ggo(a)tuxedo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Georg Gottleuber <ggo(a)tuxedo.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 25 May 2024 08:34:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Georg Gottleuber <ggo(a)tuxedo.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: Add documentation for pico-serprog
by Anastasia Klimchuk (Code Review) 25 May '24

25 May '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82229?usp=email ) Change subject: Add documentation for pico-serprog ...................................................................... Add documentation for pico-serprog This commit adds documentation for pico-serprog by stacksmashing: https://github.com/stacksmashing/pico-serprog and its fork by Riku_V: https://codeberg.org/Riku_V/pico-serprog to the serprog overview page. Change-Id: I457dfec52f89997f64b6c276c50b329359d61b77 Signed-off-by: Funkeleinhorn <git(a)funkeleinhorn.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82229 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M doc/supported_hw/supported_prog/serprog/overview.rst 1 file changed, 12 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/doc/supported_hw/supported_prog/serprog/overview.rst b/doc/supported_hw/supported_prog/serprog/overview.rst index b274b99..ba01b02 100644 --- a/doc/supported_hw/supported_prog/serprog/overview.rst +++ b/doc/supported_hw/supported_prog/serprog/overview.rst @@ -81,3 +81,15 @@ A powerful option is `stm32-vserprog <https://github.com/dword1511/stm32-vserprog#stm32-vserprog>`_, a firmware for various STM32-based boards that turns them into serprog-based programmers with SPI clock speeds up to 36 MHz. + +pico-serprog +------------ + +`pico-serprog <https://github.com/stacksmashing/pico-serprog>`_ by stacksmashing is a firmware for +`Raspberry Pi Picos <https://www.raspberrypi.com/documentation/microcontrollers/raspberry-pi-pic…>`_ and other RP2040 based boards which turns them +into a serprog programmer. + +Notable forks are: + +`Riku_V's fork <https://codeberg.org/Riku_V/pico-serprog>`_ which uses the hardware SPI implementation instead of SPI over PIO (programmable IO) which +sacrifices arbitrary pinouts. The fork also implements custom USB descriptors which allow for custom udev-rules. -- To view, visit https://review.coreboot.org/c/flashrom/+/82229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I457dfec52f89997f64b6c276c50b329359d61b77 Gerrit-Change-Number: 82229 Gerrit-PatchSet: 5 Gerrit-Owner: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: J. Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Riku Viitanen <riku.viitanen(a)protonmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in flashrom[main]: flashchips: Correct feature_bits for MX25L128*
by Anastasia Klimchuk (Code Review) 24 May '24

24 May '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Hsuan-ting Chen. ( https://review.coreboot.org/c/flashrom/+/82605?usp=email ) Change subject: flashchips: Correct feature_bits for MX25L128* ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: > Update: I checked that with `-p internal` the current ToT will pass `--wp-status`; however, with `-p […] Thank you so much for testing and fixing! So do I understand correctly that now everything works with this patch? -- To view, visit https://review.coreboot.org/c/flashrom/+/82605?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I001cde6816bd099317bc9c23904c5fcbe6003241 Gerrit-Change-Number: 82605 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 24 May 2024 14:50:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com>

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 24 May '24

24 May '24

Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) Change subject: erasure_layout: Fix get_flash_region bug ...................................................................... Patch Set 9: (12 comments) Patchset: PS9: Thank you so much for combining all the work in one patch! File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/82393/comment/6c9f690b_7d0b2429?us… : PS5, Line 240: > I'm a little hesitant to, because `chipoff_t` implies inclusive endpoints. […] I forgot that `chipoff_t` has its own comment. Now I agree with you. https://review.coreboot.org/c/flashrom/+/82393/comment/efeaeea0_4087bd38?us… : PS5, Line 369: len = min(region_end, region.end - 1) - addr + 1; > I added a new parent change that fixes get_flash_region's incorrect use of chipoff_t (now the upper […] This is great, thank you! File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/82393/comment/ec18619d_d61c83fa?us… : PS9, Line 766: 16 To be consistent with your previous patch, and also I think tests are partially a documentation (a special kind of it), this needs to be 15, right? https://review.coreboot.org/c/flashrom/+/82393/comment/d03af7c3_02c698ba?us… : PS9, Line 951: get_erase_protected_region_algo_tests I think these ones can be also run through write operation, and verifying the memory state after write. The change in flashrom.c affects write as well. But this can be in a separate patch. https://review.coreboot.org/c/flashrom/+/82393/comment/e4e82405_a99070ef?us… : PS9, Line 953: const size_t num_unparameterized = 1; Do you think this is needed? It's the same name and the same function. If you change it to 2, it will run the same test twice. Maybe remove this and allocate one test? https://review.coreboot.org/c/flashrom/+/82393/comment/d42614b2_b3d5edc6?us… : PS9, Line 973: .name = "erase failure for unskipped unwritable regions", I had the trouble with test names, because I wanted an index (a test case #) in the name. This one is just one, so a name without an index is fine. I looked into the test `test_erase_fails_for_unwritable_region` I think it's only flag FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS that prevents in from being in the list? setup and teardown seem innocent, won't hurt to have them here. So it's not being unparameterized , but FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS=false is what makes this test different? Anyway, this can be changed later. https://review.coreboot.org/c/flashrom/+/82393/comment/80c8e648_8638f708?us… : PS9, Line 1103: 16 This is `END_PROTECTED_REGION + 1`, just for perfection (with my other comment about END_PROTECTED_REGION) https://review.coreboot.org/c/flashrom/+/82393/comment/7d1ee9a4_154eadc2?us… : PS9, Line 1138: < <= if you agree that it should be inclusive https://review.coreboot.org/c/flashrom/+/82393/comment/8c05f3fd_11e1d2f6?us… : PS9, Line 1140: < also here <= File tests/tests.h: https://review.coreboot.org/c/flashrom/+/82393/comment/310596a6_375a8f10?us… : PS9, Line 113: void erase_unwritable_regions_skipflag_off_test_success(void **state); this also can be removed from this patch https://review.coreboot.org/c/flashrom/+/82393/comment/f5a48954_05c164c1?us… : PS9, Line 114: erase_unwritable_regions_skipflag_on_test_success I am wondering maybe this is also not needed. Because the function which is called in tests.c is `get_erase_func_algo_tests` but not this one. I added this line, but then also tests ended up having a special invocation and I think it's not needed anymore. -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 9 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 24 May 2024 14:36:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[S] Change in flashrom[main]: Correct get_flash_region() to use inclusive upper bounds
by Anastasia Klimchuk (Code Review) 24 May '24

24 May '24

Attention is currently required from: Aarya, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email ) Change subject: Correct get_flash_region() to use inclusive upper bounds ...................................................................... Patch Set 2: (4 comments) Patchset: PS2: Thank you for fixing old tech debt ! File flashrom.c: https://review.coreboot.org/c/flashrom/+/82496/comment/772c9434_a3c6054d?us… : PS2, Line 1518: region.end - 1 `region.end` like in everywhere else? https://review.coreboot.org/c/flashrom/+/82496/comment/d494ed4d_802c4ab2?us… : PS2, Line 1524: region.end - 1 also `region.end` ? File ichspi.c: https://review.coreboot.org/c/flashrom/+/82496/comment/d5160f98_f0187702?us… : PS2, Line 1454: limit Why you are changing this? I understand why region end changes, but why region start? Is this another bug? -- To view, visit https://review.coreboot.org/c/flashrom/+/82496?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Gerrit-Change-Number: 82496 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 24 May 2024 09:54:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in flashrom[main]: tests/erase: record the opcode for each erase
by Anastasia Klimchuk (Code Review) 24 May '24

24 May '24

Attention is currently required from: Nikolai Artemiev, Peter Marheine, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82251?usp=email ) Change subject: tests/erase: record the opcode for each erase ...................................................................... Patch Set 6: (7 comments) Patchset: PS6: Sorry for delay in review! This looks great, I only have few small comments. This is very useful addition to unit tests! File include/flash.h: https://review.coreboot.org/c/flashrom/+/82251/comment/ac3849e9_432c58bd?us… : PS6, Line 334: /* special cases must come last. */ I think this comment better go under FLASHROM_TEST, otherwise it is orphaned in the non-test code. Don't know if anyone ever looks into generated code after pre-processor stage (I did few times only), but still. https://review.coreboot.org/c/flashrom/+/82251/comment/62256236_74314662?us… : PS6, Line 335: #ifndef FLASHROM_TEST : }; : #else : TEST_ERASE_INJECTOR_1, : TEST_ERASE_INJECTOR_2, : TEST_ERASE_INJECTOR_3, : TEST_ERASE_INJECTOR_4, : TEST_ERASE_INJECTOR_5, : }; : : #define NUM_TEST_ERASE_INJECTORS 5 : extern erasefunc_t *g_test_erase_injector[NUM_TEST_ERASE_INJECTORS]; : #endif This style is different from the others (read and write), ifndef vs ifdef and I know it saves few lines, but I think maybe in the case consistency would be good? File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/82251/comment/76917ae6_6e134596?us… : PS5, Line 198: block_erase_chip, block_erase_chip, block_erase_chip, block_erase_chip, block_erase_chip > I had avoided doing that just because it requires updating all of the existing test cases, but done […] Thank you for doing this. I think it's valuable to test, if we have test running we prevent it from being broken in future! File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/82251/comment/db5f2548_b18f0b00?us… : PS6, Line 205: block_erase_chip_1, : block_erase_chip_2, : block_erase_chip_3, : block_erase_chip_4, : block_erase_chip_5, I think it can help readers, to add per-line comment, since the suffixes (1, 2, 3, 4, 5) do not correspond to block-length/opcodes (1b, 2b, 4b, 8b, 16b). I know this can be deducted by going through all the code, but a comment here can be a good time-saver. Either, each line N comment `TEST_ERASE_INJECTOR_N` or, `erasefn for opcode with corresponds to block size N` or maybe you prefer some other comment https://review.coreboot.org/c/flashrom/+/82251/comment/e9aa0334_08b0bae1?us… : PS6, Line 649: {0xf, 0x1, TEST_ERASE_INJECTOR_1}, : {0x8, 0x1, TEST_ERASE_INJECTOR_1}, : {0x9, 0x1, TEST_ERASE_INJECTOR_1}, : {0xa, 0x1, TEST_ERASE_INJECTOR_1}, : {0xb, 0x1, TEST_ERASE_INJECTOR_1}, : {0xc, 0x1, TEST_ERASE_INJECTOR_1}, : {0xd, 0x1, TEST_ERASE_INJECTOR_1}, : {0xe, 0x1, TEST_ERASE_INJECTOR_1}, : {0x3, 0x1, TEST_ERASE_INJECTOR_1}, : {0x4, 0x1, TEST_ERASE_INJECTOR_1}, : {0x5, 0x1, TEST_ERASE_INJECTOR_1}, : {0x6, 0x1, TEST_ERASE_INJECTOR_1}, : {0x7, 0x1, TEST_ERASE_INJECTOR_1}, : {0x0, 0x1, TEST_ERASE_INJECTOR_1}, : {0x1, 0x1, TEST_ERASE_INJECTOR_1}, These lines have trailing whitespace (shows as bright pink in gerrit :)) File tests/meson.build: https://review.coreboot.org/c/flashrom/+/82251/comment/fe23678e_d7ef82fa?us… : PS5, Line 127: '-DFLASHROM_TEST', > Yes, building the regular flashrom binary still works. […] After thinking more, I agree with you. -- To view, visit https://review.coreboot.org/c/flashrom/+/82251?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I3983fe42c2e7f06668a1bd20d2db7fafa93b8043 Gerrit-Change-Number: 82251 Gerrit-PatchSet: 6 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 24 May 2024 09:21:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[M] Change in flashrom[main]: flashchips: Add support for GigaDevice GD25LR256E, GD251R512ME
by Anastasia Klimchuk (Code Review) 24 May '24

24 May '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58025?usp=email ) Change subject: flashchips: Add support for GigaDevice GD25LR256E, GD251R512ME ...................................................................... flashchips: Add support for GigaDevice GD25LR256E, GD251R512ME BUG=none BRANCH=none TEST= Flash image using Flashrom Tool flashrom -p raiden_debug_spi -w <test_binary> flashrom -p dediprog -w <test_binary> Also tested by two people on the mailing list: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/TCT5… Change-Id: I2fe6bc1219cd1ee19b93caabab69de938cfc44b0 Signed-off-by: Ravi Sarawadi <ravishankar.sarawadi(a)intel.com> Signed-off-by: Martin Roth <gaumless(a)gmail.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58025 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M flashchips.c M include/flashchips.h 2 files changed, 112 insertions(+), 0 deletions(-) Approvals: Peter Marheine: Looks good to me, approved build bot (Jenkins): Verified diff --git a/flashchips.c b/flashchips.c index b4f957b..776b3a8 100644 --- a/flashchips.c +++ b/flashchips.c @@ -6898,6 +6898,116 @@ }, { + .vendor = "GigaDevice", + .name = "GD25LR256E", + .bustype = BUS_SPI, + .manufacture_id = GIGADEVICE_ID, + .model_id = GIGADEVICE_GD25LR256E, + .total_size = 32768, + .page_size = 256, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_4BA, + .tested = TEST_OK_PREW, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = SPI_BLOCK_ERASE_21, + }, { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_5C, + }, { + .eraseblocks = { {32 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = SPI_BLOCK_ERASE_DC, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {32 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {32 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD, + .unlock = SPI_DISABLE_BLOCKPROTECT_BP4_SRWD, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, + .voltage = {1650, 2000}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like TB */ + }, + .decode_range = DECODE_RANGE_SPI25, + + }, + + { + .vendor = "GigaDevice", + .name = "GD25LR512ME", + .bustype = BUS_SPI, + .manufacture_id = GIGADEVICE_ID, + .model_id = GIGADEVICE_GD25LR512ME, + .total_size = 65536, + .page_size = 256, + /* OTP: 4096B total; read 0x48; write 0x42, erase 0x44 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_4BA, + .tested = TEST_OK_PREW, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = SPI_BLOCK_ERASE_21, + }, { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = SPI_BLOCK_ERASE_5C, + }, { + .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_DC, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD, + .unlock = SPI_DISABLE_BLOCKPROTECT_BP4_SRWD, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, + .voltage = {1650, 2000}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like TB */ + }, + .decode_range = DECODE_RANGE_SPI25, + }, + + { .vendor = "GigaDevice", .name = "GD25Q10", .bustype = BUS_SPI, diff --git a/include/flashchips.h b/include/flashchips.h index 2ec5aaf..b8a58b2 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -405,6 +405,8 @@ #define GIGADEVICE_GD25LQ128CD 0x6018 #define GIGADEVICE_GD25LQ255E 0x6019 #define GIGADEVICE_GD25LF128E 0x6318 +#define GIGADEVICE_GD25LR256E 0x6719 +#define GIGADEVICE_GD25LR512ME 0x671A #define GIGADEVICE_GD25WQ80E 0x6514 #define GIGADEVICE_GD29GL064CAB 0x7E0601 -- To view, visit https://review.coreboot.org/c/flashrom/+/58025?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I2fe6bc1219cd1ee19b93caabab69de938cfc44b0 Gerrit-Change-Number: 58025 Gerrit-PatchSet: 12 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit