Re: [PATCH 2/5] board: phytec: introduce eeprom struct member 'valid'

Teresa Remmet
2024-04-23T07:27:04Z

Am Freitag, dem 19.04.2024 um 08:55 -0700 schrieb Daniel Schultz:
> From: Yannic Moog <y.moog@phytec.de>
>
> Add a new nember to the eeprom_data that indicates whether the
> associated data is valid or not. Make use of this new member in the
> phytec_eeprom_data_init function by setting the valid value
> appropriately.
> Move the eeprom data to a new struct payload that holds
> the payload of the eeprom.
>
> Signed-off-by: Yannic Moog <y.moog@phytec.de>
> Signed-off-by: Daniel Schultz <d.schultz@phytec.de>

Reviewed-by: Teresa Remmet <t.remmet@phytec.de>

> ---
>  board/phytec/common/imx8m_som_detection.c  | 10 ++--
>  board/phytec/common/phytec_som_detection.c | 56 ++++++++++++--------
> --
>  board/phytec/common/phytec_som_detection.h | 11 +++--
>  3 files changed, 45 insertions(+), 32 deletions(-)
>
> diff --git a/board/phytec/common/imx8m_som_detection.c
> b/board/phytec/common/imx8m_som_detection.c
> index 214b75db3b0..7571076a09e 100644
> --- a/board/phytec/common/imx8m_som_detection.c
> +++ b/board/phytec/common/imx8m_som_detection.c
> @@ -34,10 +34,10 @@ int __maybe_unused phytec_imx8m_detect(struct
> phytec_eeprom_data *data)
>                 data = &eeprom_data;
>
>         /* We can not do the check for early API revisions */
> -       if (data->api_rev < PHYTEC_API_REV2)
> +       if (data->payload.api_rev < PHYTEC_API_REV2)
>                 return -1;
>
> -       som = data->data.data_api2.som_no;
> +       som = data->payload.data.data_api2.som_no;
>         debug("%s: som id: %u\n", __func__, som);
>
>         opt = phytec_get_opt(data);
> @@ -99,7 +99,7 @@ u8 __maybe_unused phytec_get_imx8m_spi(struct
> phytec_eeprom_data *data)
>         if (!data)
>                 data = &eeprom_data;
>
> -       if (data->api_rev < PHYTEC_API_REV2)
> +       if (data->payload.api_rev < PHYTEC_API_REV2)
>                 return PHYTEC_EEPROM_INVAL;
>
>         opt = phytec_get_opt(data);
> @@ -126,7 +126,7 @@ u8 __maybe_unused phytec_get_imx8m_eth(struct
> phytec_eeprom_data *data)
>         if (!data)
>                 data = &eeprom_data;
>
> -       if (data->api_rev < PHYTEC_API_REV2)
> +       if (data->payload.api_rev < PHYTEC_API_REV2)
>                 return PHYTEC_EEPROM_INVAL;
>
>         opt = phytec_get_opt(data);
> @@ -154,7 +154,7 @@ u8 __maybe_unused phytec_get_imx8mp_rtc(struct
> phytec_eeprom_data *data)
>         if (!data)
>                 data = &eeprom_data;
>
> -       if (data->api_rev < PHYTEC_API_REV2)
> +       if (data->payload.api_rev < PHYTEC_API_REV2)
>                 return PHYTEC_EEPROM_INVAL;
>
>         opt = phytec_get_opt(data);
> diff --git a/board/phytec/common/phytec_som_detection.c
> b/board/phytec/common/phytec_som_detection.c
> index d167a77c25b..7913764be0a 100644
> --- a/board/phytec/common/phytec_som_detection.c
> +++ b/board/phytec/common/phytec_som_detection.c
> @@ -54,6 +54,7 @@ int phytec_eeprom_data_init(struct
> phytec_eeprom_data *data,
>         int ret, i;
>         unsigned int crc;
>         int *ptr;
> +       const unsigned int payload_size = sizeof(struct
> phytec_eeprom_payload);
>
>         if (!data)
>                 data = &eeprom_data;
> @@ -64,14 +65,13 @@ int phytec_eeprom_data_init(struct
> phytec_eeprom_data *data,
>         ret = i2c_get_chip_for_busnum(bus_num, addr, 2, &dev);
>         if (ret) {
>                 pr_err("%s: i2c EEPROM not found: %i.\n", __func__,
> ret);
> -               return ret;
> +               goto err;
>         }
>
> -       ret = dm_i2c_read(dev, 0, (uint8_t *)data,
> -                         sizeof(struct phytec_eeprom_data));
> +       ret = dm_i2c_read(dev, 0, (uint8_t *)data, payload_size);
>         if (ret) {
> -               pr_err("%s: Unable to read EEPROM data\n", __func__);
> -               return ret;
> +               pr_err("%s: Unable to read EEPROM data: %i\n",
> __func__, ret);
> +               goto err;
>         }
>  #else
>         i2c_set_bus_num(bus_num);
> @@ -79,36 +79,44 @@ int phytec_eeprom_data_init(struct
> phytec_eeprom_data *data,
>                        sizeof(struct phytec_eeprom_data));
>  #endif
>
> -       if (data->api_rev == 0xff) {
> +       if (data->payload.api_rev == 0xff) {
>                 pr_err("%s: EEPROM is not flashed. Prototype?\n",
> __func__);
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto err;
>         }
>
>         ptr = (int *)data;
> -       for (i = 0; i < sizeof(struct phytec_eeprom_data); i++)
> +       for (i = 0; i < payload_size; ++i)
>                 if (ptr[i] != 0x0)
>                         break;
>
> -       if (i == sizeof(struct phytec_eeprom_data)) {
> +       if (i == payload_size) {
>                 pr_err("%s: EEPROM data is all zero. Erased?\n",
> __func__);
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto err;
>         }
>
>         /* We are done here for early revisions */
> -       if (data->api_rev <= PHYTEC_API_REV1)
> +       if (data->payload.api_rev <= PHYTEC_API_REV1) {
> +               data->valid = true;
>                 return 0;
> +       }
>
> -       crc = crc8(0, (const unsigned char *)data,
> -                  sizeof(struct phytec_eeprom_data));
> +       crc = crc8(0, (const unsigned char *)&data->payload,
> payload_size);
>         debug("%s: crc: %x\n", __func__, crc);
>
>         if (crc) {
> -               pr_err("%s: CRC mismatch. EEPROM data is not
> usable\n",
> +               pr_err("%s: CRC mismatch. EEPROM data is not
> usable.\n",
>                        __func__);
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto err;
>         }
>
> +       data->valid = true;
>         return 0;
> +err:
> +       data->valid = false;
> +       return ret;
>  }
>
>  void __maybe_unused phytec_print_som_info(struct phytec_eeprom_data
> *data)
> @@ -120,10 +128,10 @@ void __maybe_unused
> phytec_print_som_info(struct phytec_eeprom_data *data)
>         if (!data)
>                 data = &eeprom_data;
>
> -       if (data->api_rev < PHYTEC_API_REV2)
> +       if (data->payload.api_rev < PHYTEC_API_REV2)
>                 return;
>
> -       api2 = &data->data.data_api2;
> +       api2 = &data->payload.data.data_api2;
>
>         /* Calculate PCB subrevision */
>         pcb_sub_rev = api2->pcb_sub_opt_rev & 0x0f;
> @@ -182,10 +190,10 @@ char * __maybe_unused phytec_get_opt(struct
> phytec_eeprom_data *data)
>         if (!data)
>                 data = &eeprom_data;
>
> -       if (data->api_rev < PHYTEC_API_REV2)
> -               opt = data->data.data_api0.opt;
> +       if (data->payload.api_rev < PHYTEC_API_REV2)
> +               opt = data->payload.data.data_api0.opt;
>         else
> -               opt = data->data.data_api2.opt;
> +               opt = data->payload.data.data_api2.opt;
>
>         return opt;
>  }
> @@ -197,10 +205,10 @@ u8 __maybe_unused phytec_get_rev(struct
> phytec_eeprom_data *data)
>         if (!data)
>                 data = &eeprom_data;
>
> -       if (data->api_rev < PHYTEC_API_REV2)
> +       if (data->payload.api_rev < PHYTEC_API_REV2)
>                 return PHYTEC_EEPROM_INVAL;
>
> -       api2 = &data->data.data_api2;
> +       api2 = &data->payload.data.data_api2;
>
>         return api2->pcb_rev;
>  }
> @@ -209,10 +217,10 @@ u8 __maybe_unused phytec_get_som_type(struct
> phytec_eeprom_data *data)
>  {
>         if (!data)
>                 data = &eeprom_data;
> -       if (data->api_rev < PHYTEC_API_REV2)
> +       if (data->payload.api_rev < PHYTEC_API_REV2)
>                 return PHYTEC_EEPROM_INVAL;
>
> -       return data->data.data_api2.som_type;
> +       return data->payload.data.data_api2.som_type;
>  }
>
>  #if IS_ENABLED(CONFIG_CMD_EXTENSION)
> diff --git a/board/phytec/common/phytec_som_detection.h
> b/board/phytec/common/phytec_som_detection.h
> index ea99a687fee..0ad5c14ef4e 100644
> --- a/board/phytec/common/phytec_som_detection.h
> +++ b/board/phytec/common/phytec_som_detection.h
> @@ -55,7 +55,7 @@ struct phytec_api2_data {
>         u8 crc8;                /* checksum */
>  } __packed;
>
> -struct phytec_eeprom_data {
> +struct phytec_eeprom_payload {
>         u8 api_rev;
>         union {
>                 struct phytec_api0_data data_api0;
> @@ -63,13 +63,18 @@ struct phytec_eeprom_data {
>         } data;
>  } __packed;
>
> +struct phytec_eeprom_data {
> +       struct phytec_eeprom_payload payload;
> +       bool valid;
> +};
> +
>  int phytec_eeprom_data_setup_fallback(struct phytec_eeprom_data
> *data,
>                                       int bus_num, int addr,
>                                       int addr_fallback);
>  int phytec_eeprom_data_setup(struct phytec_eeprom_data *data,
>                              int bus_num, int addr);
> -int phytec_eeprom_data_init(struct phytec_eeprom_data *data,
> -                           int bus_num, int addr);
> +int phytec_eeprom_data_init(struct phytec_eeprom_data *data, int
> bus_num,
> +                           int addr);
>  void __maybe_unused phytec_print_som_info(struct phytec_eeprom_data
> *data);
>
>  char * __maybe_unused phytec_get_opt(struct phytec_eeprom_data
> *data);

--
PHYTEC Messtechnik GmbH | Barcelona-Allee 1 | 55129 Mainz, Germany

Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber,
Dipl.-Ing. (FH) Markus Lickes | Handelsregister Mainz HRB 4656 |
Finanzamt Mainz | St.Nr. 266500608, DE 149059855