Re: [PATCH] clkdev: report over-sized strings when creating clkdev

[Russell King (Oracle)]

On Mon, May 27, 2024 at 02:18:23PM +0100, Russell King (Oracle) wrote:
> On Mon, May 27, 2024 at 03:45:15AM -0700, Ron Economos wrote:
> > On Fri, May 17, 2024 at 03:09:14PM -0700, Guenter Roeck wrote:
> > > Hi,
> > >
> > > On Fri, Mar 15, 2024 at 11:47:55AM +0000, Russell King (Oracle) wrote:
> > > > Report an error when an attempt to register a clkdev entry results in a
> > > > truncated string so the problem can be easily spotted.
> > > >
> > > > Reported by: Duanqiang Wen <duanqiangwen@xxxxxxxxxxxxx>
> > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@xxxxxxxxxxxxxxx>
> > > > Reviewed-by: Stephen Boyd <sboyd@xxxxxxxxxx>
> > >
> > > With this patch in the mainline kernel, I get
> > >
> > > 10000000.clock-controller:corepll: device ID is greater than 24
> > > sifive-clk-prci 10000000.clock-controller: Failed to register clkdev for
> > corepll: -12
> > > sifive-clk-prci 10000000.clock-controller: could not register clocks: -12
> > > sifive-clk-prci 10000000.clock-controller: probe with driver
> > sifive-clk-prci failed with error -12
> > > ...
> > > platform 10060000.gpio: deferred probe pending: platform: supplier
> > 10000000.clock-controller not ready
> > > platform 10010000.serial: deferred probe pending: platform: supplier
> > 10000000.clock-controller not ready
> > > platform 10011000.serial: deferred probe pending: platform: supplier
> > 10000000.clock-controller not ready
> > > platform 10040000.spi: deferred probe pending: platform: supplier
> > 10000000.clock-controller not ready
> > > platform 10050000.spi: deferred probe pending: platform: supplier
> > 10000000.clock-controller not ready
> > > platform 10090000.ethernet: deferred probe pending: platform: supplier
> > 10000000.clock-controller not ready
> > >
> > > when trying to boot sifive_u in qemu.
> > >
> > > Apparently, "10000000.clock-controller" is too long. Any suggestion on
> > > how to solve the problem ? I guess using dev_name(dev) as dev_id parameter
> > > for clk_hw_register_clkdev() is not or no longer a good idea.
> > > What else should be used instead ?
> > 
> > This issue causes a complete boot failure on real hardware (SiFive
> > Unmatched). The boot only gets as far as "Starting kernel ..." with no other
> > indication of what's going on.
> > 
> > Guenter's suggested patch solves the issue.
> > 
> > diff --git a/drivers/clk/sifive/sifive-prci.c
> > b/drivers/clk/sifive/sifive-prci.c
> > index 25b8e1a80ddc..20cc8f42d9eb 100644
> > --- a/drivers/clk/sifive/sifive-prci.c
> > +++ b/drivers/clk/sifive/sifive-prci.c
> > @@ -537,7 +537,7 @@ static int __prci_register_clocks(struct device *dev,
> > struct __prci_data *pd,
> >                          return r;
> >                  }
> > 
> > -               r = clk_hw_register_clkdev(&pic->hw, pic->name,
> > dev_name(dev));
> > +               r = clk_hw_register_clkdev(&pic->hw, pic->name, "prci");
> 
> How about just changing this to:
> 
> 		r = clk_hw_register(dev, &pic->hw);
> 
> ?
> 
> Since, if the device name is over-sized and thus truncated in the clk
> lookup array that clkdev maintains, *nothing* will be able to match
> the entry. Hence, I suspect all those clkdev registrations are
> completely redundant for this driver (and do nothing other than
> waste memory!)

Note that I mentioned *exactly* this point in my first reply to the
report of the regression in:

https://lore.kernel.org/r/ZkfYqj+OcAxd9O2t@xxxxxxxxxxxxxxxxxxxxx

"We need to think about (a) whether your use of clk_hw_register_clkdev()
is still appropriate, and (b) whether we need to increase the size of
the strings."

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!