- flashrom-gerrit - mail.coreboot.org

[XL] Change in flashrom[master]: move manpage to sphinx
by Thomas Heijligen (Code Review) 20 Feb '23

20 Feb '23

Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72619 to look at the new patch set (#7). Change subject: move manpage to sphinx ...................................................................... move manpage to sphinx Use sphinx (sphinx-doc.org) to generate the UNIX man page from an reStructuredText file instead of dealing with plain groff. Use `meson setup -Dman-pages=enabled ...` to build the man page, and `meson setup -Ddocumentation=enabled ..` to build the web documentation. Both are enabled automaticly if sphix-build is found. The install paths are `share/man/man8/flashrom.8` and `share/doc/flashrom/index.html` The Makefile builds only the man-page format. Change-Id: Iee9f1164c5913e47385e6f7d51dc7775a58b5a67 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M .gitignore M Makefile A doc/classic_cli_manpage.rst A doc/conf.py A doc/index.rst A doc/meson.build D flashrom.8.tmpl M meson.build M meson_options.txt 9 files changed, 1,461 insertions(+), 1,861 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/19/72619/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/72619 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee9f1164c5913e47385e6f7d51dc7775a58b5a67 Gerrit-Change-Number: 72619 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(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-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: dummyflasher: use new API to register shutdown function
by Anastasia Klimchuk (Code Review) 20 Feb '23

20 Feb '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/72408 ) Change subject: dummyflasher: use new API to register shutdown function ...................................................................... dummyflasher: use new API to register shutdown function This allows masters to register shutdown function in *_master struct, which means there is no need to call register_shutdown in init function, since this call is now a part of register_*_master. A dummy programmer can register masters for multiple buses that share a programmer's data (a pointer to struct emu_data) with each other. To avoid unexpected memory freeing by shutdown function, we need to keep track of how many buses are using the shared resource. Use the reference counting technique to achieve this. TEST=ninja test Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Ticket: https://ticket.coreboot.org/issues/391 Reviewed-on: https://review.coreboot.org/c/flashrom/+/72408 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M dummyflasher.c 1 file changed, 52 insertions(+), 15 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/dummyflasher.c b/dummyflasher.c index 2bdd10c..0897332 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -75,6 +75,10 @@ unsigned int spi_write_256_chunksize; uint8_t *flashchip_contents; + + /* An instance of this structure is shared between multiple masters, so + * store the number of references to clean up only once at shutdown time. */ + uint8_t refs_cnt; }; /* A legit complete SFDP table based on the MX25L6436E (rev. 1.8) datasheet. */ @@ -905,6 +909,11 @@ { msg_pspew("%s\n", __func__); struct emu_data *emu_data = (struct emu_data *)data; + + emu_data->refs_cnt--; + if (emu_data->refs_cnt != 0) + return 0; + if (emu_data->emu_chip != EMULATE_NONE) { if (emu_data->emu_persistent_image && emu_data->emu_modified) { msg_pdbg("Writing %s\n", emu_data->emu_persistent_image); @@ -933,6 +942,7 @@ .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = dummy_spi_write_256, + .shutdown = dummy_shutdown, .probe_opcode = dummy_spi_probe_opcode, .delay = dummy_nop_delay, }; @@ -948,15 +958,17 @@ .chip_writew = dummy_chip_writew, .chip_writel = dummy_chip_writel, .chip_writen = dummy_chip_writen, + .shutdown = dummy_shutdown, .delay = dummy_nop_delay, }; static const struct opaque_master opaque_master_dummyflasher = { - .probe = probe_variable_size, - .read = dummy_opaque_read, - .write = dummy_opaque_write, - .erase = dummy_opaque_erase, - .delay = dummy_nop_delay, + .probe = probe_variable_size, + .read = dummy_opaque_read, + .write = dummy_opaque_write, + .erase = dummy_opaque_erase, + .shutdown = dummy_shutdown, + .delay = dummy_nop_delay, }; static int init_data(const struct programmer_cfg *cfg, @@ -1410,21 +1422,20 @@ } dummy_init_out: - if (register_shutdown(dummy_shutdown, data)) { - free(data->emu_persistent_image); - free(data->flashchip_contents); - free(data); - return 1; - } - - if (dummy_buses_supported & BUS_PROG) + if (dummy_buses_supported & BUS_PROG) { + data->refs_cnt++; ret |= register_opaque_master(&opaque_master_dummyflasher, data); - if (dummy_buses_supported & BUS_NONSPI) + } + if ((dummy_buses_supported & BUS_NONSPI) && !ret) { + data->refs_cnt++; ret |= register_par_master(&par_master_dummyflasher, dummy_buses_supported & BUS_NONSPI, data); - if (dummy_buses_supported & BUS_SPI) + } + if ((dummy_buses_supported & BUS_SPI) && !ret) { + data->refs_cnt++; ret |= register_spi_master(&spi_master_dummyflasher, data); + } return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/72408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Gerrit-Change-Number: 72408 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[L] Change in flashrom[master]: ch347_spi.c: Add initial support for the WCH CH347
by Alexander Goncharov (Code Review) 20 Feb '23

20 Feb '23

Attention is currently required from: Thomas Heijligen, Angel Pons, qianfan, Anastasia Klimchuk, Nicholas Chin. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70573 ) Change subject: ch347_spi.c: Add initial support for the WCH CH347 ...................................................................... Patch Set 4: (11 comments) Patchset: PS4: Thank you so much for your work! I have just few comments. File ch347_spi.c: https://review.coreboot.org/c/flashrom/+/70573/comment/78d0f7b9_1b81bf18 PS4, Line 55: struct ch347_spi_data { : struct libusb_device_handle *handle; : }; Place this declaration before `devs_ch347_spi` variable. https://review.coreboot.org/c/flashrom/+/70573/comment/c16eef57_e1a6991a PS4, Line 62: if (ch347_data->handle == NULL) : return -1; drop condition https://review.coreboot.org/c/flashrom/+/70573/comment/5b62cb50_8ac45a05 PS4, Line 72: ch347_data->handle = NULL; in the next line of the code, we do `free(data)`, so there's no reason to care about this pointer https://review.coreboot.org/c/flashrom/+/70573/comment/d996c3ac_9290667c PS4, Line 109: uint8_t buffer[CH347_PACKET_SIZE] = {0}; How about moving this variable to the rest of data? So the code will be after the data. https://review.coreboot.org/c/flashrom/+/70573/comment/67230b02_a8440e57 PS4, Line 265: struct libusb_device_handle *handle = NULL; : struct ch347_spi_data *ch347_data = calloc(1, sizeof(*ch347_data)); As far as you allocate `ch347_spi_data` at the beginning of initialization, you can drop `struct libusb_device_handle *handle` variable. https://review.coreboot.org/c/flashrom/+/70573/comment/56a7cc8d_4f36d55c PS4, Line 269: -1 Please, use `1` value just to be consistency with other initialization functions. https://review.coreboot.org/c/flashrom/+/70573/comment/0f9739b2_345b5fa9 PS4, Line 275: -1 Same here, `1` https://review.coreboot.org/c/flashrom/+/70573/comment/bce6fca4_e65d55ca PS4, Line 287: handle ch347_data->handle = ...; https://review.coreboot.org/c/flashrom/+/70573/comment/4f8a3336_3e142fae PS4, Line 290: -1 Same here, `1` https://review.coreboot.org/c/flashrom/+/70573/comment/71912292_99ca8a5d PS4, Line 335: -1 Same here, `1` -- To view, visit https://review.coreboot.org/c/flashrom/+/70573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba Gerrit-Change-Number: 70573 Gerrit-PatchSet: 4 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Mon, 20 Feb 2023 08:25:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: ch347_spi.c: Add initial support for the WCH CH347
by Nicholas Chin (Code Review) 19 Feb '23

19 Feb '23

Attention is currently required from: Thomas Heijligen, Angel Pons, qianfan, Anastasia Klimchuk. Nicholas Chin has removed Felix Singer from this change. ( https://review.coreboot.org/c/flashrom/+/70573 ) Change subject: ch347_spi.c: Add initial support for the WCH CH347 ...................................................................... Removed reviewer Felix Singer <felixsinger(a)posteo.net>. -- To view, visit https://review.coreboot.org/c/flashrom/+/70573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba Gerrit-Change-Number: 70573 Gerrit-PatchSet: 4 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: deleteReviewer

1 0

[L] Change in flashrom[master]: ch347_spi.c: Add initial support for the WCH CH347
by Nicholas Chin (Code Review) 19 Feb '23

19 Feb '23

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons, qianfan, Anastasia Klimchuk. Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70573 ) Change subject: ch347_spi.c: Add initial support for the WCH CH347 ...................................................................... Patch Set 4: (11 comments) Patchset: PS3: > After a number of discussions in several irc channels, the conclusion is: this patch is going ahead […] I believe I've done all those things now. Let me know if I'm still missing anything File ch347_spi.c: https://review.coreboot.org/c/flashrom/+/70573/comment/93507707_1148e153 PS3, Line 62: /* TODO: Set this depending on the mode */ > new line before Done https://review.coreboot.org/c/flashrom/+/70573/comment/6d23e2c2_4b5fd952 PS3, Line 68: ch347_data->handle = NULL; > new line before Done https://review.coreboot.org/c/flashrom/+/70573/comment/e5cc41ef_ff0f8c97 PS3, Line 69: free(data); > Yes please, free the data :) […] Done https://review.coreboot.org/c/flashrom/+/70573/comment/5cca6826_fb595c44 PS3, Line 106: uint8_t *buffer = malloc(packet_len); > +1 Done https://review.coreboot.org/c/flashrom/+/70573/comment/e48f5f4f_6d7a846c PS3, Line 123: Could not read write response > It is confusing, given that both "read" and "write" are the operations, what does it mean "Could not […] Done https://review.coreboot.org/c/flashrom/+/70573/comment/7b37ff3f_c0d40a97 PS3, Line 145: malloc(CH347_MAX_DATA_LEN + 3); > Same as above Done https://review.coreboot.org/c/flashrom/+/70573/comment/f26aef5e_6dd69049 PS3, Line 149: command_buf[0] = CH347_CMD_SPI_IN; : command_buf[1] = 4; : command_buf[2] = 0; : command_buf[3] = readcnt & 0xFF; : command_buf[4] = (readcnt & 0xFF00) >> 8; : command_buf[5] = (readcnt & 0xFF0000) >> 16; : command_buf[6] = (readcnt & 0xFF000000) >> 24; > Use an initialiser as done in `ch347_set_cs()`? Done https://review.coreboot.org/c/flashrom/+/70573/comment/75fcdd49_4180b609 PS3, Line 223: -3 > nit: spacing here and in the line below Done https://review.coreboot.org/c/flashrom/+/70573/comment/b568f0e4_1aabbdec PS3, Line 222: buff[0] = CH347_CMD_SPI_CFG; : buff[1] = (sizeof(buff)-3) & 0xFF; : buff[2] = ((sizeof(buff)-3) & 0xFF00) >> 8; : /* Not sure what these two bytes do, but the vendor : * drivers seem to unconditionally set these values : */ : buff[5] = 4; : buff[6] = 1; : /* Clock polarity: bit 1 */ : buff[9] = 0; : /* Clock phase: bit 0 */ : buff[11] = 0; : /* Another mystery byte */ : buff[14] = 2; : /* Clock divisor: bits 5:3 */ : buff[15] = (divisor & 0x7) << 3; : /* Bit order: bit 7, 0=MSB */ : buff[17] = 0; : /* Yet another mystery byte */ : buff[19] = 7; : /* CS polarity: bit 7 CS2, bit 6 CS1. 0 = active low */ : buff[24] = 0; > Use an initialiser as done in `ch347_set_cs()`? Done. I had initially done it like this as I was previously using `buff` to read the original SPI configuration and then overwriting it with these values, as I thought that was a necessary step in order for the SPI_CFG command to respond. Seems like reading the initial config isn't actually necessary so I removed the initial read and initialized `buff` with the config. https://review.coreboot.org/c/flashrom/+/70573/comment/2b8fb497_70d08469 PS3, Line 261: 64 * 1024 > I was reading over some of Thomas's comments in CB:72057 (Patchset 19, lines 120 and 121) and it see […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/70573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba Gerrit-Change-Number: 70573 Gerrit-PatchSet: 4 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 20 Feb 2023 03:40:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: ch347_spi.c: Add initial support for the WCH CH347
by Nicholas Chin (Code Review) 19 Feb '23

19 Feb '23

Attention is currently required from: Thomas Heijligen, Angel Pons, qianfan. Hello build bot (Jenkins), Thomas Heijligen, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70573 to look at the new patch set (#4). Change subject: ch347_spi.c: Add initial support for the WCH CH347 ...................................................................... ch347_spi.c: Add initial support for the WCH CH347 Add support for the WCH CH347, a high-speed USB to bus converter supporting multiple protocols interfaces including SPI. Currently only mode 1 (vendor defined communication interface) is supported, mode 2 (USB HID communication interface) support will be added later. The code is currently hard coded to use a CS1 and a SPI clock of 60 MHz, though there are 2 CS lines and 6 other GPIO lines available, as well as a configurable clock divisor. Support for these will be exposed through programmer parameters in later commits. This currently uses the synchronous libusb API. Performance seems to be alright so far, if it becomes an issue I may switch to the asynchronous API. Tested with a MX25L1606E flash chip and a hard coded divisor of 2 for a SPI clock speed of 15 MHz, as I was having signal integrity issues at higher clock speeds. Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba --- M MAINTAINERS M Makefile A ch347_spi.c M flashrom.8.tmpl M include/programmer.h M meson.build M meson_options.txt M programmer_table.c M test_build.sh 9 files changed, 410 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/70573/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/70573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba Gerrit-Change-Number: 70573 Gerrit-PatchSet: 4 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: qianfan <qianfanguijin(a)163.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: Add support for VIA VL805 USB controller flashing
by ChrisEric1 CECL (Code Review) 18 Feb '23

18 Feb '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Hello build bot (Jenkins), Thomas Heijligen, Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72057 to look at the new patch set (#22). Change subject: Add support for VIA VL805 USB controller flashing ...................................................................... Add support for VIA VL805 USB controller flashing It works fine for me on my Raspberry Pi 4 Model B. Was able read read, erase, write, verify. Only thing is the WP with the W25X10 gives a untested warning. Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38 Signed-off-by: Christopher Lentocha <christopherericlentocha(a)gmail.com> --- M Makefile M flashrom.8.tmpl M include/programmer.h M meson.build M meson_options.txt M programmer_table.c A vl805.c 7 files changed, 212 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/72057/22 -- To view, visit https://review.coreboot.org/c/flashrom/+/72057 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38 Gerrit-Change-Number: 72057 Gerrit-PatchSet: 22 Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: Add support for VIA VL805 USB Controller flashing
by ChrisEric1 CECL (Code Review) 18 Feb '23

18 Feb '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. ChrisEric1 CECL has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72057 ) Change subject: Add support for VIA VL805 USB Controller flashing ...................................................................... Patch Set 21: (15 comments) File vl805.c: https://review.coreboot.org/c/flashrom/+/72057/comment/685ebe4b_bac9b1b3 PS18, Line 70: unsigned int i, j; > delete Done https://review.coreboot.org/c/flashrom/+/72057/comment/5bcf1ffe_472bb996 PS18, Line 71: uint32_t outdata; : uint32_t indata; : unsigned int curwritecnt; : unsigned int curreadcnt; > scope to respective loop constructs.. Assuming you rewrote what you meant later. File vl805.c: https://review.coreboot.org/c/flashrom/+/72057/comment/2acaf72c_5e1c7784 PS20, Line 24: #include "programmer.h" : #include "spi.h" : #include "flash.h" : #include "platform/pci.h" : #include <stdlib.h> > ``` […] Done https://review.coreboot.org/c/flashrom/+/72057/comment/d0ecb84c_71a9f3c4 PS20, Line 70: uint32_t outdata; : uint32_t indata; > delete Done https://review.coreboot.org/c/flashrom/+/72057/comment/e8dc4ce5_3542903c PS20, Line 75: for (j = 0; j < writecnt; j += 4) { > `for (unsigned int i = 0; i < writecnt; i += 4) {` Done https://review.coreboot.org/c/flashrom/+/72057/comment/6d7455e0_805fabcf PS20, Line 76: j > i Done https://review.coreboot.org/c/flashrom/+/72057/comment/eaaf8640_64fda1e6 PS20, Line 76: u > const Done https://review.coreboot.org/c/flashrom/+/72057/comment/01c3b724_afeb1f6e PS20, Line 77: o > you can just do: […] Done https://review.coreboot.org/c/flashrom/+/72057/comment/23533c2b_40d2f9d8 PS20, Line 78: for (i = 0; i < curwritecnt; i++) { > `for (unsigned int j = 0; j < curwritecnt; j++) {` Done https://review.coreboot.org/c/flashrom/+/72057/comment/0bc41809_8c5f4b53 PS20, Line 89: for (j = 0; j < readcnt; j += 4) { > `for (unsigned int i = 0; i < readcnt; i += 4) {` Done https://review.coreboot.org/c/flashrom/+/72057/comment/0b66f310_80affdc6 PS20, Line 90: j > i Done https://review.coreboot.org/c/flashrom/+/72057/comment/04954815_54a7504b PS20, Line 90: u > const Done https://review.coreboot.org/c/flashrom/+/72057/comment/9ef6592f_e7eab7cf PS20, Line 92: indata > `uint32_t data =` Done https://review.coreboot.org/c/flashrom/+/72057/comment/39c3b489_b57b0e3a PS20, Line 93: for (i = 0; i < curreadcnt; i++) { > `for (unsigned int j = 0; j < curreadcnt; j++) {` Done https://review.coreboot.org/c/flashrom/+/72057/comment/ce7f2125_668bc2f5 PS20, Line 94: i > j Done -- To view, visit https://review.coreboot.org/c/flashrom/+/72057 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38 Gerrit-Change-Number: 72057 Gerrit-PatchSet: 21 Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 19 Feb 2023 01:16:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: Add support for VIA VL805 USB Controller flashing
by ChrisEric1 CECL (Code Review) 18 Feb '23

18 Feb '23

Attention is currently required from: ChrisEric1 CECL, Thomas Heijligen. Hello build bot (Jenkins), Thomas Heijligen, Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72057 to look at the new patch set (#21). Change subject: Add support for VIA VL805 USB Controller flashing ...................................................................... Add support for VIA VL805 USB Controller flashing It works fine for me on my Raspberry Pi 4 Model B. Was able read read, erase, write, verify. Only thing is the WP with the W25X10 gives a untested warning. Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38 Signed-off-by: Christopher Lentocha <christopherericlentocha(a)gmail.com> --- M Makefile M flashrom.8.tmpl M include/programmer.h M meson.build M meson_options.txt M programmer_table.c A vl805.c 7 files changed, 212 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/72057/21 -- To view, visit https://review.coreboot.org/c/flashrom/+/72057 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38 Gerrit-Change-Number: 72057 Gerrit-PatchSet: 21 Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c: Trivial code style fix
by Edward O'Callaghan (Code Review) 16 Feb '23

16 Feb '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/72968 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flashrom.c: Trivial code style fix ...................................................................... flashrom.c: Trivial code style fix Change-Id: Iacdb62067a8d22261d4eabe73ad96168eb11417c Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/72968 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sam McNally <sammc(a)google.com> --- M flashrom.c 1 file changed, 14 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index d17e04b..62e5757 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1282,8 +1282,7 @@ ret = 1; } } - if(curr_eraseblock_count > prev_eraseblock_count) - { + if (curr_eraseblock_count > prev_eraseblock_count) { msg_gerr("ERROR: Flash chip %s erase function %i is not " "in order. Please report a bug at flashrom(a)flashrom.org\n", chip->name, k); -- To view, visit https://review.coreboot.org/c/flashrom/+/72968 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iacdb62067a8d22261d4eabe73ad96168eb11417c Gerrit-Change-Number: 72968 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit