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>
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
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>
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
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>
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>