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

[Stephen Boyd <sboyd@kernel.org>]

Quoting Tony Lindgren (2019-09-08 12:42:41)
> * Stephen Boyd <sboyd@kernel.org> [190907 03:55]:
> > Quoting Tony Lindgren (2019-09-05 14:55:32)
> > > We currently have a hidden dependency to the device tree node name for
> > > the clkctrl clocks. Instead of using standard node name like "clock", we
> > > must use "l4-per-clkctrl" naming so the clock driver can find the
> > 
> > The node name is "clk" though.
> 
> Yes for some it's "clk" and for some it's "l4-per-clkctrl".
> 
> In general, the clock node name should be "clock@foo", rather than
> "clk@foo", right?

I don't think it really matters but sure, clock is nicer because that's
a more standard node name than the vowel-less one.

> 
> > > associated clock domain. Further, if "clk" is specified for a clock node
> > > name, the driver sets TI_CLK_CLKCTRL_COMPAT flag that uses different
> > > logic with earlier naming for the clock node name.
> > > 
> > > If the clock node naming dependency is not understood, the related
> > > clockdomain is not found, or a wrong one can get used if a clock manager
> > > instance has multiple domains.
> > > 
> > > As each clkctrl instance represents a single clock domain with it's
> > > reg property describing the clocks available in that clock domain,
> > > we can simply use "reg-names" property for the clock domain.
> > > 
> > > This simplifies things and removes the hidden dependency to the node
> > > name. And then later on, we should be able to drop the related code
> > > for parsing the node names.
> > > 
> > > Let's also update the binding to use standard "clock" node naming
> > > instead of "clk".
> > > 
> > > Cc: devicetree@vger.kernel.org
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > > ---
> > >  Documentation/devicetree/bindings/clock/ti-clkctrl.txt |  6 +++++-
> > >  drivers/clk/ti/clkctrl.c                               | 10 ++++++++--
> > >  2 files changed, 13 insertions(+), 3 deletions(-)
> > > 
> > > 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
> > 
> > Is this necessary? I'd rather see that the names of the clks don't
> > actually matter by means of connecting the clk tree through the "clocks"
> > property when the parent clks are provided by external nodes and through
> > direct pointers when they're within a controller. If that works then it
> > should be possible to ignore this name in general?
> 
> This is not for names of the clocks :) This is to name the clock
> provider register range. The name of the register range is the
> same as the clockdomain that this range of clocks belongs to.
> This property is used by the clock provider on init to initialize the
> clock provider, not when a clock is requested.
> 
> In this case a clkctrl clock provider instance has one to some tens
> clocks where they all belong to the same domain. If some similar clock
> would have multiple domains, then it would simply just have multiple
> reg ranges and multiple reg-names properties.
> 
> Or do you have some better ideas on how to name a clock controller
> in the device tree?
> 

Why does the name of the clock controller or clkdm_name matter? Using a
string from reg-names smells like a workaround to describe some sort of
linkage between things that isn't being described in DT today.