October 2023 - flashrom-gerrit

[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

[XS] Change in flashrom[main]: ich_descriptors: Fix debug print
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/+/78300?usp=email to review the following change. Change subject: ich_descriptors: Fix debug print ...................................................................... ich_descriptors: Fix debug print Allow up nm to be qqual to ARRAY_SIZE(master_names). The loop below makes sure that it doesn't access master_names 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/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index a2a99bd..4b440ed 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -493,9 +493,9 @@ const char *const master_names[] = { "BIOS", "ME", "GbE", "unknown", "EC", }; - if (nm >= (ssize_t)ARRAY_SIZE(master_names)) { - msg_pdbg2("%s: number of masters too high (%d).\n", __func__, - desc->content.NM + 1); + + if (nm > (ssize_t)ARRAY_SIZE(master_names)) { + msg_pdbg2("%s: number of masters too high (%zd).\n", __func__, nm); return; } -- 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: 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

[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: 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 (#2). The following approvals got outdated and were removed: Code-Review+1 by Patrick Georgi, Verified+1 by build bot (Jenkins) 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, 43 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/78186/2 -- 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: 2 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: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[main]: raiden: Support target index with generic REQ_ENABLE
by Jes Klinke (Code Review) 11 Oct '23

11 Oct '23

Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan. Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77999?usp=email ) Change subject: raiden: Support target index with generic REQ_ENABLE ...................................................................... Patch Set 2: (3 comments) Patchset: PS2: I will address the smaller comments, once we have settled on the way ahead wrt. the meaning of `request_enable` and the `get_target()` signature. File raiden_debug_spi.c: https://review.coreboot.org/c/flashrom/+/77999/comment/8065aaee_1d660c98 : PS2, Line 1596: request_enable & 0xFF > Does this still work for target values EC and AP ? See below. Yes, the EC and AP values fit inside 8 bits. (The bRequest parameter here is `uint8_t`, so the `& 0xFF` is actually a no-op included for clarity, as with my change `request_enable` can for the first time be non-zero in the second byte.) https://review.coreboot.org/c/flashrom/+/77999/comment/9cdbcfbd_03d2d743 : PS2, Line 1596: request_enable & 0xFF, : (request_enable >> 8) & 0xFF, > A question: why do you initialise `request_enable` to a value that you never use by itself, just as […] It was already the case that some Raiden devices had multiple SPI ports, or at least knew multiple ways of controlling electrical mux'es or buffers. These SPI ports would be known as "ap" or "ec", and particular values of bRequest was used to access one or the other. I have been developing firmware for another debugger using the Raiden protocol, which has three or more SPI ports, and they do not have any particular names, just numbers. I have extended the Raiden protocol such that the lower byte of `wValue` can indicate which one of such an array of similar SPI ports to use (while bRequest is set to the "default" value), and the goal of this CL is to be able to access that functionality. The `int get_target()` method used to return a 8-bit value to be passed in the `bRequest` field of the control request to "open" the SPI port (while the wValue field was always set to zero). In a somewhat hacky way, I used the upper bits of the return value from `get_target()` to convey the desired value of wValue. Perhaps I should instead change the signature into `void get_target(const struct programmer_cfg *cfg, uint8_t *request_enable, uint16_t *request_parameter)` such that the function can "return" two separate values. Let me know if you prefer that, and I can modify my CL. -- To view, visit https://review.coreboot.org/c/flashrom/+/77999?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: I03bf4f3210186fb5937b42e298761907b03e08b7 Gerrit-Change-Number: 77999 Gerrit-PatchSet: 2 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Oct 2023 19:35:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2023