Received-SPF: pass (google.com: domain of linux-kernel+bounces-190699-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Mon, 27 May 2024 14:30:00 +0100
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Ron Economos <re@w6rz.net>
Cc: Guenter Roeck <linux@roeck-us.net>, regressions@lists.linux.dev,
	linux-clk@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-riscv <linux-riscv@lists.infradead.org>
Subject: Re: [PATCH] clkdev: report over-sized strings when creating clkdev
Message-ID: <ZlSK2CcbLaWFRpnR@shell.armlinux.org.uk>
References: <28114882-f8d7-21bf-4536-a186e8d7a22a@w6rz.net>
 <ZlSIH1/etwPQe+bQ@shell.armlinux.org.uk>
 <ZlSJkCtx6eYPCQwG@shell.armlinux.org.uk>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ZlSJkCtx6eYPCQwG@shell.armlinux.org.uk>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>

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@net-swift.com>
> > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > > > Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> > > >
> > > > 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@shell.armlinux.org.uk
> 
> "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!