[PATCH v2 2/3] board: phytec: imx93: Add eeprom-based hardware introspection
Christoph Stoidner
C.Stoidner at phytec.de
Tue Nov 19 17:45:37 CET 2024
On Do, 2024-11-14 at 07:30 +0100, Yannic Moog wrote:
> Hi Christoph,
>
> On Wed, 2024-11-13 at 17:00 +0100, Christoph Stoidner wrote:
> > The phyCORE-i.MX 93 is available in various variants. Relevant
> > variant
> > options for the spl/u-boot are:
> > - with or without HS400 support for the eMMC
> > - with 1GB ram chip, or 2GB ram chip
> >
> > The phyCORE's eeprom contains all information about the existing
> > variant
> > options. Add evaluation of the eeprom data to the spl/u-boot to
> > enable/disable HS400 and to select the appropriate ram
> > configuration at
> > startup.
> >
> > Signed-off-by: Christoph Stoidner <c.stoidner at phytec.de>
> > ---
> > Cc: Mathieu Othacehe <m.othacehe at gmail.com>
> > Cc: Christoph Stoidner <c.stoidner at phytec.de>
> > Cc: Stefano Babic <sbabic at denx.de>
> > Cc: Fabio Estevam <festevam at gmail.com>
> > Cc: "NXP i.MX U-Boot Team" <uboot-imx at nxp.com>
> > Cc: Tom Rini <trini at konsulko.com>
> > Cc: Yannic Moog <y.moog at phytec.de>
> > Cc: Primoz Fiser <primoz.fiser at norik.com>
> > Cc: Andrej Picej <andrej.picej at norik.com>
> > Cc: Wadim Egorov <w.egorov at phytec.de>
> > ---
> > Changes in v2:
> > - encapsulate handling of feature flag VOLTAGE into own function
> > - move definition of enum phytec_imx93_ddr_eeprom_code into header
> > file
> >
> > arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi | 19 +++
> > arch/arm/mach-imx/imx9/Kconfig | 2 +
> > arch/arm/mach-imx/imx9/soc.c | 2 +-
> > board/phytec/common/Kconfig | 8 ++
> > board/phytec/common/Makefile | 1 +
> > board/phytec/common/imx93_som_detection.c | 111
> > ++++++++++++++++++
> > board/phytec/common/imx93_som_detection.h | 51 ++++++++
> > board/phytec/phycore_imx93/Kconfig | 28 +++++
> > board/phytec/phycore_imx93/MAINTAINERS | 5 +-
> > board/phytec/phycore_imx93/phycore-imx93.c | 51 ++++++++
> > board/phytec/phycore_imx93/spl.c | 48 ++++++++
> > 11 files changed, 324 insertions(+), 2 deletions(-)
> > create mode 100644 board/phytec/common/imx93_som_detection.c
> > create mode 100644 board/phytec/common/imx93_som_detection.h
> >
> > diff --git a/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
> > b/arch/arm/dts/imx93-phyboard-segin-u-
> > boot.dtsi
> > index 6897c91f4d..25c778bb07 100644
> > --- a/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
> > +++ b/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
> > @@ -305,4 +305,23 @@
> > };
> > };
> > };
> > +
> > + eeprom at 50 {
> > + bootph-pre-ram;
> > + bootph-some-ram;
> > + compatible = "atmel,24c32";
> > + reg = <0x50>;
> > + pagesize = <32>;
> > + vcc-supply = <&buck4>;
> > + };
> > +
> > + eepromid at 58 {
> > + bootph-pre-ram;
> > + bootph-some-ram;
> > + compatible = "atmel,24c32";
> > + pagesize = <32>;
> > + reg = <0x58>;
> > + size = <32>;
> > + vcc-supply = <&buck4>;
> > + };
> > };
> > diff --git a/arch/arm/mach-imx/imx9/Kconfig b/arch/arm/mach-
> > imx/imx9/Kconfig
> > index 5c1054138f..2465e31d73 100644
> > --- a/arch/arm/mach-imx/imx9/Kconfig
> > +++ b/arch/arm/mach-imx/imx9/Kconfig
> > @@ -45,6 +45,8 @@ config TARGET_PHYCORE_IMX93
> > bool "phycore_imx93"
> > select IMX93
> > select IMX9_LPDDR4X
> > + select OF_BOARD_FIXUP
> > + select OF_BOARD_SETUP
> >
> > endchoice
> >
> > diff --git a/arch/arm/mach-imx/imx9/soc.c b/arch/arm/mach-
> > imx/imx9/soc.c
> > index 7c28fa39e1..237354f507 100644
> > --- a/arch/arm/mach-imx/imx9/soc.c
> > +++ b/arch/arm/mach-imx/imx9/soc.c
> > @@ -628,7 +628,7 @@ static int low_drive_freq_update(void *blob)
> > return 0;
> > }
> >
> > -#ifdef CONFIG_OF_BOARD_FIXUP
> > +#if defined(CONFIG_OF_BOARD_FIXUP) &&
> > !defined(CONFIG_TARGET_PHYCORE_IMX93)
>
> Why do you do this?
>
> It doesn't look right to me. When you have to modify soc.c to add
> your own board config, then either
> there is a problem with the file and it should be fixed properly here
> or there is a problem with
> your board and it should be fixed in your board code.
The function board_fix_fdt() is an u-boot callback, that can be
implemented by the board-code. It enables the board-code to modify
the Device Tree. The modified device tree is then subsequently used
by U-Boot.
We need that in our board-code to enable or disable HS400 for the
eMMC, depending of what module variant we detected from the
information in the eeprom.
Unfortunately, NXP has already implemented that function in their
SOC-code, so it is no more available for the board-code. From my
opinion, the best solution would be to move board_fix_fdt()
from soc.c to all imx9 board codes that need it. Then that
function is again available for the board code.
My proposal: To avoid loosig the focus of that patchset we accept
it now as it is here. After that, I will then take care to send
another patchset to move board_fix_fdt() for all boards accordingly.
Do you agree with that?
>
> > #ifndef CONFIG_XPL_BUILD
> > int board_fix_fdt(void *fdt)
> > {
> > diff --git a/board/phytec/common/Kconfig
> > b/board/phytec/common/Kconfig
> > index f394ace786..bc5511707a 100644
> > --- a/board/phytec/common/Kconfig
> > +++ b/board/phytec/common/Kconfig
> > @@ -19,6 +19,14 @@ config PHYTEC_IMX8M_SOM_DETECTION
> > Support of I2C EEPROM based SoM detection. Supported
> > for PHYTEC i.MX8MM/i.MX8MP boards
> >
> > +config PHYTEC_IMX93_SOM_DETECTION
> > + bool "Support SoM detection for i.MX93 PHYTEC platforms"
> > + depends on ARCH_IMX9 && PHYTEC_SOM_DETECTION
> > + default y
> > + help
> > + Support of I2C EEPROM based SoM detection. Supported
> > + for PHYTEC i.MX93 based boards
> > +
> > config PHYTEC_AM62_SOM_DETECTION
> > bool "Support SoM detection for AM62x PHYTEC platforms"
> > depends on (TARGET_PHYCORE_AM62X_A53 ||
> > TARGET_PHYCORE_AM62X_R5) && \
> > diff --git a/board/phytec/common/Makefile
> > b/board/phytec/common/Makefile
> > index cd78f7686f..8126f7356e 100644
> > --- a/board/phytec/common/Makefile
> > +++ b/board/phytec/common/Makefile
> > @@ -10,3 +10,4 @@ endif
> > obj-y += phytec_som_detection.o phytec_som_detection_blocks.o
> > obj-$(CONFIG_ARCH_K3) += am6_som_detection.o k3/
> > obj-$(CONFIG_ARCH_IMX8M) += imx8m_som_detection.o
> > +obj-$(CONFIG_ARCH_IMX9) += imx93_som_detection.o
> > diff --git a/board/phytec/common/imx93_som_detection.c
> > b/board/phytec/common/imx93_som_detection.c
> > new file mode 100644
> > index 0000000000..a28cdf6b51
> > --- /dev/null
> > +++ b/board/phytec/common/imx93_som_detection.c
> > @@ -0,0 +1,111 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2024 PHYTEC Messtechnik GmbH
> > + * Author: Primoz Fiser <primoz.fiser at norik.com>
> > + */
> > +
> > +#include <asm/arch/sys_proto.h>
> > +#include <dm/device.h>
> > +#include <dm/uclass.h>
> > +#include <i2c.h>
> > +#include <u-boot/crc.h>
> > +
> > +#include "imx93_som_detection.h"
> > +
> > +extern struct phytec_eeprom_data eeprom_data;
> > +
> > +#if IS_ENABLED(CONFIG_PHYTEC_IMX93_SOM_DETECTION)
> > +
> > +/* Check if the SoM is actually one of the following products:
> > + * - i.MX93
> > + *
> > + * Returns 0 in case it's a known SoM. Otherwise, returns 1.
> > + */
> > +u8 __maybe_unused phytec_imx93_detect(struct phytec_eeprom_data
> > *data)
> > +{
> > + u8 som;
> > +
> > + if (!data)
> > + data = &eeprom_data;
> > +
> > + /* Early API revisions are not supported */
> > + if (!data->valid || data->payload.api_rev <
> > PHYTEC_API_REV2)
> > + return 1;
> > +
> > + som = data->payload.data.data_api2.som_no;
> > + debug("%s: som id: %u\n", __func__, som);
> > +
> > + if (som == PHYTEC_IMX93_SOM && is_imx93())
> > + return 0;
> > +
> > + pr_err("%s: SoM ID does not match. Wrong EEPROM data?\n",
> > __func__);
> > + return 1;
> > +}
> > +
> > +/*
> > + * Filter PHYTEC i.MX93 SoM options by option index
> > + *
> > + * Returns:
> > + * - option value
> > + * - PHYTEC_EEPROM_INVAL when the data is invalid
> > + *
> > + */
> > +u8 __maybe_unused phytec_imx93_get_opt(struct phytec_eeprom_data
> > *data,
> > + enum
> > phytec_imx93_option_index idx)
> > +{
> > + char *opt;
> > + u8 opt_id;
> > +
> > + if (!data)
> > + data = &eeprom_data;
> > +
> > + if (!data->valid || data->payload.api_rev <
> > PHYTEC_API_REV2)
> > + return PHYTEC_EEPROM_INVAL;
> > +
> > + opt = phytec_get_opt(data);
> > + if (opt)
> > + opt_id = PHYTEC_GET_OPTION(opt[idx]);
> > + else
> > + opt_id = PHYTEC_EEPROM_INVAL;
> > +
> > + debug("%s: opt[%d] id: %u\n", __func__, idx, opt_id);
> > + return opt_id;
> > +}
> > +
> > +/*
> > + * Filter PHYTEC i.MX93 SoM voltage
> > + *
> > + * Returns:
> > + * - PHYTEC_IMX93_VOLTAGE_1V8 or PHYTEC_IMX93_VOLTAGE_3V8
> > + * - PHYTEC_EEPROM_INVAL when the data is invalid
> > + *
> > + */
> > +enum phytec_imx93_voltage __maybe_unused
> > phytec_imx93_get_voltage(struct phytec_eeprom_data
> > *data)
> > +{
> > + u8 option = phytec_imx93_get_opt(data,
> > PHYTEC_IMX93_OPT_FEAT);
> > +
> > + if (option == PHYTEC_EEPROM_INVAL)
> > + return PHYTEC_IMX93_VOLTAGE_INVALID;
> > + return (option & 0x01) ? PHYTEC_IMX93_VOLTAGE_1V8 :
> > PHYTEC_IMX93_VOLTAGE_3V3;
> > +}
> > +
> > +#else
> > +
> > +inline u8 __maybe_unused phytec_imx93_detect(struct
> > phytec_eeprom_data *data)
> > +{
> > + return 1;
> > +}
> > +
> > +inline u8 __maybe_unused phytec_imx93_get_opt(struct
> > phytec_eeprom_data *data,
> > + enum
> > phytec_imx93_option_index idx)
> > +{
> > + return PHYTEC_EEPROM_INVAL;
> > +}
> > +
> > +inline enum phytec_imx93_voltage __maybe_unused
> > phytec_imx93_get_voltage
> > + (struct phytec_eeprom_data *data)
> > +{
> > + return PHYTEC_EEPROM_INVAL;
> > +}
> > +
> > +#endif /* IS_ENABLED(CONFIG_PHYTEC_IMX93_SOM_DETECTION) */
> > diff --git a/board/phytec/common/imx93_som_detection.h
> > b/board/phytec/common/imx93_som_detection.h
> > new file mode 100644
> > index 0000000000..6d3d9283d8
> > --- /dev/null
> > +++ b/board/phytec/common/imx93_som_detection.h
> > @@ -0,0 +1,51 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2024 PHYTEC Messtechnik GmbH
> > + * Author: Primoz Fiser <primoz.fiser at norik.com>
> > + */
> > +
> > +#ifndef _PHYTEC_IMX93_SOM_DETECTION_H
> > +#define _PHYTEC_IMX93_SOM_DETECTION_H
> > +
> > +#include "phytec_som_detection.h"
> > +
> > +#define PHYTEC_IMX93_SOM 77
> > +
> > +enum phytec_imx93_option_index {
> > + PHYTEC_IMX93_OPT_DDR = 0,
> > + PHYTEC_IMX93_OPT_EMMC,
> > + PHYTEC_IMX93_OPT_CPU,
> > + PHYTEC_IMX93_OPT_FREQ,
> > + PHYTEC_IMX93_OPT_NPU,
> > + PHYTEC_IMX93_OPT_DISP,
> > + PHYTEC_IMX93_OPT_ETH,
> > + PHYTEC_IMX93_OPT_FEAT,
> > + PHYTEC_IMX93_OPT_TEMP,
> > + PHYTEC_IMX93_OPT_BOOT,
> > + PHYTEC_IMX93_OPT_LED,
> > + PHYTEC_IMX93_OPT_EEPROM,
> > +};
> > +
> > +enum phytec_imx93_voltage {
> > + PHYTEC_IMX93_VOLTAGE_INVALID = PHYTEC_EEPROM_INVAL,
> > + PHYTEC_IMX93_VOLTAGE_3V3 = 0,
> > + PHYTEC_IMX93_VOLTAGE_1V8,
> > +};
> > +
> > +enum phytec_imx93_ddr_eeprom_code {
> > + PHYTEC_IMX93_DDR_INVALID = PHYTEC_EEPROM_INVAL,
> > + PHYTEC_IMX93_LPDDR4X_512MB = 0,
> > + PHYTEC_IMX93_LPDDR4X_1GB = 1,
> > + PHYTEC_IMX93_LPDDR4X_2GB = 2,
> > + PHYTEC_IMX93_LPDDR4_512MB = 3,
> > + PHYTEC_IMX93_LPDDR4_1GB = 4,
> > + PHYTEC_IMX93_LPDDR4_2GB = 5,
> > +};
>
> This is inconsistent within enums. For the ddr, you set each value
> explicitly, but for the others
> you don't. Please choose one style.
>
> Yannic
>
> > [...]
> >
>
>
More information about the U-Boot
mailing list