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

[Russell King (Oracle)]

On Mon, May 27, 2024 at 02:24:32PM +0100, Russell King (Oracle) wrote:
> 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."

Note this will be my last reply on this today - it's Bank Holiday Monday
in the UK and I have the right to take this time off from working on the
kernel.

Thanks.

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