June 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: ich_descriptors: Revise detection for chipsets w/ ICCRIBA
by Nico Huber (Code Review) 17 Jun '21

17 Jun '21

Attention is currently required from: Michał Żygowski, Angel Pons. Hello Michał Żygowski, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55647 to look at the new patch set (#2). Change subject: ich_descriptors: Revise detection for chipsets w/ ICCRIBA ...................................................................... ich_descriptors: Revise detection for chipsets w/ ICCRIBA Detection based on ICCRIBA and FMSBA became a little messy lately. However, there's a new static difference: Since 300 series (Cannon Point), there is an MDTBA field in FLUMAP1 that has always been 0 (reserved) before. Taking this into account, we can relax the checks on ICCRIBA. Change-Id: I587ad1abe390843d4a9e74431b6fc4b63f8ba512 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ich_descriptors.c M ich_descriptors.h 2 files changed, 26 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/55647/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55647 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I587ad1abe390843d4a9e74431b6fc4b63f8ba512 Gerrit-Change-Number: 55647 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ich_descriptors.c: Fix PCH detection for Tiger Lake
by Nico Huber (Code Review) 17 Jun '21

17 Jun '21

Attention is currently required from: Michał Żygowski, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55577 ) Change subject: ich_descriptors.c: Fix PCH detection for Tiger Lake ...................................................................... Patch Set 1: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/55577/comment/d335c210_a4a756ae PS1, Line 955: CHIPSET_300_SERIES_CANNON_POINT > if it is not zero, then we have 300 series+ chipset > right? exactly. The whole thing became a bit more complex, so I went ahead and prepared things: CB:55647. Adding TGP on top should be much easier. I'd say if (ICCRIBA == 0x34) return 300_SERIES if ("CPU Soft Strap Offset from PMC Base" != 0x6c) warn return 500_series I wonder if the field is really shifted by 2 bits or if it's just Intels weird way to say the 2 least-significant bits are 0. -- To view, visit https://review.coreboot.org/c/flashrom/+/55577 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib61d432ab4eaf00aa4eef50d2844940e73b5cad6 Gerrit-Change-Number: 55577 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 17 Jun 2021 20:26:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors: Refactor read_ich_descriptors_from_dump()
by Nico Huber (Code Review) 17 Jun '21

17 Jun '21

Attention is currently required from: Michał Żygowski, Angel Pons. Hello Michał Żygowski, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/55646 to review the following change. Change subject: ich_descriptors: Refactor read_ich_descriptors_from_dump() ...................................................................... ich_descriptors: Refactor read_ich_descriptors_from_dump() Process the "upper map" early as it doesn't depend on the descriptor generation. This way, we can use it to guess the generation. Change-Id: Ia2786b762ccefdce31b63397119bd89879e887ff Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ich_descriptors.c 1 file changed, 17 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/55646/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index bf47ec0..4ae3a1b 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -1037,6 +1037,23 @@ desc->component.FLILL = dump[(getFCBA(&desc->content) >> 2) + 1]; desc->component.FLPB = dump[(getFCBA(&desc->content) >> 2) + 2]; + /* upper map */ + desc->upper.FLUMAP1 = dump[(UPPER_MAP_OFFSET >> 2) + 0]; + + /* VTL is 8 bits long. Quote from the Ibex Peak SPI programming guide: + * "Identifies the 1s based number of DWORDS contained in the VSCC + * Table. Each SPI component entry in the table is 2 DWORDS long." So + * the maximum of 255 gives us 127.5 SPI components(!?) 8 bytes each. A + * check ensures that the maximum offset actually accessed is available. + */ + if (len < getVTBA(&desc->upper) + (desc->upper.VTL / 2 * 8)) + return ICH_RET_OOB; + + for (i = 0; i < desc->upper.VTL/2; i++) { + desc->upper.vscc_table[i].JID = dump[(getVTBA(&desc->upper) >> 2) + i * 2 + 0]; + desc->upper.vscc_table[i].VSCC = dump[(getVTBA(&desc->upper) >> 2) + i * 2 + 1]; + } + if (*cs == CHIPSET_ICH_UNKNOWN) { *cs = guess_ich_chipset(&desc->content, &desc->component); prettyprint_ich_chipset(*cs); @@ -1056,23 +1073,6 @@ for (i = 0; i < nm; i++) desc->master.FLMSTRs[i] = dump[(getFMBA(&desc->content) >> 2) + i]; - /* upper map */ - desc->upper.FLUMAP1 = dump[(UPPER_MAP_OFFSET >> 2) + 0]; - - /* VTL is 8 bits long. Quote from the Ibex Peak SPI programming guide: - * "Identifies the 1s based number of DWORDS contained in the VSCC - * Table. Each SPI component entry in the table is 2 DWORDS long." So - * the maximum of 255 gives us 127.5 SPI components(!?) 8 bytes each. A - * check ensures that the maximum offset actually accessed is available. - */ - if (len < getVTBA(&desc->upper) + (desc->upper.VTL / 2 * 8)) - return ICH_RET_OOB; - - for (i = 0; i < desc->upper.VTL/2; i++) { - desc->upper.vscc_table[i].JID = dump[(getVTBA(&desc->upper) >> 2) + i * 2 + 0]; - desc->upper.vscc_table[i].VSCC = dump[(getVTBA(&desc->upper) >> 2) + i * 2 + 1]; - } - /* MCH/PROC (aka. North) straps */ if (len < getFMSBA(&desc->content) + desc->content.MSL * 4) return ICH_RET_OOB; -- To view, visit https://review.coreboot.org/c/flashrom/+/55646 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia2786b762ccefdce31b63397119bd89879e887ff Gerrit-Change-Number: 55646 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ich_descriptors: Revise detection for chipsets w/ ICCRIBA
by Nico Huber (Code Review) 17 Jun '21

17 Jun '21

Attention is currently required from: Michał Żygowski, Angel Pons. Hello Michał Żygowski, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/55647 to review the following change. Change subject: ich_descriptors: Revise detection for chipsets w/ ICCRIBA ...................................................................... ich_descriptors: Revise detection for chipsets w/ ICCRIBA Detection based on ICCRIBA and FMSBA became a little messy lately. However, there's a new static difference: Since 300 series (Cannon Point), there is a MDTBA field in FLUMAP1 that has always been 0 (reserved) before. Taking this into account, we can relax the checks on ICCRIBA. Change-Id: I587ad1abe390843d4a9e74431b6fc4b63f8ba512 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ich_descriptors.c M ich_descriptors.h 2 files changed, 26 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/55647/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index 4ae3a1b..da49326 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -921,9 +921,10 @@ /* * Guesses a minimum chipset version based on the maximum number of - * soft straps per generation. + * soft straps per generation and presence of the MIP base (MDTBA). */ -static enum ich_chipset guess_ich_chipset_from_content(const struct ich_desc_content *const content) +static enum ich_chipset guess_ich_chipset_from_content(const struct ich_desc_content *const content, + const struct ich_desc_upper_map *const upper) { if (content->ICCRIBA == 0x00) { if (content->MSL == 0 && content->ISL <= 2) @@ -942,25 +943,26 @@ } warn_peculiar_desc(true, "Ibex Peak"); return CHIPSET_5_SERIES_IBEX_PEAK; - } else if (content->ICCRIBA < 0x31 && content->FMSBA < 0x30) { - if (content->MSL == 0 && content->ISL <= 17) - return CHIPSET_BAYTRAIL; - if (content->MSL <= 1 && content->ISL <= 18) - return CHIPSET_6_SERIES_COUGAR_POINT; - if (content->MSL <= 1 && content->ISL <= 21) - return CHIPSET_8_SERIES_LYNX_POINT; - warn_peculiar_desc(true, "Wildcat Point"); - return CHIPSET_9_SERIES_WILDCAT_POINT; - } else if (content->ICCRIBA < 0x34) { - if (content->NM == 6) + } else if (upper->MDTBA == 0x0000) { + if (content->ICCRIBA < 0x31 && content->FMSBA < 0x30) { + if (content->MSL == 0 && content->ISL <= 17) + return CHIPSET_BAYTRAIL; + if (content->MSL <= 1 && content->ISL <= 18) + return CHIPSET_6_SERIES_COUGAR_POINT; + if (content->MSL <= 1 && content->ISL <= 21) + return CHIPSET_8_SERIES_LYNX_POINT; + warn_peculiar_desc(true, "Wildcat Point"); + return CHIPSET_9_SERIES_WILDCAT_POINT; + } + if (content->NM == 6) { + warn_peculiar_desc(content->ICCRIBA > 0x34, "C620 series"); return CHIPSET_C620_SERIES_LEWISBURG; + } + warn_peculiar_desc(content->ICCRIBA != 0x31, "100 series"); return CHIPSET_100_SERIES_SUNRISE_POINT; - } else if (content->ICCRIBA == 0x34) { - if (content->NM == 6) - return CHIPSET_C620_SERIES_LEWISBURG; - return CHIPSET_300_SERIES_CANNON_POINT; } else { - msg_pwarn("Unknown flash descriptor, assuming 300 series compatibility.\n"); + if (content->ICCRIBA != 0x34) + msg_pwarn("Unknown firmware descriptor, assuming 300 series compatibility.\n"); return CHIPSET_300_SERIES_CANNON_POINT; } } @@ -972,9 +974,10 @@ * tinction because of the dropped number of regions field (NR). */ static enum ich_chipset guess_ich_chipset(const struct ich_desc_content *const content, - const struct ich_desc_component *const component) + const struct ich_desc_component *const component, + const struct ich_desc_upper_map *const upper) { - const enum ich_chipset guess = guess_ich_chipset_from_content(content); + const enum ich_chipset guess = guess_ich_chipset_from_content(content, upper); switch (guess) { case CHIPSET_300_SERIES_CANNON_POINT: @@ -1055,7 +1058,7 @@ } if (*cs == CHIPSET_ICH_UNKNOWN) { - *cs = guess_ich_chipset(&desc->content, &desc->component); + *cs = guess_ich_chipset(&desc->content, &desc->component, &desc->upper); prettyprint_ich_chipset(*cs); } diff --git a/ich_descriptors.h b/ich_descriptors.h index 727a31b..274d0fe 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -521,7 +521,8 @@ struct { uint32_t VTBA :8, /* ME VSCC Table Base Address */ VTL :8, /* ME VSCC Table Length */ - :16; + :8, + MDTBA :8; /* MIP Descr. Table Base Addr. (new since Cannon Point/300) */ }; }; struct { -- To view, visit https://review.coreboot.org/c/flashrom/+/55647 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I587ad1abe390843d4a9e74431b6fc4b63f8ba512 Gerrit-Change-Number: 55647 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ich_descriptors: Drop some unnecessary `else` after `return`
by Nico Huber (Code Review) 17 Jun '21

17 Jun '21

Attention is currently required from: Michał Żygowski, Angel Pons. Hello Michał Żygowski, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/55645 to review the following change. Change subject: ich_descriptors: Drop some unnecessary `else` after `return` ...................................................................... ich_descriptors: Drop some unnecessary `else` after `return` Change-Id: Id739bc12832e3b441e8e7e1dcdcc4c05b260d7ad Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ich_descriptors.c 1 file changed, 8 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/55645/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index fc04a24..bf47ec0 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -928,13 +928,13 @@ if (content->ICCRIBA == 0x00) { if (content->MSL == 0 && content->ISL <= 2) return CHIPSET_ICH8; - else if (content->ISL <= 2) + if (content->ISL <= 2) return CHIPSET_ICH9; - else if (content->ISL <= 10) + if (content->ISL <= 10) return CHIPSET_ICH10; - else if (content->ISL <= 16) + if (content->ISL <= 16) return CHIPSET_5_SERIES_IBEX_PEAK; - else if (content->FLMAP2 == 0) { + if (content->FLMAP2 == 0) { if (content->ISL == 23) return CHIPSET_GEMINI_LAKE; warn_peculiar_desc(content->ISL != 19, "Apollo Lake"); @@ -945,22 +945,20 @@ } else if (content->ICCRIBA < 0x31 && content->FMSBA < 0x30) { if (content->MSL == 0 && content->ISL <= 17) return CHIPSET_BAYTRAIL; - else if (content->MSL <= 1 && content->ISL <= 18) + if (content->MSL <= 1 && content->ISL <= 18) return CHIPSET_6_SERIES_COUGAR_POINT; - else if (content->MSL <= 1 && content->ISL <= 21) + if (content->MSL <= 1 && content->ISL <= 21) return CHIPSET_8_SERIES_LYNX_POINT; warn_peculiar_desc(true, "Wildcat Point"); return CHIPSET_9_SERIES_WILDCAT_POINT; } else if (content->ICCRIBA < 0x34) { if (content->NM == 6) return CHIPSET_C620_SERIES_LEWISBURG; - else - return CHIPSET_100_SERIES_SUNRISE_POINT; + return CHIPSET_100_SERIES_SUNRISE_POINT; } else if (content->ICCRIBA == 0x34) { if (content->NM == 6) return CHIPSET_C620_SERIES_LEWISBURG; - else - return CHIPSET_300_SERIES_CANNON_POINT; + return CHIPSET_300_SERIES_CANNON_POINT; } else { msg_pwarn("Unknown flash descriptor, assuming 300 series compatibility.\n"); return CHIPSET_300_SERIES_CANNON_POINT; -- To view, visit https://review.coreboot.org/c/flashrom/+/55645 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id739bc12832e3b441e8e7e1dcdcc4c05b260d7ad Gerrit-Change-Number: 55645 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ich_descriptors: Revise descriptor messages
by Nico Huber (Code Review) 17 Jun '21

17 Jun '21

Attention is currently required from: Michał Żygowski, Angel Pons. Hello Michał Żygowski, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/55644 to review the following change. Change subject: ich_descriptors: Revise descriptor messages ...................................................................... ich_descriptors: Revise descriptor messages Correct "firmware descriptor" to "flash descriptor". And also move the check for peculiar descriptors and the message into an inline function. Change-Id: I7f15780e03d2fa17ca6d8328275cae5af13ae424 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ich_descriptors.c 1 file changed, 13 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/55644/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index a6ac881..fc04a24 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -912,6 +912,13 @@ msg_pdbg2("\n"); } +static inline void warn_peculiar_desc(const bool warn_if, const char *const name) +{ + if (!warn_if) + return; + msg_pwarn("Peculiar flash descriptor, assuming %s compatibility.\n", name); +} + /* * Guesses a minimum chipset version based on the maximum number of * soft straps per generation. @@ -930,11 +937,10 @@ else if (content->FLMAP2 == 0) { if (content->ISL == 23) return CHIPSET_GEMINI_LAKE; - else if (content->ISL != 19) - msg_pwarn("Peculiar firmware descriptor, assuming Apollo Lake compatibility.\n"); + warn_peculiar_desc(content->ISL != 19, "Apollo Lake"); return CHIPSET_APOLLO_LAKE; } - msg_pwarn("Peculiar firmware descriptor, assuming Ibex Peak compatibility.\n"); + warn_peculiar_desc(true, "Ibex Peak"); return CHIPSET_5_SERIES_IBEX_PEAK; } else if (content->ICCRIBA < 0x31 && content->FMSBA < 0x30) { if (content->MSL == 0 && content->ISL <= 17) @@ -943,7 +949,7 @@ return CHIPSET_6_SERIES_COUGAR_POINT; else if (content->MSL <= 1 && content->ISL <= 21) return CHIPSET_8_SERIES_LYNX_POINT; - msg_pwarn("Peculiar firmware descriptor, assuming Wildcat Point compatibility.\n"); + warn_peculiar_desc(true, "Wildcat Point"); return CHIPSET_9_SERIES_WILDCAT_POINT; } else if (content->ICCRIBA < 0x34) { if (content->NM == 6) @@ -956,7 +962,7 @@ else return CHIPSET_300_SERIES_CANNON_POINT; } else { - msg_pwarn("Unknown firmware descriptor, assuming 300 series compatibility.\n"); + msg_pwarn("Unknown flash descriptor, assuming 300 series compatibility.\n"); return CHIPSET_300_SERIES_CANNON_POINT; } } @@ -982,7 +988,7 @@ case CHIPSET_C620_SERIES_LEWISBURG: case CHIPSET_APOLLO_LAKE: if (component->modes.freq_read != 6) { - msg_pwarn("\nThe firmware descriptor looks like a Skylake/Sunrise Point descriptor.\n" + msg_pwarn("\nThe flash descriptor looks like a Skylake/Sunrise Point descriptor.\n" "However, the read frequency isn't set to 17MHz (the only valid value).\n" "Please report this message, the output of `ich_descriptors_tool` for\n" "your descriptor and the output of `lspci -nn` to flashrom(a)flashrom.org\n\n"); @@ -991,7 +997,7 @@ return guess; default: if (component->modes.freq_read == 6) { - msg_pwarn("\nThe firmware descriptor has the read frequency set to 17MHz. However,\n" + msg_pwarn("\nThe flash descriptor has the read frequency set to 17MHz. However,\n" "it doesn't look like a Skylake/Sunrise Point compatible descriptor.\n" "Please report this message, the output of `ich_descriptors_tool` for\n" "your descriptor and the output of `lspci -nn` to flashrom(a)flashrom.org\n\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/55644 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7f15780e03d2fa17ca6d8328275cae5af13ae424 Gerrit-Change-Number: 55644 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ich_descriptors.c: Fix PCH detection for Tiger Lake
by Michał Żygowski (Code Review) 17 Jun '21

17 Jun '21

Attention is currently required from: Nico Huber, Angel Pons. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55577 ) Change subject: ich_descriptors.c: Fix PCH detection for Tiger Lake ...................................................................... Patch Set 1: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/55577/comment/0a067069_c7b5b8b7 PS1, Line 955: CHIPSET_300_SERIES_CANNON_POINT > Intel refers to it as TGP (also for Rocket Lake), so Tiger Point seems […] FMSBA is just shifted 2 bits left in the register and occupies 10 bits, that is why it did not catch the `< 0x30` condition. Right, FLUMAP[31:15] now contains some data. So basically what you mean is: if it is not zero, then we have 300 series+ chipset right? -- To view, visit https://review.coreboot.org/c/flashrom/+/55577 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib61d432ab4eaf00aa4eef50d2844940e73b5cad6 Gerrit-Change-Number: 55577 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 17 Jun 2021 14:25:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: lspcon: restart MPU on programmer shutdown
by Edward O'Callaghan (Code Review) 17 Jun '21

17 Jun '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/55403 ) Change subject: lspcon: restart MPU on programmer shutdown ...................................................................... lspcon: restart MPU on programmer shutdown Programmer initialization stops the on-chip MPU, and it was never restarted. Leaving it stopped seems to prevent some display detection from working, so implement restarting the MPU on programmer shutdown. BUG=b:190359231 TEST=display hotplug works reliably after device communication Change-Id: I66cd68f8f6905a2bfaf5b085bf08dcb218f42855 Reviewed-on: https://review.coreboot.org/c/flashrom/+/55403 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Sam McNally <sammc(a)google.com> --- M lspcon_i2c_spi.c 1 file changed, 9 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Sam McNally: Looks good to me, approved diff --git a/lspcon_i2c_spi.c b/lspcon_i2c_spi.c index 9f3bb9a..b590f1d 100644 --- a/lspcon_i2c_spi.c +++ b/lspcon_i2c_spi.c @@ -318,11 +318,13 @@ return ret; } -static int lspcon_i2c_spi_reset_mpu_stop(int fd) +static int lspcon_i2c_spi_set_mpu_active(int fd, int running) { int ret = 0; - ret |= lspcon_i2c_spi_write_register(fd, MPU, 0xc0); // cmd mode - ret |= lspcon_i2c_spi_write_register(fd, MPU, 0x40); // stop mcu + // Cmd mode + ret |= lspcon_i2c_spi_write_register(fd, MPU, 0xc0); + // Stop or release MPU + ret |= lspcon_i2c_spi_write_register(fd, MPU, running ? 0 : 0x40); return ret; } @@ -418,15 +420,16 @@ .write_aai = lspcon_i2c_spi_write_aai, }; -/* TODO: MPU still stopped at this point, probably need to reset it. */ static int lspcon_i2c_spi_shutdown(void *data) { int ret = 0; struct lspcon_i2c_spi_data *lspcon_data = (struct lspcon_i2c_spi_data *)data; int fd = lspcon_data->fd; + ret |= lspcon_i2c_spi_enable_write_protection(fd); ret |= lspcon_i2c_spi_toggle_register_protection(fd, 0); + ret |= lspcon_i2c_spi_set_mpu_active(fd, 1); i2c_close(fd); free(data); @@ -439,9 +442,9 @@ if (fd < 0) return fd; - int ret = lspcon_i2c_spi_reset_mpu_stop(fd); + int ret = lspcon_i2c_spi_set_mpu_active(fd, 0); if (ret) { - msg_perr("%s: call to reset_mpu_stop failed.\n", __func__); + msg_perr("%s: call to set_mpu_active failed.\n", __func__); i2c_close(fd); return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/55403 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I66cd68f8f6905a2bfaf5b085bf08dcb218f42855 Gerrit-Change-Number: 55403 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: lspcon: restart MPU on programmer shutdown
by Sam McNally (Code Review) 16 Jun '21

16 Jun '21

Attention is currently required from: Peter Marheine. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55403 ) Change subject: lspcon: restart MPU on programmer shutdown ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55403 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I66cd68f8f6905a2bfaf5b085bf08dcb218f42855 Gerrit-Change-Number: 55403 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 17 Jun 2021 00:28:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: lspcon: restart MPU on programmer shutdown
by Edward O'Callaghan (Code Review) 16 Jun '21

16 Jun '21

Attention is currently required from: Sam McNally, Peter Marheine. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55403 ) Change subject: lspcon: restart MPU on programmer shutdown ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55403 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I66cd68f8f6905a2bfaf5b085bf08dcb218f42855 Gerrit-Change-Number: 55403 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 17 Jun 2021 00:16:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021