[U-Boot] [PATCH 0/3] ti: common: board_detect: basic fixes for blank eeprom

[U-Boot] [PATCH 0/3] ti: common: board_detect: basic fixes for blank eeprom

[Nishanth Menon]

In some cases where eeprom has not been programmed, Brad reported
issues of crash at strcmp against NULL, this series was triggered
thanks to issue in manufacturing flow where the mandatory eeprom
programming was missed.

Nishanth Menon (3):
  ti: common: board_detect: Replace hardcoded value with macro
  ti: common: board_detect: Setup initial default value for config as
    well
  ti: common: board_detect: Return a valid empty string for
    un-initialized eeprom

 board/ti/common/board_detect.c | 16 ++++++----------
 board/ti/common/board_detect.h |  6 +++---
 2 files changed, 9 insertions(+), 13 deletions(-)

-- 
2.10.0

[U-Boot] [PATCH 1/3] ti: common: board_detect: Replace hardcoded value with macro

[Nishanth Menon]

We should have used TI_DEAD_EEPROM_MAGIC in the first place.

Fixes: d3b98a9eb941 ("ti: common: dra7: Add standard access for board description EEPROM")
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 board/ti/common/board_detect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
index e0ae1a51a6a4..7c552d20656a 100644
--- a/board/ti/common/board_detect.c
+++ b/board/ti/common/board_detect.c
@@ -171,7 +171,7 @@ int __maybe_unused ti_i2c_eeprom_dra7_get(int bus_addr, int dev_addr)
 		goto already_read;
 
 	/* Initialize with a known bad marker for i2c fails.. */
-	ep->header = 0xADEAD12C;
+	ep->header = TI_DEAD_EEPROM_MAGIC;
 	ep->name[0] = 0x0;
 	ep->version[0] = 0x0;
 	ep->serial[0] = 0x0;
-- 
2.10.0

[U-Boot] [PATCH 2/3] ti: common: board_detect: Setup initial default value for config as well

[Nishanth Menon]

config should have been initialized along with others as defaults.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 board/ti/common/board_detect.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
index 7c552d20656a..13aac2f03037 100644
--- a/board/ti/common/board_detect.c
+++ b/board/ti/common/board_detect.c
@@ -131,6 +131,7 @@ int __maybe_unused ti_i2c_eeprom_am_get(int bus_addr, int dev_addr)
 	ep->name[0] = 0x0;
 	ep->version[0] = 0x0;
 	ep->serial[0] = 0x0;
+	ep->config[0] = 0x0;
 
 	rc = ti_i2c_eeprom_get(bus_addr, dev_addr, TI_EEPROM_HEADER_MAGIC,
 			       sizeof(am_ep), (uint8_t *)&am_ep);
@@ -175,6 +176,7 @@ int __maybe_unused ti_i2c_eeprom_dra7_get(int bus_addr, int dev_addr)
 	ep->name[0] = 0x0;
 	ep->version[0] = 0x0;
 	ep->serial[0] = 0x0;
+	ep->config[0] = 0x0;
 	ep->emif1_size = 0;
 	ep->emif2_size = 0;
 
-- 
2.10.0

[U-Boot] [PATCH 3/3] ti: common: board_detect: Return a valid empty string for un-initialized eeprom

[Nishanth Menon]

Current logic for query of revision, board_name, config returns
NULL. Users of these functions do a direct strncmp to compare.
Unfortunately, as per conventions require two valid strings to compare
against and the current implementation causes a crash when compared
with NULL.

We'd still like to maintain the simplistic usage of these APIs instead
of redundant if (string) res=strncmp(fn(),"cmp",n); flowing all over
the place.

Hence, since the version, name and config is already pre-initialized
with empty string, just dont check for invalid header in the first
place and return the empty string to the caller.

Reported-by: Brad Griffis <bgriffis@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 board/ti/common/board_detect.c | 12 +++---------
 board/ti/common/board_detect.h |  6 +++---
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
index 13aac2f03037..6e7ca9196d22 100644
--- a/board/ti/common/board_detect.c
+++ b/board/ti/common/board_detect.c
@@ -231,9 +231,7 @@ char * __maybe_unused board_ti_get_rev(void)
 {
 	struct ti_common_eeprom *ep = TI_EEPROM_DATA;
 
-	if (ep->header == TI_DEAD_EEPROM_MAGIC)
-		return NULL;
-
+	/* if ep->header == TI_DEAD_EEPROM_MAGIC, this is empty already */
 	return ep->version;
 }
 
@@ -241,9 +239,7 @@ char * __maybe_unused board_ti_get_config(void)
 {
 	struct ti_common_eeprom *ep = TI_EEPROM_DATA;
 
-	if (ep->header == TI_DEAD_EEPROM_MAGIC)
-		return NULL;
-
+	/* if ep->header == TI_DEAD_EEPROM_MAGIC, this is empty already */
 	return ep->config;
 }
 
@@ -251,9 +247,7 @@ char * __maybe_unused board_ti_get_name(void)
 {
 	struct ti_common_eeprom *ep = TI_EEPROM_DATA;
 
-	if (ep->header == TI_DEAD_EEPROM_MAGIC)
-		return NULL;
-
+	/* if ep->header == TI_DEAD_EEPROM_MAGIC, this is empty already */
 	return ep->name;
 }
 
diff --git a/board/ti/common/board_detect.h b/board/ti/common/board_detect.h
index eb17f6f52a12..2ded3ed7764b 100644
--- a/board/ti/common/board_detect.h
+++ b/board/ti/common/board_detect.h
@@ -141,7 +141,7 @@ bool board_ti_rev_is(char *rev_tag, int cmp_len);
 /**
  * board_ti_get_rev() - Get board revision for TI EVMs
  *
- * Return: NULL if eeprom was'nt read.
+ * Return: Empty string if eeprom was'nt read.
  *	   Board revision otherwise
  */
 char *board_ti_get_rev(void);
@@ -149,7 +149,7 @@ char *board_ti_get_rev(void);
 /**
  * board_ti_get_config() - Get board config for TI EVMs
  *
- * Return: NULL if eeprom was'nt read.
+ * Return: Empty string if eeprom was'nt read.
  *	   Board config otherwise
  */
 char *board_ti_get_config(void);
@@ -157,7 +157,7 @@ char *board_ti_get_config(void);
 /**
  * board_ti_get_name() - Get board name for TI EVMs
  *
- * Return: NULL if eeprom was'nt read.
+ * Return: Empty string if eeprom was'nt read.
  *	   Board name otherwise
  */
 char *board_ti_get_name(void);
-- 
2.10.0

[U-Boot] [PATCH 1/3] ti: common: board_detect: Replace hardcoded value with macro

[Lokesh Vutla]

On Tuesday 11 October 2016 11:09 PM, Nishanth Menon wrote:
> We should have used TI_DEAD_EEPROM_MAGIC in the first place.
> 
> Fixes: d3b98a9eb941 ("ti: common: dra7: Add standard access for board description EEPROM")
> Signed-off-by: Nishanth Menon <nm@ti.com>

Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh

> ---
>  board/ti/common/board_detect.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> index e0ae1a51a6a4..7c552d20656a 100644
> --- a/board/ti/common/board_detect.c
> +++ b/board/ti/common/board_detect.c
> @@ -171,7 +171,7 @@ int __maybe_unused ti_i2c_eeprom_dra7_get(int bus_addr, int dev_addr)
>  		goto already_read;
>  
>  	/* Initialize with a known bad marker for i2c fails.. */
> -	ep->header = 0xADEAD12C;
> +	ep->header = TI_DEAD_EEPROM_MAGIC;
>  	ep->name[0] = 0x0;
>  	ep->version[0] = 0x0;
>  	ep->serial[0] = 0x0;
> 

[U-Boot] [PATCH 2/3] ti: common: board_detect: Setup initial default value for config as well

[Lokesh Vutla]

On Tuesday 11 October 2016 11:09 PM, Nishanth Menon wrote:
> config should have been initialized along with others as defaults.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>

Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh

> ---
>  board/ti/common/board_detect.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> index 7c552d20656a..13aac2f03037 100644
> --- a/board/ti/common/board_detect.c
> +++ b/board/ti/common/board_detect.c
> @@ -131,6 +131,7 @@ int __maybe_unused ti_i2c_eeprom_am_get(int bus_addr, int dev_addr)
>  	ep->name[0] = 0x0;
>  	ep->version[0] = 0x0;
>  	ep->serial[0] = 0x0;
> +	ep->config[0] = 0x0;
>  
>  	rc = ti_i2c_eeprom_get(bus_addr, dev_addr, TI_EEPROM_HEADER_MAGIC,
>  			       sizeof(am_ep), (uint8_t *)&am_ep);
> @@ -175,6 +176,7 @@ int __maybe_unused ti_i2c_eeprom_dra7_get(int bus_addr, int dev_addr)
>  	ep->name[0] = 0x0;
>  	ep->version[0] = 0x0;
>  	ep->serial[0] = 0x0;
> +	ep->config[0] = 0x0;
>  	ep->emif1_size = 0;
>  	ep->emif2_size = 0;
>  
> 

[U-Boot] [PATCH 3/3] ti: common: board_detect: Return a valid empty string for un-initialized eeprom

[Lokesh Vutla]

On Tuesday 11 October 2016 11:09 PM, Nishanth Menon wrote:
> Current logic for query of revision, board_name, config returns
> NULL. Users of these functions do a direct strncmp to compare.
> Unfortunately, as per conventions require two valid strings to compare
> against and the current implementation causes a crash when compared
> with NULL.
> 
> We'd still like to maintain the simplistic usage of these APIs instead
> of redundant if (string) res=strncmp(fn(),"cmp",n); flowing all over
> the place.
> 
> Hence, since the version, name and config is already pre-initialized
> with empty string, just dont check for invalid header in the first
> place and return the empty string to the caller.
> 
> Reported-by: Brad Griffis <bgriffis@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>


Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh


> ---
>  board/ti/common/board_detect.c | 12 +++---------
>  board/ti/common/board_detect.h |  6 +++---
>  2 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> index 13aac2f03037..6e7ca9196d22 100644
> --- a/board/ti/common/board_detect.c
> +++ b/board/ti/common/board_detect.c
> @@ -231,9 +231,7 @@ char * __maybe_unused board_ti_get_rev(void)
>  {
>  	struct ti_common_eeprom *ep = TI_EEPROM_DATA;
>  
> -	if (ep->header == TI_DEAD_EEPROM_MAGIC)
> -		return NULL;
> -
> +	/* if ep->header == TI_DEAD_EEPROM_MAGIC, this is empty already */
>  	return ep->version;
>  }
>  
> @@ -241,9 +239,7 @@ char * __maybe_unused board_ti_get_config(void)
>  {
>  	struct ti_common_eeprom *ep = TI_EEPROM_DATA;
>  
> -	if (ep->header == TI_DEAD_EEPROM_MAGIC)
> -		return NULL;
> -
> +	/* if ep->header == TI_DEAD_EEPROM_MAGIC, this is empty already */
>  	return ep->config;
>  }
>  
> @@ -251,9 +247,7 @@ char * __maybe_unused board_ti_get_name(void)
>  {
>  	struct ti_common_eeprom *ep = TI_EEPROM_DATA;
>  
> -	if (ep->header == TI_DEAD_EEPROM_MAGIC)
> -		return NULL;
> -
> +	/* if ep->header == TI_DEAD_EEPROM_MAGIC, this is empty already */
>  	return ep->name;
>  }
>  
> diff --git a/board/ti/common/board_detect.h b/board/ti/common/board_detect.h
> index eb17f6f52a12..2ded3ed7764b 100644
> --- a/board/ti/common/board_detect.h
> +++ b/board/ti/common/board_detect.h
> @@ -141,7 +141,7 @@ bool board_ti_rev_is(char *rev_tag, int cmp_len);
>  /**
>   * board_ti_get_rev() - Get board revision for TI EVMs
>   *
> - * Return: NULL if eeprom was'nt read.
> + * Return: Empty string if eeprom was'nt read.
>   *	   Board revision otherwise
>   */
>  char *board_ti_get_rev(void);
> @@ -149,7 +149,7 @@ char *board_ti_get_rev(void);
>  /**
>   * board_ti_get_config() - Get board config for TI EVMs
>   *
> - * Return: NULL if eeprom was'nt read.
> + * Return: Empty string if eeprom was'nt read.
>   *	   Board config otherwise
>   */
>  char *board_ti_get_config(void);
> @@ -157,7 +157,7 @@ char *board_ti_get_config(void);
>  /**
>   * board_ti_get_name() - Get board name for TI EVMs
>   *
> - * Return: NULL if eeprom was'nt read.
> + * Return: Empty string if eeprom was'nt read.
>   *	   Board name otherwise
>   */
>  char *board_ti_get_name(void);
> 

[U-Boot] [PATCH 1/3] ti: common: board_detect: Replace hardcoded value with macro

[Tom Rini]

On Tue, Oct 11, 2016 at 12:39:03PM -0500, Nishanth Menon wrote:

> We should have used TI_DEAD_EEPROM_MAGIC in the first place.
> 
> Fixes: d3b98a9eb941 ("ti: common: dra7: Add standard access for board description EEPROM")
> Signed-off-by: Nishanth Menon <nm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161013/2bed6c5f/attachment.sig>

[U-Boot] [PATCH 2/3] ti: common: board_detect: Setup initial default value for config as well

[Tom Rini]

On Tue, Oct 11, 2016 at 12:39:04PM -0500, Nishanth Menon wrote:

> config should have been initialized along with others as defaults.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161013/8212e691/attachment.sig>

[U-Boot] [PATCH 3/3] ti: common: board_detect: Return a valid empty string for un-initialized eeprom

[Tom Rini]

On Tue, Oct 11, 2016 at 12:39:05PM -0500, Nishanth Menon wrote:

> Current logic for query of revision, board_name, config returns
> NULL. Users of these functions do a direct strncmp to compare.
> Unfortunately, as per conventions require two valid strings to compare
> against and the current implementation causes a crash when compared
> with NULL.
> 
> We'd still like to maintain the simplistic usage of these APIs instead
> of redundant if (string) res=strncmp(fn(),"cmp",n); flowing all over
> the place.
> 
> Hence, since the version, name and config is already pre-initialized
> with empty string, just dont check for invalid header in the first
> place and return the empty string to the caller.
> 
> Reported-by: Brad Griffis <bgriffis@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161013/f4734b72/attachment.sig>

[U-Boot] [U-Boot, 1/3] ti: common: board_detect: Replace hardcoded value with macro

[Tom Rini]

On Tue, Oct 11, 2016 at 12:39:03PM -0500, Nishanth Menon wrote:

> We should have used TI_DEAD_EEPROM_MAGIC in the first place.
> 
> Fixes: d3b98a9eb941 ("ti: common: dra7: Add standard access for board description EEPROM")
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master (before v2016.11-rc3), thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161102/3edad257/attachment.sig>

[U-Boot] [U-Boot, 2/3] ti: common: board_detect: Setup initial default value for config as well

[Tom Rini]

On Tue, Oct 11, 2016 at 12:39:04PM -0500, Nishanth Menon wrote:

> config should have been initialized along with others as defaults.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master (before v2016.11-rc3), thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161102/0cc1f48e/attachment.sig>

[U-Boot] [U-Boot, 3/3] ti: common: board_detect: Return a valid empty string for un-initialized eeprom

[Tom Rini]

On Tue, Oct 11, 2016 at 12:39:05PM -0500, Nishanth Menon wrote:

> Current logic for query of revision, board_name, config returns
> NULL. Users of these functions do a direct strncmp to compare.
> Unfortunately, as per conventions require two valid strings to compare
> against and the current implementation causes a crash when compared
> with NULL.
> 
> We'd still like to maintain the simplistic usage of these APIs instead
> of redundant if (string) res=strncmp(fn(),"cmp",n); flowing all over
> the place.
> 
> Hence, since the version, name and config is already pre-initialized
> with empty string, just dont check for invalid header in the first
> place and return the empty string to the caller.
> 
> Reported-by: Brad Griffis <bgriffis@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master (before v2016.11-rc3), thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161102/d9f9e998/attachment.sig>