[PATCH v2 0/5] This series fixes various bugs in the phytec som_detection unit.
--- Changes in v2: - fixed accidental squashing of changes -> split into 2 separate patches
--- Yannic Moog (5): board: phytec: imx8m_som_detection: change phytec_imx8m_detect return type board: phytec: imx8m_som_detection: fix uninitialized pointer bug board: phytec: phytec_som_detection: fix eeprom_data zero check board: phytec: som_detection: move definitions to source file board: phytec: phytec_som_detection: fix uninitialized bug
board/phytec/common/imx8m_som_detection.c | 40 +++++++++++++++++++++++--- board/phytec/common/imx8m_som_detection.h | 34 +--------------------- board/phytec/common/phytec_som_detection.c | 46 ++++++++++++++++++++++++++++-- board/phytec/common/phytec_som_detection.h | 38 ------------------------ 4 files changed, 80 insertions(+), 78 deletions(-) --- base-commit: 8f5043ee6d378d7c10d947cf48b047a5954ef5b3 change-id: 20231218-wip-y-moog-phytec-de-upstream_som_detection_fixes-b442a1e16334
Best regards,
phytec_imx8m_detect returns -1 on error, but the return type is u8 leading to 255 return values. Fix this by changing the return type to int; there is no reason to keep it as u8 .
Signed-off-by: Yannic Moog y.moog@phytec.de --- board/phytec/common/imx8m_som_detection.c | 2 +- board/phytec/common/imx8m_som_detection.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/phytec/common/imx8m_som_detection.c b/board/phytec/common/imx8m_som_detection.c index c6c96ed19cb..45f5767c565 100644 --- a/board/phytec/common/imx8m_som_detection.c +++ b/board/phytec/common/imx8m_som_detection.c @@ -23,7 +23,7 @@ extern struct phytec_eeprom_data eeprom_data; * * Returns 0 in case it's a known SoM. Otherwise, returns -1. */ -u8 __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data) +int __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data) { char *opt; u8 som; diff --git a/board/phytec/common/imx8m_som_detection.h b/board/phytec/common/imx8m_som_detection.h index 88d3037bf36..442085cfe97 100644 --- a/board/phytec/common/imx8m_som_detection.h +++ b/board/phytec/common/imx8m_som_detection.h @@ -15,7 +15,7 @@
#if IS_ENABLED(CONFIG_PHYTEC_IMX8M_SOM_DETECTION)
-u8 __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data); +int __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data); u8 __maybe_unused phytec_get_imx8m_ddr_size(struct phytec_eeprom_data *data); u8 __maybe_unused phytec_get_imx8mp_rtc(struct phytec_eeprom_data *data); u8 __maybe_unused phytec_get_imx8m_spi(struct phytec_eeprom_data *data); @@ -23,7 +23,7 @@ u8 __maybe_unused phytec_get_imx8m_eth(struct phytec_eeprom_data *data);
#else
-inline u8 __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data) +inline int __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data) { return -1; }
Pointer in phytec_imx8m_detect was accessed without checking it first. Fix this by moving the pointer check in front of any accesses.
Signed-off-by: Yannic Moog y.moog@phytec.de --- board/phytec/common/imx8m_som_detection.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/phytec/common/imx8m_som_detection.c b/board/phytec/common/imx8m_som_detection.c index 45f5767c565..a229eae152d 100644 --- a/board/phytec/common/imx8m_som_detection.c +++ b/board/phytec/common/imx8m_som_detection.c @@ -28,13 +28,13 @@ int __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data) char *opt; u8 som;
+ if (!data) + data = &eeprom_data; + /* We can not do the check for early API revisions */ if (data->api_rev < PHYTEC_API_REV2) return -1;
- if (!data) - data = &eeprom_data; - som = data->data.data_api2.som_no; debug("%s: som id: %u\n", __func__, som);
In phytec_eeprom_data_init, after reading eeprom data into buffer, it is checked whether all bytes are 0x0 by iterating over chunks of the buffer. The offset, or index of the chunk, was never changed, leading to repeated comparison of only the first chunk. Use array notation and access chunk via array index to compare all chunks of the buffer.
Signed-off-by: Yannic Moog y.moog@phytec.de --- board/phytec/common/phytec_som_detection.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/phytec/common/phytec_som_detection.c b/board/phytec/common/phytec_som_detection.c index 55562731270..724b8e844b6 100644 --- a/board/phytec/common/phytec_som_detection.c +++ b/board/phytec/common/phytec_som_detection.c @@ -83,8 +83,8 @@ int phytec_eeprom_data_init(struct phytec_eeprom_data *data, }
ptr = (int *)data; - for (i = 0; i < sizeof(struct phytec_eeprom_data); i += sizeof(ptr)) - if (*ptr != 0x0) + for (i = 0; i < sizeof(struct phytec_eeprom_data); i++) + if (ptr[i] != 0x0) break;
if (i == sizeof(struct phytec_eeprom_data)) {
Move all function definitions in {phytec|imx8m}_som_detection from the header to the source file to prevent potential linker error regarding multiple definitions. Also move the #if blocks with the definitions.
Signed-off-by: Yannic Moog y.moog@phytec.de --- board/phytec/common/imx8m_som_detection.c | 32 ++++++++++++++++++++++++ board/phytec/common/imx8m_som_detection.h | 32 ------------------------ board/phytec/common/phytec_som_detection.c | 39 ++++++++++++++++++++++++++++++ board/phytec/common/phytec_som_detection.h | 38 ----------------------------- 4 files changed, 71 insertions(+), 70 deletions(-)
diff --git a/board/phytec/common/imx8m_som_detection.c b/board/phytec/common/imx8m_som_detection.c index a229eae152d..214b75db3b0 100644 --- a/board/phytec/common/imx8m_som_detection.c +++ b/board/phytec/common/imx8m_som_detection.c @@ -15,6 +15,8 @@
extern struct phytec_eeprom_data eeprom_data;
+#if IS_ENABLED(CONFIG_PHYTEC_IMX8M_SOM_DETECTION) + /* Check if the SoM is actually one of the following products: * - i.MX8MM * - i.MX8MN @@ -166,3 +168,33 @@ u8 __maybe_unused phytec_get_imx8mp_rtc(struct phytec_eeprom_data *data) debug("%s: rtc: %u\n", __func__, rtc); return rtc; } + +#else + +inline int __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data) +{ + return -1; +} + +inline u8 __maybe_unused +phytec_get_imx8m_ddr_size(struct phytec_eeprom_data *data) +{ + return PHYTEC_EEPROM_INVAL; +} + +inline u8 __maybe_unused phytec_get_imx8mp_rtc(struct phytec_eeprom_data *data) +{ + return PHYTEC_EEPROM_INVAL; +} + +inline u8 __maybe_unused phytec_get_imx8m_spi(struct phytec_eeprom_data *data) +{ + return PHYTEC_EEPROM_INVAL; +} + +inline u8 __maybe_unused phytec_get_imx8m_eth(struct phytec_eeprom_data *data) +{ + return PHYTEC_EEPROM_INVAL; +} + +#endif /* IS_ENABLED(CONFIG_PHYTEC_IMX8M_SOM_DETECTION) */ diff --git a/board/phytec/common/imx8m_som_detection.h b/board/phytec/common/imx8m_som_detection.h index 442085cfe97..0176347414f 100644 --- a/board/phytec/common/imx8m_som_detection.h +++ b/board/phytec/common/imx8m_som_detection.h @@ -13,42 +13,10 @@ #define PHYTEC_IMX8MM_SOM 69 #define PHYTEC_IMX8MP_SOM 70
-#if IS_ENABLED(CONFIG_PHYTEC_IMX8M_SOM_DETECTION) - int __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data); u8 __maybe_unused phytec_get_imx8m_ddr_size(struct phytec_eeprom_data *data); u8 __maybe_unused phytec_get_imx8mp_rtc(struct phytec_eeprom_data *data); u8 __maybe_unused phytec_get_imx8m_spi(struct phytec_eeprom_data *data); u8 __maybe_unused phytec_get_imx8m_eth(struct phytec_eeprom_data *data);
-#else - -inline int __maybe_unused phytec_imx8m_detect(struct phytec_eeprom_data *data) -{ - return -1; -} - -inline u8 __maybe_unused -phytec_get_imx8m_ddr_size(struct phytec_eeprom_data *data) -{ - return PHYTEC_EEPROM_INVAL; -} - -inline u8 __maybe_unused phytec_get_imx8mp_rtc(struct phytec_eeprom_data *data) -{ - return PHYTEC_EEPROM_INVAL; -} - -inline u8 __maybe_unused phytec_get_imx8m_spi(struct phytec_eeprom_data *data) -{ - return PHYTEC_EEPROM_INVAL; -} - -inline u8 __maybe_unused phytec_get_imx8m_eth(struct phytec_eeprom_data *data) -{ - return PHYTEC_EEPROM_INVAL; -} - -#endif /* IS_ENABLED(CONFIG_PHYTEC_IMX8M_SOM_DETECTION) */ - #endif /* _PHYTEC_IMX8M_SOM_DETECTION_H */ diff --git a/board/phytec/common/phytec_som_detection.c b/board/phytec/common/phytec_som_detection.c index 724b8e844b6..f879702df45 100644 --- a/board/phytec/common/phytec_som_detection.c +++ b/board/phytec/common/phytec_som_detection.c @@ -16,6 +16,8 @@
struct phytec_eeprom_data eeprom_data;
+#if IS_ENABLED(CONFIG_PHYTEC_SOM_DETECTION) + int phytec_eeprom_data_setup_fallback(struct phytec_eeprom_data *data, int bus_num, int addr, int addr_fallback) { @@ -201,3 +203,40 @@ u8 __maybe_unused phytec_get_rev(struct phytec_eeprom_data *data)
return api2->pcb_rev; } + +#else + +inline int phytec_eeprom_data_setup(struct phytec_eeprom_data *data, + int bus_num, int addr) +{ + return PHYTEC_EEPROM_INVAL; +} + +inline int phytec_eeprom_data_setup_fallback(struct phytec_eeprom_data *data, + int bus_num, int addr, + int addr_fallback) +{ + return PHYTEC_EEPROM_INVAL; +} + +inline int phytec_eeprom_data_init(struct phytec_eeprom_data *data, + int bus_num, int addr) +{ + return PHYTEC_EEPROM_INVAL; +} + +inline void __maybe_unused phytec_print_som_info(struct phytec_eeprom_data *data) +{ +} + +inline char *__maybe_unused phytec_get_opt(struct phytec_eeprom_data *data) +{ + return NULL; +} + +u8 __maybe_unused phytec_get_rev(struct phytec_eeprom_data *data) +{ + return PHYTEC_EEPROM_INVAL; +} + +#endif /* IS_ENABLED(CONFIG_PHYTEC_SOM_DETECTION) */ diff --git a/board/phytec/common/phytec_som_detection.h b/board/phytec/common/phytec_som_detection.h index c68e2302cc4..11009240875 100644 --- a/board/phytec/common/phytec_som_detection.h +++ b/board/phytec/common/phytec_som_detection.h @@ -56,8 +56,6 @@ struct phytec_eeprom_data { } data; } __packed;
-#if IS_ENABLED(CONFIG_PHYTEC_SOM_DETECTION) - int phytec_eeprom_data_setup_fallback(struct phytec_eeprom_data *data, int bus_num, int addr, int addr_fallback); @@ -70,40 +68,4 @@ void __maybe_unused phytec_print_som_info(struct phytec_eeprom_data *data); char * __maybe_unused phytec_get_opt(struct phytec_eeprom_data *data); u8 __maybe_unused phytec_get_rev(struct phytec_eeprom_data *data);
-#else - -inline int phytec_eeprom_data_setup(struct phytec_eeprom_data *data, - int bus_num, int addr) -{ - return PHYTEC_EEPROM_INVAL; -} - -inline int phytec_eeprom_data_setup_fallback(struct phytec_eeprom_data *data, - int bus_num, int addr, - int addr_fallback) -{ - return PHYTEC_EEPROM_INVAL; -} - -inline int phytec_eeprom_data_init(struct phytec_eeprom_data *data, - int bus_num, int addr) -{ - return PHYTEC_EEPROM_INVAL; -} - -inline void __maybe_unused phytec_print_som_info(struct phytec_eeprom_data *data) -{ -} - -inline char *__maybe_unused phytec_get_opt(struct phytec_eeprom_data *data) -{ - return NULL; -} - -u8 __maybe_unused phytec_get_rev(struct phytec_eeprom_data *data) -{ - return PHYTEC_EEPROM_INVAL; -} -#endif /* IS_ENABLED(CONFIG_PHYTEC_SOM_DETECTION) */ - #endif /* _PHYTEC_SOM_DETECTION_H */
When som_type does not match any case, it is uninitialized and the function still tries to print the SoM info. Rather, this is an error condition and the function should abort prematurely. Highlight this by printing an error message and returning early.
Signed-off-by: Yannic Moog y.moog@phytec.de --- board/phytec/common/phytec_som_detection.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/phytec/common/phytec_som_detection.c b/board/phytec/common/phytec_som_detection.c index f879702df45..1b10923b62f 100644 --- a/board/phytec/common/phytec_som_detection.c +++ b/board/phytec/common/phytec_som_detection.c @@ -161,7 +161,8 @@ void __maybe_unused phytec_print_som_info(struct phytec_eeprom_data *data) sub_som_type2 = 2; break; default: - break; + pr_err("%s: Invalid SoM type: %i", __func__, api2->som_type); + return; };
printf("SoM: %s-%03u-%s-%03u ",
On Wed, Dec 20, 2023 at 5:45 AM Yannic Moog y.moog@phytec.de wrote:
Changes in v2:
- fixed accidental squashing of changes -> split into 2 separate patches
Yannic Moog (5): board: phytec: imx8m_som_detection: change phytec_imx8m_detect return type board: phytec: imx8m_som_detection: fix uninitialized pointer bug board: phytec: phytec_som_detection: fix eeprom_data zero check board: phytec: som_detection: move definitions to source file board: phytec: phytec_som_detection: fix uninitialized bug
Applied all to u-boot-imx/next, thanks.
participants (2)
-
Fabio Estevam -
Yannic Moog