[U-Boot-Users] Suggestion on flash init

Hi,
After reading many board/*/flash.c and writing customer board driver for one of my customer, I think the current flash interface can be improved.
I suggest adding a subdir $(TOPDIR)/include/flash/ and putting flash chip information here. When you use some kinds of flash chips, just include relevant header files.
If we go further, many information in $(TOPDIR)/include/flash.h can be put into these header files.
And, even flash operation (non-CFI) sample code can be put into $(TOPDIR)/contrib/flash/.
The following is an example of AMD Spansion S29AL016M chip.
--snip-- #ifndef _FLASH_S29AL016M_H_ #define _FLASH_S29AL016M_H_
#define FLASH_S29AL016M_T_SECTORS 35 #define FLASH_S29AL016M_B_SECTORS 35
#define FLASH_S29AL016M_T_SIZE 0x00200000 #define FLASH_S29AL016M_B_SIZE 0x00200000
int FLASH_S29AL016M_T_BYTE_SA[] = { 0x000000, 0x010000, 0x020000, 0x030000, 0x040000, 0x050000, 0x060000, 0x070000, 0x080000, 0x090000, 0x0A0000, 0x0B0000, 0x0C0000, 0x0D0000, 0x0E0000, 0x0F0000, 0x100000, 0x110000, 0x120000, 0x130000, 0x140000, 0x150000, 0x160000, 0x170000, 0x180000, 0x190000, 0x1A0000, 0x1B0000, 0x1C0000, 0x1D0000, 0x1E0000, 0x1F0000, 0x1F8000, 0x1FA000, 0x1C0000 };
int FLASH_S29AL016M_T_WORD_SA[] = { 0x000000, 0x008000, 0x010000, 0x018000, 0x020000, 0x028000, 0x030000, 0x038000, 0x040000, 0x048000, 0x050000, 0x058000, 0x060000, 0x068000, 0x070000, 0x078000, 0x080000, 0x088000, 0x090000, 0x098000, 0x0A0000, 0x0A8000, 0x0B0000, 0x0B8000, 0x0C0000, 0x0C8000, 0x0D0000, 0x0D8000, 0x0E0000, 0x0E8000, 0x0F0000, 0x0F8000, 0x0FC000, 0x0FD000, 0x0FE000 };
int FLASH_S29AL016M_B_BYTE_SA[] = { 0x000000, 0x004000, 0x006000, 0x008000, 0x010000, 0x020000, 0x030000, 0x040000, 0x050000, 0x060000, 0x070000, 0x080000, 0x090000, 0x0A0000, 0x0B0000, 0x0C0000, 0x0D0000, 0x0E0000, 0x0F0000, 0x100000, 0x110000, 0x120000, 0x130000, 0x140000, 0x150000, 0x160000, 0x170000, 0x180000, 0x190000, 0x1A0000, 0x1B0000, 0x1C0000, 0x1D0000, 0x1E0000, 0x1F0000 };
int FLASH_S29AL016M_B_WORD_SA[] = { 0x000000, 0x002000, 0x003000, 0x004000, 0x008000, 0x010000, 0x018000, 0x020000, 0x028000, 0x030000, 0x038000, 0x040000, 0x048000, 0x050000, 0x058000, 0x060000, 0x068000, 0x070000, 0x078000, 0x080000, 0x088000, 0x090000, 0x098000, 0x0A0000, 0x0A8000, 0x0B0000, 0x0B8000, 0x0C0000, 0x0C8000, 0x0D0000, 0x0D8000, 0x0E0000, 0x0E8000, 0x0F0000, 0x0F8000 };
#endif --snip--
In flash_get_size() --snip-- switch (device) { case BYTEME(AMD_ID_S29AL016M_T): info->flash_id += FLASH_S29AL016M_T; info->sector_count = FLASH_S29AL016M_T_SECTORS; info->size = FLASH_S29AL016M_T_SIZE; break;
case BYTEME(AMD_ID_S29AL016M_B): info->flash_id += FLASH_S29AL016M_B; info->sector_count = FLASH_S29AL016M_B_SECTORS; info->size = FLASH_S29AL016M_B_SIZE; break; --snip--
In flash_get_offset() --snip-- switch (info->flash_id & FLASH_TYPEMASK) { case FLASH_S29AL016M_T: for (i = 0; i < info->sector_count; i++) { info->start[i] = base + FLASH_S29AL016M_T_BYTE_SA[i]; } break; case FLASH_S29AL016M_B: for (i = 0; i < info->sector_count; i++) { info->start[i] = base + FLASH_S29AL016M_B_BYTE_SA[i]; } break; --snip--

In message 20051225120433.F3F6.LARK@linux.net.cn you wrote:
After reading many board/*/flash.c and writing customer board driver for one of my customer, I think the current flash interface can be improved.
Did you have a look at the cfi_flash driver? This is supposed to replace the custom board/*/flash.c in most cses (i. e. whenever memory footprint requirements are not too limiting).
I suggest adding a subdir $(TOPDIR)/include/flash/ and putting flash chip information here. When you use some kinds of flash chips, just include relevant header files.
I think we should use the cfi_driver instead.
And BTW - a comment on the suggested code:
If we go further, many information in $(TOPDIR)/include/flash.h can be put into these header files.
...
The following is an example of AMD Spansion S29AL016M chip.
...
int FLASH_S29AL016M_T_BYTE_SA[] = { 0x000000, 0x010000, 0x020000, 0x030000, 0x040000, 0x050000, 0x060000, 0x070000,
...
All-capital-names should be reserved for preprocessor variables. It is not acceptable to use these for other variables or function names. It is also not a good idea to have such initializations in a header file which might get included more than once.
Best regards,
Wolfgang Denk

Hi Wolfgang Denk,
On Sun, 25 Dec 2005 11:19:20 +0100, Wolfgang Denk wd@denx.de wrote:
In message 20051225120433.F3F6.LARK@linux.net.cn you wrote:
After reading many board/*/flash.c and writing customer board driver for one of my customer, I think the current flash interface can be improved.
Did you have a look at the cfi_flash driver? This is supposed to replace the custom board/*/flash.c in most cses (i. e. whenever memory footprint requirements are not too limiting).
I have tested cfi_flash.c for a while. Unfortuanately, it fails for Spansion S29AL016M. The chip can be recognized, chip information is all correct, but the erase and write routines fail.
The symptom is erase and write routine complete and seem to succeed, but content remains unchanged.
The datasheet I download from Spansion (S29AL016M_00_A5_E.pdf) says:
― CFI (Common Flash Interface) compliant: allows host system to identify and accommodate multiple flash devices
Carefully read cfi_flash.c and datasheet, I think S29AL016M doesn't support the cfi_flash.c's command sequence for erasing and writing.
I suggest adding a subdir $(TOPDIR)/include/flash/ and putting flash chip information here. When you use some kinds of flash chips, just include relevant header files.
I think we should use the cfi_driver instead.
And BTW - a comment on the suggested code:
If we go further, many information in $(TOPDIR)/include/flash.h can be put into these header files.
...
The following is an example of AMD Spansion S29AL016M chip.
...
int FLASH_S29AL016M_T_BYTE_SA[] = { 0x000000, 0x010000, 0x020000, 0x030000, 0x040000, 0x050000, 0x060000, 0x070000,
...
All-capital-names should be reserved for preprocessor variables. It is not acceptable to use these for other variables or function names. It is also not a good idea to have such initializations in a header file which might get included more than once.
The code I give are JUST example, so uppper or lower case is not a problem in this sense.
I did worry about such initializations in header file, but given the fact that only flash.c will include it, I think it is not a real problem. And even it is included by multiple files, we can make these variables static. Or we can use better solution, but the idea itself is still reasonable.

In message 20051225190658.F3F9.LARK@linux.net.cn you wrote:
I have tested cfi_flash.c for a while. Unfortuanately, it fails for Spansion S29AL016M. The chip can be recognized, chip information is all correct, but the erase and write routines fail.
Then please (lets) fix the problem instead of inventing the wheel again.
And even it is included by multiple files, we can make these variables static. Or we can use better solution, but the idea itself is still
That's even worse as it would silently create multiple copies of the same data - which is just a waste of precious memory.
Best regards,
Wolfgang Denk

Hi Wolfgang Denk,
On Sun, 25 Dec 2005 17:34:49 +0100, Wolfgang Denk wd@denx.de wrote:
In message 20051225190658.F3F9.LARK@linux.net.cn you wrote:
I have tested cfi_flash.c for a while. Unfortuanately, it fails for Spansion S29AL016M. The chip can be recognized, chip information is all correct, but the erase and write routines fail.
Then please (lets) fix the problem instead of inventing the wheel again.
After a bad sleep, I read the manual again. This time, I find the cfi_flash.c DOES do as what manual says. I will set some debug information and find what happend. Stay tune.

Hi Denk,
Me again.
In my situation, S29AL016M works in x8 mode, which has different command sequence from in x16 mode.
The following are from datasheet.
Providing vu_char *addr = (vu_char *) info->start[sect];
x16 mode sector erase addr[0x555] = 0xAA; addr[0x2AA] = 0x55; addr[0x555] = 0x80; addr[0x555] = 0xAA; addr[0x2AA] = 0x55; addr[0x000] = 0x30; udelay(50); // at least 50 us before polling for status
x16 mode program addr[0x555] = 0xAA; addr[0x2AA] = 0x55; addr[0x555] = 0xA0; addr[offset] = data;
x8 mode sector erase addr[0xAAA] = 0xAA; addr[0x555] = 0x55; addr[0xAAA] = 0x80; addr[0xAAA] = 0xAA; addr[0x555] = 0x55; addr[0x000] = 0x30; udelay(50); // at least 50 us before polling for status
x8 mode program addr[0xAAA] = 0xAA; addr[0x555] = 0x55; addr[0xAAA] = 0xA0; addr[offset] = data;
I have little knowledge about other flash chips, so it is not easy for me to write clean patch for cfi_flash.c, although I want to.
Regards
On Mon, 26 Dec 2005 09:49:10 +0800, Wang Jian lark@linux.net.cn wrote:
Hi Wolfgang Denk,
On Sun, 25 Dec 2005 17:34:49 +0100, Wolfgang Denk wd@denx.de wrote:
In message 20051225190658.F3F9.LARK@linux.net.cn you wrote:
I have tested cfi_flash.c for a while. Unfortuanately, it fails for Spansion S29AL016M. The chip can be recognized, chip information is all correct, but the erase and write routines fail.
Then please (lets) fix the problem instead of inventing the wheel again.
After a bad sleep, I read the manual again. This time, I find the cfi_flash.c DOES do as what manual says. I will set some debug information and find what happend. Stay tune.

In message 20060103205317.0711.LARK@linux.net.cn you wrote:
In my situation, S29AL016M works in x8 mode, which has different command sequence from in x16 mode.
If you look closer it is not so different at all.
x16 mode sector erase addr[0x555] = 0xAA; addr[0x2AA] = 0x55; addr[0x555] = 0x80; addr[0x555] = 0xAA; addr[0x2AA] = 0x55; addr[0x000] = 0x30;
...
x8 mode sector erase addr[0xAAA] = 0xAA; addr[0x555] = 0x55; addr[0xAAA] = 0x80; addr[0xAAA] = 0xAA; addr[0x555] = 0x55; addr[0x000] = 0x30;
0x555 = 0xAAA >> 1 0x2AA = 0x555 >> 1
I have little knowledge about other flash chips, so it is not easy for me to write clean patch for cfi_flash.c, although I want to.
The code is actually the very same, just taking into account the different addressing mode.
Best regards,
Wolfgang Denk

Hi Wolfgang Denk,
On Tue, 03 Jan 2006 15:08:42 +0100, Wolfgang Denk wd@denx.de wrote:
In message 20060103205317.0711.LARK@linux.net.cn you wrote:
In my situation, S29AL016M works in x8 mode, which has different command sequence from in x16 mode.
If you look closer it is not so different at all.
x16 mode sector erase addr[0x555] = 0xAA; addr[0x2AA] = 0x55; addr[0x555] = 0x80; addr[0x555] = 0xAA; addr[0x2AA] = 0x55; addr[0x000] = 0x30;
...
x8 mode sector erase addr[0xAAA] = 0xAA; addr[0x555] = 0x55; addr[0xAAA] = 0x80; addr[0xAAA] = 0xAA; addr[0x555] = 0x55; addr[0x000] = 0x30;
0x555 = 0xAAA >> 1 0x2AA = 0x555 >> 1
I have little knowledge about other flash chips, so it is not easy for me to write clean patch for cfi_flash.c, although I want to.
The code is actually the very same, just taking into account the different addressing mode.
I am not sure if other kind of AMD flash chips use such x8/x16 command sequence. I am afraid that I will fix one thing and break other things.

In message 20060103232431.EB1F.LARK@linux.net.cn you wrote:
I am not sure if other kind of AMD flash chips use such x8/x16 command sequence. I am afraid that I will fix one thing and break other things.
They all do. At least all I can remember.
Best regards,
Wolfgang Denk

Wang Jian wrote:
Hi Wolfgang Denk,
On Sun, 25 Dec 2005 17:34:49 +0100, Wolfgang Denk wd@denx.de wrote:
In message 20051225190658.F3F9.LARK@linux.net.cn you wrote:
I have tested cfi_flash.c for a while. Unfortuanately, it fails for Spansion S29AL016M. The chip can be recognized, chip information is all correct, but the erase and write routines fail.
Then please (lets) fix the problem instead of inventing the wheel again.
After a bad sleep, I read the manual again. This time, I find the cfi_flash.c DOES do as what manual says. I will set some debug information and find what happend. Stay tune.
You might want to check list archives regarding cfi_flash.c and the a number of patches that are created over the past year that is *still* pending to be incorporated to the mainline.
And if you check the mailing list archives you might find that Spansion chip (most probably) auto locks all sectors when powered up so you will need to issue unlock sequence before write/erase can succeed. There has been some spirited discussions on this and Wolfgang as a U-Boot project leader had explained the way he wants the issue addressed.
Wolfgang, when can we expect the pull-up of patches on the cfi_flash.c driver? I am personally holding up for existing patches to go through before I work on new patches.
Best regards, Tolunay

Dear Tolunay,
in message 43BC4CD4.9020308@orkun.us you wrote:
Wolfgang, when can we expect the pull-up of patches on the cfi_flash.c driver? I am personally holding up for existing patches to go through
Well, in the last two months most of my time has been sucked up by the (long delayed) release of ELDK 4.0 (and by a little Xmas vacation :-).
Keep your finger's crossed, and tonight's ELDK build might be what we'll release as 4.0; the files in the CVS have already been tagged as 4.0. If no new problems show up, this should enable me to spend some more time on U-Boot again.
before I work on new patches.
Ummm... if you use git, you can and should make yourself independent of my patch backlog. Actually if you spend the effort to go though the list of cfi_flash driver related patches and come up with a (somewhat tested) summary patch (or branch) that would be a great help. We could do the same like with the NAND code: add it as #testing-CFI branch, and if nobody complains I will merge it a little later.
*All* feedback I receive for patches (like messages saying "added patches X, Y, and Z, works for me, here ... is the URL of my git repo where you can pull from) is welcome and helps to accelerate things.
Best regards,
Wolfgang Denk
participants (3)
-
Tolunay Orkun
-
Wang Jian
-
Wolfgang Denk