From: Stephen Boyd <sboyd@kernel.org>
To: Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org>,
Michael Turquette <mturquette@baylibre.com>,
chuan.liu@amlogic.com
Cc: linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
Chuan Liu <chuan.liu@amlogic.com>
Subject: Re: [PATCH 2/2] clk: divider: improve the execution efficiency of determine_rate()
Date: Sat, 25 Oct 2025 18:25:44 -0700 [thread overview]
Message-ID: <176144194442.3953.13998249458455926261@lazor> (raw)
In-Reply-To: <20251021-improve_efficiency_of_clk_divider-v1-2-e2bf7f625af6@amlogic.com>
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.
next prev parent reply other threads:[~2025-10-26 1:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-21 10:12 [PATCH 0/2] clk: improve the execution efficiency of determine_rate() Chuan Liu via B4 Relay
2025-10-21 10:12 ` [PATCH 1/2] clk: mux: " Chuan Liu via B4 Relay
2025-10-21 10:12 ` [PATCH 2/2] clk: divider: " Chuan Liu via B4 Relay
2025-10-26 1:25 ` Stephen Boyd [this message]
2025-10-27 8:47 ` Chuan Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=176144194442.3953.13998249458455926261@lazor \
--to=sboyd@kernel.org \
--cc=chuan.liu@amlogic.com \
--cc=devnull+chuan.liu.amlogic.com@kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mturquette@baylibre.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox