diff mbox

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

Message ID 1479268992-26811-7-git-send-email-jagan@openedev.com
State Accepted
Commit ed363b53d063b56ea6dfbb595ed13b8371d50e08
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Jagan Teki Nov. 16, 2016, 4:02 a.m. UTC
Add id length of 5 bytes numerical value to macro.

Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: York Sun <york.sun@nxp.com>
Cc: Vignesh R <vigneshr@ti.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
Signed-off-by: Jagan Teki <jagan@openedev.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jagan Teki <jagan@openedev.com>
Tested-by: Jagan Teki <jagan@openedev.com>
---
 drivers/mtd/spi/sf_internal.h | 3 ++-
 drivers/mtd/spi/spi_flash.c   | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Siva Durga Prasad Paladugu Nov. 16, 2016, 5:56 a.m. UTC | #1
Hi,


> -----Original Message-----
> From: Jagan Teki [mailto:jagan@openedev.com]
> Sent: Wednesday, November 16, 2016 9:33 AM
> To: u-boot@lists.denx.de
> Cc: Jagan Teki <jagan@openedev.com>; Bin Meng <bmeng.cn@gmail.com>;
> York Sun <york.sun@nxp.com>; Vignesh R <vigneshr@ti.com>; Mugunthan V
> N <mugunthanvnm@ti.com>; Michal Simek <michal.simek@xilinx.com>; Siva
> Durga Prasad Paladugu <sivadur@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@gmail.com>
> Cc: York Sun <york.sun@nxp.com>
> Cc: Vignesh R <vigneshr@ti.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Signed-off-by: Jagan Teki <jagan@openedev.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Jagan Teki <jagan@openedev.com>
> Tested-by: Jagan Teki <jagan@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.

Thanks,
Siva
>  	if (tmp < 0) {
>  		printf("SF: error %d reading JEDEC ID\n", tmp);
>  		return ERR_PTR(tmp);
> --
> 1.9.1
Jagan Teki Nov. 16, 2016, 12:42 p.m. UTC | #2
On Wed, Nov 16, 2016 at 11:26 AM, Siva Durga Prasad Paladugu
<siva.durga.paladugu@xilinx.com> wrote:
> Hi,
>
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jagan@openedev.com]
>> Sent: Wednesday, November 16, 2016 9:33 AM
>> To: u-boot@lists.denx.de
>> Cc: Jagan Teki <jagan@openedev.com>; Bin Meng <bmeng.cn@gmail.com>;
>> York Sun <york.sun@nxp.com>; Vignesh R <vigneshr@ti.com>; Mugunthan V
>> N <mugunthanvnm@ti.com>; Michal Simek <michal.simek@xilinx.com>; Siva
>> Durga Prasad Paladugu <sivadur@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@gmail.com>
>> Cc: York Sun <york.sun@nxp.com>
>> Cc: Vignesh R <vigneshr@ti.com>
>> Cc: Mugunthan V N <mugunthanvnm@ti.com>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Signed-off-by: Jagan Teki <jagan@openedev.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> Reviewed-by: Jagan Teki <jagan@openedev.com>
>> Tested-by: Jagan Teki <jagan@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

thanks!
Siva Durga Prasad Paladugu Nov. 16, 2016, 1:07 p.m. UTC | #3
Hi,

> -----Original Message-----
> From: Jagan Teki [mailto:jagan@openedev.com]
> Sent: Wednesday, November 16, 2016 6:12 PM
> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Cc: u-boot@lists.denx.de; Michal Simek <michal.simek@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@xilinx.com> wrote:
> > Hi,
> >
> >
> >> -----Original Message-----
> >> From: Jagan Teki [mailto:jagan@openedev.com]
> >> Sent: Wednesday, November 16, 2016 9:33 AM
> >> To: u-boot@lists.denx.de
> >> Cc: Jagan Teki <jagan@openedev.com>; Bin Meng
> <bmeng.cn@gmail.com>;
> >> York Sun <york.sun@nxp.com>; Vignesh R <vigneshr@ti.com>;
> Mugunthan V
> >> N <mugunthanvnm@ti.com>; Michal Simek <michal.simek@xilinx.com>;
> Siva
> >> Durga Prasad Paladugu <sivadur@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@gmail.com>
> >> Cc: York Sun <york.sun@nxp.com>
> >> Cc: Vignesh R <vigneshr@ti.com>
> >> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> >> Cc: Michal Simek <michal.simek@xilinx.com>
> >> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> Signed-off-by: Jagan Teki <jagan@openedev.com>
> >> Reviewed-by: Simon Glass <sjg@chromium.org>
> >> Reviewed-by: Jagan Teki <jagan@openedev.com>
> >> Tested-by: Jagan Teki <jagan@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.
diff mbox

Patch

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);
 	if (tmp < 0) {
 		printf("SF: error %d reading JEDEC ID\n", tmp);
 		return ERR_PTR(tmp);