December 2024 - flashrom-gerrit

[S] Change in flashrom[main]: doc: Add section about v1.5.1 into release notes
by Matt DeVillier (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Anastasia Klimchuk. Hello Anastasia Klimchuk, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/85679?usp=email to review the following change. Change subject: doc: Add section about v1.5.1 into release notes ...................................................................... doc: Add section about v1.5.1 into release notes Change-Id: I80f8423133bf779093d57ea6928f09d9d377d20e Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M doc/release_notes/v_1_5.rst 1 file changed, 18 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/85679/1 diff --git a/doc/release_notes/v_1_5.rst b/doc/release_notes/v_1_5.rst index 6e81353..9471df0 100644 --- a/doc/release_notes/v_1_5.rst +++ b/doc/release_notes/v_1_5.rst @@ -15,10 +15,25 @@ Anonymous checkout from the git repository at https://review.coreboot.org/flashrom.git (tag v1.5.0) -A tarball is available for download at <TODO add tarball> -(signature <TODO add signature>) +A tarball is available for download at https://download.flashrom.org/releases/flashrom-v1.5.0.tar.xz +(signature https://download.flashrom.org/releases/flashrom-v1.5.0.tar.xz.asc) -fingerprint: <TODO add fingerprint> +fingerprint: 6E6E F9A0 BA47 8006 E277 6E4C C037 BB41 3134 D111 + +Point release v1.5.1 +==================== + +v1.5.1 includes one bug fix in addition to everything which is included in v1.5.0 + +Ticket: https://ticket.coreboot.org/issues/573 + +Patch: https://review.coreboot.org/c/flashrom/+/85612 + +Who exactly may be affected by this bug: + +Users with older Intel-based platforms (Broadwell/Braswell and earlier) flashing using the internal +programmer option may encounter an 'Invalid OPCODE' error when erasing/writing which leads to an +incomplete flash and potentially a bricked device. External flashing is not affected at all. Known issues ============ -- To view, visit https://review.coreboot.org/c/flashrom/+/85679?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: I80f8423133bf779093d57ea6928f09d9d377d20e Gerrit-Change-Number: 85679 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: flashchips: Add Spansion S25FS512S
by Anton Samsonov (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Anastasia Klimchuk, Anton Samsonov, Nikolai Artemiev, Stefan Reinauer. Anton Samsonov has posted comments on this change by Anton Samsonov. ( https://review.coreboot.org/c/flashrom/+/85585?usp=email ) Change subject: flashchips: Add Spansion S25FS512S ...................................................................... Patch Set 2: (3 comments) File flashchips/spansion.c: https://review.coreboot.org/c/flashrom/+/85585/comment/d5421685_7b7138ba?us… : PS2, Line 1043: /* Note on FEATURE_4BA_ENTER: the command set only defines command 0xB7 : to enter 4BA mode, but there is no counterpart command "Exit 4BA mode", : and the code 0xE9 is assigned to another command ("Password unlock"). */ : /* Note on FEATURE_4BA_ENTER_EAR7 (not set): the "Extended address mode" bit : is stored in configuration register CR2V[7], which can only be read / written : by generic commands ("Read any register" / "Write any register"), that itself : expect a 3- or 4-byte address of register in question, which in turn depends on : the same register, that is initially set from non-volatile register CR2NV[7] : that defaults to 0, but can be programmed to 1 for starting in 32-bit mode. */ > Thank you for sharing bits of wisdom in the comment! That is not a "wisdom", but merely observations of trial and error in finding proper combination. Without `FEATURE_4BA_ENTER`, this chip behaves very strange even when the sector 0 or any other sector <16M is addressed: it reports that the erase operation succeeded, when in fact most bits were left intact and other bits turned volatile — i. e. having random state (0 or 1) on each read. Such an unreliable operation led me to Evaluate Erase Status command (code D0h, see section 9.6.4 on page 113 of the datasheet), which looked very promising as an almost-instant built-in implementation of `is_sector_blank()` functionality that abolish data transmission from chip to host for erase verification. I implemented that in s25f.c, also generalizing `s25fl_sector_erase()` into `s25f_sector_erase()` with adaptive timeouts and automatic retry of failed erases. However, it turned out that this command always returns "OK" even when no erase was attempted at all. After finding out that simply adding `FEATURE_4BA_ENTER` "just works" even with generic `spi_block_erase_dc()`, I did not went further the "proprietary" way and did not bothered with adding Blank Check During Erase functionality (CR3V[5], see section 7.5.5.2 on page 62 of the datasheet). If someone is eager to have a look at their description and perhaps give an insight on what am I missing in understanding their intended behavior and/or proper use pattern, I can upload my current implementation as a separate "WiP" patch. File include/flashchips.h: https://review.coreboot.org/c/flashrom/+/85585/comment/c6526d23_9d59c80c?us… : PS2, Line 717: 0x02200081 > 03h -> ?? where it is? It is not a part of extended Device ID, but rather a header field indicating the total ID-CFI legacy structure size and the offset where the alternate structure starts (that is, 4Dh in this field means that the legacy part ends at byte 50h, and the alternate part starts at byte 51h). While it is essential for parsing the entire structure, it is obviously not a part of Device ID, thus `probe_spi_big_spansion()` simply concatenates bytes 01–02 and 04–05 to form a 32-bit identifier. The exact chip name, for example "S25FS512S", is stored in parameter 0 of alternate structure (see Table 62 on page 140) starting at offset 02h there; and even the minor part number designation at offset 10h–11h — same as bytes at offset 06h–07h in the legacy Device ID structure. However, using such a string as identifier would require up to 14 bytes, not counting the minor designation, which only `uint128_t` can accommodate. So a hashing algorithm may be necessary to keep 32-bit compatible identifiers, such as "compute `sha256(string)` and take the first 4 bytes as a big-endian number". https://review.coreboot.org/c/flashrom/+/85585/comment/9d60c1a6_2abbcd92?us… : PS2, Line 717: SPANSION_S25FS512S > `SPANSION_S25FS512S_UL` To tell the truth, I was tearing apart between `S25FS512S` and just `S25FS512` (without the trailing "S"), as `S25FL512` already uses that naming scheme, and because the datasheet defines no major variations for 512-Mbit model. If you vote for the `_UL` suffix, then should `S25FL512` also be changed to `S25FL512S_UL` simultaneously, since this patch alters that macro anyway? -- To view, visit https://review.coreboot.org/c/flashrom/+/85585?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: I40b6c081ec7d57eac4f6d2b69cea3878bc92bb47 Gerrit-Change-Number: 85585 Gerrit-PatchSet: 2 Gerrit-Owner: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anton Samsonov <devel(a)zxlab.ru> 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-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Thu, 19 Dec 2024 13:13:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: cli_classic: Add option to use first detected chip if multiple found
by Anastasia Klimchuk (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Anton Samsonov, David Hendricks, Matt DeVillier. Anastasia Klimchuk has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/flashrom/+/85160?usp=email ) Change subject: cli_classic: Add option to use first detected chip if multiple found ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > > There are certainly cases where there is no harm in selecting the first chip of multiple. […] Anton, thank you so much for joining. You have many useful ideas! I am not doing anything right now because I am thinking what is the right thing to do here. I fully understand the problem Matt is trying to solve, the IDs are repeating, more and more with time (because we never remove chip definitions, we only add new ones). I want to make SFDP detection more smooth next year but in any case this does not cover old chips. About the idea to use first chip: I still hold my initial opinion that this is a dangerous option in general, if we look at all the variety of use cases how people use flashrom and on which environments. However if we narrow down the use case, I understand that there is a use case when this option is fine. And actually, Matt's userbase is mostly belong to that category. I don't currently have a good idea whether it is possible to add this option so that "safe target audience" would be using it, and everyone else would know it is dangerous and won't. Maybe there is no safe way to add such option on upstream. Let's keep this patch here for a while? As I said, repeated IDs are real, and this is something on my mind to improve. It was very interesting to me to read Anton's thoughts. -- To view, visit https://review.coreboot.org/c/flashrom/+/85160?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: I0427b1ef80e4eca16623f0fc9119d79f7dd62551 Gerrit-Change-Number: 85160 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Comment-Date: Thu, 19 Dec 2024 12:57:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anton Samsonov <avscomputing(a)gmail.com> Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: David Hendricks <david.hendricks(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: flashchips: Add Spansion S25FS512S
by Anastasia Klimchuk (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Anton Samsonov, Anton Samsonov, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Anton Samsonov. ( https://review.coreboot.org/c/flashrom/+/85585?usp=email ) Change subject: flashchips: Add Spansion S25FS512S ...................................................................... Patch Set 2: (4 comments) Patchset: PS2: Thank you for patch! I read your comment in the other patch and so good that you were able to resolve identification issue. File flashchips/spansion.c: https://review.coreboot.org/c/flashrom/+/85585/comment/20d5f7aa_22bdde29?us… : PS2, Line 1043: /* Note on FEATURE_4BA_ENTER: the command set only defines command 0xB7 : to enter 4BA mode, but there is no counterpart command "Exit 4BA mode", : and the code 0xE9 is assigned to another command ("Password unlock"). */ : /* Note on FEATURE_4BA_ENTER_EAR7 (not set): the "Extended address mode" bit : is stored in configuration register CR2V[7], which can only be read / written : by generic commands ("Read any register" / "Write any register"), that itself : expect a 3- or 4-byte address of register in question, which in turn depends on : the same register, that is initially set from non-volatile register CR2NV[7] : that defaults to 0, but can be programmed to 1 for starting in 32-bit mode. */ Thank you for sharing bits of wisdom in the comment! File include/flashchips.h: https://review.coreboot.org/c/flashrom/+/85585/comment/cefeb7bf_02b64c15?us… : PS2, Line 717: 0x02200081 Can I ask, for my education, I want to understand how this ID is formed, `02200081` I am looking at page 136 (12.3.1 Device ID) and I can see, by byte address 05h -> 81 (last two digits) 04h -> 00 03h -> ?? where it is? 02h -> 20 01h -> 02 00h is also not here, but that's manufacturer ID, not device ID, maybe it's fine https://review.coreboot.org/c/flashrom/+/85585/comment/cd64a820_7b77a7ad?us… : PS2, Line 717: SPANSION_S25FS512S Maybe let's follow the pattern and name this macro to `SPANSION_S25FS512S_UL` -- To view, visit https://review.coreboot.org/c/flashrom/+/85585?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: I40b6c081ec7d57eac4f6d2b69cea3878bc92bb47 Gerrit-Change-Number: 85585 Gerrit-PatchSet: 2 Gerrit-Owner: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anton Samsonov <devel(a)zxlab.ru> 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: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Thu, 19 Dec 2024 08:26:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[1.5.x]: doc: Add section about v1.5.1 into release notes
by Anastasia Klimchuk (Code Review) 18 Dec '24

18 Dec '24

Attention is currently required from: Matt DeVillier. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85655?usp=email ) Change subject: doc: Add section about v1.5.1 into release notes ...................................................................... Patch Set 1: (1 comment) File doc/release_notes/v_1_5.rst: https://review.coreboot.org/c/flashrom/+/85655/comment/a3134928_f47ae8d7?us… : PS1, Line 34: <TODO ADD WHO IS AFFECTED> Matt, do you remember we discussed that you can help with writing this piece? :) I appreciate your help so much, You can write as a comment here, or you upload new patchset, whatever you prefer. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/85655?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: 1.5.x Gerrit-Change-Id: I80f8423133bf779093d57ea6928f09d9d377d20e Gerrit-Change-Number: 85655 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Wed, 18 Dec 2024 11:40:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[1.5.x]: doc: Add section about v1.5.1 into release notes
by Anastasia Klimchuk (Code Review) 18 Dec '24

18 Dec '24

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/85655?usp=email ) Change subject: doc: Add section about v1.5.1 into release notes ...................................................................... doc: Add section about v1.5.1 into release notes Change-Id: I80f8423133bf779093d57ea6928f09d9d377d20e Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/release_notes/v_1_5.rst 1 file changed, 19 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/85655/1 diff --git a/doc/release_notes/v_1_5.rst b/doc/release_notes/v_1_5.rst index 6e81353..fce8b16 100644 --- a/doc/release_notes/v_1_5.rst +++ b/doc/release_notes/v_1_5.rst @@ -15,10 +15,26 @@ Anonymous checkout from the git repository at https://review.coreboot.org/flashrom.git (tag v1.5.0) -A tarball is available for download at <TODO add tarball> -(signature <TODO add signature>) +A tarball is available for download at https://download.flashrom.org/releases/flashrom-v1.5.0.tar.xz +(signature https://download.flashrom.org/releases/flashrom-v1.5.0.tar.xz.asc) -fingerprint: <TODO add fingerprint> +fingerprint: 6E6E F9A0 BA47 8006 E277 6E4C C037 BB41 3134 D111 + +Point release v1.5.1 +==================== + +v1.5.1 includes one bug fix in addition to everything which is included in v1.5.0 + +Ticket: https://ticket.coreboot.org/issues/573 + +Patch: https://review.coreboot.org/c/flashrom/+/85612 + +Who exactly can be affected by this bug: + +<TODO ADD WHO IS AFFECTED> + +v1.5.1 can be downloaded in the same location, same fingerprint. Tarball name will be ``flashrom-v1.5.1.tar.xz``. +Or, checkout from the same git repository by the tag v1.5.1 Known issues ============ -- To view, visit https://review.coreboot.org/c/flashrom/+/85655?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: 1.5.x Gerrit-Change-Id: I80f8423133bf779093d57ea6928f09d9d377d20e Gerrit-Change-Number: 85655 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[1.5.x]: ichspi: Check whether chipset is locked when probing opcode
by Anastasia Klimchuk (Code Review) 18 Dec '24

18 Dec '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/85612?usp=email ) Change subject: ichspi: Check whether chipset is locked when probing opcode ...................................................................... ichspi: Check whether chipset is locked when probing opcode This is follow up (or fix) for commit 26a1eb514ccefc61b110068cf0eea73c397ba045 When probing opcode, all opcodes in POSSIBLE_OPCODES are reported as supported, even if the opcode is not in curopcodes. This is relying on reprogramming on-the-fly to handle the gap between POSSIBLE_OPCODES and curopcodes. However, for locked chipsets on-the-fly is not happening (is not possible, since list of opcodes is locked), so we can't rely on it. So, we need to check whether chipset is locked. Ticket: https://ticket.coreboot.org/issues/573 Change-Id: I701a86f030cfef43a1158bf075287ade569254e6 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Tested-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/85612 Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M ichspi.c 1 file changed, 6 insertions(+), 2 deletions(-) Approvals: Peter Marheine: Looks good to me, approved build bot (Jenkins): Verified Matt DeVillier: Looks good to me, but someone else must approve Stefan Reinauer: Looks good to me, approved diff --git a/ichspi.c b/ichspi.c index fc994db..94227de 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1823,8 +1823,12 @@ static bool ich_spi_probe_opcode(const struct flashctx *flash, uint8_t opcode) { int ret = find_opcode(curopcodes, opcode); - if ((ret == -1) && (lookup_spi_type(opcode) <= 3)) - /* opcode is in POSSIBLE_OPCODES, report supported. */ + if ((ret == -1) && (lookup_spi_type(opcode) <= 3) && (!ichspi_lock)) + /* opcode is in POSSIBLE_OPCODES, report supported. + * Relying on reprogramming on-the-fly + * when opcode is not in curopcodes, but is in POSSIBLE_OPCODES. + * + * on-the-fly does not work for locked chipsets, therefore checking ichspi_lock. */ return true; return ret >= 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/85612?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: 1.5.x Gerrit-Change-Id: I701a86f030cfef43a1158bf075287ade569254e6 Gerrit-Change-Number: 85612 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in flashrom[main]: ichspi: Check whether chipset is locked when probing opcode
by Anastasia Klimchuk (Code Review) 18 Dec '24

18 Dec '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/85592?usp=email ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: ichspi: Check whether chipset is locked when probing opcode ...................................................................... ichspi: Check whether chipset is locked when probing opcode This is follow up (or fix) for commit 26a1eb514ccefc61b110068cf0eea73c397ba045 When probing opcode, all opcodes in POSSIBLE_OPCODES are reported as supported, even if the opcode is not in curopcodes. This is relying on reprogramming on-the-fly to handle the gap between POSSIBLE_OPCODES and curopcodes. However, for locked chipsets on-the-fly is not happening (is not possible, since list of opcodes is locked), so we can't rely on it. So, we need to check whether chipset is locked. Ticket: https://ticket.coreboot.org/issues/573 Change-Id: I701a86f030cfef43a1158bf075287ade569254e6 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Tested-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/85592 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nikolai Artemiev <nartemiev(a)google.com> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M ichspi.c 1 file changed, 6 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, but someone else must approve Stefan Reinauer: Looks good to me, approved Nikolai Artemiev: Looks good to me, approved diff --git a/ichspi.c b/ichspi.c index fc994db..94227de 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1823,8 +1823,12 @@ static bool ich_spi_probe_opcode(const struct flashctx *flash, uint8_t opcode) { int ret = find_opcode(curopcodes, opcode); - if ((ret == -1) && (lookup_spi_type(opcode) <= 3)) - /* opcode is in POSSIBLE_OPCODES, report supported. */ + if ((ret == -1) && (lookup_spi_type(opcode) <= 3) && (!ichspi_lock)) + /* opcode is in POSSIBLE_OPCODES, report supported. + * Relying on reprogramming on-the-fly + * when opcode is not in curopcodes, but is in POSSIBLE_OPCODES. + * + * on-the-fly does not work for locked chipsets, therefore checking ichspi_lock. */ return true; return ret >= 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/85592?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: I701a86f030cfef43a1158bf075287ade569254e6 Gerrit-Change-Number: 85592 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> 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>

1 0

[XS] Change in flashrom[1.5.x]: ichspi: Check whether chipset is locked when probing opcode
by Stefan Reinauer (Code Review) 18 Dec '24

18 Dec '24

Attention is currently required from: Anastasia Klimchuk, Bill XIE. Stefan Reinauer has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85612?usp=email ) Change subject: ichspi: Check whether chipset is locked when probing opcode ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/85612?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: 1.5.x Gerrit-Change-Id: I701a86f030cfef43a1158bf075287ade569254e6 Gerrit-Change-Number: 85612 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 18 Dec 2024 04:48:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[1.5.x]: ichspi: Check whether chipset is locked when probing opcode
by Peter Marheine (Code Review) 18 Dec '24

18 Dec '24

Attention is currently required from: Anastasia Klimchuk, Bill XIE, Stefan Reinauer. Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85612?usp=email ) Change subject: ichspi: Check whether chipset is locked when probing opcode ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/85612?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: 1.5.x Gerrit-Change-Id: I701a86f030cfef43a1158bf075287ade569254e6 Gerrit-Change-Number: 85612 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 18 Dec 2024 03:57:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2024