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