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

Change in flashrom[master]: ich_descriptors: Revise detection for chipsets w/ ICCRIBA
by Michał Żygowski (Code Review) 18 Jun '21

18 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/+/55647 ) Change subject: ich_descriptors: Revise detection for chipsets w/ ICCRIBA ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: As File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/55647/comment/3a7e995c_15fa7777 PS3, Line 946: } else if (upper->MDTBA == 0x00) { As suggested in 55577 we may also reuse guess_ifd_2_chipset from coreboot's ifdtool -- 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: 3 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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 18 Jun 2021 08:52:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors: Drop some unnecessary `else` after `return`
by Michał Żygowski (Code Review) 18 Jun '21

18 Jun '21

Attention is currently required from: Nico Huber. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55645 ) Change subject: ich_descriptors: Drop some unnecessary `else` after `return` ...................................................................... Patch Set 1: Code-Review+2 -- 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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Fri, 18 Jun 2021 08:49:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors: Revise descriptor messages
by Michał Żygowski (Code Review) 18 Jun '21

18 Jun '21

Attention is currently required from: Nico Huber. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55644 ) Change subject: ich_descriptors: Revise descriptor messages ...................................................................... Patch Set 1: Code-Review+2 -- 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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Fri, 18 Jun 2021 08:48:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

18 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/098440f9_e8bfc4bd PS1, Line 955: CHIPSET_300_SERIES_CANNON_POINT > > if it is not zero, then we have 300 series+ chipset […] I also have a suggestion: we could reuse the coreboot's ifdtool detection function guess_ifd_2_chipset What do you think? -- 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 18 Jun 2021 08:45: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: Normalize chipset detection
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/+/55651 to review the following change. Change subject: ich_descriptors: Normalize chipset detection ...................................................................... ich_descriptors: Normalize chipset detection Normalize the fallback paths: * Always end with the newest, assumed compatible chipset. * Perform tighter checks when it's about warnings only. * If two chipsets seem compatible, always return the same. Change-Id: Ic5a5fee870202173b3a9813b03ec261e8ee45155 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ich_descriptors.c 1 file changed, 7 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/55651/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index 244338d..56155e4 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -933,15 +933,13 @@ return CHIPSET_ICH9; if (content->ISL <= 10) return CHIPSET_ICH10; - if (content->ISL <= 16) - return CHIPSET_5_SERIES_IBEX_PEAK; if (content->FLMAP2 == 0) { - if (content->ISL == 23) - return CHIPSET_GEMINI_LAKE; - warn_peculiar_desc(content->ISL != 19, "Apollo Lake"); - return CHIPSET_APOLLO_LAKE; + if (content->ISL == 19) + return CHIPSET_APOLLO_LAKE; + warn_peculiar_desc(content->ISL != 23, "Gemini Lake"); + return CHIPSET_GEMINI_LAKE; } - warn_peculiar_desc(true, "Ibex Peak"); + warn_peculiar_desc(content->ISL != 16, "Ibex Peak"); return CHIPSET_5_SERIES_IBEX_PEAK; } else if (upper->MDTBA == 0x00) { if (content->ICCRIBA < 0x31 && content->FMSBA < 0x30) { @@ -949,10 +947,8 @@ 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; + warn_peculiar_desc(content->MSL != 1 || content->ISL != 21, "Lynx Point"); + return CHIPSET_8_SERIES_LYNX_POINT; } if (content->NM == 6) { warn_peculiar_desc(content->ICCRIBA > 0x34, "C620 series"); -- To view, visit https://review.coreboot.org/c/flashrom/+/55651 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5a5fee870202173b3a9813b03ec261e8ee45155 Gerrit-Change-Number: 55651 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 reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55647 to look at the new patch set (#3). 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/3 -- 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: 3 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]: [RFC] ich_descriptors: Don't base chipset detection on `freq_read`
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/+/55650 to review the following change. Change subject: [RFC] ich_descriptors: Don't base chipset detection on `freq_read` ...................................................................... [RFC] ich_descriptors: Don't base chipset detection on `freq_read` Only warn if the `freq_read` setting looks odd but don't override our previous guess. The `freq_read` check was taken from `ifdtool` but seems less reliable than our own detection scheme. Change-Id: I658d76ec2567d1d660a18d0b0ae71c744e603e8f Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ich_descriptors.c 1 file changed, 3 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/55650/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index da49326..edc6bf4 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -984,7 +984,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_GEMINI_LAKE: /* `freq_read` was repurposed, so can't check on it any more. */ - return guess; + break; case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: case CHIPSET_APOLLO_LAKE: @@ -993,19 +993,17 @@ "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"); - return CHIPSET_9_SERIES_WILDCAT_POINT; } - return guess; + break; default: if (component->modes.freq_read == 6) { 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"); - return CHIPSET_100_SERIES_SUNRISE_POINT; } - return guess; } + return guess; } /* len is the length of dump in bytes */ -- To view, visit https://review.coreboot.org/c/flashrom/+/55650 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I658d76ec2567d1d660a18d0b0ae71c744e603e8f Gerrit-Change-Number: 55650 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 Angel Pons (Code Review) 17 Jun '21

17 Jun '21

Attention is currently required from: Nico Huber, Michał Żygowski. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55645 ) Change subject: ich_descriptors: Drop some unnecessary `else` after `return` ...................................................................... Patch Set 1: Code-Review+2 -- 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Thu, 17 Jun 2021 20:32:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

17 Jun '21

Attention is currently required from: Nico Huber, Michał Żygowski. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55644 ) Change subject: ich_descriptors: Revise descriptor messages ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/55644/comment/4cc7b107_9aa82755 PS1, Line 9: Correct I've seen both in Intel docs. However, I believe the IFD describes the flash more than it describes the firmware. Plus, this helps distinguish between IFD and IFWI. -- 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Thu, 17 Jun 2021 20:32:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

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 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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021