| Message ID | 20240507065317.3214186-1-m.szyprowski@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | clk: samsung: Don't register clkdev lookup for the fixed rate clocks | expand |
On Tue, May 7, 2024 at 1:53 AM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Commit 4d11c62ca8d7 ("clkdev: report over-sized strings when creating > clkdev entries") revealed that clock lookup is registered for all fixed > clocks. The mentioned commit added a check if the registered name is not > too long. This fails for some clocks registered for Exynos542x SoCs family. > This lookup is a left-over from early common clock framework days, not > really needed nowadays, so remove it to avoid further issues. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > Here is the fix for the mentioned 4d11c62ca8d7 commit, which fixes > booting of Exynos542x SoCs: > https://lore.kernel.org/all/20240507064434.3213933-1-m.szyprowski@samsung.com/ > > This change is independent fix. I've tested it on all Exynos based boards > I have in my test lab. It would be great if someone could test it on > s3c64xx and s3c24xx based boards. > --- Thanks Marek, it fixes some warnings when booting the E850-96 board with current kernel-next (next-20240508), like: samsung_clk_register_fixed_rate: failed to register clock lookup for clk_rco_i3c_pmic ... But I guess 'ret' variable should be removed now, I'm seeing this build warning after applying this patch: drivers/clk/samsung/clk.c: In function 'samsung_clk_register_fixed_rate': drivers/clk/samsung/clk.c:142:20: warning: unused variable 'ret' [-Wunused-variable] Also, maybe add corresponding "Fixes" tag? Other than that: Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org> Tested-by: Sam Protsenko <semen.protsenko@linaro.org> Thanks! [snip]
On 08.05.2024 22:46, Sam Protsenko wrote: > On Tue, May 7, 2024 at 1:53 AM Marek Szyprowski > <m.szyprowski@samsung.com> wrote: >> Commit 4d11c62ca8d7 ("clkdev: report over-sized strings when creating >> clkdev entries") revealed that clock lookup is registered for all fixed >> clocks. The mentioned commit added a check if the registered name is not >> too long. This fails for some clocks registered for Exynos542x SoCs family. >> This lookup is a left-over from early common clock framework days, not >> really needed nowadays, so remove it to avoid further issues. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> Here is the fix for the mentioned 4d11c62ca8d7 commit, which fixes >> booting of Exynos542x SoCs: >> https://lore.kernel.org/all/20240507064434.3213933-1-m.szyprowski@samsung.com/ >> >> This change is independent fix. I've tested it on all Exynos based boards >> I have in my test lab. It would be great if someone could test it on >> s3c64xx and s3c24xx based boards. >> --- > Thanks Marek, it fixes some warnings when booting the E850-96 board > with current kernel-next (next-20240508), like: > > samsung_clk_register_fixed_rate: failed to register clock lookup > for clk_rco_i3c_pmic > ... > > But I guess 'ret' variable should be removed now, I'm seeing this > build warning after applying this patch: > > drivers/clk/samsung/clk.c: In function 'samsung_clk_register_fixed_rate': > drivers/clk/samsung/clk.c:142:20: warning: unused variable 'ret' > [-Wunused-variable] Indeed I've missed that. I will send v2 then. > Also, maybe add corresponding "Fixes" tag? The problem is which commit should be pointed there. The removed code originates from commit 721c42a351b1 ("clk: samsung: add common clock framework helper functions for Samsung platforms"), but that time such workaround might have been required, so this doesn't match the 'Fixes' tag usage. > Other than that: > > Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org> > Tested-by: Sam Protsenko <semen.protsenko@linaro.org> > > Thanks! > > [snip] > Best regards
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c index b6701905f254..6342ff411757 100644 --- a/drivers/clk/samsung/clk.c +++ b/drivers/clk/samsung/clk.c @@ -151,15 +151,6 @@ void __init samsung_clk_register_fixed_rate(struct samsung_clk_provider *ctx, } samsung_clk_add_lookup(ctx, clk_hw, list->id); - - /* - * Unconditionally add a clock lookup for the fixed rate clocks. - * There are not many of these on any of Samsung platforms. - */ - ret = clk_hw_register_clkdev(clk_hw, list->name, NULL); - if (ret) - pr_err("%s: failed to register clock lookup for %s", - __func__, list->name); } }
Commit 4d11c62ca8d7 ("clkdev: report over-sized strings when creating clkdev entries") revealed that clock lookup is registered for all fixed clocks. The mentioned commit added a check if the registered name is not too long. This fails for some clocks registered for Exynos542x SoCs family. This lookup is a left-over from early common clock framework days, not really needed nowadays, so remove it to avoid further issues. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- Here is the fix for the mentioned 4d11c62ca8d7 commit, which fixes booting of Exynos542x SoCs: https://lore.kernel.org/all/20240507064434.3213933-1-m.szyprowski@samsung.com/ This change is independent fix. I've tested it on all Exynos based boards I have in my test lab. It would be great if someone could test it on s3c64xx and s3c24xx based boards. --- drivers/clk/samsung/clk.c | 9 --------- 1 file changed, 9 deletions(-)