| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5aa9265e-4e93-b221-2cf4-8344b8a0a4b3@denx.de>
Date: Mon, 28 Oct 2024 14:52:04 +0100
From: Heiko Schocher <hs@...x.de>
To: Krzysztof Kozlowski <krzk@...nel.org>, linux-kernel@...r.kernel.org
Cc: Conor Dooley <conor+dt@...nel.org>, Fabio Estevam <festevam@...il.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Rob Herring <robh@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
Shawn Guo <shawnguo@...nel.org>, devicetree@...r.kernel.org,
imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v1 2/2] arm64: dts: imx8mp: add aristainetos3 board
support
Hello Krzysztof,
On 28.10.24 13:44, Krzysztof Kozlowski wrote:
> 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@...x.de>
>>>>>> ---
>>>>>> 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@...x.de>
>>>>>> + */
>>>>>> +
>>>>>> +#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.
Okay, I try to find out, what devices are connected else I remove them.
>>>>
>>>>>
>>>>>> + 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.
Hmm.. I made this based on linux-stable 6.6 and yes, comments
I have from adlink sources.
Again sorry... I was too fast sending my patch after local rebase
to v6.12
>>>>> 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.
I see, and will rework!
BTW: I now finished running
make W=1 O=$BDIR dt_binding_check
make W=1 O=$BDIR dtbs_check
and there are tons of warnings .. yes, a lot come from my changes
(because I wasn't aware of the checks and I made stuff based on linux 6.6
stable and simply just rebased my work with 6.12-rc5 so I have to
apologize!)
But not all warnings come from my files...
I will check, fix, rework my patchset!
Thanks for your comments and patience!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs@...x.de
Powered by blists - more mailing lists