January 2023 - flashrom-gerrit

[M] Change in flashrom[master]: tests/: Add non-aligned write within a region unit-test
by Edward O'Callaghan (Code Review) 10 Jan '23

10 Jan '23

Attention is currently required from: Sam McNally, Edward O'Callaghan, Anastasia Klimchuk. Hello Sam McNally, build bot (Jenkins), Evan Benn, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71659 to look at the new patch set (#3). Change subject: tests/: Add non-aligned write within a region unit-test ...................................................................... tests/: Add non-aligned write within a region unit-test A written region that is sized below that of the erasure granularity can result in a incorrectly read region that does not include prior content within the region before the write op. This was dealt with in ChromeOS downstream by expanding out the read to match the erase granularity however does not seem to impact upstream. Add a unit-test to avoid regression as this is important behaviour to cover. Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/chip.c M tests/tests.c M tests/tests.h 3 files changed, 110 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/71659/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom.c: Rewrite prepare_chip_access() 4BA enablement
by Jonathon Hall (Code Review) 09 Jan '23

09 Jan '23

Attention is currently required from: Sam McNally, Edward O'Callaghan, Angel Pons. Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71742 ) Change subject: flashrom.c: Rewrite prepare_chip_access() 4BA enablement ...................................................................... Patch Set 1: (2 comments) File flashrom.c: https://review.coreboot.org/c/flashrom/+/71742/comment/e4a27f2c_8a507ddc PS1, Line 1916: if (!spi_master_no_4ba_modes(flash)) { I don't really understand this check - the comment on SPI_MASTER_NO_4BA_MODES says "Compatibility modes (..., 4BA mode switch) don't work" - would this flag mean the master has 4BA but the mode switch doesn't work? I would assume it is not just missing 4BA mode since that would be the same as !spi_master_4ba(), there would probably be a reason for two distinct flags right? It looks like 'dediprog' could set both flags, I don't have any device to test that though. Do you know what this flag means? It used to possibly enter/exit 4ba in this state depending on spi_chip_4ba()/spi_master_4ba(). https://review.coreboot.org/c/flashrom/+/71742/comment/a3bbedc4_6afea3fb PS1, Line 1938: int ret = spi_master_4ba(flash) ? spi_enter_4ba(flash): spi_exit_4ba(flash); It's not possible to reach spi_exit_4ba() here due to returning if spi_master_4ba() was false above. I think the intent was that if the chip supports 4ba but doesn't require it (<= 16M), and master doesn't support 4ba, it would exit 4ba here. I'm not sure if that makes sense though, why would such a chip be in 4ba in the first place? Maybe if some buggy program changed it? So I'm not sure if the answer is to delete the unreachable code or restore the disable path, what do you think? -- To view, visit https://review.coreboot.org/c/flashrom/+/71742 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I10a426331cb2a3485e3dd786fa771e20870cbd84 Gerrit-Change-Number: 71742 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 09 Jan 2023 14:31:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tree/: Make driver init func names consistent
by Edward O'Callaghan (Code Review) 09 Jan '23

09 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71748 ) Change subject: tree/: Make driver init func names consistent ...................................................................... tree/: Make driver init func names consistent Almost the whole tree has the form 'name_bus_init()'. Make the few exceptions consistent with the rest of the tree for easy grep. Change-Id: I275af7cc2e6911670c646b15775fcd721ed9fe99 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M chipset_enable.c M ichspi.c M include/programmer.h M internal.c M it87spi.c M sb600spi.c 6 files changed, 25 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/71748/1 diff --git a/chipset_enable.c b/chipset_enable.c index b9144d1..cd82366 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -817,7 +817,7 @@ void *spibar = rcrb + spibar_offset; /* This adds BUS_SPI */ - int ret_spi = ich_init_spi(cfg, spibar, ich_generation); + int ret_spi = ichspi_init(cfg, spibar, ich_generation); if (ret_spi == ERROR_FLASHROM_FATAL) return ret_spi; @@ -961,7 +961,7 @@ msg_pdbg("SPIBAR = 0x%0*" PRIxPTR " (phys = 0x%08x)\n", PRIxPTR_WIDTH, (uintptr_t)spibar, phys_spibar); /* This adds BUS_SPI */ - const int ret_spi = ich_init_spi(cfg, spibar, pch_generation); + const int ret_spi = ichspi_init(cfg, spibar, pch_generation); if (ret_spi != ERROR_FLASHROM_FATAL) { if (ret_bc || ret_spi) ret = ERROR_FLASHROM_NONFATAL; @@ -1078,7 +1078,7 @@ */ enable_flash_ich_bios_cntl_memmapped(ich_generation, spibar + 0xFC); - int ret_spi = ich_init_spi(cfg, spibar, ich_generation); + int ret_spi = ichspi_init(cfg, spibar, ich_generation); if (ret_spi == ERROR_FLASHROM_FATAL) return ret_spi; @@ -1408,7 +1408,7 @@ internal_buses_supported &= BUS_LPC | BUS_FWH; - ret = sb600_probe_spi(cfg, dev); + ret = sb600spi_init(cfg, dev); /* Read ROM strap override register. */ OUTB(0x8f, 0xcd6); diff --git a/ichspi.c b/ichspi.c index 1522d2b..2ea8a74 100644 --- a/ichspi.c +++ b/ichspi.c @@ -2356,7 +2356,7 @@ return 0; } -int ich_init_spi(const struct programmer_cfg *cfg, void *spibar, enum ich_chipset ich_gen) +int ichspi_init(const struct programmer_cfg *cfg, void *spibar, enum ich_chipset ich_gen) { ich_generation = ich_gen; ich_spibar = spibar; diff --git a/include/programmer.h b/include/programmer.h index ad3c502..1cbaf6c 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -363,7 +363,7 @@ /* ichspi.c */ #if CONFIG_INTERNAL == 1 -int ich_init_spi(const struct programmer_cfg *cfg, void *spibar, enum ich_chipset ich_generation); +int ichspi_init(const struct programmer_cfg *cfg, void *spibar, enum ich_chipset ich_generation); int via_init_spi(uint32_t mmio_base); /* amd_imc.c */ @@ -373,7 +373,7 @@ void enter_conf_mode_ite(uint16_t port); void exit_conf_mode_ite(uint16_t port); void probe_superio_ite(void); -int init_superio_ite(const struct programmer_cfg *cfg); +int ite_superio_init(const struct programmer_cfg *cfg); /* trivial wrapper to avoid cluttering internal_init() with #if */ static inline int try_mtd(const struct programmer_cfg *cfg) @@ -391,7 +391,7 @@ /* sb600spi.c */ -int sb600_probe_spi(const struct programmer_cfg *cfg, struct pci_dev *dev); +int sb600spi_init(const struct programmer_cfg *cfg, struct pci_dev *dev); /* wbsio_spi.c */ int wbsio_check_for_spi(void); diff --git a/internal.c b/internal.c index 1161652..f477903 100644 --- a/internal.c +++ b/internal.c @@ -298,7 +298,7 @@ #if defined(__i386__) || defined(__x86_64__) /* Probe unconditionally for ITE Super I/O chips. This enables LPC->SPI translation on IT87* and * parallel writes on IT8705F. Also, this handles the manual chip select for Gigabyte's DualBIOS. */ - init_superio_ite(cfg); + ite_superio_init(cfg); if (board_flash_enable(board_vendor, board_model, cb_vendor, cb_model)) { msg_perr("Aborting to be safe.\n"); diff --git a/it87spi.c b/it87spi.c index 34dbea0..601e8dc 100644 --- a/it87spi.c +++ b/it87spi.c @@ -443,7 +443,7 @@ return register_spi_master(&spi_master_it87xx, data); } -int init_superio_ite(const struct programmer_cfg *cfg) +int ite_superio_init(const struct programmer_cfg *cfg) { int i; int ret = 0; diff --git a/sb600spi.c b/sb600spi.c index 5b9ac45..6e11e53 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -634,7 +634,7 @@ .probe_opcode = default_spi_probe_opcode, }; -int sb600_probe_spi(const struct programmer_cfg *cfg, struct pci_dev *dev) +int sb600spi_init(const struct programmer_cfg *cfg, struct pci_dev *dev) { struct pci_dev *smbus_dev; uint32_t tmp; -- To view, visit https://review.coreboot.org/c/flashrom/+/71748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I275af7cc2e6911670c646b15775fcd721ed9fe99 Gerrit-Change-Number: 71748 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: wip
by Edward O'Callaghan (Code Review) 09 Jan '23

09 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71747 ) Change subject: wip ...................................................................... wip Change-Id: Ie02803d93a02ab57a070a284d8ff3dd2a7d90e47 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M atahpt.c M atapromise.c M atavia.c M buspirate_spi.c M ch341a_spi.c M dediprog.c M developerbox_spi.c M digilent_spi.c M dirtyjtag_spi.c M drkaiser.c M dummyflasher.c M ft2232_spi.c M gfxnvidia.c M internal.c M it8212.c M jlink_spi.c M linux_mtd.c M linux_spi.c M mediatek_i2c_spi.c M mstarddc_spi.c M ni845x_spi.c M nic3com.c M nicintel.c M nicintel_eeprom.c M nicintel_spi.c M nicnatsemi.c M nicrealtek.c M ogp_spi.c M parade_lspcon.c M pcidev.c M pickit2_spi.c M pony_spi.c M raiden_debug_spi.c M rayer_spi.c M realtek_mst_i2c_spi.c M satamv.c M satasii.c M serprog.c M stlinkv3_spi.c M tests/tests.c M usb_device.c M usbblaster_spi.c 42 files changed, 53 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/71747/1 diff --git a/atahpt.c b/atahpt.c index 71fd4d5..8abbc4a 100644 --- a/atahpt.c +++ b/atahpt.c @@ -78,7 +78,7 @@ .shutdown = atahpt_shutdown, }; -static int atahpt_init(const struct programmer_cfg *cfg) +static int atahpt_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uint32_t io_base_addr; diff --git a/atapromise.c b/atapromise.c index 9beebf1..6073770 100644 --- a/atapromise.c +++ b/atapromise.c @@ -117,7 +117,7 @@ .shutdown = atapromise_shutdown, }; -static int atapromise_init(const struct programmer_cfg *cfg) +static int atapromise_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uint32_t io_base_addr; diff --git a/atavia.c b/atavia.c index f35771a..d9dbda9 100644 --- a/atavia.c +++ b/atavia.c @@ -138,7 +138,7 @@ .chip_writeb = atavia_chip_writeb, }; -static int atavia_init(const struct programmer_cfg *cfg) +static int atavia_init(struct programmer_cfg *cfg) { char *arg = extract_programmer_param_str(cfg, "offset"); if (arg) { diff --git a/buspirate_spi.c b/buspirate_spi.c index a40fb70..86d3950 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -314,7 +314,7 @@ */ #define BP_DIVISOR(baud) ((4000000/(baud)) - 1) -static int buspirate_spi_init(const struct programmer_cfg *cfg) +static int buspirate_spi_init(struct programmer_cfg *cfg) { char *tmp; char *dev; diff --git a/ch341a_spi.c b/ch341a_spi.c index 75eaf86..142fd4e 100644 --- a/ch341a_spi.c +++ b/ch341a_spi.c @@ -421,7 +421,7 @@ .delay = ch341a_spi_delay, }; -static int ch341a_spi_init(const struct programmer_cfg *cfg) +static int ch341a_spi_init(struct programmer_cfg *cfg) { if (handle != NULL) { msg_cerr("%s: handle already set! Please report a bug at flashrom(a)flashrom.org\n", __func__); diff --git a/dediprog.c b/dediprog.c index 723f4a8..7fd6c9d 100644 --- a/dediprog.c +++ b/dediprog.c @@ -1076,7 +1076,7 @@ return 0; } -static int dediprog_init(const struct programmer_cfg *cfg) +static int dediprog_init(struct programmer_cfg *cfg) { char *param_str; int spispeed_idx = 1; diff --git a/developerbox_spi.c b/developerbox_spi.c index 64b7e8a..c29fcc5 100644 --- a/developerbox_spi.c +++ b/developerbox_spi.c @@ -142,7 +142,7 @@ return 0; } -static int developerbox_spi_init(const struct programmer_cfg *cfg) +static int developerbox_spi_init(struct programmer_cfg *cfg) { struct libusb_context *usb_ctx; libusb_device_handle *cp210x_handle; diff --git a/digilent_spi.c b/digilent_spi.c index 55cb6ec..68a0e21 100644 --- a/digilent_spi.c +++ b/digilent_spi.c @@ -373,7 +373,7 @@ { NULL, 0 }, }; -static int digilent_spi_init(const struct programmer_cfg *cfg) +static int digilent_spi_init(struct programmer_cfg *cfg) { char *param_str; uint32_t speed_hz = spispeeds[0].speed; diff --git a/dirtyjtag_spi.c b/dirtyjtag_spi.c index 42169ed..9035e91 100644 --- a/dirtyjtag_spi.c +++ b/dirtyjtag_spi.c @@ -201,7 +201,7 @@ .probe_opcode = default_spi_probe_opcode, }; -static int dirtyjtag_spi_init(const struct programmer_cfg *cfg) +static int dirtyjtag_spi_init(struct programmer_cfg *cfg) { struct libusb_device_handle *handle = NULL; struct dirtyjtag_spi_data *djtag_data = NULL; diff --git a/drkaiser.c b/drkaiser.c index ebf5119..17343dc 100644 --- a/drkaiser.c +++ b/drkaiser.c @@ -75,7 +75,7 @@ .shutdown = drkaiser_shutdown, }; -static int drkaiser_init(const struct programmer_cfg *cfg) +static int drkaiser_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uint32_t addr; diff --git a/dummyflasher.c b/dummyflasher.c index 1960e92..4928a77 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -1343,7 +1343,7 @@ return 0; } -static int dummy_init(const struct programmer_cfg *cfg) +static int dummy_init(struct programmer_cfg *cfg) { struct stat image_stat; diff --git a/ft2232_spi.c b/ft2232_spi.c index dfec1a0..228a15b 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -304,7 +304,7 @@ }; /* Returns 0 upon success, a negative number upon errors. */ -static int ft2232_spi_init(const struct programmer_cfg *cfg) +static int ft2232_spi_init(struct programmer_cfg *cfg) { int ret = 0; unsigned char buf[512]; diff --git a/gfxnvidia.c b/gfxnvidia.c index b420975..d7fd842 100644 --- a/gfxnvidia.c +++ b/gfxnvidia.c @@ -100,7 +100,7 @@ .shutdown = gfxnvidia_shutdown, }; -static int gfxnvidia_init(const struct programmer_cfg *cfg) +static int gfxnvidia_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uint32_t reg32; diff --git a/internal.c b/internal.c index f21c4fc..1161652 100644 --- a/internal.c +++ b/internal.c @@ -191,7 +191,7 @@ return 0; } -static int internal_init(const struct programmer_cfg *cfg) +static int internal_init(struct programmer_cfg *cfg) { int ret = 0; bool force_laptop; diff --git a/it8212.c b/it8212.c index 3c1161d..de45e61 100644 --- a/it8212.c +++ b/it8212.c @@ -68,7 +68,7 @@ .shutdown = it8212_shutdown, }; -static int it8212_init(const struct programmer_cfg *cfg) +static int it8212_init(struct programmer_cfg *cfg) { uint8_t *bar; diff --git a/jlink_spi.c b/jlink_spi.c index ad2ca78..48fd6ca 100644 --- a/jlink_spi.c +++ b/jlink_spi.c @@ -192,7 +192,7 @@ .probe_opcode = default_spi_probe_opcode, }; -static int jlink_spi_init(const struct programmer_cfg *cfg) +static int jlink_spi_init(struct programmer_cfg *cfg) { char *arg; unsigned long speed = 0; diff --git a/linux_mtd.c b/linux_mtd.c index 495db9a..5f747ac 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -494,7 +494,7 @@ return ret; } -static int linux_mtd_init(const struct programmer_cfg *cfg) +static int linux_mtd_init(struct programmer_cfg *cfg) { char *param_str; int dev_num = 0; diff --git a/linux_spi.c b/linux_spi.c index d65b3e8..b8ffa5d 100644 --- a/linux_spi.c +++ b/linux_spi.c @@ -164,7 +164,7 @@ return result; } -static int linux_spi_init(const struct programmer_cfg *cfg) +static int linux_spi_init(struct programmer_cfg *cfg) { char *param_str, *endp; uint32_t speed_hz = 2 * 1000 * 1000; diff --git a/mediatek_i2c_spi.c b/mediatek_i2c_spi.c index c15df05..eb233e2 100644 --- a/mediatek_i2c_spi.c +++ b/mediatek_i2c_spi.c @@ -483,7 +483,7 @@ return ret; } -static int mediatek_init(const struct programmer_cfg *cfg) +static int mediatek_init(struct programmer_cfg *cfg) { int ret; bool allow_brick; diff --git a/mstarddc_spi.c b/mstarddc_spi.c index bd65281..c62579c 100644 --- a/mstarddc_spi.c +++ b/mstarddc_spi.c @@ -150,7 +150,7 @@ }; /* Returns 0 upon success, a negative number upon errors. */ -static int mstarddc_spi_init(const struct programmer_cfg *cfg) +static int mstarddc_spi_init(struct programmer_cfg *cfg) { int ret = 0; int mstarddc_fd = -1; diff --git a/ni845x_spi.c b/ni845x_spi.c index fd5057d..2f83625 100644 --- a/ni845x_spi.c +++ b/ni845x_spi.c @@ -539,7 +539,7 @@ .probe_opcode = default_spi_probe_opcode, }; -static int ni845x_spi_init(const struct programmer_cfg *cfg) +static int ni845x_spi_init(struct programmer_cfg *cfg) { char *speed_str = NULL; char *CS_str = NULL; diff --git a/nic3com.c b/nic3com.c index a578d48..1ed10cf 100644 --- a/nic3com.c +++ b/nic3com.c @@ -95,7 +95,7 @@ .shutdown = nic3com_shutdown, }; -static int nic3com_init(const struct programmer_cfg *cfg) +static int nic3com_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uint32_t io_base_addr = 0; diff --git a/nicintel.c b/nicintel.c index feb07b6..8671781 100644 --- a/nicintel.c +++ b/nicintel.c @@ -71,7 +71,7 @@ .shutdown = nicintel_shutdown, }; -static int nicintel_init(const struct programmer_cfg *cfg) +static int nicintel_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uintptr_t addr; diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index 80ccd88..99d0b1c 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -479,7 +479,7 @@ .shutdown = nicintel_ee_shutdown_i210, }; -static int nicintel_ee_init(const struct programmer_cfg *cfg) +static int nicintel_ee_init(struct programmer_cfg *cfg) { const struct opaque_master *mst; uint32_t eec = 0; diff --git a/nicintel_spi.c b/nicintel_spi.c index 2821d23..80f7ecf 100644 --- a/nicintel_spi.c +++ b/nicintel_spi.c @@ -285,7 +285,7 @@ return 0; } -static int nicintel_spi_init(const struct programmer_cfg *cfg) +static int nicintel_spi_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; diff --git a/nicnatsemi.c b/nicnatsemi.c index 65377dc..ca04a5b 100644 --- a/nicnatsemi.c +++ b/nicnatsemi.c @@ -82,7 +82,7 @@ .shutdown = nicnatsemi_shutdown, }; -static int nicnatsemi_init(const struct programmer_cfg *cfg) +static int nicnatsemi_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uint32_t io_base_addr; diff --git a/nicrealtek.c b/nicrealtek.c index 5937e35..c81f839 100644 --- a/nicrealtek.c +++ b/nicrealtek.c @@ -91,7 +91,7 @@ .shutdown = nicrealtek_shutdown, }; -static int nicrealtek_init(const struct programmer_cfg *cfg) +static int nicrealtek_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uint32_t io_base_addr = 0; diff --git a/ogp_spi.c b/ogp_spi.c index d85c823..b17e8f2 100644 --- a/ogp_spi.c +++ b/ogp_spi.c @@ -107,7 +107,7 @@ return 0; } -static int ogp_spi_init(const struct programmer_cfg *cfg) +static int ogp_spi_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; char *type; diff --git a/parade_lspcon.c b/parade_lspcon.c index a50346e..f8b5856 100644 --- a/parade_lspcon.c +++ b/parade_lspcon.c @@ -460,7 +460,7 @@ return ret; } -static int parade_lspcon_init(const struct programmer_cfg *cfg) +static int parade_lspcon_init(struct programmer_cfg *cfg) { bool allow_brick; diff --git a/pcidev.c b/pcidev.c index 0b35944..2c1b1c8 100644 --- a/pcidev.c +++ b/pcidev.c @@ -257,7 +257,7 @@ * also matches the specified bus:device.function. * For convenience, this function also registers its own undo handlers. */ -struct pci_dev *pcidev_init(const struct programmer_cfg *cfg, const struct dev_entry *devs, int bar) +struct pci_dev *pcidev_init(struct programmer_cfg *cfg, const struct dev_entry *devs, int bar) { struct pci_dev *dev; struct pci_dev *found_dev = NULL; diff --git a/pickit2_spi.c b/pickit2_spi.c index 86c5403..786d51d 100644 --- a/pickit2_spi.c +++ b/pickit2_spi.c @@ -387,7 +387,7 @@ .probe_opcode = default_spi_probe_opcode, }; -static int pickit2_spi_init(const struct programmer_cfg *cfg) +static int pickit2_spi_init(struct programmer_cfg *cfg) { uint8_t buf[CMD_LENGTH] = { CMD_EXEC_SCRIPT, diff --git a/pony_spi.c b/pony_spi.c index 0647f91..6143e58 100644 --- a/pony_spi.c +++ b/pony_spi.c @@ -160,7 +160,7 @@ return ret; } -static int pony_spi_init(const struct programmer_cfg *cfg) +static int pony_spi_init(struct programmer_cfg *cfg) { int i, data_out; enum pony_type type; diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index d1a09d7..4f62605 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -1486,7 +1486,7 @@ dev = usb_device_free(dev); } -static int raiden_debug_spi_init(const struct programmer_cfg *cfg) +static int raiden_debug_spi_init(struct programmer_cfg *cfg) { struct usb_match match; char *serial = extract_programmer_param_str(cfg, "serial"); diff --git a/rayer_spi.c b/rayer_spi.c index 27caf2b..28bcff3 100644 --- a/rayer_spi.c +++ b/rayer_spi.c @@ -296,7 +296,7 @@ return *prog ? 0 : -1; } -static int rayer_spi_init(const struct programmer_cfg *cfg) +static int rayer_spi_init(struct programmer_cfg *cfg) { const struct rayer_programmer *prog; struct rayer_pinout *pinout = NULL; diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 2a5d5ed..99d35fd 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -492,7 +492,7 @@ return ret; } -static int realtek_mst_i2c_spi_init(const struct programmer_cfg *cfg) +static int realtek_mst_i2c_spi_init(struct programmer_cfg *cfg) { int ret = 0; bool reset, enter_isp, allow_brick; diff --git a/satamv.c b/satamv.c index fea4c2b..713daad 100644 --- a/satamv.c +++ b/satamv.c @@ -108,7 +108,7 @@ * 0xc08 PCI BAR2 (Flash/NVRAM) Control * 0x1046c Flash Parameters */ -static int satamv_init(const struct programmer_cfg *cfg) +static int satamv_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uintptr_t addr; diff --git a/satasii.c b/satasii.c index e881f96..ae3a9e6 100644 --- a/satasii.c +++ b/satasii.c @@ -99,7 +99,7 @@ .shutdown = satasii_shutdown, }; -static int satasii_init(const struct programmer_cfg *cfg) +static int satasii_init(struct programmer_cfg *cfg) { struct pci_dev *dev = NULL; uint32_t addr; diff --git a/serprog.c b/serprog.c index 5ddbe3d..1f372f1 100644 --- a/serprog.c +++ b/serprog.c @@ -601,7 +601,7 @@ static enum chipbustype serprog_buses_supported = BUS_NONE; -static int serprog_init(const struct programmer_cfg *cfg) +static int serprog_init(struct programmer_cfg *cfg) { uint16_t iface; unsigned char pgmname[17]; diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 16a66b4..7396d9d 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -473,7 +473,7 @@ .probe_opcode = default_spi_probe_opcode, }; -static int stlinkv3_spi_init(const struct programmer_cfg *cfg) +static int stlinkv3_spi_init(struct programmer_cfg *cfg) { uint16_t sck_freq_kHz = 1000; // selecting 1 MHz SCK is a good bet char *param_str; diff --git a/tests/tests.c b/tests/tests.c index a1dcaca..70d072f 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -53,7 +53,7 @@ .device_id = NON_ZERO, }; -struct pci_dev *__wrap_pcidev_init(const struct programmer_cfg *cfg, void *devs, int bar) +struct pci_dev *__wrap_pcidev_init(struct programmer_cfg *cfg, void *devs, int bar) { LOG_ME; return &mock_pci_dev; diff --git a/usb_device.c b/usb_device.c index b3c966c..d52d6c6 100644 --- a/usb_device.c +++ b/usb_device.c @@ -28,7 +28,7 @@ * Possibly extract a programmer parameter and use it to initialize the given * match value structure. */ -static void usb_match_value_init(const struct programmer_cfg *cfg, +static void usb_match_value_init(struct programmer_cfg *cfg, struct usb_match_value *match, char const *parameter) { @@ -49,7 +49,7 @@ #define USB_MATCH_VALUE_INIT(PPARAM, NAME) \ usb_match_value_init(PPARAM, &match->NAME, #NAME) -void usb_match_init(const struct programmer_cfg *cfg, struct usb_match *match) +void usb_match_init(struct programmer_cfg *cfg, struct usb_match *match) { USB_MATCH_VALUE_INIT(cfg, vid); USB_MATCH_VALUE_INIT(cfg, pid); diff --git a/usbblaster_spi.c b/usbblaster_spi.c index 75d5be7..b1a9d01 100644 --- a/usbblaster_spi.c +++ b/usbblaster_spi.c @@ -176,7 +176,7 @@ }; /* Returns 0 upon success, a negative number upon errors. */ -static int usbblaster_spi_init(const struct programmer_cfg *cfg) +static int usbblaster_spi_init(struct programmer_cfg *cfg) { uint8_t buf[BUF_SIZE + 1] = { 0 }; struct ftdi_context ftdic; -- To view, visit https://review.coreboot.org/c/flashrom/+/71747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie02803d93a02ab57a070a284d8ff3dd2a7d90e47 Gerrit-Change-Number: 71747 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: parallel: Drop explicit fallback_chip_X boilerplate
by Stefan Reinauer (Code Review) 09 Jan '23

09 Jan '23

Attention is currently required from: Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71745 ) Change subject: parallel: Drop explicit fallback_chip_X boilerplate ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If25c0048a07057aa72be6ffa8d8ad7f0a568dcf7 Gerrit-Change-Number: 71745 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 09 Jan 2023 01:35:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer.h: Guard against sending spi commands on non-spi mst
by Sam McNally (Code Review) 09 Jan '23

09 Jan '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71269 ) Change subject: programmer.h: Guard against sending spi commands on non-spi mst ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Gerrit-Change-Number: 71269 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 09 Jan 2023 01:26:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: parallel: Drop explicit fallback_chip_X boilerplate
by Edward O'Callaghan (Code Review) 09 Jan '23

09 Jan '23

Attention is currently required from: Stefan Reinauer. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71745 ) Change subject: parallel: Drop explicit fallback_chip_X boilerplate ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Stefan, can you test with your satasii.c case. -- To view, visit https://review.coreboot.org/c/flashrom/+/71745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If25c0048a07057aa72be6ffa8d8ad7f0a568dcf7 Gerrit-Change-Number: 71745 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 09 Jan 2023 00:18:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: parallel: Drop explicit fallback_chip_X boilerplate
by Edward O'Callaghan (Code Review) 09 Jan '23

09 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71745 ) Change subject: parallel: Drop explicit fallback_chip_X boilerplate ...................................................................... parallel: Drop explicit fallback_chip_X boilerplate A NULL func pointer is necessary and sufficient for the condition `NULL func pointer => fallback_chip_X' as to not need this explicit specification. Therefore drop the explicit need to specify these fallback callback function pointer in the par_master struct. This is a reasonable default for every driver in the tree. Furthermore, move the 'fallback_chip_X()' func from the generic programmer.c register logic into its relevant home of parallel.c and make static local to clean up link-time symbol space. This simplifies the code and driver development. Change-Id: If25c0048a07057aa72be6ffa8d8ad7f0a568dcf7 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M atahpt.c M atapromise.c M atavia.c M drkaiser.c M gfxnvidia.c M include/programmer.h M internal.c M it8212.c M nic3com.c M nicintel.c M nicnatsemi.c M nicrealtek.c M parallel.c M programmer.c M satamv.c M satasii.c M serprog.c 17 files changed, 95 insertions(+), 144 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/71745/1 diff --git a/atahpt.c b/atahpt.c index 89b44c2..71fd4d5 100644 --- a/atahpt.c +++ b/atahpt.c @@ -74,13 +74,7 @@ static const struct par_master par_master_atahpt = { .chip_readb = atahpt_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = atahpt_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = atahpt_shutdown, }; diff --git a/atapromise.c b/atapromise.c index e1b8638..9beebf1 100644 --- a/atapromise.c +++ b/atapromise.c @@ -113,13 +113,7 @@ static const struct par_master par_master_atapromise = { .chip_readb = atapromise_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = atapromise_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = atapromise_shutdown, }; diff --git a/atavia.c b/atavia.c index 8af52d6..f35771a 100644 --- a/atavia.c +++ b/atavia.c @@ -135,13 +135,7 @@ static const struct par_master lpc_master_atavia = { .map_flash_region = atavia_map, .chip_readb = atavia_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = atavia_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, }; static int atavia_init(const struct programmer_cfg *cfg) diff --git a/drkaiser.c b/drkaiser.c index 96f2c99..ebf5119 100644 --- a/drkaiser.c +++ b/drkaiser.c @@ -71,13 +71,7 @@ static const struct par_master par_master_drkaiser = { .chip_readb = drkaiser_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = drkaiser_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = drkaiser_shutdown, }; diff --git a/gfxnvidia.c b/gfxnvidia.c index 1d484ea..b420975 100644 --- a/gfxnvidia.c +++ b/gfxnvidia.c @@ -96,13 +96,7 @@ static const struct par_master par_master_gfxnvidia = { .chip_readb = gfxnvidia_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = gfxnvidia_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = gfxnvidia_shutdown, }; diff --git a/include/programmer.h b/include/programmer.h index db7a05a..ad3c502 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -442,12 +442,6 @@ int register_par_master(const struct par_master *mst, const enum chipbustype buses, void *data); /* programmer.c */ -void fallback_chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr); -void fallback_chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr); -void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len); -uint16_t fallback_chip_readw(const struct flashctx *flash, const chipaddr addr); -uint32_t fallback_chip_readl(const struct flashctx *flash, const chipaddr addr); -void fallback_chip_readn(const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len); struct registered_master { enum chipbustype buses_supported; struct { diff --git a/internal.c b/internal.c index 9b80f37..f21c4fc 100644 --- a/internal.c +++ b/internal.c @@ -116,7 +116,6 @@ .chip_writeb = internal_chip_writeb, .chip_writew = internal_chip_writew, .chip_writel = internal_chip_writel, - .chip_writen = fallback_chip_writen, }; static int get_params(const struct programmer_cfg *cfg, diff --git a/it8212.c b/it8212.c index 8fe2b59..3c1161d 100644 --- a/it8212.c +++ b/it8212.c @@ -64,13 +64,7 @@ static const struct par_master par_master_it8212 = { .chip_readb = it8212_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = it8212_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = it8212_shutdown, }; diff --git a/nic3com.c b/nic3com.c index efba979..a578d48 100644 --- a/nic3com.c +++ b/nic3com.c @@ -91,13 +91,7 @@ static const struct par_master par_master_nic3com = { .chip_readb = nic3com_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = nic3com_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = nic3com_shutdown, }; diff --git a/nicintel.c b/nicintel.c index 47edbb7..feb07b6 100644 --- a/nicintel.c +++ b/nicintel.c @@ -67,13 +67,7 @@ static const struct par_master par_master_nicintel = { .chip_readb = nicintel_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = nicintel_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = nicintel_shutdown, }; diff --git a/nicnatsemi.c b/nicnatsemi.c index efa879a..65377dc 100644 --- a/nicnatsemi.c +++ b/nicnatsemi.c @@ -78,13 +78,7 @@ static const struct par_master par_master_nicnatsemi = { .chip_readb = nicnatsemi_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = nicnatsemi_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = nicnatsemi_shutdown, }; diff --git a/nicrealtek.c b/nicrealtek.c index 103ea9e..5937e35 100644 --- a/nicrealtek.c +++ b/nicrealtek.c @@ -87,13 +87,7 @@ static const struct par_master par_master_nicrealtek = { .chip_readb = nicrealtek_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = nicrealtek_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = nicrealtek_shutdown, }; diff --git a/parallel.c b/parallel.c index cea9e7a..b193723 100644 --- a/parallel.c +++ b/parallel.c @@ -27,19 +27,49 @@ flash->mst->par.chip_writeb(flash, val, addr); } +/* Little-endian fallback for drivers not supporting 16 bit accesses */ +static void fallback_chip_writew(const struct flashctx *flash, uint16_t val, + chipaddr addr) +{ + chip_writeb(flash, val & 0xff, addr); + chip_writeb(flash, (val >> 8) & 0xff, addr + 1); +} + void chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr) { - flash->mst->par.chip_writew(flash, val, addr); + if (flash->mst->par.chip_writew) + flash->mst->par.chip_writew(flash, val, addr); + fallback_chip_writew(flash, val, addr); +} + +/* Little-endian fallback for drivers not supporting 32 bit accesses */ +static void fallback_chip_writel(const struct flashctx *flash, uint32_t val, + chipaddr addr) +{ + chip_writew(flash, val & 0xffff, addr); + chip_writew(flash, (val >> 16) & 0xffff, addr + 2); } void chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr) { - flash->mst->par.chip_writel(flash, val, addr); + if (flash->mst->par.chip_writel) + flash->mst->par.chip_writel(flash, val, addr); + fallback_chip_writel(flash, val, addr); +} + +static void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len) +{ + size_t i; + for (i = 0; i < len; i++) + chip_writeb(flash, buf[i], addr + i); + return; } void chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len) { - flash->mst->par.chip_writen(flash, buf, addr, len); + if (flash->mst->par.chip_writen) + flash->mst->par.chip_writen(flash, buf, addr, len); + fallback_chip_writen(flash, buf, addr, len); } uint8_t chip_readb(const struct flashctx *flash, const chipaddr addr) @@ -47,20 +77,53 @@ return flash->mst->par.chip_readb(flash, addr); } +/* Little-endian fallback for drivers not supporting 16 bit accesses */ +static uint16_t fallback_chip_readw(const struct flashctx *flash, const chipaddr addr) +{ + uint16_t val; + val = chip_readb(flash, addr); + val |= chip_readb(flash, addr + 1) << 8; + return val; +} + uint16_t chip_readw(const struct flashctx *flash, const chipaddr addr) { - return flash->mst->par.chip_readw(flash, addr); + if (flash->mst->par.chip_readw) + return flash->mst->par.chip_readw(flash, addr); + return fallback_chip_readw(flash, addr); +} + +/* Little-endian fallback for drivers not supporting 32 bit accesses */ +static uint32_t fallback_chip_readl(const struct flashctx *flash, const chipaddr addr) +{ + uint32_t val; + val = chip_readw(flash, addr); + val |= chip_readw(flash, addr + 2) << 16; + return val; } uint32_t chip_readl(const struct flashctx *flash, const chipaddr addr) { - return flash->mst->par.chip_readl(flash, addr); + if (flash->mst->par.chip_readl) + return flash->mst->par.chip_readl(flash, addr); + return fallback_chip_readl(flash, addr); +} + +static void fallback_chip_readn(const struct flashctx *flash, uint8_t *buf, + chipaddr addr, size_t len) +{ + size_t i; + for (i = 0; i < len; i++) + buf[i] = chip_readb(flash, addr + i); + return; } void chip_readn(const struct flashctx *flash, uint8_t *buf, chipaddr addr, size_t len) { - flash->mst->par.chip_readn(flash, buf, addr, len); + if (flash->mst->par.chip_readn) + flash->mst->par.chip_readn(flash, buf, addr, len); + fallback_chip_readn(flash, buf, addr, len); } int register_par_master(const struct par_master *mst, @@ -87,9 +150,7 @@ return ERROR_FLASHROM_BUG; } - if (!mst->chip_writeb || !mst->chip_writew || !mst->chip_writel || - !mst->chip_writen || !mst->chip_readb || !mst->chip_readw || - !mst->chip_readl || !mst->chip_readn) { + if (!mst->chip_writeb || !mst->chip_readb) { msg_perr("%s called with incomplete master definition. " "Please report a bug at flashrom(a)flashrom.org\n", __func__); diff --git a/programmer.c b/programmer.c index d4f15a1..15fd1b8 100644 --- a/programmer.c +++ b/programmer.c @@ -17,57 +17,6 @@ #include "flash.h" #include "programmer.h" -/* Little-endian fallback for drivers not supporting 16 bit accesses */ -void fallback_chip_writew(const struct flashctx *flash, uint16_t val, - chipaddr addr) -{ - chip_writeb(flash, val & 0xff, addr); - chip_writeb(flash, (val >> 8) & 0xff, addr + 1); -} - -/* Little-endian fallback for drivers not supporting 16 bit accesses */ -uint16_t fallback_chip_readw(const struct flashctx *flash, const chipaddr addr) -{ - uint16_t val; - val = chip_readb(flash, addr); - val |= chip_readb(flash, addr + 1) << 8; - return val; -} - -/* Little-endian fallback for drivers not supporting 32 bit accesses */ -void fallback_chip_writel(const struct flashctx *flash, uint32_t val, - chipaddr addr) -{ - chip_writew(flash, val & 0xffff, addr); - chip_writew(flash, (val >> 16) & 0xffff, addr + 2); -} - -/* Little-endian fallback for drivers not supporting 32 bit accesses */ -uint32_t fallback_chip_readl(const struct flashctx *flash, const chipaddr addr) -{ - uint32_t val; - val = chip_readw(flash, addr); - val |= chip_readw(flash, addr + 2) << 16; - return val; -} - -void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len) -{ - size_t i; - for (i = 0; i < len; i++) - chip_writeb(flash, buf[i], addr + i); - return; -} - -void fallback_chip_readn(const struct flashctx *flash, uint8_t *buf, - chipaddr addr, size_t len) -{ - size_t i; - for (i = 0; i < len; i++) - buf[i] = chip_readb(flash, addr + i); - return; -} - /* The limit of 4 is totally arbitrary. */ #define MASTERS_MAX 4 struct registered_master registered_masters[MASTERS_MAX]; diff --git a/satamv.c b/satamv.c index dcd53c7..fea4c2b 100644 --- a/satamv.c +++ b/satamv.c @@ -88,13 +88,7 @@ static const struct par_master par_master_satamv = { .chip_readb = satamv_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = satamv_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = satamv_shutdown, }; diff --git a/satasii.c b/satasii.c index 011562e..e881f96 100644 --- a/satasii.c +++ b/satasii.c @@ -95,13 +95,7 @@ static const struct par_master par_master_satasii = { .chip_readb = satasii_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, - .chip_readn = fallback_chip_readn, .chip_writeb = satasii_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .shutdown = satasii_shutdown, }; diff --git a/serprog.c b/serprog.c index 4e131e5..5ddbe3d 100644 --- a/serprog.c +++ b/serprog.c @@ -594,13 +594,8 @@ static const struct par_master par_master_serprog = { .map_flash_region = serprog_map, .chip_readb = serprog_chip_readb, - .chip_readw = fallback_chip_readw, - .chip_readl = fallback_chip_readl, .chip_readn = serprog_chip_readn, .chip_writeb = serprog_chip_writeb, - .chip_writew = fallback_chip_writew, - .chip_writel = fallback_chip_writel, - .chip_writen = fallback_chip_writen, .delay = serprog_delay, }; -- To view, visit https://review.coreboot.org/c/flashrom/+/71745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If25c0048a07057aa72be6ffa8d8ad7f0a568dcf7 Gerrit-Change-Number: 71745 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: tree/: Change chip restore data type from uint8_t to void ptr
by Nikolai Artemiev (Code Review) 08 Jan '23

08 Jan '23

Attention is currently required from: Nikolai Artemiev. Hello build bot (Jenkins), Edward O'Callaghan, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70349 to look at the new patch set (#7). Change subject: tree/: Change chip restore data type from uint8_t to void ptr ...................................................................... tree/: Change chip restore data type from uint8_t to void ptr Chip restore callbacks currently are used by - spi25_statusreg.c unlock functions to restore status register 1. - s25f.c to restore config register 3. Both of these cases only need to save a single uint8_t value to restore the original chip state, however storing a void pointer will allow more flexible chip restore behaviour. In particular, it will allow flashrom_wp_cfg objects to be saved and restored, enabling writeprotect-based unlocking. BUG=b:237485865,b:247421511 BRANCH=none TEST=Tested on grunt DUT (prog: sb600spi, flash: W25Q128.W): `flashrom --wp-range 0x0,0x1000000 \ flashrom --wp-status # Result: range=0x0,0x1000000 \ flashrom -w random.bin # Result: success \ flashrom -v random.bin # Result: success \ flashrom --wp-status # Result: range=0x0,0x1000000` Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c M include/flash.h M s25f.c M spi25_statusreg.c 4 files changed, 57 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/70349/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/70349 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Gerrit-Change-Number: 70349 Gerrit-PatchSet: 7 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: programmer.h: Guard against sending spi commands on non-spi mst
by Edward O'Callaghan (Code Review) 08 Jan '23

08 Jan '23

Attention is currently required from: Sam McNally, Felix Singer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71269 ) Change subject: programmer.h: Guard against sending spi commands on non-spi mst ...................................................................... Patch Set 2: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/71269/comment/08fd139d_ef244c11 PS1, Line 1937: spi_master_no_4ba_modes > I feel like the code needs a bit of work.. […] Sam, https://review.coreboot.org/c/flashrom/+/71742 -- To view, visit https://review.coreboot.org/c/flashrom/+/71269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Gerrit-Change-Number: 71269 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 08 Jan 2023 01:42:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sam McNally <sammc(a)google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023