* [PATCH] clk: qcom: gcc-qcs404: fix initial rate of GPLL3
@ 2024-10-22 9:45 Gabor Juhos
2024-10-22 9:53 ` Bryan O'Donoghue
2024-10-23 4:15 ` Bjorn Andersson
0 siblings, 2 replies; 4+ messages in thread
From: Gabor Juhos @ 2024-10-22 9:45 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Dmitry Baryshkov, Taniya Das
Cc: Vinod Koul, linux-arm-msm, linux-clk, linux-kernel, stable,
Gabor Juhos
The comment before the config of the GPLL3 PLL says that the
PLL should run at 930 MHz. In contrary to this, calculating
the frequency from the current configuration values by using
19.2 MHz as input frequency defined in 'qcs404.dtsi', it gives
921.6 MHz:
$ xo=19200000; l=48; alpha=0x0; alpha_hi=0x0
$ echo "$xo * ($((l)) + $(((alpha_hi << 32 | alpha) >> 8)) / 2^32)" | bc -l
921600000.00000000000000000000
Set 'alpha_hi' in the configuration to a value used in downstream
kernels [1][2] in order to get the correct output rate:
$ xo=19200000; l=48; alpha=0x0; alpha_hi=0x70
$ echo "$xo * ($((l)) + $(((alpha_hi << 32 | alpha) >> 8)) / 2^32)" | bc -l
930000000.00000000000000000000
The change is based on static code analysis, compile tested only.
[1] https://git.codelinaro.org/clo/la/kernel/msm-5.4/-/blob/kernel.lnx.5.4.r56-rel/drivers/clk/qcom/gcc-qcs404.c?ref_type=heads#L335
[2} https://git.codelinaro.org/clo/la/kernel/msm-5.15/-/blob/kernel.lnx.5.15.r49-rel/drivers/clk/qcom/gcc-qcs404.c?ref_type=heads#L127
Cc: stable@vger.kernel.org
Fixes: 652f1813c113 ("clk: qcom: gcc: Add global clock controller driver for QCS404")
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Note: due to a bug in the clk_alpha_pll_configure() function, the following
patch is also needed in order for this fix to take effect:
https://lore.kernel.org/all/20241019-qcs615-mm-clockcontroller-v1-1-4cfb96d779ae@quicinc.com/
---
drivers/clk/qcom/gcc-qcs404.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
index c3cfd572e7c1e0a987519be2cb2050c9bc7992c7..5ca003c9bfba89bee2e626b3c35936452cc02765 100644
--- a/drivers/clk/qcom/gcc-qcs404.c
+++ b/drivers/clk/qcom/gcc-qcs404.c
@@ -131,6 +131,7 @@ static struct clk_alpha_pll gpll1_out_main = {
/* 930MHz configuration */
static const struct alpha_pll_config gpll3_config = {
.l = 48,
+ .alpha_hi = 0x70,
.alpha = 0x0,
.alpha_en_mask = BIT(24),
.post_div_mask = 0xf << 8,
---
base-commit: 03dc72319cee7d0dfefee9ae7041b67732f6b8cd
change-id: 20241021-fix-gcc-qcs404-gpll3-f314335c8ecf
Best regards,
--
Gabor Juhos <j4g8y7@gmail.com>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] clk: qcom: gcc-qcs404: fix initial rate of GPLL3
2024-10-22 9:45 [PATCH] clk: qcom: gcc-qcs404: fix initial rate of GPLL3 Gabor Juhos
@ 2024-10-22 9:53 ` Bryan O'Donoghue
2024-10-22 13:01 ` Gabor Juhos
2024-10-23 4:15 ` Bjorn Andersson
1 sibling, 1 reply; 4+ messages in thread
From: Bryan O'Donoghue @ 2024-10-22 9:53 UTC (permalink / raw)
To: Gabor Juhos, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Dmitry Baryshkov, Taniya Das
Cc: Vinod Koul, linux-arm-msm, linux-clk, linux-kernel, stable
On 22/10/2024 10:45, Gabor Juhos wrote:
> The comment before the config of the GPLL3 PLL says that the
> PLL should run at 930 MHz. In contrary to this, calculating
> the frequency from the current configuration values by using
> 19.2 MHz as input frequency defined in 'qcs404.dtsi', it gives
> 921.6 MHz:
>
> $ xo=19200000; l=48; alpha=0x0; alpha_hi=0x0
> $ echo "$xo * ($((l)) + $(((alpha_hi << 32 | alpha) >> 8)) / 2^32)" | bc -l
> 921600000.00000000000000000000
>
> Set 'alpha_hi' in the configuration to a value used in downstream
> kernels [1][2] in order to get the correct output rate:
>
> $ xo=19200000; l=48; alpha=0x0; alpha_hi=0x70
> $ echo "$xo * ($((l)) + $(((alpha_hi << 32 | alpha) >> 8)) / 2^32)" | bc -l
> 930000000.00000000000000000000
>
> The change is based on static code analysis, compile tested only.
>
> [1] https://git.codelinaro.org/clo/la/kernel/msm-5.4/-/blob/kernel.lnx.5.4.r56-rel/drivers/clk/qcom/gcc-qcs404.c?ref_type=heads#L335
> [2} https://git.codelinaro.org/clo/la/kernel/msm-5.15/-/blob/kernel.lnx.5.15.r49-rel/drivers/clk/qcom/gcc-qcs404.c?ref_type=heads#L127
>
> Cc: stable@vger.kernel.org
> Fixes: 652f1813c113 ("clk: qcom: gcc: Add global clock controller driver for QCS404")
> Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
It should be possible to test / verify this change with debugcc on qcs404
https://github.com/linux-msm/debugcc/blob/master/qcs404.c
---
bod
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] clk: qcom: gcc-qcs404: fix initial rate of GPLL3
2024-10-22 9:53 ` Bryan O'Donoghue
@ 2024-10-22 13:01 ` Gabor Juhos
0 siblings, 0 replies; 4+ messages in thread
From: Gabor Juhos @ 2024-10-22 13:01 UTC (permalink / raw)
To: Bryan O'Donoghue, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Rob Herring, Dmitry Baryshkov, Taniya Das
Cc: Vinod Koul, linux-arm-msm, linux-clk, linux-kernel, stable
Hi Bryan,
2024. 10. 22. 11:53 keltezéssel, Bryan O'Donoghue írta:
> On 22/10/2024 10:45, Gabor Juhos wrote:
>> The comment before the config of the GPLL3 PLL says that the
>> PLL should run at 930 MHz. In contrary to this, calculating
>> the frequency from the current configuration values by using
>> 19.2 MHz as input frequency defined in 'qcs404.dtsi', it gives
>> 921.6 MHz:
>>
>> $ xo=19200000; l=48; alpha=0x0; alpha_hi=0x0
>> $ echo "$xo * ($((l)) + $(((alpha_hi << 32 | alpha) >> 8)) / 2^32)" | bc -l
>> 921600000.00000000000000000000
>>
>> Set 'alpha_hi' in the configuration to a value used in downstream
>> kernels [1][2] in order to get the correct output rate:
>>
>> $ xo=19200000; l=48; alpha=0x0; alpha_hi=0x70
>> $ echo "$xo * ($((l)) + $(((alpha_hi << 32 | alpha) >> 8)) / 2^32)" | bc -l
>> 930000000.00000000000000000000
>>
>> The change is based on static code analysis, compile tested only.
>>
>> [1] https://git.codelinaro.org/clo/la/kernel/msm-5.4/-/blob/
>> kernel.lnx.5.4.r56-rel/drivers/clk/qcom/gcc-qcs404.c?ref_type=heads#L335
>> [2} https://git.codelinaro.org/clo/la/kernel/msm-5.15/-/blob/
>> kernel.lnx.5.15.r49-rel/drivers/clk/qcom/gcc-qcs404.c?ref_type=heads#L127
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 652f1813c113 ("clk: qcom: gcc: Add global clock controller driver for
>> QCS404")
>> Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
> It should be possible to test / verify this change with debugcc on qcs404
>
> https://github.com/linux-msm/debugcc/blob/master/qcs404.c
Thank you for the suggestion. Unfortunately, I have no suitable hardware to test
that.
-Gabor
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] clk: qcom: gcc-qcs404: fix initial rate of GPLL3
2024-10-22 9:45 [PATCH] clk: qcom: gcc-qcs404: fix initial rate of GPLL3 Gabor Juhos
2024-10-22 9:53 ` Bryan O'Donoghue
@ 2024-10-23 4:15 ` Bjorn Andersson
1 sibling, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2024-10-23 4:15 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Dmitry Baryshkov,
Taniya Das, Gabor Juhos
Cc: Vinod Koul, linux-arm-msm, linux-clk, linux-kernel, stable
On Tue, 22 Oct 2024 11:45:56 +0200, Gabor Juhos wrote:
> The comment before the config of the GPLL3 PLL says that the
> PLL should run at 930 MHz. In contrary to this, calculating
> the frequency from the current configuration values by using
> 19.2 MHz as input frequency defined in 'qcs404.dtsi', it gives
> 921.6 MHz:
>
> $ xo=19200000; l=48; alpha=0x0; alpha_hi=0x0
> $ echo "$xo * ($((l)) + $(((alpha_hi << 32 | alpha) >> 8)) / 2^32)" | bc -l
> 921600000.00000000000000000000
>
> [...]
Applied, thanks!
[1/1] clk: qcom: gcc-qcs404: fix initial rate of GPLL3
commit: 36d202241d234fa4ac50743510d098ad52bd193a
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-10-23 4:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-22 9:45 [PATCH] clk: qcom: gcc-qcs404: fix initial rate of GPLL3 Gabor Juhos
2024-10-22 9:53 ` Bryan O'Donoghue
2024-10-22 13:01 ` Gabor Juhos
2024-10-23 4:15 ` Bjorn Andersson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox