March 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: tests/linux_spi: Validate param file path
by Anastasia Klimchuk (Code Review) 02 Mar '22

02 Mar '22

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62319 ) Change subject: tests/linux_spi: Validate param file path ...................................................................... Patch Set 4: (6 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62319/comment/a214cb5d_ecae28a2 PS4, Line 10: linux_spi unit-test does not : check I think unit-tests in general had no implementation to mock repeated calls to open, it is not specific to linux_spi. Since you are adding generic open mock and state which support multiple calls to open, you can say something like "add ABCD infrastructure to unit tests and use this infra for linux_spi test"? Also, maybe it is just me, but I didn't understand what "subsequent path validation" is until I read the code. Can we call it "repeated calls to open"? It really only supports open, not fopen or any other path. Also I would emphasise it more, in a separate paragraph (even if small) that this patch also adds something to cover "/dev/null". File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/62319/comment/46b4d526_cf15c469 PS4, Line 39: #define DEFAULT_MOCK_FD 0xC0FE Is there a specific meaning behind this number? We have already a NON_ZERO which is defined as #define NON_ZERO (0xf000baaa) is possible to reuse? NON_ZERO is defined in include/test.h so it can be used anywhere in tests. https://review.coreboot.org/c/flashrom/+/62319/comment/0186ca20_981c8ca1 PS4, Line 41: default_io_mock_state I would like to rename this, I have two reasons in mind. The most important, "default" doesn't tell much about what the state actually use. Something like "multipath_io_state" gives information about the state. Another reason, I am not sure how much this is actually default, because default is no state. Also further in the chain in CB:62324 you create another struct with the same name, but it can't be two defaults with the same name. https://review.coreboot.org/c/flashrom/+/62319/comment/dab03cc0_d7c4c5de PS4, Line 43: 2 Just wondering, why 2 ? https://review.coreboot.org/c/flashrom/+/62319/comment/8ce143f4_396b04d2 PS4, Line 48: default_open For the same reasons as above, I would rename this to something more self-explanatory, maybe "multipath_open" ? https://review.coreboot.org/c/flashrom/+/62319/comment/88ec349b_a9bb9031 PS4, Line 305: data I would keep the naming to indicate this piece of data belongs to linux_spi test, "linux_spi_io_state". Even if the struct type is shared between multiple tests, this piece of data belongs to this one test. -- To view, visit https://review.coreboot.org/c/flashrom/+/62319 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If5d24c65f291c53a35509fea5d2f5b3fdb51c306 Gerrit-Change-Number: 62319 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 02 Mar 2022 01:39:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Alder Lake support
by Sam McNally (Code Review) 02 Mar '22

02 Mar '22

Attention is currently required from: Rizwan Qureshi, Edward O'Callaghan, Angel Pons. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 ) Change subject: ichspi: Add Alder Lake support ...................................................................... Patch Set 12: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62251/comment/452004b1_2386a95f PS12, Line 12: TEST=```localhost ~ # flashrom --flash-name > Any other tests, e.g. reading? If the ME region is locked, you can still read the other regions. […] ``` flashrom -p internal --ifd -i fd -i bios -r /tmp/filename.rom flashrom unknown on Linux 5.15.22 (x86_64) flashrom is free software, get the source code at https://flashrom.org Using clock_gettime for delay loops (clk_id: 1, resolution: 1ns). coreboot table found at 0x768a7000. Found chipset "Intel Alder Lake-N". Enabling flash write... Warning: Setting BIOS Control at 0xdc from 0x8b to 0x89 failed. New value is 0x8b. SPI Configuration is locked down. OK. Found Winbond flash chip "W25Q256JV_M" (32768 kB, Programmer-specific) mapped at physical address 0x0000000000000000. Error accessing W25Q256JV_M, 0x2000000 bytes at 0x00000000fe000000 /dev/mem mmap failed: Resource temporarily unavailable Could not map flash chip W25Q256JV_M at 0x00000000fe000000. Reading ich descriptor... done. Using regions: "bios", "fd". Error accessing W25Q256JV_M, 0x2000000 bytes at 0x00000000fe000000 /dev/mem mmap failed: Resource temporarily unavailable Could not map flash chip W25Q256JV_M at 0x00000000fe000000. Reading flash... done. SUCCESS ``` Reading with flashrom -r /tmp/filename2.rom -i SI_BIOS -i SI_DESC produces an identical file. A glance through a hexdump shows reasonable-looking content with the expected gap for the ME region. -- To view, visit https://review.coreboot.org/c/flashrom/+/62251 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf Gerrit-Change-Number: 62251 Gerrit-PatchSet: 12 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 02 Mar 2022 01:39:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq
by Edward O'Callaghan (Code Review) 02 Mar '22

02 Mar '22

Attention is currently required from: Patrick Georgi, Rizwan Qureshi, Stefan Reinauer, Angel Pons, Sridhar Siricilla, Alex Levin, YH Lin, Nico Huber, Martin Roth, Subrata Banik, Caveh Jalali, David Hendricks, Tim Wawrzynczak, Nick Vaccaro, Boris Mittelberg. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61854 ) Change subject: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq ...................................................................... Patch Set 13: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61854 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484 Gerrit-Change-Number: 61854 Gerrit-PatchSet: 13 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Alex Levin <levinale(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Alex Levin <levinale(a)chromium.org> Gerrit-Attention: YH Lin <yueherngl(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-Comment-Date: Wed, 02 Mar 2022 01:30:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ite_ecfw: Implement support for flashing ITE ECs found on TUXEDO laptops
by Nico Huber (Code Review) 02 Mar '22

02 Mar '22

Attention is currently required from: Felix Singer, Michał Żygowski, Paul Menzel, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: ite_ecfw: Implement support for flashing ITE ECs found on TUXEDO laptops ...................................................................... Patch Set 29: (12 comments) Patchset: PS29: > The internal DMI decoder checks the legacy region for SMBIOS entry. […] I was wondering for some time why the external DMI option is not the default. Maybe it's time to change that. However, this seems like an independent problem. Thanks for the clarification. PS29: > You mean compile the files when CONFIG_INTERNAL is selected? Michael also wanted to add more interfa […] I mean it should be fully included in `-p internal` eventually. The `internal` programmer is meant for flash chips on the system mainboard. All the infra- structure is there, and I hope it wouldn't be hard to hook an EC driver properly up. Basically a board-enable entry would call the init function of this driver, and that would register another master. Later logic could decide if this master is to be used, or we could filter the call beforehand, e.g. something like `-p internal:master=ec`. To make use of the board-enable infrastructure for other than the `internal` programmers would need a lot of refactoring I guess. For now, I would rather keep the current design. Unless there is something I miss, i.e. any reason not to make EC drivers part of `-p internal`? As I said, I'd be ok with merging it as a separate programmer. But we'd have to expect that it will change in the future (i.e. I would welcome patches that make it part of `-p internal`). File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/55715/comment/6520b66f_2c6cd84d PS29, Line 1423: .B " flashrom \-p ite_ecfw:portpair=pairnum" > We can add a field that would specify the default port pair for given board. […] Yes, I would much prefer default values in the board list. https://review.coreboot.org/c/flashrom/+/55715/comment/724d6f9e_cc711baa PS29, Line 1471: .B " flashrom \-p ite_ecfw:boardmismatch=force" > There is a System76 EC firmware which serves as a replacement for the original firmware being used h […] AIUI (hopefully I'm not wrong) the original boardmismatch option is only useful in combination with coreboot and the image checks. To be able to force a board-enable entry there is this syntax: ``` flashrom -p internal:mainboard=<vendor>:<board> ``` This has the advantage that one can force board-specific code or default, i.e. could be useful in combination with defaults for port pairs like mentioned above. IOW, if we add per-board defaults, we'd want something like this too. File it87spi.c: https://review.coreboot.org/c/flashrom/+/55715/comment/7ffe7fd8_8114618f PS29, Line 105: case 0x89: > We can split it, however I have no means to answer your question (no hardware with ITE of ID startin […] Yes, please split. I'd want to carefully read the related code. It's a bit annoying (definitely not your fault) that the internal programmer always calls this code, no matter what board was detected. Your call is actually safer ;) File ite_ecfw.c: https://review.coreboot.org/c/flashrom/+/55715/comment/b38e8e56_a4e053de PS29, Line 411: } > We replay the behaviour of the original EFI file doing the update. […] AFAICS, if we'd allow a partial erase, flashrom could decide to only write the first block. Then this function would never be called for the last block and the first kilobyte wouldn't get written? I know it's only a theoretical problem as long as we erase the whole chip. Also, when the end of the image is empty, i.e. 0xff, same problem could occure, hmmm. This wouldn't be fun to test /o\ That's also rather theoretical, right? I guess these chips are always fully stuffed with data? https://review.coreboot.org/c/flashrom/+/55715/comment/817078ef_65df9ffe PS29, Line 493: ctx_data->ite_string_offset >= start && : ctx_data->ite_string_offset <= (start + len - sizeof(struct ite_string)) > The code should skip the autoload patching if offset is 0, I have to fix it. […] I rather meant what if the string offset is within start+len but start is not 0. I would expect ite_ecfw_patch_autoload() to patch the wrong location (`ite_string_offset` instead of `ite_string_offset - start`?). But let's discuss the autoload feature further first. https://review.coreboot.org/c/flashrom/+/55715/comment/83e544be_51866ca9 PS29, Line 495: (uint8_t *) > The autoload option breaks the verification indeed. […] To be honest I don't see such modifications in the flashrom tool. Wouldn't mind it in another tool in the project, of course. It's kind of an unwritten rule (well, unwritten beside the `const`). Flashrom should just sync file contents with flash contents (maybe I'm too much a fan of the Unix phylosophy). I would feel very uncomfortable to change that rule on my own, without a prior discussion on the mailing list where everybody would have a chance to raise their concerns. Also, removing the `const` from the API would make it less flexible. Currently the patching works by coincidence but a refactoring of the generic code could break that (unless we make it officially non-const constrain refactorings). I would like to propose an alternative: Leave it to the usual content sync'ing in `flashrom`. But provide another tool that uses libflashrom and takes care of everything, including the project check and the autoload patching. I would be happy to help with the new CLI code if that is an issue. https://review.coreboot.org/c/flashrom/+/55715/comment/df328cf8_c7780104 PS29, Line 603: > Probably we can set the erase size to 256 bytes or 1024 bytes, this needs testing. […] I was more wondering about erasing 4 * 64KiB individually instead of 256KiB at once (what the code tells flashrom to do). Multiples of 64KiB is what the first if in ite_ecfw_erase() enforces. But we send individual commands per chunk indeed. Somewhat odd. Let's leave it for now. https://review.coreboot.org/c/flashrom/+/55715/comment/2e5af0f4_54a31a8a PS29, Line 696: } > It looks like RDID, but the EC doesn't return reliable data. […] Does it show something useful yet? https://review.coreboot.org/c/flashrom/+/55715/comment/7999a2b2_f02f5cf7 PS29, Line 812: Probing > No longer true. If the board is not on the list of supported boards (matched by PIC and PCI subsystem of the host bridge), the code won't get to writing to EC RAM, unless we force it. What I mean is that the message says we would force "probing", but it's more a force using this driver. Not important atm. https://review.coreboot.org/c/flashrom/+/55715/comment/24b63c24_d4d557e5 PS29, Line 905: int ite_ecfw_verify_file_project(uint8_t *const newcontents, > Currently not, but I wish it would be at some point. […] We could probably find a place for a hook. Could that also be used for the autoload patching? But again, it might fit a separate tool much better. -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 29 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Wed, 02 Mar 2022 01:05:18 +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> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Add FTDI FT4233H
by Nico Huber (Code Review) 01 Mar '22

01 Mar '22

Attention is currently required from: Charles Parent, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62480 ) Change subject: ft2232_spi.c: Add FTDI FT4233H ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73cee8fd2a6613a8fbc26508d99bbe67da2b4f72 Gerrit-Change-Number: 62480 Gerrit-PatchSet: 2 Gerrit-Owner: Charles Parent <cha.parent(a)gmail.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: Charles Parent <cha.parent(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 01 Mar 2022 19:42:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Add FTDI FT4233H
by Charles Parent (Code Review) 01 Mar '22

01 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Charles Parent has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62480 ) Change subject: ft2232_spi.c: Add FTDI FT4233H ...................................................................... Patch Set 2: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62480/comment/35720ccd_4fecdae1 PS1, Line 7: Add FTDI FT4233H > nit: add the file as prefix: […] Done https://review.coreboot.org/c/flashrom/+/62480/comment/b74cc0e3_ec90024d PS1, Line 8: > Has this change been tested? If so, how? I'd appreciate if you could add this information to the com […] Yes it has been tested to interact with a MT25QU256. I will add the logs as well on the mailing list has flashrom indicated that this EEPROM had not been tested with this library https://review.coreboot.org/c/flashrom/+/62480/comment/fe62fd09_5f996c4f PS1, Line 9: Change-Id: I73cee8fd2a6613a8fbc26508d99bbe67da2b4f72 > You'd need to sign-off the commit as per https://flashrom. […] Done Patchset: PS2: Hello Angel, I just resolved the issues you mentionned -- To view, visit https://review.coreboot.org/c/flashrom/+/62480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73cee8fd2a6613a8fbc26508d99bbe67da2b4f72 Gerrit-Change-Number: 62480 Gerrit-PatchSet: 2 Gerrit-Owner: Charles Parent <cha.parent(a)gmail.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: Tue, 01 Mar 2022 13:20:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Alder Lake support
by Nico Huber (Code Review) 01 Mar '22

01 Mar '22

Attention is currently required from: Rizwan Qureshi, Edward O'Callaghan. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 ) Change subject: ichspi: Add Alder Lake support ...................................................................... Patch Set 12: (1 comment) Patchset: PS12: Same question as for Jasper Lake, does there anything differ from the previous generation so we need a new enum entry? (not saying there is no difference, haven't looked into docs yet) -- To view, visit https://review.coreboot.org/c/flashrom/+/62251 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf Gerrit-Change-Number: 62251 Gerrit-PatchSet: 12 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Mar 2022 13:19:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Add FTDI FT4233H
by Charles Parent (Code Review) 01 Mar '22

01 Mar '22

Attention is currently required from: Nico Huber, Charles Parent. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62480 to look at the new patch set (#2). Change subject: ft2232_spi.c: Add FTDI FT4233H ...................................................................... ft2232_spi.c: Add FTDI FT4233H Change tested to probe, read, write and erase a MT25QU256 through a FT4233H FTDI Change-Id: I73cee8fd2a6613a8fbc26508d99bbe67da2b4f72 Signed-off-by: Charles Parent <charles.parent(a)orolia2s.com> --- M ft2232_spi.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/62480/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73cee8fd2a6613a8fbc26508d99bbe67da2b4f72 Gerrit-Change-Number: 62480 Gerrit-PatchSet: 2 Gerrit-Owner: Charles Parent <cha.parent(a)gmail.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: Charles Parent <cha.parent(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Add Alder Lake support
by Angel Pons (Code Review) 01 Mar '22

01 Mar '22

Attention is currently required from: Nico Huber, Rizwan Qureshi, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 ) Change subject: ichspi: Add Alder Lake support ...................................................................... Patch Set 12: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62251/comment/2e13eff3_09e0111a PS12, Line 12: TEST=```localhost ~ # flashrom --flash-name Any other tests, e.g. reading? If the ME region is locked, you can still read the other regions. A good test would be: flashrom -p internal --ifd -i fd -i bios -r filename.rom This makes use of flashrom's IFD parsing code to decide which parts of the flash chip should be read. This tests the changes to `guess_ich_chipset_from_content()`, please check the flashrom log to verify the correct chipset is detected. -- To view, visit https://review.coreboot.org/c/flashrom/+/62251 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf Gerrit-Change-Number: 62251 Gerrit-PatchSet: 12 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Mar 2022 10:27:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add FTDI FT4233H
by Angel Pons (Code Review) 01 Mar '22

01 Mar '22

Attention is currently required from: Charles Parent. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62480 ) Change subject: Add FTDI FT4233H ...................................................................... Patch Set 1: Code-Review+1 (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62480/comment/ff803ef4_47b286f9 PS1, Line 7: Add FTDI FT4233H nit: add the file as prefix: ft2232_spi.c: Add FTDI FT4233H https://review.coreboot.org/c/flashrom/+/62480/comment/686f1853_47fd5023 PS1, Line 8: Has this change been tested? If so, how? I'd appreciate if you could add this information to the commit message. https://review.coreboot.org/c/flashrom/+/62480/comment/2431836c_f712f852 PS1, Line 9: Change-Id: I73cee8fd2a6613a8fbc26508d99bbe67da2b4f72 You'd need to sign-off the commit as per https://flashrom.org/Development_Guidelines#Sign-off_Procedure Patchset: PS1: Welcome! Looks good, just some comments regarding the commit message. You can update the commit message by amending the commit and repushing: as long as the Change-Id remains the same, Gerrit will treat the new commit as a new patchset (version) of this change. Or you can update the commit message directly using Gerrit's Web UI. -- To view, visit https://review.coreboot.org/c/flashrom/+/62480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73cee8fd2a6613a8fbc26508d99bbe67da2b4f72 Gerrit-Change-Number: 62480 Gerrit-PatchSet: 1 Gerrit-Owner: Charles Parent <cha.parent(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Charles Parent <cha.parent(a)gmail.com> Gerrit-Comment-Date: Tue, 01 Mar 2022 10:22:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022