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

2024-11-20 Thread Christoph Stoidner


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 
> > ---
> > Cc: Mathieu Othacehe 
> > Cc: Christoph Stoidner 
> > Cc: Stefano Babic 
> > Cc: Fabio Estevam 
> > Cc: "NXP i.MX U-Boot Team" 
> > Cc: Tom Rini 
> > Cc: Yannic Moog 
> > Cc: Primoz Fiser 
> > Cc: Andrej Picej 
> > Cc: Wadim Egorov 
> > ---
> > 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 {
> > +   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_BU

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

2024-11-20 Thread Yannic Moog
On Tue, 2024-11-19 at 17:45 +0100, Christoph Stoidner wrote:
> 
> 
> 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 
> > > ---
> > > Cc: Mathieu Othacehe 
> > > Cc: Christoph Stoidner 
> > > Cc: Stefano Babic 
> > > Cc: Fabio Estevam 
> > > Cc: "NXP i.MX U-Boot Team" 
> > > Cc: Tom Rini 
> > > Cc: Yannic Moog 
> > > Cc: Primoz Fiser 
> > > Cc: Andrej Picej 
> > > Cc: Wadim Egorov 
> > > ---
> > > 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 {
> > > +   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 

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

2024-11-19 Thread Christoph Stoidner
On Do, 2024-11-14 at 09:37 +0100, Primoz Fiser wrote:
> Hi Christoph,
> 
> On 13. 11. 24 17:00, 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 
> > ---
> > Cc: Mathieu Othacehe 
> > Cc: Christoph Stoidner 
> > Cc: Stefano Babic 
> > Cc: Fabio Estevam 
> > Cc: "NXP i.MX U-Boot Team" 
> > Cc: Tom Rini 
> > Cc: Yannic Moog 
> > Cc: Primoz Fiser 
> > Cc: Andrej Picej 
> > Cc: Wadim Egorov 
> > ---
> > 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 {
> > +   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)
> >  #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_so

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

2024-11-14 Thread Christoph Stoidner


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 
> > ---
> > Cc: Mathieu Othacehe 
> > Cc: Christoph Stoidner 
> > Cc: Stefano Babic 
> > Cc: Fabio Estevam 
> > Cc: "NXP i.MX U-Boot Team" 
> > Cc: Tom Rini 
> > Cc: Yannic Moog 
> > Cc: Primoz Fiser 
> > Cc: Andrej Picej 
> > Cc: Wadim Egorov 
> > ---
> > 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 


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

2024-11-14 Thread Christoph Stoidner


On Mi, 2024-11-13 at 15:29 -0300, Fabio Estevam wrote:
> On Wed, Nov 13, 2024 at 1:16 PM Christoph Stoidner
>  wrote:
> 
> > +
> > +   eeprom@50 {
> > +   bootph-pre-ram;
> > +   bootph-some-ram;
> > +   compatible = "atmel,24c32";
> > +   reg = <0x50>;
> > +   pagesize = <32>;
> > +   vcc-supply = <&buck4>;
> > +   };
> > +
> > +   eepromid@58 {
> > +   bootph-pre-ram;
> > +   bootph-some-ram;
> > +   compatible = "atmel,24c32";
> > +   pagesize = <32>;
> > +   reg = <0x58>;
> > +   size = <32>;
> > +   vcc-supply = <&buck4>;
> > +   };
> 
> It is OK to place this in imx93-phyboard-segin-u-boot.dtsi for now,
> but I assume this will also be sent upstream.
> 
> Please confirm.

Thats true. I will upstream that into the linux device tree. And when
the dts is synced back here, I will remove it from the -u-boot.dtsi.



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

2024-11-14 Thread Primoz Fiser
Hi Christoph,

On 13. 11. 24 17:00, 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 
> ---
> Cc: Mathieu Othacehe 
> Cc: Christoph Stoidner 
> Cc: Stefano Babic 
> Cc: Fabio Estevam 
> Cc: "NXP i.MX U-Boot Team" 
> Cc: Tom Rini 
> Cc: Yannic Moog 
> Cc: Primoz Fiser 
> Cc: Andrej Picej 
> Cc: Wadim Egorov 
> ---
> 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 {
> + 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)
>  #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 00..a28cdf6b51
> --- /dev/null
> +++ b/board/phytec/common/imx93_som_detection.c
> @@ -0,0 +1,111 @@

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

2024-11-13 Thread Yannic Moog
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 
> ---
> Cc: Mathieu Othacehe 
> Cc: Christoph Stoidner 
> Cc: Stefano Babic 
> Cc: Fabio Estevam 
> Cc: "NXP i.MX U-Boot Team" 
> Cc: Tom Rini 
> Cc: Yannic Moog 
> Cc: Primoz Fiser 
> Cc: Andrej Picej 
> Cc: Wadim Egorov 
> ---
> 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 {
> + 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.

>  #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
>

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

2024-11-13 Thread Fabio Estevam
On Wed, Nov 13, 2024 at 1:16 PM Christoph Stoidner  wrote:

> +
> +   eeprom@50 {
> +   bootph-pre-ram;
> +   bootph-some-ram;
> +   compatible = "atmel,24c32";
> +   reg = <0x50>;
> +   pagesize = <32>;
> +   vcc-supply = <&buck4>;
> +   };
> +
> +   eepromid@58 {
> +   bootph-pre-ram;
> +   bootph-some-ram;
> +   compatible = "atmel,24c32";
> +   pagesize = <32>;
> +   reg = <0x58>;
> +   size = <32>;
> +   vcc-supply = <&buck4>;
> +   };

It is OK to place this in imx93-phyboard-segin-u-boot.dtsi for now,
but I assume this will also be sent upstream.

Please confirm.


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

2024-11-13 Thread Wadim Egorov




Am 13.11.24 um 19:29 schrieb Fabio Estevam:

On Wed, Nov 13, 2024 at 1:16 PM Christoph Stoidner  wrote:


+
+   eeprom@50 {
+   bootph-pre-ram;
+   bootph-some-ram;
+   compatible = "atmel,24c32";
+   reg = <0x50>;
+   pagesize = <32>;
+   vcc-supply = <&buck4>;
+   };
+
+   eepromid@58 {
+   bootph-pre-ram;
+   bootph-some-ram;
+   compatible = "atmel,24c32";
+   pagesize = <32>;
+   reg = <0x58>;
+   size = <32>;
+   vcc-supply = <&buck4>;
+   };


It is OK to place this in imx93-phyboard-segin-u-boot.dtsi for now,
but I assume this will also be sent upstream.

Please confirm.


Agree, we should add the EEPROM nodes to the Linux device tree.


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

2024-11-13 Thread Wadim Egorov




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 
---
Cc: Mathieu Othacehe 
Cc: Christoph Stoidner 
Cc: Stefano Babic 
Cc: Fabio Estevam 
Cc: "NXP i.MX U-Boot Team" 
Cc: Tom Rini 
Cc: Yannic Moog 
Cc: Primoz Fiser 
Cc: Andrej Picej 
Cc: Wadim Egorov 
---
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,

Reviewed-by: Wadim Egorov