diff mbox series

clk: meson: meson8b: Simplify notifier clock lookup

Message ID 20230731042807.1322972-1-wenst@chromium.org
State New
Headers
Series clk: meson: meson8b: Simplify notifier clock lookup |

Commit Message

Chen-Yu Tsai July 31, 2023, 4:27 a.m. UTC 
  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(-)

Comments

Jerome Brunet Aug. 8, 2023, 1:21 p.m. UTC | #1 
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__);
Stephen Boyd Aug. 9, 2023, 9:55 p.m. UTC | #2 
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 mbox series

Patch

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__);