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

[Geert Uytterhoeven]

Hi Stephen,

On Sat, Sep 7, 2024 at 1:01 AM Stephen Boyd <sboyd@xxxxxxxxxx> wrote:
> Quoting Geert Uytterhoeven (2024-09-06 00:28:38)
> > On Thu, Sep 5, 2024 at 8:09 PM Stephen Boyd <sboyd@xxxxxxxxxx> wrote:
> > > Quoting claudiu beznea (2024-09-04 05:17:30)
> > > > On 03.09.2024 22:48, Stephen Boyd wrote:
> > > > > The node name should be something like clock-<frequency> but if the
> > > > > frequency is different per-board then I don't know what should happen
> > > > > here.
> > > >
> > > > The frequency should be always around 32768 Hz but not necessarily exactly
> > > > 32768 Hz. It depends on what is installed on the board, indeed. RTC can do
> > > > time error adjustments based on the variations around 32768 Hz.
> > > >
> > > > > Can you leave the vbattb_xtal phandle up above and then require
> > > > > the node to be defined in the board with the proper frequency after the
> > > > > dash?
> > > >
> > > > Is it OK for you something like this (applied on top of this series)?
> > >
> > > Yes, it's too bad we can't append to a property in DT, or somehow leave
> > > alone certain cells and only modify one of them.
> >
> > 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...

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?

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.

So I prefer the original approach, like is done for all other external
SoC clock inputs on Renesas SoCs.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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