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

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25R2035F
by DZ (Code Review) April 28, 2024

April 28, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. DZ has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81839?usp=email ) Change subject: flashchips: Add support for MXIC MX25R2035F ...................................................................... Patch Set 6: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/81839/comment/181941dc_3b161514 : PS5, Line 10216: .name = "MX25R2035F", > Same as in the other patch: being sorted alphabetically by name, this should be between R16 and R32 Done -- To view, visit https://review.coreboot.org/c/flashrom/+/81839?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I00e76ef942976e3e102cf71fe695c6287b392b64 Gerrit-Change-Number: 81839 Gerrit-PatchSet: 6 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> 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, 29 Apr 2024 02:49:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L1636E
by DZ (Code Review) April 28, 2024

April 28, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. DZ has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81835?usp=email ) Change subject: flashchips: Add support for MXIC MX25L1636E ...................................................................... Patch Set 3: (3 comments) File flashchips.c: https://review.coreboot.org/c/flashrom/+/81835/comment/3a5a08d4_3b33c196 : PS1, Line 9283: OTP: 64B total > I know it was like this before, but datasheets for both chips say 4k-bit secured OTP, so seems like […] Done https://review.coreboot.org/c/flashrom/+/81835/comment/c464616f_e4b1ca3f : PS1, Line 9284: FEATURE_CFGR > None of datasheets have configuration register? Done https://review.coreboot.org/c/flashrom/+/81835/comment/13364474_cb424798 : PS1, Line 9313: .tb = {CONFIG, 3, OTP} > Same as above, none of datasheets have configuration register Done -- To view, visit https://review.coreboot.org/c/flashrom/+/81835?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I415e2d6c89d3d59ba44e22753001c6f69421c39d Gerrit-Change-Number: 81835 Gerrit-PatchSet: 3 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> 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, 29 Apr 2024 02:48:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L1633E
by DZ (Code Review) April 28, 2024

April 28, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. DZ has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81836?usp=email ) Change subject: flashchips: Add support for MXIC MX25L1633E ...................................................................... Patch Set 5: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/81836/comment/c36c63e5_9397a4a9 : PS3, Line 9280: MACRONIX_MX25L1635D > Just one thing here: if you can please mention new chip MX25L1633E in a comment to existing macro MA […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/81836?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I63ee0182ad6e62b7408136285aa0e927d53f7bc8 Gerrit-Change-Number: 81836 Gerrit-PatchSet: 5 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> 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, 29 Apr 2024 02:47:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L3273F
by DZ (Code Review) April 28, 2024

April 28, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. DZ has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81562?usp=email ) Change subject: flashchips: Add support for MXIC MX25L3273F ...................................................................... Patch Set 10: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/81562/comment/41c924e4_a6b20fe1 : PS7, Line 9522: 1024B > Daniel, I noticed that you updated the patch but didn't mark comment as fixed. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/81562?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4adaaa796d1db34702e7b0ed8e6fb167a3a5f6d7 Gerrit-Change-Number: 81562 Gerrit-PatchSet: 10 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> 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, 29 Apr 2024 02:46:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: Make sleep threshold for delays configurable
by Peter Marheine (Code Review) April 28, 2024

April 28, 2024

Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen. Hello Anastasia Klimchuk, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81606?usp=email to look at the new patch set (#4). Change subject: Make sleep threshold for delays configurable ...................................................................... Make sleep threshold for delays configurable This allows the minimum time that default_delay() will choose to sleep for instead of polling to be configured at build-time. The default remains unchanged at 100 milliseconds for now. The test's correctness has been checked by testing with minimum sleep time left at its default and set to a non-default value smaller than 100 microseconds (both pass without sleeping, verified with strace) and with the minimum sleep time set to 0 (causing the test to be skipped). The configured value from the macro needs to be stored in a const to avoid -Werror=type-limits errors when configured to be zero. Change-Id: Ida96e0816ac914ed69d6fd82ad90ebe89cdef1cc Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M Makefile M meson.build M meson_options.txt M tests/udelay.c M udelay.c M udelay_dos.c 6 files changed, 36 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/81606/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/81606?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ida96e0816ac914ed69d6fd82ad90ebe89cdef1cc Gerrit-Change-Number: 81606 Gerrit-PatchSet: 4 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[main]: Make sleep threshold for delays configurable
by Peter Marheine (Code Review) April 28, 2024

April 28, 2024

Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81606?usp=email ) Change subject: Make sleep threshold for delays configurable ...................................................................... Patch Set 3: (3 comments) File Makefile: https://review.coreboot.org/c/flashrom/+/81606/comment/7d6bd934_c2352815 : PS2, Line 545: CONFIG_DELAY_MINIMUM_SLEEP_US ?= 100000 > meson_options. […] Done File tests/udelay.c: https://review.coreboot.org/c/flashrom/+/81606/comment/559241cb_26a68d48 : PS2, Line 45: udelay_test_short > For completeness, it would be good to build and run unit tests with few different settings: […] Done, also needed to change the code a little to not generate spurious errors when it's set to zero. https://review.coreboot.org/c/flashrom/+/81606/comment/f6d76b38_3abb2884 : PS2, Line 52: if (delay_us >= CONFIG_DELAY_MINIMUM_SLEEP_US) > why this condition is needed? […] The test always takes at least as long to run as the chosen delay time, so 0.1ms seemed like a reasonable tradeoff between a meaninful delay and not wasting a lot of time running the test. 1ms still seems like an okay maximum if you wanted one that's more than the current value, though. -- To view, visit https://review.coreboot.org/c/flashrom/+/81606?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ida96e0816ac914ed69d6fd82ad90ebe89cdef1cc Gerrit-Change-Number: 81606 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 29 Apr 2024 01:17:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: Make sleep threshold for delays configurable
by Peter Marheine (Code Review) April 28, 2024

April 28, 2024

Attention is currently required from: Peter Marheine, Thomas Heijligen. Hello Anastasia Klimchuk, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81606?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: Make sleep threshold for delays configurable ...................................................................... Make sleep threshold for delays configurable This allows the minimum time that default_delay() will choose to sleep for instead of polling to be configured at build-time. The default remains unchanged at 100 milliseconds for now. The test's correctness has been checked by testing with minimum sleep time left at its default and set to a non-default value smaller than 100 microseconds (both pass without sleeping, verified with strace) and with the minimum sleep time set to 0 (causing the test to be skipped). The configured value from the macro needs to be stored in a const to avoid -Werror=type-limits errors when configured to be zero. Change-Id: Ida96e0816ac914ed69d6fd82ad90ebe89cdef1cc Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M Makefile M meson.build M meson_options.txt M tests/udelay.c M udelay.c M udelay_dos.c 6 files changed, 36 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/81606/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/81606?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ida96e0816ac914ed69d6fd82ad90ebe89cdef1cc Gerrit-Change-Number: 81606 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[XS] Change in flashrom[main]: flashrom_tester: Correct "WP screw" message
by Angel Pons (Code Review) April 28, 2024

April 28, 2024

Attention is currently required from: Brian Norris, Hsuan-ting Chen, Thomas Heijligen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82083?usp=email ) Change subject: flashrom_tester: Correct "WP screw" message ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82083?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I45f06db51e92e68bf724b13bdf5b31bba511d270 Gerrit-Change-Number: 82083 Gerrit-PatchSet: 5 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Sun, 28 Apr 2024 11:22:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake
by Anastasia Klimchuk (Code Review) April 28, 2024

April 28, 2024

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Subrata Banik. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81357?usp=email ) Change subject: ichspi.c: Add support for region 9 and beyond in Meteor Lake ...................................................................... Patch Set 10: (11 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/81357/comment/e96b43c1_539e5275 : PS10, Line 28: Is MTL a Meteor Lake and ADL Alder Point? Is it possible to test the chipsets that used to be after Meteor in the enum, and now they are before? File ichspi.c: https://review.coreboot.org/c/flashrom/+/81357/comment/f25256b6_1456dcf5 : PS10, Line 1853: mmio_readw Why it is `readw` (reads into uint16_t) here, but `readl` below (reads into uint32_t)? You declare `region_read_access` and `region_write_access` as uint32_t actually https://review.coreboot.org/c/flashrom/+/81357/comment/e789d35a_3e357092 : PS10, Line 1854: *region_write_access = mmio_readw(ich_spibar + ICH_REG_BIOS_BM_WAP); Maybe you can also add some debugging prints in this branch too https://review.coreboot.org/c/flashrom/+/81357/comment/47f1115f_18bd15d7 : PS10, Line 1876: inline I don't think this needs to be inline? https://review.coreboot.org/c/flashrom/+/81357/comment/8a8684c3_be89553c : PS10, Line 1877: if (ich_generation >= CHIPSET_METEOR_LAKE) : return 16; : return 8; maybe make it one line `return (ich_generation >= CHIPSET_METEOR_LAKE) ? 16 : 8;` https://review.coreboot.org/c/flashrom/+/81357/comment/2b4ac08b_fdc32cc8 : PS10, Line 2109: case CHIPSET_METEOR_LAKE: Let's re-order Meteor Lake here too https://review.coreboot.org/c/flashrom/+/81357/comment/735165af_ce23a858 : PS10, Line 2149: case CHIPSET_METEOR_LAKE: and here https://review.coreboot.org/c/flashrom/+/81357/comment/119727ec_9cb0c1f7 : PS10, Line 2211: case CHIPSET_METEOR_LAKE: and here https://review.coreboot.org/c/flashrom/+/81357/comment/7ed5d9c0_fe2647b9 : PS10, Line 2229: you can leave the word "registers", so that it is "... and region access registers" https://review.coreboot.org/c/flashrom/+/81357/comment/923bd8a6_b0f2d5b8 : PS10, Line 2291: case CHIPSET_METEOR_LAKE: and here https://review.coreboot.org/c/flashrom/+/81357/comment/e5a66a3d_d85dc523 : PS10, Line 2331: case CHIPSET_METEOR_LAKE: and here -- To view, visit https://review.coreboot.org/c/flashrom/+/81357?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 10 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Sun, 28 Apr 2024 08:39:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashrom_tester: Correct "WP screw" message
by Anastasia Klimchuk (Code Review) April 27, 2024

April 27, 2024

Attention is currently required from: Brian Norris, Hsuan-ting Chen, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82083?usp=email ) Change subject: flashrom_tester: Correct "WP screw" message ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82083?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I45f06db51e92e68bf724b13bdf5b31bba511d270 Gerrit-Change-Number: 82083 Gerrit-PatchSet: 5 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Sat, 27 Apr 2024 09:51:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0