[PATCH 0/2] Fix Agera PLL config of CAMCC for SM6350 & SM7150

[PATCH 0/2] Fix Agera PLL config of CAMCC for SM6350 & SM7150

[Luca Weiss]

On SM6350 I noticed that CAMCC_MCLK* refuses to turn on (being stuck on
off). The cause seems that PLL2 (of type Agera) is not configured
correctly due to the implementation in clk-alpha-pll.c differing between
downstream (e.g. msm-4.19) and upstream, and therefore the USER_CTL
value is not configured.

While looking around, it looks like camcc-sm7150.c has exactly the same
issue, so fix that one as well.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
Luca Weiss (2):
      clk: qcom: camcc-sm6350: Fix PLL config of PLL2
      clk: qcom: camcc-sm7150: Fix PLL config of PLL2

 drivers/clk/qcom/camcc-sm6350.c | 6 +-----
 drivers/clk/qcom/camcc-sm7150.c | 6 +-----
 2 files changed, 2 insertions(+), 10 deletions(-)
---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251021-agera-pll-fixups-dad6b12c715a

Best regards,
-- 
Luca Weiss <luca.weiss@fairphone.com>

[PATCH 1/2] clk: qcom: camcc-sm6350: Fix PLL config of PLL2

[Luca Weiss]

The 'Agera' PLLs (with clk_agera_pll_configure) do not take some of the
parameters that are provided in the vendor driver. Instead the upstream
configuration should provide the final user_ctl value that is written to
the USER_CTL register.

Fix the config so that the PLL is configured correctly, and fixes
CAMCC_MCLK* being stuck off.

Fixes: 80f5451d9a7c ("clk: qcom: Add camera clock controller driver for SM6350")
Suggested-by: Taniya Das <taniya.das@oss.qualcomm.com>
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/clk/qcom/camcc-sm6350.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sm6350.c b/drivers/clk/qcom/camcc-sm6350.c
index 8aac97d29ce3..87806392a59d 100644
--- a/drivers/clk/qcom/camcc-sm6350.c
+++ b/drivers/clk/qcom/camcc-sm6350.c
@@ -145,15 +145,11 @@ static struct clk_alpha_pll_postdiv camcc_pll1_out_even = {
 static const struct alpha_pll_config camcc_pll2_config = {
 	.l = 0x64,
 	.alpha = 0x0,
-	.post_div_val = 0x3 << 8,
-	.post_div_mask = 0x3 << 8,
-	.aux_output_mask = BIT(1),
-	.main_output_mask = BIT(0),
-	.early_output_mask = BIT(3),
 	.config_ctl_val = 0x20000800,
 	.config_ctl_hi_val = 0x400003d2,
 	.test_ctl_val = 0x04000400,
 	.test_ctl_hi_val = 0x00004000,
+	.user_ctl_val = 0x0000030b,
 };
 
 static struct clk_alpha_pll camcc_pll2 = {

-- 
2.51.1

Re: [PATCH 1/2] clk: qcom: camcc-sm6350: Fix PLL config of PLL2

[Taniya Das]

On 10/21/2025 11:38 PM, Luca Weiss wrote:
> The 'Agera' PLLs (with clk_agera_pll_configure) do not take some of the
> parameters that are provided in the vendor driver. Instead the upstream
> configuration should provide the final user_ctl value that is written to
> the USER_CTL register.
> 
> Fix the config so that the PLL is configured correctly, and fixes
> CAMCC_MCLK* being stuck off.
> 
> Fixes: 80f5451d9a7c ("clk: qcom: Add camera clock controller driver for SM6350")
> Suggested-by: Taniya Das <taniya.das@oss.qualcomm.com>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  drivers/clk/qcom/camcc-sm6350.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/qcom/camcc-sm6350.c b/drivers/clk/qcom/camcc-sm6350.c
> index 8aac97d29ce3..87806392a59d 100644
> --- a/drivers/clk/qcom/camcc-sm6350.c
> +++ b/drivers/clk/qcom/camcc-sm6350.c
> @@ -145,15 +145,11 @@ static struct clk_alpha_pll_postdiv camcc_pll1_out_even = {
>  static const struct alpha_pll_config camcc_pll2_config = {
>  	.l = 0x64,
>  	.alpha = 0x0,
> -	.post_div_val = 0x3 << 8,
> -	.post_div_mask = 0x3 << 8,
> -	.aux_output_mask = BIT(1),
> -	.main_output_mask = BIT(0),
> -	.early_output_mask = BIT(3),
>  	.config_ctl_val = 0x20000800,
>  	.config_ctl_hi_val = 0x400003d2,
>  	.test_ctl_val = 0x04000400,
>  	.test_ctl_hi_val = 0x00004000,
> +	.user_ctl_val = 0x0000030b,
>  };
>  
>  static struct clk_alpha_pll camcc_pll2 = {
> 

Reviewed-by: Taniya Das <taniya.das@oss.qualcomm.com>

-- 
Thanks,
Taniya Das

Re: [PATCH 1/2] clk: qcom: camcc-sm6350: Fix PLL config of PLL2

[Abel Vesa]

On 25-10-21 20:08:54, Luca Weiss wrote:
> The 'Agera' PLLs (with clk_agera_pll_configure) do not take some of the
> parameters that are provided in the vendor driver. Instead the upstream
> configuration should provide the final user_ctl value that is written to
> the USER_CTL register.
> 
> Fix the config so that the PLL is configured correctly, and fixes
> CAMCC_MCLK* being stuck off.
> 
> Fixes: 80f5451d9a7c ("clk: qcom: Add camera clock controller driver for SM6350")
> Suggested-by: Taniya Das <taniya.das@oss.qualcomm.com>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

Re: [PATCH 1/2] clk: qcom: camcc-sm6350: Fix PLL config of PLL2

[Konrad Dybcio]

On 10/21/25 8:08 PM, Luca Weiss wrote:
> The 'Agera' PLLs (with clk_agera_pll_configure) do not take some of the
> parameters that are provided in the vendor driver. Instead the upstream
> configuration should provide the final user_ctl value that is written to
> the USER_CTL register.

This is perhaps wishful thinking due to potential complexity, but maybe
we could add some sanity checks to make sure that putting things in
unused fields doesn't happen

Konrad

Re: [PATCH 1/2] clk: qcom: camcc-sm6350: Fix PLL config of PLL2

[Dmitry Baryshkov]

On Wed, Oct 22, 2025 at 01:19:16PM +0200, Konrad Dybcio wrote:
> On 10/21/25 8:08 PM, Luca Weiss wrote:
> > The 'Agera' PLLs (with clk_agera_pll_configure) do not take some of the
> > parameters that are provided in the vendor driver. Instead the upstream
> > configuration should provide the final user_ctl value that is written to
> > the USER_CTL register.
> 
> This is perhaps wishful thinking due to potential complexity, but maybe
> we could add some sanity checks to make sure that putting things in
> unused fields doesn't happen

Should we just drop those fields and always write the register value?

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] clk: qcom: camcc-sm6350: Fix PLL config of PLL2

[Konrad Dybcio]

On 10/22/25 5:09 PM, Dmitry Baryshkov wrote:
> On Wed, Oct 22, 2025 at 01:19:16PM +0200, Konrad Dybcio wrote:
>> On 10/21/25 8:08 PM, Luca Weiss wrote:
>>> The 'Agera' PLLs (with clk_agera_pll_configure) do not take some of the
>>> parameters that are provided in the vendor driver. Instead the upstream
>>> configuration should provide the final user_ctl value that is written to
>>> the USER_CTL register.
>>
>> This is perhaps wishful thinking due to potential complexity, but maybe
>> we could add some sanity checks to make sure that putting things in
>> unused fields doesn't happen
> 
> Should we just drop those fields and always write the register value?

They're used in other_kind_of_alpha_pll_configure.. and we have a lot
of drivers using either of these approaches, so converting that and
not breaking anything sounds a little difficult

Konrad

[PATCH 2/2] clk: qcom: camcc-sm7150: Fix PLL config of PLL2

[Luca Weiss]

The 'Agera' PLLs (with clk_agera_pll_configure) do not take some of the
parameters that are provided in the vendor driver. Instead the upstream
configuration should provide the final user_ctl value that is written to
the USER_CTL register.

Fix the config so that the PLL is configured correctly.

Fixes: 9f0532da4226 ("clk: qcom: Add Camera Clock Controller driver for SM7150")
Suggested-by: Taniya Das <taniya.das@oss.qualcomm.com>
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/clk/qcom/camcc-sm7150.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sm7150.c b/drivers/clk/qcom/camcc-sm7150.c
index 4a3baf5d8e85..590548cac45b 100644
--- a/drivers/clk/qcom/camcc-sm7150.c
+++ b/drivers/clk/qcom/camcc-sm7150.c
@@ -139,13 +139,9 @@ static struct clk_fixed_factor camcc_pll1_out_even = {
 /* 1920MHz configuration */
 static const struct alpha_pll_config camcc_pll2_config = {
 	.l = 0x64,
-	.post_div_val = 0x3 << 8,
-	.post_div_mask = 0x3 << 8,
-	.early_output_mask = BIT(3),
-	.aux_output_mask = BIT(1),
-	.main_output_mask = BIT(0),
 	.config_ctl_hi_val = 0x400003d6,
 	.config_ctl_val = 0x20000954,
+	.user_ctl_val = 0x0000030b,
 };
 
 static struct clk_alpha_pll camcc_pll2 = {

-- 
2.51.1

Re: [PATCH 2/2] clk: qcom: camcc-sm7150: Fix PLL config of PLL2

[Taniya Das]

On 10/21/2025 11:38 PM, Luca Weiss wrote:
> The 'Agera' PLLs (with clk_agera_pll_configure) do not take some of the
> parameters that are provided in the vendor driver. Instead the upstream
> configuration should provide the final user_ctl value that is written to
> the USER_CTL register.
> 
> Fix the config so that the PLL is configured correctly.
> 
> Fixes: 9f0532da4226 ("clk: qcom: Add Camera Clock Controller driver for SM7150")
> Suggested-by: Taniya Das <taniya.das@oss.qualcomm.com>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  drivers/clk/qcom/camcc-sm7150.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/qcom/camcc-sm7150.c b/drivers/clk/qcom/camcc-sm7150.c
> index 4a3baf5d8e85..590548cac45b 100644
> --- a/drivers/clk/qcom/camcc-sm7150.c
> +++ b/drivers/clk/qcom/camcc-sm7150.c
> @@ -139,13 +139,9 @@ static struct clk_fixed_factor camcc_pll1_out_even = {
>  /* 1920MHz configuration */
>  static const struct alpha_pll_config camcc_pll2_config = {
>  	.l = 0x64,
> -	.post_div_val = 0x3 << 8,
> -	.post_div_mask = 0x3 << 8,
> -	.early_output_mask = BIT(3),
> -	.aux_output_mask = BIT(1),
> -	.main_output_mask = BIT(0),
>  	.config_ctl_hi_val = 0x400003d6,
>  	.config_ctl_val = 0x20000954,
> +	.user_ctl_val = 0x0000030b,
>  };
>  
>  static struct clk_alpha_pll camcc_pll2 = {
> 

Reviewed-by: Taniya Das <taniya.das@oss.qualcomm.com>

-- 
Thanks,
Taniya Das

Re: [PATCH 2/2] clk: qcom: camcc-sm7150: Fix PLL config of PLL2

[Abel Vesa]

On 25-10-21 20:08:55, Luca Weiss wrote:
> The 'Agera' PLLs (with clk_agera_pll_configure) do not take some of the
> parameters that are provided in the vendor driver. Instead the upstream
> configuration should provide the final user_ctl value that is written to
> the USER_CTL register.
> 
> Fix the config so that the PLL is configured correctly.
> 
> Fixes: 9f0532da4226 ("clk: qcom: Add Camera Clock Controller driver for SM7150")
> Suggested-by: Taniya Das <taniya.das@oss.qualcomm.com>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

Re: [PATCH 0/2] Fix Agera PLL config of CAMCC for SM6350 & SM7150

[Bjorn Andersson]

On Tue, 21 Oct 2025 20:08:53 +0200, Luca Weiss wrote:
> On SM6350 I noticed that CAMCC_MCLK* refuses to turn on (being stuck on
> off). The cause seems that PLL2 (of type Agera) is not configured
> correctly due to the implementation in clk-alpha-pll.c differing between
> downstream (e.g. msm-4.19) and upstream, and therefore the USER_CTL
> value is not configured.
> 
> While looking around, it looks like camcc-sm7150.c has exactly the same
> issue, so fix that one as well.
> 
> [...]

Applied, thanks!

[1/2] clk: qcom: camcc-sm6350: Fix PLL config of PLL2
      commit: ab0e13141d679fdffdd3463a272c5c1b10be1794
[2/2] clk: qcom: camcc-sm7150: Fix PLL config of PLL2
      commit: 415aad75c7e5cdb72e0672dc1159be1a99535ecd

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>