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

Change in flashrom[master]: programmer: Make use of new register_par_master() API
by Edward O'Callaghan (Code Review) 22 May '21

22 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54172 ) Change subject: programmer: Make use of new register_par_master() API ...................................................................... programmer: Make use of new register_par_master() API Pass pointers to dynamically allocated data to register_par_master(). This way we can avoid a mutable global. BUG=b:185191942 TEST=builds Change-Id: I76572e43d01f8a5e1aa73b1b9e8a187465ed8fef Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54172 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M dummyflasher.c 1 file changed, 2 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/dummyflasher.c b/dummyflasher.c index c1c393c..d549607 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -622,7 +622,7 @@ .write_aai = default_spi_write_aai, }; -static struct par_master par_master_dummy = { +static const struct par_master par_master_dummy = { .chip_readb = dummy_chip_readb, .chip_readw = dummy_chip_readw, .chip_readl = dummy_chip_readl, @@ -674,7 +674,6 @@ } data->emu_chip = EMULATE_NONE; data->delay_us = 0; - par_master_dummy.data = data; msg_pspew("%s\n", __func__); @@ -1028,7 +1027,7 @@ if (dummy_buses_supported & BUS_NONSPI) register_par_master(&par_master_dummy, dummy_buses_supported & BUS_NONSPI, - NULL); + data); if (dummy_buses_supported & BUS_SPI) register_spi_master(&spi_master_dummyflasher, data); -- To view, visit https://review.coreboot.org/c/flashrom/+/54172 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I76572e43d01f8a5e1aa73b1b9e8a187465ed8fef Gerrit-Change-Number: 54172 Gerrit-PatchSet: 5 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-MessageType: merged

1 0

Change in flashrom[master]: programmer: Make use of new register_opaque_master() API
by Edward O'Callaghan (Code Review) 22 May '21

22 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54171 ) Change subject: programmer: Make use of new register_opaque_master() API ...................................................................... programmer: Make use of new register_opaque_master() API Pass pointers to dynamically allocated data to register_opaque_master(). This way we can avoid a mutable global. BUG=b:185191942 TEST=builds Change-Id: I160810cd67f782131962e96fc6d20e2987fb0390 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54171 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M linux_mtd.c 1 file changed, 2 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/linux_mtd.c b/linux_mtd.c index aeaecce..a1af069 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -295,7 +295,7 @@ return 0; } -static struct opaque_master programmer_linux_mtd = { +static const struct opaque_master programmer_linux_mtd = { /* max_data_{read,write} don't have any effect for this programmer */ .max_data_read = MAX_DATA_UNSPECIFIED, .max_data_write = MAX_DATA_UNSPECIFIED, @@ -418,13 +418,12 @@ goto linux_mtd_init_exit; } - programmer_linux_mtd.data = data; if (register_shutdown(linux_mtd_shutdown, (void *)data)) { free(data); goto linux_mtd_init_exit; } - register_opaque_master(&programmer_linux_mtd, NULL); + register_opaque_master(&programmer_linux_mtd, data); ret = 0; linux_mtd_init_exit: -- To view, visit https://review.coreboot.org/c/flashrom/+/54171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I160810cd67f782131962e96fc6d20e2987fb0390 Gerrit-Change-Number: 54171 Gerrit-PatchSet: 5 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-MessageType: merged

1 0

Change in flashrom[master]: programmer: Smoothen register_opaque_master() API
by Edward O'Callaghan (Code Review) 22 May '21

22 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54170 ) Change subject: programmer: Smoothen register_opaque_master() API ...................................................................... programmer: Smoothen register_opaque_master() API It was impossible to register a const struct opaque_master that would point to dynamically allocated `data`. Fix that so that we won't have to create more mutable globals. BUG=b:185191942 TEST=builds Change-Id: Id3adb4cf04ae04dbe87ddb96f30871cb5f7c8ff0 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54170 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M ichspi.c M linux_mtd.c M nicintel_eeprom.c M opaque.c M programmer.h 5 files changed, 8 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/ichspi.c b/ichspi.c index 57cbdee..80fcff5 100644 --- a/ichspi.c +++ b/ichspi.c @@ -2046,7 +2046,7 @@ } hwseq_data.size_comp1 = tmpi; - register_opaque_master(&opaque_master_ich_hwseq); + register_opaque_master(&opaque_master_ich_hwseq, NULL); } else { register_spi_master(&spi_master_ich9, NULL); } diff --git a/linux_mtd.c b/linux_mtd.c index 0f7ac7e..aeaecce 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -424,7 +424,7 @@ goto linux_mtd_init_exit; } - register_opaque_master(&programmer_linux_mtd); + register_opaque_master(&programmer_linux_mtd, NULL); ret = 0; linux_mtd_init_exit: diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index f95107f..e170fcf 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -486,7 +486,7 @@ return 1; } - return register_opaque_master(&opaque_master_nicintel_ee_82580); + return register_opaque_master(&opaque_master_nicintel_ee_82580, NULL); } else { nicintel_eebar = rphysmap("Intel i210 NIC w/ emulated EEPROM", io_base_addr + 0x12000, MEMMAP_SIZE); @@ -496,7 +496,7 @@ if (register_shutdown(nicintel_ee_shutdown_i210, NULL)) return 1; - return register_opaque_master(&opaque_master_nicintel_ee_i210); + return register_opaque_master(&opaque_master_nicintel_ee_i210, NULL); } return 1; diff --git a/opaque.c b/opaque.c index e3103c8..f098ffc 100644 --- a/opaque.c +++ b/opaque.c @@ -46,7 +46,7 @@ return flash->mst->opaque.erase(flash, blockaddr, blocklen); } -int register_opaque_master(const struct opaque_master *mst) +int register_opaque_master(const struct opaque_master *mst, void *data) { struct registered_master rmst = {0}; @@ -58,5 +58,7 @@ } rmst.buses_supported = BUS_PROG; rmst.opaque = *mst; + if (data) + rmst.opaque.data = data; return register_master(&rmst); } diff --git a/programmer.h b/programmer.h index ac6b058..790fcc8 100644 --- a/programmer.h +++ b/programmer.h @@ -725,7 +725,7 @@ int (*erase) (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen); void *data; }; -int register_opaque_master(const struct opaque_master *mst); +int register_opaque_master(const struct opaque_master *mst, void *data); /* programmer.c */ void *fallback_map(const char *descr, uintptr_t phys_addr, size_t len); -- To view, visit https://review.coreboot.org/c/flashrom/+/54170 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3adb4cf04ae04dbe87ddb96f30871cb5f7c8ff0 Gerrit-Change-Number: 54170 Gerrit-PatchSet: 4 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-MessageType: merged

1 0

Change in flashrom[master]: programmer: Smoothen register_par_master API
by Edward O'Callaghan (Code Review) 22 May '21

22 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54169 ) Change subject: programmer: Smoothen register_par_master API ...................................................................... programmer: Smoothen register_par_master API It was impossible to register a const struct par_master that would point to dynamically allocated `data`. Fix that so that we won't have to create more mutable globals. BUG=b:185191942 TEST=builds Change-Id: I95bc92f6c54c5bcdac1c522ca87054aaffed0f40 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54169 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M atahpt.c M atapromise.c M atavia.c M drkaiser.c M dummyflasher.c M gfxnvidia.c M internal.c M it8212.c M nic3com.c M nicintel.c M nicnatsemi.c M nicrealtek.c M programmer.c M programmer.h M satamv.c M satasii.c M serprog.c 17 files changed, 21 insertions(+), 17 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/atahpt.c b/atahpt.c index dae0222..620e894 100644 --- a/atahpt.c +++ b/atahpt.c @@ -75,7 +75,7 @@ reg32 |= (1 << 24); rpci_write_long(dev, REG_FLASH_ACCESS, reg32); - register_par_master(&par_master_atahpt, BUS_PARALLEL); + register_par_master(&par_master_atahpt, BUS_PARALLEL, NULL); return 0; } diff --git a/atapromise.c b/atapromise.c index ab34a1e..f32bdb4 100644 --- a/atapromise.c +++ b/atapromise.c @@ -140,7 +140,7 @@ } max_rom_decode.parallel = rom_size; - register_par_master(&par_master_atapromise, BUS_PARALLEL); + register_par_master(&par_master_atapromise, BUS_PARALLEL, NULL); msg_pwarn("Do not use this device as a generic programmer. It will leave anything outside\n" "the first %zu kB of the flash chip in an undefined state. It works fine for the\n" diff --git a/atavia.c b/atavia.c index b407a30..e04950a 100644 --- a/atavia.c +++ b/atavia.c @@ -160,7 +160,7 @@ return 1; } - register_par_master(&lpc_master_atavia, BUS_LPC); + register_par_master(&lpc_master_atavia, BUS_LPC, NULL); return 0; } diff --git a/drkaiser.c b/drkaiser.c index ac49df3..fa562b4 100644 --- a/drkaiser.c +++ b/drkaiser.c @@ -77,7 +77,7 @@ return 1; max_rom_decode.parallel = 128 * 1024; - register_par_master(&par_master_drkaiser, BUS_PARALLEL); + register_par_master(&par_master_drkaiser, BUS_PARALLEL, NULL); return 0; } diff --git a/dummyflasher.c b/dummyflasher.c index e19e46c..c1c393c 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -1027,7 +1027,8 @@ } if (dummy_buses_supported & BUS_NONSPI) register_par_master(&par_master_dummy, - dummy_buses_supported & BUS_NONSPI); + dummy_buses_supported & BUS_NONSPI, + NULL); if (dummy_buses_supported & BUS_SPI) register_spi_master(&spi_master_dummyflasher, data); diff --git a/gfxnvidia.c b/gfxnvidia.c index d8ea4d6..fe6f982 100644 --- a/gfxnvidia.c +++ b/gfxnvidia.c @@ -103,7 +103,7 @@ /* Write/erase doesn't work. */ programmer_may_write = 0; - register_par_master(&par_master_gfxnvidia, BUS_PARALLEL); + register_par_master(&par_master_gfxnvidia, BUS_PARALLEL, NULL); return 0; } diff --git a/internal.c b/internal.c index bdbe32d..bb32435 100644 --- a/internal.c +++ b/internal.c @@ -335,7 +335,7 @@ #endif /* IS_X86 */ if (internal_buses_supported & BUS_NONSPI) - register_par_master(&par_master_internal, internal_buses_supported); + register_par_master(&par_master_internal, internal_buses_supported, NULL); /* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */ if (is_laptop && !laptop_ok) { diff --git a/it8212.c b/it8212.c index ac53a6f..2f2d43e 100644 --- a/it8212.c +++ b/it8212.c @@ -67,7 +67,7 @@ rpci_write_long(dev, PCI_ROM_ADDRESS, io_base_addr | 0x01); max_rom_decode.parallel = IT8212_MEMMAP_SIZE; - register_par_master(&par_master_it8212, BUS_PARALLEL); + register_par_master(&par_master_it8212, BUS_PARALLEL, NULL); return 0; } diff --git a/nic3com.c b/nic3com.c index b7b967a..6ff91e7 100644 --- a/nic3com.c +++ b/nic3com.c @@ -120,7 +120,7 @@ return 1; max_rom_decode.parallel = 128 * 1024; - register_par_master(&par_master_nic3com, BUS_PARALLEL); + register_par_master(&par_master_nic3com, BUS_PARALLEL, NULL); return 0; } diff --git a/nicintel.c b/nicintel.c index 4672890..afd2682 100644 --- a/nicintel.c +++ b/nicintel.c @@ -99,7 +99,7 @@ pci_rmmio_writew(0x0001, nicintel_control_bar + CSR_FCR); max_rom_decode.parallel = NICINTEL_MEMMAP_SIZE; - register_par_master(&par_master_nicintel, BUS_PARALLEL); + register_par_master(&par_master_nicintel, BUS_PARALLEL, NULL); return 0; } diff --git a/nicnatsemi.c b/nicnatsemi.c index 085768d..77b5440 100644 --- a/nicnatsemi.c +++ b/nicnatsemi.c @@ -71,7 +71,7 @@ * functions below wants to be 0x0000FFFF. */ max_rom_decode.parallel = 131072; - register_par_master(&par_master_nicnatsemi, BUS_PARALLEL); + register_par_master(&par_master_nicnatsemi, BUS_PARALLEL, NULL); return 0; } diff --git a/nicrealtek.c b/nicrealtek.c index 5454b63..2208179 100644 --- a/nicrealtek.c +++ b/nicrealtek.c @@ -86,7 +86,7 @@ if (register_shutdown(nicrealtek_shutdown, NULL)) return 1; - register_par_master(&par_master_nicrealtek, BUS_PARALLEL); + register_par_master(&par_master_nicrealtek, BUS_PARALLEL, NULL); return 0; } diff --git a/programmer.c b/programmer.c index 42ea2e3..c276a69 100644 --- a/programmer.c +++ b/programmer.c @@ -81,7 +81,8 @@ } int register_par_master(const struct par_master *mst, - const enum chipbustype buses) + const enum chipbustype buses, + void *data) { struct registered_master rmst = {0}; @@ -96,6 +97,8 @@ rmst.buses_supported = buses; rmst.par = *mst; + if (data) + rmst.par.data = data; return register_master(&rmst); } diff --git a/programmer.h b/programmer.h index 1a2e758..ac6b058 100644 --- a/programmer.h +++ b/programmer.h @@ -747,7 +747,7 @@ void (*chip_readn) (const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len); void *data; }; -int register_par_master(const struct par_master *mst, const enum chipbustype buses); +int register_par_master(const struct par_master *mst, const enum chipbustype buses, void *data); struct registered_master { enum chipbustype buses_supported; struct { diff --git a/satamv.c b/satamv.c index 31265ea..2640b78 100644 --- a/satamv.c +++ b/satamv.c @@ -148,7 +148,7 @@ /* 512 kByte with two 8-bit latches, and * 4 MByte with additional 3-bit latch. */ max_rom_decode.parallel = 4 * 1024 * 1024; - register_par_master(&par_master_satamv, BUS_PARALLEL); + register_par_master(&par_master_satamv, BUS_PARALLEL, NULL); return 0; } diff --git a/satasii.c b/satasii.c index 8a0938d..c9dfdad 100644 --- a/satasii.c +++ b/satasii.c @@ -100,7 +100,7 @@ if ((id != 0x0680) && (!(pci_mmio_readl(sii_bar) & (1 << 26)))) msg_pwarn("Warning: Flash seems unconnected.\n"); - register_par_master(&par_master_satasii, BUS_PARALLEL); + register_par_master(&par_master_satasii, BUS_PARALLEL, NULL); return 0; } diff --git a/serprog.c b/serprog.c index 056d8d1..e6028aa 100644 --- a/serprog.c +++ b/serprog.c @@ -885,7 +885,7 @@ if (serprog_buses_supported & BUS_SPI) register_spi_master(&spi_master_serprog, NULL); if (serprog_buses_supported & BUS_NONSPI) - register_par_master(&par_master_serprog, serprog_buses_supported & BUS_NONSPI); + register_par_master(&par_master_serprog, serprog_buses_supported & BUS_NONSPI, NULL); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/54169 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I95bc92f6c54c5bcdac1c522ca87054aaffed0f40 Gerrit-Change-Number: 54169 Gerrit-PatchSet: 4 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-MessageType: merged

1 0

Change in flashrom[master]: buspirate_spi.c: Separate shutdown from failed init cleanup
by Edward O'Callaghan (Code Review) 22 May '21

22 May '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52877 ) Change subject: buspirate_spi.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52877 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0 Gerrit-Change-Number: 52877 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: Miklós Márton <martonmiklosqdev(a)gmail.com> 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: Sun, 23 May 2021 03:01:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: stlinkv3_spi.c: Drop stlinkv3_ prefix for spi data struct member
by Edward O'Callaghan (Code Review) 21 May '21

21 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54043 ) Change subject: stlinkv3_spi.c: Drop stlinkv3_ prefix for spi data struct member ...................................................................... stlinkv3_spi.c: Drop stlinkv3_ prefix for spi data struct member The name of the struct already contains stlinkv3_ prefix, so prefix doesn't need to be repeated in members name. BUG=b:185191942 TEST=builds Change-Id: Ibddac9371ab8f08276d499642a9bdd6dbecea0ca Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54043 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Miklós Márton <martonmiklosqdev(a)gmail.com> --- M stlinkv3_spi.c 1 file changed, 5 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified Miklós Márton: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 5264d74..2453162 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -121,7 +121,7 @@ struct stlinkv3_spi_data { struct libusb_context *usb_ctx; - libusb_device_handle *stlinkv3_handle; + libusb_device_handle *handle; }; static int stlinkv3_command(uint8_t *command, size_t command_length, @@ -349,7 +349,7 @@ unsigned char *read_arr) { struct stlinkv3_spi_data *stlinkv3_data = flash->mst->spi.data; - libusb_device_handle *stlinkv3_handle = stlinkv3_data->stlinkv3_handle; + libusb_device_handle *stlinkv3_handle = stlinkv3_data->handle; uint8_t command[16]; int rc = 0; int actual_length = 0; @@ -464,9 +464,9 @@ stlinkv3_command(command, sizeof(command), answer, sizeof(answer), "STLINK_BRIDGE_CLOSE", - stlinkv3_data->stlinkv3_handle); + stlinkv3_data->handle); - libusb_close(stlinkv3_data->stlinkv3_handle); + libusb_close(stlinkv3_data->handle); libusb_exit(stlinkv3_data->usb_ctx); free(data); @@ -543,7 +543,7 @@ } stlinkv3_data->usb_ctx = usb_ctx; - stlinkv3_data->stlinkv3_handle = stlinkv3_handle; + stlinkv3_data->handle = stlinkv3_handle; if (register_shutdown(stlinkv3_spi_shutdown, stlinkv3_data)) goto init_err_cleanup_exit; -- To view, visit https://review.coreboot.org/c/flashrom/+/54043 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibddac9371ab8f08276d499642a9bdd6dbecea0ca Gerrit-Change-Number: 54043 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: 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-MessageType: merged

1 0

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

21 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54042 ) Change subject: stlinkv3_spi.c: Refactor singleton states into reentrant pattern ...................................................................... stlinkv3_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: Id044661b864b506028720ea809bc524f0640469f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54042 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Miklós Márton <martonmiklosqdev(a)gmail.com> --- M stlinkv3_spi.c 1 file changed, 66 insertions(+), 29 deletions(-) Approvals: build bot (Jenkins): Verified Miklós Márton: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 100a94b..5264d74 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -119,11 +119,14 @@ {0} }; -static struct libusb_context *usb_ctx; -static libusb_device_handle *stlinkv3_handle; +struct stlinkv3_spi_data { + struct libusb_context *usb_ctx; + libusb_device_handle *stlinkv3_handle; +}; static int stlinkv3_command(uint8_t *command, size_t command_length, - uint8_t *answer, size_t answer_length, const char *command_name) + uint8_t *answer, size_t answer_length, const char *command_name, + libusb_device_handle *stlinkv3_handle) { int actual_length = 0; int rc = libusb_bulk_transfer(stlinkv3_handle, STLINK_EP_OUT, @@ -151,7 +154,7 @@ /** * @param[out] bridge_input_clk Current input frequency in kHz of the given com. */ -static int stlinkv3_get_clk(uint32_t *bridge_input_clk) +static int stlinkv3_get_clk(uint32_t *bridge_input_clk, libusb_device_handle *stlinkv3_handle) { uint8_t command[16]; uint8_t answer[12]; @@ -165,7 +168,10 @@ command[1] = STLINK_BRIDGE_GET_CLOCK; command[2] = STLINK_SPI_COM; - if (stlinkv3_command(command, sizeof(command), answer, sizeof(answer), "STLINK_BRIDGE_GET_CLOCK") != 0) + if (stlinkv3_command(command, sizeof(command), + answer, sizeof(answer), + "STLINK_BRIDGE_GET_CLOCK", + stlinkv3_handle) != 0) return -1; *bridge_input_clk = (uint32_t)answer[4] @@ -178,13 +184,14 @@ static int stlinkv3_spi_calc_prescaler(uint16_t reqested_freq_in_kHz, enum spi_prescaler *prescaler, - uint16_t *calculated_freq_in_kHz) + uint16_t *calculated_freq_in_kHz, + libusb_device_handle *stlinkv3_handle) { uint32_t bridge_clk_in_kHz; uint32_t calculated_prescaler = 1; uint16_t prescaler_value; - if (stlinkv3_get_clk(&bridge_clk_in_kHz)) + if (stlinkv3_get_clk(&bridge_clk_in_kHz, stlinkv3_handle)) return -1; calculated_prescaler = bridge_clk_in_kHz/reqested_freq_in_kHz; @@ -224,7 +231,7 @@ return 0; } -static int stlinkv3_check_version(enum fw_version_check_result *result) +static int stlinkv3_check_version(enum fw_version_check_result *result, libusb_device_handle *stlinkv3_handle) { uint8_t answer[12]; uint8_t command[16]; @@ -234,7 +241,10 @@ command[0] = ST_GETVERSION_EXT; command[1] = 0x80; - if (stlinkv3_command(command, sizeof(command), answer, sizeof(answer), "ST_GETVERSION_EXT") != 0) + if (stlinkv3_command(command, sizeof(command), + answer, sizeof(answer), + "ST_GETVERSION_EXT", + stlinkv3_handle) != 0) return -1; msg_pinfo("Connected to STLink V3 with bridge FW version: %d\n", answer[4]); @@ -244,7 +254,7 @@ return 0; } -static int stlinkv3_spi_open(uint16_t reqested_freq_in_kHz) +static int stlinkv3_spi_open(uint16_t reqested_freq_in_kHz, libusb_device_handle *stlinkv3_handle) { uint8_t command[16]; uint8_t answer[2]; @@ -252,7 +262,7 @@ enum spi_prescaler prescaler; enum fw_version_check_result fw_check_result; - if (stlinkv3_check_version(&fw_check_result)) { + if (stlinkv3_check_version(&fw_check_result, stlinkv3_handle)) { msg_perr("Failed to query FW version\n"); return -1; } @@ -267,7 +277,8 @@ if (stlinkv3_spi_calc_prescaler(reqested_freq_in_kHz, &prescaler, - &SCK_freq_in_kHz)) { + &SCK_freq_in_kHz, + stlinkv3_handle)) { msg_perr("Failed to calculate SPI clock prescaler\n"); return -1; } @@ -286,10 +297,13 @@ command[5] = SPI_NSS_SOFT; command[6] = (uint8_t)prescaler; - return stlinkv3_command(command, sizeof(command), answer, sizeof(answer), "STLINK_BRIDGE_INIT_SPI"); + return stlinkv3_command(command, sizeof(command), + answer, sizeof(answer), + "STLINK_BRIDGE_INIT_SPI", + stlinkv3_handle); } -static int stlinkv3_get_last_readwrite_status(uint32_t *status) +static int stlinkv3_get_last_readwrite_status(uint32_t *status, libusb_device_handle *stlinkv3_handle) { uint8_t command[16]; uint16_t answer[4]; @@ -301,14 +315,15 @@ if (stlinkv3_command(command, sizeof(command), (uint8_t *)answer, sizeof(answer), - "STLINK_BRIDGE_GET_RWCMD_STATUS") != 0) + "STLINK_BRIDGE_GET_RWCMD_STATUS", + stlinkv3_handle) != 0) return -1; *status = (uint32_t)answer[2] | (uint32_t)answer[3]<<16; return 0; } -static int stlinkv3_spi_set_SPI_NSS(enum spi_nss_level nss_level) +static int stlinkv3_spi_set_SPI_NSS(enum spi_nss_level nss_level, libusb_device_handle *stlinkv3_handle) { uint8_t command[16]; uint8_t answer[2]; @@ -319,7 +334,10 @@ command[1] = STLINK_BRIDGE_CS_SPI; command[2] = (uint8_t) (nss_level); - if (stlinkv3_command(command, sizeof(command), answer, sizeof(answer), "STLINK_BRIDGE_CS_SPI") != 0) + if (stlinkv3_command(command, sizeof(command), + answer, sizeof(answer), + "STLINK_BRIDGE_CS_SPI", + stlinkv3_handle) != 0) return -1; return 0; } @@ -330,13 +348,15 @@ const unsigned char *write_arr, unsigned char *read_arr) { + struct stlinkv3_spi_data *stlinkv3_data = flash->mst->spi.data; + libusb_device_handle *stlinkv3_handle = stlinkv3_data->stlinkv3_handle; uint8_t command[16]; int rc = 0; int actual_length = 0; uint32_t rw_status = 0; unsigned int i; - if (stlinkv3_spi_set_SPI_NSS(SPI_NSS_LOW)) { + if (stlinkv3_spi_set_SPI_NSS(SPI_NSS_LOW, stlinkv3_handle)) { msg_perr("Failed to set the NSS pin to low\n"); return -1; } @@ -374,7 +394,7 @@ } } - if (stlinkv3_get_last_readwrite_status(&rw_status)) + if (stlinkv3_get_last_readwrite_status(&rw_status, stlinkv3_handle)) return -1; if (rw_status != 0) { @@ -409,7 +429,7 @@ } } - if (stlinkv3_get_last_readwrite_status(&rw_status)) + if (stlinkv3_get_last_readwrite_status(&rw_status, stlinkv3_handle)) goto transmit_err; if (rw_status != 0) { @@ -417,20 +437,21 @@ goto transmit_err; } - if (stlinkv3_spi_set_SPI_NSS(SPI_NSS_HIGH)) { + if (stlinkv3_spi_set_SPI_NSS(SPI_NSS_HIGH, stlinkv3_handle)) { msg_perr("Failed to set the NSS pin to high\n"); return -1; } return 0; transmit_err: - if (stlinkv3_spi_set_SPI_NSS(SPI_NSS_HIGH)) + if (stlinkv3_spi_set_SPI_NSS(SPI_NSS_HIGH, stlinkv3_handle)) msg_perr("Failed to set the NSS pin to high\n"); return -1; } static int stlinkv3_spi_shutdown(void *data) { + struct stlinkv3_spi_data *stlinkv3_data = data; uint8_t command[16]; uint8_t answer[2]; @@ -440,11 +461,15 @@ command[1] = STLINK_BRIDGE_CLOSE; command[2] = STLINK_SPI_COM; - stlinkv3_command(command, sizeof(command), answer, sizeof(answer), "STLINK_BRIDGE_CLOSE"); + stlinkv3_command(command, sizeof(command), + answer, sizeof(answer), + "STLINK_BRIDGE_CLOSE", + stlinkv3_data->stlinkv3_handle); - libusb_close(stlinkv3_handle); - libusb_exit(usb_ctx); + libusb_close(stlinkv3_data->stlinkv3_handle); + libusb_exit(stlinkv3_data->usb_ctx); + free(data); return 0; } @@ -465,6 +490,9 @@ char *serialno = NULL; char *endptr = NULL; int ret = 1; + struct libusb_context *usb_ctx; + libusb_device_handle *stlinkv3_handle; + struct stlinkv3_spi_data *stlinkv3_data; libusb_init(&usb_ctx); if (!usb_ctx) { @@ -505,19 +533,28 @@ free(speed_str); } - if (stlinkv3_spi_open(sck_freq_kHz)) + if (stlinkv3_spi_open(sck_freq_kHz, stlinkv3_handle)) goto init_err_exit; - if (register_shutdown(stlinkv3_spi_shutdown, NULL)) + stlinkv3_data = calloc(1, sizeof(*stlinkv3_data)); + if (!stlinkv3_data) { + msg_perr("Unable to allocate space for SPI master data\n"); + goto init_err_exit; + } + + stlinkv3_data->usb_ctx = usb_ctx; + stlinkv3_data->stlinkv3_handle = stlinkv3_handle; + + if (register_shutdown(stlinkv3_spi_shutdown, stlinkv3_data)) goto init_err_cleanup_exit; - if (register_spi_master(&spi_programmer_stlinkv3, NULL)) + if (register_spi_master(&spi_programmer_stlinkv3, stlinkv3_data)) return 1; /* shutdown function does cleanup */ return 0; init_err_cleanup_exit: - stlinkv3_spi_shutdown(NULL); + stlinkv3_spi_shutdown(stlinkv3_data); return 1; init_err_exit: -- 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: 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: 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-MessageType: merged

1 0

Change in flashrom[master]: dummyflasher.c: Extract params processing into a separate function
by Edward O'Callaghan (Code Review) 21 May '21

21 May '21

Attention is currently required from: Nico Huber, Namyoon Woo, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54748 ) Change subject: dummyflasher.c: Extract params processing into a separate function ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Can you rebase on top of the other series https://review.coreboot.org/c/flashrom/+/54722 there will be some conflict otherwise. -- To view, visit https://review.coreboot.org/c/flashrom/+/54748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04f55f77bb4703f1d88b2191c45a22be3c97bf87 Gerrit-Change-Number: 54748 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: 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Namyoon Woo <namyoon(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 21 May 2021 09:52:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

20 May '21

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk 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 2: (1 comment) File mstarddc_spi.c: https://review.coreboot.org/c/flashrom/+/54033/comment/345cb3b3_fb9ae5e2 PS1, Line 236: goto out; > I have fixed exactly that... […] (marking as resolved to indicate this is ready for review) -- 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: 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 21 May 2021 05:38:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher.c: Extract params processing into a separate function
by Anastasia Klimchuk (Code Review) 20 May '21

20 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54748 ) Change subject: dummyflasher.c: Extract params processing into a separate function ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: I noticed something in dummy, not in scope of this patch but maybe for future work, wrote a message below. Thank you! File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/54748/comment/f03b8f3b_292cf166 PS1, Line 26: /* Remove the #define below if you don't want SPI flash chip emulation. */ : #define EMULATE_SPI_CHIP 1 If this line is removed, dummy does not compile. Because below, if EMULATE_SPI_CHIP is not defined , EMULATE_CHIP won't be defined either - which means emu_data is not defined, so dummy does not compile. Another thing, if only EMULATE_CHIP is defined (and EMULATE_SPI_CHIP undefined) then dummy does not link, linking fails with an error "undefined reference to `probe_variable_size'". I wanted to fix that, but I don't know what is expected behaviour behind EMULATE_SPI_CHIP and EMULATE_CHIP, and which combinations of these two flags are valid? And maybe, we can split into two dummies, dummy_spi and dummy_par, if they are very different? (I am not changing any behaviour in this patch, but eventually I want to understand what is the correct one). -- To view, visit https://review.coreboot.org/c/flashrom/+/54748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04f55f77bb4703f1d88b2191c45a22be3c97bf87 Gerrit-Change-Number: 54748 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 21 May 2021 05:34:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021