[PATCH] clk: Fix invalid execution of clk_set_rate

[PATCH] clk: Fix invalid execution of clk_set_rate

[Chuan Liu via B4 Relay]

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

Some clocks have rates that can be changed elsewhere, so add a flag
CLK_GET_RATE_NOCACHE(such as scmi_clk) to these clocks to ensure that
the real-time rate is obtained.

When clk_set_rate is called, it is returned if the request to set rate
is consistent with the current rate. Getting the current rate in
clk_set_rate returns the rate stored in clk_core. CLK_GET_RATE_NOCACHE
does not take effect here.

Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
Some clocks have rates that can be changed elsewhere, so add a flag
CLK_GET_RATE_NOCACHE(such as scmi_clk) to these clocks to ensure that
the real-time rate is obtained.

When clk_set_rate is called, it is returned if the request to set rate
is consistent with the current rate. Getting the current rate in
clk_set_rate returns the rate stored in clk_core. CLK_GET_RATE_NOCACHE
does not take effect here.
---
 drivers/clk/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 285ed1ad8a37..79b453b82528 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2536,7 +2536,7 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
 	rate = clk_core_req_round_rate_nolock(core, req_rate);
 
 	/* bail early if nothing to do */
-	if (rate == clk_core_get_rate_nolock(core))
+	if (rate == clk_core_get_rate_recalc(core))
 		return 0;
 
 	/* fail on a direct rate set of a protected provider */

---
base-commit: 2cd6543d82b9dc7b971b9b64f22ad34a1cae3076
change-id: 20240910-fix_clk-71bcd20bd33f

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

Re: [PATCH] clk: Fix invalid execution of clk_set_rate

[Stephen Boyd]

Quoting Chuan Liu via B4 Relay (2024-09-09 22:53:44)
> From: Chuan Liu <chuan.liu@amlogic.com>
> 
> Some clocks have rates that can be changed elsewhere, so add a flag
> CLK_GET_RATE_NOCACHE(such as scmi_clk) to these clocks to ensure that
> the real-time rate is obtained.
> 
> When clk_set_rate is called, it is returned if the request to set rate
> is consistent with the current rate. Getting the current rate in
> clk_set_rate returns the rate stored in clk_core. CLK_GET_RATE_NOCACHE
> does not take effect here.
> 
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>

This looks obviously correct but I'm worried that it will cause some
problem somewhere. It would be nice if there were some kunit tests
associated with this. The worst case situation is that we recalc rates
if the CLK_GET_RATE_NOCACHE flag is set, right? I guess I'll just apply
this as a cleanup for the next merge window and see if it causes
problems for anyone.

Re: [PATCH] clk: Fix invalid execution of clk_set_rate

[Aishwarya TCV]

On 10/09/2024 06:53, Chuan Liu via B4 Relay wrote:
> From: Chuan Liu <chuan.liu@amlogic.com>
> 
> Some clocks have rates that can be changed elsewhere, so add a flag
> CLK_GET_RATE_NOCACHE(such as scmi_clk) to these clocks to ensure that
> the real-time rate is obtained.
> 
> When clk_set_rate is called, it is returned if the request to set rate
> is consistent with the current rate. Getting the current rate in
> clk_set_rate returns the rate stored in clk_core. CLK_GET_RATE_NOCACHE
> does not take effect here.
> 
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
> ---

Hi Chaun,

Currently, when booting the kernel against next-master (next-20241128)
with Arm64 on the Qualcomm RB5 board, the following logs are printed
repeatedly. This issue does not cause a failure.

A bisect (full log provided below) identified this patch as the source
of the log prints. The bisect was performed on the tag "next-20241118"
in the repository located at
"https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git".

This works fine on Linux 6.12


Sample log from run on RB5:
------
<3>[   17.159611] cpu cpu7: _opp_config_clk_single: failed to set clock
rate: -22
<3>[   17.165534] cpu cpu4: _opp_config_clk_single: failed to set clock
rate: -22
<3>[   17.169338] cpu cpu0: _opp_config_clk_single: failed to set clock
rate: -22
<3>[   17.171850] cpu cpu7: _opp_config_clk_single: failed to set clock
rate: -22
<3>[   17.171913] cpu cpu0: _opp_config_clk_single: failed to set clock
rate: -22
<3>[   17.196804] cpu cpu4: _opp_config_clk_single: failed to set clock
rate: -22

------


Bisect log:
------
git bisect start
# good: [adc218676eef25575469234709c2d87185ca223a] Linux 6.12
git bisect good adc218676eef25575469234709c2d87185ca223a
# bad: [ae58226b89ac0cffa05ba7357733776542e40216] Add linux-next
specific files for 20241118
git bisect bad ae58226b89ac0cffa05ba7357733776542e40216
# bad: [864039014c6a549c57796b83f99cc7fe25bf1afe] Merge branch 'main' of
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
git bisect bad 864039014c6a549c57796b83f99cc7fe25bf1afe
# bad: [50c7979120c4cff994ea6eaf14747bb0c01a0cd0] Merge branch
'xtensa-for-next' of git://github.com/jcmvbkbc/linux-xtensa.git
git bisect bad 50c7979120c4cff994ea6eaf14747bb0c01a0cd0
# good: [daa20223dee942ebea45bc72b517480af226c370] soc: document merges
git bisect good daa20223dee942ebea45bc72b517480af226c370
# good: [5cc2c18659e40c5d2f93d429d2a4a38234184767] Merge branch
'for-next/core' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux
git bisect good 5cc2c18659e40c5d2f93d429d2a4a38234184767
# bad: [618a651f965804164b2f8c446d729197a5fdfd3e] Merge branch
'linux-next' of git://github.com/c-sky/csky-linux.git
git bisect bad 618a651f965804164b2f8c446d729197a5fdfd3e
# good: [d3faed6e9d54a50ad2c505df7e9455752405888c] Merge branch
'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git
git bisect good d3faed6e9d54a50ad2c505df7e9455752405888c
# good: [44ea50e086eb287f923460c0119b95d263ec76de] Merge branch
'clk-imx' into clk-next
git bisect good 44ea50e086eb287f923460c0119b95d263ec76de
# good: [ae7a050d03ca95e8e57bc709af8c5a56d1c1b216] Merge branch
'clk-mediatek' into clk-next
git bisect good ae7a050d03ca95e8e57bc709af8c5a56d1c1b216
# bad: [d6c3666ba425fc5a1a4c938591e89279203a25e1] Merge branch
'clk-cleanup' into clk-next
git bisect bad d6c3666ba425fc5a1a4c938591e89279203a25e1
# good: [7b51444e86d11a346c338a03e96d142fc82b95ac] Merge branch
'clk-adi' into clk-next
git bisect good 7b51444e86d11a346c338a03e96d142fc82b95ac
# good: [5e01124a2c0a42dc6b587b0b09b204a5389f8d7b] clk: eyeq: add EyeQ5
fixed factor clocks
git bisect good 5e01124a2c0a42dc6b587b0b09b204a5389f8d7b
# good: [caabff73ef137ff183e4f6626332eb6b0a886522] Merge branch
'clk-mobileye' into clk-next
git bisect good caabff73ef137ff183e4f6626332eb6b0a886522
# bad: [eb70c83178b25e8d7934c51ba6f1b04da2867265] clk: Fix invalid
execution of clk_set_rate
git bisect bad eb70c83178b25e8d7934c51ba6f1b04da2867265
# good: [ac678a1810665f9820934b89157094afd0975a80] clk: clk-loongson2:
Fix memory corruption bug in struct loongson2_clk_provider
git bisect good ac678a1810665f9820934b89157094afd0975a80
# first bad commit: [eb70c83178b25e8d7934c51ba6f1b04da2867265] clk: Fix
invalid execution of clk_set_rate
------

Thanks,
Aishwarya