linux-kernel - Re: [PATCH 04/13] arm64: dts: ti: k3-am65: Enable OSPI nodes at the board level

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a274dabf-3987-0885-54f5-3bd7e7f2d1d5@siemens.com>
Date:   Tue, 8 Aug 2023 07:27:17 +0200
From:   Jan Kiszka <jan.kiszka@...mens.com>
To:     Andrew Davis <afd@...com>, Dhruva Gole <d-gole@...com>,
        Nishanth Menon <nm@...com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Tero Kristo <kristo@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        linux-arm-kernel@...ts.infradead.org
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/13] arm64: dts: ti: k3-am65: Enable OSPI nodes at the
 board level

On 07.08.23 17:18, Andrew Davis wrote:
> On 8/7/23 1:16 AM, Dhruva Gole wrote:
>> Hi Andrew,
>>
>> On 03/08/23 02:23, Andrew Davis wrote:
>>> OSPI nodes defined in the top-level AM65x SoC dtsi files are incomplete
>>> and may not be functional unless they are extended with pinmux and
>>> device information.
>>>
>>> As the attached OSPI device is only known about at the board integration
>>> level, these nodes should only be enabled when provided with this
>>> information.
>>>
>>> Disable the OSPI nodes in the dtsi files and only enable the ones that
>>> are actually pinned out on a given board.
>>>
>>> Signed-off-by: Andrew Davis <afd@...com>
>>> ---
>>>   arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi | 1 +
>>>   arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi            | 2 ++
>>>   arch/arm64/boot/dts/ti/k3-am654-base-board.dts     | 1 +
>>>   3 files changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
>>> b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
>>> index e26bd988e5224..6041862d5aa75 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
>>> +++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
>>> @@ -593,6 +593,7 @@ adc {
>>>   };
>>>   &ospi0 {
>>> +    status = "okay";
>>
>> Ok, so this k3-am65-iot2050 series of DT files seem to be structured in
>> a bit different manner than our SKs and EVMs?
>>
>> The terminologies like advanced, advanced-m2, basic, etc. are a little
>> confusing to me. However, I am wondering if we don't do any status = ..
>> here, and rather make ospi status okays from the iot2050 dts files?
>>
>> Pardon me if I am making an invalid suggestion, I don't have much
>> background on these boards.
>>
> 
> This is a valid question, and yes the IOT2050 DTS organization is
> slightly different than the one we use with our SK/EVMs.
> 
> The way these DT files tend to work is layering more functionality
> or information in each file, starting with the core/most common
> in the base .dtsi, and ending with .dts that is specific to a given
> board. (In that way I would consider instances of "/delete-node/"
> to be an indicator of bad layering, but that is a different topic..)
> 
> Any node that is only partially defined in a layer should be marked
> disabled, and then only enabled in the layer that finally completes
> the node. That is often the pinmux info at the board level.
> 
> In this case, the OSPI nodes are complete after this point, there
> is no additional information given in the DTS files, so we can
> enable it here in this .dtsi file.
> 

Ack, this file is the right place to enable OSPI because all our boards
have OSPI in use, and therefore it is configured at this common level
already.

And the reasons for delete-node is obviously that there is no dtsi file
that describes the AM6528 with its two cores only. If you consider that
bad layering, you should change your dtsi files ;). But I see no real
problem here, that pattern is not uncommon.

Jan

-- 
Siemens AG, Technology
Linux Expert Center