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

[Andrew Davis]

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@xxxxxx>
> > > > ---
> > > >    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