I don't know if this is a bug, or if there are somehow two completely different versions of the ST M25P40 SPI chip.
I just tried to burn such a chip with flashrom, but it wouldn't detect it at all, so I checked the datasheet for it, and it turned out that the probe was completely different from what one would think, reading flashrom's source.
It seems that the M25P40 doesn't use the RDID command to send its ID, but rather a "RES" (Read Electronic Signature) command instead, which returns just one byte. I added support for it, and then it worked perfectly for me.
I'm attaching my patch to fix the problem. I would imagine that the other M25Pxx chips would be affected by the same problem, but since I can neither verify nor test what IDs they would have, they are excluded from my patch.
Fredrik Tolf
On Sat, Mar 29, 2008 at 04:42:24AM +0100, Fredrik Tolf wrote:
I don't know if this is a bug, or if there are somehow two completely different versions of the ST M25P40 SPI chip.
I just tried to burn such a chip with flashrom, but it wouldn't detect it at all, so I checked the datasheet for it, and it turned out that the probe was completely different from what one would think, reading flashrom's source.
It seems that the M25P40 doesn't use the RDID command to send its ID, but rather a "RES" (Read Electronic Signature) command instead, which returns just one byte. I added support for it, and then it worked perfectly for me.
I'm attaching my patch to fix the problem. I would imagine that the other M25Pxx chips would be affected by the same problem, but since I can neither verify nor test what IDs they would have, they are excluded from my patch.
I don't know too much about SPI, I'll let Carl-Daniel review this patch ;) But please resend with a Signed-off-by line, otherwise we cannot commit the patch, see
http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure
Thanks, Uwe.
On Sat, 2008-03-29 at 17:28 +0100, Uwe Hermann wrote:
On Sat, Mar 29, 2008 at 04:42:24AM +0100, Fredrik Tolf wrote: [...] But please resend with a Signed-off-by line, otherwise we cannot commit the patch
OK, I'll do that.
Signed-off-by: Fredrik Tolf fredrik@dolda2000.com
On Sat, 2008-03-29 at 17:28 +0100, Uwe Hermann wrote:
But please resend with a Signed-off-by line, otherwise we cannot commit the patch, see
http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure
By the way, the link in that Wiki page (which points to http://osdlab.org/newsroom/press_releases/2004/2004_05_24_dco.html) is broken.
Fredrik Tolf
On 29.03.2008 04:42, Fredrik Tolf wrote:
I don't know if this is a bug, or if there are somehow two completely different versions of the ST M25P40 SPI chip.
The data sheet says: "The Read Identification (RDID) instruction is available in products with Process Technology code X only." That means the previous code was correct for only a subset of devices. Unfortunately, that limitation is not mentioned in the ST M25P application note.
I just tried to burn such a chip with flashrom, but it wouldn't detect it at all, so I checked the datasheet for it, and it turned out that the probe was completely different from what one would think, reading flashrom's source.
It seems that the M25P40 doesn't use the RDID command to send its ID, but rather a "RES" (Read Electronic Signature) command instead, which returns just one byte. I added support for it, and then it worked perfectly for me.
See above.
I'm attaching my patch to fix the problem. I would imagine that the other M25Pxx chips would be affected by the same problem, but since I can neither verify nor test what IDs they would have, they are excluded from my patch.
Thanks for investigating and coding this. However, I have to veto the patch in its current form. I know at least 4 flash chips with different sizes from different manufacturers which have the same RES code. Can you post the RDID output for your device? Maybe we can figure out a way to identify it safely with the help of RDID and RES and possibly READ.
Regards, Carl-Daniel
On Sat, 2008-03-29 at 22:17 +0100, Carl-Daniel Hailfinger wrote:
I'm attaching my patch to fix the problem. I would imagine that the other M25Pxx chips would be affected by the same problem, but since I can neither verify nor test what IDs they would have, they are excluded from my patch.
Thanks for investigating and coding this. However, I have to veto the patch in its current form. I know at least 4 flash chips with different sizes from different manufacturers which have the same RES code. Can you post the RDID output for your device? Maybe we can figure out a way to identify it safely with the help of RDID and RES and possibly READ.
The RDID output was just FF FF FF, so I'm pretty sure it just doesn't recognize the command at all. How do you suppose the chip's identity could be deduced from a READ command?
Would it perhaps not be wise to create a command-line switch to enable detection for chips like this, that might not be possible to auto-identify safely? The flashchip struct could be gifted with a pre-initialized boolean that indicates if a chip should be auto-identified without being explicitly specified.
Fredrik Tolf
On Sat, 2008-03-29 at 22:58 +0100, Fredrik Tolf wrote:
Would it perhaps not be wise to create a command-line switch to enable detection for chips like this, that might not be possible to auto-identify safely? The flashchip struct could be gifted with a pre-initialized boolean that indicates if a chip should be auto-identified without being explicitly specified.
Oh, just to be clear: I intended to do the actual work for that. I just wanted to see if you think it's a good idea.
Fredrik Tolf
On 29.03.2008 22:58, Fredrik Tolf wrote:
On Sat, 2008-03-29 at 22:17 +0100, Carl-Daniel Hailfinger wrote:
I'm attaching my patch to fix the problem. I would imagine that the other M25Pxx chips would be affected by the same problem, but since I can neither verify nor test what IDs they would have, they are excluded from my patch.
Thanks for investigating and coding this. However, I have to veto the patch in its current form. I know at least 4 flash chips with different sizes from different manufacturers which have the same RES code. Can you post the RDID output for your device? Maybe we can figure out a way to identify it safely with the help of RDID and RES and possibly READ.
The RDID output was just FF FF FF, so I'm pretty sure it just doesn't recognize the command at all.
Good. That means we can probably use RDID in combination with RES to identify this special ST M25P40 variant.
How do you suppose the chip's identity could be deduced from a READ command?
You can read the contents of the chip and deduce a minimum size by analyzing repetitions. While it is possible to underestimate the size of the chip (in case 2^n identical images are behind each other), I can't see a way to overestimate chip size.
Would it perhaps not be wise to create a command-line switch to enable detection for chips like this, that might not be possible to auto-identify safely? The flashchip struct could be gifted with a pre-initialized boolean that indicates if a chip should be auto-identified without being explicitly specified.
Yes, a command line switch would probably work, but I'm currently working on a redesign of the SPI part of flashrom, so I'd like to delay that a bit. Details will be announced after Tuesday.
Some parts of your patch (except the modification of the ST_M25P40 #define and the flashchips.c changes) are definitely worth merging instantly, though. Could you split these parts from the patch and resend them with a Signed-off-by statement?
Regards, Carl-Daniel
Hi Fredrik,
On 29.03.2008 23:17, Carl-Daniel Hailfinger wrote:
On 29.03.2008 22:58, Fredrik Tolf wrote:
On Sat, 2008-03-29 at 22:17 +0100, Carl-Daniel Hailfinger wrote:
I'm attaching my patch to fix the problem. I would imagine that the other M25Pxx chips would be affected by the same problem, but since I can neither verify nor test what IDs they would have, they are excluded from my patch.
Thanks for investigating and coding this. However, I have to veto the patch in its current form. I know at least 4 flash chips with different sizes from different manufacturers which have the same RES code. Can you post the RDID output for your device? Maybe we can figure out a way to identify it safely with the help of RDID and RES and possibly READ.
The RDID output was just FF FF FF, so I'm pretty sure it just doesn't recognize the command at all.
Good. That means we can probably use RDID in combination with RES to identify this special ST M25P40 variant.
Can you test the patch below? It should have a lower error probability and work fine for you. If you are OK with my changes to your patch, please re-add your Signed-off-by line.
It would be interesting to try the REMS (0x90) instruction as well and match on that one if possible, but this patch is good enough for now.
Regards, Carl-Daniel
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-res/flash.h =================================================================== --- flashrom-res/flash.h (Revision 3296) +++ flashrom-res/flash.h (Arbeitskopie) @@ -249,6 +249,7 @@ #define ST_M25P10A 0x2011 #define ST_M25P20 0x2012 #define ST_M25P40 0x2013 +#define ST_M25P40_RES 0x12 #define ST_M25P80 0x2014 #define ST_M25P16 0x2015 #define ST_M25P32 0x2016 @@ -347,7 +348,8 @@ extern char *lb_part, *lb_vendor;
/* spi.c */ -int probe_spi(struct flashchip *flash); +int probe_spi_rdid(struct flashchip *flash); +int probe_spi_res(struct flashchip *flash); int it87xx_probe_spi_flash(const char *name); int spi_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); void spi_write_enable(); Index: flashrom-res/flashchips.c =================================================================== --- flashrom-res/flashchips.c (Revision 3296) +++ flashrom-res/flashchips.c (Arbeitskopie) @@ -49,25 +49,25 @@ {"Fujitsu", "MBM29F400TC", FUJITSU_ID, MBM29F400TC_STRANGE, 512, 64 * 1024, TEST_UNTESTED, probe_m29f400bt, erase_m29f400bt, write_coreboot_m29f400bt}, {"Intel", "82802AB", INTEL_ID, 173, 512, 64 * 1024, TEST_UNTESTED, probe_82802ab, erase_82802ab, write_82802ab}, {"Intel", "82802AC", INTEL_ID, 172, 1024, 64 * 1024, TEST_UNTESTED, probe_82802ab, erase_82802ab, write_82802ab}, - {"Macronix", "MX25L3205", MX_ID, MX_25L3205, 4096, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"Macronix", "MX25L4005", MX_ID, MX_25L4005, 512, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"Macronix", "MX25L8005", MX_ID, MX_25L8005, 1024, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"Macronix", "MX25L3205", MX_ID, MX_25L3205, 4096, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"Macronix", "MX25L4005", MX_ID, MX_25L4005, 512, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"Macronix", "MX25L8005", MX_ID, MX_25L8005, 1024, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, {"Macronix", "MX29F002", MX_ID, MX_29F002, 256, 64 * 1024, TEST_UNTESTED, probe_29f002, erase_29f002, write_29f002}, #ifndef DISABLE_DOC {"M-Systems", "MD-2802", MSYSTEMS_ID, MSYSTEMS_MD2802, 8, 8 * 1024, TEST_UNTESTED, probe_md2802, erase_md2802, write_md2802, read_md2802}, #endif - {"PMC", "Pm25LV010", PMC_ID, PMC_25LV010, 128, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"PMC", "Pm25LV016B", PMC_ID, PMC_25LV016B, 2048, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"PMC", "Pm25LV020", PMC_ID, PMC_25LV020, 256, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"PMC", "Pm25LV040", PMC_ID, PMC_25LV040, 512, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"PMC", "Pm25LV080B", PMC_ID, PMC_25LV080B, 1024, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"PMC", "Pm25LV512", PMC_ID, PMC_25LV512, 64, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"PMC", "Pm25LV010", PMC_ID, PMC_25LV010, 128, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"PMC", "Pm25LV016B", PMC_ID, PMC_25LV016B, 2048, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"PMC", "Pm25LV020", PMC_ID, PMC_25LV020, 256, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"PMC", "Pm25LV040", PMC_ID, PMC_25LV040, 512, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"PMC", "Pm25LV080B", PMC_ID, PMC_25LV080B, 1024, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"PMC", "Pm25LV512", PMC_ID, PMC_25LV512, 64, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, {"PMC", "Pm49FL002", PMC_ID_NOPREFIX,PMC_49FL002, 256, 16 * 1024, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_49fl004}, {"PMC", "Pm49FL004", PMC_ID_NOPREFIX,PMC_49FL004, 512, 64 * 1024, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_49fl004}, {"Sharp", "LHF00L04", SHARP_ID, SHARP_LHF00L04, 1024, 64 * 1024, TEST_UNTESTED, probe_lhf00l04, erase_lhf00l04, write_lhf00l04}, - {"Spansion", "S25FL016A", SPANSION_ID, SPANSION_S25FL016A, 2048, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"SST", "SST25VF016B", SST_ID, SST_25VF016B, 2048, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"SST", "SST25VF040B", SST_ID, SST_25VF040B, 512, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"Spansion", "S25FL016A", SPANSION_ID, SPANSION_S25FL016A, 2048, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"SST", "SST25VF016B", SST_ID, SST_25VF016B, 2048, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"SST", "SST25VF040B", SST_ID, SST_25VF040B, 512, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, {"SST", "SST28SF040A", SST_ID, SST_28SF040, 512, 256, TEST_UNTESTED, probe_28sf040, erase_28sf040, write_28sf040}, {"SST", "SST29EE020A", SST_ID, SST_29EE020A, 256, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec}, {"SST", "SST39SF010A", SST_ID, SST_39SF010, 128, 4096, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_39sf020}, @@ -87,15 +87,16 @@ {"SST", "SST49LF040B", SST_ID, SST_49LF040B, 512, 64 * 1024, TEST_UNTESTED, probe_sst_fwhub, erase_sst_fwhub, write_sst_fwhub}, {"SST", "SST49LF080A", SST_ID, SST_49LF080A, 1024, 4096, TEST_UNTESTED, probe_jedec, erase_49lf040, write_49lf040}, {"SST", "SST49LF160C", SST_ID, SST_49LF160C, 2048, 4 * 1024, TEST_UNTESTED, probe_49lfxxxc, erase_49lfxxxc, write_49lfxxxc}, - {"ST", "M25P05-A", ST_ID, ST_M25P05A, 64, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"ST", "M25P10-A", ST_ID, ST_M25P10A, 128, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"ST", "M25P128", ST_ID, ST_M25P128, 16384, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"ST", "M25P16", ST_ID, ST_M25P16, 2048, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"ST", "M25P20", ST_ID, ST_M25P20, 256, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"ST", "M25P32", ST_ID, ST_M25P32, 4096, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"ST", "M25P40", ST_ID, ST_M25P40, 512, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"ST", "M25P64", ST_ID, ST_M25P64, 8192, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"ST", "M25P80", ST_ID, ST_M25P80, 1024, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P05-A", ST_ID, ST_M25P05A, 64, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P10-A", ST_ID, ST_M25P10A, 128, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P128", ST_ID, ST_M25P128, 16384, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P16", ST_ID, ST_M25P16, 2048, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P20", ST_ID, ST_M25P20, 256, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P32", ST_ID, ST_M25P32, 4096, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P40", ST_ID, ST_M25P40, 512, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P40(old)", ST_ID, ST_M25P40_RES, 512, 256, TEST_UNTESTED, probe_spi_res, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P64", ST_ID, ST_M25P64, 8192, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"ST", "M25P80", ST_ID, ST_M25P80, 1024, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, {"ST", "M29F002B", ST_ID, ST_M29F002B, 256, 64 * 1024, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec}, {"ST", "M29F002T/NT", ST_ID, ST_M29F002T, 256, 64 * 1024, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec}, {"ST", "M29F040B", ST_ID, ST_M29F040B, 512, 64 * 1024, TEST_UNTESTED, probe_29f040b, erase_29f040b, write_29f040b}, @@ -114,10 +115,10 @@ {"SyncMOS", "S29C51001T", SYNCMOS_ID, S29C51001T, 128, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_49f002}, {"SyncMOS", "S29C51002T", SYNCMOS_ID, S29C51002T, 256, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_49f002}, {"SyncMOS", "S29C51004T", SYNCMOS_ID, S29C51004T, 512, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_49f002}, - {"Winbond", "W25x10", WINBOND_NEX_ID, W_25X10, 128, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"Winbond", "W25x20", WINBOND_NEX_ID, W_25X20, 256, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"Winbond", "W25x40", WINBOND_NEX_ID, W_25X40, 512, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, - {"Winbond", "W25x80", WINBOND_NEX_ID, W_25X80, 1024, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"Winbond", "W25x10", WINBOND_NEX_ID, W_25X10, 128, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"Winbond", "W25x20", WINBOND_NEX_ID, W_25X20, 256, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"Winbond", "W25x40", WINBOND_NEX_ID, W_25X40, 512, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, + {"Winbond", "W25x80", WINBOND_NEX_ID, W_25X80, 1024, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read}, {"Winbond", "W29C011", WINBOND_ID, W_29C011, 128, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec}, {"Winbond", "W29C020C", WINBOND_ID, W_29C020C, 256, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec}, {"Winbond", "W29C040P", WINBOND_ID, W_29C040P, 512, 256, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec}, @@ -132,11 +133,11 @@ {"Winbond", "W39V080FA", WINBOND_ID, W_39V080FA, 1024, 64*1024, TEST_UNTESTED, probe_winbond_fwhub, erase_winbond_fwhub, write_winbond_fwhub}, {"Winbond", "W39V080FA (dual mode)",WINBOND_ID, W_39V080FA_DM, 512, 64*1024, TEST_UNTESTED, probe_winbond_fwhub, erase_winbond_fwhub, write_winbond_fwhub},
- {"EON", "unknown SPI chip", EON_ID_NOPREFIX,GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi, NULL, NULL}, - {"Macronix", "unknown SPI chip", MX_ID, GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi, NULL, NULL}, - {"PMC", "unknown SPI chip", PMC_ID, GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi, NULL, NULL}, - {"SST", "unknown SPI chip", SST_ID, GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi, NULL, NULL}, - {"ST", "unknown SPI chip", ST_ID, GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi, NULL, NULL}, + {"EON", "unknown SPI chip", EON_ID_NOPREFIX,GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi_rdid, NULL, NULL}, + {"Macronix", "unknown SPI chip", MX_ID, GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi_rdid, NULL, NULL}, + {"PMC", "unknown SPI chip", PMC_ID, GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi_rdid, NULL, NULL}, + {"SST", "unknown SPI chip", SST_ID, GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi_rdid, NULL, NULL}, + {"ST", "unknown SPI chip", ST_ID, GENERIC_DEVICE_ID, 0, 0, TEST_UNTESTED, probe_spi_rdid, NULL, NULL},
{NULL,} }; Index: flashrom-res/spi.c =================================================================== --- flashrom-res/spi.c (Revision 3296) +++ flashrom-res/spi.c (Arbeitskopie) @@ -36,6 +36,11 @@ #define JEDEC_RDID_OUTSIZE 0x01 #define JEDEC_RDID_INSIZE 0x03
+/* Read Electronic Signature */ +#define JEDEC_RES 0xab +#define JEDEC_RES_OUTSIZE 0x04 +#define JEDEC_RES_INSIZE 0x01 + /* Write Enable */ #define JEDEC_WREN {0x06} #define JEDEC_WREN_OUTSIZE 0x01 @@ -259,6 +264,16 @@ return 0; }
+static int spi_res(unsigned char *readarr) +{ + const unsigned char cmd[] = {JEDEC_RES, 0, 0, 0}; + + if (spi_command(JEDEC_RES_OUTSIZE, JEDEC_RES_INSIZE, cmd, readarr)) + return 1; + printf_debug("RES returned %02x.\n", readarr[0]); + return 0; +} + void spi_write_enable() { const unsigned char cmd[] = JEDEC_WREN; @@ -275,7 +290,7 @@ spi_command(JEDEC_WRDI_OUTSIZE, JEDEC_WRDI_INSIZE, cmd, NULL); }
-int probe_spi(struct flashchip *flash) +int probe_spi_rdid(struct flashchip *flash) { unsigned char readarr[3]; uint32_t manuf_id; @@ -308,6 +323,35 @@ return 0; }
+int probe_spi_res(struct flashchip *flash) +{ + unsigned char readarr[3]; + uint32_t model_id; + if (!spi_rdid(readarr)) { + /* Check if RDID returns 0xff 0xff 0xff, then we use RES. */ + if ((readarr[0] != 0xff) || (readarr[1] != 0xff) || + (readarr[2] != 0xff)) + return 0; + } else { + /* We couldn't issue RDID, it's pointless to try RES. */ + return 0; + } + if (!spi_res(readarr)) { + model_id = readarr[0]; + printf_debug("%s: id 0x%x\n", __FUNCTION__, model_id); + if (model_id == flash->model_id) { + /* Print the status register to tell the + * user about possible write protection. + */ + spi_prettyprint_status_register(flash); + + return 1; + } + } + + return 0; +} + uint8_t spi_read_status_register() { const unsigned char cmd[] = JEDEC_RDSR;
On Sun, 2008-05-11 at 15:32 +0200, Carl-Daniel Hailfinger wrote:
Can you test the patch below? It should have a lower error probability and work fine for you. If you are OK with my changes to your patch, please re-add your Signed-off-by line.
Yes, the flashrom code with that patch seems to detect my chip and read its contents quite well. However, the same code does not detect the original chip on the board (an MX25L4005) satisfactory, and it seems to be a conflict with the SPI code. Here's the output:
Calibrating delay loop... OK. No coreboot table found. Found chipset "NVIDIA MCP55", enabling flash write... OK. Found board "GIGABYTE GA-M57SLI-S4": enabling flash write... Serial flash segment 0xfffe0000-0xffffffff enabled Serial flash segment 0x000e0000-0x000fffff enabled Serial flash segment 0xffee0000-0xffefffff disabled Serial flash segment 0xfff80000-0xfffeffff enabled LPC write to serial flash enabled serial flash pin 29 OK. MX25L4005 found at physical address 0xfff80000. unknown SPI chip found at physical address 0x0. Multiple flash chips were detected: MX25L4005 unknown SPI chip Please specify which chip to use with the -c <chipname> option.
Fredrik Tolf
Peter? This is a side effect of r3291, the "multiple flash chip" patch. All of our SPI chips where a generic per-vendor fallback detection exists (that's all of them right now) will exhibit this problem.
On 11.05.2008 18:01, Fredrik Tolf wrote:
On Sun, 2008-05-11 at 15:32 +0200, Carl-Daniel Hailfinger wrote:
Can you test the patch below? It should have a lower error probability and work fine for you. If you are OK with my changes to your patch, please re-add your Signed-off-by line.
Yes, the flashrom code with that patch seems to detect my chip and read its contents quite well. However, the same code does not detect the original chip on the board (an MX25L4005) satisfactory, and it seems to be a conflict with the SPI code. Here's the output:
Calibrating delay loop... OK. No coreboot table found. Found chipset "NVIDIA MCP55", enabling flash write... OK. Found board "GIGABYTE GA-M57SLI-S4": enabling flash write... Serial flash segment 0xfffe0000-0xffffffff enabled Serial flash segment 0x000e0000-0x000fffff enabled Serial flash segment 0xffee0000-0xffefffff disabled Serial flash segment 0xfff80000-0xfffeffff enabled LPC write to serial flash enabled serial flash pin 29 OK. MX25L4005 found at physical address 0xfff80000. unknown SPI chip found at physical address 0x0. Multiple flash chips were detected: MX25L4005 unknown SPI chip Please specify which chip to use with the -c <chipname> option.
Fredrik, you should be able to work around this by calling flashrom -c MX25L4005 The patch I sent does not have anything to do with the MX25L4005 problem you're seeing because that problem was introduced in r3291. Can you signoff this patch since it works for you?
Regards, Carl-Daniel
On Sun, May 11, 2008 at 06:17:48PM +0200, Carl-Daniel Hailfinger wrote:
On 11.05.2008 18:01, Fredrik Tolf wrote:
Yes, the flashrom code with that patch seems to detect my chip and read its contents quite well. However, the same code does not detect the original chip on the board??? (an MX25L4005) satisfactory, and it seems to be a conflict with the SPI code.
Peter? This is a side effect of r3291, the "multiple flash chip" patch.
Yes, 3291 exposes this.
All of our SPI chips where a generic per-vendor fallback detection exists (that's all of them right now) will exhibit this problem.
I had reservations about "unknown SPI chip" and this is why.
This is also why I don't like fake flash chip entries for forcing reads from undetectable chips.
Both simply do not accurately describe or model what is actually going on and just end up confusing users and code.
It is ridiculous to claim that an "unknown chip" has been detected. If a chip had been positively detected it would not be unknown. This is behavior/language I could expect from commercial OS vendors.
I think we have to stop saying this and instead flashrom needs infrastructure and commands that can correctly model and handle SPI situation where the memory bus master is no longer transparent.
Fredrik, you should be able to work around this by calling flashrom -c MX25L4005 The patch I sent does not have anything to do with the MX25L4005 problem you're seeing because that problem was introduced in r3291. Can you signoff this patch since it works for you?
Please do sign off the patch if it works for you Fredrik. There are issues in flashrom, but as Carl-Daniel writes they are not closely related and I think the proposed RES change can go in for now.
//Peter
On 12.05.2008 05:16, Peter Stuge wrote:
On Sun, May 11, 2008 at 06:17:48PM +0200, Carl-Daniel Hailfinger wrote:
On 11.05.2008 18:01, Fredrik Tolf wrote:
Yes, the flashrom code with that patch seems to detect my chip and read its contents quite well. However, the same code does not detect the original chip on the board??? (an MX25L4005) satisfactory, and it seems to be a conflict with the SPI code.
Peter? This is a side effect of r3291, the "multiple flash chip" patch.
Yes, 3291 exposes this.
All of our SPI chips where a generic per-vendor fallback detection exists (that's all of them right now) will exhibit this problem.
I had reservations about "unknown SPI chip" and this is why.
This is also why I don't like fake flash chip entries for forcing reads from undetectable chips.
Both simply do not accurately describe or model what is actually going on and just end up confusing users and code.
Feel free to propose an alternative that works and doesn't uglify the flashrom architecture. Be warned that this is difficult.
It is ridiculous to claim that an "unknown chip" has been detected. If a chip had been positively detected it would not be unknown. This is behavior/language I could expect from commercial OS vendors.
That was a mismerge. The correct message would have been "unknown Macronix SPI flash chip" and that's a big improvement over "no chip found".
I think we have to stop saying this and instead flashrom needs infrastructure and commands that can correctly model and handle SPI situation where the memory bus master is no longer transparent.
Which infrastructure are you missing?
Fredrik, you should be able to work around this by calling flashrom -c MX25L4005 The patch I sent does not have anything to do with the MX25L4005 problem you're seeing because that problem was introduced in r3291. Can you signoff this patch since it works for you?
Please do sign off the patch if it works for you Fredrik. There are issues in flashrom, but as Carl-Daniel writes they are not closely related and I think the proposed RES change can go in for now.
Indeed. I'm also looking for an Ack from someone who is not Fredrik or me. The patch is bitrotting in my tree and all SPI restructuring conflicts with it, so I have to merge manually every time.
Regards, Carl-Daniel
Fredrik, you should be able to work around this by calling flashrom -c MX25L4005
Yes, it does. I even tried that before, but forgot to mention it.
The patch I sent does not have anything to do with the MX25L4005 problem you're seeing because that problem was introduced in r3291. Can you signoff this patch since it works for you?
I see. I thought it was related, since the patch was for SPI in some generality. Sure thing, then. I'm not sure what to do in this particular case, though -- am I supposed to re-quote the entire patch with your sign as well? If not, I'm just adding it here:
Signed-off-by: Fredrik Tolf fredrik@dolda2000.com
(OT: Though, I have to question the utility of these signatures. I've questioned it silently for myself for the Linux kernel team as well, but at least they use Git which seals the signature along with the entire development history with SHA-1, while Coreboot's signatures seem very much looser than that.)
On Sun, May 11, 2008 at 03:32:46PM +0200, Carl-Daniel Hailfinger wrote:
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Needs a commit message of course.
- {"ST", "M25P40", ST_ID, ST_M25P40, 512, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
- {"ST", "M25P40", ST_ID, ST_M25P40, 512, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
- {"ST", "M25P40(old)", ST_ID, ST_M25P40_RES, 512, 256, TEST_UNTESTED, probe_spi_res, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
Hmm. probe_spi_res() calls spi_rdid() first. I don't like duplicating chips? If multiple checks are needed for one chip I think they should be in code?
//Peter
On 15.05.2008 04:22, Peter Stuge wrote:
On Sun, May 11, 2008 at 03:32:46PM +0200, Carl-Daniel Hailfinger wrote:
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Needs a commit message of course.
Yes. How about Add support for the JEDEC RES (Read Electronic Signature and Resume from Powerdown) SPI command to flashrom to identify older SPI chips which can't handle JEDEC RDID. Since RES gives a one-byte identifier which is shared among many different vendors and even different sizes, we want to match RES as a last resort if RDID returns 0xff 0xff 0xff.
- {"ST", "M25P40", ST_ID, ST_M25P40, 512, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
- {"ST", "M25P40", ST_ID, ST_M25P40, 512, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
- {"ST", "M25P40(old)", ST_ID, ST_M25P40_RES, 512, 256, TEST_UNTESTED, probe_spi_res, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
Hmm. probe_spi_res() calls spi_rdid() first. I don't like duplicating chips? If multiple checks are needed for one chip I think they should be in code?
To be honest, the "M25P40(old)" entry should more look like "STM25P40/SST25something/AT26something/dozens_of_other_chips". For more details, see the changelog.
Regards, Carl-Daniel
On Thu, May 15, 2008 at 04:37:18AM +0200, Carl-Daniel Hailfinger wrote:
Needs a commit message of course.
Add support for the JEDEC RES (Read Electronic Signature and Resume from Powerdown) SPI command to flashrom to identify older SPI chips which can't handle JEDEC RDID. Since RES gives a one-byte identifier which is shared among many different vendors and even different sizes, we want to match RES as a last resort if RDID returns 0xff 0xff 0xff.
Great!
- {"ST", "M25P40", ST_ID, ST_M25P40, 512, 256, TEST_UNTESTED, probe_spi, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
- {"ST", "M25P40", ST_ID, ST_M25P40, 512, 256, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
- {"ST", "M25P40(old)", ST_ID, ST_M25P40_RES, 512, 256, TEST_UNTESTED, probe_spi_res, spi_chip_erase_c7, spi_chip_write, spi_chip_read},
Hmm. probe_spi_res() calls spi_rdid() first. I don't like duplicating chips? If multiple checks are needed for one chip I think they should be in code?
To be honest, the "M25P40(old)" entry should more look like "STM25P40/SST25something/AT26something/dozens_of_other_chips". For more details, see the changelog.
With name change per IRC discussion:
Acked-by: Peter Stuge peter@stuge.se
On 15.05.2008 05:06, Peter Stuge wrote:
On Thu, May 15, 2008 at 04:37:18AM +0200, Carl-Daniel Hailfinger wrote:
To be honest, the "M25P40(old)" entry should more look like "STM25P40/SST25something/AT26something/dozens_of_other_chips". For more details, see the changelog.
With name change per IRC discussion:
Acked-by: Peter Stuge peter@stuge.se
Thanks, r3320. A big merge pain is now gone on my side.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Fredrik Tolf -
Peter Stuge -
Uwe Hermann