October 2023 - flashrom-gerrit

[M] Change in flashrom[main]: ichspi: Add support for C740 PCH
by Patrick Rudolph (Code Review) 16 Oct '23

16 Oct '23

Attention is currently required from: Anastasia Klimchuk, David Hendricks, Kyösti Mälkki, Patrick Georgi, Patrick Rudolph. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78186?usp=email ) Change subject: ichspi: Add support for C740 PCH ...................................................................... Patch Set 4: (3 comments) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/78186/comment/fee8885f_f805e7fe : PS4, Line 1024: if (content->ISL <= 80) : return CHIPSET_C620_SERIES_LEWISBURG > I am a bit confused, what happens with support for LEWISBURG ? Why are you just removing it? […] It's not removed. This code path is never taken on C620 as ICCRIBA==0x34. C620 is still being detected in the code path below. https://review.coreboot.org/c/flashrom/+/78186/comment/23abf067_6866f2a2 : PS4, Line 490: to be compatible with 500 Series PCH below > Maybe "to align with the SPI programming guide, which also conveniently makes it compatible with 500 […] It says there are 6 masters (thus NM=6), but can only name 5. If there's a 6th then it's undocumented. However the first 5 are matching '500 Series PCH' and since C740 is a 500 Series clone Intel probably only forget to update this field when writing the document. So it's not aligning with the SPI programming guide. It's more like fixing it? https://review.coreboot.org/c/flashrom/+/78186/comment/f39d3c5f_d857ef98 : PS4, Line 1039: return CHIPSET_C740_SERIES_EMMITSBURG; > For Lewisburg the ISL value should be 0x8b (139) (see also: CB:57581). […] This code path is never taken on C620 as ICCRIBA==0x34 -- To view, visit https://review.coreboot.org/c/flashrom/+/78186?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: I80eebc0fcc14de9df823aceaee77870ad136f94a Gerrit-Change-Number: 78186 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Comment-Date: Mon, 16 Oct 2023 06:34:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: David Hendricks <david.hendricks(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: ichspi: Add support for C740 PCH
by Anastasia Klimchuk (Code Review) 16 Oct '23

16 Oct '23

Attention is currently required from: Kyösti Mälkki, Patrick Georgi, Patrick Rudolph. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78186?usp=email ) Change subject: ichspi: Add support for C740 PCH ...................................................................... Patch Set 4: (3 comments) Patchset: PS4: Thanks for the patch! Jenkins fail looks temporary, should go away next time you rebase. File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/78186/comment/ab26b585_493fe6e8 : PS4, Line 1024: if (content->ISL <= 80) : return CHIPSET_C620_SERIES_LEWISBURG I am a bit confused, what happens with support for LEWISBURG ? Why are you just removing it? I remember there were lots of debates two years ago, I tried to follow the chain of patches but there are lots of abandoned ones. I can understand this line might still need a fix. But it would be in a separate commit anyway? And if it's possible to fix, would be great to fix, instead of removing ? https://review.coreboot.org/c/flashrom/+/78186/comment/5a27e98e_22bc5d8b : PS4, Line 490: to be compatible with 500 Series PCH below Maybe "to align with the SPI programming guide, which also conveniently makes it compatible with 500 Series PCH below" ? If SPI guide mentions 6 masters, you would keep it at 6 ? -- To view, visit https://review.coreboot.org/c/flashrom/+/78186?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: I80eebc0fcc14de9df823aceaee77870ad136f94a Gerrit-Change-Number: 78186 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Comment-Date: Mon, 16 Oct 2023 02:57:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: ichspi: Add support for C740 PCH
by David Hendricks (Code Review) 16 Oct '23

16 Oct '23

Attention is currently required from: Anastasia Klimchuk, Kyösti Mälkki, Patrick Georgi, Patrick Rudolph. David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78186?usp=email ) Change subject: ichspi: Add support for C740 PCH ...................................................................... Patch Set 4: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/78186/comment/49ca7976_35ea16fb : PS4, Line 1039: return CHIPSET_C740_SERIES_EMMITSBURG; For Lewisburg the ISL value should be 0x8b (139) (see also: CB:57581). That matches the value seen when running `ifdtool -d` on the latest (2023.3 IPU) BKC images as well as the PCH strap length value mentioned in the C620 SPI PG (doc #559021). Not sure if it's better to fix it here or in another patch. Also, there was some discussion a while ago as to whether we should use `==` or `<=` for these comparisons, and IIRC the consensus at the time was to use '<=`. Anyway, the change you've made here for EBG looks fine, but we should probably correct the LBG value rather than removing it. -- To view, visit https://review.coreboot.org/c/flashrom/+/78186?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: I80eebc0fcc14de9df823aceaee77870ad136f94a Gerrit-Change-Number: 78186 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Comment-Date: Mon, 16 Oct 2023 00:08:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashchips: Add GD25Q128E name to the GD25Q127C/GD25Q128C entry
by Hsuan-ting Chen (Code Review) 13 Oct '23

13 Oct '23

Hsuan-ting Chen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/78348?usp=email ) Change subject: flashchips: Add GD25Q128E name to the GD25Q127C/GD25Q128C entry ...................................................................... flashchips: Add GD25Q128E name to the GD25Q127C/GD25Q128C entry Q128E and Q127C/Q128C have compatible main functions, their differences are: * Q128E uses 55 nm process, while Q127C/Q128C use 65nm * Q128E does not support QPI * Q128E have OTP: 3072B, while Q127C/Q128C are 1536B * Q128E's fast read clock frequency is 133MHz, while Q127C/Q128C are 104MHZ We also tested that Q128E could pass flashrom_tester while probing it as 127C/128C, so the main functionalities are compatible. Change the chip name from GD25Q127C/GD25Q128C to GD25Q127C/GD25Q128C/GD25Q128E to make it more accurate. Chip revision history: - The 'GD25Q127C/GD25Q128C' definition was added in `commit e0c7abf219b81ad049d09a4671ebc9196153d308` as 'GD25Q128C' and later renamed to 'GD25Q127C/GD25Q128C' BUG=b:304863141, b:293545382 BRANCH=none TEST=flashrom_tester with flashrom binary could pass with Q128E Signed-off-by: Hsuan Ting Chen <roccochen(a)google.com> Change-Id: I3300671b1cf74b3ea0469b9c5a833489ab4914f5 --- M flashchips.c M include/flashchips.h 2 files changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/78348/1 diff --git a/flashchips.c b/flashchips.c index e156ffb..3b73ba9 100644 --- a/flashchips.c +++ b/flashchips.c @@ -6792,13 +6792,14 @@ { .vendor = "GigaDevice", - .name = "GD25Q127C/GD25Q128C", + .name = "GD25Q127C/GD25Q128C/GD25Q128E", .bustype = BUS_SPI, .manufacture_id = GIGADEVICE_ID, .model_id = GIGADEVICE_GD25Q128, .total_size = 16384, .page_size = 256, /* OTP: 1536B total; read 0x48; write 0x42, erase 0x44 */ + /* FIXME: 128E's OTP: 3072B total and doesn't support QPI */ /* QPI: enable 0x38, disable 0xFF */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_WRSR2, .tested = TEST_OK_PREWB, diff --git a/include/flashchips.h b/include/flashchips.h index 962e65e..04dc9b0 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -390,7 +390,7 @@ #define GIGADEVICE_GD25Q16 0x4015 /* Same as GD25Q16B (which has OTP) */ #define GIGADEVICE_GD25Q32 0x4016 /* Same as GD25Q32B */ #define GIGADEVICE_GD25Q64 0x4017 /* Same as GD25Q64B */ -#define GIGADEVICE_GD25Q128 0x4018 /* GD25Q128B and GD25Q128C only, can be distinguished by SFDP */ +#define GIGADEVICE_GD25Q128 0x4018 /* Same as GD25Q128B, GD25B128B, GD25Q127C, and GD25Q128E, can be distinguished by SFDP */ #define GIGADEVICE_GD25Q256D 0x4019 #define GIGADEVICE_GD25VQ21B 0x4212 #define GIGADEVICE_GD25VQ41B 0x4213 /* Same as GD25VQ40C, can be distinguished by SFDP */ -- To view, visit https://review.coreboot.org/c/flashrom/+/78348?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: I3300671b1cf74b3ea0469b9c5a833489ab4914f5 Gerrit-Change-Number: 78348 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[main]: flashchips: Add write-protect support for MXIC MX25U25643G
by Anastasia Klimchuk (Code Review) 13 Oct '23

13 Oct '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/78142?usp=email ) Change subject: flashchips: Add write-protect support for MXIC MX25U25643G ...................................................................... flashchips: Add write-protect support for MXIC MX25U25643G The MX25U25643G has a WPSEL bit in the security register, but the MX25U25635F does not. Therefore, take them apart. We have tested --wp-enable, --wp-disable, --wp-list and --wp-range commands for write-protect feature. The MX25U25643G has been tested by ch341a programmer : read, write, erase and wp. MX25U25643G datasheet is available at the following URL: https://www.mxic.com.tw/en-us/products/NOR-Flash/Serial-NOR-Flash/Pages/spe… MX25U25635F datasheet is available at the following URL: https://www.macronix.com/en-us/products/NOR-Flash/Serial-NOR-Flash/Pages/sp… Change-Id: I43de9ed123b9736c04d070754bcf9c32be5697ad Signed-off-by: rogeryou <rogeryou(a)mxic.com.cn> Reviewed-on: https://review.coreboot.org/c/flashrom/+/78142 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashchips.c 1 file changed, 58 insertions(+), 1 deletion(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified diff --git a/flashchips.c b/flashchips.c index db20cba..e156ffb 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10207,7 +10207,7 @@ { .vendor = "Macronix", - .name = "MX25U25635F/MX25U25643G", + .name = "MX25U25635F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, .model_id = MACRONIX_MX25U25635F, @@ -10256,6 +10256,63 @@ { .vendor = "Macronix", + .name = "MX25U25643G", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25U25635F, + .total_size = 32768, + .page_size = 256, + /* OTP: 512B total; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_4BA | FEATURE_CFGR | FEATURE_SCUR, + .tested = TEST_OK_PREWB, + .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, + } + }, + /* TODO: security register */ + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit6 is quad enable */ + .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, + .write = SPI_CHIP_WRITE256, /* Multi I/O supported */ + .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {1650, 2000}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {CONFIG, 3, OTP}, + .wps = {SECURITY, 7, OTP}, /* This bit is set by WPSEL command */ + }, + .decode_range = DECODE_RANGE_SPI25, + }, + + { + .vendor = "Macronix", .name = "MX25U3235E/F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, -- To view, visit https://review.coreboot.org/c/flashrom/+/78142?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: I43de9ed123b9736c04d070754bcf9c32be5697ad Gerrit-Change-Number: 78142 Gerrit-PatchSet: 6 Gerrit-Owner: rogeryou 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[main]: ichspi: Add support for C740 PCH
by Patrick Rudolph (Code Review) 12 Oct '23

12 Oct '23

Attention is currently required from: Kyösti Mälkki, Patrick Georgi, Patrick Rudolph. Hello Patrick Georgi, Patrick Rudolph, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/78186?usp=email to look at the new patch set (#4). Change subject: ichspi: Add support for C740 PCH ...................................................................... ichspi: Add support for C740 PCH Clean commit 51e1d0e4b7670e5822560acc724a6a8dd00b6af4 'Add support for Intel Emmitsburg PCH' which broke CHIPSET_5_SERIES_IBEX_PEAK detection and which assumes C740 is the same as C620, while its more a close relative to Intel's H570 PCH. Based on Intel SPI Programming Guide #619386. Test: Run on Intel ArcherCity CRB with Intel's C741 PCH using the 'internal' programmer. Test: Run on BMC and accessed the SPI flash chip over 'linux_mtd' programmer. Change-Id: I80eebc0fcc14de9df823aceaee77870ad136f94a Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M chipset_enable.c M ich_descriptors.c M ichspi.c M include/programmer.h 4 files changed, 42 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/78186/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/78186?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: I80eebc0fcc14de9df823aceaee77870ad136f94a Gerrit-Change-Number: 78186 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[main]: ichspi: Add support for C740 PCH
by Patrick Rudolph (Code Review) 12 Oct '23

12 Oct '23

Attention is currently required from: Kyösti Mälkki, Patrick Georgi, Patrick Rudolph. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78186?usp=email ) Change subject: ichspi: Add support for C740 PCH ...................................................................... Patch Set 3: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/78186/comment/44aa9632_5c8110fb : PS1, Line 1032: if (content->ISL == 0x50) > That's 80 in decimal and is hit with <= 80 above? That line wasn't present when I written the patch against v1.2. Used latest main for testing and fixed a couple of placed where C620 was being used on C740. -- To view, visit https://review.coreboot.org/c/flashrom/+/78186?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: I80eebc0fcc14de9df823aceaee77870ad136f94a Gerrit-Change-Number: 78186 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Comment-Date: Thu, 12 Oct 2023 14:15:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: ich_descriptors: Fix debug print
by Patrick Rudolph (Code Review) 12 Oct '23

12 Oct '23

Hello Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/78300?usp=email to look at the new patch set (#2). Change subject: ich_descriptors: Fix debug print ...................................................................... ich_descriptors: Fix debug print Allow nm, the number of flash masters, to be equal to ARRAY_SIZE(master_names). The previous logic was probably overlocked when ich_number_of_masters() was introduced. The loop below makes sure that it doesn't access the master_names array out of bounds. Change-Id: Ib9276a6c29952487db6e60fb583942c0f24cd6ef Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M ich_descriptors.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/00/78300/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/78300?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: Ib9276a6c29952487db6e60fb583942c0f24cd6ef Gerrit-Change-Number: 78300 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[main]: dmi: Scan in sysfs first
by Patrick Rudolph (Code Review) 12 Oct '23

12 Oct '23

Attention is currently required from: Patrick Rudolph. Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/78302?usp=email to review the following change. Change subject: dmi: Scan in sysfs first ...................................................................... dmi: Scan in sysfs first When not running on Windows scan the sysfs for DMI table 3 first. This is faster and less intrusive than accessing the memory directly. Needs tests on internal programmer. Change-Id: I0d0f461b64a6f6ed6c1a3a62d759b9c395ad0ec8 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M dmi.c 1 file changed, 51 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/78302/1 diff --git a/dmi.c b/dmi.c index 94e47fb..5a50705 100644 --- a/dmi.c +++ b/dmi.c @@ -30,6 +30,10 @@ #include <stdbool.h> #include <stdio.h> #include <stdlib.h> +#include <errno.h> +#if !IS_WINDOWS +#include <sys/stat.h> +#endif #include "flash.h" #include "hwaccess_physmap.h" @@ -41,6 +45,8 @@ /* Strings longer than 4096 in DMI are just insane. */ #define DMI_MAX_ANSWER_LEN 4096 +#define LINUX_DMI_SYSFS_TABLE_3 "/sys/firmware/dmi/entries/3-0/raw" + static bool g_has_dmi_support = false; bool dmi_is_supported(void) @@ -232,6 +238,45 @@ physunmap(dmi_table_mem, len); } + +#if !IS_WINDOWS +/* read the chassis type from a sysfs file */ +static int dmi_sysfs(int *is_laptop) +{ + size_t bytes_read; + uint8_t buf[6]; + struct stat s; + FILE *fp; + + msg_pdbg("Using sysfs DMI decoder.\n"); + + if (stat(LINUX_DMI_SYSFS_TABLE_3, &s) < 0) { + msg_pdbg("Cannot stat \"%s\": %s\n", LINUX_DMI_SYSFS_TABLE_3, + strerror(errno)); + return 1; + } + + if ((fp = fopen(LINUX_DMI_SYSFS_TABLE_3, "r")) == NULL) { + msg_perr("Cannot open %s\n", LINUX_DMI_SYSFS_TABLE_3); + return 1; + } + + clearerr(fp); + bytes_read = fread(buf, 1, sizeof(buf), fp); + if (feof(fp) || ferror(fp) || bytes_read < sizeof(buf)) { + msg_perr("Error occurred when reading %s\n", + LINUX_DMI_SYSFS_TABLE_3); + fclose(fp); + return 1; + } + fclose(fp); + + *is_laptop = dmi_chassis_type(buf[5]); + + return 0; +} +#endif + #if SM_SUPPORT static int smbios_decode(uint8_t *buf, int *is_laptop) { @@ -400,6 +445,12 @@ return; } +#if !IS_WINDOWS + /* Try to read sysfs first */ + if (!dmi_sysfs(is_laptop)) + return; +#endif + /* dmi_fill fills the dmi_strings array, and if possible set the is_laptop parameter. */ if (dmi_fill(is_laptop) != 0) return; -- To view, visit https://review.coreboot.org/c/flashrom/+/78302?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: I0d0f461b64a6f6ed6c1a3a62d759b9c395ad0ec8 Gerrit-Change-Number: 78302 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

[XS] Change in flashrom[main]: ich_descriptors: Fix table printing
by Patrick Rudolph (Code Review) 12 Oct '23

12 Oct '23

Attention is currently required from: Patrick Rudolph. Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/78301?usp=email to review the following change. Change subject: ich_descriptors: Fix table printing ...................................................................... ich_descriptors: Fix table printing The code uses "%-4s" to print the master names and thus assumes that all names have 4 characters or less. Convert "unknown" to "DevE", short for Device Expansion in order to properly display the table. Test: Run flashrom -p internal -VV Change-Id: I0d10e2771c7a27c1a73ed53a33e68a04eb9e1e00 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M ich_descriptors.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/78301/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index 4b440ed..c3d062e 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -491,7 +491,7 @@ cs == CHIPSET_600_SERIES_ALDER_POINT || cs == CHIPSET_JASPER_LAKE || cs == CHIPSET_METEOR_LAKE) { const char *const master_names[] = { - "BIOS", "ME", "GbE", "unknown", "EC", + "BIOS", "ME", "GbE", "DevE", "EC", }; if (nm > (ssize_t)ARRAY_SIZE(master_names)) { -- To view, visit https://review.coreboot.org/c/flashrom/+/78301?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: I0d10e2771c7a27c1a73ed53a33e68a04eb9e1e00 Gerrit-Change-Number: 78301 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2023