- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: flashrom.c: Make {read,write}_buf_from_include_args() public
by Anastasia Klimchuk (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Edward O'Callaghan, Daniel Campello, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60069 ) Change subject: flashrom.c: Make {read,write}_buf_from_include_args() public ...................................................................... Patch Set 3: (1 comment) File flash.h: https://review.coreboot.org/c/flashrom/+/60069/comment/7272fcc8_30e8faf5 PS3, Line 353: read_buf_from_include_args There are few existing include_args functions in layout.h. Is there any reason these two can't go into layout.h? These both work with layout, context is only used to get layout from context and nothing else. -- To view, visit https://review.coreboot.org/c/flashrom/+/60069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia0abec655a682ca449d0e8ba620886a2d616b86d Gerrit-Change-Number: 60069 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 20 Jan 2022 05:46:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add Elkhart Lake support
by Werner Zeh (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Nico Huber. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60711 ) Change subject: Add Elkhart Lake support ...................................................................... Patch Set 2: (9 comments) File chipset_enable.c: https://review.coreboot.org/c/flashrom/+/60711/comment/f0e343e6_af6b006f PS1, Line 709: boot_straps = boot_straps_pch500; > The EDS says `bbs == 1` would be "UFS". Not sure how that's connected. […] In EDS there is just SPI defined (bbs=0), whereas bbs=1 is marked as reserved. So I guess it is more like it was on APL/GLK. I will move the case CHIPSET_ELKHART_LAKE one down. https://review.coreboot.org/c/flashrom/+/60711/comment/59bb318f_6cb10714 PS1, Line 995: dev > So AFAICS, this is already 1f.5. We could save us the dance to find it […] I guess this would be something for a follow-up patch, right? File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/60711/comment/cdc888e7_1ed666be PS1, Line 538: IFWI > Intel likes to haunt us with this it seems. Now all the non-BIOS firmware is […] Yeah, it is this back and forth with every new platform... https://review.coreboot.org/c/flashrom/+/60711/comment/ab33e822_e7732301 PS1, Line 1033: return CHIPSET_500_SERIES_TIGER_POINT; > If the SPI guide is correct and I followed it, we would end up on this path. […] Yes, I was starring at this code back than for a while. Your approach seems to be OK, at least I can see the value 0x6c for EHL in the docs. Will change accordingly. https://review.coreboot.org/c/flashrom/+/60711/comment/ebde677d_075a53a5 PS1, Line 1055: case CHIPSET_GEMINI_LAKE: > EHL here? With the change above, yes. Will add. File ichspi.c: https://review.coreboot.org/c/flashrom/+/60711/comment/7dc2342f_734c966d PS1, Line 452: pprint_reg(HSFS, FLOCKDN, reg_val, "\n"); > Looks like there are several new SAF (slave-attached flash?) bits. We […] I have checked the printed bits and they do match the docs. https://review.coreboot.org/c/flashrom/+/60711/comment/b5b70256_9367a620 PS1, Line 464: case CHIPSET_500_SERIES_TIGER_POINT: > Looks like we missed APL/GLK here? EHL should go here too it seems. Cheked the bits in the register and they do macth with EHL. Will add. https://review.coreboot.org/c/flashrom/+/60711/comment/74b2b55c_ad1d4abd PS1, Line 1774: case CHIPSET_ELKHART_LAKE: > This is tricky, the additional regs are at 0xe0. So it actually has 16. Oh yes, I wasn't aware. Will shift back to 16 (had it in the beginning). https://review.coreboot.org/c/flashrom/+/60711/comment/21fe4c4d_4430a4c0 PS1, Line 2027: ich_gen == CHIPSET_ELKHART_LAKE)) { > Hmmm, would fit better below. Or maybe just merge the two if's. The idea […] If moved below, something like this? msg_pdbg("Enabling hardware sequencing by default for Apollo/Gemini/Elkhart Lake.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/60711 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I711e39a3ec9cd7098389231eaa1cb864d615a475 Gerrit-Change-Number: 60711 Gerrit-PatchSet: 2 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.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-Comment-Date: Thu, 20 Jan 2022 05:39:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add Elkhart Lake support
by Werner Zeh (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Werner Zeh. Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/60711 to look at the new patch set (#2). Change subject: Add Elkhart Lake support ...................................................................... Add Elkhart Lake support Elkhart Lake has a chipset called Mule Creek Canyon which is quite compatible with 300 series chipsets. There are a few differences though, e.g. different encoding for the SPI clock values for read and write in the FLCOMP register. In addition Elkhart Lake has a new PCI device ID for the SPI controller which is added, too. TEST=Read and flash complete flash on Siemens MC EHL1 Change-Id: I711e39a3ec9cd7098389231eaa1cb864d615a475 Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com> --- 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, 51 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/11/60711/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/60711 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I711e39a3ec9cd7098389231eaa1cb864d615a475 Gerrit-Change-Number: 60711 Gerrit-PatchSet: 2 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.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: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: cli_classic.c: Convert do_erase() to libflashrom call
by Anastasia Klimchuk (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60068 ) Change subject: cli_classic.c: Convert do_erase() to libflashrom call ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/60068 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8566164e7dbad69cf478b24208014f10fb99e4d0 Gerrit-Change-Number: 60068 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(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-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 20 Jan 2022 05:32:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: build hwaccess only on x86
by Peter Marheine (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Thomas Heijligen. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61198 ) Change subject: meson: build hwaccess only on x86 ...................................................................... Patch Set 3: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/61198/comment/676574e0_ccedf6a0 PS3, Line 357: error('RAW access is not available on non-x86 platforms. Disable the programmer which needs it. Use `meson build -Dpciutils=false -Dconfig_rayer_spi=false`') > A relatively simple fix might be to move the stub implementation into the header as an inline static […] Or it seems https://review.coreboot.org/c/flashrom/+/60841 and the following changes may fix most of the issue, but then pciutils shouldn't imply need_raw_access. -- To view, visit https://review.coreboot.org/c/flashrom/+/61198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafabe8b4b2a95697773a739501dfc62d880d3f6d Gerrit-Change-Number: 61198 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Thu, 20 Jan 2022 04:50:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: replace target_machine by host_machine
by Edward O'Callaghan (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Felix Singer, Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61197 ) Change subject: meson: replace target_machine by host_machine ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61197 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4a32c73052c3707607738c72256fb4366a2bf8ce Gerrit-Change-Number: 61197 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Thu, 20 Jan 2022 02:49:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: build hwaccess only on x86
by Edward O'Callaghan (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61198 ) Change subject: meson: build hwaccess only on x86 ...................................................................... Patch Set 3: -Code-Review -- To view, visit https://review.coreboot.org/c/flashrom/+/61198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafabe8b4b2a95697773a739501dfc62d880d3f6d Gerrit-Change-Number: 61198 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Thu, 20 Jan 2022 01:59:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: build hwaccess only on x86
by Peter Marheine (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Thomas Heijligen. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61198 ) Change subject: meson: build hwaccess only on x86 ...................................................................... Patch Set 3: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/61198/comment/967bab93_ef9f3b3a PS3, Line 357: error('RAW access is not available on non-x86 platforms. Disable the programmer which needs it. Use `meson build -Dpciutils=false -Dconfig_rayer_spi=false`') > I don't think this is accurate, because PCI-based programmers work on non-x86 platforms but simply l […] A relatively simple fix might be to move the stub implementation into the header as an inline static function. As-is the organization seems incorrect since the non-x86 stub is currently in the "_x86" C file. -- To view, visit https://review.coreboot.org/c/flashrom/+/61198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafabe8b4b2a95697773a739501dfc62d880d3f6d Gerrit-Change-Number: 61198 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Thu, 20 Jan 2022 01:57:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests for verify operation
by Anastasia Klimchuk (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59239 to look at the new patch set (#6). Change subject: tests: Add tests for verify operation ...................................................................... tests: Add tests for verify operation This patch adds two tests which cover verify operation, and adds io_mock for fread. BUG=b:181803212 TEST=ninja test Change-Id: I1cc6f73f9b1e385eb963adccf20759c13a40ed3b Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c M tests/tests.c M tests/tests.h 3 files changed, 100 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/59239/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/59239 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1cc6f73f9b1e385eb963adccf20759c13a40ed3b Gerrit-Change-Number: 59239 Gerrit-PatchSet: 6 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: meson: build hwaccess only on x86
by Peter Marheine (Code Review) 20 Jan '22

20 Jan '22

Attention is currently required from: Thomas Heijligen. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61198 ) Change subject: meson: build hwaccess only on x86 ...................................................................... Patch Set 3: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/61198/comment/8172e132_1235d836 PS3, Line 357: error('RAW access is not available on non-x86 platforms. Disable the programmer which needs it. Use `meson build -Dpciutils=false -Dconfig_rayer_spi=false`') I don't think this is accurate, because PCI-based programmers work on non-x86 platforms but simply lack ioperm() or iopl() since those are x86-specific. rget_io_perms() even has a stub implementation that works on other systems (with the comment specifically calling out MIPS, PPC and ARM) so it seems pretty clear that it's supposed to work on other platforms. -- To view, visit https://review.coreboot.org/c/flashrom/+/61198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafabe8b4b2a95697773a739501dfc62d880d3f6d Gerrit-Change-Number: 61198 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Thu, 20 Jan 2022 01:20:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit