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

Change in flashrom[master]: ich_descriptors.c: Fix PCH detection for Tiger Lake
by Michał Żygowski (Code Review) July 1, 2021

July 1, 2021

Michał Żygowski has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/55577 ) Change subject: ich_descriptors.c: Fix PCH detection for Tiger Lake ...................................................................... Abandoned -- 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-MessageType: abandon

1 0

Change in flashrom[master]: ich_descriptors_tool: Add missing Comet Point in usage
by Michał Żygowski (Code Review) July 1, 2021

July 1, 2021

Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55993 ) Change subject: ich_descriptors_tool: Add missing Comet Point in usage ...................................................................... ich_descriptors_tool: Add missing Comet Point in usage Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Change-Id: Ia1e3e231944513521d5db064340a0247f1884290 --- M util/ich_descriptors_tool/ich_descriptors_tool.c 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/55993/1 diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index bfeedc1..e738455 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -135,6 +135,7 @@ "\t- \"9\" or \"wildcat\" for Intel's 9 series chipsets.\n" "\t- \"100\" or \"sunrise\" for Intel's 100 series chipsets.\n" "\t- \"300\" or \"cannon\" for Intel's 300 series chipsets.\n" +"\t- \"400\" or \"comet\" for Intel's 400 series chipsets.\n" "If '-d' is specified some regions such as the BIOS image as seen by the CPU or\n" "the GbE blob that is required to initialize the GbE are also dumped to files.\n", argv[0], argv[0]); @@ -229,6 +230,9 @@ else if ((strcmp(csn, "400") == 0) || (strcmp(csn, "comet") == 0)) cs = CHIPSET_400_SERIES_COMET_POINT; + else if ((strcmp(csn, "500") == 0) || + (strcmp(csn, "tiger") == 0)) + cs = CHIPSET_500_SERIES_TIGER_POINT; else if (strcmp(csn, "apollo") == 0) cs = CHIPSET_APOLLO_LAKE; else if (strcmp(csn, "gemini") == 0) -- To view, visit https://review.coreboot.org/c/flashrom/+/55993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia1e3e231944513521d5db064340a0247f1884290 Gerrit-Change-Number: 55993 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Add Tiger Lake U Premium support
by Michał Żygowski (Code Review) July 1, 2021

July 1, 2021

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55578 to look at the new patch set (#2). Change subject: Add Tiger Lake U Premium support ...................................................................... Add Tiger Lake U Premium support Tiger Lake has very low ICCRIBA (TGL=0x11, CNL=0x34 and CML=0x34) and detects as uknown chipset compatible with 300 series chipset. Add a new enum CHIPSET_500_SERIES_TIGER_POINT and treat it identically to CHIPSET_400_SERIES_COMET_POINT except the ICCRIBA in descriptor guessing, pprint_freq and boot straps. TEST=Flash BIOS region on Intel i5-1135G7 Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Change-Id: I28f3b6fe9f8ce9e976a6808683f46b6f4ec72bdd --- M chipset_enable.c M ich_descriptors.c M ichspi.c M programmer.h M util/ich_descriptors_tool/ich_descriptors_tool.c 5 files changed, 39 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/55578/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55578 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I28f3b6fe9f8ce9e976a6808683f46b6f4ec72bdd Gerrit-Change-Number: 55578 Gerrit-PatchSet: 2 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-MessageType: newpatchset

1 0

Change in flashrom[master]: programmer: Introduce default shutdown function
by Angel Pons (Code Review) July 1, 2021

July 1, 2021

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54890 ) Change subject: programmer: Introduce default shutdown function ...................................................................... Patch Set 5: Code-Review+2 (1 comment) Patchset: PS5: > There are two reasons why I thought of default shutdown function. […] Personally, I don't mind adding something that ends up being removed later, as long as it provides some benefit. I can see why this `default_shutdown()` function may end up being unnecessary in the future, but this commit would ease (even if only slightly) follow-up works as there would be less shutdown functions to adapt and/or remove. -- To view, visit https://review.coreboot.org/c/flashrom/+/54890 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8b20717ba549e12edffbc5d1643ff064a4f0c517 Gerrit-Change-Number: 54890 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 01 Jul 2021 09:09:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c: Fix PCH detection for Tiger Lake
by Michał Żygowski (Code Review) July 1, 2021

July 1, 2021

Michał Żygowski has restored this change. ( https://review.coreboot.org/c/flashrom/+/55577 ) Change subject: ich_descriptors.c: Fix PCH detection for Tiger Lake ...................................................................... Restored -- 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-MessageType: restore

1 0

Change in flashrom[master]: ich_descriptors.c: Fix PCH detection for Tiger Lake
by Michał Żygowski (Code Review) July 1, 2021

July 1, 2021

Michał Żygowski has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/55577 ) Change subject: ich_descriptors.c: Fix PCH detection for Tiger Lake ...................................................................... Abandoned ICH descriptor probing clean up in separate patch series -- 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-MessageType: abandon

1 0

Change in flashrom[master]: dediprog: Init-shutdown test for dediprog
by Anastasia Klimchuk (Code Review) July 1, 2021

July 1, 2021

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55934 ) Change subject: dediprog: Init-shutdown test for dediprog ...................................................................... Patch Set 3: (5 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/55934/comment/032ff14a_606a4d45 PS3, Line 9: This patch adds mocks for libusb functions. To avoid dependency on > I expect that some of the code could be reused to test other libusb programmers. […] Yes, hopefully all __wrap functions will be reused for other programmers, because programmer-specific behaviour goes into current_io. Actually, the wraps here are not expected to have much code, if something needs code that code would probably go into current_io and live in the test itself. Moving mocks into separate compilation unit would need a bit of refactoring, because at the moment there is current_io in this file, and also io_mock_register. Ideally, I would add one or two more tests which use libusb wraps, and then maybe it will be more obvious [to me] how to move it / where to move it? File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/55934/comment/88644d17_af5ef6ad PS3, Line 98: /* dediprog_cmds CMD_READ_PROG_INFO */ > Hmmm, how about making a patch that moves the dediprog definitions the tests need into a header? This question came up in the review process of commit 21e22ba8a7750f1cfe5cd3323e3137695ffef0a4, discussion started at Patchset 10 May 01 01:59 init_shutdown.c#53 and the decision was not to move definitions into the header. But things could have changed, if everyone agrees it's a good idea I can do it. File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/55934/comment/f5519b3b_0d77aa24 PS3, Line 34: /* Define libusb symbols to avoid dependency on libusb.h */ > Now that I realized that we don't mock `libusb.h` for the code under […] I wrote this tests few weeks ago, but was waiting for other patches for tests/ to be done. So initially I included libusb.h without much thinking. But then commit 20ff3d464ba20b385b84aa0eb132eeff3345accc and especially commit 4abb62c4ac1f25e40f2569535322342b9c939558 have happened, and I became more aware on what can be a consequence of including a header. Returned back to tests, and first thing I saw was libusb.h with may not be supported. In an environment where libusb.h is not supported, I can't include it right? This would mean adding pre-processor guards, which I understand better to avoid? Then I thought the only reason I included the header is for the libusb_device_handle type. But real handles are not needed for tests, so I replaced with void* and boom, everything works and header not needed. Individual tests are guarded by CONFIG_XXX anyway, so test would be skipped if driver is not built? and driver not built is libusb not supported. File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/55934/comment/ebc6961a_0765c50b PS2, Line 52: */ > Oh, I missed earlier that you avoid the libusb dependency here while […] WOW thank you for explanation... So many opportunities to hack things! File tests/tests.c: https://review.coreboot.org/c/flashrom/+/55934/comment/a03f0286_26c59f57 PS3, Line 140: 2021 > Is this magic number the current year? Or does it have another meaning? Yeah that's the current year. I needed something to pretend it's a valid file descriptor, and then something as a valid device handle, and there will be more of that I am sure. The number itself doesn't matter. Is this an acceptable approach, or maybe I should create a VALID_POINTER macro instead of inline magic number? -- To view, visit https://review.coreboot.org/c/flashrom/+/55934 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I38508dfb6d7c24d42522f22fcae0c5e410c5f7ea Gerrit-Change-Number: 55934 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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, 01 Jul 2021 00:48:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0