When flashrom JEDEC code sends the ID command to the chip, it expects to see IDs in the default flash location. However, sometimes the chip does not react to the ID command, either because it doesn't understand the command or because the command never reached it. One way to detect this is to compare ID output with flash chip contents for the same location. If they are identical, there is a high chance you're not actually seeing ID output. Warn the user in that case.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-real-jedec-id/jedec.c =================================================================== --- flashrom-real-jedec-id/jedec.c (Revision 470) +++ flashrom-real-jedec-id/jedec.c (Arbeitskopie) @@ -92,6 +92,7 @@ volatile uint8_t *bios = flash->virtual_memory; uint8_t id1, id2; uint32_t largeid1, largeid2; + uint32_t flashcontent1, flashcontent2;
/* Issue JEDEC Product ID Entry command */ chip_writeb(0xAA, bios + 0x5555); @@ -133,6 +134,26 @@ printf_debug("%s: id1 0x%02x, id2 0x%02x", __FUNCTION__, largeid1, largeid2); if (!oddparity(id1)) printf_debug(", id1 parity violation"); + + /* Read the product ID location again. We should now see normal flash contents. */ + flashcontent1 = chip_readb(bios); + flashcontent2 = chip_readb(bios + 0x01); + + /* Check if it is a continuation ID, this should be a while loop. */ + if (flashcontent1 == 0x7F) { + flashcontent1 <<= 8; + flashcontent1 |= chip_readb(bios + 0x100); + } + if (flashcontent2 == 0x7F) { + flashcontent2 <<= 8; + flashcontent2 |= chip_readb(bios + 0x101); + } + + if (largeid1 == flashcontent1) + printf_debug(", id1 is normal flash content"); + if (largeid2 == flashcontent2) + printf_debug(", id2 is normal flash content"); + printf_debug("\n"); if (largeid1 == flash->manufacture_id && largeid2 == flash->model_id) return 1;
On 07.05.2009 00:16, Carl-Daniel Hailfinger wrote:
When flashrom JEDEC code sends the ID command to the chip, it expects to see IDs in the default flash location. However, sometimes the chip does not react to the ID command, either because it doesn't understand the command or because the command never reached it. One way to detect this is to compare ID output with flash chip contents for the same location. If they are identical, there is a high chance you're not actually seeing ID output. Warn the user in that case.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Idwer Vollering tested the patch and it triggers each time the ID isn't a real ID. Thanks!
Regards, Carl-Daniel
On 07.05.2009 00:16, Carl-Daniel Hailfinger wrote:
When flashrom JEDEC code sends the ID command to the chip, it expects to see IDs in the default flash location. However, sometimes the chip does not react to the ID command, either because it doesn't understand the command or because the command never reached it. One way to detect this is to compare ID output with flash chip contents for the same location. If they are identical, there is a high chance you're not actually seeing ID output. Warn the user in that case.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Ping? This patch helps a lot when a chip is not recognized and we want to check if the probe responses are real IDs or just random flash chip contents.
This should probably be added to all probe functions, but probe_jedec is called for all sizes and thus flashrom will check this condition at least once per size, making sure we can cross-match the warning.
Regards, Carl-Daniel
Index: flashrom-real-jedec-id/jedec.c
--- flashrom-real-jedec-id/jedec.c (Revision 470) +++ flashrom-real-jedec-id/jedec.c (Arbeitskopie) @@ -92,6 +92,7 @@ volatile uint8_t *bios = flash->virtual_memory; uint8_t id1, id2; uint32_t largeid1, largeid2;
uint32_t flashcontent1, flashcontent2;
/* Issue JEDEC Product ID Entry command */ chip_writeb(0xAA, bios + 0x5555);
@@ -133,6 +134,26 @@ printf_debug("%s: id1 0x%02x, id2 0x%02x", __FUNCTION__, largeid1, largeid2); if (!oddparity(id1)) printf_debug(", id1 parity violation");
- /* Read the product ID location again. We should now see normal flash contents. */
- flashcontent1 = chip_readb(bios);
- flashcontent2 = chip_readb(bios + 0x01);
- /* Check if it is a continuation ID, this should be a while loop. */
- if (flashcontent1 == 0x7F) {
flashcontent1 <<= 8;flashcontent1 |= chip_readb(bios + 0x100);- }
- if (flashcontent2 == 0x7F) {
flashcontent2 <<= 8;flashcontent2 |= chip_readb(bios + 0x101);- }
- if (largeid1 == flashcontent1)
printf_debug(", id1 is normal flash content");- if (largeid2 == flashcontent2)
printf_debug(", id2 is normal flash content");- printf_debug("\n"); if (largeid1 == flash->manufacture_id && largeid2 == flash->model_id) return 1;
Carl-Daniel Hailfinger wrote:
One way to detect this is to compare ID output with flash chip contents for the same location. If they are identical, there is a high chance you're not actually seeing ID output. Warn the user in that case.
There are already several examples on the list. This patch is much needed.
Minor opinion below and it is Acked-by: FENG Yu Ning fengyuning1984@gmail.com
Index: flashrom-real-jedec-id/jedec.c =================================================================== --- flashrom-real-jedec-id/jedec.c (Revision 470) +++ flashrom-real-jedec-id/jedec.c (Arbeitskopie) @@ -92,6 +92,7 @@ volatile uint8_t *bios = flash->virtual_memory; uint8_t id1, id2; uint32_t largeid1, largeid2; + uint32_t flashcontent1, flashcontent2;
I think b1 and b2 are enough for the names. Whenever the naming is not confusing, shorter names make me feel more comfortable. A taste matter, though.
yu ning
On 11.05.2009 16:08, FENG Yu Ning wrote:
Carl-Daniel Hailfinger wrote:
One way to detect this is to compare ID output with flash chip contents for the same location. If they are identical, there is a high chance you're not actually seeing ID output. Warn the user in that case.
There are already several examples on the list. This patch is much needed.
Minor opinion below and it is Acked-by: FENG Yu Ning fengyuning1984@gmail.com
Thanks, committed in r494.
Index: flashrom-real-jedec-id/jedec.c
--- flashrom-real-jedec-id/jedec.c (Revision 470) +++ flashrom-real-jedec-id/jedec.c (Arbeitskopie) @@ -92,6 +92,7 @@ volatile uint8_t *bios = flash->virtual_memory; uint8_t id1, id2; uint32_t largeid1, largeid2;
- uint32_t flashcontent1, flashcontent2;
I think b1 and b2 are enough for the names. Whenever the naming is not confusing, shorter names make me feel more comfortable. A taste matter, though.
I first had short names, but I felt the long names would be more readable since we have 3 different variables per ID and telling them apart is easier when their names make their purpose painfully obvious. I don't have a really strong opinion about this topic, though.
Please feel free to change variable naming to b1 and b2. I will probably even ack it.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
FENG Yu Ning