August 2024 - flashrom-gerrit - mail.coreboot.org

[XS] Change in flashrom[main]: flashchips: add GD25B128E and GD25R128E
by Anastasia Klimchuk (Code Review) 20 Aug '24

20 Aug '24

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim. Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/83967?usp=email ) Change subject: flashchips: add GD25B128E and GD25R128E ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/83967?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: I14e3c44ebbcc65640042a7719401615b5aa66cc2 Gerrit-Change-Number: 83967 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.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: Victor Lim <vlim(a)gigadevice.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 20 Aug 2024 06:26:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: [stlinkv3_spi]Mark STLinkV3-Mini not working
by Anastasia Klimchuk (Code Review) 20 Aug '24

20 Aug '24

Attention is currently required from: Miklós Márton. Anastasia Klimchuk has posted comments on this change by Miklós Márton. ( https://review.coreboot.org/c/flashrom/+/83921?usp=email ) Change subject: [stlinkv3_spi]Mark STLinkV3-Mini not working ...................................................................... Patch Set 1: (3 comments) Patchset: PS1: Miklos, do you plan to merge this? I did review just in case :) Commit Message: https://review.coreboot.org/c/flashrom/+/83921/comment/7e2552d9_3b98c92c?us… : PS1, Line 7: [stlinkv3_spi]Mark STLinkV3-Mini not working We usually format commit title like this: > stlinkv3_spi: Mark STLinkV3-Mini not working (no square brackets, :, and space) File stlinkv3_spi.c: https://review.coreboot.org/c/flashrom/+/83921/comment/5f253230_00041653?us… : PS1, Line 504: goto init_err_exit; You need to do `free(param_str);` before goto -- To view, visit https://review.coreboot.org/c/flashrom/+/83921?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: Ic1ea4ca7acedca5d374cff8fcef450c18e55a9e8 Gerrit-Change-Number: 83921 Gerrit-PatchSet: 1 Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Comment-Date: Tue, 20 Aug 2024 06:00:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: chipset_enable.c: Use PCI_ACCESS_ECAM to access pci register
by Peter Marheine (Code Review) 20 Aug '24

20 Aug '24

Attention is currently required from: Anastasia Klimchuk, Hsuan Ting Chen, Kane Chen, Subrata Banik. Peter Marheine has posted comments on this change by Kane Chen. ( https://review.coreboot.org/c/flashrom/+/83896?usp=email ) Change subject: chipset_enable.c: Use PCI_ACCESS_ECAM to access pci register ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/83896?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: I4549f87c8b01da0a1d3d8ce0b3b75c1f5fa2cbab Gerrit-Change-Number: 83896 Gerrit-PatchSet: 7 Gerrit-Owner: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 20 Aug 2024 00:25:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: flashchips: Update test status for Fudan FM25Q08 and FM25Q128
by Samantaz Fox (Code Review) 19 Aug '24

19 Aug '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Samantaz Fox has posted comments on this change by Samantaz Fox. ( https://review.coreboot.org/c/flashrom/+/83970?usp=email ) Change subject: flashchips: Update test status for Fudan FM25Q08 and FM25Q128 ...................................................................... Patch Set 2: (2 comments) Patchset: PS2: Done! File flashchips.c: https://review.coreboot.org/c/flashrom/+/83970/comment/1dd4e0c3_2ca3def3?us… : PS1, Line 6243: { .probe = NT, .read = OK, .erase = OK, .write = OK }, > Did you mean that to say that only Read operation is tested? […] Ah, for some reason I was convinced that flashrom would not probe when `-c name` was passed ^^ -- To view, visit https://review.coreboot.org/c/flashrom/+/83970?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: Ib3c94f03a132a912bb4bb9d36e8783f4468587c4 Gerrit-Change-Number: 83970 Gerrit-PatchSet: 2 Gerrit-Owner: Samantaz Fox <coding(a)samantaz.fr> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 19 Aug 2024 18:32:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: flashchips: Add definitions for Fudan FM25Q04, FM25Q64 and FM25Q128
by Samantaz Fox (Code Review) 19 Aug '24

19 Aug '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Samantaz Fox has posted comments on this change by Samantaz Fox. ( https://review.coreboot.org/c/flashrom/+/83969?usp=email ) Change subject: flashchips: Add definitions for Fudan FM25Q04, FM25Q64 and FM25Q128 ...................................................................... Patch Set 2: (3 comments) Patchset: PS2: I've made the requested changes 😊 Commit Message: https://review.coreboot.org/c/flashrom/+/83969/comment/f3f51d23_d9d1dbc4?us… : PS1, Line 7: Fuldan > Fuldan -> Fudan (a typo) Done https://review.coreboot.org/c/flashrom/+/83969/comment/f867f705_b02c4192?us… : PS1, Line 9: These chips have the exact same characteristics as their 8/16/32 counterparts, : except for the different flash size. > Do you have link to the datasheet, if you could add a link in commit message? […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/83969?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: I88fcc2bbb9706c8adb3722da6aa0e1d2d04c3fde Gerrit-Change-Number: 83969 Gerrit-PatchSet: 2 Gerrit-Owner: Samantaz Fox <coding(a)samantaz.fr> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 19 Aug 2024 18:29:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: flashchips: Update test status for Fudan FM25Q08 and FM25Q128
by Samantaz Fox (Code Review) 19 Aug '24

19 Aug '24

Attention is currently required from: Nikolai Artemiev, Samantaz Fox, Stefan Reinauer. Hello Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/83970?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: flashchips: Update test status for Fudan FM25Q08 and FM25Q128 ...................................................................... flashchips: Update test status for Fudan FM25Q08 and FM25Q128 Both of these chips were tested in-circuit with an SOIC-8 clamp and two different BusPirate boards: the BPv3.6 from Adafruit (sku 237) and the BPv3.6a from Sparkfun (sku TOL-12942), on a Fedora 38 host, using flashrom 9a570318 (changes rebased since then). Change-Id: Ib3c94f03a132a912bb4bb9d36e8783f4468587c4 Signed-off-by: Samantaz Fox <coding(a)samantaz.fr> --- M flashchips.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/70/83970/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/83970?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: Ib3c94f03a132a912bb4bb9d36e8783f4468587c4 Gerrit-Change-Number: 83970 Gerrit-PatchSet: 2 Gerrit-Owner: Samantaz Fox <coding(a)samantaz.fr> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Samantaz Fox <coding(a)samantaz.fr>

1 0

[M] Change in flashrom[main]: flashchips: Add definitions for Fudan FM25Q04, FM25Q64 and FM25Q128
by Samantaz Fox (Code Review) 19 Aug '24

19 Aug '24

Attention is currently required from: Nikolai Artemiev, Samantaz Fox, Stefan Reinauer. Hello Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/83969?usp=email to look at the new patch set (#2). Change subject: flashchips: Add definitions for Fudan FM25Q04, FM25Q64 and FM25Q128 ...................................................................... flashchips: Add definitions for Fudan FM25Q04, FM25Q64 and FM25Q128 These chips have the exact same characteristics as their 8/16/32 counterparts, except for the different flash size. Relevant datasheets (I've also included the FM25Q32 as a reference): * https://www.fmsh.com/nvm/FM25Q04_ds_eng.pdf * https://www.fmsh.com/nvm/FM25Q32_ds_eng.pdf * https://www.fmsh.com/nvm/FM25Q64_ds_eng.pdf * https://www.fmsh.com/nvm/FM25Q128_ds_eng.pdf Testing status will be updated in a subsequent commit. Change-Id: I88fcc2bbb9706c8adb3722da6aa0e1d2d04c3fde Signed-off-by: Samantaz Fox <coding(a)samantaz.fr> --- M flashchips.c M include/flashchips.h 2 files changed, 135 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/83969/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/83969?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: I88fcc2bbb9706c8adb3722da6aa0e1d2d04c3fde Gerrit-Change-Number: 83969 Gerrit-PatchSet: 2 Gerrit-Owner: Samantaz Fox <coding(a)samantaz.fr> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Samantaz Fox <coding(a)samantaz.fr>

1 0

[XS] Change in flashrom[main]: flashchips: Update test status for Fuldan FM25Q08 and FM25Q128
by Anastasia Klimchuk (Code Review) 19 Aug '24

19 Aug '24

Attention is currently required from: Nikolai Artemiev, Samantaz Fox, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Samantaz Fox. ( https://review.coreboot.org/c/flashrom/+/83970?usp=email ) Change subject: flashchips: Update test status for Fuldan FM25Q08 and FM25Q128 ...................................................................... Patch Set 1: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/83970/comment/a2b7e6d8_9147e480?us… : PS1, Line 6243: { .probe = NT, .read = OK, .erase = OK, .write = OK }, Did you mean that to say that only Read operation is tested? You need to use the macro `TEST_OK_PR` (all options are in /include/flash.h). Probing is done with any operation anyway, so you also tested it. -- To view, visit https://review.coreboot.org/c/flashrom/+/83970?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: Ib3c94f03a132a912bb4bb9d36e8783f4468587c4 Gerrit-Change-Number: 83970 Gerrit-PatchSet: 1 Gerrit-Owner: Samantaz Fox <coding(a)samantaz.fr> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Samantaz Fox <coding(a)samantaz.fr> Gerrit-Comment-Date: Mon, 19 Aug 2024 10:39:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in flashrom[main]: flashchips: Add definitions for Fuldan FM25Q04, FM25Q64 and FM25Q128
by Anastasia Klimchuk (Code Review) 19 Aug '24

19 Aug '24

Attention is currently required from: Nikolai Artemiev, Samantaz Fox, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Samantaz Fox. ( https://review.coreboot.org/c/flashrom/+/83969?usp=email ) Change subject: flashchips: Add definitions for Fuldan FM25Q04, FM25Q64 and FM25Q128 ...................................................................... Patch Set 1: (4 comments) Patchset: PS1: Hello Samantaz, thank you for the contribution! I have few comments. Commit Message: https://review.coreboot.org/c/flashrom/+/83969/comment/62541d5c_b69e8510?us… : PS1, Line 7: Fuldan Fuldan -> Fudan (a typo) https://review.coreboot.org/c/flashrom/+/83969/comment/bd307f86_12b54cc5?us… : PS1, Line 9: These chips have the exact same characteristics as their 8/16/32 counterparts, : except for the different flash size. Do you have link to the datasheet, if you could add a link in commit message? I will check the definitions then. Thank you! https://review.coreboot.org/c/flashrom/+/83969/comment/c9c64096_f313f1d3?us… : PS1, Line 12: Testing status will be updated in a subsequent commit. Just in case for future: usually if you add support for the model, and it is tested, you can set the test status in the same commit. But I see that here you add 3 models, and then updated test status for another 2, so you can leave it in two commits, all good. -- To view, visit https://review.coreboot.org/c/flashrom/+/83969?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: I88fcc2bbb9706c8adb3722da6aa0e1d2d04c3fde Gerrit-Change-Number: 83969 Gerrit-PatchSet: 1 Gerrit-Owner: Samantaz Fox <coding(a)samantaz.fr> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Samantaz Fox <coding(a)samantaz.fr> Gerrit-Comment-Date: Mon, 19 Aug 2024 10:33:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in flashrom[main]: ch347_spi: Add spi clock frequency selection
by Anastasia Klimchuk (Code Review) 19 Aug '24

19 Aug '24

Attention is currently required from: Nicholas Chin, ZhiYuanNJ. Anastasia Klimchuk has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82776?usp=email ) Change subject: ch347_spi: Add spi clock frequency selection ...................................................................... Patch Set 11: (2 comments) File ch347_spi.c: https://review.coreboot.org/c/flashrom/+/82776/comment/07382b7a_6f740b48?us… : PS11, Line 291: speed_index = 1; Default speed_index also needs to be 2 (to index the entry 15M) I just thought, you now have two values, `divisor` and `speed_index` which are meant to be in sync, but it's easy to get them out of sync and this can be potential bug. Maybe you can leave `speed_index` only? with default 2. > int speed_index = 2; /* default 15M SPI */ And then below, just always use `spispeeds[speed_index].divisor` File doc/classic_cli_manpage.rst: https://review.coreboot.org/c/flashrom/+/82776/comment/6f90e76f_4c64de52?us… : PS11, Line 1025: The default SPI speed is 30MHz if no value is specified. Since we are returning back to 15M, you need to update here as well > The default SPI speed is 15MHz if no value is specified (which corresponds to ``spispeed=15M``). -- To view, visit https://review.coreboot.org/c/flashrom/+/82776?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: If2be48929db540a6598ac0b60b37e64597156db7 Gerrit-Change-Number: 82776 Gerrit-PatchSet: 11 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: ZhiYuanNJ Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Mon, 19 Aug 2024 10:01:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2024