Re: [PATCH v2 3/3] board: phytec: imx93: Add phyCORE-i.MX 93 support for all SOM variants

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

On Do, 2024-11-14 at 14:27 +0100, Yannic Moog wrote:
> On Thu, 2024-11-14 at 12:48 +0100, Christoph Stoidner wrote:
> > 
> > 
> > On Do, 2024-11-14 at 07:37 +0100, Yannic Moog wrote:
> > > On Wed, 2024-11-13 at 17:00 +0100, Christoph Stoidner wrote:
> > > > The phyCORE-i.MX 93 is available in various variants (e.g.
> > > > different ram
> > > > sizes, eMMC HS400 yes/no). Enable hardware introspection for
> > > > the
> > > > imx93-phyboard-segin_defconfig, so that during startup the SOM
> > > > module
> > > > variant can be detected, and the hardware can be configured
> > > > accordingly.
> > > > The resulting SPL and u-boot binary shall able to boot each
> > > > phyCORE-i.MX 93 module variant on each carrier board. Finally
> > > > rename
> > > > imx93-phyboard-segin_defconfig to imx93-phycore_defconfig, to
> > > > highlight
> > > > its SOM scope.
> > > > 
> > > > 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:
> > > > - just rename imx93-phyboard-segin_defconfig and add needed
> > > > CONFIGs,
> > > >   instead of creating an entirely new second defconfig
> > > > - remove wrong comment from imx93-phyboard-segin-u-boot.dtsi
> > > > - improve commit message
> > > > 
> > > >  arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi               |
> > > > 6
> > > > ++++--
> > > >  board/phytec/phycore_imx93/MAINTAINERS                      |
> > > > 2 +-
> > > >  ...x93-phyboard-segin_defconfig => imx93-phycore_defconfig} |
> > > > 4
> > > > +++-
> > > >  3 files changed, 8 insertions(+), 4 deletions(-)
> > > >  rename configs/{imx93-phyboard-segin_defconfig => imx93-
> > > > phycore_defconfig} (98%)
> > > > 
> > > > diff --git a/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
> > > > b/arch/arm/dts/imx93-phyboard-segin-u-
> > > > boot.dtsi
> > > > index 25c778bb07..2245461032 100644
> > > > --- a/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
> > > > +++ b/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
> > > > @@ -2,15 +2,17 @@
> > > >  /*
> > > >   * Copyright (C) 2023 PHYTEC Messtechnik GmbH
> > > >   * Christoph Stoidner <c.stoidner@phytec.de>
> > > > + * Copyright (C) 2024 PHYTEC Messtechnik GmbH
> > > >   *
> > > >   * Product homepage:
> > > > - * phyBOARD-Segin carrier board is reused for the i.MX93
> > > > design.
> > > > - * https://www.phytec.eu/en/produkte/single-board-
> > > > computer/phyboard-segin-imx6ul/
> > > > +   https://www.phytec.de/produkte/system-on-modules/phycore-
> > > > imx-
> > > > 91-93/
> > > >   */
> > > >  
> > > >  #include "imx93-u-boot.dtsi"
> > > >  
> > > >  / {
> > > > +       model = "PHYTEC phyCORE-i.MX93";
> > > > +
> > > >         wdt-reboot {
> > > >                 compatible = "wdt-reboot";
> > > >                 wdt = <&wdog3>;
> > > > diff --git a/board/phytec/phycore_imx93/MAINTAINERS
> > > > b/board/phytec/phycore_imx93/MAINTAINERS
> > > > index cea817ffdc..718f89a084 100644
> > > > --- a/board/phytec/phycore_imx93/MAINTAINERS
> > > > +++ b/board/phytec/phycore_imx93/MAINTAINERS
> > > > @@ -9,5 +9,5 @@ F:      arch/arm/dts/imx93-phyboard-segin-u-
> > > > boot.dtsi
> > > >  F:      board/phytec/phycore_imx93/
> > > >  F:      board/phytec/common/imx93_som_detection.c
> > > >  F:      board/phytec/common/imx93_som_detection.h
> > > > -F:      configs/imx93-phyboard-segin_defconfig
> > > > +F:      configs/imx93-phycore_defconfig
> > > 
> > > How will you handle this when/if you have another board?
> > 
> > That imx93-phycore_defconfig has no more dependency to the carrier
> > board.
> 
> Not true, 
> CONFIG_DEFAULT_DEVICE_TREE="imx93-phyboard-segin"
> It has a dependency to a board. That is why I am asking.
> 
> Yannic


The imx93-phycore_defconfig uses the imx93-phyboard-segin.dts
as reference, but does only make use of its SOM (imx93-phycore)
peripherals. I will add a according comment into the
imx93-phyboard-segin-u-boot.dtsi to make it clearer.

> 
> >  Thats why I renamed it. The intention for the defconfig is that
> > the resulting SPL and u-boot binary can to boot each phyCORE-i.MX
> > 93
> > module variant on each carrier board.
> > 
> > Thats why the carrier board is no more relevant here.
> 
> > 
> > > 
> > > Also, please update documentation!
> > > 
> > > Yannic
> > > 
> > > >  F:      include/configs/phycore_imx93.h
> > > > diff --git a/configs/imx93-phyboard-segin_defconfig
> > > > b/configs/imx93-phycore_defconfig
> > > > similarity index 98%
> > > > rename from configs/imx93-phyboard-segin_defconfig
> > > > rename to configs/imx93-phycore_defconfig
> > > > index 18a4087e4b..cf9800118a 100644
> > > > --- a/configs/imx93-phyboard-segin_defconfig
> > > > +++ b/configs/imx93-phycore_defconfig
> > > > @@ -6,13 +6,13 @@ CONFIG_SYS_MALLOC_F_LEN=0x20000
> > > >  CONFIG_SPL_LIBCOMMON_SUPPORT=y
> > > >  CONFIG_SPL_LIBGENERIC_SUPPORT=y
> > > >  CONFIG_NR_DRAM_BANKS=2
> > > > +CONFIG_PHYTEC_SOM_DETECTION=y
> > > >  CONFIG_ENV_SOURCE_FILE="phycore_imx93"
> > > >  CONFIG_ENV_SIZE=0x10000
> > > >  CONFIG_ENV_OFFSET=0x700000
> > > >  CONFIG_IMX_CONFIG="arch/arm/mach-imx/imx9/imximage.cfg"
> > > >  CONFIG_DM_GPIO=y
> > > >  CONFIG_DEFAULT_DEVICE_TREE="imx93-phyboard-segin"
> > > > -CONFIG_SPL_TEXT_BASE=0x2049A000
> > > >  CONFIG_AHAB_BOOT=y
> > > >  CONFIG_TARGET_PHYCORE_IMX93=y
> > > >  CONFIG_OF_LIBFDT_OVERLAY=y
> > > > @@ -20,6 +20,7 @@ CONFIG_SYS_MONITOR_LEN=524288
> > > >  CONFIG_SPL_SERIAL=y
> > > >  CONFIG_SPL_DRIVERS_MISC=y
> > > >  CONFIG_SPL_STACK=0x20519dd0
> > > > +CONFIG_SPL_TEXT_BASE=0x2049A000
> > > >  CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
> > > >  CONFIG_SPL_BSS_START_ADDR=0x2051a000
> > > >  CONFIG_SPL_BSS_MAX_SIZE=0x2000
> > > > @@ -31,6 +32,7 @@ CONFIG_SPL_IMX_ROMAPI_LOADADDR=0x88000000
> > > >  CONFIG_SYS_MEMTEST_START=0x80000000
> > > >  CONFIG_SYS_MEMTEST_END=0x90000000
> > > >  CONFIG_REMAKE_ELF=y
> > > > +# CONFIG_ANDROID_BOOT_IMAGE is not set
> > > >  CONFIG_DISTRO_DEFAULTS=y
> > > >  CONFIG_OF_SYSTEM_SETUP=y
> > > >  CONFIG_BOOTCOMMAND="mmc dev ${mmcdev}; if mmc rescan; then if
> > > > run
> > > > loadimage; then run mmcboot;
> > > > else run netboot; fi; fi;"
> > > 
>