- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: flashchips: Add Macronix MX25U51245G
by Nico Huber (Code Review) 17 Aug '18

17 Aug '18

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/26949 ) Change subject: flashchips: Add Macronix MX25U51245G ...................................................................... flashchips: Add Macronix MX25U51245G Copy 'n paste support for Macronix MX25U51245G. I don't pretend to know a whole lot about SPI FLASH so its mostly copied from other MX25U devices and double checked a few bits and pieces against the datasheet. I have tested basic probe, read, erase and write using layout files. I tested both with 4MB@0x0000000 and 64K0@0x3f00000 (the later means I have tested 4-byte addressing). Change-Id: I2117fc205006088967f3d97644375d10db1791f1 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> Reviewed-on: https://review.coreboot.org/26949 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M flashchips.c M flashchips.h 2 files changed, 50 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 17cb1b4..2bbf60f 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8563,6 +8563,55 @@ { .vendor = "Macronix", + .name = "MX25U51245G", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25U51245G, + .total_size = 65536, + .page_size = 256, + /* OTP: 512B factory programmed and 512B customer programmed; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_4BA, + .tested = TEST_OK_PREW, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = spi_block_erase_21, + }, { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = spi_block_erase_5c, + }, { + .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = spi_block_erase_dc, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + /* TODO: security register */ + .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ + .unlock = spi_disable_blockprotect_bp3_srwd, + .write = spi_chip_write_256, /* Multi I/O supported */ + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {1650, 2000}, + }, + + { + .vendor = "Macronix", .name = "MX25L6495F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, diff --git a/flashchips.h b/flashchips.h index 4dce573..f4880d6 100644 --- a/flashchips.h +++ b/flashchips.h @@ -491,6 +491,7 @@ #define MACRONIX_MX25U6435E 0x2537 /* Same as MX25U6435F */ #define MACRONIX_MX25U12835E 0x2538 /* Same as MX25U12835F */ #define MACRONIX_MX25U25635F 0x2539 +#define MACRONIX_MX25U51245G 0x253a #define MACRONIX_MX25L3235D 0x5E16 /* MX25L3225D/MX25L3235D/MX25L3237D */ #define MACRONIX_MX25L6495F 0x9517 -- To view, visit https://review.coreboot.org/26949 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2117fc205006088967f3d97644375d10db1791f1 Gerrit-Change-Number: 26949 Gerrit-PatchSet: 6 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: programmer: Add Developerbox/CP2104 bit bang driver
by Nico Huber (Code Review) 17 Aug '18

17 Aug '18

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/26948 ) Change subject: programmer: Add Developerbox/CP2104 bit bang driver ...................................................................... programmer: Add Developerbox/CP2104 bit bang driver The 96Boards Developerbox (a.k.a. Synquacer E-series) provides a CP2102 debug UART with its GPIO pins hooked up to the SPI NOR FLASH. The circuit is intended to provide emergency recovery functions without requiring any additional tools (such as a JTAG or SPI programmer). This was expected to be very slow (and it is) but CP2102 is much cheaper than a full dual channel USB comms chip. Read performance is roughly on par with a 2400 baud modem (between 60 and 70 minutes per megabyte if you prefer) and write performance is 50% slower still. The full recovery process, with backup and verification of 4MB data written takes between 14 and 15 hours. Thus it is only really practical as an emergency recovery tool, firmware developers will need to use an alternative programmer. Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/26948 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- M Makefile A developerbox_spi.c M flashrom.c M programmer.h 4 files changed, 278 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve diff --git a/Makefile b/Makefile index cd72972..ddfd711 100644 --- a/Makefile +++ b/Makefile @@ -154,12 +154,17 @@ else override CONFIG_PONY_SPI = no endif -# Dediprog, USB-Blaster, PICkit2, CH341A and FT2232 are not supported under DOS (missing USB support). +# Dediprog, Developerbox, USB-Blaster, PICkit2, CH341A and FT2232 are not supported under DOS (missing USB support). ifeq ($(CONFIG_DEDIPROG), yes) UNSUPPORTED_FEATURES += CONFIG_DEDIPROG=yes else override CONFIG_DEDIPROG = no endif +ifeq ($(CONFIG_DEVELOPERBOX_SPI), yes) +UNSUPPORTED_FEATURES += CONFIG_DEVELOPERBOX_SPI=yes +else +override CONFIG_DEVELOPERBOX_SPI = no +endif ifeq ($(CONFIG_FT2232_SPI), yes) UNSUPPORTED_FEATURES += CONFIG_FT2232_SPI=yes else @@ -311,12 +316,17 @@ else override CONFIG_PONY_SPI = no endif -# Dediprog, USB-Blaster, PICkit2, CH341A and FT2232 are not supported with libpayload (missing libusb support). +# Dediprog, Developerbox, USB-Blaster, PICkit2, CH341A and FT2232 are not supported with libpayload (missing libusb support). ifeq ($(CONFIG_DEDIPROG), yes) UNSUPPORTED_FEATURES += CONFIG_DEDIPROG=yes else override CONFIG_DEDIPROG = no endif +ifeq ($(CONFIG_DEVELOPERBOX_SPI), yes) +UNSUPPORTED_FEATURES += CONFIG_DEVELOPERBOX_SPI=yes +else +override CONFIG_DEVELOPERBOX_SPI = no +endif ifeq ($(CONFIG_FT2232_SPI), yes) UNSUPPORTED_FEATURES += CONFIG_FT2232_SPI=yes else @@ -627,6 +637,9 @@ # Always enable Dediprog SF100 for now. CONFIG_DEDIPROG ?= yes +# Always enable Developerbox emergency recovery for now. +CONFIG_DEVELOPERBOX_SPI ?= yes + # Always enable Marvell SATA controllers for now. CONFIG_SATAMV ?= yes @@ -671,6 +684,7 @@ override CONFIG_CH341A_SPI = no override CONFIG_DEDIPROG = no override CONFIG_DIGILENT_SPI = no +override CONFIG_DEVELOPERBOX_SPI = no endif ifeq ($(CONFIG_ENABLE_LIBPCI_PROGRAMMERS), no) override CONFIG_INTERNAL = no @@ -907,6 +921,12 @@ NEED_LIBUSB1 += CONFIG_DEDIPROG endif +ifeq ($(CONFIG_DEVELOPERBOX_SPI), yes) +FEATURE_CFLAGS += -D'CONFIG_DEVELOPERBOX_SPI=1' +PROGRAMMER_OBJS += developerbox_spi.o +NEED_LIBUSB1 += CONFIG_DEVELOPERBOX_SPI +endif + ifeq ($(CONFIG_SATAMV), yes) FEATURE_CFLAGS += -D'CONFIG_SATAMV=1' PROGRAMMER_OBJS += satamv.o diff --git a/developerbox_spi.c b/developerbox_spi.c new file mode 100644 index 0000000..8482dea --- /dev/null +++ b/developerbox_spi.c @@ -0,0 +1,232 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2018 Linaro Limited + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +/* + * Bit bang driver for the 96Boards Developerbox (a.k.a. Synquacer E-series) + * on-board debug UART. The Developerbox implements its debug UART using a + * CP2102N, a USB to UART bridge which also provides four GPIO pins. On + * Developerbox these can be hooked up to the onboard SPI NOR FLASH and used + * for emergency de-brick without any additional hardware programmer. Bit + * banging over USB is extremely slow compared to a proper SPI programmer so + * this is only practical as a de-brick tool. + * + * Schematic is available here: + * https://www.96boards.org/documentation/enterprise/developerbox/hardware-doc… + * + * To prepare a Developerbox for programming via the debug UART, DSW4 must be + * changed from the default 00000000 to 10001000 (i.e. DSW4-1 and DSW4-5 + * should be turned on). + */ + +#include "platform.h" + +#include <stdlib.h> +#include <string.h> +#include <libusb.h> +#include "programmer.h" +#include "spi.h" + +/* Bit positions for each pin. */ +#define DEVELOPERBOX_SPI_SCK 0 +#define DEVELOPERBOX_SPI_CS 1 +#define DEVELOPERBOX_SPI_MISO 2 +#define DEVELOPERBOX_SPI_MOSI 3 + +/* Config request types */ +#define REQTYPE_HOST_TO_DEVICE 0x40 +#define REQTYPE_DEVICE_TO_HOST 0xc0 + +/* Config request codes */ +#define CP210X_VENDOR_SPECIFIC 0xff + +/* CP210X_VENDOR_SPECIFIC */ +#define CP210X_WRITE_LATCH 0x37e1 +#define CP210X_READ_LATCH 0x00c2 + +const struct dev_entry devs_developerbox_spi[] = { + {0x10c4, 0xea60, OK, "Silicon Labs", "CP2102N USB to UART Bridge Controller"}, + {0}, +}; + +struct libusb_context *usb_ctx; +static libusb_device_handle *cp210x_handle; + +static int cp210x_gpio_get(void) +{ + int res; + uint8_t gpio; + + res = libusb_control_transfer(cp210x_handle, REQTYPE_DEVICE_TO_HOST, + CP210X_VENDOR_SPECIFIC, CP210X_READ_LATCH, + 0, &gpio, 1, 0); + if (res < 0) { + msg_perr("Failed to read GPIO pins (%s)\n", libusb_error_name(res)); + return 0; + } + + return gpio; +} + +static void cp210x_gpio_set(uint8_t val, uint8_t mask) +{ + int res; + uint16_t gpio; + + gpio = ((val & 0xf) << 8) | (mask & 0xf); + + /* Set relay state on the card */ + res = libusb_control_transfer(cp210x_handle, REQTYPE_HOST_TO_DEVICE, + CP210X_VENDOR_SPECIFIC, CP210X_WRITE_LATCH, + gpio, NULL, 0, 0); + if (res < 0) + msg_perr("Failed to read GPIO pins (%s)\n", libusb_error_name(res)); +} + +static void cp210x_bitbang_set_cs(int val) +{ + cp210x_gpio_set(val << DEVELOPERBOX_SPI_CS, 1 << DEVELOPERBOX_SPI_CS); +} + +static void cp210x_bitbang_set_sck(int val) +{ + cp210x_gpio_set(val << DEVELOPERBOX_SPI_SCK, 1 << DEVELOPERBOX_SPI_SCK); +} + +static void cp210x_bitbang_set_mosi(int val) +{ + cp210x_gpio_set(val << DEVELOPERBOX_SPI_MOSI, 1 << DEVELOPERBOX_SPI_MOSI); +} + +static int cp210x_bitbang_get_miso(void) +{ + return !!(cp210x_gpio_get() & (1 << DEVELOPERBOX_SPI_MISO)); +} + +static void cp210x_bitbang_set_sck_set_mosi(int sck, int mosi) +{ + cp210x_gpio_set(sck << DEVELOPERBOX_SPI_SCK | mosi << DEVELOPERBOX_SPI_MOSI, + 1 << DEVELOPERBOX_SPI_SCK | 1 << DEVELOPERBOX_SPI_MOSI); +} + +static const struct bitbang_spi_master bitbang_spi_master_cp210x = { + .type = BITBANG_SPI_MASTER_DEVELOPERBOX, + .set_cs = cp210x_bitbang_set_cs, + .set_sck = cp210x_bitbang_set_sck, + .set_mosi = cp210x_bitbang_set_mosi, + .get_miso = cp210x_bitbang_get_miso, + .set_sck_set_mosi = cp210x_bitbang_set_sck_set_mosi, +}; + +static struct libusb_device_handle *get_device_by_vid_pid_serial(uint16_t vid, uint16_t pid, + const char *serialno) +{ + struct libusb_device **list; + ssize_t count = libusb_get_device_list(usb_ctx, &list); + if (count < 0) { + msg_perr("Getting the USB device list failed (%s)!\n", libusb_error_name(count)); + return NULL; + } + + ssize_t i = 0; + for (i = 0; i < count; i++) { + struct libusb_device *dev = list[i]; + struct libusb_device_descriptor desc; + struct libusb_device_handle *handle; + + int res = libusb_get_device_descriptor(dev, &desc); + if (res != 0) { + msg_perr("Reading the USB device descriptor failed (%s)!\n", libusb_error_name(res)); + continue; + } + + if ((desc.idVendor != vid) && (desc.idProduct != pid)) + continue; + + msg_pdbg("Found USB device %04"PRIx16":%04"PRIx16" at address %d-%d.\n", + desc.idVendor, desc.idProduct, + libusb_get_bus_number(dev), libusb_get_device_address(dev)); + + res = libusb_open(dev, &handle); + if (res != 0) { + msg_perr("Opening the USB device failed (%s)!\n", libusb_error_name(res)); + continue; + } + + if (serialno) { + unsigned char myserial[64]; + res = libusb_get_string_descriptor_ascii(handle, desc.iSerialNumber, myserial, + sizeof(myserial)); + if (res < 0) { + msg_perr("Reading the USB serialno failed (%s)!\n", libusb_error_name(res)); + libusb_close(handle); + continue; + } + msg_pdbg("Serial number is %s\n", myserial); + + /* Filter out any serial number that does not commence with serialno */ + if (0 != strncmp(serialno, (char *) myserial, strlen(serialno))) { + libusb_close(handle); + continue; + } + } + + libusb_free_device_list(list, 1); + return handle; + } + + libusb_free_device_list(list, 1); + return NULL; +} + +static int developerbox_spi_shutdown(void *data) +{ + libusb_close(cp210x_handle); + libusb_exit(usb_ctx); + + return 0; +} + +int developerbox_spi_init(void) +{ + libusb_init(&usb_ctx); + if (!usb_ctx) { + msg_perr("Could not initialize libusb!\n"); + return 1; + } + + char *serialno = extract_programmer_param("serial"); + if (serialno) + msg_pdbg("Looking for serial number commencing %s\n", serialno); + cp210x_handle = get_device_by_vid_pid_serial( + devs_developerbox_spi[0].vendor_id, devs_developerbox_spi[0].device_id, serialno); + free(serialno); + if (!cp210x_handle) { + msg_perr("Could not find a Developerbox programmer on USB.\n"); + goto err_exit; + } + + if (register_shutdown(developerbox_spi_shutdown, NULL)) + goto err_exit; + + if (register_spi_bitbang_master(&bitbang_spi_master_cp210x)) + goto err_exit; + + return 0; + +err_exit: + libusb_exit(usb_ctx); + return 1; +} diff --git a/flashrom.c b/flashrom.c index 1866a18..094630c 100644 --- a/flashrom.c +++ b/flashrom.c @@ -255,6 +255,18 @@ }, #endif +#if CONFIG_DEVELOPERBOX_SPI == 1 + { + .name = "developerbox", + .type = USB, + .devs.dev = devs_developerbox_spi, + .init = developerbox_spi_init, + .map_flash_region = fallback_map, + .unmap_flash_region = fallback_unmap, + .delay = internal_delay, + }, +#endif + #if CONFIG_RAYER_SPI == 1 { .name = "rayer_spi", diff --git a/programmer.h b/programmer.h index 7e530b6..300cf5f 100644 --- a/programmer.h +++ b/programmer.h @@ -73,6 +73,9 @@ #if CONFIG_DEDIPROG == 1 PROGRAMMER_DEDIPROG, #endif +#if CONFIG_DEVELOPERBOX_SPI == 1 + PROGRAMMER_DEVELOPERBOX_SPI, +#endif #if CONFIG_RAYER_SPI == 1 PROGRAMMER_RAYER_SPI, #endif @@ -172,6 +175,9 @@ #if CONFIG_OGP_SPI == 1 BITBANG_SPI_MASTER_OGP, #endif +#if CONFIG_DEVELOPERBOX_SPI == 1 + BITBANG_SPI_MASTER_DEVELOPERBOX, +#endif }; struct bitbang_spi_master { @@ -548,6 +554,12 @@ extern const struct dev_entry devs_dediprog[]; #endif +/* developerbox_spi.c */ +#if CONFIG_DEVELOPERBOX_SPI == 1 +int developerbox_spi_init(void); +extern const struct dev_entry devs_developerbox_spi[]; +#endif + /* ch341a_spi.c */ #if CONFIG_CH341A_SPI == 1 int ch341a_spi_init(void); -- To view, visit https://review.coreboot.org/26948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Gerrit-Change-Number: 26948 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com>

1 0

Change in flashrom[master]: bitbang_spi: Add half-duplex optimizations
by Nico Huber (Code Review) 17 Aug '18

17 Aug '18

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/26947 ) Change subject: bitbang_spi: Add half-duplex optimizations ...................................................................... bitbang_spi: Add half-duplex optimizations Currently, the core of bitbang_spi is a full-duplex SPI loop but in practice this code is only ever used half-duplex. Spliting this code into two half duplex loops allows us to optimize performance by reducing communications and/or CPU pipeline stalls. The speed up varies depending on how much the overhead of getting/setting pins dominates execution time. For a USB bit bang driver running on a 7th generation Core i5, the time to probe drops from ~7.7 seconds to ~6.7 seconds when this patch is applied. Change-Id: I33b9f363716f651146c09113bda5fffe53b16738 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> Reviewed-on: https://review.coreboot.org/26947 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M bitbang_spi.c 1 file changed, 19 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/bitbang_spi.c b/bitbang_spi.c index 2c7a3f1..4b39164 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -119,14 +119,16 @@ return 0; } -static uint8_t bitbang_spi_rw_byte(const struct bitbang_spi_master *master, - uint8_t val) +static uint8_t bitbang_spi_read_byte(const struct bitbang_spi_master *master) { uint8_t ret = 0; int i; for (i = 7; i >= 0; i--) { - bitbang_spi_set_sck_set_mosi(master, 0, (val >> i) & 1); + if (i == 0) + bitbang_spi_set_sck_set_mosi(master, 0, 0); + else + bitbang_spi_set_sck(master, 0); programmer_delay(master->half_period); ret <<= 1; ret |= bitbang_spi_set_sck_get_miso(master, 1); @@ -135,6 +137,18 @@ return ret; } +static void bitbang_spi_write_byte(const struct bitbang_spi_master *master, uint8_t val) +{ + int i; + + for (i = 7; i >= 0; i--) { + bitbang_spi_set_sck_set_mosi(master, 0, (val >> i) & 1); + programmer_delay(master->half_period); + bitbang_spi_set_sck(master, 1); + programmer_delay(master->half_period); + } +} + static int bitbang_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, @@ -150,9 +164,9 @@ bitbang_spi_request_bus(master); bitbang_spi_set_cs(master, 0); for (i = 0; i < writecnt; i++) - bitbang_spi_rw_byte(master, writearr[i]); + bitbang_spi_write_byte(master, writearr[i]); for (i = 0; i < readcnt; i++) - readarr[i] = bitbang_spi_rw_byte(master, 0); + readarr[i] = bitbang_spi_read_byte(master); bitbang_spi_set_sck(master, 0); programmer_delay(master->half_period); -- To view, visit https://review.coreboot.org/26947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I33b9f363716f651146c09113bda5fffe53b16738 Gerrit-Change-Number: 26947 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: bitbang_spi: Add functions to optimize xfers
by Nico Huber (Code Review) 17 Aug '18

17 Aug '18

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/26946 ) Change subject: bitbang_spi: Add functions to optimize xfers ...................................................................... bitbang_spi: Add functions to optimize xfers On systems where the overhead of getting/setting pins is much greater than the half period (for example, USB bit banging) it significantly boosts performance if we can bang more than one bit at the same time. Add support for setting sck at the same time as mosi or miso activity. The speed up varies depending on how much the overhead of getting/setting pins dominates execution time. For a USB bit bang driver running on a 7th generation Core i5, the time to probe drops from ~9.2 seconds to ~7.7 seconds when set_clk_set_mosi() is implemented. Change-Id: Ic3430a9df34844cdfa82e109456be788eaa1789a Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> Reviewed-on: https://review.coreboot.org/26946 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi(a)google.com> Reviewed-by: Idwer Vollering <vidwer(a)gmail.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M bitbang_spi.c M programmer.h 2 files changed, 27 insertions(+), 16 deletions(-) Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, but someone else must approve Idwer Vollering: Looks good to me, but someone else must approve Nico Huber: Looks good to me, approved diff --git a/bitbang_spi.c b/bitbang_spi.c index edbc3e9..2c7a3f1 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -32,16 +32,6 @@ master->set_sck(val); } -static void bitbang_spi_set_mosi(const struct bitbang_spi_master * const master, int val) -{ - master->set_mosi(val); -} - -static int bitbang_spi_get_miso(const struct bitbang_spi_master * const master) -{ - return master->get_miso(); -} - static void bitbang_spi_request_bus(const struct bitbang_spi_master * const master) { if (master->request_bus) @@ -54,6 +44,26 @@ master->release_bus(); } +static void bitbang_spi_set_sck_set_mosi(const struct bitbang_spi_master * const master, int sck, int mosi) +{ + if (master->set_sck_set_mosi) { + master->set_sck_set_mosi(sck, mosi); + return; + } + + master->set_sck(sck); + master->set_mosi(mosi); +} + +static int bitbang_spi_set_sck_get_miso(const struct bitbang_spi_master * const master, int sck) +{ + if (master->set_sck_get_miso) + return master->set_sck_get_miso(sck); + + master->set_sck(sck); + return master->get_miso(); +} + static int bitbang_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, @@ -101,8 +111,7 @@ /* Only mess with the bus if we're sure nobody else uses it. */ bitbang_spi_request_bus(master); bitbang_spi_set_cs(master, 1); - bitbang_spi_set_sck(master, 0); - bitbang_spi_set_mosi(master, 0); + bitbang_spi_set_sck_set_mosi(master, 0, 0); /* FIXME: Release SPI bus here and request it again for each command or * don't release it now and only release it on programmer shutdown? */ @@ -117,13 +126,11 @@ int i; for (i = 7; i >= 0; i--) { - bitbang_spi_set_mosi(master, (val >> i) & 1); + bitbang_spi_set_sck_set_mosi(master, 0, (val >> i) & 1); programmer_delay(master->half_period); - bitbang_spi_set_sck(master, 1); ret <<= 1; - ret |= bitbang_spi_get_miso(master); + ret |= bitbang_spi_set_sck_get_miso(master, 1); programmer_delay(master->half_period); - bitbang_spi_set_sck(master, 0); } return ret; } @@ -147,6 +154,7 @@ for (i = 0; i < readcnt; i++) readarr[i] = bitbang_spi_rw_byte(master, 0); + bitbang_spi_set_sck(master, 0); programmer_delay(master->half_period); bitbang_spi_set_cs(master, 1); programmer_delay(master->half_period); diff --git a/programmer.h b/programmer.h index ff81036..7e530b6 100644 --- a/programmer.h +++ b/programmer.h @@ -184,6 +184,9 @@ int (*get_miso) (void); void (*request_bus) (void); void (*release_bus) (void); + /* optional functions to optimize xfers */ + void (*set_sck_set_mosi) (int sck, int mosi); + int (*set_sck_get_miso) (int sck); /* Length of half a clock period in usecs. */ unsigned int half_period; }; -- To view, visit https://review.coreboot.org/26946 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic3430a9df34844cdfa82e109456be788eaa1789a Gerrit-Change-Number: 26946 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: programmer: Add Developerbox/CP2104 bit bang driver
by Paul Menzel (Code Review) 17 Aug '18

17 Aug '18

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/26948 ) Change subject: programmer: Add Developerbox/CP2104 bit bang driver ...................................................................... Patch Set 6: Code-Review+1 -- To view, visit https://review.coreboot.org/26948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Gerrit-Change-Number: 26948 Gerrit-PatchSet: 6 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Comment-Date: Fri, 17 Aug 2018 11:18:24 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: programmer: Add Developerbox/CP2104 bit bang driver
by Nico Huber (Code Review) 16 Aug '18

16 Aug '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/26948 ) Change subject: programmer: Add Developerbox/CP2104 bit bang driver ...................................................................... Patch Set 6: Code-Review+2 Added my signed-off-by for the relicensed USB device filter code. -- To view, visit https://review.coreboot.org/26948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Gerrit-Change-Number: 26948 Gerrit-PatchSet: 6 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Comment-Date: Thu, 16 Aug 2018 20:49:14 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: programmer: Add Developerbox/CP2104 bit bang driver
by Nico Huber (Code Review) 16 Aug '18

16 Aug '18

Nico Huber has uploaded a new patch set (#6) to the change originally created by Daniel Thompson. ( https://review.coreboot.org/26948 ) Change subject: programmer: Add Developerbox/CP2104 bit bang driver ...................................................................... programmer: Add Developerbox/CP2104 bit bang driver The 96Boards Developerbox (a.k.a. Synquacer E-series) provides a CP2102 debug UART with its GPIO pins hooked up to the SPI NOR FLASH. The circuit is intended to provide emergency recovery functions without requiring any additional tools (such as a JTAG or SPI programmer). This was expected to be very slow (and it is) but CP2102 is much cheaper than a full dual channel USB comms chip. Read performance is roughly on par with a 2400 baud modem (between 60 and 70 minutes per megabyte if you prefer) and write performance is 50% slower still. The full recovery process, with backup and verification of 4MB data written takes between 14 and 15 hours. Thus it is only really practical as an emergency recovery tool, firmware developers will need to use an alternative programmer. Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M Makefile A developerbox_spi.c M flashrom.c M programmer.h 4 files changed, 278 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/26948/6 -- To view, visit https://review.coreboot.org/26948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Gerrit-Change-Number: 26948 Gerrit-PatchSet: 6 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[master]: Add initial J-Link SPI programmer
by Nico Huber (Code Review) 16 Aug '18

16 Aug '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/28087 ) Change subject: Add initial J-Link SPI programmer ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/28087/1/jlink_spi.c File jlink_spi.c: https://review.coreboot.org/#/c/28087/1/jlink_spi.c@168 PS1, Line 168: .max_data_read = JTAG_MAX_TRANSFER_SIZE, The correct interpretation of the comment above is that we have to tell here how much payload we can take. Which is `JTAG_MAX_TRANSFER_SIZE - 4` or `- 5` if we allow 4B addresses. (The interface is just wrong and should change in the future. The SPI drivers shouldn't predict the protocol overhead. But for now we have to do the calcu- lation here.) Please add `- 5` to both `.max_data_(read|write)`, and set `.features = SPI_MASTER_4BA,` (I don't see a reason why it shouldn't work oob). -- To view, visit https://review.coreboot.org/28087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie03a054a75457ec9e1cab36ea124bb53b10e8d7e Gerrit-Change-Number: 28087 Gerrit-PatchSet: 1 Gerrit-Owner: Marc Schink <flashrom-dev(a)marcschink.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 16 Aug 2018 20:34:38 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: digilent_spi: add a driver for the iCEblink40 development board
by Nico Huber (Code Review) 16 Aug '18

16 Aug '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23338 ) Change subject: digilent_spi: add a driver for the iCEblink40 development board ...................................................................... Patch Set 8: (1 comment) https://review.coreboot.org/#/c/23338/4/digilent_spi.c File digilent_spi.c: https://review.coreboot.org/#/c/23338/4/digilent_spi.c@276 PS4, Line 276: ret = spi_start_io(read_follows, writecnt); : if (ret != 0) > The overhead is already reduced by setting it to 252. Now, I agree, […] Hmmm, might have overlooked something here. The "empirically determined" 252 were tested with 3B addresses I guess? but we advertise 4B address support. So this should probably be 251B (256B - 4B address - 1B opcode). (One nice day, this should be refactored throughout flashrom, so the SPI drivers wouldn't have to care about the protocol overhead.) -- To view, visit https://review.coreboot.org/23338 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ffcd9a2db4395816f0e8b6ce6c3b0d8e930c9e6 Gerrit-Change-Number: 23338 Gerrit-PatchSet: 8 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> Gerrit-Reviewer: Lubomir Rintel <lkundrak(a)v3.sk> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 16 Aug 2018 20:26:38 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add initial J-Link SPI programmer
by Nico Huber (Code Review) 15 Aug '18

15 Aug '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/28087 ) Change subject: Add initial J-Link SPI programmer ...................................................................... Patch Set 1: (17 comments) https://review.coreboot.org/#/c/28087/1/Makefile File Makefile: https://review.coreboot.org/#/c/28087/1/Makefile@646 PS1, Line 646: # Enable J-Link for now. please update the comment https://review.coreboot.org/#/c/28087/1/flashrom.8.tmpl File flashrom.8.tmpl: PS1: nice documentation https://review.coreboot.org/#/c/28087/1/flashrom.8.tmpl@1133 PS1, Line 1133: 4: GND On the first look, I thought the listing has the same order as the physical pins. Let's see how it'd look like if it did: +-------+ | 1 2 | 1: VTref 2: | 3 4 | 3: TRST 4: GND | 5 6 | 5: TDI 6: GND +-+ 7 8 | 7: 8: GND | 9 10 | 9: TCK 10: GND | 11 12 | 11: 12: GND +-+ 13 14 | 13: TDO 14: | 15 16 | 15: RESET 16: | 17 18 | 17: 18: | 19 20 | 19: PWR_5V 20: +-------+ IMO, more convenient, your call. https://review.coreboot.org/#/c/28087/1/flashrom.8.tmpl@1157 PS1, Line 1157: syntax where \fBfrequency\fP is the SPI clock frequency in Hz. For all other programmers that take arbitrary numbers for `spispeed` it's in kHz. Please use that here, too. (There is always talk about unifying the code to parse `spispeed` throughout flashrom, patches are welcome. :)) https://review.coreboot.org/#/c/28087/1/jlink_spi.c File jlink_spi.c: PS1: 112 columns line limit, please remove unnecessary line breaks. good coding overall :) https://review.coreboot.org/#/c/28087/1/jlink_spi.c@8 PS1, Line 8: * the Free Software Foundation; version 2 of the License. My usual question here: Is the limitation to v2 intended (or just copy pasted from one of the problematic files)? https://review.coreboot.org/#/c/28087/1/jlink_spi.c@14 PS1, Line 14: * : * You should have received a copy of the GNU General Public License : * along with this program; if not, write to the Free Software : * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA Please remove. https://review.coreboot.org/#/c/28087/1/jlink_spi.c@20 PS1, Line 20: Please start with /* on an otherwise empty line (like in your other comments). https://review.coreboot.org/#/c/28087/1/jlink_spi.c@21 PS1, Line 21: * See https://www.segger.com/ for more info. Usually, URLs are discouraged in the code, but I guess this one is somewhat stable? https://review.coreboot.org/#/c/28087/1/jlink_spi.c@39 PS1, Line 39: UINT16_MAX / 8 This is limited by the *software* interface, right? Please mention that in the commant, if so. https://review.coreboot.org/#/c/28087/1/jlink_spi.c@71 PS1, Line 71: jaylink_strerror(ret)); We exercise a 112 column limit for flashrom, no need to break the line. https://review.coreboot.org/#/c/28087/1/jlink_spi.c@78 PS1, Line 78: jaylink_jtag_clear_trst Not sure if the distinction which function was actually called would be helpful for debugging (you'd know which one it was by the state of `reset_cs`). OTOH, it would save some lines if you'd move the return check out of the outer if block (e.g. with "Asserting CS failed: %s.\n"). https://review.coreboot.org/#/c/28087/1/jlink_spi.c@95 PS1, Line 95: clear set https://review.coreboot.org/#/c/28087/1/jlink_spi.c@103 PS1, Line 103: clear set https://review.coreboot.org/#/c/28087/1/jlink_spi.c@130 PS1, Line 130: free(buffer); Do not free it here or set `buffer = NULL;`. Otherwise we'd double free on shutdown, I guess. https://review.coreboot.org/#/c/28087/1/jlink_spi.c@134 PS1, Line 134: buffer = tmp; `buffer_size = length;` might be a good idea here oO https://review.coreboot.org/#/c/28087/1/jlink_spi.c@378 PS1, Line 378: JAYLINK_DEV_EXT_CAPS_SIZE `sizeof(caps)` preferred -- To view, visit https://review.coreboot.org/28087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie03a054a75457ec9e1cab36ea124bb53b10e8d7e Gerrit-Change-Number: 28087 Gerrit-PatchSet: 1 Gerrit-Owner: Marc Schink <flashrom-dev(a)marcschink.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 15 Aug 2018 18:45:57 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit