[U-Boot-Users] [PATCH] CFI driver AMD Command Set Top boot geometry reversal, etc. [Updated]
This patch (replaces patch submitted via DNX#2006110942000016):
* Adds support for AMD command set Top Boot flash geometry reversal * Adds support for reading JEDEC Manufacturer ID and Device ID * Adds support for displaying command set, manufacturer id and device ids (flinfo) * Makes flinfo output to be consistent when CFG_FLASH_EMPTY_INFO defined * Removes outdated change history (refer to git log instead)
Signed-off-by: Tolunay Orkun listmember@orkun.us
---
drivers/cfi_flash.c | 207 +++++++++++++++++++++++++++++++++++++-------------- include/flash.h | 8 ++ 2 files changed, 159 insertions(+), 56 deletions(-)
Hi Tolunay,
On Friday 10 November 2006 00:46, Tolunay Orkun wrote:
This patch (replaces patch submitted via DNX#2006110942000016):
- Adds support for AMD command set Top Boot flash geometry reversal
- Adds support for reading JEDEC Manufacturer ID and Device ID
- Adds support for displaying command set, manufacturer id and device ids (flinfo)
- Makes flinfo output to be consistent when CFG_FLASH_EMPTY_INFO defined
- Removes outdated change history (refer to git log instead)
I tied you patch one a board with a ST M29W160ET FLASH, that couldn't be handled by the "old" CFI driver, because of the geometry reversal. I had hoped your patch would have solved this issue. Unfortunately not:
=> fli
Bank # 1: CFI conformant FLASH (16 x 16) Size: 2 MB in 35 Sectors AMD Standard command set, Manufacturer ID: 0x05, Device ID: 0x56 Erase timeout: 8192 ms, write timeout: 1 ms
Sector Start Addresses: FFE00000 FFE04000 FFE06000 FFE08000 FFE10000 FFE20000 E FFE30000 E FFE40000 E FFE50000 E FFE60000 E FFE70000 E FFE80000 E FFE90000 E FFEA0000 E FFEB0000 E FFEC0000 E FFED0000 E FFEE0000 E FFEF0000 E FFF00000 E FFF10000 E FFF20000 E FFF30000 E FFF40000 E FFF50000 E FFF60000 E FFF70000 E FFF80000 E FFF90000 E FFFA0000 RO FFFB0000 RO FFFC0000 RO FFFD0000 RO FFFE0000 RO FFFF0000 RO
As you may notice, even the ID's are not correct (0020 and 22c4 are correct) and the geometry is not correct (bottom instead of top).
I didn't look into this deeply and since you have the insight right now (;-)), perhaps you have an idea what is going wrong here.
Thanks a lot.
Best regards, Stefan
Stefan,
Stefan Roese wrote:
Hi Tolunay,
On Friday 10 November 2006 00:46, Tolunay Orkun wrote:
This patch (replaces patch submitted via DNX#2006110942000016):
- Adds support for AMD command set Top Boot flash geometry reversal
- Adds support for reading JEDEC Manufacturer ID and Device ID
- Adds support for displaying command set, manufacturer id and device ids (flinfo)
- Makes flinfo output to be consistent when CFG_FLASH_EMPTY_INFO defined
- Removes outdated change history (refer to git log instead)
I tied you patch one a board with a ST M29W160ET FLASH, that couldn't be handled by the "old" CFI driver, because of the geometry reversal. I had hoped your patch would have solved this issue. Unfortunately not:
=> fli
Bank # 1: CFI conformant FLASH (16 x 16) Size: 2 MB in 35 Sectors AMD Standard command set, Manufacturer ID: 0x05, Device ID: 0x56 Erase timeout: 8192 ms, write timeout: 1 ms
Sector Start Addresses: FFE00000 FFE04000 FFE06000 FFE08000 FFE10000 FFE20000 E FFE30000 E FFE40000 E FFE50000 E FFE60000 E FFE70000 E FFE80000 E FFE90000 E FFEA0000 E FFEB0000 E FFEC0000 E FFED0000 E FFEE0000 E FFEF0000 E FFF00000 E FFF10000 E FFF20000 E FFF30000 E FFF40000 E FFF50000 E FFF60000 E FFF70000 E FFF80000 E FFF90000 E FFFA0000 RO FFFB0000 RO FFFC0000 RO FFFD0000 RO FFFE0000 RO FFFF0000 RO
As you may notice, even the ID's are not correct (0020 and 22c4 are correct) and the geometry is not correct (bottom instead of top).
The DeviceID detection seems still broken for AMD at this moment. I might be reading the actual data or possibly wrong offsets. The Manufacturer ID should be 0x20 and Device Id should be displayed as 0xC4 (I might add code to get that 0x22 but if a word/byte device is in byte mode 0x22 part is not available)
I need your help. Can you compile cfi_flash.c with DEBUG and capture the output as U-Boot boots. Also, the output of following commands please.
mw.w ffe00555 00aa mw.w ffe002aa 0055 mw.w ffe00555 0090 md ffe00000 mw.w ffe00000 00f0
Even if detection is right we might fail sometimes with CFI version 1.0 which your ST part is according to datasheet. But in your case if we can identify the device id as 0xC4 it would be designated as top boot (because msb is set) and it would work. The bottom boot version of the device would have device id 0x49 [By the way device ids have odd parity according to MTD code]
We might need to add specific device IDs to the driver as exceptions for some CFI 1.0 cases. Linux driver did not have a good solution for this either. But for your case as soon as device id algorithm is fixed it should work fine.
Stefan Roese wrote:
As you may notice, even the ID's are not correct (0020 and 22c4 are correct) and the geometry is not correct (bottom instead of top).
It looks like the problem with the IDs (I have them too) is that flash_read_jedec_ids() is broken. After sending the commands, the function just reads the regular data instead of the command reply.
I'm reading the CFI spec, and I see this:
"The software must write a 90h to the first location in the device. If the device returns a Manufacturer’s ID and Component ID, the flash device may be accessed as it has been in the past, based on the Manufacturer and Component ID. If the device does not return a Manufacturer and Component ID, then the device is not a flash memory and other routines are necessary to determine what type of device is installed."
The address that flash_read_jedec_ids() uses is fe000000, which is definitely the first byte of flash, so I don't know how it can claim that it isn't. It gets even more screwy. According to the flowchart in appendix A1, after writing 90h to fe000000, you're supposed to read back a 90h. Well, I get a 4h, which is what's normally there. I presume these reads and writes only work if the memory hasn't been erased? Because then, if I write anything to fe000000, I better get back the same value.
But isn't it possible to write to unerased flash anyway? Erasing just changes all the zeros to ones, and writing can only change a 1 to a 0, so if I write a 90h to a flash location, I should get back, 0, 90h, 80h, or 10h, right?
The flowchart says that if you don't get back 90h, you're supposed to restore the previous value. But how can you restore it without erasing the entire sector?
Is it possible that my AMD flash chips just don't support the JEDEC interface at all, and only CFI? Does the even make sense?
Timur Tabi wrote:
Stefan Roese wrote:
As you may notice, even the ID's are not correct (0020 and 22c4 are correct) and the geometry is not correct (bottom instead of top).
It looks like the problem with the IDs (I have them too) is that flash_read_jedec_ids() is broken. After sending the commands, the function just reads the regular data instead of the command reply.
Well, I think I fixed it.
In flash_read_jedec_ids(), make this change:
- flash_write_cmd(info, 0, 0, FLASH_CMD_READ_ID); + flash_write_cmd(info, 0, AMD_ADDR_START, FLASH_CMD_READ_ID);
I got the value of AMD_ADDR_START from the MX29LV640BT/BB reference manual, which says that the third bus cycle should be a write of 90h to 555 or AAA, depending on the width.
Now when I run flinfo, I get this:
Bank # 1: CFI conformant FLASH (16 x 16) Size: 8 MB in 135 Sectors AMD Standard command set, Manufacturer ID: 0xC2, Device ID: 0xC9 Erase timeout: 16384 ms, write timeout: 1 ms
If this fix is real, then it means that flash_read_jedec_ids() never worked for any AMD part.
Timur Tabi wrote:
Timur Tabi wrote:
Stefan Roese wrote:
As you may notice, even the ID's are not correct (0020 and 22c4 are correct) and the geometry is not correct (bottom instead of top).
It looks like the problem with the IDs (I have them too) is that flash_read_jedec_ids() is broken. After sending the commands, the function just reads the regular data instead of the command reply.
Well, I think I fixed it.
Great :)
In flash_read_jedec_ids(), make this change:
flash_write_cmd(info, 0, 0, FLASH_CMD_READ_ID);
flash_write_cmd(info, 0, AMD_ADDR_START,FLASH_CMD_READ_ID);
I think this change is what was missing which I overlooked. Intel does not need a specific address to write the command nor unlock sequence.
I got the value of AMD_ADDR_START from the MX29LV640BT/BB reference manual, which says that the third bus cycle should be a write of 90h to 555 or AAA, depending on the width.
The documentation on this matter is a bit scattered. Bits and pieces are here and there. It was educational to me as well.
Now when I run flinfo, I get this:
Bank # 1: CFI conformant FLASH (16 x 16) Size: 8 MB in 135 Sectors AMD Standard command set, Manufacturer ID: 0xC2, Device ID: 0xC9 Erase timeout: 16384 ms, write timeout: 1 ms
If this fix is real, then it means that flash_read_jedec_ids() never worked for any AMD part.
There was no flash_read_jedec_ids() before I sent the last patch. I wrote it from scratch and the bug there belongs to me :) Since, I did not have AMD style flash myself was not able to test that path and that is why I was asking you to collect debug data . It is hard to prepare patches for an issue that you do not have in house :) Anyway, I guess we will not need the debug data now.
I will update the patch tonight and resend it yet again.
Tolunay
Hi Tolunay,
On Sunday 12 November 2006 05:42, Tolunay Orkun wrote:
In flash_read_jedec_ids(), make this change:
flash_write_cmd(info, 0, 0, FLASH_CMD_READ_ID);
flash_write_cmd(info, 0, AMD_ADDR_START,FLASH_CMD_READ_ID);
I think this change is what was missing which I overlooked. Intel does not need a specific address to write the command nor unlock sequence.
And that's the reason I don't like those Intel devices. I got some devices accidentally screwed in a project once.
Now when I run flinfo, I get this:
Bank # 1: CFI conformant FLASH (16 x 16) Size: 8 MB in 135 Sectors AMD Standard command set, Manufacturer ID: 0xC2, Device ID: 0xC9 Erase timeout: 16384 ms, write timeout: 1 ms
And I get:
=> fli
Bank # 1: CFI conformant FLASH (16 x 16) Size: 2 MB in 35 Sectors AMD Standard command set, Manufacturer ID: 0x20, Device ID: 0xC4 Erase timeout: 8192 ms, write timeout: 1 ms
Sector Start Addresses: FFE00000 FFE10000 FFE20000 E FFE30000 E FFE40000 E FFE50000 E FFE60000 E FFE70000 E FFE80000 E FFE90000 E FFEA0000 E FFEB0000 E FFEC0000 E FFED0000 E FFEE0000 E FFEF0000 E FFF00000 E FFF10000 E FFF20000 E FFF30000 E FFF40000 E FFF50000 E FFF60000 E FFF70000 E FFF80000 E FFF90000 E FFFA0000 RO FFFB0000 RO FFFC0000 RO FFFD0000 RO FFFE0000 RO FFFF0000 RO FFFF8000 E FFFFA000 E FFFFC000
So FLASH ID's are now read back correctly.
There was no flash_read_jedec_ids() before I sent the last patch. I wrote it from scratch and the bug there belongs to me :)
;-)
Since, I did not have AMD style flash myself was not able to test that path and that is why I was asking you to collect debug data . It is hard to prepare patches for an issue that you do not have in house :)
As usual the rule applies: "never tested, never running". Thanks a lot for even trying to fix a patch for this without having the hardware. :)
I will update the patch tonight and resend it yet again.
Not necessary. I'll forward this patch with the fix to Wolfgang. Thanks again.
And when it's included we can continue to extend the code with additional features, like fixing this ST FLASH geometry reversal (or others) by identifying it's device ID.
Best regards, Stefan
Stefan Roese wrote:
flash_write_cmd(info, 0, 0, FLASH_CMD_READ_ID);
flash_write_cmd(info, 0, AMD_ADDR_START,FLASH_CMD_READ_ID);
I think this change is what was missing which I overlooked. Intel does not need a specific address to write the command nor unlock sequence.
And that's the reason I don't like those Intel devices. I got some devices accidentally screwed in a project once.
I hear you. I was not talking about merits.
So FLASH ID's are now read back correctly.
Great. Your flash device should validate CFI = 1.0 code path for geometry reversal detection as well.
I will update the patch tonight and resend it yet again.
Not necessary. I'll forward this patch with the fix to Wolfgang. Thanks again.
I did it anyway [DNX#2006111242000019]. Plus a few more minor changes: I've added my copyright line. Added a few more comments and removed some other change history comments included with previous copyright lines. Copyrights are still there as it should be.
I prefer if you can use this patch if it is not too late.
And when it's included we can continue to extend the code with additional features, like fixing this ST FLASH geometry reversal (or others) by identifying it's device ID.
It looks the sector addresses you included (below) is now that of top boot. Is it not working for you? When I enabled DEBUG the flash driver would have trouble with writes even for Intel (possibily clearing the status register prematurely). Make sure you test with DEBUG disabled for cp command.
Sector Start Addresses: FFE00000 FFE10000 FFE20000 E FFE30000 E FFE40000 E FFE50000 E FFE60000 E FFE70000 E FFE80000 E FFE90000 E FFEA0000 E FFEB0000 E FFEC0000 E FFED0000 E FFEE0000 E FFEF0000 E FFF00000 E FFF10000 E FFF20000 E FFF30000 E FFF40000 E FFF50000 E FFF60000 E FFF70000 E FFF80000 E FFF90000 E FFFA0000 RO FFFB0000 RO FFFC0000 RO FFFD0000 RO FFFE0000 RO FFFF0000 RO FFFF8000 E FFFFA000 E FFFFC000
I think should probably re-org the code sometime to be more table driven. We can avoid testing for vendor everywhere and make code much easier to read and extend. If a vendor is sticking to AMD style chips we could also have option to exclude the Intel support (or vice versa) completely to save some space that way.
Best regards, Tolunay
Hi Tolunay,
On Sunday 12 November 2006 23:04, Tolunay Orkun wrote:
I did it anyway [DNX#2006111242000019]. Plus a few more minor changes: I've added my copyright line. Added a few more comments and removed some other change history comments included with previous copyright lines. Copyrights are still there as it should be.
I prefer if you can use this patch if it is not too late.
No problem. The updated patch is on it's way...
And when it's included we can continue to extend the code with additional features, like fixing this ST FLASH geometry reversal (or others) by identifying it's device ID.
It looks the sector addresses you included (below) is now that of top boot. Is it not working for you?
Yes, you're right of course. I didn't notice it. Sorry for the noise. It's working now (also tested with the new patch). Thanks again.
Best regards, Stefan
Tolunay Orkun wrote:
There was no flash_read_jedec_ids() before I sent the last patch.
Ah, that function wasn't in your first patch, and for some reason, I didn't think to check your most recent patch.
participants (3)
-
Stefan Roese -
Timur Tabi -
Tolunay Orkun