: Workaround for Numonyx Axcell P33/P30 256-Mbit 65nm bug

Hello Wolfgang & list,

I have "ported" U-boot to a in house made board with Numonyx Axcell P33/P30 256-Mbit 65nm flash chips.

After some time :( searching for bugs in our board or soft, we have discovered that those chips have a small but annoying bug, documented in "Numonyx Axcell P33/P30 256-Mbit Specification Update"

Hello Wolfgang & list,

This is a revised patch, with comments and indentation fixed, I hope.

I have "ported" U-boot to a in house made board with Numonyx Axcell P33/P30 256-Mbit 65nm flash chips.

After some time :( searching for bugs in our board or soft, we have discovered that those chips have a small but annoying bug, documented in "Numonyx Axcell P33/P30 256-Mbit Specification Update"

It states :
When customer uses [...] block unlock, the block lock status might be altered inadvertently. Lock status might be set to either 01h or 03h unexpectedly (00h as expected data), which leads to program/erase failure on certain blocks.

A working workaround is given, which I have applied and tested with success.

Signed-off-by Philippe De Muyter <[email protected]>

---
 drivers/mtd/cfi_flash.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

Dear Stefan,

Quoted 17 lines · In message <[email protected]> Philippe De Muyter wrote:

Hello Wolfgang & list,

This is a revised patch, with comments and indentation fixed, I hope.

I have "ported" U-boot to a in house made board with Numonyx Axcell P33/P30 256-Mbit 65nm flash chips.

After some time :( searching for bugs in our board or soft, we have discovered that those chips have a small but annoying bug, documented in "Numonyx Axcell P33/P30 256-Mbit Specification Update"

It states :
When customer uses [...] block unlock, the block lock status might be altered inadvertently. Lock status might be set to either 01h or 03h unexpectedly (00h as expected data), which leads to program/erase failure on certain blocks.

Quoted 7 lines

A working workaround is given, which I have applied and tested with success.

Signed-off-by Philippe De Muyter <[email protected]>

---
 drivers/mtd/cfi_flash.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

I didn't see comments from you?

Best regards,

Wolfgang Denk

Quoted 33 lines

Dear Stefan,

Quoted 17 lines · In message <[email protected]> Philippe De Muyter wrote:

Hello Wolfgang & list,

This is a revised patch, with comments and indentation fixed, I hope.

I have "ported" U-boot to a in house made board with Numonyx Axcell P33/P30 256-Mbit 65nm flash chips.

After some time :( searching for bugs in our board or soft, we have discovered that those chips have a small but annoying bug, documented in "Numonyx Axcell P33/P30 256-Mbit Specification Update"

It states :
When customer uses [...] block unlock, the block lock status might be altered inadvertently. Lock status might be set to either 01h or 03h unexpectedly (00h as expected data), which leads to program/erase failure on certain blocks.

Quoted 7 lines

A working workaround is given, which I have applied and tested with success.

Signed-off-by Philippe De Muyter <[email protected]>

---
 drivers/mtd/cfi_flash.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

I didn't see comments from you?

Best regards,

Quoted 1 line

Wolfgang Denk

Doesn't the Linux kernel need the same fix? Would be great if you(Philippe) could provide one. I too have such chips but I have never seen this problem so I guess I have been lucky so far :)

Jocke

Hello Joakim,

Quoted 33 lines · Joakim Tjernlund

Quoted 33 lines

Dear Stefan,

Quoted 17 lines · In message <[email protected]> Philippe De Muyter wrote:

Hello Wolfgang & list,

This is a revised patch, with comments and indentation fixed, I hope.

I have "ported" U-boot to a in house made board with Numonyx Axcell P33/P30 256-Mbit 65nm flash chips.

After some time :( searching for bugs in our board or soft, we have discovered that those chips have a small but annoying bug, documented in "Numonyx Axcell P33/P30 256-Mbit Specification Update"

It states :
When customer uses [...] block unlock, the block lock status might be altered inadvertently. Lock status might be set to either 01h or 03h unexpectedly (00h as expected data), which leads to program/erase failure on certain blocks.

Quoted 7 lines

A working workaround is given, which I have applied and tested with success.

Signed-off-by Philippe De Muyter <[email protected]>

---
 drivers/mtd/cfi_flash.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

I didn't see comments from you?

Best regards,

Quoted 7 lines

Quoted 1 line

Wolfgang Denk

Doesn't the Linux kernel need the same fix? Would be great if you(Philippe) could provide one. I too have such chips but I have never seen this problem so I guess I have been lucky so far :)

Jocke

I use linux on those boards, of course, so I also thought that I'd need to fix linux, but I didn't have to. On the boards I had, the bug was easily repeatable with u-boot or the bdm-tools's bdm-based flash programmer I used. The same blocks always gave the same error, while other blocks always worked perfectly. From linux, I tested with eraseall from mtd-utils also on the bad blocks and I never had any problem, so linux probably already does the erase sequence the way numonyx testers do it. The key point is that the CFI unlock command sequence must come just before the CFI erase command sequence, not long before.

Philippe

Hello Joakim,

Quoted 33 lines · Joakim Tjernlund

Quoted 33 lines

Dear Stefan,

Quoted 17 lines · In message <[email protected]> Philippe De Muyter wrote:

Hello Wolfgang & list,

This is a revised patch, with comments and indentation fixed, I hope.

I have "ported" U-boot to a in house made board with Numonyx Axcell P33/P30 256-Mbit 65nm flash chips.

After some time :( searching for bugs in our board or soft, we have discovered that those chips have a small but annoying bug, documented in "Numonyx Axcell P33/P30 256-Mbit Specification Update"

It states :
When customer uses [...] block unlock, the block lock status might be altered inadvertently. Lock status might be set to either 01h or 03h unexpectedly (00h as expected data), which leads to program/erase failure on certain blocks.

Quoted 7 lines

A working workaround is given, which I have applied and tested with success.

Signed-off-by Philippe De Muyter <[email protected]>

---
 drivers/mtd/cfi_flash.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

I didn't see comments from you?

Best regards,

Quoted 7 lines

Quoted 1 line

Wolfgang Denk

Doesn't the Linux kernel need the same fix? Would be great if you(Philippe) could provide one. I too have such chips but I have never seen this problem so I guess I have been lucky so far :)

Jocke

I use linux on those boards, of course, so I also thought that I'd need to fix linux, but I didn't have to. On the boards I had, the bug was easily repeatable with u-boot or the bdm-tools's bdm-based flash programmer I used. The same blocks always gave the same error, while other blocks always worked perfectly. From linux, I tested with eraseall from mtd-utils also on the bad blocks and I never had any problem, so linux probably already does the erase sequence the way numonyx testers do it. The key point is that the CFI unlock command sequence must come just before the CFI erase command sequence, not long before.

Dear Stefan,

Quoted 4 lines · In message <[email protected]> Philippe De Muyter wrote:

Hello Wolfgang & list,

This is a revised patch, with comments and indentation fixed, I hope.

drivers/mtd/cfi_flash.c +3 -4

index 3267c5d..4bf0807 100644
@@ -1347,17 +1347,34 @@ int flash_real_protect (flash_info_t * info, long
 sector, int prot)
 
        switch (info->vendor) {
 
                case CFI_CMDSET_INTEL_PROG_REGIONS:
 
                case CFI_CMDSET_INTEL_STANDARD:
-               case CFI_CMDSET_INTEL_EXTENDED:
-                       flash_write_cmd (info, sector, 0,
-                                        FLASH_CMD_CLEAR_STATUS);
-                       flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT);
+               case CFI_CMDSET_INTEL_EXTENDED: {
+                       unsigned short cmd;
+

Hi Philippe,

first sorry for the late review (thanks Wolfgang for reminding).

Quoted 7 lines · On Saturday 07 August 2010 01:11:02 Wolfgang Denk wrote:

Dear Stefan,

Quoted 4 lines · In message <[email protected]> Philippe De Muyter wrote:

Hello Wolfgang & list,

This is a revised patch, with comments and indentation fixed, I hope.

Minor note: Such commentary lines should be moved below the "---" line below. They shouldn't appear in the git history.

<snip>

Quoted 26 lines

drivers/mtd/cfi_flash.c +4 -6

index 3267c5d..4bf0807 100644
@@ -1347,17 +1347,34 @@ int flash_real_protect (flash_info_t * info, long
 sector, int prot)
 
        switch (info->vendor) {
 
                case CFI_CMDSET_INTEL_PROG_REGIONS:
 
                case CFI_CMDSET_INTEL_STANDARD:
-               case CFI_CMDSET_INTEL_EXTENDED:
-                       flash_write_cmd (info, sector, 0,
-                                        FLASH_CMD_CLEAR_STATUS);
-                       flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT);
+               case CFI_CMDSET_INTEL_EXTENDED: {
+                       unsigned short cmd;
+
 
                        if (prot)
 
-                               flash_write_cmd (info, sector, 0,
-                                       FLASH_CMD_PROTECT_SET);
+                               cmd = FLASH_CMD_PROTECT_SET;
 

Quoted 3 lines

else

+ cmd = FLASH_CMD_PROTECT_CLEAR;

Please move this variable declaration of "cmd" and assignment further down (see below). You can remove the "{" "}" of this block then.

drivers/mtd/cfi_flash.c int flash_real_protect (flash_info_t * info, long sector, int prot) · line 1364

Quoted 8 lines

drivers/mtd/cfi_flash.c +8 -0

+                       /*
+                        * see errata called
+                        * "Numonyx Axcell P33/P30 Specification Update" :)
+                        */
+                       flash_write_cmd (info, sector, 0, FLASH_CMD_READ_ID);
+                       if (!flash_isequal (info, sector, FLASH_OFFSET_PROTECT,
+                                           prot)) {
+                               int flag = disable_interrupts ();

Declare and assign "cmd" here please.

Please change and resubmit. Thanks.

Quoted 2 lines

Please move this variable declaration of "cmd" and assignment further down (see below). You can remove the "{" "}" of this block then.

Here is the excerpt from the errata :

Workaround: If the interval between 60h and its subsequent command
can be guaranteed within 20?s, Option I is recommended, otherwise Option II (involves hardware) should be selected.
Option I: The table below lists the detail command sequences:
Command
Data bus Address bus Remarks
Sequence
1 90h Block Address
Read Lock Status
2 Read Block Address + 02h
(2)(3) (1)
3 60h Block Address
(2)(3) (1) Lock/Unlock/RCR
Configuration 4 D0h/01h/03h Block Address
Notes:
(1) Block Address refers to RCR configuration data only when the 60h command sequence is used to set RCR register combined with 03h subsequent command.
(2) For the third and fourth command sequences, the Block Address must be the same.
(3) The interval between 60h command and its subsequent D0h/01h/2Fh/03h commands should be less than 20?s.

Because of requirement (3), I choosed to minimize the number of instructions between the read lock status' and the unlock' commands, hence the initialisation of cmd' moved before the read lock status' (hidden in `flash_isequal').

Quoted 10 lines

Quoted 9 lines

drivers/mtd/cfi_flash.c +3 -0

+                       /*
+                        * see errata called
+                        * "Numonyx Axcell P33/P30 Specification

Update" :) + */
+ flash_write_cmd (info, sector, 0,
FLASH_CMD_READ_ID); + if (!flash_isequal (info,
sector, FLASH_OFFSET_PROTECT, +
prot)) {
+ int flag = disable_interrupts ();

Quoted 3 lines

Declare and assign "cmd" here please.

Please change and resubmit. Thanks.

If you still prefer it changed, speak :)

Hi Philippe,

Quoted 32 lines · On Monday 09 August 2010 17:46:03 Philippe De Muyter wrote:

Quoted 2 lines

Please move this variable declaration of "cmd" and assignment further down (see below). You can remove the "{" "}" of this block then.

Here is the excerpt from the errata :

Workaround: If the interval between 60h and its subsequent command
can be guaranteed within 20?s, Option I is recommended, otherwise Option II (involves hardware) should be selected.
Option I: The table below lists the detail command sequences:
Command
Data bus Address bus Remarks
Sequence
1 90h Block Address
Read Lock Status
2 Read Block Address + 02h
(2)(3) (1)
3 60h Block Address
(2)(3) (1) Lock/Unlock/RCR
Configuration 4 D0h/01h/03h Block Address
Notes:
(1) Block Address refers to RCR configuration data only when the 60h command sequence is used to set RCR register combined with 03h subsequent command.
(2) For the third and fourth command sequences, the Block Address must be the same.
(3) The interval between 60h command and its subsequent D0h/01h/2Fh/03h commands should be less than 20?s.

Because of requirement (3), I choosed to minimize the number of instructions between the read lock status' and the unlock' commands, hence the initialisation of cmd' moved before the read lock status' (hidden in `flash_isequal').

From my understanding, "only" the last 2 operations need to be in max. 20?s interval. If this is the case, then I would prefer this code version:

		case CFI_CMDSET_INTEL_EXTENDED:
			/*
			 * see errata called
			 * "Numonyx Axcell P33/P30 Specification Update" :)
			 */
			flash_write_cmd (info, sector, 0, FLASH_CMD_READ_ID);
			if (!flash_isequal (info, sector, FLASH_OFFSET_PROTECT,
					    prot)) {
				int flag = disable_interrupts ();
				unsigned short cmd;

				if (prot)

					cmd = FLASH_CMD_PROTECT_SET;
				else
					cmd = FLASH_CMD_PROTECT_CLEAR;

				/*
				 * cmd must come before FLASH_CMD_PROTECT + 20us
				 * Disable interrupts which might cause a timeout here.
				 */

				flash_write_cmd (info, sector, 0,
						  FLASH_CMD_PROTECT);
				flash_write_cmd (info, sector, 0, cmd);
				/* re-enable interrupts if necessary */
				if (flag)
					enable_interrupts ();
			}

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

drivers/mtd/cfi_flash.c +3 -0

+                       /*
+                        * see errata called
+                        * "Numonyx Axcell P33/P30 Specification

Update" :) + */
+ flash_write_cmd (info, sector, 0,
FLASH_CMD_READ_ID); + if (!flash_isequal (info,
sector, FLASH_OFFSET_PROTECT, +
prot)) {
+ int flag = disable_interrupts ();

Quoted 5 lines

Quoted 3 lines

Declare and assign "cmd" here please.

Please change and resubmit. Thanks.

If you still prefer it changed, speak :)

Yes, please let me know if this patch version also fixes the bug. If this is the case, I would prefer that you change it accordingly.

Quoted 4 lines

Quoted 4 lines

Because of requirement (3), I choosed to minimize the number of instructions between the read lock status' and the unlock' commands, hence the initialisation of cmd' moved before the read lock status' (hidden in `flash_isequal').

Yes, you're right. I had misread the doc :(

Quoted 3 lines

From my understanding, "only" the last 2 operations need to be in max. 20?s interval. If this is the case, then I would prefer this code version:

I prefer it too.

Quoted 20 lines

		case CFI_CMDSET_INTEL_EXTENDED:
			/*
			 * see errata called
			 * "Numonyx Axcell P33/P30 Specification Update" :)
			 */
			
			flash_write_cmd (info, sector, 0, FLASH_CMD_READ_ID);
			if (!flash_isequal (info, sector, FLASH_OFFSET_PROTECT,
			
					    prot)) {
				
				int flag = disable_interrupts ();
				unsigned short cmd;
				
				if (prot)
				
					cmd = FLASH_CMD_PROTECT_SET;
				
				else

Quoted 6 lines

					cmd = FLASH_CMD_PROTECT_CLEAR;
				
				/*
				 * cmd must come before FLASH_CMD_PROTECT + 20us
				 * Disable interrupts which might cause a timeout here.
				 */

Should the above comment not stay closer to the disable_interrupts () call?

Quoted 2 lines

Yes, please let me know if this patch version also fixes the bug. If this is the case, I would prefer that you change it accordingly.

I surmise it does, but I currently do not have a board to test. I'll get new boards (with the same bug) soon, and I'll let you know then.

Here is the excerpt from the errata :

Workaround: If the interval between 60h and its subsequent command
can be guaranteed within 20?s, Option I is recommended, otherwise Option II (involves hardware) should be selected.
Option I: The table below lists the detail command sequences:
Command
Data bus Address bus Remarks
Sequence
1 90h Block Address
Read Lock Status
2 Read Block Address + 02h
(2)(3) (1)
3 60h Block Address
(2)(3) (1) Lock/Unlock/RCR
Configuration 4 D0h/01h/03h Block Address

Notes:
(1) Block Address refers to RCR configuration data only when the 60h command sequence is used to set RCR register combined with 03h subsequent command.
(2) For the third and fourth command sequences, the Block Address must be the same.
(3) The interval between 60h command and its subsequent D0h/01h/2Fh/03h commands should be less than 20?s.

Hi Philippe,

one more thing:

Quoted 16 lines · On Monday 09 August 2010 17:46:03 Philippe De Muyter wrote:

Here is the excerpt from the errata :

Workaround: If the interval between 60h and its subsequent command
can be guaranteed within 20?s, Option I is recommended, otherwise Option II (involves hardware) should be selected.
Option I: The table below lists the detail command sequences:
Command
Data bus Address bus Remarks
Sequence
1 90h Block Address
Read Lock Status
2 Read Block Address + 02h
(2)(3) (1)
3 60h Block Address
(2)(3) (1) Lock/Unlock/RCR
Configuration 4 D0h/01h/03h Block Address

Quoted 8 lines

Notes:
(1) Block Address refers to RCR configuration data only when the 60h command sequence is used to set RCR register combined with 03h subsequent command.
(2) For the third and fourth command sequences, the Block Address must be the same.
(3) The interval between 60h command and its subsequent D0h/01h/2Fh/03h commands should be less than 20?s.

Please add this workaround text from the errata to the commit text in your next patch version. This makes things easier - no need to search for the errata.