Re: [PATCH] clk: ti: clkctrl: Fix hidden dependency to node name with reg-names

[Stephen Boyd <sboyd@kernel.org>]

Quoting Tony Lindgren (2019-09-19 07:12:24)
> Hi,
> 
> * Tero Kristo <t-kristo@ti.com> [190919 06:46]:
> > On 06/09/2019 00:55, Tony Lindgren wrote:
> > > diff --git a/Documentation/devicetree/bindings/clock/ti-clkctrl.txt b/Documentation/devicetree/bindings/clock/ti-clkctrl.txt
> > > --- a/Documentation/devicetree/bindings/clock/ti-clkctrl.txt
> > > +++ b/Documentation/devicetree/bindings/clock/ti-clkctrl.txt
> > > @@ -20,15 +20,19 @@ Required properties :
> > >   - #clock-cells : shall contain 2 with the first entry being the instance
> > >              offset from the clock domain base and the second being the
> > >              clock index
> > > +- reg : clock registers
> > > +- reg-names : clock register names for the clock, should be same as the
> > > +         domain name
> > 
> > Hmm, I think using the reg-names property like this is kind of wrong.
> > Basically, reg and reg-names have pretty much nothing in common. Shouldn't
> > you instead use something like ti,clkdm-name? This also breaks with SoCs
> > like am3, which have mutant clkctrl entries like the one here:
> > 
> >                 l4ls_clkctrl: l4ls-clkctrl@38 {
> >                         compatible = "ti,clkctrl";
> >                         reg = <0x38 0x2c>, <0x6c 0x28>, <0xac 0xc>, <0xc0
> > 0x1c>, <0xec 0xc>, <0x10c 0x8>, <0x130 0x4>;
> >                         #clock-cells = <2>;
> >                 };
> > 
> > What would you think single entry in reg-names would mean in this case?
> 
> Oh right, I forgot about the mixed register case again.
> These are all in l4ls domain..
> 
> So sounds like the best option is just to allow adding more
> specific compatible values like this for the omap4 rng case:
> 
>         l4_secure_clkctrl: clock@1a0 {
>                 compatible = "ti,clkctrl-omap4-l4-secure", "ti,clkctrl";
>                 reg = <0x1a0 0x28>;
>                 #clock-cells = <2>;
>         };
> 
> And then use match data to get the domain name on init.
> 

The existing ti,clkctrl binding is pretty weird. I still believe that
the CM container node should be the only node and it should be logic in
the driver that describes the clks provided by the CM node. I guess I
have to just ignore this stuff because it's all working!