Bug in m25p80.c during whole-chip erase
PATCH 1/1 Bug in m25p80.c during whole-chip erase
There is a logic error in "whole chip erase" for the m25p80 family. If the whole device is successfully erased, erase_chip() will return 0, and the code will fall through to the "else" clause, and do sector-by-sector erase in addition to the whole-chip erase. This patch corrects that.
Also, the MAX_READY_WAIT_COUNT is insufficient for an m25p16 connected to a 400 MHz powerpc. Increasing it allows me to successfully program the device on my board.
---
drivers/mtd/devices/m25p80.c +7 -5
@@ -53,7 +53,7 @@#define SR_SRWD 0x80/* SR write protect */ /* Define max times to check status register before we give up. */ -#define MAX_READY_WAIT_COUNT 100000 +#define MAX_READY_WAIT_COUNT 1000000#define CMD_SIZE 4#ifdef CONFIG_M25PXX_USE_FAST_READ
@@ -251,10 +251,12 @@ static int m25p80_erase(struct mtd_info *mtd, struct erase_info *instr)mutex_lock (&flash->lock);/* whole-chip erase? */ -if (len == flash->mtd.size &&erase_chip (flash)) { - instr->state =MTD_ERASE_FAILED ; -mutex_unlock (&flash->lock); -return -EIO ; +if (len == flash->mtd.size) { +if (erase_chip (flash)) { + instr->state =MTD_ERASE_FAILED ; +mutex_unlock (&flash->lock); +return -EIO ; + }/* REVISIT in some cases we could speed up erasing large regions * by using OPCODE_SE instead of OPCODE_BE_4K. We may have set up
Quoted 8 lines · Steven A. Falco
There is a logic error in "whole chip erase" for the m25p80 family. If the whole device is successfully erased,
erase_chip()will return 0, and the code will fall through to the "else" clause, and do sector-by-sector erase in addition to the whole-chip erase. This patch corrects that.Also, the
MAX_READY_WAIT_COUNTis insufficient for an m25p16 connected to a 400 MHz powerpc. Increasing it allows me to successfully program the device on my board.
in general, trying to set timeouts "close" to the spec gains nothing with spi flash devices. the timeout limit is hit only when an error occurs, and errors should not occur during the normal run of things. on the other side, having a timeout be wrongly hit when the device is operating correctly is a much worse situation to be in. -mike
Quoted 10 lines · Mike Frysinger
Quoted 8 lines · Steven A. Falco
There is a logic error in "whole chip erase" for the m25p80 family. If the whole device is successfully erased,
erase_chip()will return 0, and the code will fall through to the "else" clause, and do sector-by-sector erase in addition to the whole-chip erase. This patch corrects that.Also, the
MAX_READY_WAIT_COUNTis insufficient for an m25p16 connected to a 400 MHz powerpc. Increasing it allows me to successfully program the device on my board.
Quoted 7 lines
in general, trying to set timeouts "close" to the spec gains nothing with spi flash devices. the timeout limit is hit only when an error occurs, and errors should not occur during the normal run of things. on the other side, having a timeout be wrongly hit when the device is operating correctly is a much worse situation to be in. -mike
Right. This chip takes 13 seconds (typical) to bulk erase, according to the Numonyx data sheet. So increasing the timeout to account for such a slow part is necessary, and allows me to successfully erase the part.
Are there any changes you'd like to see in this patch, or is it ok as written?
Steve
Quoted 26 lines · Peter Horton
Quoted 24 lines · Steven A. Falco
Quoted 11 lines · Mike Frysinger
Quoted 10 lines · Steven A. Falco
There is a logic error in "whole chip erase" for the m25p80 family. If the whole device is successfully erased,
erase_chip()will return 0, and
the code will fall through to the "else" clause, and do sector-by-sector
erase in addition to the whole-chip erase. This patch corrects that.Also, the
MAX_READY_WAIT_COUNTis insufficient for an m25p16 connected to a 400 MHz powerpc. Increasing it allows me to successfully program the device on my board.Quoted 7 lines
in general, trying to set timeouts "close" to the spec gains nothing with spi flash devices. the timeout limit is hit only when an error occurs, and errors should not occur during the normal run of things. on the other side, having a timeout be wrongly hit when the device is operating correctly is a much worse situation to be in. -mike
Right. This chip takes 13 seconds (typical) to bulk erase, according to the Numonyx data sheet. So increasing the timeout to account for such a slow part is necessary, and allows me to successfully erase the part.
Quoted 9 lines
Quoted 3 lines
Are there any changes you'd like to see in this patch, or is it ok as written?
Check the list archives. I submitted a patch for this a few weeks ago which has been merged into AKPMs tree.
P.
Your patch addresses the timing issues, but not the duplicate erase.
I'll resubmit my patch with just the duplicate erase fix, as this is still needed.
Steve
Quoted 17 lines · Steven A. Falco
Quoted 10 lines · Mike Frysinger
Quoted 8 lines · Steven A. Falco
There is a logic error in "whole chip erase" for the m25p80 family. If the whole device is successfully erased,
erase_chip()will return 0, and the code will fall through to the "else" clause, and do sector-by-sector erase in addition to the whole-chip erase. This patch corrects that.Also, the
MAX_READY_WAIT_COUNTis insufficient for an m25p16 connected to a 400 MHz powerpc. Increasing it allows me to successfully program the device on my board.Quoted 5 lines
in general, trying to set timeouts "close" to the spec gains nothing with spi flash devices. the timeout limit is hit only when an error occurs, and errors should not occur during the normal run of things. on the other side, having a timeout be wrongly hit when the device is operating correctly is a much worse situation to be in.
Quoted 6 lines
Right. This chip takes 13 seconds (typical) to bulk erase, according to the Numonyx data sheet. So increasing the timeout to account for such a slow part is necessary, and allows me to successfully erase the part.
Are there any changes you'd like to see in this patch, or is it ok as written?
changes look fine to me, but i'm not in any position to get things merged for you ;)
-mike