Re: [PATCH v2 2/2] arm: dts: k3-am642-evm/sk: Enable OSPI support in SPL

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2906 bytes --]

On Sat, Feb 24, 2024 at 11:34:36AM -0600, Jon Humphreys wrote:
> Tom Rini <trini@konsulko.com> writes:
> 
> > On Fri, Feb 23, 2024 at 06:17:02PM -0600, Jonathan Humphreys wrote:
> >> Add bootph DT tags to enable OSPI in SPL.
> >> Set OSPI regs for R5 SPL to address OSPI's boot region.
> >> 
> >> Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com>
> >> ---
> >>  arch/arm/dts/k3-am642-evm-u-boot.dtsi | 16 ++++++++++++++++
> >>  arch/arm/dts/k3-am642-r5-evm.dts      |  5 +++++
> >>  arch/arm/dts/k3-am642-r5-sk.dts       |  5 +++++
> >>  arch/arm/dts/k3-am642-sk-u-boot.dtsi  | 16 ++++++++++++++++
> >>  4 files changed, 42 insertions(+)
> >> 
> >> diff --git a/arch/arm/dts/k3-am642-evm-u-boot.dtsi b/arch/arm/dts/k3-am642-evm-u-boot.dtsi
> >> index b843078243..60b219c0be 100644
> >> --- a/arch/arm/dts/k3-am642-evm-u-boot.dtsi
> >> +++ b/arch/arm/dts/k3-am642-evm-u-boot.dtsi
> >> @@ -182,3 +182,19 @@
> >>  &cpsw_port2 {
> >>  	status = "disabled";
> >>  };
> >> +
> >> +&ospi0_pins_default {
> >> +	bootph-all;
> >> +};
> >> +
> >> +&fss {
> >> +	bootph-all;
> >> +};
> >> +
> >> +&ospi0 {
> >> +	bootph-all;
> >> +
> >> +	flash@0 {
> >> +		bootph-all;
> >> +	};
> >> +};
> >
> > So this gets back to what I was asking in the first series, is this
> > needed in SPL or full U-Boot as well? The bootph-* properties are
> > supposed to be transitive, but originally the tooling didn't handle this
> > and now the tooling handles SPL but not full U-Boot. Which also brings
> > back the is this _needed_ question and is bootph-all right, rather than
> > just the big hammer?
> >
> By "this", are you referring to the original phypattern partition nodes,
> or the ospi0 node itself?  The partition nodes are not needed at all, so
> removed.  The ospi node is needed in both SPL and U-Boot.  In that case,
> using the bootph-all tag is the proper way, correct?
> 
> What do you mean by the 'big hammer'?
> 
> Please advise and thanks.

So, part of the answer is that the documentation isn't as clear and well
formatted as I'd like (aside, include/dm/ofnode.h::ofnode_pre_reloc
comment should be reworded to render better). First, I want to point to
the schema itself:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/bootph.yaml
and then next:
https://docs.u-boot.org/en/latest/develop/driver-model/design.html#pre-relocation-support

And in this case, the "pre" options are a bit less clear as TI platforms
don't do the TPL->SPL->Full U-Boot dance that others like say Rockchip
do but instead the Cortex-R/Cortex-A dance for the K3 architecture.

Which gets back to what I was trying to ask. What, functionally,
requires that property to be present? And then, for the cortex-a
platforms these should be in the upstream dtb and I forget if you said
that's in progress for these platforms or not.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]