On 28/10/2024 12:21, Heiko Schocher wrote:
> Hello Krzysztof,
>
> On 28.10.24 11:49, Krzysztof Kozlowski wrote:
>> On 28/10/2024 11:41, Heiko Schocher wrote:
>>> Hello Krzysztof,
>>>
>>> On 28.10.24 11:24, Krzysztof Kozlowski wrote:
>>>> On 28/10/2024 09:23, Heiko Schocher wrote:
>>>>> Add support for the i.MX8MP based aristainetos3 boards from ABB.
>>>>>
>>>>> The board uses a ABB specific SoM from ADLink, based on NXP
>>>>> i.MX8MP SoC. The SoM is used on 3 different carrier boards,
>>>>> with small differences, which are all catched up in
>>>>> devicetree overlays. The kernel image, the basic dtb
>>>>> and all dtbos are collected in a fitimage. As bootloader
>>>>> is used U-Boot which detects in his SPL stage the carrier
>>>>> board by probing some i2c devices. When the correct
>>>>> carrier is probed, the SPL applies all needed dtbos to
>>>>> the dtb with which U-Boot gets loaded. Same principle
>>>>> later before linux image boot, U-Boot applies the dtbos
>>>>> needed for the carrier board before booting Linux.
>>>>>
>>>>> Signed-off-by: Heiko Schocher <hs@xxxxxxx>
>>>>> ---
>>>>> checkpatch dropped the following warnings:
>>>>> arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi:248: warning: DT compatible string "ethernet-phy-id2000.a231" appears un-documented -- check ./Documentation/devicetree/bindings/
>>>>>
>>>>> ignored, as this compatible string is usedin other dts too, for example in
>>>>>
>>>>> arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi
>>>>>
>>>>> arch/arm64/boot/dts/freescale/Makefile | 5 +
>>>>> .../imx8mp-aristainetos3-adpismarc.dtsi | 64 +
>>>>> .../imx8mp-aristainetos3-adpismarc.dtso | 14 +
>>>>> .../imx8mp-aristainetos3-helios-lvds.dtsi | 89 ++
>>>>> .../imx8mp-aristainetos3-helios-lvds.dtso | 13 +
>>>>> .../imx8mp-aristainetos3-helios.dtsi | 103 ++
>>>>> .../imx8mp-aristainetos3-helios.dtso | 13 +
>>>>> .../imx8mp-aristainetos3-proton2s.dtsi | 176 +++
>>>>> .../imx8mp-aristainetos3-proton2s.dtso | 13 +
>>>>> .../imx8mp-aristainetos3a-som-v1.dts | 18 +
>>>>> .../imx8mp-aristainetos3a-som-v1.dtsi | 1210 +++++++++++++++++
>>>>> 11 files changed, 1718 insertions(+)
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtsi
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtso
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtsi
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios-lvds.dtso
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios.dtsi
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-helios.dtso
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-proton2s.dtsi
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-proton2s.dtso
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dts
>>>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
>>>>> index 9d3df8b218a2..7c3586509b8b 100644
>>>>> --- a/arch/arm64/boot/dts/freescale/Makefile
>>>>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>>>>> @@ -163,6 +163,11 @@ imx8mn-tqma8mqnl-mba8mx-usbotg-dtbs += imx8mn-tqma8mqnl-mba8mx.dtb imx8mn-tqma8m
>>>>> dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx-lvds-tm070jvhg33.dtb
>>>>> dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx-usbotg.dtb
>>>>>
>>>>> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-aristainetos3a-som-v1.dtb \
>>>>> + imx8mp-aristainetos3-adpismarc.dtbo \
>>>>> + imx8mp-aristainetos3-proton2s.dtbo \
>>>>> + imx8mp-aristainetos3-helios.dtbo \
>>>>> + imx8mp-aristainetos3-helios-lvds.dtbo
>>>>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-beacon-kit.dtb
>>>>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-data-modul-edm-sbc.dtb
>>>>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-debix-model-a.dtb
>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtsi
>>>>> new file mode 100644
>>>>> index 000000000000..cc0cddaa33ea
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-adpismarc.dtsi
>>>>> @@ -0,0 +1,64 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>> +/*
>>>>> + * Copyright (C) 2024 Heiko Schocher <hs@xxxxxxx>
>>>>> + */
>>>>> +
>>>>> +#include <dt-bindings/gpio/gpio.h>
>>>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>>>> +
>>>>> +&ecspi1 {
>>>>> + spidev0: spi@0 {
>>>>> + reg = <0>;
>>>>> + compatible = "rohm,dh2228fv";
>>>>
>>>> Hm? I have some doubts, what device is here?
>>>
>>> $ grep -lr dh2228fv drivers/
>>> drivers/spi/spidev.c
>>>
>>> Customer uses an userspace implementation...
>>
>> That's not the question. I asked what device is here.
>
> I do not know, as on carrier boards there are only connectors,
> to which a spi device can be attached. So may I need to use here
> a more generic entry?
So this description is just not true. You have here nothing connected
and this cannot be in the DTS.
>
>>>
>>>>
>>>>> + spi-max-frequency = <500000>;
>>>>> + };
>>>>> +};
>>>>> +
>>>>> +&ecspi2 {
>>>>> + spidev1: spi@0 {
>>>>> + reg = <0>;
>>>>> + compatible = "rohm,dh2228fv";
>>>>> + spi-max-frequency = <500000>;
>>>>> + };
>>>>> +};
>>>>> +
>>>>> +&i2c2 {
>>>>> + /* SX1509(2) U1001@IPi SMARC Plus */
>>>>> + gpio8: i2c2_gpioext0@3e {
>>>>
>>>> Uh, no, please never send us downstream code.
>>>>
>>>> Please follow DTS coding style in all upstream submissions.
>>>
>>> driver is in here:
>>>
>>> $ grep -lr probe-reset drivers/pinctrl/
>>> drivers/pinctrl/pinctrl-sx150x.c
>>
>> This so not related... Your driver does not matter. You send us poor
>> quality downstream code.
>
> The driver is upstream... see:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/pinctrl-sx150x.c
>
> or may I misunderstood you here too?
>
> Poor is my dts, checks are running and I fix them.
My comment was that I see this as you sent DTS code which is taken
straight from some downstream code.
>>>> And why this is DTSO, I have no clue...
>>
>> Why is this a DTSO, not a DTS?
>
> Hmm... the idea is, that the bootloader applies the dtbo on runtime,
> when it has detected the carrier board it runs on, I tried to explain
> in cover letter.
Then there is some mess here. First, SoM cannot be DTS, because it
cannot be booted. Second, specific board/carrier is the DTS. Third,
overlays bring some subset of features, not new board.
Best regards,
Krzysztof
