[flashrom-gerrit] Change in flashrom[master]: SST 49LF0008A - Erase problem (and solution)

[Nico Huber (Code Review)]

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

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