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

Change in flashrom[master]: pickit2_spi.c: Refactor singleton states into reentrant pattern
by Nico Huber (Code Review) 12 May '21

12 May '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52774 ) Change subject: pickit2_spi.c: Refactor singleton states into reentrant pattern ...................................................................... pickit2_spi.c: Refactor singleton states into reentrant pattern Move global singleton states into a struct and store within the spi_master data field for the life-time of the driver. This is one of the steps on the way to move spi_master data memory management behind the initialisation API, for more context see other patches under the same topic "register_master_api". TEST=builds BUG=b:185191942 Change-Id: Ibacc4738bee02c371c41583d321e0337128ad18a Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52774 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M pickit2_spi.c 1 file changed, 37 insertions(+), 16 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Angel Pons: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved diff --git a/pickit2_spi.c b/pickit2_spi.c index 367d0fc..3d0c775 100644 --- a/pickit2_spi.c +++ b/pickit2_spi.c @@ -52,7 +52,9 @@ {0} }; -static libusb_device_handle *pickit2_handle; +struct pickit2_spi_data { + libusb_device_handle *pickit2_handle; +}; /* Default USB transaction timeout in ms */ #define DFLT_TIMEOUT 10000 @@ -89,7 +91,7 @@ #define SCR_VDD_OFF 0xFE #define SCR_VDD_ON 0xFF -static int pickit2_get_firmware_version(void) +static int pickit2_get_firmware_version(libusb_device_handle *pickit2_handle) { int ret; uint8_t command[CMD_LENGTH] = {CMD_GET_VERSION, CMD_END_OF_BUFFER}; @@ -113,7 +115,7 @@ return 0; } -static int pickit2_set_spi_voltage(int millivolt) +static int pickit2_set_spi_voltage(libusb_device_handle *pickit2_handle, int millivolt) { double voltage_selector; switch (millivolt) { @@ -172,7 +174,7 @@ { NULL, 0x0 }, }; -static int pickit2_set_spi_speed(unsigned int spispeed_idx) +static int pickit2_set_spi_speed(libusb_device_handle *pickit2_handle, unsigned int spispeed_idx) { msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed_idx].name); @@ -198,6 +200,7 @@ static int pickit2_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { + struct pickit2_spi_data *pickit2_data = flash->mst->spi.data; /* Maximum number of bytes per transaction (including command overhead) is 64. Lets play it safe * and always assume the worst case scenario of 20 bytes command overhead. @@ -255,7 +258,8 @@ buf[i++] = CMD_END_OF_BUFFER; int transferred; - int ret = libusb_interrupt_transfer(pickit2_handle, ENDPOINT_OUT, buf, CMD_LENGTH, &transferred, DFLT_TIMEOUT); + int ret = libusb_interrupt_transfer(pickit2_data->pickit2_handle, + ENDPOINT_OUT, buf, CMD_LENGTH, &transferred, DFLT_TIMEOUT); if (ret != 0) { msg_perr("Send SPI failed!\n"); @@ -264,7 +268,8 @@ if (readcnt) { int length = 0; - ret = libusb_interrupt_transfer(pickit2_handle, ENDPOINT_IN, buf, CMD_LENGTH, &length, DFLT_TIMEOUT); + ret = libusb_interrupt_transfer(pickit2_data->pickit2_handle, + ENDPOINT_IN, buf, CMD_LENGTH, &length, DFLT_TIMEOUT); if (length == 0 || ret != 0) { msg_perr("Receive SPI failed\n"); @@ -335,7 +340,7 @@ return millivolt; } -static const struct spi_master spi_master_pickit2 = { +static struct spi_master spi_master_pickit2 = { .max_data_read = 40, .max_data_write = 40, .command = pickit2_spi_send_command, @@ -347,6 +352,8 @@ static int pickit2_shutdown(void *data) { + struct pickit2_spi_data *pickit2_data = data; + /* Set all pins to float and turn voltages off */ uint8_t command[CMD_LENGTH] = { CMD_EXEC_SCRIPT, @@ -363,18 +370,21 @@ }; int transferred; - int ret = libusb_interrupt_transfer(pickit2_handle, ENDPOINT_OUT, command, CMD_LENGTH, &transferred, DFLT_TIMEOUT); + int ret = libusb_interrupt_transfer(pickit2_data->pickit2_handle, + ENDPOINT_OUT, command, CMD_LENGTH, &transferred, DFLT_TIMEOUT); if (ret != 0) { msg_perr("Command Shutdown failed!\n"); ret = 1; } - if (libusb_release_interface(pickit2_handle, 0) != 0) { + if (libusb_release_interface(pickit2_data->pickit2_handle, 0) != 0) { msg_perr("Could not release USB interface!\n"); ret = 1; } - libusb_close(pickit2_handle); + libusb_close(pickit2_data->pickit2_handle); libusb_exit(NULL); + + free(data); return ret; } @@ -397,7 +407,8 @@ CMD_END_OF_BUFFER }; - + libusb_device_handle *pickit2_handle; + struct pickit2_spi_data *pickit2_data; int spispeed_idx = 0; char *spispeed = extract_programmer_param("spispeed"); if (spispeed != NULL) { @@ -458,16 +469,26 @@ return 1; } - if (pickit2_get_firmware_version()) + pickit2_data = calloc(1, sizeof(*pickit2_data)); + if (!pickit2_data) { + msg_perr("Unable to allocate space for SPI master data\n"); + libusb_close(pickit2_handle); + libusb_exit(NULL); + return 1; + } + pickit2_data->pickit2_handle = pickit2_handle; + spi_master_pickit2.data = pickit2_data; + + if (pickit2_get_firmware_version(pickit2_handle)) goto init_err_cleanup_exit; /* Command Set SPI Speed */ - if (pickit2_set_spi_speed(spispeed_idx)) + if (pickit2_set_spi_speed(pickit2_handle, spispeed_idx)) goto init_err_cleanup_exit; /* Command Set SPI Voltage */ msg_pdbg("Setting voltage to %i mV.\n", millivolt); - if (pickit2_set_spi_voltage(millivolt) != 0) + if (pickit2_set_spi_voltage(pickit2_handle, millivolt) != 0) goto init_err_cleanup_exit; /* Perform basic setup. @@ -478,13 +499,13 @@ goto init_err_cleanup_exit; } - if (register_shutdown(pickit2_shutdown, NULL)) + if (register_shutdown(pickit2_shutdown, pickit2_data)) goto init_err_cleanup_exit; register_spi_master(&spi_master_pickit2); return 0; init_err_cleanup_exit: - pickit2_shutdown(NULL); + pickit2_shutdown(pickit2_data); return 1; } -- To view, visit https://review.coreboot.org/c/flashrom/+/52774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibacc4738bee02c371c41583d321e0337128ad18a Gerrit-Change-Number: 52774 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: pickit2_spi.c: Separate shutdown from failed init cleanup
by Nico Huber (Code Review) 12 May '21

12 May '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52773 ) Change subject: pickit2_spi.c: Separate shutdown from failed init cleanup ...................................................................... pickit2_spi.c: Separate shutdown from failed init cleanup Shutdown function was covering two different jobs here: 1) the actual shutdown which is run at the end of the driver's lifecycle and 2) cleanup in cases when initialisation failed. Now, shutdown is only doing its main job (#1), and the driver itself is doing cleanup when init fails (#2). The good thing is that now resources are released/closed immediately in cases when init fails (vs shutdown function which was run at some point later), and the driver leaves clean space after itself if init fails. And very importantly this unlocks API change which plans to move register_shutdown inside register master API, see https://review.coreboot.org/c/flashrom/+/51761 TEST=builds BUG=b:185191942 Change-Id: I1b672b33169a7a1b6ceab190ad3f48c2f35c3a1f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52773 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M pickit2_spi.c 1 file changed, 13 insertions(+), 14 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/pickit2_spi.c b/pickit2_spi.c index 0bc17af..367d0fc 100644 --- a/pickit2_spi.c +++ b/pickit2_spi.c @@ -458,34 +458,33 @@ return 1; } - if (register_shutdown(pickit2_shutdown, NULL) != 0) { - return 1; - } - - if (pickit2_get_firmware_version()) { - return 1; - } + if (pickit2_get_firmware_version()) + goto init_err_cleanup_exit; /* Command Set SPI Speed */ - if (pickit2_set_spi_speed(spispeed_idx)) { - return 1; - } + if (pickit2_set_spi_speed(spispeed_idx)) + goto init_err_cleanup_exit; /* Command Set SPI Voltage */ msg_pdbg("Setting voltage to %i mV.\n", millivolt); - if (pickit2_set_spi_voltage(millivolt) != 0) { - return 1; - } + if (pickit2_set_spi_voltage(millivolt) != 0) + goto init_err_cleanup_exit; /* Perform basic setup. * Configure pin directions and logic levels, turn Vdd on, turn busy LED on and clear buffers. */ int transferred; if (libusb_interrupt_transfer(pickit2_handle, ENDPOINT_OUT, buf, CMD_LENGTH, &transferred, DFLT_TIMEOUT) != 0) { msg_perr("Command Setup failed!\n"); - return 1; + goto init_err_cleanup_exit; } + if (register_shutdown(pickit2_shutdown, NULL)) + goto init_err_cleanup_exit; register_spi_master(&spi_master_pickit2); return 0; + +init_err_cleanup_exit: + pickit2_shutdown(NULL); + return 1; } -- To view, visit https://review.coreboot.org/c/flashrom/+/52773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b672b33169a7a1b6ceab190ad3f48c2f35c3a1f Gerrit-Change-Number: 52773 Gerrit-PatchSet: 2 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Anastasia Klimchuk (Code Review) 12 May '21

12 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > Thanks! Yes, let's add data pointer early. […] Just went through my patches once again, there are three which have +2s, everyone had a look, and no comments - probably could be submitted? CB:52773 CB:52774 CB:54044 If you submit them (unless you have comments of course!), those files will go into CB:54067. That would be really really nice! -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 23:12:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: Fix 4BA advertisement for dummy chip
by Nico Huber (Code Review) 12 May '21

12 May '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54068 ) Change subject: flashchips: Fix 4BA advertisement for dummy chip ...................................................................... flashchips: Fix 4BA advertisement for dummy chip The dummyflasher only supports the native 4BA read and write commands, so only advertise these. Change-Id: Ia7340835ce1680d197f250bdb5990ab2ffe3671f Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54068 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M flashchips.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 6482f67..007bcc9 100644 --- a/flashchips.c +++ b/flashchips.c @@ -19569,7 +19569,7 @@ .model_id = PROGDEV_ID, .total_size = 64, /* This size is set temporarily */ .page_size = 256, - .feature_bits = FEATURE_4BA, + .feature_bits = FEATURE_4BA_READ | FEATURE_4BA_WRITE, .tested = TEST_OK_PREW, .probe = probe_variable_size, .block_erasers = -- To view, visit https://review.coreboot.org/c/flashrom/+/54068 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia7340835ce1680d197f250bdb5990ab2ffe3671f Gerrit-Change-Number: 54068 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: stlinkv3_spi.c: Refactor singleton states into reentrant pattern
by Nico Huber (Code Review) 12 May '21

12 May '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54042 ) Change subject: stlinkv3_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 1: (1 comment) File stlinkv3_spi.c: https://review.coreboot.org/c/flashrom/+/54042/comment/aadf8a9f_251ffb76 PS1, Line 533: stlinkv3_spi_open(sck_freq_kHz, stlinkv3_handle > >>> Before this commit, the handle was closed unless register_shutdown() failed. […] It's just the one more `goto err_exit` that you added (in case calloc fails). It's one more case where resources are left open than before. It's tiny compared to the other flaws around. So don't worry too much about it. I just realized there's even a double-libusb_exit() in case register_ spi_master() fails... and a new double-free() in this patch. So yes, I think it's probably best to fix the error paths first. This change would be easier to review if the code was sane already. I can also fix things, it's more my fault that the code looks like this ;) -- To view, visit https://review.coreboot.org/c/flashrom/+/54042 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id044661b864b506028720ea809bc524f0640469f Gerrit-Change-Number: 54042 Gerrit-PatchSet: 1 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: Miklós Márton <martonmiklosqdev(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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 23:01:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> 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

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Anastasia Klimchuk (Code Review) 12 May '21

12 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: > I know about CB:51761. It is focused on the shutdown pointer, though. I still […] Thanks! Yes, let's add data pointer early. I will do the same for par/opaque and then moving few lines into register_master is internal change, caller won't need to be changed. Some of my patches still need more work (unresolved comments), not all of them have +2 - not all of them ready to submit anyway. So maybe, if you submit this chain of 2 patches soon, I will rebase? File spi.c: https://review.coreboot.org/c/flashrom/+/54066/comment/ad3eb69b_66fd94fe PS1, Line 151: if (data) : rmst.spi.data = data; > You can do it. Oh great, I will do! I think, would better do it now, before I touched any par master, to get all benefits. -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 22:20:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> 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

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Nico Huber (Code Review) 12 May '21

12 May '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Mine was CB:51761 which has everyone +2s and it was the end goal to submit after I do all refactorin […] I know about CB:51761. It is focused on the shutdown pointer, though. I still believe adding the data pointer early makes things easier. Didn't mean to interrupt your work; I can rebase it if you'd push your patches on a single branch. -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 21:33:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: Only enable I2C programmers on Linux
by Edward O'Callaghan (Code Review) 12 May '21

12 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/53999 ) Change subject: Makefile: Only enable I2C programmers on Linux ...................................................................... Makefile: Only enable I2C programmers on Linux The I2C support code they use is Linux-specific. Change-Id: I04d25ff8f8c3097428ac8695669b1757c38f49e9 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/53999 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M Makefile 1 file changed, 10 insertions(+), 30 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/Makefile b/Makefile index dd19e14..b8b40ca 100644 --- a/Makefile +++ b/Makefile @@ -212,16 +212,6 @@ else override CONFIG_STLINKV3_SPI = no endif -ifeq ($(CONFIG_LSPCON_I2C_SPI), yes) -UNSUPPORTED_FEATURES += CONFIG_LSPCON_I2C_SPI=yes -else -override CONFIG_LSPCON_I2C_SPI = no -endif -ifeq ($(CONFIG_REALTEK_MST_I2C_SPI), yes) -UNSUPPORTED_FEATURES += CONFIG_REALTEK_MST_I2C_SPI=yes -else -override CONFIG_REALTEK_MST_I2C_SPI = no -endif # libjaylink is also not available for DOS ifeq ($(CONFIG_JLINK_SPI), yes) UNSUPPORTED_FEATURES += CONFIG_JLINK_SPI=yes @@ -338,16 +328,6 @@ else override CONFIG_SATAMV = no endif -ifeq ($(CONFIG_LSPCON_I2C_SPI), yes) -UNSUPPORTED_FEATURES += CONFIG_LSPCON_I2C_SPI=yes -else -override CONFIG_LSPCON_I2C_SPI = no -endif -ifeq ($(CONFIG_REALTEK_MST_I2C_SPI), yes) -UNSUPPORTED_FEATURES += CONFIG_REALTEK_MST_I2C_SPI=yes -else -override CONFIG_REALTEK_MST_I2C_SPI = no -endif endif ifneq ($(TARGET_OS), MinGW) @@ -433,16 +413,6 @@ else override CONFIG_STLINKV3_SPI = no endif -ifeq ($(CONFIG_LSPCON_I2C_SPI), yes) -UNSUPPORTED_FEATURES += CONFIG_LSPCON_I2C_SPI=yes -else -override CONFIG_LSPCON_I2C_SPI = no -endif -ifeq ($(CONFIG_REALTEK_MST_I2C_SPI), yes) -UNSUPPORTED_FEATURES += CONFIG_REALTEK_MST_I2C_SPI=yes -else -override CONFIG_REALTEK_MST_I2C_SPI = no -endif ifeq ($(CONFIG_CH341A_SPI), yes) UNSUPPORTED_FEATURES += CONFIG_CH341A_SPI=yes else @@ -467,6 +437,16 @@ UNSUPPORTED_FEATURES += CONFIG_MSTARDDC_SPI=yes else override CONFIG_MSTARDDC_SPI = no +ifeq ($(CONFIG_LSPCON_I2C_SPI), yes) +UNSUPPORTED_FEATURES += CONFIG_LSPCON_I2C_SPI=yes +else +override CONFIG_LSPCON_I2C_SPI = no +endif +ifeq ($(CONFIG_REALTEK_MST_I2C_SPI), yes) +UNSUPPORTED_FEATURES += CONFIG_REALTEK_MST_I2C_SPI=yes +else +override CONFIG_REALTEK_MST_I2C_SPI = no +endif endif endif endif -- To view, visit https://review.coreboot.org/c/flashrom/+/53999 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04d25ff8f8c3097428ac8695669b1757c38f49e9 Gerrit-Change-Number: 53999 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: realtek_mst_i2c_spi.c: Rename `PAGE_SIZE` macro
by Edward O'Callaghan (Code Review) 12 May '21

12 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/53998 ) Change subject: realtek_mst_i2c_spi.c: Rename `PAGE_SIZE` macro ...................................................................... realtek_mst_i2c_spi.c: Rename `PAGE_SIZE` macro This fixes building with musl libc on alpine:amd64-v3.9. Change-Id: I043e3d8c2d2498e94b5e7577a7378c8c3e0e6c81 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/53998 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M realtek_mst_i2c_spi.c 1 file changed, 9 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 64579e1..10a1dff 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -28,7 +28,7 @@ #define MCU_I2C_SLAVE_ADDR 0x94 #define REGISTER_ADDRESS (0x94 >> 1) -#define PAGE_SIZE 128 +#define RTK_PAGE_SIZE 128 #define MAX_SPI_WAIT_RETRIES 1000 #define MCU_MODE 0x6F @@ -310,8 +310,8 @@ * Using static buffer with maximum possible size, * extra byte is needed for prefixing the data port register at index 0. */ - uint8_t wbuf[PAGE_SIZE + 1] = { MCU_DATA_PORT }; - if (len > PAGE_SIZE) + uint8_t wbuf[RTK_PAGE_SIZE + 1] = { MCU_DATA_PORT }; + if (len > RTK_PAGE_SIZE) return SPI_GENERIC_ERROR; memcpy(&wbuf[1], buf, len); @@ -352,9 +352,9 @@ uint8_t dummy; realtek_mst_i2c_spi_read_register(fd, MCU_DATA_PORT, &dummy); - for (i = 0; i < len; i += PAGE_SIZE) { + for (i = 0; i < len; i += RTK_PAGE_SIZE) { ret |= realtek_mst_i2c_spi_read_data(fd, REGISTER_ADDRESS, - buf + i, min(len - i, PAGE_SIZE)); + buf + i, min(len - i, RTK_PAGE_SIZE)); if (ret) return ret; } @@ -376,11 +376,11 @@ return SPI_GENERIC_ERROR; ret |= realtek_mst_i2c_spi_write_register(fd, 0x6D, 0x02); /* write opcode */ - ret |= realtek_mst_i2c_spi_write_register(fd, 0x71, (PAGE_SIZE - 1)); /* fit len=256 */ + ret |= realtek_mst_i2c_spi_write_register(fd, 0x71, (RTK_PAGE_SIZE - 1)); /* fit len=256 */ - for (i = 0; i < len; i += PAGE_SIZE) { - uint16_t page_len = min(len - i, PAGE_SIZE); - if (len - i < PAGE_SIZE) + for (i = 0; i < len; i += RTK_PAGE_SIZE) { + uint16_t page_len = min(len - i, RTK_PAGE_SIZE); + if (len - i < RTK_PAGE_SIZE) ret |= realtek_mst_i2c_spi_write_register(fd, 0x71, page_len-1); ret |= realtek_mst_i2c_spi_map_page(fd, start + i); if (ret) -- To view, visit https://review.coreboot.org/c/flashrom/+/53998 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I043e3d8c2d2498e94b5e7577a7378c8c3e0e6c81 Gerrit-Change-Number: 53998 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: flashchips: Fix 4BA advertisement for dummy chip
by Edward O'Callaghan (Code Review) 12 May '21

12 May '21

Attention is currently required from: Nico Huber, Namyoon Woo. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54068 ) Change subject: flashchips: Fix 4BA advertisement for dummy chip ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54068 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia7340835ce1680d197f250bdb5990ab2ffe3671f Gerrit-Change-Number: 54068 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Namyoon Woo <namyoon(a)google.com> Gerrit-Comment-Date: Wed, 12 May 2021 11:53:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021