Re: [U-Boot] [PATCH] Add SPI Flash STMicro's N25Q512A & add flag status check during SPI Flash write

Hi,

Thanks for your new changes.

I don't know may be you sent these changes intentionally. I personally not encouraging these as you sent all changes in one patch, attached a patch series to mail and did n't follow commit message header. http://www.denx.de/wiki/view/U-Boot/Patches#Commit_message_conventions

On Mon, Jun 3, 2013 at 10:16 PM, Insop Song Insop.Song@cohere.net wrote:

Hi,

I've made two changes while I was working on STMidro's SPI Flash N25Q512A

• add the device entry for STMidro's SPI Flash N25Q512A
• add Flag status check during Without this flag checking "sf write" will fail and SPI flash is locked up

Please see the patch and let me know your feedback.

From 97572b32c49d06ca6f8548ed88e6e381fb719a08 Mon Sep 17 00:00:00 2001 From: Insop Song insop.song@cohere.net Date: Sun, 2 Jun 2013 13:33:37 -0700 Subject: [PATCH] Add SPI Flash STMicro's N25Q512A & add flag status check during SPI Flash write

Signed-off-by: Insop Song insop.song@cohere.net

drivers/mtd/spi/spi_flash.c | 25 +++++++++++++++++++++++++ drivers/mtd/spi/spi_flash_internal.h | 1 + drivers/mtd/spi/stmicro.c | 6 ++++++ 3 files changed, 32 insertions(+)

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index 00aece9..f53756d 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -72,6 +72,7 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset, size_t chunk_len, actual; int ret; u8 cmd[4];

•   u8 flag_status;
  
    page_size = flash->page_size;
    page_addr = offset / page_size;
  

@@ -83,6 +84,18 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset, return ret; }

+wait_flag:

•   ret = spi_flash_cmd(flash->spi, CMD_READ_FLAG_STATUS, &flag_status, sizeof(flag_status));
  

•   if (ret < 0) {
  

•           printf("SF: reading flag failed\n");
  

•           return ret;
  

•   }
  

•   debug("flag_status %d\n", flag_status);
  

•   if ((flag_status & 0x80) == 0) {
  

•                   udelay(10);
  

•                   goto wait_flag;
  

•   }
  

•   cmd[0] = CMD_PAGE_PROGRAM;
    for (actual = 0; actual < len; actual += chunk_len) {
            chunk_len = min(len - actual, page_size - byte_addr);
  

@@ -106,6 +119,18 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset, debug("SF: write failed\n"); break; }

•           /* check status */
  

+flag_check:

•           ret = spi_flash_cmd(flash->spi, CMD_READ_FLAG_STATUS, &flag_status, sizeof(flag_status));
  

•           if (ret < 0) {
  

•                   printf("SF: reading flag failed\n");
  

•                   break;
  

•           }
  

•           debug("flag_status %d\n", flag_status);
  

•           if (!(flag_status & 0x80)) {
  

•                   udelay(100);
  

•                   goto flag_check;
  

•           }
  

Instead doing this poll on actaual write, may be do it on spi_flash_cmd_wait_ready() for code compatibility. Did you tested these changes, i think the same Flag_status must require on erase as well.

            ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT);
            if (ret)

diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h index 141cfa8..90b6993 100644 --- a/drivers/mtd/spi/spi_flash_internal.h +++ b/drivers/mtd/spi/spi_flash_internal.h @@ -22,6 +22,7 @@ #define CMD_PAGE_PROGRAM 0x02 #define CMD_WRITE_DISABLE 0x04 #define CMD_READ_STATUS 0x05 +#define CMD_READ_FLAG_STATUS 0x70 #define CMD_WRITE_ENABLE 0x06 #define CMD_ERASE_4K 0x20 #define CMD_ERASE_32K 0x52 diff --git a/drivers/mtd/spi/stmicro.c b/drivers/mtd/spi/stmicro.c index 30b626a..ad94ad2 100644 --- a/drivers/mtd/spi/stmicro.c +++ b/drivers/mtd/spi/stmicro.c @@ -110,6 +110,12 @@ static const struct stmicro_spi_flash_params stmicro_spi_flash_table[] = { .nr_sectors = 512, .name = "N25Q256", },

•   {
  

•           .id = 0xba20,
  

This is wrong, 0xba20 is for N25Q512, 0xbb20 is for N25Q512A., agree?

Please see there is a patch available in spi bucket http://patchwork.ozlabs.org/patch/247953/

The main agenda about this patch is trying to use same wait_poll func which is used for read_status register, to make code reliable and modular.

Please test the above patch http://patchwork.ozlabs.org/patch/247953/ Let me know if you have any concerns/issues.

-- Thanks, Jagan.