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

[Jagan Teki]

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 at 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 at 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 at 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.