Re: [PATCH v2 2/3] board: phytec: imx93: Add eeprom-based hardware introspection

[Christoph Stoidner <C.Stoidner@phytec.de>]

On Mi, 2024-11-13 at 20:42 +0100, Wadim Egorov wrote:
> 
> 
> Am 13.11.24 um 17:00 schrieb Christoph Stoidner:
> > 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@phytec.de>
> > ---
> > Cc: Mathieu Othacehe <m.othacehe@gmail.com>
> > Cc: Christoph Stoidner <c.stoidner@phytec.de>
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Fabio Estevam <festevam@gmail.com>
> > Cc: "NXP i.MX U-Boot Team" <uboot-imx@nxp.com>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Yannic Moog <y.moog@phytec.de>
> > Cc: Primoz Fiser <primoz.fiser@norik.com>
> > Cc: Andrej Picej <andrej.picej@norik.com>
> > Cc: Wadim Egorov <w.egorov@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@50 {
> > +               bootph-pre-ram;
> > +               bootph-some-ram;
> > +               compatible = "atmel,24c32";
> > +               reg = <0x50>;
> > +               pagesize = <32>;
> > +               vcc-supply = <&buck4>;
> > +       };
> > +
> > +       eepromid@58 {
> 
> Please use a generic node name, 
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-
> devicetree-basics.html#generic-names-recommendation
> 
> With that fixed,

In fact, it is better to remove this eepromid node. 

We want to avoid the use of the id-page anyway, because that ID page
is a very chip-specific feature that complicates finding a compatible
replacement eeprom-chip, whenever needed.

So I will just remove the eepromid node in a v3.

> 
> Reviewed-by: Wadim Egorov <w.egorov@phytec.de>