| Message ID | 20181002115210.15541-1-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | clk: samsung: Use clk_hw API for calling clk framework from clk notifiers | expand |
On 10/02/2018 01:52 PM, Marek Szyprowski wrote: > clk_notifier_register() documentation states, that the provided notifier > callbacks associated with the notifier must not re-enter into the clk > framework by calling any top-level clk APIs. Fix this by replacing > clk_get_rate() calls with clk_hw_get_rate(), which is safe in this > context. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Thanks for fixing this, I have applied the patch to samsung-clk tree.
Quoting Marek Szyprowski (2018-10-02 04:52:10) > @@ -432,7 +432,7 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, > else > cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb; > > - cpuclk->alt_parent = __clk_lookup(alt_parent); > + cpuclk->alt_parent = __clk_get_hw(__clk_lookup(alt_parent)); It would be nice to get rid of __clk_lookup() from here too. Please try to do that if at all possible. > if (!cpuclk->alt_parent) { > pr_err("%s: could not lookup alternate parent %s\n", > __func__, alt_parent);
Hi Stephen, On 2018-10-08 04:47, Stephen Boyd wrote: > Quoting Marek Szyprowski (2018-10-02 04:52:10) >> @@ -432,7 +432,7 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, >> else >> cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb; >> >> - cpuclk->alt_parent = __clk_lookup(alt_parent); >> + cpuclk->alt_parent = __clk_get_hw(__clk_lookup(alt_parent)); > It would be nice to get rid of __clk_lookup() from here too. Please try > to do that if at all possible. So far it won't be possible to get rid of __clk_lookup() completely from samsung/clk-cpu.c, because there is a need to register clk notifier for CPU clock, but there is no clk_hw_notifier_register() api. I'm not sure if this is the right way to add it. > ... Best regards
Quoting Marek Szyprowski (2018-10-08 07:51:30) > Hi Stephen, > > On 2018-10-08 04:47, Stephen Boyd wrote: > > Quoting Marek Szyprowski (2018-10-02 04:52:10) > >> @@ -432,7 +432,7 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, > >> else > >> cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb; > >> > >> - cpuclk->alt_parent = __clk_lookup(alt_parent); > >> + cpuclk->alt_parent = __clk_get_hw(__clk_lookup(alt_parent)); > > It would be nice to get rid of __clk_lookup() from here too. Please try > > to do that if at all possible. > > So far it won't be possible to get rid of __clk_lookup() completely from > samsung/clk-cpu.c, because there is a need to register clk notifier for > CPU clock, but there is no clk_hw_notifier_register() api. I'm not sure if > this is the right way to add it. > Probably there needs to be some way to indicate that the alt_parent is going here and then grab the clk_hw pointer to it. Instead of looking up a clk by random string name. I don't really care about the clk_notifier_register() API, that's fine. Eventually I'd like to see that API not be used by clk provider drivers.
diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c index d2c99d8916b8..a5fddebbe530 100644 --- a/drivers/clk/samsung/clk-cpu.c +++ b/drivers/clk/samsung/clk-cpu.c @@ -152,7 +152,7 @@ static int exynos_cpuclk_pre_rate_change(struct clk_notifier_data *ndata, struct exynos_cpuclk *cpuclk, void __iomem *base) { const struct exynos_cpuclk_cfg_data *cfg_data = cpuclk->cfg; - unsigned long alt_prate = clk_get_rate(cpuclk->alt_parent); + unsigned long alt_prate = clk_hw_get_rate(cpuclk->alt_parent); unsigned long alt_div = 0, alt_div_mask = DIV_MASK; unsigned long div0, div1 = 0, mux_reg; unsigned long flags; @@ -280,7 +280,7 @@ static int exynos5433_cpuclk_pre_rate_change(struct clk_notifier_data *ndata, struct exynos_cpuclk *cpuclk, void __iomem *base) { const struct exynos_cpuclk_cfg_data *cfg_data = cpuclk->cfg; - unsigned long alt_prate = clk_get_rate(cpuclk->alt_parent); + unsigned long alt_prate = clk_hw_get_rate(cpuclk->alt_parent); unsigned long alt_div = 0, alt_div_mask = DIV_MASK; unsigned long div0, div1 = 0, mux_reg; unsigned long flags; @@ -432,7 +432,7 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, else cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb; - cpuclk->alt_parent = __clk_lookup(alt_parent); + cpuclk->alt_parent = __clk_get_hw(__clk_lookup(alt_parent)); if (!cpuclk->alt_parent) { pr_err("%s: could not lookup alternate parent %s\n", __func__, alt_parent); diff --git a/drivers/clk/samsung/clk-cpu.h b/drivers/clk/samsung/clk-cpu.h index d4b6b517fe1b..bd38c6aa3897 100644 --- a/drivers/clk/samsung/clk-cpu.h +++ b/drivers/clk/samsung/clk-cpu.h @@ -49,7 +49,7 @@ struct exynos_cpuclk_cfg_data { */ struct exynos_cpuclk { struct clk_hw hw; - struct clk *alt_parent; + struct clk_hw *alt_parent; void __iomem *ctrl_base; spinlock_t *lock; const struct exynos_cpuclk_cfg_data *cfg;
clk_notifier_register() documentation states, that the provided notifier callbacks associated with the notifier must not re-enter into the clk framework by calling any top-level clk APIs. Fix this by replacing clk_get_rate() calls with clk_hw_get_rate(), which is safe in this context. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/clk/samsung/clk-cpu.c | 6 +++--- drivers/clk/samsung/clk-cpu.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)