[U-Boot] [PATCH v6 01/21] sf: Adopt flash table INFO macro from Linux

Siva Durga Prasad Paladugu siva.durga.paladugu at xilinx.com
Wed Nov 16 13:51:14 CET 2016


Hi,

> -----Original Message-----
> From: Jagan Teki [mailto:jagan at openedev.com]
> Sent: Wednesday, November 16, 2016 6:09 PM
> To: Siva Durga Prasad Paladugu <sivadur at xilinx.com>
> Cc: u-boot at lists.denx.de; Michal Simek <michal.simek at xilinx.com>
> Subject: Re: [U-Boot] [PATCH v6 01/21] sf: Adopt flash table INFO macro from
> Linux
> 
> On Wed, Nov 16, 2016 at 10:53 AM, Siva Durga Prasad Paladugu
> <siva.durga.paladugu at xilinx.com> wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Jagan Teki [mailto:jagan at openedev.com]
> >> Sent: Wednesday, November 16, 2016 9:33 AM
> >> To: u-boot at lists.denx.de
> >> Cc: Jagan Teki <jagan at openedev.com>; Simon Glass
> <sjg at chromium.org>;
> >> Bin Meng <bmeng.cn at gmail.com>; York Sun <york.sun at nxp.com>;
> Vignesh R
> >> <vigneshr at ti.com>; Mugunthan V N <mugunthanvnm at ti.com>; Michal
> Simek
> >> <michal.simek at xilinx.com>; Siva Durga Prasad Paladugu
> >> <sivadur at xilinx.com>
> >> Subject: [PATCH v6 01/21] sf: Adopt flash table INFO macro from Linux
> >>
> >> INFO macro make flash table entries more adjustable like adding new
> >> flash_info attributes, update ID length bytes and so on and more over
> >> it will sync to Linux way of defining flash_info attributes.
> >>
> >> - Add JEDEC_ID
> >> - Add JEDEC_EXT macro
> >> - Add JEDEC_MFR
> >> - spi_flash_params => spi_flash_info
> >> - params => info
> >>
> >> Cc: Simon Glass <sjg at chromium.org>
> >> Cc: Bin Meng <bmeng.cn at gmail.com>
> >> Cc: York Sun <york.sun at nxp.com>
> >> Cc: Vignesh R <vigneshr at ti.com>
> >> Cc: Mugunthan V N <mugunthanvnm at ti.com>
> >> Cc: Michal Simek <michal.simek at xilinx.com>
> >> Cc: Siva Durga Prasad Paladugu <sivadur at xilinx.com>
> >> Reviewed-by: Jagan Teki <jagan at openedev.com>
> >> Tested-by: Jagan Teki <jagan at openedev.com>
> >> Signed-off-by: Jagan Teki <jagan at openedev.com>
> >> ---
> >>  drivers/mtd/spi/sandbox.c     |  10 +-
> >>  drivers/mtd/spi/sf_internal.h |  26 +++--
> >>  drivers/mtd/spi/sf_params.c   | 217 ++++++++++++++++++++++---------------
> ----
> >> -
> >>  drivers/mtd/spi/spi_flash.c   | 136 +++++++++++++-------------
> >>  include/linux/err.h           |   5 +
> >>  5 files changed, 214 insertions(+), 180 deletions(-)
> >>
> >> diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
> >> index f59134f..d68ee4a 100644
> >> --- a/drivers/mtd/spi/sandbox.c
> >> +++ b/drivers/mtd/spi/sandbox.c
> >> @@ -88,7 +88,7 @@ struct sandbox_spi_flash {
> >>       /* The current flash status (see STAT_XXX defines above) */
> >>       u16 status;
> >>       /* Data describing the flash we're emulating */
> >> -     const struct spi_flash_params *data;
> >> +     const struct spi_flash_info *data;
> >>       /* The file on disk to serv up data from */
> >>       int fd;
> >>  };
> >> @@ -112,7 +112,7 @@ static int sandbox_sf_probe(struct udevice *dev)
> >>       struct sandbox_spi_flash *sbsf = dev_get_priv(dev);
> >>       const char *file;
> >>       size_t len, idname_len;
> >> -     const struct spi_flash_params *data;
> >> +     const struct spi_flash_info *data;
> >>       struct sandbox_spi_flash_plat_data *pdata = dev_get_platdata(dev);
> >>       struct sandbox_state *state = state_get_current();
> >>       struct udevice *bus = dev->parent; @@ -168,7 +168,7 @@ static
> >> int sandbox_sf_probe(struct udevice *dev)
> >>       }
> >>       debug("%s: device='%s'\n", __func__, spec);
> >>
> >> -     for (data = spi_flash_params_table; data->name; data++) {
> >> +     for (data = spi_flash_ids; data->name; data++) {
> >>               len = strlen(data->name);
> >>               if (idname_len != len)
> >>                       continue;
> >> @@ -359,7 +359,9 @@ static int sandbox_sf_xfer(struct udevice *dev,
> >> unsigned int bitlen,
> >>                       debug(" id: off:%u tx:", sbsf->off);
> >>                       if (sbsf->off < IDCODE_LEN) {
> >>                               /* Extract correct byte from ID 0x00aabbcc */
> >> -                             id = sbsf->data->jedec >>
> >> +                             id = ((((sbsf->data)->id[0]) << 16) |
> >> +                                     (((sbsf->data)->id[1]) << 8 |
> >> +                                     ((sbsf->data)->id[2]))) >>
> >>                                       (8 * (IDCODE_LEN - 1 -
> >> sbsf->off));
> > Please, no magic 16 and 8 here and everywhere
> 
> These are existing macro exapnsions will update on future if required.
Its upto you.
> 
> >>                       } else {
> >>                               id = 0; diff --git
> >> a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index
> >> cde4cfb..a9455ac 100644
> >> --- a/drivers/mtd/spi/sf_internal.h
> >> +++ b/drivers/mtd/spi/sf_internal.h
> >> @@ -103,24 +103,36 @@ int sst_write_bp(struct spi_flash *flash, u32
> >> offset, size_t len,
> >>  #define CMD_SPANSION_RDAR    0x65 /* Read any device register */
> >>  #define CMD_SPANSION_WRAR    0x71 /* Write any device register */
> >>  #endif
> >> +
> >> +#define JEDEC_MFR(info)              ((info)->id[0])
> >> +#define JEDEC_ID(info)               (((info)->id[1]) << 8 | ((info)->id[2]))
> >> +#define JEDEC_EXT(info)              (((info)->id[3]) << 8 | ((info)->id[4]))
> >> +
> >>  /**
> >> - * struct spi_flash_params - SPI/QSPI flash device params structure
> >> + * struct spi_flash_info - SPI/QSPI flash device params structure
> >>   *
> >>   * @name:            Device name
> >> ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO])
> >> - * @jedec:           Device jedec ID (0x[1byte_manuf_id][2byte_dev_id])
> >> - * @ext_jedec:               Device ext_jedec ID
> >>   * @sector_size:     Isn't necessarily a sector size from vendor,
> >>   *                   the size listed here is what works with
> >> CMD_ERASE_64K
> >>   * @nr_sectors:              No.of sectors on this device
> >>   * @flags:           Important param, for flash specific behaviour
> >>   */
> >> -struct spi_flash_params {
> >> +struct spi_flash_info {
> >>       const char *name;
> >> -     u32 jedec;
> >> -     u16 ext_jedec;
> >> +
> >> +     /*
> >> +      * This array stores the ID bytes.
> >> +      * The first three bytes are the JEDIC ID.
> >> +      * JEDEC ID zero means "no ID" (mostly older chips).
> >> +      */
> >> +     u8              id[5];
> >> +     u8              id_len;
> >> +
> >>       u32 sector_size;
> >>       u32 nr_sectors;
> >>
> >> +     u16 page_size;
> >> +
> >>       u16 flags;
> >>  #define SECT_4K                      BIT(0)
> >>  #define E_FSR                        BIT(1)
> >> @@ -133,7 +145,7 @@ struct spi_flash_params {
> >>  #define RD_FULL                      (RD_QUAD | RD_DUAL | RD_QUADIO
> >> | RD_DUALIO)
> >>  };
> >>
> >> -extern const struct spi_flash_params spi_flash_params_table[];
> >> +extern const struct spi_flash_info spi_flash_ids[];
> >>
> >>  /* Send a single-byte command to the device and read the response */
> >> int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response,
> >> size_t len); diff - -git a/drivers/mtd/spi/sf_params.c
> >> b/drivers/mtd/spi/sf_params.c index 5b50114..7fcc3bc 100644
> >> --- a/drivers/mtd/spi/sf_params.c
> >> +++ b/drivers/mtd/spi/sf_params.c
> >> @@ -12,125 +12,140 @@
> >>
> >>  #include "sf_internal.h"
> >>
> >> +/* Used when the "_ext_id" is two bytes at most */
> >> +#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)   \
> >> +             .id = {                                                 \
> >> +                     ((_jedec_id) >> 16) & 0xff,                     \
> >> +                     ((_jedec_id) >> 8) & 0xff,                      \
> >> +                     (_jedec_id) & 0xff,                             \
> >> +                     ((_ext_id) >> 8) & 0xff,                        \
> >> +                     (_ext_id) & 0xff,                               \
> >> +                     },                                              \
> >> +             .id_len = (!(_jedec_id) ? 0 : (3 + ((_ext_id) ? 2 : 0))),       \
> >> +             .sector_size = (_sector_size),                          \
> >> +             .nr_sectors = (_n_sectors),                             \
> >> +             .page_size = 256,                                       \
> >> +             .flags = (_flags),
> >> +
> > Is it just the default page size? Because, there are some Spansion
> > parts with 512 bytes page size, I hope we are taking care of it in runtime
> and it is just the default one you are filling here, please confirm.
> >
> >>  /* SPI/QSPI flash device params structure */ -const struct
> >> spi_flash_params spi_flash_params_table[] = {
> >> +const struct spi_flash_info spi_flash_ids[] = {
> >>  #ifdef CONFIG_SPI_FLASH_ATMEL                /* ATMEL */
> >> -     {"AT45DB011D",     0x1f2200, 0x0,       64 * 1024,     4,
> >> SECT_4K},
> >> -     {"AT45DB021D",     0x1f2300, 0x0,       64 * 1024,     8,
> >> SECT_4K},
> >> -     {"AT45DB041D",     0x1f2400, 0x0,       64 * 1024,     8,
> >> SECT_4K},
> >> -     {"AT45DB081D",     0x1f2500, 0x0,       64 * 1024,    16,
> >> SECT_4K},
> >> -     {"AT45DB161D",     0x1f2600, 0x0,       64 * 1024,    32,
> >> SECT_4K},
> >> -     {"AT45DB321D",     0x1f2700, 0x0,       64 * 1024,    64,
> >> SECT_4K},
> >> -     {"AT45DB641D",     0x1f2800, 0x0,       64 * 1024,   128,
> >> SECT_4K},
> >> -     {"AT25DF321A",     0x1f4701, 0x0,       64 * 1024,    64, SECT_4K},
> >> -     {"AT25DF321",      0x1f4700, 0x0,       64 * 1024,    64, SECT_4K},
> >> -     {"AT26DF081A",     0x1f4501, 0x0,       64 * 1024,    16, SECT_4K},
> >> +     {"AT45DB011D",     INFO(0x1f2200, 0x0, 64 * 1024,     4,
> >> SECT_4K) },
> >> +     {"AT45DB021D",     INFO(0x1f2300, 0x0, 64 * 1024,     8,
> >> SECT_4K) },
> >> +     {"AT45DB041D",     INFO(0x1f2400, 0x0, 64 * 1024,     8,
> >> SECT_4K) },
> >> +     {"AT45DB081D",     INFO(0x1f2500, 0x0, 64 * 1024,    16,
> >> SECT_4K) },
> >> +     {"AT45DB161D",     INFO(0x1f2600, 0x0, 64 * 1024,    32,
> >> SECT_4K) },
> >> +     {"AT45DB321D",     INFO(0x1f2700, 0x0, 64 * 1024,    64,
> >> SECT_4K) },
> >> +     {"AT45DB641D",     INFO(0x1f2800, 0x0, 64 * 1024,   128,
> >> SECT_4K) },
> >> +     {"AT25DF321A",     INFO(0x1f4701, 0x0, 64 * 1024,    64, SECT_4K) },
> >> +     {"AT25DF321",      INFO(0x1f4700, 0x0, 64 * 1024,    64, SECT_4K) },
> >> +     {"AT26DF081A",     INFO(0x1f4501, 0x0, 64 * 1024,    16, SECT_4K) },
> >>  #endif
> >>  #ifdef CONFIG_SPI_FLASH_EON          /* EON */
> >> -     {"EN25Q32B",       0x1c3016, 0x0,       64 * 1024,    64, 0},
> >> -     {"EN25Q64",        0x1c3017, 0x0,       64 * 1024,   128, SECT_4K},
> >> -     {"EN25Q128B",      0x1c3018, 0x0,       64 * 1024,   256, 0},
> >> -     {"EN25S64",        0x1c3817, 0x0,       64 * 1024,   128, 0},
> >> +     {"EN25Q32B",       INFO(0x1c3016, 0x0, 64 * 1024,    64, 0) },
> >> +     {"EN25Q64",        INFO(0x1c3017, 0x0, 64 * 1024,   128, SECT_4K) },
> >> +     {"EN25Q128B",      INFO(0x1c3018, 0x0, 64 * 1024,   256, 0) },
> >> +     {"EN25S64",        INFO(0x1c3817, 0x0, 64 * 1024,   128, 0) },
> >>  #endif
> >>  #ifdef CONFIG_SPI_FLASH_GIGADEVICE   /* GIGADEVICE */
> >> -     {"GD25Q64B",       0xc84017, 0x0,       64 * 1024,   128, SECT_4K},
> >> -     {"GD25LQ32",       0xc86016, 0x0,       64 * 1024,    64, SECT_4K},
> >> +     {"GD25Q64B",       INFO(0xc84017, 0x0, 64 * 1024,   128, SECT_4K) },
> >> +     {"GD25LQ32",       INFO(0xc86016, 0x0, 64 * 1024,    64, SECT_4K) },
> >>  #endif
> >>  #ifdef CONFIG_SPI_FLASH_ISSI         /* ISSI */
> >> -     {"IS25LP032",      0x9d6016, 0x0,       64 * 1024,    64, 0},
> >> -     {"IS25LP064",      0x9d6017, 0x0,       64 * 1024,   128, 0},
> >> -     {"IS25LP128",      0x9d6018, 0x0,       64 * 1024,   256, 0},
> >> +     {"IS25LP032",      INFO(0x9d6016, 0x0, 64 * 1024,    64, 0) },
> >> +     {"IS25LP064",      INFO(0x9d6017, 0x0, 64 * 1024,   128, 0) },
> >> +     {"IS25LP128",      INFO(0x9d6018, 0x0, 64 * 1024,   256, 0) },
> >>  #endif
> >>  #ifdef CONFIG_SPI_FLASH_MACRONIX     /* MACRONIX */
> >> -     {"MX25L2006E",     0xc22012, 0x0,       64 * 1024,     4, 0},
> >> -     {"MX25L4005",      0xc22013, 0x0,       64 * 1024,     8, 0},
> >> -     {"MX25L8005",      0xc22014, 0x0,       64 * 1024,    16, 0},
> >> -     {"MX25L1605D",     0xc22015, 0x0,       64 * 1024,    32, 0},
> >> -     {"MX25L3205D",     0xc22016, 0x0,       64 * 1024,    64, 0},
> >> -     {"MX25L6405D",     0xc22017, 0x0,       64 * 1024,   128, 0},
> >> -     {"MX25L12805",     0xc22018, 0x0,       64 * 1024,   256,
> >> RD_FULL | WR_QPP},
> >> -     {"MX25L25635F",    0xc22019, 0x0,       64 * 1024,   512,
> >> RD_FULL | WR_QPP},
> >> -     {"MX25L51235F",    0xc2201a, 0x0,       64 * 1024,  1024,
> >> RD_FULL | WR_QPP},
> >> -     {"MX25L12855E",    0xc22618, 0x0,       64 * 1024,   256,
> >> RD_FULL | WR_QPP},
> >> +     {"MX25L2006E",     INFO(0xc22012, 0x0, 64 * 1024,     4, 0) },
> >> +     {"MX25L4005",      INFO(0xc22013, 0x0, 64 * 1024,     8, 0) },
> >> +     {"MX25L8005",      INFO(0xc22014, 0x0, 64 * 1024,    16, 0) },
> >> +     {"MX25L1605D",     INFO(0xc22015, 0x0, 64 * 1024,    32, 0) },
> >> +     {"MX25L3205D",     INFO(0xc22016, 0x0, 64 * 1024,    64, 0) },
> >> +     {"MX25L6405D",     INFO(0xc22017, 0x0, 64 * 1024,   128, 0) },
> >> +     {"MX25L12805",     INFO(0xc22018, 0x0, 64 * 1024,   256,
> >> RD_FULL | WR_QPP) },
> >> +     {"MX25L25635F",    INFO(0xc22019, 0x0, 64 * 1024,   512,
> >> RD_FULL | WR_QPP) },
> >> +     {"MX25L51235F",    INFO(0xc2201a, 0x0, 64 * 1024,  1024,
> >> RD_FULL | WR_QPP) },
> >> +     {"MX25L12855E",    INFO(0xc22618, 0x0, 64 * 1024,   256,
> >> RD_FULL | WR_QPP) },
> >>  #endif
> >>  #ifdef CONFIG_SPI_FLASH_SPANSION     /* SPANSION */
> >> -     {"S25FL008A",      0x010213, 0x0,       64 * 1024,    16, 0},
> >> -     {"S25FL016A",      0x010214, 0x0,       64 * 1024,    32, 0},
> >> -     {"S25FL032A",      0x010215, 0x0,       64 * 1024,    64, 0},
> >> -     {"S25FL064A",      0x010216, 0x0,       64 * 1024,   128, 0},
> >> -     {"S25FL116K",      0x014015, 0x0,       64 * 1024,   128, 0},
> >> -     {"S25FL164K",      0x014017, 0x0140,    64 * 1024,   128, 0},
> >> -     {"S25FL128P_256K", 0x012018, 0x0300,   256 * 1024,    64, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL128P_64K",  0x012018, 0x0301,    64 * 1024,   256, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL032P",      0x010215, 0x4d00,    64 * 1024,    64, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL064P",      0x010216, 0x4d00,    64 * 1024,   128, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL128S_256K", 0x012018, 0x4d00,   256 * 1024,    64, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL128S_64K",  0x012018, 0x4d01,    64 * 1024,   256, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL256S_256K", 0x010219, 0x4d00,   256 * 1024,   128, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL256S_64K",  0x010219, 0x4d01,    64 * 1024,   512, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FS512S",      0x010220, 0x4D00,   128 * 1024,   512, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL512S_256K", 0x010220, 0x4d00,   256 * 1024,   256, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL512S_64K",  0x010220, 0x4d01,    64 * 1024,  1024, RD_FULL |
> >> WR_QPP},
> >> -     {"S25FL512S_512K", 0x010220, 0x4f00,   256 * 1024,   256, RD_FULL |
> >> WR_QPP},
> >> +     {"S25FL008A",      INFO(0x010213, 0x0, 64 * 1024,    16, 0) },
> >> +     {"S25FL016A",      INFO(0x010214, 0x0, 64 * 1024,    32, 0) },
> >> +     {"S25FL032A",      INFO(0x010215, 0x0, 64 * 1024,    64, 0) },
> >> +     {"S25FL064A",      INFO(0x010216, 0x0, 64 * 1024,   128, 0) },
> >> +     {"S25FL116K",      INFO(0x014015, 0x0, 64 * 1024,   128, 0) },
> >> +     {"S25FL164K",      INFO(0x014017, 0x0140,  64 * 1024,   128, 0) },
> >> +     {"S25FL128P_256K", INFO(0x012018, 0x0300, 256 * 1024,    64,
> >> RD_FULL | WR_QPP) },
> >> +     {"S25FL128P_64K",  INFO(0x012018, 0x0301,  64 * 1024,   256,
> >> RD_FULL | WR_QPP) },
> >> +     {"S25FL032P",      INFO(0x010215, 0x4d00,  64 * 1024,    64, RD_FULL |
> >> WR_QPP) },
> >> +     {"S25FL064P",      INFO(0x010216, 0x4d00,  64 * 1024,   128, RD_FULL
> >> | WR_QPP) },
> >> +     {"S25FL128S_256K", INFO(0x012018, 0x4d00, 256 * 1024,    64,
> >> RD_FULL | WR_QPP) },
> >> +     {"S25FL128S_64K",  INFO(0x012018, 0x4d01,  64 * 1024,   256,
> >> RD_FULL | WR_QPP) },
> >> +     {"S25FL256S_256K", INFO(0x010219, 0x4d00, 256 * 1024,   128,
> >> RD_FULL | WR_QPP) },
> >> +     {"S25FL256S_64K",  INFO(0x010219, 0x4d01,  64 * 1024,   512,
> >> RD_FULL | WR_QPP) },
> >> +     {"S25FS512S",      INFO(0x010220, 0x4D00, 128 * 1024,   512, RD_FULL
> >> | WR_QPP) },
> >> +     {"S25FL512S_256K", INFO(0x010220, 0x4d00, 256 * 1024,   256,
> >> RD_FULL | WR_QPP) },
> >> +     {"S25FL512S_64K",  INFO(0x010220, 0x4d01,  64 * 1024,  1024,
> >> RD_FULL | WR_QPP) },
> >> +     {"S25FL512S_512K", INFO(0x010220, 0x4f00, 256 * 1024,   256,
> >> RD_FULL | WR_QPP) },
> >>  #endif
> >>  #ifdef CONFIG_SPI_FLASH_STMICRO              /* STMICRO */
> >> -     {"M25P10",         0x202011, 0x0,       32 * 1024,     4, 0},
> >> -     {"M25P20",         0x202012, 0x0,       64 * 1024,     4, 0},
> >> -     {"M25P40",         0x202013, 0x0,       64 * 1024,     8, 0},
> >> -     {"M25P80",         0x202014, 0x0,       64 * 1024,    16, 0},
> >> -     {"M25P16",         0x202015, 0x0,       64 * 1024,    32, 0},
> >> -     {"M25PE16",        0x208015, 0x1000,    64 * 1024,    32, 0},
> >> -     {"M25PX16",        0x207115, 0x1000,    64 * 1024,    32, RD_QUAD |
> >> RD_DUAL},
> >> -     {"M25P32",         0x202016, 0x0,       64 * 1024,    64, 0},
> >> -     {"M25P64",         0x202017, 0x0,       64 * 1024,   128, 0},
> >> -     {"M25P128",        0x202018, 0x0,      256 * 1024,    64, 0},
> >> -     {"M25PX64",        0x207117, 0x0,       64 * 1024,   128, SECT_4K},
> >> -     {"N25Q016A",       0x20bb15, 0x0,       64 * 1024,    32, SECT_4K},
> >> -     {"N25Q32",         0x20ba16, 0x0,       64 * 1024,    64, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"N25Q32A",        0x20bb16, 0x0,       64 * 1024,    64, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"N25Q64",         0x20ba17, 0x0,       64 * 1024,   128, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"N25Q64A",        0x20bb17, 0x0,       64 * 1024,   128, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"N25Q128",        0x20ba18, 0x0,       64 * 1024,   256, RD_FULL |
> >> WR_QPP},
> >> -     {"N25Q128A",       0x20bb18, 0x0,       64 * 1024,   256, RD_FULL |
> >> WR_QPP},
> >> -     {"N25Q256",        0x20ba19, 0x0,       64 * 1024,   512, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"N25Q256A",       0x20bb19, 0x0,       64 * 1024,   512, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"N25Q512",        0x20ba20, 0x0,       64 * 1024,  1024, RD_FULL |
> >> WR_QPP | E_FSR | SECT_4K},
> >> -     {"N25Q512A",       0x20bb20, 0x0,       64 * 1024,  1024, RD_FULL |
> >> WR_QPP | E_FSR | SECT_4K},
> >> -     {"N25Q1024",       0x20ba21, 0x0,       64 * 1024,  2048, RD_FULL |
> >> WR_QPP | E_FSR | SECT_4K},
> >> -     {"N25Q1024A",      0x20bb21, 0x0,       64 * 1024,  2048, RD_FULL |
> >> WR_QPP | E_FSR | SECT_4K},
> >> +     {"M25P10",         INFO(0x202011, 0x0, 32 * 1024,     4, 0) },
> >> +     {"M25P20",         INFO(0x202012, 0x0, 64 * 1024,     4, 0) },
> >> +     {"M25P40",         INFO(0x202013, 0x0, 64 * 1024,     8, 0) },
> >> +     {"M25P80",         INFO(0x202014, 0x0, 64 * 1024,    16, 0) },
> >> +     {"M25P16",         INFO(0x202015, 0x0, 64 * 1024,    32, 0) },
> >> +     {"M25PE16",        INFO(0x208015, 0x1000, 64 * 1024, 32, 0) },
> >> +     {"M25PX16",        INFO(0x207115, 0x1000, 64 * 1024, 32, RD_QUAD |
> >> RD_DUAL) },
> >> +     {"M25P32",         INFO(0x202016, 0x0,  64 * 1024,    64, 0) },
> >> +     {"M25P64",         INFO(0x202017, 0x0,  64 * 1024,   128, 0) },
> >> +     {"M25P128",        INFO(0x202018, 0x0, 256 * 1024,    64, 0) },
> >> +     {"M25PX64",        INFO(0x207117, 0x0,  64 * 1024,   128, SECT_4K) },
> >> +     {"N25Q016A",       INFO(0x20bb15, 0x0,  64 * 1024,    32, SECT_4K) },
> >> +     {"N25Q32",         INFO(0x20ba16, 0x0,  64 * 1024,    64, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"N25Q32A",        INFO(0x20bb16, 0x0,  64 * 1024,    64, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"N25Q64",         INFO(0x20ba17, 0x0,  64 * 1024,   128, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"N25Q64A",        INFO(0x20bb17, 0x0,  64 * 1024,   128, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"N25Q128",        INFO(0x20ba18, 0x0,  64 * 1024,   256, RD_FULL |
> >> WR_QPP) },
> >> +     {"N25Q128A",       INFO(0x20bb18, 0x0,  64 * 1024,   256, RD_FULL |
> >> WR_QPP) },
> >> +     {"N25Q256",        INFO(0x20ba19, 0x0,  64 * 1024,   512, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"N25Q256A",       INFO(0x20bb19, 0x0,  64 * 1024,   512, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"N25Q512",        INFO(0x20ba20, 0x0,  64 * 1024,  1024, RD_FULL |
> >> WR_QPP | E_FSR | SECT_4K) },
> >> +     {"N25Q512A",       INFO(0x20bb20, 0x0,  64 * 1024,  1024, RD_FULL |
> >> WR_QPP | E_FSR | SECT_4K) },
> >> +     {"N25Q1024",       INFO(0x20ba21, 0x0,  64 * 1024,  2048, RD_FULL |
> >> WR_QPP | E_FSR | SECT_4K) },
> >> +     {"N25Q1024A",      INFO(0x20bb21, 0x0,  64 * 1024,  2048, RD_FULL |
> >> WR_QPP | E_FSR | SECT_4K) },
> >>  #endif
> >>  #ifdef CONFIG_SPI_FLASH_SST          /* SST */
> >> -     {"SST25VF040B",    0xbf258d, 0x0,       64 * 1024,     8,
> >> SECT_4K | SST_WR},
> >> -     {"SST25VF080B",    0xbf258e, 0x0,       64 * 1024,    16,
> >> SECT_4K | SST_WR},
> >> -     {"SST25VF016B",    0xbf2541, 0x0,       64 * 1024,    32,
> >> SECT_4K | SST_WR},
> >> -     {"SST25VF032B",    0xbf254a, 0x0,       64 * 1024,    64,
> >> SECT_4K | SST_WR},
> >> -     {"SST25VF064C",    0xbf254b, 0x0,       64 * 1024,   128,
> >> SECT_4K},
> >> -     {"SST25WF512",     0xbf2501, 0x0,       64 * 1024,     1,
> >> SECT_4K | SST_WR},
> >> -     {"SST25WF010",     0xbf2502, 0x0,       64 * 1024,     2,
> >> SECT_4K | SST_WR},
> >> -     {"SST25WF020",     0xbf2503, 0x0,       64 * 1024,     4,
> >> SECT_4K | SST_WR},
> >> -     {"SST25WF040",     0xbf2504, 0x0,       64 * 1024,     8,
> >> SECT_4K | SST_WR},
> >> -     {"SST25WF040B",    0x621613, 0x0,       64 * 1024,     8,
> >> SECT_4K},
> >> -     {"SST25WF080",     0xbf2505, 0x0,       64 * 1024,    16,
> >> SECT_4K | SST_WR},
> >> +     {"SST25VF040B",    INFO(0xbf258d, 0x0,  64 * 1024,     8,
> >> SECT_4K | SST_WR) },
> >> +     {"SST25VF080B",    INFO(0xbf258e, 0x0,  64 * 1024,    16,
> >> SECT_4K | SST_WR) },
> >> +     {"SST25VF016B",    INFO(0xbf2541, 0x0,  64 * 1024,    32,
> >> SECT_4K | SST_WR) },
> >> +     {"SST25VF032B",    INFO(0xbf254a, 0x0,  64 * 1024,    64,
> >> SECT_4K | SST_WR) },
> >> +     {"SST25VF064C",    INFO(0xbf254b, 0x0,  64 * 1024,   128,
> >> SECT_4K) },
> >> +     {"SST25WF512",     INFO(0xbf2501, 0x0,  64 * 1024,     1,
> >> SECT_4K | SST_WR) },
> >> +     {"SST25WF010",     INFO(0xbf2502, 0x0,  64 * 1024,     2,
> >> SECT_4K | SST_WR) },
> >> +     {"SST25WF020",     INFO(0xbf2503, 0x0,  64 * 1024,     4,
> >> SECT_4K | SST_WR) },
> >> +     {"SST25WF040",     INFO(0xbf2504, 0x0,  64 * 1024,     8,
> >> SECT_4K | SST_WR) },
> >> +     {"SST25WF040B",    INFO(0x621613, 0x0,  64 * 1024,     8,
> >> SECT_4K) },
> >> +     {"SST25WF080",     INFO(0xbf2505, 0x0,  64 * 1024,    16,
> >> SECT_4K | SST_WR) },
> >>  #endif
> >>  #ifdef CONFIG_SPI_FLASH_WINBOND              /* WINBOND */
> >> -     {"W25P80",         0xef2014, 0x0,       64 * 1024,    16, 0},
> >> -     {"W25P16",         0xef2015, 0x0,       64 * 1024,    32, 0},
> >> -     {"W25P32",         0xef2016, 0x0,       64 * 1024,    64, 0},
> >> -     {"W25X40",         0xef3013, 0x0,       64 * 1024,     8, SECT_4K},
> >> -     {"W25X16",         0xef3015, 0x0,       64 * 1024,    32, SECT_4K},
> >> -     {"W25X32",         0xef3016, 0x0,       64 * 1024,    64, SECT_4K},
> >> -     {"W25X64",         0xef3017, 0x0,       64 * 1024,   128, SECT_4K},
> >> -     {"W25Q80BL",       0xef4014, 0x0,       64 * 1024,    16, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q16CL",       0xef4015, 0x0,       64 * 1024,    32, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q32BV",       0xef4016, 0x0,       64 * 1024,    64, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q64CV",       0xef4017, 0x0,       64 * 1024,   128, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q128BV",      0xef4018, 0x0,       64 * 1024,   256, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q256",        0xef4019, 0x0,       64 * 1024,   512, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q80BW",       0xef5014, 0x0,       64 * 1024,    16, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q16DW",       0xef6015, 0x0,       64 * 1024,    32, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q32DW",       0xef6016, 0x0,       64 * 1024,    64, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q64DW",       0xef6017, 0x0,       64 * 1024,   128, RD_FULL |
> >> WR_QPP | SECT_4K},
> >> -     {"W25Q128FW",      0xef6018, 0x0,       64 * 1024,   256,
> >> RD_FULL | WR_QPP | SECT_4K},
> >> +     {"W25P80",         INFO(0xef2014, 0x0,  64 * 1024,    16, 0) },
> >> +     {"W25P16",         INFO(0xef2015, 0x0,  64 * 1024,    32, 0) },
> >> +     {"W25P32",         INFO(0xef2016, 0x0,  64 * 1024,    64, 0) },
> >> +     {"W25X40",         INFO(0xef3013, 0x0,  64 * 1024,     8, SECT_4K) },
> >> +     {"W25X16",         INFO(0xef3015, 0x0,  64 * 1024,    32, SECT_4K) },
> >> +     {"W25X32",         INFO(0xef3016, 0x0,  64 * 1024,    64, SECT_4K) },
> >> +     {"W25X64",         INFO(0xef3017, 0x0,  64 * 1024,   128, SECT_4K) },
> >> +     {"W25Q80BL",       INFO(0xef4014, 0x0,  64 * 1024,    16, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q16CL",       INFO(0xef4015, 0x0,  64 * 1024,    32, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q32BV",       INFO(0xef4016, 0x0,  64 * 1024,    64, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q64CV",       INFO(0xef4017, 0x0,  64 * 1024,   128, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q128BV",      INFO(0xef4018, 0x0,  64 * 1024,   256, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q256",        INFO(0xef4019, 0x0,  64 * 1024,   512, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q80BW",       INFO(0xef5014, 0x0,  64 * 1024,    16, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q16DW",       INFO(0xef6015, 0x0,  64 * 1024,    32, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q32DW",       INFO(0xef6016, 0x0,  64 * 1024,    64, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q64DW",       INFO(0xef6017, 0x0,  64 * 1024,   128, RD_FULL |
> >> WR_QPP | SECT_4K) },
> >> +     {"W25Q128FW",      INFO(0xef6018, 0x0,  64 * 1024,   256,
> >> RD_FULL | WR_QPP | SECT_4K) },
> >>  #endif
> >>       {},     /* Empty entry to terminate the list */
> >>       /*
> >> diff --git a/drivers/mtd/spi/spi_flash.c
> >> b/drivers/mtd/spi/spi_flash.c index 7f6e9ae..95ee5ac 100644
> >> --- a/drivers/mtd/spi/spi_flash.c
> >> +++ b/drivers/mtd/spi/spi_flash.c
> >> @@ -165,7 +165,8 @@ bar_end:
> >>       return flash->bank_curr;
> >>  }
> >>
> >> -static int spi_flash_read_bar(struct spi_flash *flash, u8 idcode0)
> >> +static int spi_flash_read_bar(struct spi_flash *flash,
> >> +                           const struct spi_flash_info *info)
> >>  {
> >>       u8 curr_bank = 0;
> >>       int ret;
> >> @@ -173,7 +174,7 @@ static int spi_flash_read_bar(struct spi_flash
> >> *flash,
> >> u8 idcode0)
> >>       if (flash->size <= SPI_FLASH_16MB_BOUN)
> >>               goto bar_end;
> >>
> >> -     switch (idcode0) {
> >> +     switch (JEDEC_MFR(info)) {
> >>       case SPI_FLASH_CFI_MFR_SPANSION:
> >>               flash->bank_read_cmd = CMD_BANKADDR_BRRD;
> >>               flash->bank_write_cmd = CMD_BANKADDR_BRWR; @@ -
> >> 924,9 +925,35 @@ static int micron_quad_enable(struct spi_flash
> >> *flash)  } #endif
> >>
> >> -static int set_quad_mode(struct spi_flash *flash, u8 idcode0)
> >> +static const struct spi_flash_info *spi_flash_read_id(struct
> >> +spi_flash
> >> +*flash)
> >>  {
> >> -     switch (idcode0) {
> >> +     int                             tmp;
> >> +     u8                              id[5];
> >> +     const struct spi_flash_info     *info;
> >> +
> >> +     tmp = spi_flash_cmd(flash->spi, CMD_READ_ID, id, 5);
> >> +     if (tmp < 0) {
> >> +             printf("SF: error %d reading JEDEC ID\n", tmp);
> >> +             return ERR_PTR(tmp);
> >> +     }
> >> +
> >> +     info = spi_flash_ids;
> >> +     for (; info->name != NULL; info++) {
> >> +             if (info->id_len) {
> >> +                     if (!memcmp(info->id, id, info->id_len))
> >> +                             return info;
> >> +             }
> >> +     }
> >> +
> >> +     printf("SF: unrecognized JEDEC id bytes: %02x, %02x, %02x\n",
> >> +            id[0], id[1], id[2]);
> >> +     return ERR_PTR(-ENODEV);
> >> +}
> >> +
> >> +static int set_quad_mode(struct spi_flash *flash,
> >> +                      const struct spi_flash_info *info) {
> >> +     switch (JEDEC_MFR(info)) {
> >>  #ifdef CONFIG_SPI_FLASH_MACRONIX
> >>       case SPI_FLASH_CFI_MFR_MACRONIX:
> >>               return macronix_quad_enable(flash); @@ -941,7 +968,8 @@
> >> static int set_quad_mode(struct spi_flash *flash, u8
> >> idcode0)
> >>               return micron_quad_enable(flash);  #endif
> >>       default:
> >> -             printf("SF: Need set QEB func for %02x flash\n", idcode0);
> >> +             printf("SF: Need set QEB func for %02x flash\n",
> >> +                    JEDEC_MFR(info));
> >>               return -1;
> >>       }
> >>  }
> >> @@ -1011,45 +1039,12 @@ static int
> >> spansion_s25fss_disable_4KB_erase(struct spi_slave *spi)  int
> >> spi_flash_scan(struct spi_flash *flash)  {
> >>       struct spi_slave *spi = flash->spi;
> >> -     const struct spi_flash_params *params;
> >> -     u16 jedec, ext_jedec;
> >> -     u8 idcode[5];
> >> -     int ret;
> >> -
> >> -     /* Read the ID codes */
> >> -     ret = spi_flash_cmd(spi, CMD_READ_ID, idcode, sizeof(idcode));
> >> -     if (ret) {
> >> -             printf("SF: Failed to get idcodes\n");
> >> -             return ret;
> >> -     }
> >> -
> >> -#ifdef DEBUG
> >> -     printf("SF: Got idcodes\n");
> >> -     print_buffer(0, idcode, 1, sizeof(idcode), 0);
> >> -#endif
> >> -
> >> -     jedec = idcode[1] << 8 | idcode[2];
> >> -     ext_jedec = idcode[3] << 8 | idcode[4];
> >> -
> >> -     /* Validate params from spi_flash_params table */
> >> -     params = spi_flash_params_table;
> >> -     for (; params->name != NULL; params++) {
> >> -             if ((params->jedec >> 16) == idcode[0]) {
> >> -                     if ((params->jedec & 0xFFFF) == jedec) {
> >> -                             if (params->ext_jedec == 0)
> >> -                                     break;
> >> -                             else if (params->ext_jedec == ext_jedec)
> >> -                                     break;
> >> -                     }
> >> -             }
> >> -     }
> >> +     const struct spi_flash_info *info = NULL;
> >> +     int ret = -1;
> >>
> >> -     if (!params->name) {
> >> -             printf("SF: Unsupported flash IDs: ");
> >> -             printf("manuf %02x, jedec %04x, ext_jedec %04x\n",
> >> -                    idcode[0], jedec, ext_jedec);
> >> -             return -EPROTONOSUPPORT;
> >> -     }
> >> +     info = spi_flash_read_id(flash);
> >> +     if (IS_ERR_OR_NULL(info))
> >> +             return -ENOENT;
> >>
> >>  #ifdef CONFIG_SPI_FLASH_SPANSION
> >>       /*
> >> @@ -1065,11 +1060,17 @@ int spi_flash_scan(struct spi_flash *flash)
> >>        * sector that is not overlaid by the parameter sectors.
> >>        * The uniform sector erase command has no effect on parameter
> >> sectors.
> >>        */
> >> -     if ((jedec == 0x0219 || (jedec == 0x0220)) &&
> >> -         (ext_jedec & 0xff00) == 0x4d00) {
> >> +     if ((JEDEC_ID(info) == 0x0219 || (JEDEC_ID(info) == 0x0220)) &&
> >> +         (JEDEC_EXT(info) & 0xff00) == 0x4d00) {
> >>               int ret;
> >> +             u8 idcode[5];
> >>               u8 id[6];
> >>
> >> +             /* Read the ID codes again, 5 bytes */
> >> +             ret = spi_flash_cmd(flash->spi, CMD_READ_ID, idcode,
> >> sizeof(idcode));
> >> +             if (ret)
> >> +                     return -EIO;
> >> +
> > why are we reading id again, cant it be available as part of info?
> 
> Yes, but I can say this is removable code getting idcodes from info it's again a
> separate code task so, for the proper bisectable I am reading the idcode
> based on the existing code logic. of-course this is removing in later patch.
OK That’s fine. while reviewing this 1/21, I am not aware  
that its removed in follow up patches in series.


> 
> >
> >>               /* Read the ID codes again, 6 bytes */
> >>               ret = spi_flash_cmd(flash->spi, CMD_READ_ID, id, sizeof(id));
> >>               if (ret)
> >> @@ -1088,18 +1089,18 @@ int spi_flash_scan(struct spi_flash *flash)
> >>       }
> >>  #endif
> >>       /* Flash powers up read-only, so clear BP# bits */
> >> -     if (idcode[0] == SPI_FLASH_CFI_MFR_ATMEL ||
> >> -         idcode[0] == SPI_FLASH_CFI_MFR_MACRONIX ||
> >> -         idcode[0] == SPI_FLASH_CFI_MFR_SST)
> >> +     if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
> >> +         JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
> >> +         JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
> >>               write_sr(flash, 0);
> >>
> >>       /* Assign spi data */
> >> -     flash->name = params->name;
> >> +     flash->name = info->name;
> >>       flash->memory_map = spi->memory_map;
> >>       flash->dual_flash = spi->option;
> >>
> >>       /* Assign spi flash flags */
> >> -     if (params->flags & SST_WR)
> >> +     if (info->flags & SST_WR)
> >>               flash->flags |= SNOR_F_SST_WR;
> >>
> >>       /* Assign spi_flash ops */
> >> @@ -1118,7 +1119,7 @@ int spi_flash_scan(struct spi_flash *flash)
> >> #endif
> >>
> >>       /* lock hooks are flash specific - assign them based on idcode0 */
> >> -     switch (idcode[0]) {
> >> +     switch (JEDEC_MFR(info)) {
> >>  #if defined(CONFIG_SPI_FLASH_STMICRO) ||
> >> defined(CONFIG_SPI_FLASH_SST)
> >>       case SPI_FLASH_CFI_MFR_STMICRO:
> >>       case SPI_FLASH_CFI_MFR_SST:
> >> @@ -1128,28 +1129,26 @@ int spi_flash_scan(struct spi_flash *flash)
> #endif
> >>               break;
> >>       default:
> >> -             debug("SF: Lock ops not supported for %02x flash\n",
> >> idcode[0]);
> >> +             debug("SF: Lock ops not supported for %02x flash\n",
> >> +JEDEC_MFR(info));
> >>       }
> >>
> >>       /* Compute the flash size */
> >>       flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1
> >> : 0;
> >> +     flash->page_size = info->page_size;
> >>       /*
> >>        * The Spansion S25FL032P and S25FL064P have 256b pages, yet
> >> use the
> >>        * 0x4d00 Extended JEDEC code. The rest of the Spansion flashes
> >> with
> >>        * the 0x4d00 Extended JEDEC code have 512b pages. All of the
> >> others
> >>        * have 256b pages.
> >>        */
> >> -     if (ext_jedec == 0x4d00) {
> >> -             if ((jedec == 0x0215) || (jedec == 0x216) || (jedec == 0x220))
> >> -                     flash->page_size = 256;
> >> -             else
> >> +     if (JEDEC_EXT(info) == 0x4d00) {
> >> +             if ((JEDEC_ID(info) != 0x0215) &&
> >> +                 (JEDEC_ID(info) != 0x0216))
> >>                       flash->page_size = 512;
> >> -     } else {
> >> -             flash->page_size = 256;
> >>       }
> >>       flash->page_size <<= flash->shift;
> >> -     flash->sector_size = params->sector_size << flash->shift;
> >> -     flash->size = flash->sector_size * params->nr_sectors << flash->shift;
> >> +     flash->sector_size = info->sector_size << flash->shift;
> >> +     flash->size = flash->sector_size * info->nr_sectors <<
> >> + flash->shift;
> >
> > This is incorrect, Dont do flash->shift again, as you already did above for
> sector size calculation.
> > Doing this for second time causes double the actual size.
> 
> How come? This is an the existing code with params replaced by info.
Probably, the issue exist in old code as well, I found while reviewing this.
You are doing shift two times, which will result double the actual size.

Thanks,
Siva
> 
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream
> Maintainer Hyderabad, India.


More information about the U-Boot mailing list