[PATCH 3/4] arm64: dts: imx8mn: Remove unreferenced clock-controller clocks in SPL DTs

[Simon Glass]

Hi Adam,

On Fri, Nov 22, 2024, 14:44 Adam Ford <aford173 at gmail.com> wrote:
>
> On Fri, Nov 22, 2024 at 8:33 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Marek,
> >
> > On Thu, 21 Nov 2024 at 18:59, Marek Vasut <marex at denx.de> wrote:
> > >
> > > Since 50cdd3f74af3 ("clk: imx: clk-imx8mn Fix nand and spi clock parent"),
> > > drivers/clk/imx/clk-imx8mn.c clk_get_by_name(dev, "osc_24m", &osc_24m_clk)
> > > fails with error -22 EINVAL in SPL. This is because clk_get_by_name() in the
> > > end calls fdtdec_parse_phandle_with_args(), which iterates over all phandles
> > > in clock-controller at 30380000 { clocks = <&osc_32k>, <&osc_24m>, ... } node
> > > 'clocks' property in an attempt to find the "osc_24m" clock, but fails to
> > > resolve the &osc_32k phandle and returns with -EINVAL.
> > >
> > > Remove the clocks property phandles which cannot be resolved because their
> > > matching nodes are removed from SPL DTs, so fdtdec_parse_phandle_with_args()
> > > can resolve all the remaining valid nodes and successfully look up the osc_24m
> > > clock.
> > >
> > > Fixes: 50cdd3f74af3 ("clk: imx: clk-imx8mn Fix nand and spi clock parent")
> > > Signed-off-by: Marek Vasut <marex at denx.de>
> > > ---
> > > Cc: "NXP i.MX U-Boot Team" <uboot-imx at nxp.com>
> > > Cc: Bryan Brattlof <bb at ti.com>
> > > Cc: Fabio Estevam <festevam at gmail.com>
> > > Cc: Greg Malysa <greg.malysa at timesys.com>
> > > Cc: Ian Roberts <ian.roberts at timesys.com>
> > > Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > Cc: Jonathan Humphreys <j-humphreys at ti.com>
> > > Cc: Masahisa Kojima <kojima.masahisa at socionext.com>
> > > Cc: Nathan Barrett-Morrison <nathan.morrison at timesys.com>
> > > Cc: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> > > Cc: Simon Glass <sjg at chromium.org>
> > > Cc: Stefano Babic <sbabic at denx.de>
> > > Cc: Sumit Garg <sumit.garg at linaro.org>
> > > Cc: Tim Harvey <tharvey at gateworks.com>
> > > Cc: Tom Rini <trini at konsulko.com>
> > > Cc: u-boot at lists.denx.de
> > > ---
> > > NOTE: For v2025.04
> > > ---
> > >  arch/arm/dts/imx8mn-u-boot.dtsi | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/arm/dts/imx8mn-u-boot.dtsi b/arch/arm/dts/imx8mn-u-boot.dtsi
> > > index 6d80d856365..38e1934aee8 100644
> > > --- a/arch/arm/dts/imx8mn-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mn-u-boot.dtsi
> > > @@ -51,6 +51,10 @@
> > >         /delete-property/ assigned-clocks;
> > >         /delete-property/ assigned-clock-parents;
> > >         /delete-property/ assigned-clock-rates;
> > > +#ifdef CONFIG_SPL_BUILD
> > > +       clocks = <&osc_24m>;
> > > +       clock-names = "osc_24m";
> > > +#endif
> > >  };
> > >
> > >  #ifdef CONFIG_FSL_CAAM
> > > --
> > > 2.45.2
> > >
> >
> > We should not be checking U-Boot CONFIG things in .dts sources, since
> > there is no way it could go upstream.
>
> This file has the -u-boot.dtsi ending, so doesn't that mean it would
> be unique to U-Boot and not be pushed to Linux?
>

No, we upstream all bindings so we should only have things in there on
a temporary basis.

The correct solution is to enhance fdtgrep which I think I explained
on the other thread. Let me know if I can assist with that.

Regards,
Simon

>
>
> adam
> >
> > get_phandle_argc() shows how this is done in dtoc. It seems we will
> > need similar logic in fdtgrep to handle this. You'll be pleased to
> > hear it is written in C :-)
> >
> > BTW the bootph tags should be able to go upstream and be dropped from
> > the .dtsi - the only thing in the dtsi these days in the Binman
> > definition, which is barely started upstream.
> >
> > Regards,
> > Simon