[U-Boot] [PATCH] Fixing spi erase for S25FL128P_256K
The spansion_erase currently only works when the sector size is 64KB. cmd[1] should contain the higher 8 bit of the 24 bit address of the sector to be erased. Currently it is holding the sector index to be erased which happens to be the same thing when the sector size is 64KB.
Signed-off-by: Marc-Andre Hebert marc-andre.hebert@humanware.com --- drivers/mtd/spi/spansion.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/spi/spansion.c b/drivers/mtd/spi/spansion.c index d6c1a5f..94489af 100644 --- a/drivers/mtd/spi/spansion.c +++ b/drivers/mtd/spi/spansion.c @@ -262,7 +262,6 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) return -1; }
- len /= sector_size; cmd[0] = CMD_S25FLXX_SE; cmd[2] = 0x00; cmd[3] = 0x00; @@ -274,8 +273,8 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) }
ret = 0; - for (actual = 0; actual < len; actual++) { - cmd[1] = (offset / sector_size) + actual; + for (actual = 0; actual < len; actual+=sector_size) { + cmd[1] = (offset + actual) >> 16;
ret = spi_flash_cmd(flash->spi, CMD_S25FLXX_WREN, NULL, 0); if (ret < 0) { @@ -298,7 +297,7 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) }
debug("SF: SPANSION: Successfully erased %u bytes @ 0x%x\n", - len * sector_size, offset); + len, offset);
spi_release_bus(flash->spi); return ret;
Hello.
Marc-André Hébert wrote:
The spansion_erase currently only works when the sector size is 64KB. cmd[1] should contain the higher 8 bit of the 24 bit address of the sector to be erased. Currently it is holding the sector index to be erased which happens to be the same thing when the sector size is 64KB.
Signed-off-by: Marc-Andre Hebert marc-andre.hebert@humanware.com
drivers/mtd/spi/spansion.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/spi/spansion.c b/drivers/mtd/spi/spansion.c index d6c1a5f..94489af 100644 --- a/drivers/mtd/spi/spansion.c +++ b/drivers/mtd/spi/spansion.c
[...]
@@ -274,8 +273,8 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) }
ret = 0;
- for (actual = 0; actual < len; actual++) {
cmd[1] = (offset / sector_size) + actual;
- for (actual = 0; actual < len; actual+=sector_size) {
Keep the coding style consistent please -- add spaces around +=.
WBR, Sergei
The spansion_erase currently only works when the sector size is 64KB. cmd[1] should contain the higher 8 bit of the 24 bit address of the sector to be erased. Currently it is holding the sector index to be erased which happens to be the same thing when the sector size is 64KB.
Signed-off-by: Marc-Andre Hebert marc-andre.hebert@humanware.com --- drivers/mtd/spi/spansion.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/spi/spansion.c b/drivers/mtd/spi/spansion.c index d6c1a5f..94489af 100644 --- a/drivers/mtd/spi/spansion.c +++ b/drivers/mtd/spi/spansion.c @@ -262,7 +262,6 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) return -1; }
- len /= sector_size; cmd[0] = CMD_S25FLXX_SE; cmd[2] = 0x00; cmd[3] = 0x00; @@ -274,8 +273,8 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) }
ret = 0; - for (actual = 0; actual < len; actual++) { - cmd[1] = (offset / sector_size) + actual; + for (actual = 0; actual < len; actual += sector_size) { + cmd[1] = (offset + actual) >> 16;
ret = spi_flash_cmd(flash->spi, CMD_S25FLXX_WREN, NULL, 0); if (ret < 0) { @@ -298,7 +297,7 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) }
debug("SF: SPANSION: Successfully erased %u bytes @ 0x%x\n", - len * sector_size, offset); + len, offset);
spi_release_bus(flash->spi); return ret;
2010/8/10 Marc-André Hébert:
The spansion_erase currently only works when the sector size is 64KB. cmd[1] should contain the higher 8 bit of the 24 bit address of the sector to be erased. Currently it is holding the sector index to be erased which happens to be the same thing when the sector size is 64KB.
ugh, this is why the current sf framework annoys me. so much duplication across drivers including bugs.
--- a/drivers/mtd/spi/spansion.c +++ b/drivers/mtd/spi/spansion.c @@ -274,8 +273,8 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) }
ret = 0;
- for (actual = 0; actual < len; actual++) {
- cmd[1] = (offset / sector_size) + actual;
- for (actual = 0; actual < len; actual += sector_size) {
- cmd[1] = (offset + actual) >> 16;
how about the bug fix that went into stmicro: - cmd[1] = (offset / sector_size) + actual; + cmd[1] = offset >> 16; + offset += sector_size; -mike
2010/8/10 Mike Frysinger vapier@gentoo.org:
2010/8/10 Marc-André Hébert:
The spansion_erase currently only works when the sector size is 64KB. cmd[1] should contain the higher 8 bit of the 24 bit address of the sector to be erased. Currently it is holding the sector index to be erased which happens to be the same thing when the sector size is 64KB.
ugh, this is why the current sf framework annoys me. so much duplication across drivers including bugs.
--- a/drivers/mtd/spi/spansion.c +++ b/drivers/mtd/spi/spansion.c @@ -274,8 +273,8 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) }
ret = 0;
- for (actual = 0; actual < len; actual++) {
- cmd[1] = (offset / sector_size) + actual;
- for (actual = 0; actual < len; actual += sector_size) {
- cmd[1] = (offset + actual) >> 16;
how about the bug fix that went into stmicro:
- cmd[1] = (offset / sector_size) + actual;
- cmd[1] = offset >> 16;
- offset += sector_size;
-mike
Yeah I saw how stmicro did it, but I didn't do it that way simply because modifying the offset invalidates the debug statement afterwards:
debug("SF: SPANSION: Successfully erased %u bytes @ 0x%x\n", len, offset); Marc
2010/8/11 Marc-André Hébert :
2010/8/10 Mike Frysinger:
2010/8/10 Marc-André Hébert:
The spansion_erase currently only works when the sector size is 64KB. cmd[1] should contain the higher 8 bit of the 24 bit address of the sector to be erased. Currently it is holding the sector index to be erased which happens to be the same thing when the sector size is 64KB.
ugh, this is why the current sf framework annoys me. so much duplication across drivers including bugs.
--- a/drivers/mtd/spi/spansion.c +++ b/drivers/mtd/spi/spansion.c @@ -274,8 +273,8 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) }
ret = 0;
- for (actual = 0; actual < len; actual++) {
- cmd[1] = (offset / sector_size) + actual;
- for (actual = 0; actual < len; actual += sector_size) {
- cmd[1] = (offset + actual) >> 16;
how about the bug fix that went into stmicro:
- cmd[1] = (offset / sector_size) + actual;
- cmd[1] = offset >> 16;
- offset += sector_size;
Yeah I saw how stmicro did it, but I didn't do it that way simply because modifying the offset invalidates the debug statement afterwards:
debug("SF: SPANSION: Successfully erased %u bytes @ 0x%x\n", len, offset);
and this just reinforces my point. you're basically saying the stmicro flash code has a broken debug() statement now. -mike
On Tuesday, August 10, 2010 09:02:09 Marc-André Hébert wrote:
@@ -262,7 +262,6 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) return -1;
seems your mailer too is broken. please use `git send-email` when posting patches so they dont get screwed up. -mike
On Monday, August 23, 2010 17:37:44 Mike Frysinger wrote:
On Tuesday, August 10, 2010 09:02:09 Marc-André Hébert wrote:
@@ -262,7 +262,6 @@ int spansion_erase(struct spi_flash *flash, u32 offset, size_t len)
return -1;seems your mailer too is broken. please use `git send-email` when posting patches so they dont get screwed up.
although i fixed up the commit and merged it manually. but you only get one pass, so next time i'll wait for a proper resend :P.
this is in my sf branch so it wont get lost -mike
participants (3)
-
Marc-André Hébert -
Mike Frysinger -
Sergei Shtylyov