* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
@ 2024-05-27 10:45 ` Ron Economos
0 siblings, 0 replies; 18+ messages in thread
From: Ron Economos @ 2024-05-27 10:45 UTC (permalink / raw)
To: Guenter Roeck, Russell King (Oracle), regressions, linux-clk,
Linux Kernel Mailing List, linux-riscv
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");
if (r) {
dev_warn(dev, "Failed to register clkdev for
%s: %d\n",
init.name, r);
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
2024-05-27 10:45 ` Ron Economos
@ 2024-05-27 11:16 ` Linux regression tracking (Thorsten Leemhuis)
-1 siblings, 0 replies; 18+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2024-05-27 11:16 UTC (permalink / raw)
To: Ron Economos, Guenter Roeck, Russell King (Oracle), regressions,
linux-clk, Linux Kernel Mailing List, linux-riscv
On 27.05.24 12:45, Ron Economos wrote:
> On Fri, May 17, 2024 at 03:09:14PM -0700, Guenter Roeck wrote:
>>
>> 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
>> [...]
>> 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).
Hmmm. That and because nobody afaics has time/motivation to fix this
anytime soon (or am I mistaken there?) makes me wonder if we should
revert this change for now (and remerge it later once the problem this
change exposed was fixed). Or is another solution in sight somewhere?
Ciao, Thorsten
> 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");
> if (r) {
> dev_warn(dev, "Failed to register clkdev for
> %s: %d\n",
> init.name, r);
>
>
>
#regzbot dup:
https://lore.kernel.org/lkml/7eda7621-0dde-4153-89e4-172e4c095d01@roeck-us.net/
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
@ 2024-05-27 11:16 ` Linux regression tracking (Thorsten Leemhuis)
0 siblings, 0 replies; 18+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2024-05-27 11:16 UTC (permalink / raw)
To: Ron Economos, Guenter Roeck, Russell King (Oracle), regressions,
linux-clk, Linux Kernel Mailing List, linux-riscv
On 27.05.24 12:45, Ron Economos wrote:
> On Fri, May 17, 2024 at 03:09:14PM -0700, Guenter Roeck wrote:
>>
>> 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
>> [...]
>> 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).
Hmmm. That and because nobody afaics has time/motivation to fix this
anytime soon (or am I mistaken there?) makes me wonder if we should
revert this change for now (and remerge it later once the problem this
change exposed was fixed). Or is another solution in sight somewhere?
Ciao, Thorsten
> 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");
> if (r) {
> dev_warn(dev, "Failed to register clkdev for
> %s: %d\n",
> init.name, r);
>
>
>
#regzbot dup:
https://lore.kernel.org/lkml/7eda7621-0dde-4153-89e4-172e4c095d01@roeck-us.net/
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
2024-05-27 11:16 ` Linux regression tracking (Thorsten Leemhuis)
@ 2024-05-27 12:57 ` Russell King (Oracle)
-1 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-27 12:57 UTC (permalink / raw)
To: Linux regression tracking (Thorsten Leemhuis)
Cc: Ron Economos, Guenter Roeck, regressions, linux-clk,
Linux Kernel Mailing List, linux-riscv
On Mon, May 27, 2024 at 01:16:06PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 27.05.24 12:45, Ron Economos wrote:
> > On Fri, May 17, 2024 at 03:09:14PM -0700, Guenter Roeck wrote:
> >>
> >> 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
> >> [...]
> >> 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).
>
> Hmmm. That and because nobody afaics has time/motivation to fix this
> anytime soon (or am I mistaken there?) makes me wonder if we should
> revert this change for now (and remerge it later once the problem this
> change exposed was fixed). Or is another solution in sight somewhere?
I'm sorry, but clearly I should tell my employer that I can't do work
for them because there's been a mainline kernel regression, and of
course I should be working on this bank holiday Monday...
No, please wait a bit longer.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
@ 2024-05-27 12:57 ` Russell King (Oracle)
0 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-27 12:57 UTC (permalink / raw)
To: Linux regression tracking (Thorsten Leemhuis)
Cc: Ron Economos, Guenter Roeck, regressions, linux-clk,
Linux Kernel Mailing List, linux-riscv
On Mon, May 27, 2024 at 01:16:06PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 27.05.24 12:45, Ron Economos wrote:
> > On Fri, May 17, 2024 at 03:09:14PM -0700, Guenter Roeck wrote:
> >>
> >> 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
> >> [...]
> >> 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).
>
> Hmmm. That and because nobody afaics has time/motivation to fix this
> anytime soon (or am I mistaken there?) makes me wonder if we should
> revert this change for now (and remerge it later once the problem this
> change exposed was fixed). Or is another solution in sight somewhere?
I'm sorry, but clearly I should tell my employer that I can't do work
for them because there's been a mainline kernel regression, and of
course I should be working on this bank holiday Monday...
No, please wait a bit longer.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
2024-05-27 10:45 ` Ron Economos
@ 2024-05-27 13:18 ` Russell King (Oracle)
-1 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-27 13:18 UTC (permalink / raw)
To: Ron Economos
Cc: Guenter Roeck, regressions, linux-clk, Linux Kernel Mailing List,
linux-riscv
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!)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
@ 2024-05-27 13:18 ` Russell King (Oracle)
0 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-27 13:18 UTC (permalink / raw)
To: Ron Economos
Cc: Guenter Roeck, regressions, linux-clk, Linux Kernel Mailing List,
linux-riscv
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!)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
2024-05-27 13:18 ` Russell King (Oracle)
@ 2024-05-27 13:24 ` Russell King (Oracle)
-1 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-27 13:24 UTC (permalink / raw)
To: Ron Economos
Cc: Guenter Roeck, regressions, linux-clk, Linux Kernel Mailing List,
linux-riscv
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!
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
@ 2024-05-27 13:24 ` Russell King (Oracle)
0 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-27 13:24 UTC (permalink / raw)
To: Ron Economos
Cc: Guenter Roeck, regressions, linux-clk, Linux Kernel Mailing List,
linux-riscv
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!
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
2024-05-27 13:24 ` Russell King (Oracle)
@ 2024-05-27 13:30 ` Russell King (Oracle)
-1 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-27 13:30 UTC (permalink / raw)
To: Ron Economos
Cc: Guenter Roeck, regressions, linux-clk, Linux Kernel Mailing List,
linux-riscv
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!
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] clkdev: report over-sized strings when creating clkdev
@ 2024-05-27 13:30 ` Russell King (Oracle)
0 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-27 13:30 UTC (permalink / raw)
To: Ron Economos
Cc: Guenter Roeck, regressions, linux-clk, Linux Kernel Mailing List,
linux-riscv
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!
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] clk: clkdev: don't fail clkdev_alloc() if over-sized
2024-05-27 10:45 ` Ron Economos
(?)
@ 2024-05-28 8:15 ` Russell King (Oracle)
-1 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-28 8:15 UTC (permalink / raw)
To: linux-clk
Cc: Stephen Boyd, Ron Economos, Samuel Holland, Guenter Roeck,
AngeloGioacchino Del Regno, Dinh Nguyen, Krzysztof Kozlowski,
Michael Turquette, Paul Walmsley, Rob Herring, Yang Li,
linux-kernel, linux-riscv, linux-arm-kernel, regressions
Don't fail clkdev_alloc() if the strings are over-sized. In this case,
the entry will not match during lookup, so its useless. However, since
code fails if we return NULL leading to boot failure, return a dummy
entry with the connection and device IDs set to "bad".
Leave the warning so these problems can be found, and the useless
wasteful clkdev registrations removed.
Fixes: 8d532528ff6a ("clkdev: report over-sized strings when creating clkdev entries")
Closes: https://lore.kernel.org/linux-clk/7eda7621-0dde-4153-89e4-172e4c095d01@roeck-us.net.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
Please try this patch, which should allow the platform to boot, bit will
intentionally issue lots of warnings. There is a separate patch posted
recently that removes the useless registration with clkdev.
drivers/clk/clkdev.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 6a77d7e201a9..2f83fb97c6fb 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -204,8 +204,15 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
pr_err("%pV:%s: %s ID is greater than %zu\n",
&vaf, con_id, failure, max_size);
va_end(ap_copy);
- kfree(cla);
- return NULL;
+
+ /*
+ * Don't fail in this case, but as the entry won't ever match just
+ * fill it with something that also won't match.
+ */
+ strscpy(cla->con_id, "bad", sizeof(cla->con_id));
+ strscpy(cla->dev_id, "bad", sizeof(cla->dev_id));
+
+ return &cla->cl;
}
static struct clk_lookup *
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH] clk: clkdev: don't fail clkdev_alloc() if over-sized
@ 2024-05-28 8:15 ` Russell King (Oracle)
0 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-28 8:15 UTC (permalink / raw)
To: linux-clk
Cc: Stephen Boyd, Ron Economos, Samuel Holland, Guenter Roeck,
AngeloGioacchino Del Regno, Dinh Nguyen, Krzysztof Kozlowski,
Michael Turquette, Paul Walmsley, Rob Herring, Yang Li,
linux-kernel, linux-riscv, linux-arm-kernel, regressions
Don't fail clkdev_alloc() if the strings are over-sized. In this case,
the entry will not match during lookup, so its useless. However, since
code fails if we return NULL leading to boot failure, return a dummy
entry with the connection and device IDs set to "bad".
Leave the warning so these problems can be found, and the useless
wasteful clkdev registrations removed.
Fixes: 8d532528ff6a ("clkdev: report over-sized strings when creating clkdev entries")
Closes: https://lore.kernel.org/linux-clk/7eda7621-0dde-4153-89e4-172e4c095d01@roeck-us.net.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
Please try this patch, which should allow the platform to boot, bit will
intentionally issue lots of warnings. There is a separate patch posted
recently that removes the useless registration with clkdev.
drivers/clk/clkdev.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 6a77d7e201a9..2f83fb97c6fb 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -204,8 +204,15 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
pr_err("%pV:%s: %s ID is greater than %zu\n",
&vaf, con_id, failure, max_size);
va_end(ap_copy);
- kfree(cla);
- return NULL;
+
+ /*
+ * Don't fail in this case, but as the entry won't ever match just
+ * fill it with something that also won't match.
+ */
+ strscpy(cla->con_id, "bad", sizeof(cla->con_id));
+ strscpy(cla->dev_id, "bad", sizeof(cla->dev_id));
+
+ return &cla->cl;
}
static struct clk_lookup *
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH] clk: clkdev: don't fail clkdev_alloc() if over-sized
@ 2024-05-28 8:15 ` Russell King (Oracle)
0 siblings, 0 replies; 18+ messages in thread
From: Russell King (Oracle) @ 2024-05-28 8:15 UTC (permalink / raw)
To: linux-clk
Cc: Stephen Boyd, Ron Economos, Samuel Holland, Guenter Roeck,
AngeloGioacchino Del Regno, Dinh Nguyen, Krzysztof Kozlowski,
Michael Turquette, Paul Walmsley, Rob Herring, Yang Li,
linux-kernel, linux-riscv, linux-arm-kernel, regressions
Don't fail clkdev_alloc() if the strings are over-sized. In this case,
the entry will not match during lookup, so its useless. However, since
code fails if we return NULL leading to boot failure, return a dummy
entry with the connection and device IDs set to "bad".
Leave the warning so these problems can be found, and the useless
wasteful clkdev registrations removed.
Fixes: 8d532528ff6a ("clkdev: report over-sized strings when creating clkdev entries")
Closes: https://lore.kernel.org/linux-clk/7eda7621-0dde-4153-89e4-172e4c095d01@roeck-us.net.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
Please try this patch, which should allow the platform to boot, bit will
intentionally issue lots of warnings. There is a separate patch posted
recently that removes the useless registration with clkdev.
drivers/clk/clkdev.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 6a77d7e201a9..2f83fb97c6fb 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -204,8 +204,15 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
pr_err("%pV:%s: %s ID is greater than %zu\n",
&vaf, con_id, failure, max_size);
va_end(ap_copy);
- kfree(cla);
- return NULL;
+
+ /*
+ * Don't fail in this case, but as the entry won't ever match just
+ * fill it with something that also won't match.
+ */
+ strscpy(cla->con_id, "bad", sizeof(cla->con_id));
+ strscpy(cla->dev_id, "bad", sizeof(cla->dev_id));
+
+ return &cla->cl;
}
static struct clk_lookup *
--
2.30.2
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] clk: clkdev: don't fail clkdev_alloc() if over-sized
2024-05-28 8:15 ` Russell King (Oracle)
(?)
@ 2024-05-28 11:21 ` Ron Economos
-1 siblings, 0 replies; 18+ messages in thread
From: Ron Economos @ 2024-05-28 11:21 UTC (permalink / raw)
To: Russell King (Oracle), linux-clk
Cc: Stephen Boyd, Samuel Holland, Guenter Roeck,
AngeloGioacchino Del Regno, Dinh Nguyen, Krzysztof Kozlowski,
Michael Turquette, Paul Walmsley, Rob Herring, Yang Li,
linux-kernel, linux-riscv, linux-arm-kernel, regressions
On 5/28/24 1:15 AM, Russell King (Oracle) wrote:
> Don't fail clkdev_alloc() if the strings are over-sized. In this case,
> the entry will not match during lookup, so its useless. However, since
> code fails if we return NULL leading to boot failure, return a dummy
> entry with the connection and device IDs set to "bad".
>
> Leave the warning so these problems can be found, and the useless
> wasteful clkdev registrations removed.
>
> Fixes: 8d532528ff6a ("clkdev: report over-sized strings when creating clkdev entries")
> Closes: https://lore.kernel.org/linux-clk/7eda7621-0dde-4153-89e4-172e4c095d01@roeck-us.net.
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>
> Please try this patch, which should allow the platform to boot, bit will
> intentionally issue lots of warnings. There is a separate patch posted
> recently that removes the useless registration with clkdev.
>
> drivers/clk/clkdev.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 6a77d7e201a9..2f83fb97c6fb 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -204,8 +204,15 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
> pr_err("%pV:%s: %s ID is greater than %zu\n",
> &vaf, con_id, failure, max_size);
> va_end(ap_copy);
> - kfree(cla);
> - return NULL;
> +
> + /*
> + * Don't fail in this case, but as the entry won't ever match just
> + * fill it with something that also won't match.
> + */
> + strscpy(cla->con_id, "bad", sizeof(cla->con_id));
> + strscpy(cla->dev_id, "bad", sizeof(cla->dev_id));
> +
> + return &cla->cl;
> }
>
> static struct clk_lookup *
Works good. Here's what it looks like on HiFive Unmatched.
[0.389138] riscv-plic c000000.interrupt-controller: mapped 69 interrupts
with 4 handlers for 9 contexts.
[0.390710] shpchp: Standard Hot Plug PCI Controller Driver version: 0.4
[0.392743] 10000000.clock-controller:corepll: device ID is greater than 24
[0.392792] 10000000.clock-controller:ddrpll: device ID is greater than 24
[0.392820] 10000000.clock-controller:gemgxlpll: device ID is greater
than 24
[0.392847] 10000000.clock-controller:dvfscorepll: device ID is greater
than 24
[0.392876] 10000000.clock-controller:hfpclkpll: device ID is greater
than 24
[0.392903] 10000000.clock-controller:cltxpll: device ID is greater than 24
[0.392929] 10000000.clock-controller:tlclk: device ID is greater than 24
[0.392955] 10000000.clock-controller:pclk: device ID is greater than 24
[0.392981] 10000000.clock-controller:pcie_aux: device ID is greater than 24
[0.394620] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[0.413222] SuperH (H)SCI(F) driver initialized
Tested-by: Ron Economos <re@w6rz.net>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] clk: clkdev: don't fail clkdev_alloc() if over-sized
@ 2024-05-28 11:21 ` Ron Economos
0 siblings, 0 replies; 18+ messages in thread
From: Ron Economos @ 2024-05-28 11:21 UTC (permalink / raw)
To: Russell King (Oracle), linux-clk
Cc: Stephen Boyd, Samuel Holland, Guenter Roeck,
AngeloGioacchino Del Regno, Dinh Nguyen, Krzysztof Kozlowski,
Michael Turquette, Paul Walmsley, Rob Herring, Yang Li,
linux-kernel, linux-riscv, linux-arm-kernel, regressions
On 5/28/24 1:15 AM, Russell King (Oracle) wrote:
> Don't fail clkdev_alloc() if the strings are over-sized. In this case,
> the entry will not match during lookup, so its useless. However, since
> code fails if we return NULL leading to boot failure, return a dummy
> entry with the connection and device IDs set to "bad".
>
> Leave the warning so these problems can be found, and the useless
> wasteful clkdev registrations removed.
>
> Fixes: 8d532528ff6a ("clkdev: report over-sized strings when creating clkdev entries")
> Closes: https://lore.kernel.org/linux-clk/7eda7621-0dde-4153-89e4-172e4c095d01@roeck-us.net.
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>
> Please try this patch, which should allow the platform to boot, bit will
> intentionally issue lots of warnings. There is a separate patch posted
> recently that removes the useless registration with clkdev.
>
> drivers/clk/clkdev.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 6a77d7e201a9..2f83fb97c6fb 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -204,8 +204,15 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
> pr_err("%pV:%s: %s ID is greater than %zu\n",
> &vaf, con_id, failure, max_size);
> va_end(ap_copy);
> - kfree(cla);
> - return NULL;
> +
> + /*
> + * Don't fail in this case, but as the entry won't ever match just
> + * fill it with something that also won't match.
> + */
> + strscpy(cla->con_id, "bad", sizeof(cla->con_id));
> + strscpy(cla->dev_id, "bad", sizeof(cla->dev_id));
> +
> + return &cla->cl;
> }
>
> static struct clk_lookup *
Works good. Here's what it looks like on HiFive Unmatched.
[0.389138] riscv-plic c000000.interrupt-controller: mapped 69 interrupts
with 4 handlers for 9 contexts.
[0.390710] shpchp: Standard Hot Plug PCI Controller Driver version: 0.4
[0.392743] 10000000.clock-controller:corepll: device ID is greater than 24
[0.392792] 10000000.clock-controller:ddrpll: device ID is greater than 24
[0.392820] 10000000.clock-controller:gemgxlpll: device ID is greater
than 24
[0.392847] 10000000.clock-controller:dvfscorepll: device ID is greater
than 24
[0.392876] 10000000.clock-controller:hfpclkpll: device ID is greater
than 24
[0.392903] 10000000.clock-controller:cltxpll: device ID is greater than 24
[0.392929] 10000000.clock-controller:tlclk: device ID is greater than 24
[0.392955] 10000000.clock-controller:pclk: device ID is greater than 24
[0.392981] 10000000.clock-controller:pcie_aux: device ID is greater than 24
[0.394620] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[0.413222] SuperH (H)SCI(F) driver initialized
Tested-by: Ron Economos <re@w6rz.net>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] clk: clkdev: don't fail clkdev_alloc() if over-sized
@ 2024-05-28 11:21 ` Ron Economos
0 siblings, 0 replies; 18+ messages in thread
From: Ron Economos @ 2024-05-28 11:21 UTC (permalink / raw)
To: Russell King (Oracle), linux-clk
Cc: Stephen Boyd, Samuel Holland, Guenter Roeck,
AngeloGioacchino Del Regno, Dinh Nguyen, Krzysztof Kozlowski,
Michael Turquette, Paul Walmsley, Rob Herring, Yang Li,
linux-kernel, linux-riscv, linux-arm-kernel, regressions
On 5/28/24 1:15 AM, Russell King (Oracle) wrote:
> Don't fail clkdev_alloc() if the strings are over-sized. In this case,
> the entry will not match during lookup, so its useless. However, since
> code fails if we return NULL leading to boot failure, return a dummy
> entry with the connection and device IDs set to "bad".
>
> Leave the warning so these problems can be found, and the useless
> wasteful clkdev registrations removed.
>
> Fixes: 8d532528ff6a ("clkdev: report over-sized strings when creating clkdev entries")
> Closes: https://lore.kernel.org/linux-clk/7eda7621-0dde-4153-89e4-172e4c095d01@roeck-us.net.
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>
> Please try this patch, which should allow the platform to boot, bit will
> intentionally issue lots of warnings. There is a separate patch posted
> recently that removes the useless registration with clkdev.
>
> drivers/clk/clkdev.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 6a77d7e201a9..2f83fb97c6fb 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -204,8 +204,15 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
> pr_err("%pV:%s: %s ID is greater than %zu\n",
> &vaf, con_id, failure, max_size);
> va_end(ap_copy);
> - kfree(cla);
> - return NULL;
> +
> + /*
> + * Don't fail in this case, but as the entry won't ever match just
> + * fill it with something that also won't match.
> + */
> + strscpy(cla->con_id, "bad", sizeof(cla->con_id));
> + strscpy(cla->dev_id, "bad", sizeof(cla->dev_id));
> +
> + return &cla->cl;
> }
>
> static struct clk_lookup *
Works good. Here's what it looks like on HiFive Unmatched.
[0.389138] riscv-plic c000000.interrupt-controller: mapped 69 interrupts
with 4 handlers for 9 contexts.
[0.390710] shpchp: Standard Hot Plug PCI Controller Driver version: 0.4
[0.392743] 10000000.clock-controller:corepll: device ID is greater than 24
[0.392792] 10000000.clock-controller:ddrpll: device ID is greater than 24
[0.392820] 10000000.clock-controller:gemgxlpll: device ID is greater
than 24
[0.392847] 10000000.clock-controller:dvfscorepll: device ID is greater
than 24
[0.392876] 10000000.clock-controller:hfpclkpll: device ID is greater
than 24
[0.392903] 10000000.clock-controller:cltxpll: device ID is greater than 24
[0.392929] 10000000.clock-controller:tlclk: device ID is greater than 24
[0.392955] 10000000.clock-controller:pclk: device ID is greater than 24
[0.392981] 10000000.clock-controller:pcie_aux: device ID is greater than 24
[0.394620] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[0.413222] SuperH (H)SCI(F) driver initialized
Tested-by: Ron Economos <re@w6rz.net>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread