- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: spi_master: Add shutdown function in spi_master struct
by Anastasia Klimchuk (Code Review) 06 Jul '21

06 Jul '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55932 ) Change subject: spi_master: Add shutdown function in spi_master struct ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: I promoted this from RFC into a chain of patches for spi masters. File programmer.h: https://review.coreboot.org/c/flashrom/+/55932/comment/e99e38c8_67e28f0c PS1, Line 359: shutdown > >>>for this to be ideally called `deinit` and logically one level up in `struct programmer_entry` […] I uploaded the chain for review, my understanding we all agree to go ahead? -- To view, visit https://review.coreboot.org/c/flashrom/+/55932 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2dc80dceca2f8204bcd0dad1f51753d7e79f1af5 Gerrit-Change-Number: 55932 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 06 Jul 2021 07:02:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi_master: Use new API to register shutdown function
by Anastasia Klimchuk (Code Review) 06 Jul '21

06 Jul '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56103 ) Change subject: spi_master: Use new API to register shutdown function ...................................................................... spi_master: Use new API to register shutdown function TEST ME ON DEVICES This allows spi masters to register shutdown function in spi_master struct, which means there is no need to call register_shutdown in init function, since this call is now a part of register_spi_master. Testing: when I comment out free(data) in linux_spi_shutdown, test fails with error ../linux_spi.c:235: note: block 0x55a4db276510 allocated here ERROR: linux_spi_init_and_shutdown_test_success leaked 1 block(s) Means, shutdown function is invoked. BUG=b:185191942 TEST=builds and ninja test Change-Id: Ib60300f9ddb295a255d5ef3f8da0e07064207140 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M dediprog.c M digilent_spi.c M ene_lpc.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 mec1308.c M mstarddc_spi.c M ni845x_spi.c M pickit2_spi.c M realtek_mst_i2c_spi.c M sb600spi.c M stlinkv3_spi.c M usbblaster_spi.c M wbsio_spi.c 21 files changed, 40 insertions(+), 112 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/56103/1 diff --git a/bitbang_spi.c b/bitbang_spi.c index 83c3501..7c9a04d 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -147,6 +147,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = bitbang_spi_shutdown, }; int register_spi_bitbang_master(const struct bitbang_spi_master *master, void *spi_data) @@ -168,7 +169,6 @@ data->spi_data = spi_data; register_spi_master(&mst, data); - register_shutdown(bitbang_spi_shutdown, data); /* Only mess with the bus if we're sure nobody else uses it. */ bitbang_spi_request_bus(master, spi_data); diff --git a/buspirate_spi.c b/buspirate_spi.c index 3c0a6ff..6d7fba7 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -181,6 +181,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = buspirate_spi_shutdown, }; static const struct buspirate_speeds spispeeds[] = { @@ -698,13 +699,7 @@ goto init_err_cleanup_exit; } - if (register_shutdown(buspirate_spi_shutdown, bp_data) != 0) { - ret = 1; - goto init_err_cleanup_exit; - } - register_spi_master(&spi_master_buspirate, bp_data); - - return 0; + return register_spi_master(&spi_master_buspirate, bp_data); init_err_cleanup_exit: buspirate_spi_shutdown(bp_data); diff --git a/ch341a_spi.c b/ch341a_spi.c index 22c9781..4a76286 100644 --- a/ch341a_spi.c +++ b/ch341a_spi.c @@ -417,6 +417,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = ch341a_spi_shutdown, }; static int ch341a_spi_init(void) @@ -505,10 +506,7 @@ if ((config_stream(CH341A_STM_I2C_100K) < 0) || (enable_pins(true) < 0)) goto dealloc_transfers; - register_shutdown(ch341a_spi_shutdown, NULL); - register_spi_master(&spi_master_ch341a_spi, NULL); - - return 0; + return register_spi_master(&spi_master_ch341a_spi, NULL); dealloc_transfers: for (i = 0; i < USB_IN_TRANSFERS; i++) { diff --git a/dediprog.c b/dediprog.c index d98af49..323dcd0 100644 --- a/dediprog.c +++ b/dediprog.c @@ -1019,6 +1019,7 @@ .read = dediprog_spi_read, .write_256 = dediprog_spi_write_256, .write_aai = dediprog_spi_write_aai, + .shutdown = dediprog_shutdown, }; /* @@ -1271,9 +1272,6 @@ if (protocol() >= PROTOCOL_V2) spi_master_dediprog.features |= SPI_MASTER_4BA; - if (register_shutdown(dediprog_shutdown, NULL)) - goto init_err_cleanup_exit; - if (register_spi_master(&spi_master_dediprog, NULL) || dediprog_set_leds(LED_NONE)) return 1; /* shutdown function does cleanup */ diff --git a/digilent_spi.c b/digilent_spi.c index 0ec402e..e2cfcce 100644 --- a/digilent_spi.c +++ b/digilent_spi.c @@ -337,6 +337,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = digilent_spi_shutdown, }; static bool default_reset(struct libusb_device_handle *handle) @@ -454,10 +455,7 @@ digilent_data->reset_board = reset_board; digilent_data->handle = handle; - register_shutdown(digilent_spi_shutdown, digilent_data); - register_spi_master(&spi_master_digilent_spi, digilent_data); - - return 0; + return register_spi_master(&spi_master_digilent_spi, digilent_data); close_handle: libusb_close(handle); diff --git a/ene_lpc.c b/ene_lpc.c index 23ba12b..374d1fd 100644 --- a/ene_lpc.c +++ b/ene_lpc.c @@ -512,6 +512,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = ene_leave_flash_mode, }; static int check_params(void) @@ -570,16 +571,7 @@ internal_buses_supported |= BUS_LPC; - if (register_shutdown(ene_leave_flash_mode, ctx_data)) - goto init_err_cleanup_exit; - register_spi_master(&spi_master_ene, ctx_data); - msg_pdbg("%s: successfully initialized ene\n", __func__); - - return 0; - -init_err_cleanup_exit: - ene_leave_flash_mode(ctx_data); - return 1; + return register_spi_master(&spi_master_ene, ctx_data); init_err_exit: free(ctx_data); diff --git a/ft2232_spi.c b/ft2232_spi.c index d853b09..cf2589f 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -291,6 +291,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = ft2232_shutdown, }; /* Returns 0 upon success, a negative number upon errors. */ @@ -575,13 +576,7 @@ spi_data->pindir = pindir; spi_data->ftdic_context = ftdic; - if (register_shutdown(ft2232_shutdown, spi_data)) { - free(spi_data); - goto ftdi_err; - } - register_spi_master(&spi_master_ft2232, spi_data); - - return 0; + return register_spi_master(&spi_master_ft2232, spi_data); ftdi_err: if ((f = ftdi_usb_close(&ftdic)) < 0) { diff --git a/it85spi.c b/it85spi.c index 9812fc7..dd0cb6c 100644 --- a/it85spi.c +++ b/it85spi.c @@ -289,6 +289,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = it85xx_shutdown, }; int it85xx_spi_init(struct superio s) @@ -353,20 +354,13 @@ data->ce_high = ((unsigned char *)base) + 0xE00; /* 0xFFFFFE00 */ data->ce_low = ((unsigned char *)base) + 0xD00; /* 0xFFFFFD00 */ - if (register_shutdown(it85xx_shutdown, data)) { - free(data); - return 1; - } - /* FIXME: Really leave FWH enabled? We can't use this region * anymore since accessing it would mess up IT85 communication. * If we decide to disable FWH for this region, we should print * a debug message about it. */ /* Set this as SPI controller. */ - register_spi_master(&spi_master_it85xx, data); - - return 0; + return register_spi_master(&spi_master_it85xx, data); } #endif diff --git a/it87spi.c b/it87spi.c index e552328..3e422d7 100644 --- a/it87spi.c +++ b/it87spi.c @@ -304,6 +304,7 @@ .read = it8716f_spi_chip_read, .write_256 = it8716f_spi_chip_write_256, .write_aai = spi_chip_write_1, + .shutdown = it8716f_shutdown, }; static uint16_t it87spi_probe(uint16_t port) @@ -419,13 +420,10 @@ data->flashport = flashport; data->fast_spi = 1; - register_shutdown(it8716f_shutdown, data); - if (internal_buses_supported & BUS_SPI) msg_pdbg("Overriding chipset SPI with IT87 SPI.\n"); /* FIXME: Add the SPI bus or replace the other buses with it? */ - register_spi_master(&spi_master_it87xx, data); - return 0; + return register_spi_master(&spi_master_it87xx, data); } int init_superio_ite(void) diff --git a/jlink_spi.c b/jlink_spi.c index daa8eb6..4b34bfd 100644 --- a/jlink_spi.c +++ b/jlink_spi.c @@ -179,6 +179,7 @@ .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, .features = SPI_MASTER_4BA, + .shutdown = jlink_spi_shutdown, }; static int jlink_spi_init(void) @@ -468,11 +469,7 @@ if (!deassert_cs(jlink_data)) goto init_err; - if (register_shutdown(jlink_spi_shutdown, jlink_data)) - goto init_err; - register_spi_master(&spi_master_jlink_spi, jlink_data); - - return 0; + return register_spi_master(&spi_master_jlink_spi, jlink_data); init_err: if (jaylink_devh) diff --git a/linux_spi.c b/linux_spi.c index 46779a0..bd9ffb3 100644 --- a/linux_spi.c +++ b/linux_spi.c @@ -122,6 +122,7 @@ .read = linux_spi_read, .write_256 = linux_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = linux_spi_shutdown, }; /* Read max buffer size from sysfs, or use page size as fallback. */ @@ -239,12 +240,7 @@ spi_data->fd = fd; spi_data->max_kernel_buf_size = max_kernel_buf_size; - if (register_shutdown(linux_spi_shutdown, spi_data)) { - free(spi_data); - goto init_err; - } - register_spi_master(&spi_master_linux, spi_data); - return 0; + return register_spi_master(&spi_master_linux, spi_data); init_err: close(fd); diff --git a/lspcon_i2c_spi.c b/lspcon_i2c_spi.c index e9ff2dd..bed7efc 100644 --- a/lspcon_i2c_spi.c +++ b/lspcon_i2c_spi.c @@ -434,6 +434,7 @@ .read = lspcon_i2c_spi_read, .write_256 = lspcon_i2c_spi_write_256, .write_aai = lspcon_i2c_spi_write_aai, + .shutdown = lspcon_i2c_spi_shutdown, }; static int lspcon_i2c_spi_init(void) @@ -458,10 +459,7 @@ data->fd = fd; - ret |= register_shutdown(lspcon_i2c_spi_shutdown, data); - ret |= register_spi_master(&spi_master_i2c_lspcon, data); - - return ret; + return register_spi_master(&spi_master_i2c_lspcon, data); } const struct programmer_entry programmer_lspcon_i2c_spi = { diff --git a/mec1308.c b/mec1308.c index c085ff2..e9c6214 100644 --- a/mec1308.c +++ b/mec1308.c @@ -405,6 +405,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = mec1308_shutdown, }; static int check_params(void) @@ -507,12 +508,7 @@ internal_buses_supported |= BUS_LPC; /* for LPC <--> SPI bridging */ - if (register_shutdown(mec1308_shutdown, ctx_data)) - goto init_err_cleanup_exit; - register_spi_master(&spi_master_mec1308, ctx_data); - msg_pdbg("%s(): successfully initialized mec1308\n", __func__); - - return 0; + return register_spi_master(&spi_master_mec1308, ctx_data); init_err_cleanup_exit: mec1308_shutdown(ctx_data); diff --git a/mstarddc_spi.c b/mstarddc_spi.c index a7816a4..8e2d5a8 100644 --- a/mstarddc_spi.c +++ b/mstarddc_spi.c @@ -147,6 +147,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = mstarddc_spi_shutdown, }; /* Returns 0 upon success, a negative number upon errors. */ @@ -240,9 +241,6 @@ mstarddc_data->addr = mstarddc_addr; mstarddc_data->doreset = mstarddc_doreset; - // Register shutdown function - register_shutdown(mstarddc_spi_shutdown, mstarddc_data); - // Register programmer register_spi_master(&spi_master_mstarddc, mstarddc_data); out: diff --git a/ni845x_spi.c b/ni845x_spi.c index 2153510..87a1dd8 100644 --- a/ni845x_spi.c +++ b/ni845x_spi.c @@ -536,6 +536,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = ni845x_spi_shutdown, }; static int ni845x_spi_init(void) @@ -625,14 +626,7 @@ return 1; } - if (register_shutdown(ni845x_spi_shutdown, NULL)) { - ni845x_spi_shutdown(NULL); - return 1; - } - - register_spi_master(&spi_programmer_ni845x, NULL); - - return 0; + return register_spi_master(&spi_programmer_ni845x, NULL); } const struct programmer_entry programmer_ni845x_spi = { diff --git a/pickit2_spi.c b/pickit2_spi.c index 73ace4b..17da1d9 100644 --- a/pickit2_spi.c +++ b/pickit2_spi.c @@ -386,6 +386,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = pickit2_shutdown, }; static int pickit2_spi_init(void) @@ -498,11 +499,7 @@ goto init_err_cleanup_exit; } - if (register_shutdown(pickit2_shutdown, pickit2_data)) - goto init_err_cleanup_exit; - register_spi_master(&spi_master_pickit2, pickit2_data); - - return 0; + return register_spi_master(&spi_master_pickit2, pickit2_data); init_err_cleanup_exit: pickit2_shutdown(pickit2_data); diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index f8949f7..1de1f08 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -440,6 +440,7 @@ .read = realtek_mst_i2c_spi_read, .write_256 = realtek_mst_i2c_spi_write_256, .write_aai = realtek_mst_i2c_spi_write_aai, + .shutdown = realtek_mst_i2c_spi_shutdown, }; static int get_params(int *reset, int *enter_isp) @@ -512,10 +513,7 @@ data->fd = fd; data->reset = reset; - ret |= register_shutdown(realtek_mst_i2c_spi_shutdown, data); - ret |= register_spi_master(&spi_master_i2c_realtek_mst, data); - - return ret; + return register_spi_master(&spi_master_i2c_realtek_mst, data); } const struct programmer_entry programmer_realtek_mst_i2c_spi = { diff --git a/sb600spi.c b/sb600spi.c index 5c65475..fe7920f 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -589,6 +589,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = sb600spi_shutdown, }; static const struct spi_master spi_master_yangtze = { @@ -599,6 +600,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = sb600spi_shutdown, }; static const struct spi_master spi_master_promontory = { @@ -609,6 +611,7 @@ .read = promontory_read_memmapped, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = sb600spi_shutdown, }; int sb600_probe_spi(struct pci_dev *dev) @@ -786,8 +789,6 @@ data->flash = NULL; data->spibar = sb600_spibar; - register_shutdown(sb600spi_shutdown, data); - /* Starting with Yangtze the SPI controller got a different interface with a much bigger buffer. */ if (amd_gen < CHIPSET_YANGTZE) register_spi_master(&spi_master_sb600, data); diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 3a5aeb3..58e7a41 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -467,6 +467,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = stlinkv3_spi_shutdown, }; static int stlinkv3_spi_init(void) @@ -531,17 +532,7 @@ stlinkv3_data->usb_ctx = usb_ctx; stlinkv3_data->handle = stlinkv3_handle; - if (register_shutdown(stlinkv3_spi_shutdown, stlinkv3_data)) - goto init_err_cleanup_exit; - - if (register_spi_master(&spi_programmer_stlinkv3, stlinkv3_data)) - return 1; /* shutdown function does cleanup */ - - return 0; - -init_err_cleanup_exit: - stlinkv3_spi_shutdown(stlinkv3_data); - return 1; + return register_spi_master(&spi_programmer_stlinkv3, stlinkv3_data); init_err_exit: if (stlinkv3_handle) diff --git a/usbblaster_spi.c b/usbblaster_spi.c index dfd98ec..bd4b1c9 100644 --- a/usbblaster_spi.c +++ b/usbblaster_spi.c @@ -174,6 +174,7 @@ .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, + .shutdown = usbblaster_shutdown, }; /* Returns 0 upon success, a negative number upon errors. */ @@ -224,12 +225,7 @@ } usbblaster_data->ftdic = ftdic; - if (register_shutdown(usbblaster_shutdown, usbblaster_data)) { - free(usbblaster_data); - return -1; - } - register_spi_master(&spi_master_usbblaster, usbblaster_data); - return 0; + return register_spi_master(&spi_master_usbblaster, usbblaster_data); } const struct programmer_entry programmer_usbblaster_spi = { diff --git a/wbsio_spi.c b/wbsio_spi.c index 2336845..a175b21 100644 --- a/wbsio_spi.c +++ b/wbsio_spi.c @@ -191,6 +191,7 @@ .read = wbsio_spi_read, .write_256 = spi_chip_write_1, .write_aai = spi_chip_write_1, + .shutdown = wbsio_spi_shutdown, }; int wbsio_check_for_spi(void) @@ -214,10 +215,7 @@ } data->spibase = wbsio_spibase; - register_shutdown(wbsio_spi_shutdown, data); - register_spi_master(&spi_master_wbsio, data); - - return 0; + return register_spi_master(&spi_master_wbsio, data); } #endif /* defined(__i386__) || defined(__x86_64__) */ -- To view, visit https://review.coreboot.org/c/flashrom/+/56103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib60300f9ddb295a255d5ef3f8da0e07064207140 Gerrit-Change-Number: 56103 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: spi_master: Add default write_aai function to masters that have none
by Anastasia Klimchuk (Code Review) 06 Jul '21

06 Jul '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56102 ) Change subject: spi_master: Add default write_aai function to masters that have none ...................................................................... spi_master: Add default write_aai function to masters that have none write_aai is required to register a spi master, if it is not set in spi_master struct then register_spi_master returns ERROR_FLASHROM_BUG. Masters in this patch did not have it set in the struct, and register_spi_master always returned an error for them. However return value of register_spi_master was ignored, so this was hard to notice. Next patch in the chain checks return value of register_spi_master. BUG=b:185191942 TEST=builds and ninja test Change-Id: I712e74e11244e1f0ab8d8e245fcd5207ce211219 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ene_lpc.c M mec1308.c 2 files changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/56102/1 diff --git a/ene_lpc.c b/ene_lpc.c index b30eee6..23ba12b 100644 --- a/ene_lpc.c +++ b/ene_lpc.c @@ -511,6 +511,7 @@ .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, }; static int check_params(void) diff --git a/mec1308.c b/mec1308.c index 295bffb..c085ff2 100644 --- a/mec1308.c +++ b/mec1308.c @@ -404,6 +404,7 @@ .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, }; static int check_params(void) -- To view, visit https://review.coreboot.org/c/flashrom/+/56102 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I712e74e11244e1f0ab8d8e245fcd5207ce211219 Gerrit-Change-Number: 56102 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: spi_master: Move shutdown function above spi_master struct
by Anastasia Klimchuk (Code Review) 06 Jul '21

06 Jul '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56101 ) Change subject: spi_master: Move shutdown function above spi_master struct ...................................................................... spi_master: Move shutdown function above spi_master struct This patch prepares spi masters to use new API which allows to register shutdown function in spi_master struct. See also later patch in this chain, where spi masters are converted to new API. BUG=b:185191942 TEST=builds and ninja test Comparing flashrom binary before and after the patch, make clean && make CONFIG_EVERYTHING=yes VERSION=none binary is the same Change-Id: I50716686552b4ddcc6089d5afadb19ef59d9f9b4 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M dediprog.c M digilent_spi.c M dummyflasher.c M it87spi.c M jlink_spi.c M lspcon_i2c_spi.c M pickit2_spi.c M realtek_mst_i2c_spi.c M sb600spi.c M serprog.c M wbsio_spi.c 14 files changed, 184 insertions(+), 188 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/56101/1 diff --git a/bitbang_spi.c b/bitbang_spi.c index 3d973c5..83c3501 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -131,6 +131,13 @@ return 0; } +static int bitbang_spi_shutdown(void *data) +{ + /* FIXME: Run bitbang_spi_release_bus here or per command? */ + free(data); + return 0; +} + static const struct spi_master spi_master_bitbang = { .features = SPI_MASTER_4BA, .max_data_read = MAX_DATA_READ_UNLIMITED, @@ -142,13 +149,6 @@ .write_aai = default_spi_write_aai, }; -static int bitbang_spi_shutdown(void *data) -{ - /* FIXME: Run bitbang_spi_release_bus here or per command? */ - free(data); - return 0; -} - int register_spi_bitbang_master(const struct bitbang_spi_master *master, void *spi_data) { struct spi_master mst = spi_master_bitbang; diff --git a/buspirate_spi.c b/buspirate_spi.c index b56d422..3c0a6ff 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -130,38 +130,6 @@ return ret; } -static struct spi_master spi_master_buspirate = { - .features = SPI_MASTER_4BA, - .max_data_read = MAX_DATA_UNSPECIFIED, - .max_data_write = MAX_DATA_UNSPECIFIED, - .command = NULL, - .multicommand = default_spi_send_multicommand, - .read = default_spi_read, - .write_256 = default_spi_write_256, - .write_aai = default_spi_write_aai, -}; - -static const struct buspirate_speeds spispeeds[] = { - {"30k", 0x0}, - {"125k", 0x1}, - {"250k", 0x2}, - {"1M", 0x3}, - {"2M", 0x4}, - {"2.6M", 0x5}, - {"4M", 0x6}, - {"8M", 0x7}, - {NULL, 0x0} -}; - -static const struct buspirate_speeds serialspeeds[] = { - {"115200", 115200}, - {"230400", 230400}, - {"250000", 250000}, - {"2000000", 2000000}, - {"2M", 2000000}, - {NULL, 0} -}; - static int buspirate_spi_shutdown(void *data) { struct bp_spi_data *bp_data = data; @@ -204,6 +172,38 @@ return ret; } +static struct spi_master spi_master_buspirate = { + .features = SPI_MASTER_4BA, + .max_data_read = MAX_DATA_UNSPECIFIED, + .max_data_write = MAX_DATA_UNSPECIFIED, + .command = NULL, + .multicommand = default_spi_send_multicommand, + .read = default_spi_read, + .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, +}; + +static const struct buspirate_speeds spispeeds[] = { + {"30k", 0x0}, + {"125k", 0x1}, + {"250k", 0x2}, + {"1M", 0x3}, + {"2M", 0x4}, + {"2.6M", 0x5}, + {"4M", 0x6}, + {"8M", 0x7}, + {NULL, 0x0} +}; + +static const struct buspirate_speeds serialspeeds[] = { + {"115200", 115200}, + {"230400", 230400}, + {"250000", 250000}, + {"2000000", 2000000}, + {"2M", 2000000}, + {NULL, 0} +}; + static int buspirate_spi_send_command_v1(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { diff --git a/ch341a_spi.c b/ch341a_spi.c index 185e582..22c9781 100644 --- a/ch341a_spi.c +++ b/ch341a_spi.c @@ -385,20 +385,6 @@ return 0; } -static const struct spi_master spi_master_ch341a_spi = { - .features = SPI_MASTER_4BA, - /* flashrom's current maximum is 256 B. CH341A was tested on Linux and Windows to accept atleast - * 128 kB. Basically there should be no hard limit because transfers are broken up into USB packets - * sent to the device and most of their payload streamed via SPI. */ - .max_data_read = 4 * 1024, - .max_data_write = 4 * 1024, - .command = ch341a_spi_spi_send_command, - .multicommand = default_spi_send_multicommand, - .read = default_spi_read, - .write_256 = default_spi_write_256, - .write_aai = default_spi_write_aai, -}; - static int ch341a_spi_shutdown(void *data) { if (handle == NULL) @@ -419,6 +405,20 @@ return 0; } +static const struct spi_master spi_master_ch341a_spi = { + .features = SPI_MASTER_4BA, + /* flashrom's current maximum is 256 B. CH341A was tested on Linux and Windows to accept atleast + * 128 kB. Basically there should be no hard limit because transfers are broken up into USB packets + * sent to the device and most of their payload streamed via SPI. */ + .max_data_read = 4 * 1024, + .max_data_write = 4 * 1024, + .command = ch341a_spi_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = default_spi_read, + .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, +}; + static int ch341a_spi_init(void) { if (handle != NULL) { diff --git a/dediprog.c b/dediprog.c index b47e8e9..d98af49 100644 --- a/dediprog.c +++ b/dediprog.c @@ -992,6 +992,24 @@ return millivolt; } +static int dediprog_shutdown(void *data) +{ + dediprog_devicetype = DEV_UNKNOWN; + + /* URB 28. Command Set SPI Voltage to 0. */ + if (dediprog_set_spi_voltage(0x0)) + return 1; + + if (libusb_release_interface(dediprog_handle, 0)) { + msg_perr("Could not release USB interface!\n"); + return 1; + } + libusb_close(dediprog_handle); + libusb_exit(usb_ctx); + + return 0; +} + static struct spi_master spi_master_dediprog = { .features = SPI_MASTER_NO_4BA_MODES, .max_data_read = 16, /* 18 seems to work fine as well, but 19 times out sometimes with FW 5.15. */ @@ -1037,24 +1055,6 @@ return 0; } -static int dediprog_shutdown(void *data) -{ - dediprog_devicetype = DEV_UNKNOWN; - - /* URB 28. Command Set SPI Voltage to 0. */ - if (dediprog_set_spi_voltage(0x0)) - return 1; - - if (libusb_release_interface(dediprog_handle, 0)) { - msg_perr("Could not release USB interface!\n"); - return 1; - } - libusb_close(dediprog_handle); - libusb_exit(usb_ctx); - - return 0; -} - static int dediprog_init(void) { char *voltage, *id_str, *device, *spispeed, *target_str; diff --git a/digilent_spi.c b/digilent_spi.c index 8a3699d..0ec402e 100644 --- a/digilent_spi.c +++ b/digilent_spi.c @@ -315,18 +315,6 @@ return 0; } -static const struct spi_master spi_master_digilent_spi = { - .features = SPI_MASTER_4BA, - .max_data_read = 252, - .max_data_write = 252, - .command = digilent_spi_send_command, - .multicommand = default_spi_send_multicommand, - .read = default_spi_read, - .write_256 = default_spi_write_256, - .write_aai = default_spi_write_aai, -}; - - static int digilent_spi_shutdown(void *data) { struct digilent_spi_data *digilent_data = data; @@ -340,6 +328,17 @@ return 0; } +static const struct spi_master spi_master_digilent_spi = { + .features = SPI_MASTER_4BA, + .max_data_read = 252, + .max_data_write = 252, + .command = digilent_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = default_spi_read, + .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, +}; + static bool default_reset(struct libusb_device_handle *handle) { char board[17]; diff --git a/dummyflasher.c b/dummyflasher.c index 049f9d5..2311576 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -580,7 +580,23 @@ return 0; } - +static int dummy_shutdown(void *data) +{ + msg_pspew("%s\n", __func__); + struct emu_data *emu_data = (struct emu_data *)data; + if (emu_data->emu_chip != EMULATE_NONE) { + if (emu_data->emu_persistent_image && emu_data->emu_modified) { + msg_pdbg("Writing %s\n", emu_data->emu_persistent_image); + write_buf_to_file(emu_data->flashchip_contents, + emu_data->emu_chip_size, + emu_data->emu_persistent_image); + } + free(emu_data->emu_persistent_image); + free(emu_data->flashchip_contents); + } + free(data); + return 0; +} static const struct spi_master spi_master_dummyflasher = { .features = SPI_MASTER_4BA, @@ -604,24 +620,6 @@ .chip_writen = dummy_chip_writen, }; -static int dummy_shutdown(void *data) -{ - msg_pspew("%s\n", __func__); - struct emu_data *emu_data = (struct emu_data *)data; - if (emu_data->emu_chip != EMULATE_NONE) { - if (emu_data->emu_persistent_image && emu_data->emu_modified) { - msg_pdbg("Writing %s\n", emu_data->emu_persistent_image); - write_buf_to_file(emu_data->flashchip_contents, - emu_data->emu_chip_size, - emu_data->emu_persistent_image); - } - free(emu_data->emu_persistent_image); - free(emu_data->flashchip_contents); - } - free(data); - return 0; -} - static int init_data(struct emu_data *data, enum chipbustype *dummy_buses_supported) { diff --git a/it87spi.c b/it87spi.c index cbf830b..e552328 100644 --- a/it87spi.c +++ b/it87spi.c @@ -290,6 +290,12 @@ return 0; } +static int it8716f_shutdown(void *data) +{ + free(data); + return 0; +} + static const struct spi_master spi_master_it87xx = { .max_data_read = 3, .max_data_write = MAX_DATA_UNSPECIFIED, @@ -300,13 +306,6 @@ .write_aai = spi_chip_write_1, }; - -static int it8716f_shutdown(void *data) -{ - free(data); - return 0; -} - static uint16_t it87spi_probe(uint16_t port) { uint8_t tmp = 0; diff --git a/jlink_spi.c b/jlink_spi.c index 8b8533c..daa8eb6 100644 --- a/jlink_spi.c +++ b/jlink_spi.c @@ -155,19 +155,6 @@ return 0; } -static const struct spi_master spi_master_jlink_spi = { - /* Maximum data read size in one go (excluding opcode+address). */ - .max_data_read = JTAG_MAX_TRANSFER_SIZE - 5, - /* Maximum data write size in one go (excluding opcode+address). */ - .max_data_write = JTAG_MAX_TRANSFER_SIZE - 5, - .command = jlink_spi_send_command, - .multicommand = default_spi_send_multicommand, - .read = default_spi_read, - .write_256 = default_spi_write_256, - .write_aai = default_spi_write_aai, - .features = SPI_MASTER_4BA, -}; - static int jlink_spi_shutdown(void *data) { struct jlink_spi_data *jlink_data = data; @@ -181,6 +168,19 @@ return 0; } +static const struct spi_master spi_master_jlink_spi = { + /* Maximum data read size in one go (excluding opcode+address). */ + .max_data_read = JTAG_MAX_TRANSFER_SIZE - 5, + /* Maximum data write size in one go (excluding opcode+address). */ + .max_data_write = JTAG_MAX_TRANSFER_SIZE - 5, + .command = jlink_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = default_spi_read, + .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, + .features = SPI_MASTER_4BA, +}; + static int jlink_spi_init(void) { char *arg; diff --git a/lspcon_i2c_spi.c b/lspcon_i2c_spi.c index b590f1d..e9ff2dd 100644 --- a/lspcon_i2c_spi.c +++ b/lspcon_i2c_spi.c @@ -410,16 +410,6 @@ return SPI_GENERIC_ERROR; } -static const struct spi_master spi_master_i2c_lspcon = { - .max_data_read = 16, - .max_data_write = 12, - .command = lspcon_i2c_spi_send_command, - .multicommand = default_spi_send_multicommand, - .read = lspcon_i2c_spi_read, - .write_256 = lspcon_i2c_spi_write_256, - .write_aai = lspcon_i2c_spi_write_aai, -}; - static int lspcon_i2c_spi_shutdown(void *data) { int ret = 0; @@ -436,6 +426,16 @@ return ret; } +static const struct spi_master spi_master_i2c_lspcon = { + .max_data_read = 16, + .max_data_write = 12, + .command = lspcon_i2c_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = lspcon_i2c_spi_read, + .write_256 = lspcon_i2c_spi_write_256, + .write_aai = lspcon_i2c_spi_write_aai, +}; + static int lspcon_i2c_spi_init(void) { int fd = i2c_open_from_programmer_params(REGISTER_ADDRESS, 0); diff --git a/pickit2_spi.c b/pickit2_spi.c index bec9457..73ace4b 100644 --- a/pickit2_spi.c +++ b/pickit2_spi.c @@ -340,16 +340,6 @@ return millivolt; } -static const struct spi_master spi_master_pickit2 = { - .max_data_read = 40, - .max_data_write = 40, - .command = pickit2_spi_send_command, - .multicommand = default_spi_send_multicommand, - .read = default_spi_read, - .write_256 = default_spi_write_256, - .write_aai = default_spi_write_aai, -}; - static int pickit2_shutdown(void *data) { struct pickit2_spi_data *pickit2_data = data; @@ -388,6 +378,16 @@ return ret; } +static const struct spi_master spi_master_pickit2 = { + .max_data_read = 40, + .max_data_write = 40, + .command = pickit2_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = default_spi_read, + .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, +}; + static int pickit2_spi_init(void) { uint8_t buf[CMD_LENGTH] = { diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 22f7e7a..f8949f7 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -410,16 +410,6 @@ return SPI_GENERIC_ERROR; } -static const struct spi_master spi_master_i2c_realtek_mst = { - .max_data_read = 16, - .max_data_write = 8, - .command = realtek_mst_i2c_spi_send_command, - .multicommand = default_spi_send_multicommand, - .read = realtek_mst_i2c_spi_read, - .write_256 = realtek_mst_i2c_spi_write_256, - .write_aai = realtek_mst_i2c_spi_write_aai, -}; - static int realtek_mst_i2c_spi_shutdown(void *data) { int ret = 0; @@ -442,6 +432,16 @@ return ret; } +static const struct spi_master spi_master_i2c_realtek_mst = { + .max_data_read = 16, + .max_data_write = 8, + .command = realtek_mst_i2c_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = realtek_mst_i2c_spi_read, + .write_256 = realtek_mst_i2c_spi_write_256, + .write_aai = realtek_mst_i2c_spi_write_aai, +}; + static int get_params(int *reset, int *enter_isp) { char *reset_str = NULL, *isp_str = NULL; diff --git a/sb600spi.c b/sb600spi.c index c4b25e5..5c65475 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -570,6 +570,17 @@ return 0; } +static int sb600spi_shutdown(void *data) +{ + struct sb600spi_data *sb600_data = data; + struct flashctx *flash = sb600_data->flash; + if (flash) + finalize_flash_access(flash); + + free(data); + return 0; +} + static const struct spi_master spi_master_sb600 = { .max_data_read = FIFO_SIZE_OLD, .max_data_write = FIFO_SIZE_OLD - 3, @@ -600,17 +611,6 @@ .write_aai = default_spi_write_aai, }; -static int sb600spi_shutdown(void *data) -{ - struct sb600spi_data *sb600_data = data; - struct flashctx *flash = sb600_data->flash; - if (flash) - finalize_flash_access(flash); - - free(data); - return 0; -} - int sb600_probe_spi(struct pci_dev *dev) { struct pci_dev *smbus_dev; diff --git a/serprog.c b/serprog.c index e8d1f52..b207fd2 100644 --- a/serprog.c +++ b/serprog.c @@ -394,6 +394,25 @@ return ret; } +static int serprog_shutdown(void *data) +{ + if ((sp_opbuf_usage) || (sp_max_write_n && sp_write_n_bytes)) + if (sp_execute_opbuf() != 0) + msg_pwarn("Could not flush command buffer.\n"); + if (sp_check_commandavail(S_CMD_S_PIN_STATE)) { + uint8_t dis = 0; + if (sp_docommand(S_CMD_S_PIN_STATE, 1, &dis, 0, NULL) == 0) + msg_pdbg(MSGHEADER "Output drivers disabled\n"); + else + msg_pwarn(MSGHEADER "%s: Warning: could not disable output buffers\n", __func__); + } + /* FIXME: fix sockets on windows(?), especially closing */ + serialport_shutdown(&sp_fd); + if (sp_max_write_n) + free(sp_write_n_buf); + return 0; +} + static struct spi_master spi_master_serprog = { .features = SPI_MASTER_4BA, .max_data_read = MAX_DATA_READ_UNLIMITED, @@ -518,25 +537,6 @@ .chip_writen = fallback_chip_writen, }; -static int serprog_shutdown(void *data) -{ - if ((sp_opbuf_usage) || (sp_max_write_n && sp_write_n_bytes)) - if (sp_execute_opbuf() != 0) - msg_pwarn("Could not flush command buffer.\n"); - if (sp_check_commandavail(S_CMD_S_PIN_STATE)) { - uint8_t dis = 0; - if (sp_docommand(S_CMD_S_PIN_STATE, 1, &dis, 0, NULL) == 0) - msg_pdbg(MSGHEADER "Output drivers disabled\n"); - else - msg_pwarn(MSGHEADER "%s: Warning: could not disable output buffers\n", __func__); - } - /* FIXME: fix sockets on windows(?), especially closing */ - serialport_shutdown(&sp_fd); - if (sp_max_write_n) - free(sp_write_n_buf); - return 0; -} - static enum chipbustype serprog_buses_supported = BUS_NONE; static int serprog_init(void) diff --git a/wbsio_spi.c b/wbsio_spi.c index c3814aa..2336845 100644 --- a/wbsio_spi.c +++ b/wbsio_spi.c @@ -177,6 +177,12 @@ return 0; } +static int wbsio_spi_shutdown(void *data) +{ + free(data); + return 0; +} + static const struct spi_master spi_master_wbsio = { .max_data_read = MAX_DATA_UNSPECIFIED, .max_data_write = MAX_DATA_UNSPECIFIED, @@ -187,12 +193,6 @@ .write_aai = spi_chip_write_1, }; -static int wbsio_spi_shutdown(void *data) -{ - free(data); - return 0; -} - int wbsio_check_for_spi(void) { uint16_t wbsio_spibase = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/56101 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I50716686552b4ddcc6089d5afadb19ef59d9f9b4 Gerrit-Change-Number: 56101 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: spi_master: Add shutdown function in spi_master struct
by Anastasia Klimchuk (Code Review) 06 Jul '21

06 Jul '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55932 to look at the new patch set (#3). Change subject: spi_master: Add shutdown function in spi_master struct ...................................................................... spi_master: Add shutdown function in spi_master struct With this, register_spi_master can take care of register_shutdown as well, and every spi master only needs to call register_spi_master instead of calling both register_spi_master and register_shutdown. Testing: In dummyflasher, comment out free(data) in shutdown. Test fails with error: ../dummyflasher.c:949: note: block 0x55e0727a6e40 allocated here ERROR: dummy_init_and_shutdown_test_success leaked 1 block(s) Means, shutdown function is invoked for drivers with "old" API (so, transitioning from old to new API is not breaking anything). Next patch in the chain converts spi masters to use new API. BUG=b:185191942 TEST=builds and ninja test Change-Id: I2dc80dceca2f8204bcd0dad1f51753d7e79f1af5 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M programmer.h M spi.c 2 files changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/55932/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/55932 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2dc80dceca2f8204bcd0dad1f51753d7e79f1af5 Gerrit-Change-Number: 55932 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: layout: Turn overlap debug message into warning
by Edward O'Callaghan (Code Review) 06 Jul '21

06 Jul '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/55999 ) Change subject: layout: Turn overlap debug message into warning ...................................................................... layout: Turn overlap debug message into warning Let's assume this function is only called when overlapping, included regions are fatal. Change-Id: Ie969e0538d302ccd58d3fec3921265ed3621eaa5 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/55999 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> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M layout.c 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/layout.c b/layout.c index 36aa247..534deba 100644 --- a/layout.c +++ b/layout.c @@ -266,8 +266,8 @@ if (lhs->end < rhs->start) continue; - msg_gdbg("Regions %s [0x%08x-0x%08x] and %s [0x%08x-0x%08x] overlap\n", - lhs->name, lhs->start, lhs->end, rhs->name, rhs->start, rhs->end); + msg_gwarn("Regions %s [0x%08x-0x%08x] and %s [0x%08x-0x%08x] overlap\n", + lhs->name, lhs->start, lhs->end, rhs->name, rhs->start, rhs->end); overlap_detected = 1; } } -- To view, visit https://review.coreboot.org/c/flashrom/+/55999 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie969e0538d302ccd58d3fec3921265ed3621eaa5 Gerrit-Change-Number: 55999 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: Thomas Heijligen <src(a)posteo.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]: layout: Fix overlap-check routine
by Edward O'Callaghan (Code Review) 06 Jul '21

06 Jul '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/55997 ) Change subject: layout: Fix overlap-check routine ...................................................................... layout: Fix overlap-check routine Fixes a regression introduced by commit 06a89d713 (layout: Introduce layout_next()). A negation was dropped by accident which resulted in comparisons with later, not-included regions instead of included ones. Change-Id: I18168b9bd9a7d0fe2ba04fb091d1600f65d6dd5b Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/55997 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M layout.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/layout.c b/layout.c index e688f79..36aa247 100644 --- a/layout.c +++ b/layout.c @@ -257,7 +257,7 @@ const struct romentry *rhs = lhs; while ((rhs = layout_next(l, rhs))) { - if (rhs->included) + if (!rhs->included) continue; if (lhs->start > rhs->end) -- To view, visit https://review.coreboot.org/c/flashrom/+/55997 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I18168b9bd9a7d0fe2ba04fb091d1600f65d6dd5b Gerrit-Change-Number: 55997 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: Thomas Heijligen <src(a)posteo.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]: print_wiki.c: Update mailing list reference
by Edward O'Callaghan (Code Review) 05 Jul '21

05 Jul '21

Attention is currently required from: Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56030 ) Change subject: print_wiki.c: Update mailing list reference ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56030 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5c67b5b3be2f306132d8565539bbf10477222026 Gerrit-Change-Number: 56030 Gerrit-PatchSet: 1 Gerrit-Owner: 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 05 Jul 2021 01:57:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: chipset_enable.c: Mark Z97 as OK
by Felix Singer (Code Review) 05 Jul '21

05 Jul '21

Attention is currently required from: Sophie van Soest, Michael Niewöhner. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56060 ) Change subject: chipset_enable.c: Mark Z97 as OK ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/56060/comment/7a4bec9f_e76af6eb PS2, Line 12: > missing Sign-off Done -- To view, visit https://review.coreboot.org/c/flashrom/+/56060 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73bdd9afefae8e7c013d400e17a15e56d84322f4 Gerrit-Change-Number: 56060 Gerrit-PatchSet: 3 Gerrit-Owner: Sophie van Soest <sophie(a)entropie.rocks> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sophie van Soest <sophie(a)entropie.rocks> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Sun, 04 Jul 2021 18:13:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: chipset_enable.c: Mark Z97 as OK
by Sophie van Soest (Code Review) 05 Jul '21

05 Jul '21

Attention is currently required from: Sophie van Soest. Hello Felix Singer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56060 to look at the new patch set (#3). Change subject: chipset_enable.c: Mark Z97 as OK ...................................................................... chipset_enable.c: Mark Z97 as OK Tested on GIGABYTE GA-Z97-HD3. Signed-off-by: Sophie van Soest <sophie(a)entropie.rocks> Change-Id: I73bdd9afefae8e7c013d400e17a15e56d84322f4 --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/56060/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/56060 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73bdd9afefae8e7c013d400e17a15e56d84322f4 Gerrit-Change-Number: 56060 Gerrit-PatchSet: 3 Gerrit-Owner: Sophie van Soest <sophie(a)entropie.rocks> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Sophie van Soest <sophie(a)entropie.rocks> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit