[PATCH 0/2] clk: improve the execution efficiency of determine_rate()

[PATCH 0/2] clk: improve the execution efficiency of determine_rate()

[Chuan Liu via B4 Relay]

This patch series aims to reduce the execution time of
determine_rate() for mux and divider clocks.

Although the core logic of _is_best_div() and mux_is_better_rate() is
similar, they are submitted as two separate patches to minimize
maintenance risk.

Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
Chuan Liu (2):
      clk: mux: improve the execution efficiency of determine_rate()
      clk: divider: improve the execution efficiency of determine_rate()

 drivers/clk/clk-divider.c | 3 +++
 drivers/clk/clk.c         | 3 +++
 2 files changed, 6 insertions(+)
---
base-commit: 606da5bb165594c052ee11de79bf05bc38bc1aa6
change-id: 20251020-improve_efficiency_of_clk_divider-631fceb70777

Best regards,
-- 
Chuan Liu <chuan.liu@amlogic.com>

[PATCH 1/2] clk: mux: improve the execution efficiency of determine_rate()

[Chuan Liu via B4 Relay]

From: Chuan Liu <chuan.liu@amlogic.com>

According to the current logic of mux_is_better_rate(), even if a
parent clock matching req->rate is found again in the current loop,
it will not be selected.

Exit the loop once the best-matching parent clock is found to avoid
unnecessary function calls and reduce execution time.

Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
 drivers/clk/clk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 85d2f2481acf..925e4768e3ad 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -758,6 +758,9 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
 			best_parent = parent;
 			best = parent_rate;
 		}
+
+		if (best == req->rate)
+			break;
 	}
 
 	if (!best_parent)

-- 
2.42.0

[PATCH 2/2] clk: divider: improve the execution efficiency of determine_rate()

[Chuan Liu via B4 Relay]

From: Chuan Liu <chuan.liu@amlogic.com>

There is no need to evaluate further divider values once _is_best_div()
finds one that matches the target rate.

Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
 drivers/clk/clk-divider.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 2601b6155afb..b92c4f800fa9 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -339,6 +339,9 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
 			best = now;
 			*best_parent_rate = parent_rate;
 		}
+
+		if (best == rate)
+			break;
 	}
 
 	if (!bestdiv) {

-- 
2.42.0

Re: [PATCH 2/2] clk: divider: improve the execution efficiency of determine_rate()

[Stephen Boyd]

Quoting Chuan Liu via B4 Relay (2025-10-21 03:12:31)
> From: Chuan Liu <chuan.liu@amlogic.com>
> 
> There is no need to evaluate further divider values once _is_best_div()
> finds one that matches the target rate.
> 
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
> ---
>  drivers/clk/clk-divider.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 2601b6155afb..b92c4f800fa9 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -339,6 +339,9 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
>                         best = now;
>                         *best_parent_rate = parent_rate;
>                 }
> +
> +               if (best == rate)
> +                       break;

This needs a comment. I'm not even sure if it is correct either, because
the other exit from this loop happens if the parent rate can be
unchanged. I don't think we have any KUnit tests for this file, so
please add some tests that deal with this case explicitly (the parent
rate being unchanged as the desirable part).

A general comment: these patches have no benefit described in the commit
text. Do you see any performance improvement with this patch? I sorta
doubt this really matters because the number of dividers are typically
small. A single sentence commit text that only says there is no need
doesn't convince me that any work has been put in to research why the
code was written this way or even prove that making this change improves
anything.

Re: [PATCH 2/2] clk: divider: improve the execution efficiency of determine_rate()

[Chuan Liu]

Hi Stephen,

Thanks for your review.

On 10/26/2025 9:25 AM, Stephen Boyd wrote:
> [ EXTERNAL EMAIL ]
> 
> Quoting Chuan Liu via B4 Relay (2025-10-21 03:12:31)
>> From: Chuan Liu <chuan.liu@amlogic.com>
>>
>> There is no need to evaluate further divider values once _is_best_div()
>> finds one that matches the target rate.
>>
>> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
>> ---
>>   drivers/clk/clk-divider.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>> index 2601b6155afb..b92c4f800fa9 100644
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -339,6 +339,9 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
>>                          best = now;
>>                          *best_parent_rate = parent_rate;
>>                  }
>> +
>> +               if (best == rate)
>> +                       break;
> 
> This needs a comment. I'm not even sure if it is correct either, because
> the other exit from this loop happens if the parent rate can be

You're right.  Sorry, I was mainly focusing on the number of
iterations in the for loop earlier and missed the fact that the
parent clock directly participates in the division without changing.

> unchanged. I don't think we have any KUnit tests for this file, so
> please add some tests that deal with this case explicitly (the parent
> rate being unchanged as the desirable part).

I'll take some time to get familiar with the principles and methods
of KUnit testing, and I'd be happy to help improve it.

> 
> A general comment: these patches have no benefit described in the commit
> text. Do you see any performance improvement with this patch? I sorta
> doubt this really matters because the number of dividers are typically
> small. A single sentence commit text that only says there is no need
> doesn't convince me that any work has been put in to research why the
> code was written this way or even prove that making this change improves
> anything.

This patch came about because I noticed that the mux has a similar
logic:
- Even after finding a suitable clock source, it continues to iterate
through and calculate other sources (and even if a later source has
the same rate, it won't replace the current one).

 From there, I went on to examine the divider logic, but I missed the
detail that the parent clock doesn't change in the divider. Sorry
that this unreasonable patch ended up taking up your time.