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

Change in flashrom[master]: linux_mtd: move global state into programmer data field
by Edward O'Callaghan (Code Review) 12 May '21

12 May '21

Attention is currently required from: Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/53947 ) Change subject: linux_mtd: move global state into programmer data field ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/53947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ce6900e4892ed5687cfddb245dfe5461a3e2e84 Gerrit-Change-Number: 53947 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 13 May 2021 00:59:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: clear layout on flashrom_layout_release
by Anastasia Klimchuk (Code Review) 12 May '21

12 May '21

Attention is currently required from: Edward O'Callaghan, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54147 ) Change subject: libflashrom: clear layout on flashrom_layout_release ...................................................................... Patch Set 1: (2 comments) File layout.c: https://review.coreboot.org/c/flashrom/+/54147/comment/8302fa21_570575b4 PS1, Line 296: layout->entries[i].included = false; Should this line also move into flashrom_layout_release() ? https://review.coreboot.org/c/flashrom/+/54147/comment/3fb70b5c_19f1d908 PS1, Line 279: layout_cleanup_args This function is not cleaning up the layout anymore, only args. Maybe you can rename to `cleanup_layout_args`? or `args_cleanup` ? -- To view, visit https://review.coreboot.org/c/flashrom/+/54147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8ba4b356f2a40ccbfd4e91f81af01d5f6a6de515 Gerrit-Change-Number: 54147 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 13 May 2021 00:18:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: clear layout on flashrom_layout_release
by Peter Marheine (Code Review) 12 May '21

12 May '21

Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/54147 ) Change subject: libflashrom: clear layout on flashrom_layout_release ...................................................................... libflashrom: clear layout on flashrom_layout_release This changes the behavior of flashrom_layout_release to always clear the provided layout, even if it is the global layout (as is currently always the case). Not clearing it causes confusing behavior where a user may release a layout and create a new one, but because it's always the global layout which wasn't cleared there will be stale information in the returned layout. This can easily lead to writing unexpected flash regions because the API provides no way to stop including regions, and makes it effectively impossible to stop including a region without completely tearing down the library. Change-Id: I8ba4b356f2a40ccbfd4e91f81af01d5f6a6de515 Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M cli_classic.c M flash.h M layout.c 3 files changed, 7 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/54147/1 diff --git a/cli_classic.c b/cli_classic.c index 5fd8092..9dcfe71 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -852,7 +852,7 @@ for (i = 0; i < chipcount; i++) free(flashes[i].chip); - layout_cleanup(&include_args); + layout_cleanup_args(&include_args); free(filename); free(fmapfile); free(referencefile); diff --git a/flash.h b/flash.h index dd8e156..fbc3539 100644 --- a/flash.h +++ b/flash.h @@ -421,7 +421,7 @@ int register_include_arg(struct layout_include_args **args, const char *arg); int read_romlayout(const char *name); int normalize_romentries(const struct flashctx *flash); -void layout_cleanup(struct layout_include_args **args); +void layout_cleanup_args(struct layout_include_args **args); /* spi.c */ struct spi_command { diff --git a/layout.c b/layout.c index 4d1dd56..f81e01d 100644 --- a/layout.c +++ b/layout.c @@ -276,10 +276,8 @@ return overlap_detected; } -void layout_cleanup(struct layout_include_args **args) +void layout_cleanup_args(struct layout_include_args **args) { - struct flashrom_layout *const layout = get_global_layout(); - unsigned int i; struct layout_include_args *tmp; while (*args) { @@ -289,13 +287,6 @@ free(*args); *args = tmp; } - - for (i = 0; i < layout->num_entries; i++) { - free(layout->entries[i].name); - free(layout->entries[i].file); - layout->entries[i].included = false; - } - layout->num_entries = 0; } /* Validate and - if needed - normalize layout entries. */ @@ -405,14 +396,16 @@ { unsigned int i; - if (!layout || layout == get_global_layout()) + if (!layout) return; for (i = 0; i < layout->num_entries; ++i) { free(layout->entries[i].name); free(layout->entries[i].file); } - free(layout); + layout->num_entries = 0; + if (layout != get_global_layout()) + free(layout); } /** @} */ /* end flashrom-layout */ -- To view, visit https://review.coreboot.org/c/flashrom/+/54147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8ba4b356f2a40ccbfd4e91f81af01d5f6a6de515 Gerrit-Change-Number: 54147 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newchange

1 0

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

12 May '21

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/54067 to look at the new patch set (#3). Change subject: programmer: Make use of new register_spi_master() API ...................................................................... programmer: Make use of new register_spi_master() API Pass pointers to dynamically allocated data to register_spi_master(). This way we can avoid some mutable globals. Change-Id: Id7821f1db3284b7b5b3d0abfd878b979c53870a1 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M digilent_spi.c M dummyflasher.c M ft2232_spi.c M it85spi.c M it87spi.c M jlink_spi.c M linux_spi.c M lspcon_i2c_spi.c M pickit2_spi.c M usbblaster_spi.c M wbsio_spi.c 11 files changed, 22 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/67/54067/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/54067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id7821f1db3284b7b5b3d0abfd878b979c53870a1 Gerrit-Change-Number: 54067 Gerrit-PatchSet: 3 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-MessageType: newpatchset

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: Nico Huber, Edward O'Callaghan. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/54066 to look at the new patch set (#3). Change subject: programmer: Smoothen register_spi_master() API ...................................................................... programmer: Smoothen register_spi_master() API It was impossible to register a const struct spi_master that would point to dynamically allocated `data`. Fix that so that we won't have to create more mutable globals. Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M dediprog.c M digilent_spi.c M dummyflasher.c M ene_lpc.c M ft2232_spi.c M ichspi.c M it85spi.c M it87spi.c M jlink_spi.c M linux_spi.c M lspcon_i2c_spi.c M mec1308.c M mstarddc_spi.c M ni845x_spi.c M pickit2_spi.c M programmer.h M raiden_debug_spi.c M realtek_mst_i2c_spi.c M sb600spi.c M serprog.c M spi.c M stlinkv3_spi.c M usbblaster_spi.c M wbsio_spi.c 27 files changed, 33 insertions(+), 31 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/66/54066/3 -- 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: 3 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-MessageType: newpatchset

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: > I had the same thought and did right that :) amazing, thank you! :) -- 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:31:01 +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]: 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 2: (1 comment) Patchset: PS1: > Just went through my patches once again, there are three which have +2s, everyone had a look, and no […] I had the same thought and did right that :) -- 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 23:29:49 +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]: usbblaster_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/+/54044 ) Change subject: usbblaster_spi.c: Refactor singleton states into reentrant pattern ...................................................................... usbblaster_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". This patch also introduces shutdown function for usbblaster, because there was none previously and without shutdown function there is no way to free spi_master data at the end of driver lifecycle. BUG=b:185191942 TEST=builds Change-Id: Ia81f9f40c7eab430a8b304d0b197ce7c75bf5ace Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54044 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M usbblaster_spi.c 1 file changed, 30 insertions(+), 8 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/usbblaster_spi.c b/usbblaster_spi.c index 020c3bf..c92efe6 100644 --- a/usbblaster_spi.c +++ b/usbblaster_spi.c @@ -51,7 +51,9 @@ {0} }; -static struct ftdi_context ftdic; +struct usbblaster_spi_data { + struct ftdi_context ftdic; +}; // command bytes #define BIT_BYTE (1<<7) // byte mode (rather than bitbang) @@ -70,7 +72,7 @@ return ((b * 0x0802LU & 0x22110LU) | (b * 0x8020LU & 0x88440LU)) * 0x10101LU >> 16; } -static int send_write(unsigned int writecnt, const unsigned char *writearr) +static int send_write(unsigned int writecnt, const unsigned char *writearr, struct ftdi_context ftdic) { uint8_t buf[BUF_SIZE]; @@ -95,7 +97,7 @@ return 0; } -static int send_read(unsigned int readcnt, unsigned char *readarr) +static int send_read(unsigned int readcnt, unsigned char *readarr, struct ftdi_context ftdic) { int i; unsigned int n_read; @@ -135,23 +137,24 @@ static int usbblaster_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { + struct usbblaster_spi_data *usbblaster_data = flash->mst->spi.data; uint8_t cmd; int ret = 0; cmd = BIT_LED; // asserts /CS - if (ftdi_write_data(&ftdic, &cmd, 1) < 0) { + if (ftdi_write_data(&usbblaster_data->ftdic, &cmd, 1) < 0) { msg_perr("USB-Blaster enable chip select failed\n"); ret = -1; } if (!ret && writecnt) - ret = send_write(writecnt, writearr); + ret = send_write(writecnt, writearr, usbblaster_data->ftdic); if (!ret && readcnt) - ret = send_read(readcnt, readarr); + ret = send_read(readcnt, readarr, usbblaster_data->ftdic); cmd = BIT_CS; - if (ftdi_write_data(&ftdic, &cmd, 1) < 0) { + if (ftdi_write_data(&usbblaster_data->ftdic, &cmd, 1) < 0) { msg_perr("USB-Blaster disable chip select failed\n"); ret = -1; } @@ -159,8 +162,13 @@ return ret; } +static int usbblaster_shutdown(void *data) +{ + free(data); + return 0; +} -static const struct spi_master spi_master_usbblaster = { +static struct spi_master spi_master_usbblaster = { .max_data_read = 256, .max_data_write = 256, .command = usbblaster_spi_send_command, @@ -174,6 +182,8 @@ int usbblaster_spi_init(void) { uint8_t buf[BUF_SIZE + 1]; + struct ftdi_context ftdic; + struct usbblaster_spi_data *usbblaster_data; if (ftdi_init(&ftdic) < 0) return -1; @@ -210,6 +220,18 @@ return -1; } + usbblaster_data = calloc(1, sizeof(*usbblaster_data)); + if (!usbblaster_data) { + msg_perr("Unable to allocate space for SPI master data\n"); + return -1; + } + usbblaster_data->ftdic = ftdic; + spi_master_usbblaster.data = usbblaster_data; + + if (register_shutdown(usbblaster_shutdown, usbblaster_data)) { + free(usbblaster_data); + return -1; + } register_spi_master(&spi_master_usbblaster); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/54044 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia81f9f40c7eab430a8b304d0b197ce7c75bf5ace Gerrit-Change-Number: 54044 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

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

12 May '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54033 ) Change subject: mstarddc_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 1: (1 comment) File mstarddc_spi.c: https://review.coreboot.org/c/flashrom/+/54033/comment/74daabe2_2667378f PS1, Line 236: goto out; This leaves `mstarddc_fd` open. -- To view, visit https://review.coreboot.org/c/flashrom/+/54033 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia0e5f6879bfbfac591a40119ee3e0942a5cbc4b9 Gerrit-Change-Number: 54033 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: 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 23:21:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: digilent_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/+/54012 ) Change subject: digilent_spi.c: Refactor singleton states into reentrant pattern ...................................................................... digilent_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". BUG=b:185191942 TEST=builds Change-Id: I91b842913d4402a4a1bec896f19c2fe1f34772b1 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54012 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M digilent_spi.c 1 file changed, 58 insertions(+), 47 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/digilent_spi.c b/digilent_spi.c index 0f7a9da..91a28b0 100644 --- a/digilent_spi.c +++ b/digilent_spi.c @@ -45,8 +45,10 @@ #define DATA_WRITE_EP 0x03 #define DATA_READ_EP 0x84 -static struct libusb_device_handle *handle = NULL; -static bool reset_board; +struct digilent_spi_data { + struct libusb_device_handle *handle; + bool reset_board; +}; #define DIGILENT_VID 0x1443 #define DIGILENT_JTAG_PID 0x0007 @@ -97,7 +99,7 @@ CMD_SPI_TX_END = 0x87, }; -static int do_command(uint8_t *req, int req_len, uint8_t *res, int res_len) +static int do_command(uint8_t *req, int req_len, uint8_t *res, int res_len, struct libusb_device_handle *handle) { int tx_len = 0; int ret; @@ -133,41 +135,41 @@ return 0; } -static int gpio_open(void) +static int gpio_open(struct libusb_device_handle *handle) { uint8_t req[] = { 0x00, CMD_GPIO, CMD_GPIO_OPEN, 0x00 }; uint8_t res[2]; - return do_command(req, sizeof(req), res, sizeof(res)); + return do_command(req, sizeof(req), res, sizeof(res), handle); } -static int gpio_set_dir(uint8_t direction) +static int gpio_set_dir(uint8_t direction, struct libusb_device_handle *handle) { uint8_t req[] = { 0x00, CMD_GPIO, CMD_GPIO_SET_DIR, 0x00, direction, 0x00, 0x00, 0x00 }; uint8_t res[6]; - return do_command(req, sizeof(req), res, sizeof(res)); + return do_command(req, sizeof(req), res, sizeof(res), handle); } -static int gpio_set_value(uint8_t value) +static int gpio_set_value(uint8_t value, struct libusb_device_handle *handle) { uint8_t req[] = { 0x00, CMD_GPIO, CMD_GPIO_SET_VAL, 0x00, value, 0x00, 0x00, 0x00 }; uint8_t res[2]; - return do_command(req, sizeof(req), res, sizeof(res)); + return do_command(req, sizeof(req), res, sizeof(res), handle); } -static int spi_open(void) +static int spi_open(struct libusb_device_handle *handle) { uint8_t req[] = { 0x00, CMD_SPI, CMD_SPI_OPEN, 0x00 }; uint8_t res[2]; - return do_command(req, sizeof(req), res, sizeof(res)); + return do_command(req, sizeof(req), res, sizeof(res), handle); } -static int spi_set_speed(uint32_t speed) +static int spi_set_speed(uint32_t speed, struct libusb_device_handle *handle) { uint8_t req[] = { 0x00, CMD_SPI, CMD_SPI_SET_SPEED, 0x00, (speed) & 0xff, @@ -178,7 +180,7 @@ uint32_t real_speed; int ret; - ret = do_command(req, sizeof(req), res, sizeof(res)); + ret = do_command(req, sizeof(req), res, sizeof(res), handle); if (ret) return ret; @@ -189,23 +191,23 @@ return 0; } -static int spi_set_mode(uint8_t mode) +static int spi_set_mode(uint8_t mode, struct libusb_device_handle *handle) { uint8_t req[] = { 0x00, CMD_SPI, CMD_SPI_SET_MODE, 0x00, mode }; uint8_t res[2]; - return do_command(req, sizeof(req), res, sizeof(res)); + return do_command(req, sizeof(req), res, sizeof(res), handle); } -static int spi_set_cs(uint8_t cs) +static int spi_set_cs(uint8_t cs, struct libusb_device_handle *handle) { uint8_t req[] = { 0x00, CMD_SPI, CMD_SPI_SET_CS, 0x00, cs }; uint8_t res[2]; - return do_command(req, sizeof(req), res, sizeof(res)); + return do_command(req, sizeof(req), res, sizeof(res), handle); } -static int spi_start_io(uint8_t read_follows, uint32_t write_len) +static int spi_start_io(uint8_t read_follows, uint32_t write_len, struct libusb_device_handle *handle) { uint8_t req[] = { 0x00, CMD_SPI, CMD_SPI_START_IO, 0x00, 0x00, 0x00, /* meaning unknown */ @@ -216,17 +218,17 @@ (write_len >> 24) & 0xff }; uint8_t res[2]; - return do_command(req, sizeof(req), res, sizeof(res)); + return do_command(req, sizeof(req), res, sizeof(res), handle); } -static int spi_tx_end(uint8_t read_follows, uint32_t tx_len) +static int spi_tx_end(uint8_t read_follows, uint32_t tx_len, struct libusb_device_handle *handle) { uint8_t req[] = { 0x00, CMD_SPI, CMD_SPI_TX_END, 0x00 }; uint8_t res[read_follows ? 10 : 6]; int ret; uint32_t count; - ret = do_command(req, sizeof(req), res, sizeof(res)); + ret = do_command(req, sizeof(req), res, sizeof(res), handle); if (ret != 0) return ret; @@ -265,19 +267,20 @@ int tx_len = 0; uint8_t buf[len]; uint8_t read_follows = readcnt > 0 ? 1 : 0; + struct digilent_spi_data *digilent_data = flash->mst->spi.data; memcpy(buf, writearr, writecnt); memset(buf + writecnt, 0xff, readcnt); - ret = spi_set_cs(0); + ret = spi_set_cs(0, digilent_data->handle); if (ret != 0) return ret; - ret = spi_start_io(read_follows, writecnt); + ret = spi_start_io(read_follows, writecnt, digilent_data->handle); if (ret != 0) return ret; - ret = libusb_bulk_transfer(handle, DATA_WRITE_EP, buf, len, &tx_len, USB_TIMEOUT); + ret = libusb_bulk_transfer(digilent_data->handle, DATA_WRITE_EP, buf, len, &tx_len, USB_TIMEOUT); if (ret != 0) { msg_perr("%s: failed to write data: '%s'\n", __func__, libusb_error_name(ret)); return -1; @@ -288,7 +291,7 @@ } if (read_follows) { - ret = libusb_bulk_transfer(handle, DATA_READ_EP, buf, len, &tx_len, USB_TIMEOUT); + ret = libusb_bulk_transfer(digilent_data->handle, DATA_READ_EP, buf, len, &tx_len, USB_TIMEOUT); if (ret != 0) { msg_perr("%s: failed to read data: '%s'\n", __func__, libusb_error_name(ret)); return -1; @@ -299,11 +302,11 @@ } } - ret = spi_tx_end(read_follows, len); + ret = spi_tx_end(read_follows, len, digilent_data->handle); if (ret != 0) return ret; - ret = spi_set_cs(1); + ret = spi_set_cs(1, digilent_data->handle); if (ret != 0) return ret; @@ -312,7 +315,7 @@ return 0; } -static const struct spi_master spi_master_digilent_spi = { +static struct spi_master spi_master_digilent_spi = { .features = SPI_MASTER_4BA, .max_data_read = 252, .max_data_write = 252, @@ -326,16 +329,18 @@ static int digilent_spi_shutdown(void *data) { - if (reset_board) - gpio_set_dir(0); + struct digilent_spi_data *digilent_data = data; - libusb_close(handle); - handle = NULL; + if (digilent_data->reset_board) + gpio_set_dir(0, digilent_data->handle); + libusb_close(digilent_data->handle); + + free(data); return 0; } -static bool default_reset(void) +static bool default_reset(struct libusb_device_handle *handle) { char board[17]; @@ -373,11 +378,8 @@ char *p; uint32_t speed_hz = spispeeds[0].speed; int i; - - if (handle != NULL) { - msg_cerr("%s: handle already set! Please report a bug at flashrom(a)flashrom.org\n", __func__); - return -1; - } + struct libusb_device_handle *handle = NULL; + bool reset_board; int32_t ret = libusb_init(NULL); if (ret < 0) { @@ -425,32 +427,41 @@ if (p && strlen(p)) reset_board = (p[0] == '1'); else - reset_board = default_reset(); + reset_board = default_reset(handle); free(p); + if (reset_board) { - if (gpio_open() != 0) + if (gpio_open(handle) != 0) goto close_handle; - if (gpio_set_dir(1) != 0) + if (gpio_set_dir(1, handle) != 0) goto close_handle; - if (gpio_set_value(0) != 0) + if (gpio_set_value(0, handle) != 0) goto close_handle; } - if (spi_open() != 0) + if (spi_open(handle) != 0) goto close_handle; - if (spi_set_speed(speed_hz) != 0) + if (spi_set_speed(speed_hz, handle) != 0) goto close_handle; - if (spi_set_mode(0x00) != 0) + if (spi_set_mode(0x00, handle) != 0) goto close_handle; - register_shutdown(digilent_spi_shutdown, NULL); + struct digilent_spi_data *digilent_data = calloc(1, sizeof(*digilent_data)); + if (!digilent_data) { + msg_perr("Unable to allocate space for SPI master data\n"); + goto close_handle; + } + digilent_data->reset_board = reset_board; + digilent_data->handle = handle; + spi_master_digilent_spi.data = digilent_data; + + register_shutdown(digilent_spi_shutdown, digilent_data); register_spi_master(&spi_master_digilent_spi); return 0; close_handle: libusb_close(handle); - handle = NULL; return -1; } -- To view, visit https://review.coreboot.org/c/flashrom/+/54012 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I91b842913d4402a4a1bec896f19c2fe1f34772b1 Gerrit-Change-Number: 54012 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021