October 2023 - flashrom-gerrit

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

25 Oct '23

Attention is currently required from: Edward O'Callaghan, Jes Klinke, Nikolai Artemiev. Hello Edward O'Callaghan, Nikolai Artemiev, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/77999?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: raiden: Support target index with generic REQ_ENABLE ...................................................................... raiden: Support target index with generic REQ_ENABLE Some devices such as the GSC knows how it is wired to AP and EC flash chips, and can be told which specific device to talk to. Other devices such as Servo Micro and HyperDebug are generic, and do not know how they are wired, the caller is responsible for first configure the appropriate MUXes or buffers, and then tell the debugger which port to use (Servo Micro has just one SPI port, HyperDebug is the first that has multiple). The Raiden protocol allows both the cases of USB devices knowing their wiring and not. If I were to declare the protocol in Rust, this is how the information of the Raiden protocol "enable request" would be encoded: ``` enum { EnableGeneric(u8), EnableAp, EnableEc, ... } ``` The first label `EnableGeneric(u8)` is to be used with HyperDebug that does not know how its ports are wired, and allow access by index. The other labels `EnableAp` and `EnableEc` are to be used with the GSC. The actual transmission of the enum above uses the bRequest and low byte of wValue of a USB control request, but that is a detail and not conceptually important. Until now, `-p raiden_debug_spi:target=AP` or `...:target=EC` could be used to make flashrom use `EnableAp` or `EnableEc`, and if neither was given, it would default to `EnableGeneric`, which now that wValue is used means `EnableGeneric(0)`. I find it rather straight-forward, that `-p raiden_debug_spi:target=1`, `...:target=2`, etc. should translate to `EnableGeneric(1)`, etc. This patchset achieves this, by adding a second 16-bit parameter value, next to request_enable. I have tested that flashrom can detect the same Winbond flash chip "W25Q128.V..M" with two different Raiden USB devices as below. TEST=flashrom -p raiden_debug_spi:serial=0701B044-91AC3132,target=AP TEST=flashrom -p raiden_debug_spi:serial=205635783236,target=1 Signed-off-by: Jes B. Klinke <jbk(a)chromium.org> Change-Id: I03bf4f3210186fb5937b42e298761907b03e08b7 --- M doc/classic_cli_manpage.rst M raiden_debug_spi.c 2 files changed, 25 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/77999/4 -- 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: 4 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: Jes Klinke <jbk(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

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

25 Oct '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/78186?usp=email ) ( 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/78186 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M chipset_enable.c M ich_descriptors.c M ichspi.c M include/programmer.h 4 files changed, 46 insertions(+), 8 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/chipset_enable.c b/chipset_enable.c index 3846ecf..1c24b92 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -601,6 +601,7 @@ break; case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -712,6 +713,7 @@ break; case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_METEOR_LAKE: boot_straps = boot_straps_pch500; break; @@ -987,6 +989,11 @@ return enable_flash_pch100_or_c620(cfg, dev, name, 0x1f, 5, CHIPSET_C620_SERIES_LEWISBURG); } +static int enable_flash_c740(const struct programmer_cfg *cfg, struct pci_dev *const dev, const char *const name) +{ + return enable_flash_pch100_or_c620(cfg, dev, name, 0x1f, 5, CHIPSET_C740_SERIES_EMMITSBURG); +} + static int enable_flash_pch300(const struct programmer_cfg *cfg, struct pci_dev *const dev, const char *const name) { return enable_flash_pch100_or_c620(cfg, dev, name, 0x1f, 5, CHIPSET_300_SERIES_CANNON_POINT); @@ -2123,7 +2130,7 @@ {0x8086, 0xa247, B_S, NT, "Intel", "C620 Series Chipset Supersku", enable_flash_c620}, {0x8086, 0xa248, B_S, NT, "Intel", "C620 Series Chipset Supersku", enable_flash_c620}, {0x8086, 0xa249, B_S, NT, "Intel", "C620 Series Chipset Supersku", enable_flash_c620}, - {0x8086, 0x1bca, B_S, NT, "Intel", "Emmitsburg Chipset SKU", enable_flash_c620}, + {0x8086, 0x1bca, B_S, DEP, "Intel", "Emmitsburg Chipset SKU", enable_flash_c740}, {0x8086, 0xa2c4, B_S, NT, "Intel", "H270", enable_flash_pch100}, {0x8086, 0xa2c5, B_S, NT, "Intel", "Z270", enable_flash_pch100}, {0x8086, 0xa2c6, B_S, NT, "Intel", "Q270", enable_flash_pch100}, diff --git a/ich_descriptors.c b/ich_descriptors.c index c3d062e..7bdca61 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -44,6 +44,7 @@ case CHIPSET_GEMINI_LAKE: return 6; case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -75,6 +76,7 @@ { switch (cs) { case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_APOLLO_LAKE: case CHIPSET_600_SERIES_ALDER_POINT: case CHIPSET_METEOR_LAKE: @@ -117,9 +119,9 @@ "5 series Ibex Peak", "6 series Cougar Point", "7 series Panther Point", "8 series Lynx Point", "Baytrail", "8 series Lynx Point LP", "8 series Wellsburg", "9 series Wildcat Point", "9 series Wildcat Point LP", "100 series Sunrise Point", - "C620 series Lewisburg", "300 series Cannon Point", "400 series Comet Point", - "500 series Tiger Point", "600 series Alder Point", "Meteor Lake", - "Apollo Lake", "Gemini Lake", "Jasper Lake", "Elkhart Lake", + "C620 series Lewisburg", "C740 series Emmitsburg", "300 series Cannon Point", + "400 series Comet Point", "500 series Tiger Point", "600 series Alder Point", + "Meteor Lake", "Apollo Lake", "Gemini Lake", "Jasper Lake", "Elkhart Lake", }; if (cs < CHIPSET_ICH8 || cs - CHIPSET_ICH8 + 1 >= ARRAY_SIZE(chipset_names)) cs = 0; @@ -211,6 +213,7 @@ case CHIPSET_9_SERIES_WILDCAT_POINT_LP: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -313,6 +316,7 @@ return freq_str[2][value]; case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_METEOR_LAKE: return freq_str[3][value]; case CHIPSET_ELKHART_LAKE: @@ -357,6 +361,7 @@ switch (cs) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -472,15 +477,27 @@ void prettyprint_ich_descriptor_master(const enum ich_chipset cs, const struct ich_descriptors *const desc) { ssize_t i; - const ssize_t nm = ich_number_of_masters(cs, &desc->content); + ssize_t nm = ich_number_of_masters(cs, &desc->content); msg_pdbg2("=== Master Section ===\n"); if (nm < 0) { msg_pdbg2("%s: number of masters too high (%d).\n", __func__, desc->content.NM + 1); return; } + if (cs == CHIPSET_C740_SERIES_EMMITSBURG) { + /* + * The SPI programming guide says there are 6 masters (thus NM=6), but it + * 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, this field probably was not updated when writing the + * document. + * Hardcode to 5 to be compatible with '500 Series PCH' below. + */ + nm = 5; + } for (i = 0; i < nm; i++) msg_pdbg2("FLMSTR%zd 0x%08"PRIx32"\n", i + 1, desc->master.FLMSTRs[i]); + msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); @@ -489,6 +506,7 @@ cs == CHIPSET_400_SERIES_COMET_POINT || cs == CHIPSET_500_SERIES_TIGER_POINT || cs == CHIPSET_600_SERIES_ALDER_POINT || + cs == CHIPSET_C740_SERIES_EMMITSBURG || cs == CHIPSET_JASPER_LAKE || cs == CHIPSET_METEOR_LAKE) { const char *const master_names[] = { "BIOS", "ME", "GbE", "DevE", "EC", @@ -1021,8 +1039,8 @@ warn_peculiar_desc("Gemini Lake"); return CHIPSET_GEMINI_LAKE; } - if (content->ISL <= 80) - return CHIPSET_C620_SERIES_LEWISBURG; + if (content->ISL == 0x50) + return CHIPSET_C740_SERIES_EMMITSBURG; warn_peculiar_desc("Ibex Peak"); return CHIPSET_5_SERIES_IBEX_PEAK; } else if (upper->MDTBA == 0x00) { @@ -1095,6 +1113,7 @@ break; case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_APOLLO_LAKE: if (component->modes.freq_read != 6) { msg_pwarn("\nThe flash descriptor looks like a Skylake/Sunrise Point descriptor.\n" @@ -1242,6 +1261,7 @@ case CHIPSET_9_SERIES_WILDCAT_POINT_LP: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -1287,6 +1307,7 @@ switch (cs) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: diff --git a/ichspi.c b/ichspi.c index 2e38ff1..36a7b3b 100644 --- a/ichspi.c +++ b/ichspi.c @@ -457,6 +457,7 @@ switch (ich_gen) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -470,6 +471,7 @@ switch (ich_gen) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -492,6 +494,7 @@ switch (ich_gen) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -2050,6 +2053,7 @@ switch (ich_gen) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -2087,6 +2091,7 @@ *num_freg = 10; break; case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: *num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */ break; case CHIPSET_300_SERIES_CANNON_POINT: @@ -2150,6 +2155,7 @@ switch (ich_gen) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -2230,6 +2236,7 @@ case CHIPSET_ICH8: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -2269,6 +2276,7 @@ case CHIPSET_ICH8: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_C740_SERIES_EMMITSBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: @@ -2310,7 +2318,8 @@ ich_gen == CHIPSET_300_SERIES_CANNON_POINT || ich_gen == CHIPSET_400_SERIES_COMET_POINT || ich_gen == CHIPSET_500_SERIES_TIGER_POINT || - ich_gen == CHIPSET_600_SERIES_ALDER_POINT)) { + ich_gen == CHIPSET_600_SERIES_ALDER_POINT || + ich_gen == CHIPSET_C740_SERIES_EMMITSBURG)) { msg_pdbg("Enabling hardware sequencing by default for 100+ series PCH.\n"); ich_spi_mode = ich_hwseq; } diff --git a/include/programmer.h b/include/programmer.h index 562f58a..15df0d4 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -354,6 +354,7 @@ CHIPSET_9_SERIES_WILDCAT_POINT_LP, CHIPSET_100_SERIES_SUNRISE_POINT, /* also 6th/7th gen Core i/o (LP) variants */ CHIPSET_C620_SERIES_LEWISBURG, + CHIPSET_C740_SERIES_EMMITSBURG, CHIPSET_300_SERIES_CANNON_POINT, CHIPSET_400_SERIES_COMET_POINT, CHIPSET_500_SERIES_TIGER_POINT, -- 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: 7 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(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-MessageType: merged

1 0

[XS] Change in flashrom[main]: ich_descriptors: Fix table printing
by Anastasia Klimchuk (Code Review) 25 Oct '23

25 Oct '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/78301?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/78301 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M ich_descriptors.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified 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: 4 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in flashrom[main]: ich_descriptors: Fix debug print
by Anastasia Klimchuk (Code Review) 25 Oct '23

25 Oct '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/78300?usp=email ) 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 overlooked 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/78300 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ich_descriptors.c 1 file changed, 3 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved 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: 4 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in flashrom[main]: ich_descriptors: Fix debug print
by Anastasia Klimchuk (Code Review) 24 Oct '23

24 Oct '23

Attention is currently required from: Patrick Rudolph. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78300?usp=email ) Change subject: ich_descriptors: Fix debug print ...................................................................... Patch Set 3: Code-Review+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: 3 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Comment-Date: Tue, 24 Oct 2023 11:25:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

24 Oct '23

Attention is currently required from: Anastasia Klimchuk, Patrick Rudolph. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78300?usp=email ) Change subject: ich_descriptors: Fix debug print ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/78300/comment/569f4778_5f7ddabb : PS2, Line 9: 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. > You need to wrap commit message at 72 char max. […] Done -- 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: 3 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Comment-Date: Tue, 24 Oct 2023 06:18:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

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

24 Oct '23

Attention is currently required from: Patrick Rudolph, Patrick Rudolph. Hello Anastasia Klimchuk, Patrick Rudolph, build bot (Jenkins), 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 (#3). 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 overlooked 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/3 -- 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: 3 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

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

24 Oct '23

Attention is currently required from: Patrick Rudolph. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78301?usp=email ) Change subject: ich_descriptors: Fix table printing ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/78301/comment/daa34073_4927976e : PS2, Line 10: Convert "unknown" to "DevE" > Why it was "unknown" before? Is Device Expansion the names from SPI guides? Older chipsets and thus older versions of the SPI programming guide had less regions defined. Today there are 16 well known regions, even though they might not be actually used by the chipset. -- 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: 2 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Comment-Date: Tue, 24 Oct 2023 06:17:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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) 24 Oct '23

24 Oct '23

Attention is currently required from: David Hendricks, Kyösti Mälkki, Patrick Georgi, Patrick Rudolph, 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 5: (1 comment) Patchset: PS5: > The previous patches are just for pretty printing a table that is visible at verbosity=3. […] Yes I understand! But all 3 patches are modifying the same file (ich_descriptors.c) so the easier thing would be to merge the chain together. Besides, even if that's a pretty printing, but that's useful fixes and I definitely want to get all that through. And there's not much to do for the other two: just commit message format, and one more question was for my education :) Thank you! -- 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: 5 Gerrit-Owner: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(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: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Comment-Date: Tue, 24 Oct 2023 01:18:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

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

23 Oct '23

Attention is currently required from: Edward O'Callaghan, Jes Klinke, Nikolai Artemiev. Hello Edward O'Callaghan, Nikolai Artemiev, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/77999?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+2 by Nikolai Artemiev Change subject: raiden: Support target index with generic REQ_ENABLE ...................................................................... raiden: Support target index with generic REQ_ENABLE Some devices such as the GSC knows how it is wired to AP and EC flash chips, and can be told which specific device to talk to. Other devices such as Servo Micro and HyperDebug are generic, and do not know how they are wired, the caller is responsible for first configure the appropriate MUXes or buffers, and then tell the debugger which port to use (Servo Micro has just one SPI port, HyperDebug is the first that has multiple). The Raiden protocol allows both the cases of USB devices knowing their wiring and not. If I were to declare the protocol in Rust, this is how the information of the Raiden protocol "enable request" would be encoded: ``` enum { EnableGeneric(u8), EnableAp, EnableEc, ... } ``` The first label `EnableGeneric(u8)` is to be used with HyperDebug that does not know how its ports are wired, and allow access by index. The other labels `EnableAp` and `EnableEc` are to be used with the GSC. The actual transmission of the enum above uses the bRequest and low byte of wValue of a USB control request, but that is a detail and not conceptually important. Until now, `-p raiden_debug_spi:target=AP` or `...:target=EC` could be used to make flashrom use `EnableAp` or `EnableEc`, and if neither was given, it would default to `EnableGeneric`, which now that wValue is used means `EnableGeneric(0)`. I find it rather straight-forward, that `-p raiden_debug_spi:target=1`, `...:target=2`, etc. should translate to `EnableGeneric(1)`, etc. This patchset archieves this, by encoding the parameter to `EnableGeneric` as the second byte of the request_enable integer value. TEST=run flashrom with HyperDebug Signed-off-by: Jes B. Klinke <jbk(a)chromium.org> Change-Id: I03bf4f3210186fb5937b42e298761907b03e08b7 --- M raiden_debug_spi.c 1 file changed, 24 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/77999/3 -- 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: 3 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: Jes Klinke <jbk(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2023