December 2017 - flashrom-gerrit

Change in flashrom[master]: Fwd: Software info for Willem/EZOflash
by build bot (Jenkins) (Code Review) Dec. 31, 2017

Dec. 31, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23053 ) Change subject: Fwd: Software info for Willem/EZOflash ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom_gerrit/848/ : FAILURE https://qa.coreboot.org/job/flashrom-customrules/989/ : SUCCESS -- To view, visit https://review.coreboot.org/23053 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: Idc4d98593076c80fd1d6ac4596940d1d7977343c Gerrit-Change-Number: 23053 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 31 Dec 2017 20:38:18 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Fwd: Software info for Willem/EZOflash
by Nico Huber (Code Review) Dec. 31, 2017

Dec. 31, 2017

Hello Carl-Daniel Hailfinger, I'd like you to do a code review. Please visit https://review.coreboot.org/23053 to review the following change. Change subject: Fwd: Software info for Willem/EZOflash ...................................................................... Fwd: Software info for Willem/EZOflash Hi Egils, thanks for the Ezoflash programmer doc. I have tried to implement init and SPI flash support correctly, but I don't own the hardware, so I can't test. A few places are marked FIXME, and it would be really nice if you could take a look. I basically took the RayeR SPIPGM driver in flashrom and modified and extended it. If your mailer mangles the patch, you can also find it at the top of http://patchwork.coreboot.org/project/flashrom/list/ Add support for the Ezoflash programmer. Hopefully complete init. Only SPI supported for now, and with additional 1 us delay per clock. Missing shutdown. Compiles, otherwise untested. If you want to test, please apply the patch to latest flashrom, then compile with make CONFIG_EZO=yes To test the code, run ./flashrom -p ezo -V Change-Id: Idc4d98593076c80fd1d6ac4596940d1d7977343c Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> --- M Makefile A ezo.c M flashrom.8 M flashrom.c M programmer.h M spi.c 6 files changed, 317 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/23053/1 diff --git a/Makefile b/Makefile index 3c5d256..7c51b73 100644 --- a/Makefile +++ b/Makefile @@ -114,6 +114,9 @@ # RayeR SPIPGM hardware support CONFIG_RAYER_SPI ?= yes +# Ezoflash hardware support, disabled by default until it works fine. +CONFIG_EZO ?= no + # Bitbanging SPI infrastructure, default off unless needed. ifeq ($(CONFIG_RAYER_SPI), yes) override CONFIG_BITBANG_SPI = yes @@ -121,9 +124,13 @@ ifeq ($(CONFIG_INTERNAL), yes) override CONFIG_BITBANG_SPI = yes else +ifeq ($(CONFIG_EZO), yes) +override CONFIG_BITBANG_SPI = yes +else CONFIG_BITBANG_SPI ?= no endif endif +endif # Always enable 3Com NICs for now. CONFIG_NIC3COM ?= yes @@ -184,6 +191,13 @@ NEED_PCI := yes endif +ifeq ($(CONFIG_EZO), yes) +FEATURE_CFLAGS += -D'CONFIG_EZO=1' +PROGRAMMER_OBJS += ezo.o +# Actually, NEED_PCI is wrong. NEED_IOPORT_ACCESS would be more correct. +NEED_PCI := yes +endif + ifeq ($(CONFIG_BITBANG_SPI), yes) FEATURE_CFLAGS += -D'CONFIG_BITBANG_SPI=1' PROGRAMMER_OBJS += bitbang_spi.o diff --git a/ezo.c b/ezo.c new file mode 100644 index 0000000..6ce0050 --- /dev/null +++ b/ezo.c @@ -0,0 +1,248 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2009,2010 Carl-Daniel Hailfinger + * + * 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; version 2 of the License. + * + * 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. + * + * 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 + */ + +/* Driver for the Ezoflash hardware by Egils Zomerfelds "Ezo". + * See http:// for schematics and instructions. + */ + +/* This driver uses non-portable direct I/O port accesses which won't work on + * any non-x86 platform, and even on x86 there is a high chance there will be + * collisions with any loaded parallel port drivers. + * The big advantage of direct port I/O is OS independence and speed because + * most OS parport drivers will perform many unnecessary accesses although + * this driver just treats the parallel port as a GPIO set. + */ +#if defined(__i386__) || defined(__x86_64__) + +#include "flash.h" +#include "programmer.h" + +/* We have two sets of pins, out and in. The numbers for both sets are + * independent and are bitshift values, not real pin numbers. + */ +/* Pins for master->slave direction, data port */ +#define SPI_MOSI_PIN 0 +#define SPI_SCK_PIN 1 +/* Pins for slave->master direction, status port */ +#define SPI_MISO_PIN 7 +/* Pins for master->slave direction, command port */ +#define SPI_CS_PIN 3 +#define VCC_PIN 2 +#define VPP_PIN 0 +#define PAR_OE_PIN 1 +#define PAR_WE_PIN 3 + +static int lpt_iobase; + +/* FIXME: All ezo_bitbang_set_* functions could use caching of the value + * stored at port lpt_iobase+n to avoid unnecessary INB. + */ + +static void ezo_bitbang_set_cs(int val) +{ + uint8_t tmp; + + tmp = INB(lpt_iobase + 2); + tmp &= ~(1 << SPI_CS_PIN); + tmp |= (val << SPI_CS_PIN); + OUTB(tmp, lpt_iobase + 2); +} + +static void ezo_bitbang_set_sck(int val) +{ + uint8_t tmp; + + tmp = INB(lpt_iobase); + tmp &= ~(1 << SPI_SCK_PIN); + tmp |= (val << SPI_SCK_PIN); + OUTB(tmp, lpt_iobase); +} + +static void ezo_bitbang_set_mosi(int val) +{ + uint8_t tmp; + + tmp = INB(lpt_iobase); + tmp &= ~(1 << SPI_MOSI_PIN); + tmp |= (val << SPI_MOSI_PIN); + OUTB(tmp, lpt_iobase); +} + +static int ezo_bitbang_get_miso(void) +{ + uint8_t tmp; + + tmp = INB(lpt_iobase + 1); + tmp = (tmp >> SPI_MISO_PIN) & 0x1; + return tmp; +} + +static void ezo_set_vcc(int val) +{ + uint8_t tmp; + + tmp = INB(lpt_iobase + 2); + tmp &= ~(1 << VCC_PIN); + tmp |= (val << VCC_PIN); + OUTB(tmp, lpt_iobase + 2); +} + +static void ezo_set_vpp(int val) +{ + uint8_t tmp; + + tmp = INB(lpt_iobase + 2); + tmp &= ~(1 << VPP_PIN); + tmp |= (val << VPP_PIN); + OUTB(tmp, lpt_iobase + 2); +} + +/* Please note that OE# is active low, but inverted by the flasher hardware. + * ezo_set_par_oe(1) will therefore set OE# to 0. + */ +static void ezo_set_par_oe(int val) +{ + uint8_t tmp; + + tmp = INB(lpt_iobase + 2); + tmp &= ~(1 << PAR_OE_PIN); + tmp |= (val << PAR_OE_PIN); + OUTB(tmp, lpt_iobase + 2); +} + +static void ezo_set_par_we(int val) +{ + uint8_t tmp; + + tmp = INB(lpt_iobase + 2); + tmp &= ~(1 << PAR_WE_PIN); + tmp |= (val << PAR_WE_PIN); + OUTB(tmp, lpt_iobase + 2); +} + +static void ezo_bitbang_set_parallel_data(int val) +{ + OUTB(val & 0xff, lpt_iobase); +} + +static const struct bitbang_spi_master bitbang_spi_master_ezo = { + .type = BITBANG_SPI_MASTER_EZO, + .set_cs = ezo_bitbang_set_cs, + .set_sck = ezo_bitbang_set_sck, + .set_mosi = ezo_bitbang_set_mosi, + .get_miso = ezo_bitbang_get_miso, +}; + +static void ezo_prg_rst(void) +{ + ezo_set_vpp(0); + ezo_set_vcc(0); + /* FIXME: WRITE_ADRESS (0x000000) */ + ezo_set_par_oe(1); + ezo_set_par_we(0); + ezo_bitbang_set_parallel_data(0x00); +} + +int ezo_init(void) +{ + int tmp; + + /* Pick a default value for now. */ + lpt_iobase = 0x378; + + msg_pinfo("Ezoflash Willem compatible programmer driver. Untested.\n"); + msg_pdbg("Using port 0x%x as I/O base for parallel port access.\n", + lpt_iobase); + + get_io_perms(); + + /* FIXME: Check init sequence. */ + + /* VCC_ON, VCC_OFF. */ + /* FIXME: Datasheet contradiction! ON->OFF should be 1->0. */ + ezo_set_vcc(0); + ezo_set_vcc(1); + + /* VPP_ON, VPP_OFF. */ + /* FIXME: Datasheet contradiction! ON->OFF should be 1->0. */ + ezo_set_vpp(0); + ezo_set_vpp(1); + + /* OE_LOW, OE_HIGH. OE# is inverted in the flasher. */ + ezo_set_par_oe(1); + ezo_set_par_oe(0); + + /* WE_LOW, WE_HIGH. */ + ezo_set_par_we(0); + ezo_set_par_we(1); + + /* READ_SERBIT. */ + /* FIXME: Throw away the result or check it? */ + ezo_bitbang_get_miso(); + + /* PRG_RST. */ + ezo_prg_rst(); + + /* HW_CHECK. */ + /* FIXME: Check pulse? */ + /* FIXME: We just sent PRG_RST. Resend? */ + ezo_set_vcc(1); + ezo_set_par_oe(0); + /* Want a bitwise function instead of set_parallel_data? */ + ezo_bitbang_set_parallel_data(0x01); + programmer_delay(2); + tmp = ezo_bitbang_get_miso(); + if (!tmp) { + msg_perr("Hardware error! Aborting.\n"); + /* FIXME: Release I/O permissions. */ + return 1; + } + /* Want a bitwise function instead of set_parallel_data? */ + ezo_bitbang_set_parallel_data(0x00); + programmer_delay(2); + tmp = ezo_bitbang_get_miso(); + if (tmp) { + msg_perr("Hardware error! Aborting.\n"); + /* FIXME: Release I/O permissions. */ + return 1; + } + ezo_prg_rst(); + + /* FIXME: PRG_ID. */ + + /* 1 usec halfperiod delay for now. */ + if (bitbang_spi_init(&bitbang_spi_master_ezo, 1)) + return 1; + + buses_supported = CHIP_BUSTYPE_SPI; + spi_controller = SPI_CONTROLLER_EZO; + + return 0; +} + +int ezo_shutdown(void) +{ + /* FIXME: Add shutdown sequence. */ + /* FIXME: Release I/O perms. */ + return 0; +} + +#else +#error PCI port I/O access is not supported on this architecture yet. +#endif diff --git a/flashrom.8 b/flashrom.8 index 8e7c02b..11c86a6 100644 --- a/flashrom.8 +++ b/flashrom.8 @@ -190,6 +190,8 @@ .BR "* rayer_spi" " (for SPI flash ROMs attached to a RayeR parport \ based programmer)" .sp +.BR "* ezo" " (for flash ROMs attached to a Ezoflash parport based programmer)" +.sp Some programmers have optional or mandatory parameters which are described in detail in the .B PROGRAMMER SPECIFIC INFO @@ -396,6 +398,10 @@ .BR "rayer_spi " programmer No parameters defined yet. More information about the hardware is available at http://rayer.ic.cz/elektro/spipgm.htm +.TP +.BR "ezo " programmer +No parameters defined yet. More information about the hardware is available at +http:// .SH EXIT STATUS flashrom exits with 0 on success, 1 on most failures but with 2 if /dev/mem (/dev/xsvc on Solaris) can not be opened and with 3 if a call to mmap() fails. @@ -418,8 +424,8 @@ .BR gfxnvidia " and " drkaiser need PCI configuration space access and raw memory access. .sp -.B rayer_spi -needs raw I/O port access. +.BR rayer_spi " and " ezo +need raw I/O port access. .sp .B satasii needs PCI configuration space read access and raw memory access. diff --git a/flashrom.c b/flashrom.c index 45d086d..6b6c04b 100644 --- a/flashrom.c +++ b/flashrom.c @@ -49,7 +49,7 @@ * if more than one of them is selected. If only one is selected, it is clear * that the user wants that one to become the default. */ -#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_FT2232_SPI+CONFIG_SERPROG+CONFIG_BUSPIRATE_SPI+CONFIG_DEDIPROG+CONFIG_RAYER_SPI > 1 +#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_FT2232_SPI+CONFIG_SERPROG+CONFIG_BUSPIRATE_SPI+CONFIG_DEDIPROG+CONFIG_RAYER_SPI+CONFIG_EZO > 1 #error Please enable either CONFIG_DUMMY or CONFIG_INTERNAL or disable support for all programmers except one. #endif enum programmer programmer = @@ -90,6 +90,9 @@ #if CONFIG_RAYER_SPI == 1 PROGRAMMER_RAYER_SPI #endif +#if CONFIG_EZO == 1 + PROGRAMMER_EZO +#endif ; #endif @@ -414,6 +417,25 @@ }, #endif +#if CONFIG_EZO == 1 + { + .name = "ezo", + .init = ezo_init, + .shutdown = ezo_shutdown, + .map_flash_region = fallback_map, + .unmap_flash_region = fallback_unmap, + .chip_readb = noop_chip_readb, + .chip_readw = fallback_chip_readw, + .chip_readl = fallback_chip_readl, + .chip_readn = fallback_chip_readn, + .chip_writeb = noop_chip_writeb, + .chip_writew = fallback_chip_writew, + .chip_writel = fallback_chip_writel, + .chip_writen = fallback_chip_writen, + .delay = internal_delay, + }, +#endif + {}, /* This entry corresponds to PROGRAMMER_INVALID. */ }; diff --git a/programmer.h b/programmer.h index b3f99b4..ac8cf60 100644 --- a/programmer.h +++ b/programmer.h @@ -73,6 +73,9 @@ #if CONFIG_RAYER_SPI == 1 PROGRAMMER_RAYER_SPI, #endif +#if CONFIG_EZO == 1 + PROGRAMMER_EZO, +#endif PROGRAMMER_INVALID /* This must always be the last entry. */ }; @@ -115,6 +118,9 @@ BITBANG_SPI_MASTER_MCP, #endif #endif +#if CONFIG_EZO == 1 + BITBANG_SPI_MASTER_EZO, +#endif }; struct bitbang_spi_master { @@ -425,6 +431,12 @@ #endif #endif +/* ezo.c */ +#if CONFIG_EZO == 1 +int ezo_init(void); +int ezo_shutdown(void); +#endif + /* bitbang_spi.c */ int bitbang_spi_init(const struct bitbang_spi_master *master, int halfperiod); int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); @@ -494,6 +506,9 @@ #if CONFIG_RAYER_SPI == 1 SPI_CONTROLLER_RAYER, #endif +#if CONFIG_EZO == 1 + SPI_CONTROLLER_EZO, +#endif SPI_CONTROLLER_INVALID /* This must always be the last entry. */ }; extern const int spi_programmer_count; diff --git a/spi.c b/spi.c index ebacd60..3955429 100644 --- a/spi.c +++ b/spi.c @@ -136,6 +136,15 @@ }, #endif +#if CONFIG_EZO == 1 + { /* SPI_CONTROLLER_EZO */ + .command = bitbang_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = bitbang_spi_read, + .write_256 = bitbang_spi_write_256, + }, +#endif + {}, /* This entry corresponds to SPI_CONTROLLER_INVALID. */ }; -- To view, visit https://review.coreboot.org/23053 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idc4d98593076c80fd1d6ac4596940d1d7977343c Gerrit-Change-Number: 23053 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>

1 0

Change in flashrom[master]: SST 49LF0008A - Erase problem (and solution)
by Nico Huber (Code Review) Dec. 31, 2017

Dec. 31, 2017

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23052 ) Change subject: SST 49LF0008A - Erase problem (and solution) ...................................................................... Patch Set 1: Code-Review-2 Just for reference, aparently it was never tested. -- To view, visit https://review.coreboot.org/23052 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: Id309595cfd78be0a8635aa6de7e4a900482323ff Gerrit-Change-Number: 23052 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 31 Dec 2017 18:58:02 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: SST 49LF0008A - Erase problem (and solution)
by build bot (Jenkins) (Code Review) Dec. 31, 2017

Dec. 31, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23052 ) Change subject: SST 49LF0008A - Erase problem (and solution) ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom_gerrit/847/ : FAILURE https://qa.coreboot.org/job/flashrom-customrules/988/ : SUCCESS -- To view, visit https://review.coreboot.org/23052 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: Id309595cfd78be0a8635aa6de7e4a900482323ff Gerrit-Change-Number: 23052 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 31 Dec 2017 18:57:18 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: SST 49LF0008A - Erase problem (and solution)
by Nico Huber (Code Review) Dec. 31, 2017

Dec. 31, 2017

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/23052 Change subject: SST 49LF0008A - Erase problem (and solution) ...................................................................... SST 49LF0008A - Erase problem (and solution) Hi, thanks for your report. On 08.03.2010 14:41, Panino ColSalame wrote: > I'm playing with flashrom on an ASEM Industrial PC based on an Atom Z530 and the US15W (Poulsbo) System Controller Hub. > The BIOS flash is the SST 49LF0008A (Firmware Hub). > > After downloading the latest development code using git, flashrom was able to detect and read the flash, but the erase command failed. > Interesting. Such failures shouldn't happen, though. > At first I've tried to increase the delay that is used by the toggle functions in jedec.c, passing from the default 8 to 80 (just for testing). > At this point, the Erase command started to work properly. > Interesting. While the longer delay has the side effect of fixing this, having 80 ms delays per sector is pretty much unacceptable for devices with 1024 sectors or more. > SST manual reads: > The End-of-Write detection mode is incorporated into the FWH Read cycle. > The actual completion of the nonvolatile write is asynchronous with the system; therefore, either a Data# Polling or Toggle Bit read > may be simultaneous with the completion of the Write cycle. > If this occurs, the system may possibly get an erroneous result, i.e., valid data may appear to conflict with either DQ7 or DQ6. > In order to prevent spurious rejection, if an erroneous result occurs, the software routine should include a loop to read the accessed location an additional two (2) times. > If both reads are valid, then the device has completed the Write cycle, otherwise the rejection is valid. > Please note that the SST manual talks about false rejects (which are not a problem here) and doesn't talk about false accepts (which are a problem here). Since the toggle loop continues running as long as there is a reject, the additional two reads happen automatically with the current code. > So, I've set the delay for reading the toggle bit again to the default value of 8 and I've added an additional toggle bit check, in case the first one seems to be ok. > There are two possible reasons for a false accept (which the datasheet doesn't mention): 1. Foreign reads from the chip by other apps/hardware. Can happen due to ACPI, network card etc. If the number of foreign reads between subsequent toggle loop iterations is an odd number, the toggle loop will get a false accept. 2. The chip toggles too slowly. Happens on some Winbond chips, but usually only for erase. toggle_ready_jedec_slow() takes care of that. Your patch has the side effect of partially fixing case 1, but it violates the Winbond delay expectation while checking for the validity of an accept. Could you please try the patch below in verbose mode? It should issue one read less in the success case, still conform to all timing requirements, and fix false accept reason 1 with a high probability. "flashrom -EV" or "flashrom -wV" output would be highly appreciated. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Regards, Carl-Daniel Change-Id: Id309595cfd78be0a8635aa6de7e4a900482323ff --- M jedec.c 1 file changed, 8 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/23052/1 diff --git a/jedec.c b/jedec.c index af13876..2951603 100644 --- a/jedec.c +++ b/jedec.c @@ -51,7 +51,14 @@ programmer_delay(delay); tmp2 = chip_readb(flash, dst) & 0x40; if (tmp1 == tmp2) { - break; + /* Recheck to avoid spurious results. */ + programmer_delay(delay); + tmp1 = chip_readb(dst) & 0x40; + if (tmp1 == tmp2) + break; + msg_cdbg("Spurious toggle ready!\n"); + /* Avoid a false positive during next loop. */ + tmp2 = tmp1; } tmp1 = tmp2; } -- To view, visit https://review.coreboot.org/23052 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id309595cfd78be0a8635aa6de7e4a900482323ff Gerrit-Change-Number: 23052 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>

1 0

Change in flashrom[master]: Fix another SB600 SPI corner case
by Nico Huber (Code Review) Dec. 31, 2017

Dec. 31, 2017

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23051 ) Change subject: Fix another SB600 SPI corner case ...................................................................... Patch Set 1: Code-Review-1 Just saving this from patchwork, Carl-Daniel replied himself: > Turns out that the bug we were seeing is totally unrelated. Still, this > patch needs to be checked on real hardware with a logic analyzer. A > simple "looks good" ack won't do, sorry. -- To view, visit https://review.coreboot.org/23051 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: I72a3725449652773ca1d6df18ee76901d617b74c Gerrit-Change-Number: 23051 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 31 Dec 2017 18:37:38 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Fix another SB600 SPI corner case
by build bot (Jenkins) (Code Review) Dec. 31, 2017

Dec. 31, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23051 ) Change subject: Fix another SB600 SPI corner case ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/987/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/846/ : SUCCESS -- To view, visit https://review.coreboot.org/23051 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: I72a3725449652773ca1d6df18ee76901d617b74c Gerrit-Change-Number: 23051 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 31 Dec 2017 18:32:08 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Fix another SB600 SPI corner case
by Nico Huber (Code Review) Dec. 31, 2017

Dec. 31, 2017

Hello Carl-Daniel Hailfinger, I'd like you to do a code review. Please visit https://review.coreboot.org/23051 to review the following change. Change subject: Fix another SB600 SPI corner case ...................................................................... Fix another SB600 SPI corner case SB600 SPI has an off-by-one error during read, and that hardware issue is not documented anywhere yet. r661 tried to work around this, but the fix had unpleasant side effects. The new workaround only triggers if readcnt is nonzero, eliminating all possible interactions with write-only commands. Change-Id: I72a3725449652773ca1d6df18ee76901d617b74c Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> --- M sb600spi.c 1 file changed, 6 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/23051/1 diff --git a/sb600spi.c b/sb600spi.c index 68f8d8a..a5499ff 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -245,9 +245,13 @@ * an opcode and no additional data/address, the SPI controller will * read one byte too few from the chip. Basically, the last byte of * the chip response is discarded and will not end up in the FIFO. - * It is unclear if the CS# line is set high too early as well. + * It is unclear if the CS# line is set high too early as well, but + * it seems CS# line timing matches the bytecounts we specify, so there + * is a mismatch between FIFO content count and CS# timing. + * Only trigger this if readcnt is nonzero on the assumption that + * reading more doesn't hurt if we already read soemthing. */ - unsigned int readoffby1 = (writecnt > 0) ? 0 : 1; + unsigned int readoffby1 = (writecnt == 0 && readcnt > 0) ? 1 : 0; uint8_t readwrite = (readcnt + readoffby1) << 4 | (writecnt); mmio_writeb(readwrite, sb600_spibar + 1); -- To view, visit https://review.coreboot.org/23051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I72a3725449652773ca1d6df18ee76901d617b74c Gerrit-Change-Number: 23051 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>

1 0

Change in flashrom[staging]: buspirate_spi: Add support for variable serial speeds
by build bot (Jenkins) (Code Review) Dec. 31, 2017

Dec. 31, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23050 ) Change subject: buspirate_spi: Add support for variable serial speeds ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/986/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/845/ : SUCCESS -- To view, visit https://review.coreboot.org/23050 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I3706f17a94fdf056063f2ad4a5f0a219665cdcbf Gerrit-Change-Number: 23050 Gerrit-PatchSet: 2 Gerrit-Owner: Shawn Anastasio <shawnanastasio(a)yahoo.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 31 Dec 2017 08:19:18 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[staging]: buspirate_spi: Add support for variable serial speeds
by Shawn Anastasio (Code Review) Dec. 31, 2017

Dec. 31, 2017

Hello Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/23050 to look at the new patch set (#2). Change subject: buspirate_spi: Add support for variable serial speeds ...................................................................... buspirate_spi: Add support for variable serial speeds This patch introduces the ability to set the baud rate for communication between the host device and the Bus Pirate via the added 'serialspeed' programmer parameter. This is done in two parts. Firstly, the requested serial speed is looked up in a table to determine the appropriate clock divisor and the divisor is sent to the bus pirate. Then, the system's baud rate for the selected serial port is set using serial.c's 'serialport_config'. This function's prototype had to be added to programmer.h. In testing, using the 2M baud rate was able to significantly decrease flash times (down from 20+ minutes to less than 2 minutes for an 8MB flash). Change-Id: I3706f17a94fdf056063f2ad4a5f0a219665cdcbf Signed-off-by: Shawn Anastasio <shawnanastasio(a)yahoo.com> --- M buspirate_spi.c M programmer.h 2 files changed, 89 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/23050/2 -- To view, visit https://review.coreboot.org/23050 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3706f17a94fdf056063f2ad4a5f0a219665cdcbf Gerrit-Change-Number: 23050 Gerrit-PatchSet: 2 Gerrit-Owner: Shawn Anastasio <shawnanastasio(a)yahoo.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0