* Stephen Boyd <sboyd@xxxxxxxxxx> [190919 16:51]:
> Quoting Tony Lindgren (2019-09-19 07:12:24)
> > Hi,
> >
> > * Tero Kristo <t-kristo@xxxxxx> [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!
There can be multiple clockdomains within a single CM. So again using
the l4_secure_clkctrl as an example, the l4_per CM instance with finer
grained compatible properites becomes:
l4_per_cm: l4_per_cm@1400 {
compatible = "ti,omap4-cm";
reg = <0x1400 0x200>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x1400 0x200>;
l4_per_clkctrl: clk@20 {
compatible = "ti,clkctrl";
reg = <0x20 0x144>;
#clock-cells = <2>;
};
l4_secure_clkctrl: clock@1a0 {
compatible = "ti,clkctrl-omap4-l4-secure", "ti,clkctrl";
reg = <0x1a0 0x28>;
#clock-cells = <2>;
};
};
And then later on as clean-up, we could also update l4_per_clkctrl:
l4_per_clkctrl: clock@20 {
compatible = "ti,clkctrl-omap4-l4-per", "ti,clkctrl";
reg = <0x20 0x144>;
#clock-cells = <2>;
};
Regards,
Tony
