[U-Boot] [PATCH v6 06/21] sf: Add SPI_FLASH_MAX_ID_LEN

[Siva Durga Prasad Paladugu]

Hi,

> -----Original Message-----
> From: Jagan Teki [mailto:jagan at openedev.com]
> Sent: Wednesday, November 16, 2016 6:12 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 06/21] sf: Add SPI_FLASH_MAX_ID_LEN
> 
> On Wed, Nov 16, 2016 at 11:26 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>; 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 06/21] sf: Add SPI_FLASH_MAX_ID_LEN
> >>
> >> Add id length of 5 bytes numerical value to macro.
> >>
> >> 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>
> >> Signed-off-by: Jagan Teki <jagan at openedev.com>
> >> Reviewed-by: Simon Glass <sjg at chromium.org>
> >> Reviewed-by: Jagan Teki <jagan at openedev.com>
> >> Tested-by: Jagan Teki <jagan at openedev.com>
> >> ---
> >>  drivers/mtd/spi/sf_internal.h | 3 ++-
> >>  drivers/mtd/spi/spi_flash.c   | 4 ++--
> >>  2 files changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/mtd/spi/sf_internal.h
> >> b/drivers/mtd/spi/sf_internal.h index 70617f2..770f628 100644
> >> --- a/drivers/mtd/spi/sf_internal.h
> >> +++ b/drivers/mtd/spi/sf_internal.h
> >> @@ -107,6 +107,7 @@ int sst_write_bp(struct spi_flash *flash, u32
> >> offset, size_t len,
> >>  #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]))
> >> +#define SPI_FLASH_MAX_ID_LEN 5
> >>
> >>  struct spi_flash_info {
> >>       /* Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO]) */
> >> @@ -117,7 +118,7 @@ struct spi_flash_info {
> >>        * The first three bytes are the JEDIC ID.
> >>        * JEDEC ID zero means "no ID" (mostly older chips).
> >>        */
> >> -     u8              id[5];
> >> +     u8              id[SPI_FLASH_MAX_ID_LEN];
> >>       u8              id_len;
> >>
> >>       /*
> >> diff --git a/drivers/mtd/spi/spi_flash.c
> >> b/drivers/mtd/spi/spi_flash.c index
> >> f7634df..9430424 100644
> >> --- a/drivers/mtd/spi/spi_flash.c
> >> +++ b/drivers/mtd/spi/spi_flash.c
> >> @@ -928,10 +928,10 @@ static int micron_quad_enable(struct spi_flash
> >> *flash)  static const struct spi_flash_info *spi_flash_read_id(struct
> >> spi_flash
> >> *flash)  {
> >>       int                             tmp;
> >> -     u8                              id[5];
> >> +     u8                              id[SPI_FLASH_MAX_ID_LEN];
> >>       const struct spi_flash_info     *info;
> >>
> >> -     tmp = spi_flash_cmd(flash->spi, CMD_READ_ID, id, 5);
> >> +     tmp = spi_flash_cmd(flash->spi, CMD_READ_ID, id,
> >> +SPI_FLASH_MAX_ID_LEN);
> >
> > Not only here, you are reading Id again in spi_flash_scan(), use same
> > macro there as well. Also, you are reading 6 bytes of ID, please
> > consider the same there.
> 
> This call is from spi_flash_scan itself , spi_flash_read_id

I meant here in spi_flash_scan(), but anyway you removed this code in 10/21.

/* Read the ID codes again, 5 bytes */
-		ret = spi_flash_cmd(flash->spi, CMD_READ_ID, idcode, sizeof(idcode));


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