[PATCH v2 0/8] clk: qcom: Add support to attach multiple power domains in cc probe

[PATCH v2 0/8] clk: qcom: Add support to attach multiple power domains in cc probe

[Jagadeesh Kona]

In some of the recent chipsets, PLLs require more than one power domain
to be kept ON to configure the PLL. But the current code doesn't enable
all the required power domains while configuring the PLLs, this leads
to functional issues due to suboptimal settings of PLLs.

To address this, add support for handling runtime power management,
configuring plls and enabling critical clocks from qcom_cc_really_probe.
The clock controller can specify PLLs, critical clocks, and runtime PM
requirements in the descriptor data. The code in qcom_cc_really_probe()
ensures all necessary power domains are enabled before configuring PLLs
or critical clocks.

This series updates SM8450 & SM8550 videocc drivers to handle rpm,
configure PLLs and enable critical clocks from within qcom_cc_really_probe()
using above support, so video PLLs are configured properly.

This series fixes the below warning reported in SM8550 venus testing due
to video_cc_pll0 not properly getting configured during videocc probe

[   46.535132] Lucid PLL latch failed. Output may be unstable!

The patch adding support to configure the PLLs from common code is
picked from below series and updated it.
https://lore.kernel.org/all/20250113-support-pll-reconfigure-v1-0-1fae6bc1062d@quicinc.com/

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
Changes in v2:
 - Added support to handle rpm, PLL configuration and enable critical
   clocks from qcom_cc_really_probe() in common code as per v1 commments
   from Bryan, Konrad and Dmitry
 - Added patches to configure PLLs from common code
 - Updated the SM8450, SM8550 videocc patches to use the newly
   added support to handle rpm, configure PLLs from common code
 - Split the DT change for each target separately as per
   Dmitry comments
 - Added R-By and A-By tags received on v1
- Link to v1: https://lore.kernel.org/r/20250218-videocc-pll-multi-pd-voting-v1-0-cfe6289ea29b@quicinc.com

---
Jagadeesh Kona (7):
      dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain
      clk: qcom: common: Manage rpm, configure PLLs & AON clks in really probe
      clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe
      clk: qcom: videocc-sm8550: Move PLL & clk configuration to really probe
      arm64: dts: qcom: Add MXC power domain to videocc node on SM8450
      arm64: dts: qcom: Add MXC power domain to videocc node on SM8550
      arm64: dts: qcom: Add MXC power domain to videocc node on SM8650

Taniya Das (1):
      clk: qcom: common: Add support to configure PLL

 .../bindings/clock/qcom,sm8450-videocc.yaml        |   9 +-
 arch/arm64/boot/dts/qcom/sm8450.dtsi               |   3 +-
 arch/arm64/boot/dts/qcom/sm8550.dtsi               |   3 +-
 arch/arm64/boot/dts/qcom/sm8650.dtsi               |   3 +-
 drivers/clk/qcom/clk-alpha-pll.h                   |   2 +
 drivers/clk/qcom/common.c                          | 100 +++++++++++++++++++--
 drivers/clk/qcom/common.h                          |  17 ++++
 drivers/clk/qcom/videocc-sm8450.c                  |  49 +++++-----
 drivers/clk/qcom/videocc-sm8550.c                  |  50 +++++------
 9 files changed, 167 insertions(+), 69 deletions(-)
---
base-commit: e5d3fd687aac5eceb1721fa92b9f49afcf4c3717
change-id: 20250218-videocc-pll-multi-pd-voting-d614dce910e7

Best regards,
-- 
Jagadeesh Kona <quic_jkona@quicinc.com>

[PATCH v2 1/8] dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain

[Jagadeesh Kona]

To configure the video PLLs and enable the video GDSCs on SM8450,
SM8475, SM8550 and SM8650 platforms, the MXC rail must be ON along
with MMCX. Therefore, update the videocc bindings to include
the MXC power domain on these platforms.

Fixes: 1e910b2ba0ed ("dt-bindings: clock: qcom: Add SM8450 video clock controller")
Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
---
 Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
index 62714fa54db82491a7a108f7f18a253d737f8d61..737efc4b46564c1e475b02873d2dc124329fb775 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
@@ -32,9 +32,11 @@ properties:
       - description: Video AHB clock from GCC
 
   power-domains:
-    maxItems: 1
     description:
-      MMCX power domain.
+      Power domains required for the clock controller to operate
+    items:
+      - description: MMCX power domain
+      - description: MXC power domain
 
   required-opps:
     maxItems: 1
@@ -72,7 +74,8 @@ examples:
       reg = <0x0aaf0000 0x10000>;
       clocks = <&rpmhcc RPMH_CXO_CLK>,
                <&gcc GCC_VIDEO_AHB_CLK>;
-      power-domains = <&rpmhpd RPMHPD_MMCX>;
+      power-domains = <&rpmhpd RPMHPD_MMCX>,
+                      <&rpmhpd RPMHPD_MXC>;
       required-opps = <&rpmhpd_opp_low_svs>;
       #clock-cells = <1>;
       #reset-cells = <1>;

-- 
2.34.1

Re: [PATCH v2 1/8] dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain

[Dmitry Baryshkov]

On Thu, Mar 06, 2025 at 02:25:33PM +0530, Jagadeesh Kona wrote:
> To configure the video PLLs and enable the video GDSCs on SM8450,
> SM8475, SM8550 and SM8650 platforms, the MXC rail must be ON along
> with MMCX. Therefore, update the videocc bindings to include
> the MXC power domain on these platforms.
> 
> Fixes: 1e910b2ba0ed ("dt-bindings: clock: qcom: Add SM8450 video clock controller")
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Acked-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
> index 62714fa54db82491a7a108f7f18a253d737f8d61..737efc4b46564c1e475b02873d2dc124329fb775 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
> @@ -32,9 +32,11 @@ properties:
>        - description: Video AHB clock from GCC
>  
>    power-domains:
> -    maxItems: 1
>      description:
> -      MMCX power domain.
> +      Power domains required for the clock controller to operate
> +    items:
> +      - description: MMCX power domain
> +      - description: MXC power domain
>  
>    required-opps:
>      maxItems: 1
> @@ -72,7 +74,8 @@ examples:
>        reg = <0x0aaf0000 0x10000>;
>        clocks = <&rpmhcc RPMH_CXO_CLK>,
>                 <&gcc GCC_VIDEO_AHB_CLK>;
> -      power-domains = <&rpmhpd RPMHPD_MMCX>;
> +      power-domains = <&rpmhpd RPMHPD_MMCX>,
> +                      <&rpmhpd RPMHPD_MXC>;
>        required-opps = <&rpmhpd_opp_low_svs>;

As pointed out by Vladimir, you probably also need a second entry in
required-opps.

>        #clock-cells = <1>;
>        #reset-cells = <1>;
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 1/8] dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain

[Jagadeesh Kona]

On 3/6/2025 3:40 PM, Dmitry Baryshkov wrote:
> On Thu, Mar 06, 2025 at 02:25:33PM +0530, Jagadeesh Kona wrote:
>> To configure the video PLLs and enable the video GDSCs on SM8450,
>> SM8475, SM8550 and SM8650 platforms, the MXC rail must be ON along
>> with MMCX. Therefore, update the videocc bindings to include
>> the MXC power domain on these platforms.
>>
>> Fixes: 1e910b2ba0ed ("dt-bindings: clock: qcom: Add SM8450 video clock controller")
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> Acked-by: Rob Herring (Arm) <robh@kernel.org>
>> ---
>>  Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> index 62714fa54db82491a7a108f7f18a253d737f8d61..737efc4b46564c1e475b02873d2dc124329fb775 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> @@ -32,9 +32,11 @@ properties:
>>        - description: Video AHB clock from GCC
>>  
>>    power-domains:
>> -    maxItems: 1
>>      description:
>> -      MMCX power domain.
>> +      Power domains required for the clock controller to operate
>> +    items:
>> +      - description: MMCX power domain
>> +      - description: MXC power domain
>>  
>>    required-opps:
>>      maxItems: 1
>> @@ -72,7 +74,8 @@ examples:
>>        reg = <0x0aaf0000 0x10000>;
>>        clocks = <&rpmhcc RPMH_CXO_CLK>,
>>                 <&gcc GCC_VIDEO_AHB_CLK>;
>> -      power-domains = <&rpmhpd RPMHPD_MMCX>;
>> +      power-domains = <&rpmhpd RPMHPD_MMCX>,
>> +                      <&rpmhpd RPMHPD_MXC>;
>>        required-opps = <&rpmhpd_opp_low_svs>;
> 
> As pointed out by Vladimir, you probably also need a second entry in
> required-opps.
> 

Sure, will check and add second entry in the required-opps.

Thanks,
Jagadeesh

>>        #clock-cells = <1>;
>>        #reset-cells = <1>;
>>
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH v2 1/8] dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain

[Bryan O'Donoghue]

On 06/03/2025 08:55, Jagadeesh Kona wrote:
> To configure the video PLLs and enable the video GDSCs on SM8450,
> SM8475, SM8550 and SM8650 platforms, the MXC rail must be ON along
> with MMCX. Therefore, update the videocc bindings to include
> the MXC power domain on these platforms.
> 
> Fixes: 1e910b2ba0ed ("dt-bindings: clock: qcom: Add SM8450 video clock controller")
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Acked-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>   Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
> index 62714fa54db82491a7a108f7f18a253d737f8d61..737efc4b46564c1e475b02873d2dc124329fb775 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
> @@ -32,9 +32,11 @@ properties:
>         - description: Video AHB clock from GCC
> 
>     power-domains:
> -    maxItems: 1
>       description:
> -      MMCX power domain.
> +      Power domains required for the clock controller to operate
> +    items:
> +      - description: MMCX power domain
> +      - description: MXC power domain
> 
>     required-opps:
>       maxItems: 1
> @@ -72,7 +74,8 @@ examples:
>         reg = <0x0aaf0000 0x10000>;
>         clocks = <&rpmhcc RPMH_CXO_CLK>,
>                  <&gcc GCC_VIDEO_AHB_CLK>;
> -      power-domains = <&rpmhpd RPMHPD_MMCX>;
> +      power-domains = <&rpmhpd RPMHPD_MMCX>,
> +                      <&rpmhpd RPMHPD_MXC>;
>         required-opps = <&rpmhpd_opp_low_svs>;
>         #clock-cells = <1>;
>         #reset-cells = <1>;
> 
> --
> 2.34.1
> 
> 

The ordering of these patches is a bit weird with this binding first and 
then the rest of the bindings later.

Also switched my linux-arm-msm email recently so only got the first 
patch with my RB in my Linaro inbox.

Suggest as standard practice when you get review feedback to CC previous 
reviewers on all patches in subsequent series, especially if you are 
picking up an RB on one of those patches.

TL;DR please cc me on V3.

---
bod

Re: [PATCH v2 1/8] dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain

[Konrad Dybcio]

On 3/11/25 10:49 AM, Bryan O'Donoghue wrote:
> On 06/03/2025 08:55, Jagadeesh Kona wrote:
>> To configure the video PLLs and enable the video GDSCs on SM8450,
>> SM8475, SM8550 and SM8650 platforms, the MXC rail must be ON along
>> with MMCX. Therefore, update the videocc bindings to include
>> the MXC power domain on these platforms.
>>
>> Fixes: 1e910b2ba0ed ("dt-bindings: clock: qcom: Add SM8450 video clock controller")
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> Acked-by: Rob Herring (Arm) <robh@kernel.org>
>> ---
>>   Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> index 62714fa54db82491a7a108f7f18a253d737f8d61..737efc4b46564c1e475b02873d2dc124329fb775 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> @@ -32,9 +32,11 @@ properties:
>>         - description: Video AHB clock from GCC
>>
>>     power-domains:
>> -    maxItems: 1
>>       description:
>> -      MMCX power domain.
>> +      Power domains required for the clock controller to operate
>> +    items:
>> +      - description: MMCX power domain
>> +      - description: MXC power domain
>>
>>     required-opps:
>>       maxItems: 1
>> @@ -72,7 +74,8 @@ examples:
>>         reg = <0x0aaf0000 0x10000>;
>>         clocks = <&rpmhcc RPMH_CXO_CLK>,
>>                  <&gcc GCC_VIDEO_AHB_CLK>;
>> -      power-domains = <&rpmhpd RPMHPD_MMCX>;
>> +      power-domains = <&rpmhpd RPMHPD_MMCX>,
>> +                      <&rpmhpd RPMHPD_MXC>;
>>         required-opps = <&rpmhpd_opp_low_svs>;
>>         #clock-cells = <1>;
>>         #reset-cells = <1>;
>>
>> -- 
>> 2.34.1
>>
>>
> 
> The ordering of these patches is a bit weird with this binding first and then the rest of the bindings later.
> 
> Also switched my linux-arm-msm email recently so only got the first patch with my RB in my Linaro inbox.
> 
> Suggest as standard practice when you get review feedback to CC previous reviewers on all patches in subsequent series, especially if you are picking up an RB on one of those patches.
> 
> TL;DR please cc me on V3.

If you pick up review tags, running b4 prep -c again will CC the folks

Konrad

Re: [PATCH v2 1/8] dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain

[Jagadeesh Kona]

On 3/11/2025 3:51 PM, Konrad Dybcio wrote:
> On 3/11/25 10:49 AM, Bryan O'Donoghue wrote:
>> On 06/03/2025 08:55, Jagadeesh Kona wrote:
>>> To configure the video PLLs and enable the video GDSCs on SM8450,
>>> SM8475, SM8550 and SM8650 platforms, the MXC rail must be ON along
>>> with MMCX. Therefore, update the videocc bindings to include
>>> the MXC power domain on these platforms.
>>>
>>> Fixes: 1e910b2ba0ed ("dt-bindings: clock: qcom: Add SM8450 video clock controller")
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> Acked-by: Rob Herring (Arm) <robh@kernel.org>
>>> ---
>>>   Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 9 ++++++---
>>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>>> index 62714fa54db82491a7a108f7f18a253d737f8d61..737efc4b46564c1e475b02873d2dc124329fb775 100644
>>> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>>> @@ -32,9 +32,11 @@ properties:
>>>         - description: Video AHB clock from GCC
>>>
>>>     power-domains:
>>> -    maxItems: 1
>>>       description:
>>> -      MMCX power domain.
>>> +      Power domains required for the clock controller to operate
>>> +    items:
>>> +      - description: MMCX power domain
>>> +      - description: MXC power domain
>>>
>>>     required-opps:
>>>       maxItems: 1
>>> @@ -72,7 +74,8 @@ examples:
>>>         reg = <0x0aaf0000 0x10000>;
>>>         clocks = <&rpmhcc RPMH_CXO_CLK>,
>>>                  <&gcc GCC_VIDEO_AHB_CLK>;
>>> -      power-domains = <&rpmhpd RPMHPD_MMCX>;
>>> +      power-domains = <&rpmhpd RPMHPD_MMCX>,
>>> +                      <&rpmhpd RPMHPD_MXC>;
>>>         required-opps = <&rpmhpd_opp_low_svs>;
>>>         #clock-cells = <1>;
>>>         #reset-cells = <1>;
>>>
>>> -- 
>>> 2.34.1
>>>
>>>
>>
>> The ordering of these patches is a bit weird with this binding first and then the rest of the bindings later.
>>
>> Also switched my linux-arm-msm email recently so only got the first patch with my RB in my Linaro inbox.
>>
>> Suggest as standard practice when you get review feedback to CC previous reviewers on all patches in subsequent series, especially if you are picking up an RB on one of those patches.
>>
>> TL;DR please cc me on V3.
> 
> If you pick up review tags, running b4 prep -c again will CC the folks
> 

Yes, I ran b4 prep --auto-to-cc, but looks like b4 sent the email in CC only if there is an explicit
review tag. For patches that didn't have an explicit review tag, it didn't include in the CC. 

I will ensure to add Bryan in CC for all patches in v3.

Thanks,
Jagadeesh

> Konrad

[PATCH v2 2/8] clk: qcom: common: Add support to configure PLL

[Jagadeesh Kona]

From: Taniya Das <quic_tdas@quicinc.com>

Integrate PLL configuration into clk_alpha_pll structure and add support
for qcom_cc_clk_alpha_pll_configure() function which can be used to
configure the clock controller PLLs from common core code.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 drivers/clk/qcom/clk-alpha-pll.h |  2 ++
 drivers/clk/qcom/common.c        | 55 ++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/common.h        |  1 +
 3 files changed, 58 insertions(+)

diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index 79aca8525262211ae5295245427d4540abf1e09a..943320cdcd10a6c07fcd74dccb88be847dc086c2 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -81,6 +81,7 @@ struct pll_vco {
  * struct clk_alpha_pll - phase locked loop (PLL)
  * @offset: base address of registers
  * @regs: alpha pll register map (see @clk_alpha_pll_regs)
+ * @config: array of pll settings
  * @vco_table: array of VCO settings
  * @num_vco: number of VCO settings in @vco_table
  * @flags: bitmask to indicate features supported by the hardware
@@ -90,6 +91,7 @@ struct clk_alpha_pll {
 	u32 offset;
 	const u8 *regs;
 
+	const struct alpha_pll_config *config;
 	const struct pll_vco *vco_table;
 	size_t num_vco;
 #define SUPPORTS_OFFLINE_REQ		BIT(0)
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 9e3380fd718198c9fe63d7361615a91c3ecb3d60..74d062b5da0647f7f2bd8fd7a004ffdb1116c1ea 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -13,6 +13,7 @@
 #include <linux/of.h>
 
 #include "common.h"
+#include "clk-alpha-pll.h"
 #include "clk-rcg.h"
 #include "clk-regmap.h"
 #include "reset.h"
@@ -284,6 +285,60 @@ static int qcom_cc_icc_register(struct device *dev,
 						     desc->num_icc_hws, icd);
 }
 
+static void qcom_cc_clk_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
+{
+	if (!pll->config || !pll->regs)
+		return;
+
+	switch (GET_PLL_TYPE(pll)) {
+	case CLK_ALPHA_PLL_TYPE_LUCID_OLE:
+		clk_lucid_ole_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_LUCID_EVO:
+		clk_lucid_evo_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_TAYCAN_ELU:
+		clk_taycan_elu_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_RIVIAN_EVO:
+		clk_rivian_evo_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_TRION:
+		clk_trion_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_HUAYRA_2290:
+		clk_huayra_2290_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_FABIA:
+		clk_fabia_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_AGERA:
+		clk_agera_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_PONGO_ELU:
+		clk_pongo_elu_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_ZONDA:
+	case CLK_ALPHA_PLL_TYPE_ZONDA_OLE:
+		clk_zonda_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_STROMER:
+	case CLK_ALPHA_PLL_TYPE_STROMER_PLUS:
+		clk_stromer_pll_configure(pll, regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_DEFAULT:
+	case CLK_ALPHA_PLL_TYPE_DEFAULT_EVO:
+	case CLK_ALPHA_PLL_TYPE_HUAYRA:
+	case CLK_ALPHA_PLL_TYPE_HUAYRA_APSS:
+	case CLK_ALPHA_PLL_TYPE_BRAMMO:
+	case CLK_ALPHA_PLL_TYPE_BRAMMO_EVO:
+		clk_alpha_pll_configure(pll, regmap, pll->config);
+		break;
+	default:
+		break;
+	}
+}
+
 int qcom_cc_really_probe(struct device *dev,
 			 const struct qcom_cc_desc *desc, struct regmap *regmap)
 {
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index 7ace5d7f5836aa81431153ba92d8f14f2ffe8147..2066c8937936235d7bd03ab3225d4b3f4fb08dd0 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -18,6 +18,7 @@ struct clk_hw;
 #define PLL_BIAS_COUNT_MASK	0x3f
 #define PLL_VOTE_FSM_ENA	BIT(20)
 #define PLL_VOTE_FSM_RESET	BIT(21)
+#define GET_PLL_TYPE(pll)	((pll->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)
 
 struct qcom_icc_hws_data {
 	int master_id;

-- 
2.34.1

Re: [PATCH v2 2/8] clk: qcom: common: Add support to configure PLL

[Konrad Dybcio]

On 6.03.2025 9:55 AM, Jagadeesh Kona wrote:
> From: Taniya Das <quic_tdas@quicinc.com>
> 
> Integrate PLL configuration into clk_alpha_pll structure and add support
> for qcom_cc_clk_alpha_pll_configure() function which can be used to
> configure the clock controller PLLs from common core code.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---

[...]

> +static void qcom_cc_clk_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
> +{
> +	if (!pll->config || !pll->regs)
> +		return;

This should probably throw some sort of a warning

> +
> +	switch (GET_PLL_TYPE(pll)) {
> +	case CLK_ALPHA_PLL_TYPE_LUCID_OLE:
> +		clk_lucid_ole_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_LUCID_EVO:
> +		clk_lucid_evo_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_TAYCAN_ELU:
> +		clk_taycan_elu_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_RIVIAN_EVO:
> +		clk_rivian_evo_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_TRION:
> +		clk_trion_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_2290:
> +		clk_huayra_2290_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_FABIA:
> +		clk_fabia_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_AGERA:
> +		clk_agera_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_PONGO_ELU:
> +		clk_pongo_elu_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_ZONDA:
> +	case CLK_ALPHA_PLL_TYPE_ZONDA_OLE:
> +		clk_zonda_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_STROMER:
> +	case CLK_ALPHA_PLL_TYPE_STROMER_PLUS:
> +		clk_stromer_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_DEFAULT:
> +	case CLK_ALPHA_PLL_TYPE_DEFAULT_EVO:
> +	case CLK_ALPHA_PLL_TYPE_HUAYRA:
> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_APSS:
> +	case CLK_ALPHA_PLL_TYPE_BRAMMO:
> +	case CLK_ALPHA_PLL_TYPE_BRAMMO_EVO:
> +		clk_alpha_pll_configure(pll, regmap, pll->config);
> +		break;
> +	default:
> +		break;

And so should the 'default' case

Konrad

Re: [PATCH v2 2/8] clk: qcom: common: Add support to configure PLL

[Jagadeesh Kona]

On 3/6/2025 5:52 PM, Konrad Dybcio wrote:
> On 6.03.2025 9:55 AM, Jagadeesh Kona wrote:
>> From: Taniya Das <quic_tdas@quicinc.com>
>>
>> Integrate PLL configuration into clk_alpha_pll structure and add support
>> for qcom_cc_clk_alpha_pll_configure() function which can be used to
>> configure the clock controller PLLs from common core code.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
> 
> [...]
> 
>> +static void qcom_cc_clk_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
>> +{
>> +	if (!pll->config || !pll->regs)
>> +		return;
> 
> This should probably throw some sort of a warning
> 

Yes, will add a warning here and for default case in next series.

Thanks,
Jagadeesh

>> +
>> +	switch (GET_PLL_TYPE(pll)) {
>> +	case CLK_ALPHA_PLL_TYPE_LUCID_OLE:
>> +		clk_lucid_ole_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_LUCID_EVO:
>> +		clk_lucid_evo_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_TAYCAN_ELU:
>> +		clk_taycan_elu_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_RIVIAN_EVO:
>> +		clk_rivian_evo_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_TRION:
>> +		clk_trion_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_2290:
>> +		clk_huayra_2290_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_FABIA:
>> +		clk_fabia_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_AGERA:
>> +		clk_agera_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_PONGO_ELU:
>> +		clk_pongo_elu_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_ZONDA:
>> +	case CLK_ALPHA_PLL_TYPE_ZONDA_OLE:
>> +		clk_zonda_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_STROMER:
>> +	case CLK_ALPHA_PLL_TYPE_STROMER_PLUS:
>> +		clk_stromer_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_DEFAULT:
>> +	case CLK_ALPHA_PLL_TYPE_DEFAULT_EVO:
>> +	case CLK_ALPHA_PLL_TYPE_HUAYRA:
>> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_APSS:
>> +	case CLK_ALPHA_PLL_TYPE_BRAMMO:
>> +	case CLK_ALPHA_PLL_TYPE_BRAMMO_EVO:
>> +		clk_alpha_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	default:
>> +		break;
> 
> And so should the 'default' case
> 
> Konrad

Re: [PATCH v2 2/8] clk: qcom: common: Add support to configure PLL

[Bjorn Andersson]

On Thu, Mar 06, 2025 at 02:25:34PM +0530, Jagadeesh Kona wrote:
> From: Taniya Das <quic_tdas@quicinc.com>
> 
> Integrate PLL configuration into clk_alpha_pll structure and add support
> for qcom_cc_clk_alpha_pll_configure() function which can be used to
> configure the clock controller PLLs from common core code.

https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
starts with "Describe your problem."

I don't see a problem description here.

> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>  drivers/clk/qcom/clk-alpha-pll.h |  2 ++
>  drivers/clk/qcom/common.c        | 55 ++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/qcom/common.h        |  1 +
>  3 files changed, 58 insertions(+)
> 
> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
> index 79aca8525262211ae5295245427d4540abf1e09a..943320cdcd10a6c07fcd74dccb88be847dc086c2 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.h
> +++ b/drivers/clk/qcom/clk-alpha-pll.h
> @@ -81,6 +81,7 @@ struct pll_vco {
>   * struct clk_alpha_pll - phase locked loop (PLL)
>   * @offset: base address of registers
>   * @regs: alpha pll register map (see @clk_alpha_pll_regs)
> + * @config: array of pll settings
>   * @vco_table: array of VCO settings
>   * @num_vco: number of VCO settings in @vco_table
>   * @flags: bitmask to indicate features supported by the hardware
> @@ -90,6 +91,7 @@ struct clk_alpha_pll {
>  	u32 offset;
>  	const u8 *regs;
>  
> +	const struct alpha_pll_config *config;
>  	const struct pll_vco *vco_table;
>  	size_t num_vco;
>  #define SUPPORTS_OFFLINE_REQ		BIT(0)
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 9e3380fd718198c9fe63d7361615a91c3ecb3d60..74d062b5da0647f7f2bd8fd7a004ffdb1116c1ea 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -13,6 +13,7 @@
>  #include <linux/of.h>
>  
>  #include "common.h"
> +#include "clk-alpha-pll.h"
>  #include "clk-rcg.h"
>  #include "clk-regmap.h"
>  #include "reset.h"
> @@ -284,6 +285,60 @@ static int qcom_cc_icc_register(struct device *dev,
>  						     desc->num_icc_hws, icd);
>  }
>  
> +static void qcom_cc_clk_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
> +{
> +	if (!pll->config || !pll->regs)
> +		return;
> +
> +	switch (GET_PLL_TYPE(pll)) {
> +	case CLK_ALPHA_PLL_TYPE_LUCID_OLE:
> +		clk_lucid_ole_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_LUCID_EVO:
> +		clk_lucid_evo_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_TAYCAN_ELU:
> +		clk_taycan_elu_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_RIVIAN_EVO:
> +		clk_rivian_evo_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_TRION:
> +		clk_trion_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_2290:
> +		clk_huayra_2290_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_FABIA:
> +		clk_fabia_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_AGERA:
> +		clk_agera_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_PONGO_ELU:
> +		clk_pongo_elu_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_ZONDA:
> +	case CLK_ALPHA_PLL_TYPE_ZONDA_OLE:
> +		clk_zonda_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_STROMER:
> +	case CLK_ALPHA_PLL_TYPE_STROMER_PLUS:
> +		clk_stromer_pll_configure(pll, regmap, pll->config);
> +		break;
> +	case CLK_ALPHA_PLL_TYPE_DEFAULT:
> +	case CLK_ALPHA_PLL_TYPE_DEFAULT_EVO:
> +	case CLK_ALPHA_PLL_TYPE_HUAYRA:
> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_APSS:
> +	case CLK_ALPHA_PLL_TYPE_BRAMMO:
> +	case CLK_ALPHA_PLL_TYPE_BRAMMO_EVO:
> +		clk_alpha_pll_configure(pll, regmap, pll->config);
> +		break;
> +	default:

This would be annoying to hit when adding a new PLL type, a BUG(); would
be useful here.

> +		break;
> +	}
> +}
> +
>  int qcom_cc_really_probe(struct device *dev,
>  			 const struct qcom_cc_desc *desc, struct regmap *regmap)
>  {
> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
> index 7ace5d7f5836aa81431153ba92d8f14f2ffe8147..2066c8937936235d7bd03ab3225d4b3f4fb08dd0 100644
> --- a/drivers/clk/qcom/common.h
> +++ b/drivers/clk/qcom/common.h
> @@ -18,6 +18,7 @@ struct clk_hw;
>  #define PLL_BIAS_COUNT_MASK	0x3f
>  #define PLL_VOTE_FSM_ENA	BIT(20)
>  #define PLL_VOTE_FSM_RESET	BIT(21)
> +#define GET_PLL_TYPE(pll)	((pll->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)

Why would this go in qcom/common.h, when clk_alpha_pll_regs is defined
in clk-alpha-pll.h?

Regards,
Bjorn

>  
>  struct qcom_icc_hws_data {
>  	int master_id;
> 
> -- 
> 2.34.1
> 

Re: [PATCH v2 2/8] clk: qcom: common: Add support to configure PLL

[Jagadeesh Kona]

On 3/14/2025 4:09 AM, Bjorn Andersson wrote:
> On Thu, Mar 06, 2025 at 02:25:34PM +0530, Jagadeesh Kona wrote:
>> From: Taniya Das <quic_tdas@quicinc.com>
>>
>> Integrate PLL configuration into clk_alpha_pll structure and add support
>> for qcom_cc_clk_alpha_pll_configure() function which can be used to
>> configure the clock controller PLLs from common core code.
> 
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> starts with "Describe your problem."
> 
> I don't see a problem description here.
> 

Yes, I will update the commit text with problem description in next series.

>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
>>  drivers/clk/qcom/clk-alpha-pll.h |  2 ++
>>  drivers/clk/qcom/common.c        | 55 ++++++++++++++++++++++++++++++++++++++++
>>  drivers/clk/qcom/common.h        |  1 +
>>  3 files changed, 58 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
>> index 79aca8525262211ae5295245427d4540abf1e09a..943320cdcd10a6c07fcd74dccb88be847dc086c2 100644
>> --- a/drivers/clk/qcom/clk-alpha-pll.h
>> +++ b/drivers/clk/qcom/clk-alpha-pll.h
>> @@ -81,6 +81,7 @@ struct pll_vco {
>>   * struct clk_alpha_pll - phase locked loop (PLL)
>>   * @offset: base address of registers
>>   * @regs: alpha pll register map (see @clk_alpha_pll_regs)
>> + * @config: array of pll settings
>>   * @vco_table: array of VCO settings
>>   * @num_vco: number of VCO settings in @vco_table
>>   * @flags: bitmask to indicate features supported by the hardware
>> @@ -90,6 +91,7 @@ struct clk_alpha_pll {
>>  	u32 offset;
>>  	const u8 *regs;
>>  
>> +	const struct alpha_pll_config *config;
>>  	const struct pll_vco *vco_table;
>>  	size_t num_vco;
>>  #define SUPPORTS_OFFLINE_REQ		BIT(0)
>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>> index 9e3380fd718198c9fe63d7361615a91c3ecb3d60..74d062b5da0647f7f2bd8fd7a004ffdb1116c1ea 100644
>> --- a/drivers/clk/qcom/common.c
>> +++ b/drivers/clk/qcom/common.c
>> @@ -13,6 +13,7 @@
>>  #include <linux/of.h>
>>  
>>  #include "common.h"
>> +#include "clk-alpha-pll.h"
>>  #include "clk-rcg.h"
>>  #include "clk-regmap.h"
>>  #include "reset.h"
>> @@ -284,6 +285,60 @@ static int qcom_cc_icc_register(struct device *dev,
>>  						     desc->num_icc_hws, icd);
>>  }
>>  
>> +static void qcom_cc_clk_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
>> +{
>> +	if (!pll->config || !pll->regs)
>> +		return;
>> +
>> +	switch (GET_PLL_TYPE(pll)) {
>> +	case CLK_ALPHA_PLL_TYPE_LUCID_OLE:
>> +		clk_lucid_ole_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_LUCID_EVO:
>> +		clk_lucid_evo_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_TAYCAN_ELU:
>> +		clk_taycan_elu_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_RIVIAN_EVO:
>> +		clk_rivian_evo_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_TRION:
>> +		clk_trion_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_2290:
>> +		clk_huayra_2290_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_FABIA:
>> +		clk_fabia_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_AGERA:
>> +		clk_agera_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_PONGO_ELU:
>> +		clk_pongo_elu_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_ZONDA:
>> +	case CLK_ALPHA_PLL_TYPE_ZONDA_OLE:
>> +		clk_zonda_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_STROMER:
>> +	case CLK_ALPHA_PLL_TYPE_STROMER_PLUS:
>> +		clk_stromer_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_DEFAULT:
>> +	case CLK_ALPHA_PLL_TYPE_DEFAULT_EVO:
>> +	case CLK_ALPHA_PLL_TYPE_HUAYRA:
>> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_APSS:
>> +	case CLK_ALPHA_PLL_TYPE_BRAMMO:
>> +	case CLK_ALPHA_PLL_TYPE_BRAMMO_EVO:
>> +		clk_alpha_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	default:
> 
> This would be annoying to hit when adding a new PLL type, a BUG(); would
> be useful here.
> 

Yes, will add BUG() here in next series.

>> +		break;
>> +	}
>> +}
>> +
>>  int qcom_cc_really_probe(struct device *dev,
>>  			 const struct qcom_cc_desc *desc, struct regmap *regmap)
>>  {
>> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
>> index 7ace5d7f5836aa81431153ba92d8f14f2ffe8147..2066c8937936235d7bd03ab3225d4b3f4fb08dd0 100644
>> --- a/drivers/clk/qcom/common.h
>> +++ b/drivers/clk/qcom/common.h
>> @@ -18,6 +18,7 @@ struct clk_hw;
>>  #define PLL_BIAS_COUNT_MASK	0x3f
>>  #define PLL_VOTE_FSM_ENA	BIT(20)
>>  #define PLL_VOTE_FSM_RESET	BIT(21)
>> +#define GET_PLL_TYPE(pll)	((pll->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)
> 
> Why would this go in qcom/common.h, when clk_alpha_pll_regs is defined
> in clk-alpha-pll.h?
> 

Will move the macro to clk alpha pll code in next series.

Thanks,
Jagadeesh

> Regards,
> Bjorn
> 
>>  
>>  struct qcom_icc_hws_data {
>>  	int master_id;
>>
>> -- 
>> 2.34.1
>>

[PATCH v2 3/8] clk: qcom: common: Manage rpm, configure PLLs & AON clks in really probe

[Jagadeesh Kona]

Add support for runtime power management, PLL configuration and enabling
critical clocks in qcom_cc_really_probe() to commonize the clock
controller probe.

The runtime power management is not required for all clock controllers,
hence handle the rpm based on use_rpm flag in clock controller descriptor.
Also the power domains need to be kept enabled during pll configuration,
hence attach all required power domains prior to calling get_sync() on the
device.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 drivers/clk/qcom/common.c | 45 ++++++++++++++++++++++++++++++++++++---------
 drivers/clk/qcom/common.h | 16 ++++++++++++++++
 2 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 74d062b5da0647f7f2bd8fd7a004ffdb1116c1ea..ce87f74fa51639ed270a0c56fff1cd2845885647 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -9,6 +9,7 @@
 #include <linux/platform_device.h>
 #include <linux/clk-provider.h>
 #include <linux/interconnect-clk.h>
+#include <linux/pm_runtime.h>
 #include <linux/reset-controller.h>
 #include <linux/of.h>
 
@@ -350,6 +351,7 @@ int qcom_cc_really_probe(struct device *dev,
 	struct clk_regmap **rclks = desc->clks;
 	size_t num_clk_hws = desc->num_clk_hws;
 	struct clk_hw **clk_hws = desc->clk_hws;
+	struct qcom_clk_cfg *clks_cfg = desc->clks_cfg;
 
 	cc = devm_kzalloc(dev, sizeof(*cc), GFP_KERNEL);
 	if (!cc)
@@ -359,6 +361,23 @@ int qcom_cc_really_probe(struct device *dev,
 	if (ret < 0 && ret != -EEXIST)
 		return ret;
 
+	if (desc->use_rpm) {
+		ret = devm_pm_runtime_enable(dev);
+		if (ret)
+			return ret;
+
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < desc->num_plls; i++)
+		qcom_cc_clk_pll_configure(desc->plls[i], regmap);
+
+	for (i = 0 ; i < desc->num_clks_cfg; i++)
+		regmap_update_bits(regmap, clks_cfg[i].offset,
+				   clks_cfg[i].mask, clks_cfg[i].mask);
+
 	reset = &cc->reset;
 	reset->rcdev.of_node = dev->of_node;
 	reset->rcdev.ops = &qcom_reset_ops;
@@ -369,23 +388,25 @@ int qcom_cc_really_probe(struct device *dev,
 
 	ret = devm_reset_controller_register(dev, &reset->rcdev);
 	if (ret)
-		return ret;
+		goto put_rpm;
 
 	if (desc->gdscs && desc->num_gdscs) {
 		scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
-		if (!scd)
-			return -ENOMEM;
+		if (!scd) {
+			ret = -ENOMEM;
+			goto put_rpm;
+		}
 		scd->dev = dev;
 		scd->scs = desc->gdscs;
 		scd->num = desc->num_gdscs;
 		scd->pd_list = cc->pd_list;
 		ret = gdsc_register(scd, &reset->rcdev, regmap);
 		if (ret)
-			return ret;
+			goto put_rpm;
 		ret = devm_add_action_or_reset(dev, qcom_cc_gdsc_unregister,
 					       scd);
 		if (ret)
-			return ret;
+			goto put_rpm;
 	}
 
 	cc->rclks = rclks;
@@ -396,7 +417,7 @@ int qcom_cc_really_probe(struct device *dev,
 	for (i = 0; i < num_clk_hws; i++) {
 		ret = devm_clk_hw_register(dev, clk_hws[i]);
 		if (ret)
-			return ret;
+			goto put_rpm;
 	}
 
 	for (i = 0; i < num_clks; i++) {
@@ -405,14 +426,20 @@ int qcom_cc_really_probe(struct device *dev,
 
 		ret = devm_clk_register_regmap(dev, rclks[i]);
 		if (ret)
-			return ret;
+			goto put_rpm;
 	}
 
 	ret = devm_of_clk_add_hw_provider(dev, qcom_cc_clk_hw_get, cc);
 	if (ret)
-		return ret;
+		goto put_rpm;
+
+	ret = qcom_cc_icc_register(dev, desc);
+
+put_rpm:
+	if (desc->use_rpm)
+		pm_runtime_put(dev);
 
-	return qcom_cc_icc_register(dev, desc);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(qcom_cc_really_probe);
 
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index 2066c8937936235d7bd03ab3225d4b3f4fb08dd0..da27290b7da3ddc6d8ba1b064619e294bfcb686c 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -26,6 +26,17 @@ struct qcom_icc_hws_data {
 	int clk_id;
 };
 
+/**
+ * struct qcom_clk_cfg - To maintain list of clocks that needs to be
+ *                       kept ON or misc clock register settings
+ * @offset: address offset for clock register
+ * @mask: bit mask to indicate the bits to update
+ */
+struct qcom_clk_cfg {
+	unsigned int offset;
+	unsigned int mask;
+};
+
 struct qcom_cc_desc {
 	const struct regmap_config *config;
 	struct clk_regmap **clks;
@@ -39,6 +50,11 @@ struct qcom_cc_desc {
 	const struct qcom_icc_hws_data *icc_hws;
 	size_t num_icc_hws;
 	unsigned int icc_first_node_id;
+	struct qcom_clk_cfg *clks_cfg;
+	size_t num_clks_cfg;
+	struct clk_alpha_pll **plls;
+	size_t num_plls;
+	bool use_rpm;
 };
 
 /**

-- 
2.34.1

Re: [PATCH v2 3/8] clk: qcom: common: Manage rpm, configure PLLs & AON clks in really probe

[Konrad Dybcio]

On 6.03.2025 9:55 AM, Jagadeesh Kona wrote:
> Add support for runtime power management, PLL configuration and enabling
> critical clocks in qcom_cc_really_probe() to commonize the clock
> controller probe.
> 
> The runtime power management is not required for all clock controllers,
> hence handle the rpm based on use_rpm flag in clock controller descriptor.
> Also the power domains need to be kept enabled during pll configuration,
> hence attach all required power domains prior to calling get_sync() on the
> device.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---

[...]

> + * struct qcom_clk_cfg - To maintain list of clocks that needs to be
> + *                       kept ON or misc clock register settings

I have some reservations about this name, particularly since 'clk_cfg'
has already been used in the msmbus/interconnect space..

Perhaps qcom_clk_reg_settings?

> + * @offset: address offset for clock register
> + * @mask: bit mask to indicate the bits to update
> + */
> +struct qcom_clk_cfg {
> +	unsigned int offset;
> +	unsigned int mask;

"u32"

also, to expand it, we probably want "field" and "val" to replace the
calls to regmap_update_bits in some drivers

I think we may keep this /\ struct for things like:

/* Enable clock gating for MDP clocks */
regmap_update_bits(regmap, DISP_CC_MISC_CMD, 0x10, 0x10);

while keeping a separate u32 array of branch clocks to call
qcom_branch_set_clk_en() on - we would then use 3x less memory


> +};

> +
>  struct qcom_cc_desc {
>  	const struct regmap_config *config;
>  	struct clk_regmap **clks;
> @@ -39,6 +50,11 @@ struct qcom_cc_desc {
>  	const struct qcom_icc_hws_data *icc_hws;
>  	size_t num_icc_hws;
>  	unsigned int icc_first_node_id;
> +	struct qcom_clk_cfg *clks_cfg;
> +	size_t num_clks_cfg;
> +	struct clk_alpha_pll **plls;

Some ancient or "non-standard" SoCs have non-alpha PLLs, please rename
this to something like alpha_plls


Konrad

Re: [PATCH v2 3/8] clk: qcom: common: Manage rpm, configure PLLs & AON clks in really probe

[Jagadeesh Kona]

On 3/6/2025 6:15 PM, Konrad Dybcio wrote:
> On 6.03.2025 9:55 AM, Jagadeesh Kona wrote:
>> Add support for runtime power management, PLL configuration and enabling
>> critical clocks in qcom_cc_really_probe() to commonize the clock
>> controller probe.
>>
>> The runtime power management is not required for all clock controllers,
>> hence handle the rpm based on use_rpm flag in clock controller descriptor.
>> Also the power domains need to be kept enabled during pll configuration,
>> hence attach all required power domains prior to calling get_sync() on the
>> device.
>>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
> 
> [...]
> 
>> + * struct qcom_clk_cfg - To maintain list of clocks that needs to be
>> + *                       kept ON or misc clock register settings
> 
> I have some reservations about this name, particularly since 'clk_cfg'
> has already been used in the msmbus/interconnect space..
> 
> Perhaps qcom_clk_reg_settings?
> 

Sure, will update.

>> + * @offset: address offset for clock register
>> + * @mask: bit mask to indicate the bits to update
>> + */
>> +struct qcom_clk_cfg {
>> +	unsigned int offset;
>> +	unsigned int mask;
> 
> "u32"
> 
> also, to expand it, we probably want "field" and "val" to replace the
> calls to regmap_update_bits in some drivers
> 
> I think we may keep this /\ struct for things like:
> 
> /* Enable clock gating for MDP clocks */
> regmap_update_bits(regmap, DISP_CC_MISC_CMD, 0x10, 0x10);
> 
> while keeping a separate u32 array of branch clocks to call
> qcom_branch_set_clk_en() on - we would then use 3x less memory
> 
> 

Sure, will take care of these in next series.

>> +};
> 
>> +
>>  struct qcom_cc_desc {
>>  	const struct regmap_config *config;
>>  	struct clk_regmap **clks;
>> @@ -39,6 +50,11 @@ struct qcom_cc_desc {
>>  	const struct qcom_icc_hws_data *icc_hws;
>>  	size_t num_icc_hws;
>>  	unsigned int icc_first_node_id;
>> +	struct qcom_clk_cfg *clks_cfg;
>> +	size_t num_clks_cfg;
>> +	struct clk_alpha_pll **plls;
> 
> Some ancient or "non-standard" SoCs have non-alpha PLLs, please rename
> this to something like alpha_plls
> 

Sure, will update it.

Thanks,
Jagadeesh

> 
> Konrad

Re: [PATCH v2 3/8] clk: qcom: common: Manage rpm, configure PLLs & AON clks in really probe

[Dmitry Baryshkov]

On Thu, Mar 06, 2025 at 02:25:35PM +0530, Jagadeesh Kona wrote:
> Add support for runtime power management, PLL configuration and enabling
> critical clocks in qcom_cc_really_probe() to commonize the clock
> controller probe.

Please split this into two commits: one for the runtime PM and another
one for clock configuration, because ...

> 
> The runtime power management is not required for all clock controllers,
> hence handle the rpm based on use_rpm flag in clock controller descriptor.
> Also the power domains need to be kept enabled during pll configuration,
> hence attach all required power domains prior to calling get_sync() on the
> device.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>  drivers/clk/qcom/common.c | 45 ++++++++++++++++++++++++++++++++++++---------
>  drivers/clk/qcom/common.h | 16 ++++++++++++++++
>  2 files changed, 52 insertions(+), 9 deletions(-)

[...]

> +
> +	for (i = 0; i < desc->num_plls; i++)
> +		qcom_cc_clk_pll_configure(desc->plls[i], regmap);
> +
> +	for (i = 0 ; i < desc->num_clks_cfg; i++)
> +		regmap_update_bits(regmap, clks_cfg[i].offset,
> +				   clks_cfg[i].mask, clks_cfg[i].mask);
> +

... just calling regmap_update_bits() looks like a step backwards. In
the past several years we got several sensible wrappers and helpers. I
suggest having a callback instead of a fixed 'update bits' table.

>  	reset = &cc->reset;
>  	reset->rcdev.of_node = dev->of_node;
>  	reset->rcdev.ops = &qcom_reset_ops;

The RPM part is fine with me.

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/8] clk: qcom: common: Manage rpm, configure PLLs & AON clks in really probe

[Jagadeesh Kona]

On 3/7/2025 2:17 PM, Dmitry Baryshkov wrote:
> On Thu, Mar 06, 2025 at 02:25:35PM +0530, Jagadeesh Kona wrote:
>> Add support for runtime power management, PLL configuration and enabling
>> critical clocks in qcom_cc_really_probe() to commonize the clock
>> controller probe.
> 
> Please split this into two commits: one for the runtime PM and another
> one for clock configuration, because ...
> 

Sure, will split this in the next series.

>>
>> The runtime power management is not required for all clock controllers,
>> hence handle the rpm based on use_rpm flag in clock controller descriptor.
>> Also the power domains need to be kept enabled during pll configuration,
>> hence attach all required power domains prior to calling get_sync() on the
>> device.
>>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
>>  drivers/clk/qcom/common.c | 45 ++++++++++++++++++++++++++++++++++++---------
>>  drivers/clk/qcom/common.h | 16 ++++++++++++++++
>>  2 files changed, 52 insertions(+), 9 deletions(-)
> 
> [...]
> 
>> +
>> +	for (i = 0; i < desc->num_plls; i++)
>> +		qcom_cc_clk_pll_configure(desc->plls[i], regmap);
>> +
>> +	for (i = 0 ; i < desc->num_clks_cfg; i++)
>> +		regmap_update_bits(regmap, clks_cfg[i].offset,
>> +				   clks_cfg[i].mask, clks_cfg[i].mask);
>> +
> 
> ... just calling regmap_update_bits() looks like a step backwards. In
> the past several years we got several sensible wrappers and helpers. I
> suggest having a callback instead of a fixed 'update bits' table.
> 

Sure, will check and add a callback to handle all these clock config settings.

Thanks,
Jagadeesh

>>  	reset = &cc->reset;
>>  	reset->rcdev.of_node = dev->of_node;
>>  	reset->rcdev.ops = &qcom_reset_ops;
> 
> The RPM part is fine with me.
>

[PATCH v2 4/8] clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe

[Jagadeesh Kona]

Video PLLs on SM8450/SM8475 require both MMCX and MXC rails to be kept ON
to configure the PLLs properly. Hence move the PLL configuration and
enable critical clocks to qcom_cc_really_probe() which ensures all
required power domains are in enabled state before configuring the PLLs
or enabling the clocks.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 drivers/clk/qcom/videocc-sm8450.c | 49 ++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8450.c b/drivers/clk/qcom/videocc-sm8450.c
index f26c7eccb62e7eb8dbd022e2f01fa496eb570b3f..73d65ff1f1a3dc07898e553a6e38d5ada82f566a 100644
--- a/drivers/clk/qcom/videocc-sm8450.c
+++ b/drivers/clk/qcom/videocc-sm8450.c
@@ -63,6 +63,7 @@ static const struct alpha_pll_config sm8475_video_cc_pll0_config = {
 
 static struct clk_alpha_pll video_cc_pll0 = {
 	.offset = 0x0,
+	.config = &video_cc_pll0_config,
 	.vco_table = lucid_evo_vco,
 	.num_vco = ARRAY_SIZE(lucid_evo_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_EVO],
@@ -106,6 +107,7 @@ static const struct alpha_pll_config sm8475_video_cc_pll1_config = {
 
 static struct clk_alpha_pll video_cc_pll1 = {
 	.offset = 0x1000,
+	.config = &video_cc_pll1_config,
 	.vco_table = lucid_evo_vco,
 	.num_vco = ARRAY_SIZE(lucid_evo_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_EVO],
@@ -407,6 +409,17 @@ static const struct qcom_reset_map video_cc_sm8450_resets[] = {
 	[VIDEO_CC_MVS1C_CLK_ARES] = { .reg = 0x808c, .bit = 2, .udelay = 1000 },
 };
 
+static struct clk_alpha_pll *video_cc_sm8450_plls[] = {
+	&video_cc_pll0,
+	&video_cc_pll1,
+};
+
+static struct qcom_clk_cfg video_cc_sm8450_clocks_cfg[] = {
+	{ .offset = 0x80e4, .mask = BIT(0) }, /* VIDEO_CC_AHB_CLK */
+	{ .offset = 0x8114, .mask = BIT(0) }, /* VIDEO_CC_XO_CLK */
+	{ .offset = 0x8130, .mask = BIT(0) }, /* VIDEO_CC_SLEEP_CLK */
+};
+
 static const struct regmap_config video_cc_sm8450_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -423,6 +436,11 @@ static struct qcom_cc_desc video_cc_sm8450_desc = {
 	.num_resets = ARRAY_SIZE(video_cc_sm8450_resets),
 	.gdscs = video_cc_sm8450_gdscs,
 	.num_gdscs = ARRAY_SIZE(video_cc_sm8450_gdscs),
+	.plls = video_cc_sm8450_plls,
+	.num_plls = ARRAY_SIZE(video_cc_sm8450_plls),
+	.clks_cfg = video_cc_sm8450_clocks_cfg,
+	.num_clks_cfg = ARRAY_SIZE(video_cc_sm8450_clocks_cfg),
+	.use_rpm = true,
 };
 
 static const struct of_device_id video_cc_sm8450_match_table[] = {
@@ -435,21 +453,10 @@ MODULE_DEVICE_TABLE(of, video_cc_sm8450_match_table);
 static int video_cc_sm8450_probe(struct platform_device *pdev)
 {
 	struct regmap *regmap;
-	int ret;
-
-	ret = devm_pm_runtime_enable(&pdev->dev);
-	if (ret)
-		return ret;
-
-	ret = pm_runtime_resume_and_get(&pdev->dev);
-	if (ret)
-		return ret;
 
 	regmap = qcom_cc_map(pdev, &video_cc_sm8450_desc);
-	if (IS_ERR(regmap)) {
-		pm_runtime_put(&pdev->dev);
+	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
-	}
 
 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8475-videocc")) {
 		/* Update VideoCC PLL0 */
@@ -458,23 +465,11 @@ static int video_cc_sm8450_probe(struct platform_device *pdev)
 		/* Update VideoCC PLL1 */
 		video_cc_pll1.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE];
 
-		clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &sm8475_video_cc_pll0_config);
-		clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &sm8475_video_cc_pll1_config);
-	} else {
-		clk_lucid_evo_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
-		clk_lucid_evo_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
+		video_cc_pll0.config = &sm8475_video_cc_pll0_config;
+		video_cc_pll1.config = &sm8475_video_cc_pll1_config;
 	}
 
-	/* Keep some clocks always-on */
-	qcom_branch_set_clk_en(regmap, 0x80e4); /* VIDEO_CC_AHB_CLK */
-	qcom_branch_set_clk_en(regmap, 0x8130); /* VIDEO_CC_SLEEP_CLK */
-	qcom_branch_set_clk_en(regmap, 0x8114); /* VIDEO_CC_XO_CLK */
-
-	ret = qcom_cc_really_probe(&pdev->dev, &video_cc_sm8450_desc, regmap);
-
-	pm_runtime_put(&pdev->dev);
-
-	return ret;
+	return qcom_cc_really_probe(&pdev->dev, &video_cc_sm8450_desc, regmap);
 }
 
 static struct platform_driver video_cc_sm8450_driver = {

-- 
2.34.1

Re: [PATCH v2 4/8] clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe

[Bryan O'Donoghue]

On 06/03/2025 08:55, Jagadeesh Kona wrote:
> +
> +static struct qcom_clk_cfg video_cc_sm8450_clocks_cfg[] = {
> +	{ .offset = 0x80e4, .mask = BIT(0) }, /* VIDEO_CC_AHB_CLK */
> +	{ .offset = 0x8114, .mask = BIT(0) }, /* VIDEO_CC_XO_CLK */
> +	{ .offset = 0x8130, .mask = BIT(0) }, /* VIDEO_CC_SLEEP_CLK */
> +};

> 
> -	/* Keep some clocks always-on */
> -	qcom_branch_set_clk_en(regmap, 0x80e4); /* VIDEO_CC_AHB_CLK */
> -	qcom_branch_set_clk_en(regmap, 0x8130); /* VIDEO_CC_SLEEP_CLK */
> -	qcom_branch_set_clk_en(regmap, 0x8114); /* VIDEO_CC_XO_CLK */

Dmitry already said this but, this change feels like a loss of fidelity.

Please find a way to use the existing helper functions.

---
bod

[PATCH v2 5/8] clk: qcom: videocc-sm8550: Move PLL & clk configuration to really probe

[Jagadeesh Kona]

Video PLLs on SM8550/SM8650 require both MMCX and MXC rails to be kept ON
to configure the PLLs properly. Hence move the PLL configuration and
enable critical clocks to qcom_cc_really_probe() which ensures all
required power domains are in enabled state before configuring the PLLs
or enabling the clocks.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 drivers/clk/qcom/videocc-sm8550.c | 50 ++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
index 7c25a50cfa970dff55d701cb24bc3aa5924ca12d..a1076b5bc2759c0149fa00904c81064f3381254b 100644
--- a/drivers/clk/qcom/videocc-sm8550.c
+++ b/drivers/clk/qcom/videocc-sm8550.c
@@ -51,6 +51,7 @@ static struct alpha_pll_config video_cc_pll0_config = {
 
 static struct clk_alpha_pll video_cc_pll0 = {
 	.offset = 0x0,
+	.config = &video_cc_pll0_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -82,6 +83,7 @@ static struct alpha_pll_config video_cc_pll1_config = {
 
 static struct clk_alpha_pll video_cc_pll1 = {
 	.offset = 0x1000,
+	.config = &video_cc_pll1_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -511,6 +513,17 @@ static const struct qcom_reset_map video_cc_sm8550_resets[] = {
 	[VIDEO_CC_XO_CLK_ARES] = { .reg = 0x8124, .bit = 2, .udelay = 100 },
 };
 
+static struct clk_alpha_pll *video_cc_sm8550_plls[] = {
+	&video_cc_pll0,
+	&video_cc_pll1,
+};
+
+static struct qcom_clk_cfg video_cc_sm8550_clocks_cfg[] = {
+	{ .offset = 0x80f4, .mask = BIT(0) }, /* VIDEO_CC_AHB_CLK */
+	{ .offset = 0x8124, .mask = BIT(0) }, /* VIDEO_CC_XO_CLK */
+	{ .offset = 0x8140, .mask = BIT(0) }, /* VIDEO_CC_SLEEP_CLK */
+};
+
 static const struct regmap_config video_cc_sm8550_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -527,6 +540,11 @@ static struct qcom_cc_desc video_cc_sm8550_desc = {
 	.num_resets = ARRAY_SIZE(video_cc_sm8550_resets),
 	.gdscs = video_cc_sm8550_gdscs,
 	.num_gdscs = ARRAY_SIZE(video_cc_sm8550_gdscs),
+	.plls = video_cc_sm8550_plls,
+	.num_plls = ARRAY_SIZE(video_cc_sm8550_plls),
+	.clks_cfg = video_cc_sm8550_clocks_cfg,
+	.num_clks_cfg = ARRAY_SIZE(video_cc_sm8550_clocks_cfg),
+	.use_rpm = true,
 };
 
 static const struct of_device_id video_cc_sm8550_match_table[] = {
@@ -539,25 +557,12 @@ MODULE_DEVICE_TABLE(of, video_cc_sm8550_match_table);
 static int video_cc_sm8550_probe(struct platform_device *pdev)
 {
 	struct regmap *regmap;
-	int ret;
-	u32 sleep_clk_offset = 0x8140;
-
-	ret = devm_pm_runtime_enable(&pdev->dev);
-	if (ret)
-		return ret;
-
-	ret = pm_runtime_resume_and_get(&pdev->dev);
-	if (ret)
-		return ret;
 
 	regmap = qcom_cc_map(pdev, &video_cc_sm8550_desc);
-	if (IS_ERR(regmap)) {
-		pm_runtime_put(&pdev->dev);
+	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
-	}
 
 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
-		sleep_clk_offset = 0x8150;
 		video_cc_pll0_config.l = 0x1e;
 		video_cc_pll0_config.alpha = 0xa000;
 		video_cc_pll1_config.l = 0x2b;
@@ -569,21 +574,12 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
 		video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr;
 		video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr;
 		video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr;
-	}
-
-	clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
-	clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
 
-	/* Keep some clocks always-on */
-	qcom_branch_set_clk_en(regmap, 0x80f4); /* VIDEO_CC_AHB_CLK */
-	qcom_branch_set_clk_en(regmap, sleep_clk_offset); /* VIDEO_CC_SLEEP_CLK */
-	qcom_branch_set_clk_en(regmap, 0x8124); /* VIDEO_CC_XO_CLK */
-
-	ret = qcom_cc_really_probe(&pdev->dev, &video_cc_sm8550_desc, regmap);
-
-	pm_runtime_put(&pdev->dev);
+		/* Sleep clock offset changed to 0x8150 on SM8650 */
+		video_cc_sm8550_clocks_cfg[2].offset = 0x8150;
+	}
 
-	return ret;
+	return qcom_cc_really_probe(&pdev->dev, &video_cc_sm8550_desc, regmap);
 }
 
 static struct platform_driver video_cc_sm8550_driver = {

-- 
2.34.1

[PATCH v2 6/8] arm64: dts: qcom: Add MXC power domain to videocc node on SM8450

[Jagadeesh Kona]

Videocc requires both MMCX and MXC rails to be powered ON to configure
the video PLLs on SM8450 platform. Hence add MXC power domain to videocc
node on SM8450.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 9c809fc5fa45a98ff5441a0b6809931588897243..4f8dca8fc64212191780067c5d8815e3a2bb137f 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -3136,7 +3136,8 @@ videocc: clock-controller@aaf0000 {
 			reg = <0 0x0aaf0000 0 0x10000>;
 			clocks = <&rpmhcc RPMH_CXO_CLK>,
 				 <&gcc GCC_VIDEO_AHB_CLK>;
-			power-domains = <&rpmhpd RPMHPD_MMCX>;
+			power-domains = <&rpmhpd RPMHPD_MMCX>,
+					<&rpmhpd RPMHPD_MXC>;
 			required-opps = <&rpmhpd_opp_low_svs>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;

-- 
2.34.1

[PATCH v2 7/8] arm64: dts: qcom: Add MXC power domain to videocc node on SM8550

[Jagadeesh Kona]

Videocc requires both MMCX and MXC rails to be powered ON to configure
the video PLLs on SM8550 platform. Hence add MXC power domain to videocc
node on SM8550.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index eac8de4005d82f246bc50f64f09515631d895c99..a039ae71e1b7bba8124128d19de5e00c65217770 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -2889,7 +2889,8 @@ videocc: clock-controller@aaf0000 {
 			reg = <0 0x0aaf0000 0 0x10000>;
 			clocks = <&bi_tcxo_div2>,
 				 <&gcc GCC_VIDEO_AHB_CLK>;
-			power-domains = <&rpmhpd RPMHPD_MMCX>;
+			power-domains = <&rpmhpd RPMHPD_MMCX>,
+					<&rpmhpd RPMHPD_MXC>;
 			required-opps = <&rpmhpd_opp_low_svs>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;

-- 
2.34.1

[PATCH v2 8/8] arm64: dts: qcom: Add MXC power domain to videocc node on SM8650

[Jagadeesh Kona]

Videocc requires both MMCX and MXC rails to be powered ON to configure
the video PLLs on SM8650 platform. Hence add MXC power domain to videocc
node on SM8650.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 86684cb9a9325618ddb74458621cf4bbdc1cc0d1..32af2a0f7a0030f155b7d8c93faeffa384a42768 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -3524,7 +3524,8 @@ videocc: clock-controller@aaf0000 {
 			reg = <0 0x0aaf0000 0 0x10000>;
 			clocks = <&bi_tcxo_div2>,
 				 <&gcc GCC_VIDEO_AHB_CLK>;
-			power-domains = <&rpmhpd RPMHPD_MMCX>;
+			power-domains = <&rpmhpd RPMHPD_MMCX>,
+					<&rpmhpd RPMHPD_MXC>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;

-- 
2.34.1

Re: [PATCH v2 0/8] clk: qcom: Add support to attach multiple power domains in cc probe

[Bryan O'Donoghue]

On 06/03/2025 08:55, Jagadeesh Kona wrote:
> In some of the recent chipsets, PLLs require more than one power domain
> to be kept ON to configure the PLL. But the current code doesn't enable
> all the required power domains while configuring the PLLs, this leads
> to functional issues due to suboptimal settings of PLLs.
> 
> To address this, add support for handling runtime power management,
> configuring plls and enabling critical clocks from qcom_cc_really_probe.
> The clock controller can specify PLLs, critical clocks, and runtime PM
> requirements in the descriptor data. The code in qcom_cc_really_probe()
> ensures all necessary power domains are enabled before configuring PLLs
> or critical clocks.
> 
> This series updates SM8450 & SM8550 videocc drivers to handle rpm,
> configure PLLs and enable critical clocks from within qcom_cc_really_probe()
> using above support, so video PLLs are configured properly.
> 
> This series fixes the below warning reported in SM8550 venus testing due
> to video_cc_pll0 not properly getting configured during videocc probe
> 
> [   46.535132] Lucid PLL latch failed. Output may be unstable!
> 
> The patch adding support to configure the PLLs from common code is
> picked from below series and updated it.
> https://lore.kernel.org/all/20250113-support-pll-reconfigure-v1-0-1fae6bc1062d@quicinc.com/
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
> Changes in v2:
>   - Added support to handle rpm, PLL configuration and enable critical
>     clocks from qcom_cc_really_probe() in common code as per v1 commments
>     from Bryan, Konrad and Dmitry
>   - Added patches to configure PLLs from common code
>   - Updated the SM8450, SM8550 videocc patches to use the newly
>     added support to handle rpm, configure PLLs from common code
>   - Split the DT change for each target separately as per
>     Dmitry comments
>   - Added R-By and A-By tags received on v1
> - Link to v1: https://lore.kernel.org/r/20250218-videocc-pll-multi-pd-voting-v1-0-cfe6289ea29b@quicinc.com
> 
> ---
> Jagadeesh Kona (7):
>        dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain
>        clk: qcom: common: Manage rpm, configure PLLs & AON clks in really probe
>        clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe
>        clk: qcom: videocc-sm8550: Move PLL & clk configuration to really probe
>        arm64: dts: qcom: Add MXC power domain to videocc node on SM8450
>        arm64: dts: qcom: Add MXC power domain to videocc node on SM8550
>        arm64: dts: qcom: Add MXC power domain to videocc node on SM8650
> 
This list looks sparse.

- camcc is missing
- x1e is missing
- sm8650 and sm8750 and both also missing

If we are going in with the knife, lets cut once and make a 
comprehensive change.

Could you please add those platforms and clock controllers to your V3 to 
save other people having to do the extra work.

---
bod

Re: [PATCH v2 0/8] clk: qcom: Add support to attach multiple power domains in cc probe

[Vladimir Zapolskiy]

On 3/11/25 11:52, Bryan O'Donoghue wrote:
> On 06/03/2025 08:55, Jagadeesh Kona wrote:
>> In some of the recent chipsets, PLLs require more than one power domain
>> to be kept ON to configure the PLL. But the current code doesn't enable
>> all the required power domains while configuring the PLLs, this leads
>> to functional issues due to suboptimal settings of PLLs.
>>
>> To address this, add support for handling runtime power management,
>> configuring plls and enabling critical clocks from qcom_cc_really_probe.
>> The clock controller can specify PLLs, critical clocks, and runtime PM
>> requirements in the descriptor data. The code in qcom_cc_really_probe()
>> ensures all necessary power domains are enabled before configuring PLLs
>> or critical clocks.
>>
>> This series updates SM8450 & SM8550 videocc drivers to handle rpm,
>> configure PLLs and enable critical clocks from within qcom_cc_really_probe()
>> using above support, so video PLLs are configured properly.
>>
>> This series fixes the below warning reported in SM8550 venus testing due
>> to video_cc_pll0 not properly getting configured during videocc probe
>>
>> [   46.535132] Lucid PLL latch failed. Output may be unstable!
>>
>> The patch adding support to configure the PLLs from common code is
>> picked from below series and updated it.
>> https://lore.kernel.org/all/20250113-support-pll-reconfigure-v1-0-1fae6bc1062d@quicinc.com/
>>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
>> Changes in v2:
>>    - Added support to handle rpm, PLL configuration and enable critical
>>      clocks from qcom_cc_really_probe() in common code as per v1 commments
>>      from Bryan, Konrad and Dmitry
>>    - Added patches to configure PLLs from common code
>>    - Updated the SM8450, SM8550 videocc patches to use the newly
>>      added support to handle rpm, configure PLLs from common code
>>    - Split the DT change for each target separately as per
>>      Dmitry comments
>>    - Added R-By and A-By tags received on v1
>> - Link to v1: https://lore.kernel.org/r/20250218-videocc-pll-multi-pd-voting-v1-0-cfe6289ea29b@quicinc.com
>>
>> ---
>> Jagadeesh Kona (7):
>>         dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain
>>         clk: qcom: common: Manage rpm, configure PLLs & AON clks in really probe
>>         clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe
>>         clk: qcom: videocc-sm8550: Move PLL & clk configuration to really probe
>>         arm64: dts: qcom: Add MXC power domain to videocc node on SM8450
>>         arm64: dts: qcom: Add MXC power domain to videocc node on SM8550
>>         arm64: dts: qcom: Add MXC power domain to videocc node on SM8650
>>
> This list looks sparse.
> 
> - camcc is missing
> - x1e is missing
> - sm8650 and sm8750 and both also missing
> 

Since there are concerns about DT bindings ABI change of CAMCC given by
Krzysztof, likely CAMCC changes shall not be inserted into this series.

--
Best wishes,
Vladimir

Re: [PATCH v2 0/8] clk: qcom: Add support to attach multiple power domains in cc probe

[Bryan O'Donoghue]

On 11/03/2025 10:12, Vladimir Zapolskiy wrote:
> On 3/11/25 11:52, Bryan O'Donoghue wrote:
>> On 06/03/2025 08:55, Jagadeesh Kona wrote:
>>> In some of the recent chipsets, PLLs require more than one power domain
>>> to be kept ON to configure the PLL. But the current code doesn't enable
>>> all the required power domains while configuring the PLLs, this leads
>>> to functional issues due to suboptimal settings of PLLs.
>>>
>>> To address this, add support for handling runtime power management,
>>> configuring plls and enabling critical clocks from qcom_cc_really_probe.
>>> The clock controller can specify PLLs, critical clocks, and runtime PM
>>> requirements in the descriptor data. The code in qcom_cc_really_probe()
>>> ensures all necessary power domains are enabled before configuring PLLs
>>> or critical clocks.
>>>
>>> This series updates SM8450 & SM8550 videocc drivers to handle rpm,
>>> configure PLLs and enable critical clocks from within 
>>> qcom_cc_really_probe()
>>> using above support, so video PLLs are configured properly.
>>>
>>> This series fixes the below warning reported in SM8550 venus testing due
>>> to video_cc_pll0 not properly getting configured during videocc probe
>>>
>>> [   46.535132] Lucid PLL latch failed. Output may be unstable!
>>>
>>> The patch adding support to configure the PLLs from common code is
>>> picked from below series and updated it.
>>> https://lore.kernel.org/all/20250113-support-pll-reconfigure- 
>>> v1-0-1fae6bc1062d@quicinc.com/
>>>
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>> ---
>>> Changes in v2:
>>>    - Added support to handle rpm, PLL configuration and enable critical
>>>      clocks from qcom_cc_really_probe() in common code as per v1 
>>> commments
>>>      from Bryan, Konrad and Dmitry
>>>    - Added patches to configure PLLs from common code
>>>    - Updated the SM8450, SM8550 videocc patches to use the newly
>>>      added support to handle rpm, configure PLLs from common code
>>>    - Split the DT change for each target separately as per
>>>      Dmitry comments
>>>    - Added R-By and A-By tags received on v1
>>> - Link to v1: https://lore.kernel.org/r/20250218-videocc-pll-multi- 
>>> pd-voting-v1-0-cfe6289ea29b@quicinc.com
>>>
>>> ---
>>> Jagadeesh Kona (7):
>>>         dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain
>>>         clk: qcom: common: Manage rpm, configure PLLs & AON clks in 
>>> really probe
>>>         clk: qcom: videocc-sm8450: Move PLL & clk configuration to 
>>> really probe
>>>         clk: qcom: videocc-sm8550: Move PLL & clk configuration to 
>>> really probe
>>>         arm64: dts: qcom: Add MXC power domain to videocc node on SM8450
>>>         arm64: dts: qcom: Add MXC power domain to videocc node on SM8550
>>>         arm64: dts: qcom: Add MXC power domain to videocc node on SM8650
>>>
>> This list looks sparse.
>>
>> - camcc is missing
>> - x1e is missing
>> - sm8650 and sm8750 and both also missing
>>
> 
> Since there are concerns about DT bindings ABI change of CAMCC given by
> Krzysztof, likely CAMCC changes shall not be inserted into this series.
> 
> -- 
> Best wishes,
> Vladimir

drivers/clk/qcom/camcc-sm8650.c
drivers/clk/qcom/camcc-x1e80100.c

In fact we appear to be amending the dts but not the driver for the 8650 
here.

@Jagadeesh please follow up.

---
bod

Re: [PATCH v2 0/8] clk: qcom: Add support to attach multiple power domains in cc probe

[Vladimir Zapolskiy]

On 3/11/25 14:10, Bryan O'Donoghue wrote:
> On 11/03/2025 10:12, Vladimir Zapolskiy wrote:
>> On 3/11/25 11:52, Bryan O'Donoghue wrote:
>>> On 06/03/2025 08:55, Jagadeesh Kona wrote:
>>>> In some of the recent chipsets, PLLs require more than one power domain
>>>> to be kept ON to configure the PLL. But the current code doesn't enable
>>>> all the required power domains while configuring the PLLs, this leads
>>>> to functional issues due to suboptimal settings of PLLs.
>>>>
>>>> To address this, add support for handling runtime power management,
>>>> configuring plls and enabling critical clocks from qcom_cc_really_probe.
>>>> The clock controller can specify PLLs, critical clocks, and runtime PM
>>>> requirements in the descriptor data. The code in qcom_cc_really_probe()
>>>> ensures all necessary power domains are enabled before configuring PLLs
>>>> or critical clocks.
>>>>
>>>> This series updates SM8450 & SM8550 videocc drivers to handle rpm,
>>>> configure PLLs and enable critical clocks from within
>>>> qcom_cc_really_probe()
>>>> using above support, so video PLLs are configured properly.
>>>>
>>>> This series fixes the below warning reported in SM8550 venus testing due
>>>> to video_cc_pll0 not properly getting configured during videocc probe
>>>>
>>>> [   46.535132] Lucid PLL latch failed. Output may be unstable!
>>>>
>>>> The patch adding support to configure the PLLs from common code is
>>>> picked from below series and updated it.
>>>> https://lore.kernel.org/all/20250113-support-pll-reconfigure-
>>>> v1-0-1fae6bc1062d@quicinc.com/
>>>>
>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>> ---
>>>> Changes in v2:
>>>>     - Added support to handle rpm, PLL configuration and enable critical
>>>>       clocks from qcom_cc_really_probe() in common code as per v1
>>>> commments
>>>>       from Bryan, Konrad and Dmitry
>>>>     - Added patches to configure PLLs from common code
>>>>     - Updated the SM8450, SM8550 videocc patches to use the newly
>>>>       added support to handle rpm, configure PLLs from common code
>>>>     - Split the DT change for each target separately as per
>>>>       Dmitry comments
>>>>     - Added R-By and A-By tags received on v1
>>>> - Link to v1: https://lore.kernel.org/r/20250218-videocc-pll-multi-
>>>> pd-voting-v1-0-cfe6289ea29b@quicinc.com
>>>>
>>>> ---
>>>> Jagadeesh Kona (7):
>>>>          dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain
>>>>          clk: qcom: common: Manage rpm, configure PLLs & AON clks in
>>>> really probe
>>>>          clk: qcom: videocc-sm8450: Move PLL & clk configuration to
>>>> really probe
>>>>          clk: qcom: videocc-sm8550: Move PLL & clk configuration to
>>>> really probe
>>>>          arm64: dts: qcom: Add MXC power domain to videocc node on SM8450
>>>>          arm64: dts: qcom: Add MXC power domain to videocc node on SM8550
>>>>          arm64: dts: qcom: Add MXC power domain to videocc node on SM8650
>>>>
>>> This list looks sparse.
>>>
>>> - camcc is missing
>>> - x1e is missing
>>> - sm8650 and sm8750 and both also missing
>>>
>>
>> Since there are concerns about DT bindings ABI change of CAMCC given by
>> Krzysztof, likely CAMCC changes shall not be inserted into this series.
>>
>> -- 
>> Best wishes,
>> Vladimir
> 
> drivers/clk/qcom/camcc-sm8650.c
> drivers/clk/qcom/camcc-x1e80100.c
> 
> In fact we appear to be amending the dts but not the driver for the 8650
> here.

I kindly ask to elaborate here.

This series does not touch CAMCC at all, and if the series touches CAMCC,
then it changes DT ABI, which is objected. Or is it for some reason
objected only for SM8550 and not for the other platforms? More information
is needed.

--
Best wishes,
Vladimir

Re: [PATCH v2 0/8] clk: qcom: Add support to attach multiple power domains in cc probe

[Bryan O'Donoghue]

On 11/03/2025 17:55, Vladimir Zapolskiy wrote:
> 
> I kindly ask to elaborate here.
> 
> This series does not touch CAMCC at all, and if the series touches CAMCC,
> then it changes DT ABI, which is objected. Or is it for some reason
> objected only for SM8550 and not for the other platforms? More information
> is needed.

No but it _should_ Vlad, that's the ask.

Both of these clock controllers will require this same change to be 
implemented, that's what I'm asking Jagadeesh to do.

Certainly that's the case for x1e and asking Jagadeesh to also check 
that for sm8650.

---
bod

Re: [PATCH v2 0/8] clk: qcom: Add support to attach multiple power domains in cc probe

[Jagadeesh Kona]

On 3/11/2025 11:41 PM, Bryan O'Donoghue wrote:
> On 11/03/2025 17:55, Vladimir Zapolskiy wrote:
>>
>> I kindly ask to elaborate here.
>>
>> This series does not touch CAMCC at all, and if the series touches CAMCC,
>> then it changes DT ABI, which is objected. Or is it for some reason
>> objected only for SM8550 and not for the other platforms? More information
>> is needed.
> 
> No but it _should_ Vlad, that's the ask.
> 
> Both of these clock controllers will require this same change to be implemented, that's what I'm asking Jagadeesh to do.
> 
> Certainly that's the case for x1e and asking Jagadeesh to also check that for sm8650.
> 

Yes, similar changes are required for camcc on SM8450, SM8550, SM8650 and X1E80100. I will add them in the v3 series.
For X1E80100 camcc, I see changes are already raised in dt-bindings[1] and DT[2] to add multi PD support, so I will just
include the camcc driver change in v3 for X1E80100.

[1]: https://lore.kernel.org/all/20250304143152.1799966-1-vladimir.zapolskiy@linaro.org/
[2]: https://lore.kernel.org/all/20250119-b4-linux-next-24-11-18-dtsi-x1e80100-camss-v4-2-c2964504131c@linaro.org/

Thanks,
Jagadeesh

> ---
> bod

Re: [PATCH v2 0/8] clk: qcom: Add support to attach multiple power domains in cc probe

[Jagadeesh Kona]

On 3/11/2025 5:40 PM, Bryan O'Donoghue wrote:
> On 11/03/2025 10:12, Vladimir Zapolskiy wrote:
>> On 3/11/25 11:52, Bryan O'Donoghue wrote:
>>> On 06/03/2025 08:55, Jagadeesh Kona wrote:
>>>> In some of the recent chipsets, PLLs require more than one power domain
>>>> to be kept ON to configure the PLL. But the current code doesn't enable
>>>> all the required power domains while configuring the PLLs, this leads
>>>> to functional issues due to suboptimal settings of PLLs.
>>>>
>>>> To address this, add support for handling runtime power management,
>>>> configuring plls and enabling critical clocks from qcom_cc_really_probe.
>>>> The clock controller can specify PLLs, critical clocks, and runtime PM
>>>> requirements in the descriptor data. The code in qcom_cc_really_probe()
>>>> ensures all necessary power domains are enabled before configuring PLLs
>>>> or critical clocks.
>>>>
>>>> This series updates SM8450 & SM8550 videocc drivers to handle rpm,
>>>> configure PLLs and enable critical clocks from within qcom_cc_really_probe()
>>>> using above support, so video PLLs are configured properly.
>>>>
>>>> This series fixes the below warning reported in SM8550 venus testing due
>>>> to video_cc_pll0 not properly getting configured during videocc probe
>>>>
>>>> [   46.535132] Lucid PLL latch failed. Output may be unstable!
>>>>
>>>> The patch adding support to configure the PLLs from common code is
>>>> picked from below series and updated it.
>>>> https://lore.kernel.org/all/20250113-support-pll-reconfigure- v1-0-1fae6bc1062d@quicinc.com/
>>>>
>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>> ---
>>>> Changes in v2:
>>>>    - Added support to handle rpm, PLL configuration and enable critical
>>>>      clocks from qcom_cc_really_probe() in common code as per v1 commments
>>>>      from Bryan, Konrad and Dmitry
>>>>    - Added patches to configure PLLs from common code
>>>>    - Updated the SM8450, SM8550 videocc patches to use the newly
>>>>      added support to handle rpm, configure PLLs from common code
>>>>    - Split the DT change for each target separately as per
>>>>      Dmitry comments
>>>>    - Added R-By and A-By tags received on v1
>>>> - Link to v1: https://lore.kernel.org/r/20250218-videocc-pll-multi- pd-voting-v1-0-cfe6289ea29b@quicinc.com
>>>>
>>>> ---
>>>> Jagadeesh Kona (7):
>>>>         dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain
>>>>         clk: qcom: common: Manage rpm, configure PLLs & AON clks in really probe
>>>>         clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe
>>>>         clk: qcom: videocc-sm8550: Move PLL & clk configuration to really probe
>>>>         arm64: dts: qcom: Add MXC power domain to videocc node on SM8450
>>>>         arm64: dts: qcom: Add MXC power domain to videocc node on SM8550
>>>>         arm64: dts: qcom: Add MXC power domain to videocc node on SM8650
>>>>
>>> This list looks sparse.
>>>
>>> - camcc is missing
>>> - x1e is missing
>>> - sm8650 and sm8750 and both also missing
>>>
>>
>> Since there are concerns about DT bindings ABI change of CAMCC given by
>> Krzysztof, likely CAMCC changes shall not be inserted into this series.
>>
>> -- 
>> Best wishes,
>> Vladimir
> 
> drivers/clk/qcom/camcc-sm8650.c
> drivers/clk/qcom/camcc-x1e80100.c
> 
> In fact we appear to be amending the dts but not the driver for the 8650 here.
> 
> @Jagadeesh please follow up.
> 

SM8650 videocc is just reusing the SM8550 videocc driver, so no separate changes
are required for SM8650. Will add support for above camcc drivers in next series.

Thanks,
Jagadeesh
 
> ---
> bod