We've been using flashrom quite successfully for months to program an SST49LF160C chip, but today we came across one flash image that flashrom kept failing to verify after writing it.
When we dumped the contents of the flash after the verify, we saw that one byte at offset 0x1e5555 was getting changed from 0x4c to 0x08.
I managed to narrow down the corruption to the chip probe process, when probe_28sf040() is called followed by probe_jedec(). The comments in probe_28sf040() indicate that it's trying to restore the value at offset 0 after writing probe commands to that location. But the chip interprets the write to offset 0 as the beginning of a command, and the reads and writes in the subsequent call to probe_jedec() somehow forms a write-byte command if some magic values are present in the flash (is it a coincidence that the part ID is 0x4c?).
All of this sounds ridiculously unlikely, and without understanding the details of the flash protocols it's hard to know whether I'm misdiagnosing the problem. The attached patch removes the seemingly unnecessary restoring of the value at location 0 in probe_28sf040(), and indeed fixes the problem.
--Ed
On Fri, Aug 10, 2007 at 06:58:06PM -0700, Ed Swierk wrote:
All of this sounds ridiculously unlikely, and without understanding the details of the flash protocols it's hard to know whether I'm misdiagnosing the problem.
No, your diagnosis is correct.
1. flashrom needs to do certain writes in order to read the device ID from an SST28SF040 chip. (Although they don't seem to exist. Datasheet?)
2. When the ID fails noone knows what effect those certain writes had on the system. :\
This isn't safe. Tricky.
The attached patch removes the seemingly unnecessary restoring of the value at location 0 in probe_28sf040(), and indeed fixes the problem.
We can't say if that restore really is neccessary or not. Erring on the side of caution would be good, but the damage may already have been done by those certain writes that are needed to do the ID.
Maybe we should just not probe for 28sf040 by default? It's a really old chip.
//Peter
* Peter Stuge peter@stuge.se [070811 04:41]:
The attached patch removes the seemingly unnecessary restoring of the value at location 0 in probe_28sf040(), and indeed fixes the problem.
We can't say if that restore really is neccessary or not. Erring on the side of caution would be good, but the damage may already have been done by those certain writes that are needed to do the ID.
I think it is not necessary nor safe for any flash media in that area. Of course, if we fear that we might find non-flash media in the 4G-size area, we need to think about restoring.
Maybe we should just not probe for 28sf040 by default? It's a really old chip.
Or remove the restore? The code only restores for the case that there is no 28sf040 anyways...?
On 8/12/07, Stefan Reinauer stepan@coresystems.de wrote:
Or remove the restore? The code only restores for the case that there is no 28sf040 anyways...?
Makes sense to me :-)
--Ed
Signed-off-by: Ed Swierk eswierk@arastra.com
--Ed
On Sun, Aug 12, 2007 at 08:47:42PM -0700, Ed Swierk wrote:
Signed-off-by: Ed Swierk eswierk@arastra.com
Acked, r2744
On Sun, Aug 12, 2007 at 05:18:49PM +0200, Stefan Reinauer wrote:
Of course, if we fear that we might find non-flash media in the 4G-size area, we need to think about restoring.
Yeah. When we want to port flashrom to flash also on other archs, it will need a redesign. I don't care for now.
Maybe we should just not probe for 28sf040 by default? It's a really old chip.
Or remove the restore?
I like that. No other probe_ functions restore like that.
The code only restores for the case that there is no 28sf040 anyways...?
And that's the problem - because typically there is none, so that byte will always get written. Most chips don't care, but apparently some do.
//Peter
* Ed Swierk eswierk@arastra.com [070811 03:58]:
All of this sounds ridiculously unlikely, and without understanding the details of the flash protocols it's hard to know whether I'm misdiagnosing the problem. The attached patch removes the seemingly unnecessary restoring of the value at location 0 in probe_28sf040(), and indeed fixes the problem.
I think you observed an interesting point. I wonder whether anything in that area will ever require that restoration. I am pretty close to say it's just bogus and dangerous and should be dropped, just as you do in your patch.
Index: LinuxBIOSv2-2571/util/flashrom/sst28sf040.c
--- LinuxBIOSv2-2571.orig/util/flashrom/sst28sf040.c +++ LinuxBIOSv2-2571/util/flashrom/sst28sf040.c @@ -106,10 +106,7 @@ static __inline__ int write_sector_28sf0 int probe_28sf040(struct flashchip *flash) { volatile uint8_t *bios = flash->virt_addr;
- uint8_t id1, id2, tmp;
- /* save the value at the beginning of the Flash */
- tmp = *bios;
uint8_t id1, id2;
*bios = RESET; myusec_delay(10);
@@ -127,8 +124,6 @@ int probe_28sf040(struct flashchip *flas if (id1 == flash->manufacture_id && id2 == flash->model_id) return 1;
- /* if there is no SST28SF040, restore the original value */
- *bios = tmp; return 0;
}
-
Ed Swierk -
Peter Stuge -
Stefan Reinauer