| Message ID | 20230731042807.1322972-1-wenst@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: meson: meson8b: Simplify notifier clock lookup | expand |
On Mon 31 Jul 2023 at 12:27, Chen-Yu Tsai <wenst@chromium.org> wrote: > The driver registers a clock notifier by first getting the name of one > of its clocks it just registered, then uses the name to look up the > clock. The lookup is not needed, since each clock provider already > has a clock attached to it. Use that instead to get rid of a > __clk_lookup() call. > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > --- > Found this could be simplified while looking through some clk core code. > > > drivers/clk/meson/meson8b.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index 827e78fb16a8..c4336ac012bf 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -3793,7 +3793,6 @@ static void __init meson8b_clkc_init_common(struct device_node *np, > { > struct meson8b_clk_reset *rstc; > struct device_node *parent_np; > - const char *notifier_clk_name; > struct clk *notifier_clk; > struct regmap *map; > int i, ret; > @@ -3847,9 +3846,7 @@ static void __init meson8b_clkc_init_common(struct device_node *np, > * tricky programming sequence will be handled by the forthcoming > * coordinated clock rates mechanism once that feature is released. > */ > - notifier_clk_name = clk_hw_get_name(&meson8b_cpu_scale_out_sel.hw); > - notifier_clk = __clk_lookup(notifier_clk_name); > - ret = clk_notifier_register(notifier_clk, &meson8b_cpu_nb_data.nb); > + ret = clk_notifier_register(meson8b_cpu_scale_out_sel.hw.clk, &meson8b_cpu_nb_data.nb); Hi Chen-Yu, Your patch seems valid, as CCF stands right now. However, I believe there is a will to drop the 'struct clk' instance that automatically gets created with each 'struct clk_hw'. This change would not help going in this direction Stephen, what do you think ? > if (ret) { > pr_err("%s: failed to register the CPU clock notifier\n", > __func__);
Quoting Jerome Brunet (2023-08-08 06:21:46) > > On Mon 31 Jul 2023 at 12:27, Chen-Yu Tsai <wenst@chromium.org> wrote: > > @@ -3847,9 +3846,7 @@ static void __init meson8b_clkc_init_common(struct device_node *np, > > * tricky programming sequence will be handled by the forthcoming > > * coordinated clock rates mechanism once that feature is released. > > */ > > - notifier_clk_name = clk_hw_get_name(&meson8b_cpu_scale_out_sel.hw); > > - notifier_clk = __clk_lookup(notifier_clk_name); > > - ret = clk_notifier_register(notifier_clk, &meson8b_cpu_nb_data.nb); > > + ret = clk_notifier_register(meson8b_cpu_scale_out_sel.hw.clk, &meson8b_cpu_nb_data.nb); > > Hi Chen-Yu, > > Your patch seems valid, as CCF stands right now. > > However, I believe there is a will to drop the 'struct clk' instance that > automatically gets created with each 'struct clk_hw'. This change would > not help going in this direction > Yes. Use clk_hw_get_clk() here, or introduce a clk_hw_notifier_register() API that does that automatically. We will have to put the clk on unregister path as well though, so maybe struct clk_notifier needs to be extended to store the clk_hw pointer. The notifier code kinda needs an overhaul to be clk_hw/clk_core aware. I think it predates the introduction of struct clk_core? It's a bunch of work I've been putting off.
diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c index 827e78fb16a8..c4336ac012bf 100644 --- a/drivers/clk/meson/meson8b.c +++ b/drivers/clk/meson/meson8b.c @@ -3793,7 +3793,6 @@ static void __init meson8b_clkc_init_common(struct device_node *np, { struct meson8b_clk_reset *rstc; struct device_node *parent_np; - const char *notifier_clk_name; struct clk *notifier_clk; struct regmap *map; int i, ret; @@ -3847,9 +3846,7 @@ static void __init meson8b_clkc_init_common(struct device_node *np, * tricky programming sequence will be handled by the forthcoming * coordinated clock rates mechanism once that feature is released. */ - notifier_clk_name = clk_hw_get_name(&meson8b_cpu_scale_out_sel.hw); - notifier_clk = __clk_lookup(notifier_clk_name); - ret = clk_notifier_register(notifier_clk, &meson8b_cpu_nb_data.nb); + ret = clk_notifier_register(meson8b_cpu_scale_out_sel.hw.clk, &meson8b_cpu_nb_data.nb); if (ret) { pr_err("%s: failed to register the CPU clock notifier\n", __func__);
The driver registers a clock notifier by first getting the name of one of its clocks it just registered, then uses the name to look up the clock. The lookup is not needed, since each clock provider already has a clock attached to it. Use that instead to get rid of a __clk_lookup() call. Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- Found this could be simplified while looking through some clk core code. drivers/clk/meson/meson8b.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)