Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

Geert Uytterhoeven
2024-09-10T08:02:40Z

Hi Stephen,

On Mon, Sep 9, 2024 at 11:18 PM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Geert Uytterhoeven (2024-09-09 05:11:03)
> > On Sat, Sep 7, 2024 at 1:01 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > > Quoting Geert Uytterhoeven (2024-09-06 00:28:38)
> > > >
> > > > My main objections are that (1) this approach is different than the one used
> > > > for all other external clock inputs on Renesas SoCs, and (2) this requires
> > > > duplicating part of the clocks property in all board DTS files.
> > >
> > > Can 'clock-ranges' be used here? Leave the cell as null in the SoC dtsi
> > > file and then fill it in with clocks property at the parent node. I
> > > think you'd have to use clock-names for this though.
> >
> > "clock-ranges" does not seem to be well-documented...
>
> Yeah, I wasn't aware of it for years!
>
> > IUIC, your suggestion is to:
> > 1. Add "clock-ranges" to the /soc subnode,
> > 2. Completely leave out the "rtx" clock from the clocks property
> > of the vbattb@1005c000 node,
> > 3. Add the following to the board DTS:
> >
> > &soc {
> > clocks = <&vbattb_xtal>;
> > clock-names = "rtx";
> > };
> >
> > Then, when resolving "rtx" for the vbattb@1005c000 node,
> > of_parse_clkspec() would iterate up and find the proper vbattb_xtal.
> > Is that correct? And probably that should be done for other external
> > clock inputs as well?
>
> Sounds about right.
>
> > Still, it looks a bit complicated and un-intuitive. And what about
> > e.g. carrier boards with a SoM, where some clocks are provided by
> > the SoM, and some by the carrier? In that case you still have to
> > override the clock and clock-names properties in the carrier .dts,
> > thus duplicating all clocks provided by the SoM.
>
> This is the same case as the board wanting to override the soc node?

Yes, but more complicated,

> When it's a SoM is there a node for the SoM? Is the clock on the SoM?
> Does this case exist? Hopefully this isn't a straw man.

E.g. the White Hawk CPU board[1] contains extal_clk, extalr_clk, and
scif_clk, so it would need something like:

&soc {
clocks = <&extal_clk>, <&extalr_clk>, <&scif_clk>;
clocks-names = "extal", "extalr", "scif";
};

The White Hawk Break-Out board[2] contains can_clk, so it would
need to append that, by overriding (duplicate + append):

&soc {
clocks = <&extal_clk>, <&extalr_clk>, <&scif_clk>, <&can_clk>;
clocks-names = "extal", "extalr", "scif", "can";
};

Currently, [1] does:

&extal_clk {
clock-frequency = <16666666>;
};

&extalr_clk {
clock-frequency = <32768>;
};

&scif_clk {
clock-frequency = <24000000>;
};

And [2] does:

&can_clk {
clock-frequency = <40000000>;
};

[1] arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi:
[2] arch/arm64/boot/dts/renesas/white-hawk-common.dtsi

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds