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

[Andrew Davis <afd@ti.com>]

On 8/8/23 12:27 AM, Jan Kiszka wrote:
> 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@ti.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.

Yup, wasn't finger pointing, that is our layering problem. It actually
looks like an easy fix to add an k3-am652.dtsi with only two cores, I'll
go do that for next cycle.

Andrew

> 
> Jan
>