[U-Boot] [PATCH 0/3] ti: common: board_detect: basic fixes for blank eeprom
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(-)
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;
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;
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
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!
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;
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;
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
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!
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);
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 otherwisechar *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 otherwisechar *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 otherwisechar *board_ti_get_name(void);
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
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!
participants (3)
-
Lokesh Vutla -
Nishanth Menon -
Tom Rini