Quite a few probe functions in flashrom are copies of probe_jedec with additional lock bit printing or other glue. Make them call probe_jedec instead. Use the correct reset sequence for 82802AB.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-call_probe_jedec/w39v040c.c =================================================================== --- flashrom-call_probe_jedec/w39v040c.c (Revision 674) +++ flashrom-call_probe_jedec/w39v040c.c (Arbeitskopie) @@ -23,8 +23,12 @@ int probe_w39v040c(struct flashchip *flash) { chipaddr bios = flash->virtual_memory; - uint8_t id1, id2, lock; + int result = probe_jedec(flash); + uint8_t lock;
+ if (!result) + return result; + chip_writeb(0xAA, bios + 0x5555); programmer_delay(10); chip_writeb(0x55, bios + 0x2AAA); @@ -32,8 +36,6 @@ chip_writeb(0x90, bios + 0x5555); programmer_delay(10);
- id1 = chip_readb(bios); - id2 = chip_readb(bios + 1); lock = chip_readb(bios + 0xfff2);
chip_writeb(0xAA, bios + 0x5555); @@ -43,17 +45,9 @@ chip_writeb(0xF0, bios + 0x5555); programmer_delay(40);
- printf_debug("%s: id1 0x%02x, id2 0x%02x", __func__, id1, id2); - if (!oddparity(id1)) - printf_debug(", id1 parity violation"); - printf_debug("\n"); - if (flash->manufacture_id == id1 && flash->model_id == id2) { - printf("%s: Boot block #TBL is %slocked, rest of chip #WP is %slocked.\n", - __func__, lock & 0x4 ? "" : "un", lock & 0x8 ? "" : "un"); - return 1; - } - - return 0; + printf("%s: Boot block #TBL is %slocked, rest of chip #WP is %slocked.\n", + __func__, lock & 0x4 ? "" : "un", lock & 0x8 ? "" : "un"); + return 1; }
int erase_w39v040c(struct flashchip *flash) Index: flashrom-call_probe_jedec/stm50flw0x0x.c =================================================================== --- flashrom-call_probe_jedec/stm50flw0x0x.c (Revision 674) +++ flashrom-call_probe_jedec/stm50flw0x0x.c (Arbeitskopie) @@ -42,50 +42,11 @@
int probe_stm50flw0x0x(struct flashchip *flash) { - chipaddr bios = flash->virtual_memory; - uint8_t id1, id2; - uint32_t largeid1, largeid2; + int result = probe_jedec(flash);
- /* Issue JEDEC Product ID Entry command */ - chip_writeb(0xAA, bios + 0x5555); - programmer_delay(10); - chip_writeb(0x55, bios + 0x2AAA); - programmer_delay(10); - chip_writeb(0x90, bios + 0x5555); - programmer_delay(40); + if (!result) + return result;
- /* Read product ID */ - id1 = chip_readb(bios); - id2 = chip_readb(bios + 0x01); - largeid1 = id1; - largeid2 = id2; - - /* Check if it is a continuation ID, this should be a while loop. */ - if (id1 == 0x7F) { - largeid1 <<= 8; - id1 = chip_readb(bios + 0x100); - largeid1 |= id1; - } - if (id2 == 0x7F) { - largeid2 <<= 8; - id2 = chip_readb(bios + 0x101); - largeid2 |= id2; - } - - /* Issue JEDEC Product ID Exit command */ - chip_writeb(0xAA, bios + 0x5555); - programmer_delay(10); - chip_writeb(0x55, bios + 0x2AAA); - programmer_delay(10); - chip_writeb(0xF0, bios + 0x5555); - programmer_delay(40); - - printf_debug("%s: id1 0x%02x, id2 0x%02x\n", __FUNCTION__, largeid1, - largeid2); - - if (largeid1 != flash->manufacture_id || largeid2 != flash->model_id) - return 0; - map_flash_registers(flash);
return 1; Index: flashrom-call_probe_jedec/w39v080fa.c =================================================================== --- flashrom-call_probe_jedec/w39v080fa.c (Revision 674) +++ flashrom-call_probe_jedec/w39v080fa.c (Arbeitskopie) @@ -22,30 +22,11 @@
int probe_winbond_fwhub(struct flashchip *flash) { - chipaddr bios = flash->virtual_memory; - uint8_t id1, id2; + int result = probe_jedec(flash);
- /* Product Identification Entry */ - chip_writeb(0xAA, bios + 0x5555); - chip_writeb(0x55, bios + 0x2AAA); - chip_writeb(0x90, bios + 0x5555); - programmer_delay(10); + if (!result) + return result;
- /* Read product ID */ - id1 = chip_readb(bios); - id2 = chip_readb(bios + 0x01); - - /* Product Identifixation Exit */ - chip_writeb(0xAA, bios + 0x5555); - chip_writeb(0x55, bios + 0x2AAA); - chip_writeb(0xF0, bios + 0x5555); - programmer_delay(10); - - printf_debug("%s: id1 0x%x, id2 0x%x\n", __FUNCTION__, id1, id2); - - if (id1 != flash->manufacture_id || id2 != flash->model_id) - return 0; - map_flash_registers(flash);
return 1; Index: flashrom-call_probe_jedec/82802ab.c =================================================================== --- flashrom-call_probe_jedec/82802ab.c (Revision 674) +++ flashrom-call_probe_jedec/82802ab.c (Arbeitskopie) @@ -47,14 +47,11 @@ chipaddr bios = flash->virtual_memory; uint8_t id1, id2;
-#if 0 - chip_writeb(0xAA, bios + 0x5555); - chip_writeb(0x55, bios + 0x2AAA); - chip_writeb(0x90, bios + 0x5555); -#endif - - chip_writeb(0xff, bios); + /* Reset to get a clean state */ + chip_writeb(0xFF, bios); programmer_delay(10); + + /* Enter ID mode */ chip_writeb(0x90, bios); programmer_delay(10);
@@ -62,9 +59,7 @@ id2 = chip_readb(bios + 0x01);
/* Leave ID mode */ - chip_writeb(0xAA, bios + 0x5555); - chip_writeb(0x55, bios + 0x2AAA); - chip_writeb(0xF0, bios + 0x5555); + chip_writeb(0xFF, bios);
programmer_delay(10);
@@ -81,7 +76,6 @@ uint8_t wait_82802ab(chipaddr bios) { uint8_t status; - uint8_t id1, id2;
chip_writeb(0x70, bios); if ((chip_readb(bios) & 0x80) == 0) { // it's busy @@ -90,19 +84,9 @@
status = chip_readb(bios);
- // put another command to get out of status register mode + /* Reset to get a clean state */ + chip_writeb(0xFF, bios);
- chip_writeb(0x90, bios); - programmer_delay(10); - - id1 = chip_readb(bios); - id2 = chip_readb(bios + 0x01); - - // this is needed to jam it out of "read id" mode - chip_writeb(0xAA, bios + 0x5555); - chip_writeb(0x55, bios + 0x2AAA); - chip_writeb(0xF0, bios + 0x5555); - return status; }
On 8/10/09 3:36 PM, Carl-Daniel Hailfinger wrote:
Use the correct reset sequence for 82802AB.
Were you able to test this? It seems the chip was tested as PREW before, so it seems the driver might have been ok as is..?
Stefan
On 10.08.2009 15:44, Stefan Reinauer wrote:
On 8/10/09 3:36 PM, Carl-Daniel Hailfinger wrote:
Use the correct reset sequence for 82802AB.
Were you able to test this? It seems the chip was tested as PREW before, so it seems the driver might have been ok as is..?
Well, the reset sequence before ID reading was correct, so ID always worked. But the reset sequence after ID reading was a copy-paste leftover from probe_jedec and didn't have any effect. I dug up flash_and_burn from the freebios-v1 tree and found out that 82802ab.c was indeed a copy of jedec.c with lots of experimental unannotated #if 0 and #if 1. About the wait_82802ab change: Before the patch, wait_82802ab entered read status mode, switched to ID mode, then tried an incorrect and unsupported JEDEC command to exit ID mode. Nobody ever saw that this failed because all subsequent function calls had the correct reset sequence at the beginning. With the patch, wait_82802ab enters read status mode, then switches back to read mode with the official reset command.
I hope the explanation above clears it up. If you want, I can insert the text above into the changelog upon commit.
Regards, Carl-Daniel
On 10.08.2009 16:13, Carl-Daniel Hailfinger wrote:
On 10.08.2009 15:44, Stefan Reinauer wrote:
On 8/10/09 3:36 PM, Carl-Daniel Hailfinger wrote:
Use the correct reset sequence for 82802AB.
Were you able to test this? It seems the chip was tested as PREW before, so it seems the driver might have been ok as is..?
Well, the reset sequence before ID reading was correct, so ID always worked. But the reset sequence after ID reading was a copy-paste leftover from probe_jedec and didn't have any effect. I dug up flash_and_burn from the freebios-v1 tree and found out that 82802ab.c was indeed a copy of jedec.c with lots of experimental unannotated #if 0 and #if 1. About the wait_82802ab change: Before the patch, wait_82802ab entered read status mode, switched to ID mode, then tried an incorrect and unsupported JEDEC command to exit ID mode. Nobody ever saw that this failed because all subsequent function calls had the correct reset sequence at the beginning. With the patch, wait_82802ab enters read status mode, then switches back to read mode with the official reset command.
Acked-by: Stefan Reinauer stepan@coresystems.de Thanks, committed in r717.
Regards, Carl-Daniel