Hi Prabhakar,

Le 20/12/2017 à 11:32, Prabhakar Kushwaha a écrit :
> This patch add support of memories with size above 128Mib. It has
> been ported from Linux commit "mtd: spi-nor: add a
> stateless method to support memory size above 128Mib".
> 
> It convert 3byte opcode into 4byte opcode based upon
> OPCODES_4B or Spansion flash. Also the commands are malloc'ed
> at run time based on 3byte or 4byte address opcode requirement.
> 
> Signed-off-by: Prabhakar Kushwaha <[email protected]>
> CC: Cyrille Pitchen <[email protected]>
> CC: Marek Vasut <[email protected]>
> CC: Vignesh R <[email protected]>
> ---
> depends upon https://patchwork.ozlabs.org/patch/826919/
> 
>  drivers/mtd/spi/sf_internal.h |  28 ++++++++-
>  drivers/mtd/spi/spi_flash.c   | 136 
> ++++++++++++++++++++++++++++++++++++------
>  include/spi_flash.h           |   2 +
>  3 files changed, 146 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 06dee0a..164b0ea 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -27,7 +27,8 @@ enum spi_nor_option_flags {
>  };
>  
>  #define SPI_FLASH_3B_ADDR_LEN                3
> -#define SPI_FLASH_CMD_LEN            (1 + SPI_FLASH_3B_ADDR_LEN)
> +#define SPI_FLASH_4B_ADDR_LEN                4
> +#define SPI_FLASH_MAX_ADDR_WIDTH     4
>  #define SPI_FLASH_16MB_BOUN          0x1000000
>  
>  /* CFI Manufacture ID's */
> @@ -57,13 +58,30 @@ enum spi_nor_option_flags {
>  #define CMD_READ_DUAL_IO_FAST                0xbb
>  #define CMD_READ_QUAD_OUTPUT_FAST    0x6b
>  #define CMD_READ_QUAD_IO_FAST                0xeb
> +
> +/* 4B READ commands */
> +#define CMD_READ_ARRAY_SLOW_4B               0x13
> +#define CMD_READ_ARRAY_FAST_4B               0x0c
> +#define CMD_READ_DUAL_OUTPUT_FAST_4B 0x3c
> +#define CMD_READ_DUAL_IO_FAST_4B     0xbc
> +#define CMD_READ_QUAD_OUTPUT_FAST_4B 0x6c
> +#define CMD_READ_QUAD_IO_FAST_4B     0xec
> +
> +/* 4B Write commands */
> +#define CMD_PAGE_PROGRAM_4B          0x12
> +
> +/* 4B Erase commands */
> +#define CMD_ERASE_4K_4B                      0x21
> +#define CMD_ERASE_CHIP_4B            0x5c
> +#define CMD_ERASE_64K_4B             0xdc
> +
>  #define CMD_READ_ID                  0x9f
>  #define CMD_READ_STATUS                      0x05
>  #define CMD_READ_STATUS1             0x35
>  #define CMD_READ_CONFIG                      0x35
>  #define CMD_FLAG_STATUS                      0x70
> -#define CMD_EN4B                             0xB7
> -#define CMD_EX4B                             0xE9
> +#define CMD_EN4B                     0xB7
> +#define CMD_EX4B                     0xE9
>  
>  /* Bank addr access commands */
>  #ifdef CONFIG_SPI_FLASH_BAR
> @@ -133,6 +151,10 @@ struct spi_flash_info {
>  #define RD_DUAL                      BIT(5)  /* use Dual Read */
>  #define RD_QUADIO            BIT(6)  /* use Quad IO Read */
>  #define RD_DUALIO            BIT(7)  /* use Dual IO Read */
> +#define OPCODES_4B           BIT(8)  /*
> +                                      * Use dedicated 4byte address op codes
> +                                      * to support memory size above 128Mib.
> +                                      */
>  #define RD_FULL                      (RD_QUAD | RD_DUAL | RD_QUADIO | 
> RD_DUALIO)
>  };
>  
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 7581622..4d2e58e 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -22,12 +22,28 @@
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> -static void spi_flash_addr(u32 addr, u8 *cmd)
> +static void spi_flash_addr(struct spi_flash *flash, u32 addr, u8 *cmd)
>  {
>       /* cmd[0] is actual command */
> -     cmd[1] = addr >> 16;
> -     cmd[2] = addr >> 8;
> -     cmd[3] = addr >> 0;
> +
> +     switch (flash->addr_width) {
> +     case SPI_FLASH_3B_ADDR_LEN:
> +             cmd[1] = addr >> 16;
> +             cmd[2] = addr >> 8;
> +             cmd[3] = addr >> 0;
> +             break;
> +
> +     case SPI_FLASH_4B_ADDR_LEN:
> +             cmd[1] = addr >> 24;
> +             cmd[2] = addr >> 16;
> +             cmd[3] = addr >> 8;
> +             cmd[4] = addr >> 0;
> +             break;
> +
> +     default:
> +             debug("SF: Wrong opcode size\n");
> +             break;
> +     }

Not a big deal and mostly a question of personal taste but you can have a
look at what's done by the m25p80 driver from linux (m25p_addr2cmd()
function):

        cmd[1] = addr >> (flash->addr_width * 8 -  8);
        cmd[2] = addr >> (flash->addr_width * 8 - 16);
        cmd[3] = addr >> (flash->addr_width * 8 - 24);
        cmd[4] = addr >> (flash->addr_width * 8 - 32);

>  }
>  
>  static int read_sr(struct spi_flash *flash, u8 *rs)
> @@ -74,6 +90,64 @@ static int write_sr(struct spi_flash *flash, u8 ws)
>       return 0;
>  }
>  
> +static u8 spi_flash_convert_opcode(u8 opcode, const u8 table[][2], size_t 
> size)
> +{
> +     size_t i;
> +
> +     for (i = 0; i < size; i++)
> +             if (table[i][0] == opcode)
> +                     return table[i][1];
> +
> +     /* No conversion found, keep input op code. */
> +     return opcode;
> +}
> +
> +static inline u8 spi_flash_convert_3to4_read(u8 opcode)
> +{
> +     static const u8 spi_flash_3to4_read[][2] = {
> +             { CMD_READ_ARRAY_SLOW,          CMD_READ_ARRAY_SLOW_4B },
> +             { CMD_READ_ARRAY_FAST,          CMD_READ_ARRAY_FAST_4B },
> +             { CMD_READ_DUAL_OUTPUT_FAST,    CMD_READ_DUAL_OUTPUT_FAST_4B },
> +             { CMD_READ_DUAL_IO_FAST,        CMD_READ_DUAL_IO_FAST_4B },
> +             { CMD_READ_QUAD_OUTPUT_FAST,    CMD_READ_QUAD_OUTPUT_FAST_4B },
> +             { CMD_READ_QUAD_IO_FAST,        CMD_READ_QUAD_IO_FAST_4B },
> +
> +     };
> +
> +     return spi_flash_convert_opcode(opcode, spi_flash_3to4_read,
> +                                   ARRAY_SIZE(spi_flash_3to4_read));
> +}
> +
> +static inline u8 spi_flash_convert_3to4_program(u8 opcode)
> +{
> +     static const u8 spi_flash_3to4_program[][2] = {
> +             { CMD_PAGE_PROGRAM, CMD_PAGE_PROGRAM_4B },
> +     };
> +
> +     return spi_flash_convert_opcode(opcode, spi_flash_3to4_program,
> +                                   ARRAY_SIZE(spi_flash_3to4_program));
> +}
> +
> +static inline u8 spi_flash_convert_3to4_erase(u8 opcode)
> +{
> +     static const u8 spi_flash_3to4_erase[][2] = {
> +             { CMD_ERASE_4K,    CMD_ERASE_4K_4B },
> +             { CMD_ERASE_CHIP,  CMD_ERASE_CHIP_4B },
> +             { CMD_ERASE_64K,   CMD_ERASE_64K_4B },
> +     };
> +
> +     return spi_flash_convert_opcode(opcode, spi_flash_3to4_erase,
> +                                   ARRAY_SIZE(spi_flash_3to4_erase));
> +}
> +
> +static void spi_flash_set_4byte_opcodes(struct spi_flash *flash,
> +                                     const struct spi_flash_info *info)
> +{
> +     flash->read_cmd = spi_flash_convert_3to4_read(flash->read_cmd);
> +     flash->write_cmd = spi_flash_convert_3to4_program(flash->write_cmd);
> +     flash->erase_cmd = spi_flash_convert_3to4_erase(flash->erase_cmd);
> +}
> +
>  #if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
>  static int read_cr(struct spi_flash *flash, u8 *rc)
>  {
> @@ -364,7 +438,7 @@ int spi_flash_write_common(struct spi_flash *flash, const 
> u8 *cmd,
>  int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
>  {
>       u32 erase_size, erase_addr;
> -     u8 cmd[SPI_FLASH_CMD_LEN];
> +     u8 *cmd, cmdsz;
>       int ret = -1;
>  
>       erase_size = flash->erase_size;
> @@ -381,6 +455,13 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 
> offset, size_t len)
>               }
>       }
>  
> +     cmdsz = 1 + flash->addr_width;
> +     cmd = calloc(1, cmdsz);

calloc() but no free() ? Though we're not supposed to run u-boot forever,
it's still a memory leak. Why don't you simply keep u8
cmd[SPI_FLASH_CMD_LEN] as earlier, only changing the definition of
SPI_FLASH_CMD_LEN?

-#define SPI_FLASH_CMD_LEN              (1 + SPI_FLASH_3B_ADDR_LEN)
+#define SPI_FLASH_CMD_LEN              (1 + SPI_FLASH_4B_ADDR_LEN)

Of course, you still need cmdsz instead of sizeof(cmd) as you do below,
when calling spi_flash_write_common().

Best regards,

Cyrille

> +     if (!cmd) {
> +             debug("SF: Failed to allocate cmd\n");
> +             return -ENOMEM;
> +     }
> +
>       cmd[0] = flash->erase_cmd;
>       while (len) {
>               erase_addr = offset;
> @@ -394,12 +475,12 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, 
> u32 offset, size_t len)
>               if (ret < 0)
>                       return ret;
>  #endif
> -             spi_flash_addr(erase_addr, cmd);
> +             spi_flash_addr(flash, erase_addr, cmd);
>  
>               debug("SF: erase %2x %2x %2x %2x (%x)\n", cmd[0], cmd[1],
>                     cmd[2], cmd[3], erase_addr);
>  
> -             ret = spi_flash_write_common(flash, cmd, sizeof(cmd), NULL, 0);
> +             ret = spi_flash_write_common(flash, cmd, cmdsz, NULL, 0);
>               if (ret < 0) {
>                       debug("SF: erase failed\n");
>                       break;
> @@ -423,7 +504,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 
> offset,
>       unsigned long byte_addr, page_size;
>       u32 write_addr;
>       size_t chunk_len, actual;
> -     u8 cmd[SPI_FLASH_CMD_LEN];
> +     u8 *cmd, cmdsz;
>       int ret = -1;
>  
>       page_size = flash->page_size;
> @@ -436,6 +517,13 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 
> offset,
>               }
>       }
>  
> +     cmdsz = 1 + flash->addr_width;
> +     cmd = calloc(1, cmdsz);
> +     if (!cmd) {
> +             debug("SF: Failed to allocate cmd\n");
> +             return -ENOMEM;
> +     }
> +
>       cmd[0] = flash->write_cmd;
>       for (actual = 0; actual < len; actual += chunk_len) {
>               write_addr = offset;
> @@ -456,13 +544,13 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, 
> u32 offset,
>                       chunk_len = min(chunk_len,
>                                       (size_t)spi->max_write_size);
>  
> -             spi_flash_addr(write_addr, cmd);
> +             spi_flash_addr(flash, write_addr, cmd);
>  
>               debug("SF: 0x%p => cmd = { 0x%02x 0x%02x%02x%02x } chunk_len = 
> %zu\n",
>                     buf + actual, cmd[0], cmd[1], cmd[2], cmd[3], chunk_len);
>  
> -             ret = spi_flash_write_common(flash, cmd, sizeof(cmd),
> -                                     buf + actual, chunk_len);
> +             ret = spi_flash_write_common(flash, cmd, cmdsz,
> +                                          buf + actual, chunk_len);
>               if (ret < 0) {
>                       debug("SF: write failed\n");
>                       break;
> @@ -537,7 +625,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 
> offset,
>               return 0;
>       }
>  
> -     cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte;
> +     cmdsz = 1 + flash->addr_width + flash->dummy_byte;
>       cmd = calloc(1, cmdsz);
>       if (!cmd) {
>               debug("SF: Failed to allocate cmd\n");
> @@ -565,7 +653,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 
> offset,
>               else
>                       read_len = remain_len;
>  
> -             spi_flash_addr(read_addr, cmd);
> +             spi_flash_addr(flash, read_addr, cmd);
>  
>               ret = spi_flash_read_common(flash, cmd, cmdsz, data, read_len);
>               if (ret < 0) {
> @@ -1172,10 +1260,6 @@ int spi_flash_scan(struct spi_flash *flash)
>               flash->flags |= SNOR_F_USE_FSR;
>  #endif
>  
> -     /* disable 4-byte addressing if the device exceeds 16MiB */
> -     if (flash->size > SPI_FLASH_16MB_BOUN)
> -             set_4byte(flash, info, 0);
> -
>       /* Configure the BAR - discover bank cmds and read current bank */
>  #ifdef CONFIG_SPI_FLASH_BAR
>       ret = read_bar(flash, info);
> @@ -1211,5 +1295,23 @@ int spi_flash_scan(struct spi_flash *flash)
>       }
>  #endif
>  
> +     flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
> +
> +     if (flash->size > SPI_FLASH_16MB_BOUN) {
> +             /* enable 4-byte addressing if the device exceeds 16MiB */
> +             flash->addr_width = SPI_FLASH_4B_ADDR_LEN;
> +             if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SPANSION ||
> +                 info->flags & OPCODES_4B)
> +                     spi_flash_set_4byte_opcodes(flash, info);
> +             else
> +                     set_4byte(flash, info, 1);
> +     }
> +
> +     if (flash->addr_width > SPI_FLASH_MAX_ADDR_WIDTH) {
> +             dev_err(dev, "address width is too large: %u\n",
> +                     flash->addr_width);
> +             return -EINVAL;
> +     }
> +
>       return 0;
>  }
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> index be2fe3f..5a91671 100644
> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -48,6 +48,7 @@ struct spi_slave;
>   * @read_cmd:                Read cmd - Array Fast, Extn read and quad read.
>   * @write_cmd:               Write cmd - page and quad program.
>   * @dummy_byte:              Dummy cycles for read operation.
> + * @addr_width:              number of address bytes
>   * @memory_map:              Address of read-only SPI flash access
>   * @flash_lock:              lock a region of the SPI Flash
>   * @flash_unlock:    unlock a region of the SPI Flash
> @@ -84,6 +85,7 @@ struct spi_flash {
>       u8 write_cmd;
>       u8 dummy_byte;
>  
> +     u8 addr_width;
>       void *memory_map;
>  
>       int (*flash_lock)(struct spi_flash *flash, u32 ofs, size_t len);
> 

_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot

Reply via email to