Received-SPF: pass (google.com: domain of linux-kernel+bounces-190683-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Mon, 27 May 2024 14:24:32 +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: <ZlSJkCtx6eYPCQwG@shell.armlinux.org.uk>
References: <28114882-f8d7-21bf-4536-a186e8d7a22a@w6rz.net>
 <ZlSIH1/etwPQe+bQ@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: <ZlSIH1/etwPQe+bQ@shell.armlinux.org.uk>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>

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."

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