Re: [Upstream] [PATCH 3/3] board: phytec: imx93: Add phyCORE-i.MX93 support all SOM variants

[Wadim Egorov <w.egorov@phytec.de>]

Am 08.11.24 um 19:02 schrieb Christoph Stoidner:
> Hi Wadim,
> 
> On Di, 2024-11-05 at 08:42 +0100, Wadim Egorov wrote:
>> Hi Christoph,
>>
>> Am 04.11.24 um 12:25 schrieb Christoph Stoidner:
>>> The phyCORE-i.MX 93 is available in various variants (e.g.
>>> different ram
>>> sizes, eMMC HS400 yes/no). Add a new SOM-scoped defconfig that
>>> makes use
>>> of the hardware introspection of the phycore-imx93 board-code, to
>>> detect
>>> the SOM module variant, and to configure the hardware accordingly.
>>> The
>>> resulting SPL and u-boot binary shall able to boot each phyCORE-
>>> i.MX93
>>> module variant on each carrier board.
>>
>> I think it would be better to simply rename the current
>> imx93-phyboard-segin_defconfig to phycore_imx93_defconfig. This is
>> less
>> confusing and follows the idea of our other SoMs.
> 
> I see. My original reason to keep the existing
> imx93-phyboard-segin_defconfig was to avoid any breaking change. And I
> thought limiting the existing one to the SOM would be a breaking
> change. But a further look to the imx93-phyboard-segin_defconfig
> confirms what you said. It is already scoped to the SOM, only the name
> "phyboard" is confusing.
> 
> So, I will rename imx93-phyboard-segin_defconfig and just add the SOM
> detection. I will send a v2 for that.
> 
> Just one more note: Other than you suggested above I will use the final
> name "imx93-phycore_defconfig" instead of "phycore_imx93_defconfig".
> Although this does not match the Phytec names, it does fit the existing
> naming scheme in the upstream u-boot. I talked a while ago with Teresa
> about that. The existing phytec names have historically grown, and
> later the naming scheme with imx* prefix has established in upstream.
> Teresa and me agreed that we should use the upstream way.
> 
>>
>>>
>>> Signed-off-by: Christoph Stoidner <c.stoidner@phytec.de>
>>> Cc: Mathieu Othacehe <m.othacehe@gmail.com>, Christoph Stoidner
>>> <c.stoidner@phytec.de>, Stefano Babic <sbabic@denx.de>, Fabio
>>> Estevam <festevam@gmail.com>, "NXP i.MX U-Boot Team" <uboot-
>>> imx@nxp.com>, Tom Rini <trini@konsulko.com>, Yannic Moog
>>> <y.moog@phytec.de>, Primoz Fiser <primoz.fiser@norik.com>, Andrej
>>> Picej <andrej.picej@norik.com>, Wadim Egorov <w.egorov@phytec.de>
>>> ---
>>>    arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi |  14 +-
>>>    board/phytec/phycore_imx93/MAINTAINERS        |   1 +
>>>    configs/imx93-phycore_defconfig               | 156
>>> ++++++++++++++++++
>>>    3 files changed, 169 insertions(+), 2 deletions(-)
>>>    create mode 100644 configs/imx93-phycore_defconfig
>>>
>>> diff --git a/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
>>> b/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
>>> index 25c778bb07..e84476c38a 100644
>>> --- a/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
>>> +++ b/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi
>>> @@ -2,15 +2,25 @@
>>>    /*
>>>     * 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"
>>>    
>>>    / {
>>> +
>>> +       /*
>>> +        * If the u-boot build uses the device tree of a phyCORE-
>>> i.MX93 carrier
>>> +        * board (i.E. imx93-phyboard-segin.dts), then this u-
>>> boot.dtsi
>>> +        * deactivates all carrier board-specific peripherals. This
>>> means that
>>> +        * the resulting SPL and u-boot binary can boot the
>>> phyCORE-i.MX 93 module
>>> +        * on each carrier board.
>>> +        */
>> This comment does not seem to reflect what
>> imx93-phyboard-segin-u-boot.dtsi is actually doing.
> 
> I dont know what you mean. Do you see any carrier board peripherals
> that keep activated? Or anything else? What missmatch do you mean?

You are not deactivating anything in this u-boot.dtsi file so the 
comment does not make a lot of sense to me.

Regards,
Wadim