[PATCH v3 00/18] clk: qcom: Add support to attach multiple power domains in cc probe

[PATCH v3 00/18] clk: qcom: Add support to attach multiple power domains in cc probe

[Jagadeesh Kona]

In recent QCOM 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 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/

This series is dependent on bindings patch in below Vladimir's series, hence
included the Vladimir's series patches also in this series and updated them.
https://lore.kernel.org/all/20250303225521.1780611-1-vladimir.zapolskiy@linaro.org/

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
Changes in v3:
 - Updated the videocc bindings patch to add required-opps for MXC power domain [Dmitry]
   and added Bryan & Rob R/A-By tags received for this patch on v1.
 - Included the Vladimir's bindings patch for SM8450 camcc bindings to
   add multiple PD support and updated them to fix the bot warnings.
 - Moved SC8280XP camcc bindings to SA8775P camcc since SC8280XP only
   require single MMCX power domain
 - Split runtime PM and PLL configuration to separate patches [Dmitry]
 - Removed direct regmap_update_bits to configure clock CBCR's and
   using clock helpers to configure the CBCR registers [Dmitry, Bryan]
 - Added new helpers to configure all PLLs & update misc clock
   register settings from common code [Dmitry, Bryan]
 - Updated the name of qcom_clk_cfg structure to qcom_clk_reg_setting [Konrad]
 - Updated the fields in structure from unsigned int to u32 and added
   val field to this structure [Konrad]
 - Added a new u32 array for cbcr branch clocks & num_clk_cbcrs fields
   to maintain the list of critical clock cbcrs in clock controller
   descriptor [Konrad]
 - Updated the plls field to alpha_plls in descriptor structure [Konrad]
 - Added WARN() in PLL configure function if PLL type passed is not
   supported. The suggestion is to use BUG(), but updated it to
   WARN() to avoid checkpatch warning. [Bjorn]
 - Moved the pll configure and helper macros to PLL code from common code [Bjorn]
 - Updated camcc drivers for SM8450, SM8550, SM8650 and X1E80100 targets
   with support to configure PLLs from common code and added MXC power
   domain in corresponding camcc DT nodes. [Bryan]
 - Added Dmitry and Bryan R-By tags received on videocc DT node changes in v1
 - Link to v2: https://lore.kernel.org/r/20250306-videocc-pll-multi-pd-voting-v2-0-0cd00612bc0e@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 (15):
      dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain
      dt-bindings: clock: qcom: Update sc8280xp camcc bindings
      clk: qcom: common: Handle runtime power management in qcom_cc_really_probe
      clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe
      clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe
      clk: qcom: videocc-sm8550: Move PLL & clk configuration to really probe
      clk: qcom: camcc-sm8450: Move PLL & clk configuration to really probe
      clk: qcom: camcc-sm8550: Move PLL & clk configuration to really probe
      clk: qcom: camcc-sm8650: Move PLL & clk configuration to really probe
      clk: qcom: camcc-x1e80100: 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
      arm64: dts: qcom: Add MXC power domain to camcc node on SM8450
      arm64: dts: qcom: Add MXC power domain to camcc node on SM8650

Taniya Das (1):
      clk: qcom: clk-alpha-pll: Add support for common PLL configuration function

Vladimir Zapolskiy (2):
      dt-bindings: clock: qcom: sm8450-camcc: Allow to specify two power domains
      arm64: dts: qcom: sm8550: Additionally manage MXC power domain in camcc

 .../bindings/clock/qcom,sa8775p-camcc.yaml         |  2 +
 .../bindings/clock/qcom,sm8450-camcc.yaml          | 20 +++--
 .../bindings/clock/qcom,sm8450-videocc.yaml        | 18 +++--
 arch/arm64/boot/dts/qcom/sm8450.dtsi               | 12 ++-
 arch/arm64/boot/dts/qcom/sm8550.dtsi               | 12 ++-
 arch/arm64/boot/dts/qcom/sm8650.dtsi               |  6 +-
 drivers/clk/qcom/camcc-sm8450.c                    | 85 ++++++++++------------
 drivers/clk/qcom/camcc-sm8550.c                    | 81 ++++++++++-----------
 drivers/clk/qcom/camcc-sm8650.c                    | 79 ++++++++++----------
 drivers/clk/qcom/camcc-x1e80100.c                  | 63 +++++++---------
 drivers/clk/qcom/clk-alpha-pll.c                   | 63 ++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h                   |  3 +
 drivers/clk/qcom/common.c                          | 65 ++++++++++++++---
 drivers/clk/qcom/common.h                          | 20 +++++
 drivers/clk/qcom/videocc-sm8450.c                  | 54 ++++++--------
 drivers/clk/qcom/videocc-sm8550.c                  | 55 ++++++--------
 16 files changed, 377 insertions(+), 261 deletions(-)
---
base-commit: 138cfc44b3c4a5fb800388c6e27be169970fb9f7
change-id: 20250218-videocc-pll-multi-pd-voting-d614dce910e7

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

[PATCH v3 01/18] 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>
---
 .../devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
index 62714fa54db82491a7a108f7f18a253d737f8d61..93807b1448025a4f2724378346a4bd87f08a8e57 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
@@ -32,14 +32,18 @@ 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
     description:
-      A phandle to an OPP node describing required MMCX performance point.
+      Phandles to OPP nodes that describe required performance point on power domains
+    items:
+      - description: MMCX performance point
+      - description: MXC performance point
 
 required:
   - compatible
@@ -72,8 +76,10 @@ examples:
       reg = <0x0aaf0000 0x10000>;
       clocks = <&rpmhcc RPMH_CXO_CLK>,
                <&gcc GCC_VIDEO_AHB_CLK>;
-      power-domains = <&rpmhpd RPMHPD_MMCX>;
-      required-opps = <&rpmhpd_opp_low_svs>;
+      power-domains = <&rpmhpd RPMHPD_MMCX>,
+                      <&rpmhpd RPMHPD_MXC>;
+      required-opps = <&rpmhpd_opp_low_svs>,
+                      <&rpmhpd_opp_low_svs>;
       #clock-cells = <1>;
       #reset-cells = <1>;
       #power-domain-cells = <1>;

-- 
2.34.1

Re: [PATCH v3 01/18] dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain

[Krzysztof Kozlowski]

On Thu, Mar 27, 2025 at 03:22:21PM +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

Either your patches are not ordered correctly or you forgot that
SC8280xp also gets MXC.

Best regards,
Krzysztof

Re: [PATCH v3 01/18] dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain

[Krzysztof Kozlowski]

On Fri, Mar 28, 2025 at 09:09:00AM +0100, Krzysztof Kozlowski wrote:
> On Thu, Mar 27, 2025 at 03:22:21PM +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
> 
> Either your patches are not ordered correctly or you forgot that
> SC8280xp also gets MXC.

Ah, no, that's videocc, I mixed up devices.

It's fine.

Best regards,
Krzysztof

[PATCH v3 02/18] dt-bindings: clock: qcom: Update sc8280xp camcc bindings

[Jagadeesh Kona]

Move SC8280XP camcc bindings from SM8450 to SA8775P camcc.
SC8280XP camcc only requires the MMCX power domain, unlike
SM8450 camcc which will now support both MMCX and MXC power
domains.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml | 2 ++
 Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml  | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
index 81623f59d11d73839e5c551411a52427e2f28415..127c369dd452608e5e7a52c7297b6b343d1c1bf8 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
@@ -17,12 +17,14 @@ description: |
   See also:
     include/dt-bindings/clock/qcom,qcs8300-camcc.h
     include/dt-bindings/clock/qcom,sa8775p-camcc.h
+    include/dt-bindings/clock/qcom,sc8280xp-camcc.h
 
 properties:
   compatible:
     enum:
       - qcom,qcs8300-camcc
       - qcom,sa8775p-camcc
+      - qcom,sc8280xp-camcc
 
   clocks:
     items:
diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
index 9e79f8fec437b9aecb5103092f6ff2ad1cd42626..883f12e3d11fa16384108434f6de120162226a28 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
@@ -15,7 +15,6 @@ description: |
   domains on SM8450.
 
   See also:
-    include/dt-bindings/clock/qcom,sc8280xp-camcc.h
     include/dt-bindings/clock/qcom,sm8450-camcc.h
     include/dt-bindings/clock/qcom,sm8550-camcc.h
     include/dt-bindings/clock/qcom,sm8650-camcc.h
@@ -23,7 +22,6 @@ description: |
 properties:
   compatible:
     enum:
-      - qcom,sc8280xp-camcc
       - qcom,sm8450-camcc
       - qcom,sm8475-camcc
       - qcom,sm8550-camcc

-- 
2.34.1

Re: [PATCH v3 02/18] dt-bindings: clock: qcom: Update sc8280xp camcc bindings

[Bryan O'Donoghue]

On 27/03/2025 09:52, Jagadeesh Kona wrote:
> Move SC8280XP camcc bindings from SM8450 to SA8775P camcc.
> SC8280XP camcc only requires the MMCX power domain, unlike
> SM8450 camcc which will now support both MMCX and MXC power
> domains.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>   Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml | 2 ++
>   Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml  | 2 --
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
> index 81623f59d11d73839e5c551411a52427e2f28415..127c369dd452608e5e7a52c7297b6b343d1c1bf8 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
> @@ -17,12 +17,14 @@ description: |
>     See also:
>       include/dt-bindings/clock/qcom,qcs8300-camcc.h
>       include/dt-bindings/clock/qcom,sa8775p-camcc.h
> +    include/dt-bindings/clock/qcom,sc8280xp-camcc.h
>   
>   properties:
>     compatible:
>       enum:
>         - qcom,qcs8300-camcc
>         - qcom,sa8775p-camcc
> +      - qcom,sc8280xp-camcc
>   
>     clocks:
>       items:
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> index 9e79f8fec437b9aecb5103092f6ff2ad1cd42626..883f12e3d11fa16384108434f6de120162226a28 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> @@ -15,7 +15,6 @@ description: |
>     domains on SM8450.
>   
>     See also:
> -    include/dt-bindings/clock/qcom,sc8280xp-camcc.h
>       include/dt-bindings/clock/qcom,sm8450-camcc.h
>       include/dt-bindings/clock/qcom,sm8550-camcc.h
>       include/dt-bindings/clock/qcom,sm8650-camcc.h
> @@ -23,7 +22,6 @@ description: |
>   properties:
>     compatible:
>       enum:
> -      - qcom,sc8280xp-camcc
>         - qcom,sm8450-camcc
>         - qcom,sm8475-camcc
>         - qcom,sm8550-camcc
> 
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH v3 02/18] dt-bindings: clock: qcom: Update sc8280xp camcc bindings

[Krzysztof Kozlowski]

On Thu, Mar 27, 2025 at 03:22:22PM +0530, Jagadeesh Kona wrote:
> Move SC8280XP camcc bindings from SM8450 to SA8775P camcc.
> SC8280XP camcc only requires the MMCX power domain, unlike
> SM8450 camcc which will now support both MMCX and MXC power
> domains.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>  Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml | 2 ++
>  Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml  | 2 --
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
> index 81623f59d11d73839e5c551411a52427e2f28415..127c369dd452608e5e7a52c7297b6b343d1c1bf8 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
> @@ -17,12 +17,14 @@ description: |
>    See also:
>      include/dt-bindings/clock/qcom,qcs8300-camcc.h
>      include/dt-bindings/clock/qcom,sa8775p-camcc.h
> +    include/dt-bindings/clock/qcom,sc8280xp-camcc.h
>  
>  properties:
>    compatible:
>      enum:
>        - qcom,qcs8300-camcc
>        - qcom,sa8775p-camcc
> +      - qcom,sc8280xp-camcc

That's not equivalent. You miss required-opps.

Best regards,
Krzysztof

Re: [PATCH v3 02/18] dt-bindings: clock: qcom: Update sc8280xp camcc bindings

[Jagadeesh Kona]

On 3/28/2025 1:37 PM, Krzysztof Kozlowski wrote:
> On Thu, Mar 27, 2025 at 03:22:22PM +0530, Jagadeesh Kona wrote:
>> Move SC8280XP camcc bindings from SM8450 to SA8775P camcc.
>> SC8280XP camcc only requires the MMCX power domain, unlike
>> SM8450 camcc which will now support both MMCX and MXC power
>> domains.
>>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
>>  Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml | 2 ++
>>  Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml  | 2 --
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
>> index 81623f59d11d73839e5c551411a52427e2f28415..127c369dd452608e5e7a52c7297b6b343d1c1bf8 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
>> @@ -17,12 +17,14 @@ description: |
>>    See also:
>>      include/dt-bindings/clock/qcom,qcs8300-camcc.h
>>      include/dt-bindings/clock/qcom,sa8775p-camcc.h
>> +    include/dt-bindings/clock/qcom,sc8280xp-camcc.h
>>  
>>  properties:
>>    compatible:
>>      enum:
>>        - qcom,qcs8300-camcc
>>        - qcom,sa8775p-camcc
>> +      - qcom,sc8280xp-camcc
> 
> That's not equivalent. You miss required-opps.
> 

Yes, I will check and fix it in the next series.

Thanks,
Jagadeesh

> Best regards,
> Krzysztof
> 

[PATCH v3 03/18] dt-bindings: clock: qcom: sm8450-camcc: Allow to specify two power domains

[Jagadeesh Kona]

From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

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

Fixes: 9cbc64745fc6 ("dt-bindings: clock: qcom: Add SM8550 camera clock controller")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 .../devicetree/bindings/clock/qcom,sm8450-camcc.yaml   | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
index 883f12e3d11fa16384108434f6de120162226a28..927258cad89d93c7f6ce60c3fda53d094081c063 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
@@ -35,14 +35,18 @@ properties:
       - description: Sleep clock source
 
   power-domains:
-    maxItems: 1
     description:
-      A phandle and PM domain specifier for the 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
     description:
-      A phandle to an OPP node describing required MMCX performance point.
+      Phandles to OPP nodes that describe required performance point on power domains
+    items:
+      - description: MMCX performance point
+      - description: MXC performance point
 
   reg:
     maxItems: 1
@@ -80,8 +84,10 @@ examples:
                <&rpmhcc RPMH_CXO_CLK>,
                <&rpmhcc RPMH_CXO_CLK_A>,
                <&sleep_clk>;
-      power-domains = <&rpmhpd RPMHPD_MMCX>;
-      required-opps = <&rpmhpd_opp_low_svs>;
+      power-domains = <&rpmhpd RPMHPD_MMCX>,
+                      <&rpmhpd RPMHPD_MXC>;
+      required-opps = <&rpmhpd_opp_low_svs>,
+                      <&rpmhpd_opp_low_svs>;
       #clock-cells = <1>;
       #reset-cells = <1>;
       #power-domain-cells = <1>;

-- 
2.34.1

Re: [PATCH v3 03/18] dt-bindings: clock: qcom: sm8450-camcc: Allow to specify two power domains

[Bryan O'Donoghue]

On 27/03/2025 09:52, Jagadeesh Kona wrote:
> -      A phandle to an OPP node describing required MMCX performance point.
> +      Phandles to OPP nodes that describe required performance point on power domains

I believe we are dropping "Phandle to" generally as this is a redundant 
statement.

You should also pluralise performance-points.

.. required performance-points on power-domains

Other than that

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

---
bod

Re: [PATCH v3 03/18] dt-bindings: clock: qcom: sm8450-camcc: Allow to specify two power domains

[Jagadeesh Kona]

On 3/27/2025 8:58 PM, Bryan O'Donoghue wrote:
> On 27/03/2025 09:52, Jagadeesh Kona wrote:
>> -      A phandle to an OPP node describing required MMCX performance point.
>> +      Phandles to OPP nodes that describe required performance point on power domains
> 
> I believe we are dropping "Phandle to" generally as this is a redundant statement.
> 
> You should also pluralise performance-points.
> 
> .. required performance-points on power-domains
> 
> Other than that
> 
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> 

Yes, I will fix above in the next series.

Thanks,
Jagadeesh

> ---
> bod

Re: [PATCH v3 03/18] dt-bindings: clock: qcom: sm8450-camcc: Allow to specify two power domains

[Krzysztof Kozlowski]

On Thu, Mar 27, 2025 at 03:22:23PM +0530, Jagadeesh Kona wrote:
> From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> 
> To configure the camera PLLs and enable the camera GDSCs on SM8450, SM8475,
> SM8550 and SM8650 platforms, the MXC rail must be ON along with MMCX.
> Therefore, update the camcc bindings to include the MXC power domain on
> these platforms.
> 
> Fixes: 9cbc64745fc6 ("dt-bindings: clock: qcom: Add SM8550 camera clock controller")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>  .../devicetree/bindings/clock/qcom,sm8450-camcc.yaml   | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v3 04/18] clk: qcom: clk-alpha-pll: Add support for common PLL configuration function

[Jagadeesh Kona]

From: Taniya Das <quic_tdas@quicinc.com>

To properly configure the PLLs on recent chipsets, it often requires more
than one power domain to be kept ON. The support to enable multiple power
domains is being added in qcom_cc_really_probe() and PLLs should be
configured post all the required power domains are enabled.

Hence integrate PLL configuration into clk_alpha_pll structure and add
support for qcom_clk_alpha_pll_configure() function which can be called
from qcom_cc_really_probe() to configure the clock controller PLLs after
all required power domains are enabled.

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

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index cec0afea8e446010f0d4140d4ef63121706dde47..8ee842254e6690e24469053cdbd99a9953987e40 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -63,6 +63,8 @@
 #define PLL_OPMODE(p)		((p)->offset + (p)->regs[PLL_OFF_OPMODE])
 #define PLL_FRAC(p)		((p)->offset + (p)->regs[PLL_OFF_FRAC])
 
+#define GET_PLL_TYPE(pll)	(((pll)->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)
+
 const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = {
 	[CLK_ALPHA_PLL_TYPE_DEFAULT] =  {
 		[PLL_OFF_L_VAL] = 0x04,
@@ -2960,3 +2962,64 @@ const struct clk_ops clk_alpha_pll_regera_ops = {
 	.set_rate = clk_zonda_pll_set_rate,
 };
 EXPORT_SYMBOL_GPL(clk_alpha_pll_regera_ops);
+
+void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
+{
+	const struct clk_init_data *init = pll->clkr.hw.init;
+	const char *name = init->name;
+
+	if (!pll->config || !pll->regs) {
+		pr_err("%s: missing pll config or regs\n", name);
+		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:
+		WARN(1, "%s: invalid pll type\n", name);
+		break;
+	}
+}
+EXPORT_SYMBOL_GPL(qcom_clk_alpha_pll_configure);
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index 79aca8525262211ae5295245427d4540abf1e09a..7f35aaa7a35d88411beb11fd2be5d5dd5bfbe066 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)
@@ -237,5 +239,6 @@ void clk_stromer_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 			       const struct alpha_pll_config *config);
 void clk_regera_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 			     const struct alpha_pll_config *config);
+void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap);
 
 #endif

-- 
2.34.1

Re: [PATCH v3 04/18] clk: qcom: clk-alpha-pll: Add support for common PLL configuration function

[Bryan O'Donoghue]

On 27/03/2025 09:52, Jagadeesh Kona wrote:
> From: Taniya Das <quic_tdas@quicinc.com>
> 
> To properly configure the PLLs on recent chipsets, it often requires more
> than one power domain to be kept ON. The support to enable multiple power
> domains is being added in qcom_cc_really_probe() and PLLs should be
> configured post all the required power domains are enabled.
> 
> Hence integrate PLL configuration into clk_alpha_pll structure and add
> support for qcom_clk_alpha_pll_configure() function which can be called
> from qcom_cc_really_probe() to configure the clock controller PLLs after
> all required power domains are enabled.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>   drivers/clk/qcom/clk-alpha-pll.c | 63 ++++++++++++++++++++++++++++++++++++++++
>   drivers/clk/qcom/clk-alpha-pll.h |  3 ++
>   2 files changed, 66 insertions(+)
> 
> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
> index cec0afea8e446010f0d4140d4ef63121706dde47..8ee842254e6690e24469053cdbd99a9953987e40 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.c
> +++ b/drivers/clk/qcom/clk-alpha-pll.c
> @@ -63,6 +63,8 @@
>   #define PLL_OPMODE(p)		((p)->offset + (p)->regs[PLL_OFF_OPMODE])
>   #define PLL_FRAC(p)		((p)->offset + (p)->regs[PLL_OFF_FRAC])
>   
> +#define GET_PLL_TYPE(pll)	(((pll)->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)
> +
>   const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = {
>   	[CLK_ALPHA_PLL_TYPE_DEFAULT] =  {
>   		[PLL_OFF_L_VAL] = 0x04,
> @@ -2960,3 +2962,64 @@ const struct clk_ops clk_alpha_pll_regera_ops = {
>   	.set_rate = clk_zonda_pll_set_rate,
>   };
>   EXPORT_SYMBOL_GPL(clk_alpha_pll_regera_ops);
> +
> +void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
> +{
> +	const struct clk_init_data *init = pll->clkr.hw.init;
> +	const char *name = init->name;
> +
> +	if (!pll->config || !pll->regs) {
> +		pr_err("%s: missing pll config or regs\n", name);
> +		return;
> +	}

Seems like a strange check - you are calling this function in a loop 
which looks like

for (i = 0; i < desc->num_alpha_plls; i++)
	qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);

Can num_alpha_plls be true but alpha_plls be NULL and why is regmap 
considered valid ?

I think you can drop this check.

> +
> +	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:
> +		WARN(1, "%s: invalid pll type\n", name);
> +		break;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(qcom_clk_alpha_pll_configure);
> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
> index 79aca8525262211ae5295245427d4540abf1e09a..7f35aaa7a35d88411beb11fd2be5d5dd5bfbe066 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)
> @@ -237,5 +239,6 @@ void clk_stromer_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
>   			       const struct alpha_pll_config *config);
>   void clk_regera_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
>   			     const struct alpha_pll_config *config);
> +void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap);
>   
>   #endif
> 

Re: [PATCH v3 04/18] clk: qcom: clk-alpha-pll: Add support for common PLL configuration function

[Dmitry Baryshkov]

On Thu, Mar 27, 2025 at 03:51:33PM +0000, Bryan O'Donoghue wrote:
> On 27/03/2025 09:52, Jagadeesh Kona wrote:
> > From: Taniya Das <quic_tdas@quicinc.com>
> > 
> > To properly configure the PLLs on recent chipsets, it often requires more
> > than one power domain to be kept ON. The support to enable multiple power
> > domains is being added in qcom_cc_really_probe() and PLLs should be
> > configured post all the required power domains are enabled.
> > 
> > Hence integrate PLL configuration into clk_alpha_pll structure and add
> > support for qcom_clk_alpha_pll_configure() function which can be called
> > from qcom_cc_really_probe() to configure the clock controller PLLs after
> > all required power domains are enabled.
> > 
> > Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> > ---
> >   drivers/clk/qcom/clk-alpha-pll.c | 63 ++++++++++++++++++++++++++++++++++++++++
> >   drivers/clk/qcom/clk-alpha-pll.h |  3 ++
> >   2 files changed, 66 insertions(+)
> > 
> > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
> > index cec0afea8e446010f0d4140d4ef63121706dde47..8ee842254e6690e24469053cdbd99a9953987e40 100644
> > --- a/drivers/clk/qcom/clk-alpha-pll.c
> > +++ b/drivers/clk/qcom/clk-alpha-pll.c
> > @@ -63,6 +63,8 @@
> >   #define PLL_OPMODE(p)		((p)->offset + (p)->regs[PLL_OFF_OPMODE])
> >   #define PLL_FRAC(p)		((p)->offset + (p)->regs[PLL_OFF_FRAC])
> > +#define GET_PLL_TYPE(pll)	(((pll)->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)
> > +
> >   const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = {
> >   	[CLK_ALPHA_PLL_TYPE_DEFAULT] =  {
> >   		[PLL_OFF_L_VAL] = 0x04,
> > @@ -2960,3 +2962,64 @@ const struct clk_ops clk_alpha_pll_regera_ops = {
> >   	.set_rate = clk_zonda_pll_set_rate,
> >   };
> >   EXPORT_SYMBOL_GPL(clk_alpha_pll_regera_ops);
> > +
> > +void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
> > +{
> > +	const struct clk_init_data *init = pll->clkr.hw.init;
> > +	const char *name = init->name;
> > +
> > +	if (!pll->config || !pll->regs) {
> > +		pr_err("%s: missing pll config or regs\n", name);
> > +		return;
> > +	}
> 
> Seems like a strange check - you are calling this function in a loop which
> looks like
> 
> for (i = 0; i < desc->num_alpha_plls; i++)
> 	qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
> 
> Can num_alpha_plls be true but alpha_plls be NULL and why is regmap
> considered valid ?
> 
> I think you can drop this check.

I think pll->config should be moved to a calling code.

> 
> > +
> > +	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:
> > +		WARN(1, "%s: invalid pll type\n", name);
> > +		break;
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_clk_alpha_pll_configure);
> > diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
> > index 79aca8525262211ae5295245427d4540abf1e09a..7f35aaa7a35d88411beb11fd2be5d5dd5bfbe066 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)
> > @@ -237,5 +239,6 @@ void clk_stromer_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
> >   			       const struct alpha_pll_config *config);
> >   void clk_regera_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
> >   			     const struct alpha_pll_config *config);
> > +void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap);
> >   #endif
> > 

-- 
With best wishes
Dmitry

Re: [PATCH v3 04/18] clk: qcom: clk-alpha-pll: Add support for common PLL configuration function

[Jagadeesh Kona]

On 3/27/2025 11:50 PM, Dmitry Baryshkov wrote:
> On Thu, Mar 27, 2025 at 03:51:33PM +0000, Bryan O'Donoghue wrote:
>> On 27/03/2025 09:52, Jagadeesh Kona wrote:
>>> From: Taniya Das <quic_tdas@quicinc.com>
>>>
>>> To properly configure the PLLs on recent chipsets, it often requires more
>>> than one power domain to be kept ON. The support to enable multiple power
>>> domains is being added in qcom_cc_really_probe() and PLLs should be
>>> configured post all the required power domains are enabled.
>>>
>>> Hence integrate PLL configuration into clk_alpha_pll structure and add
>>> support for qcom_clk_alpha_pll_configure() function which can be called
>>> from qcom_cc_really_probe() to configure the clock controller PLLs after
>>> all required power domains are enabled.
>>>
>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>> ---
>>>   drivers/clk/qcom/clk-alpha-pll.c | 63 ++++++++++++++++++++++++++++++++++++++++
>>>   drivers/clk/qcom/clk-alpha-pll.h |  3 ++
>>>   2 files changed, 66 insertions(+)
>>>
>>> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
>>> index cec0afea8e446010f0d4140d4ef63121706dde47..8ee842254e6690e24469053cdbd99a9953987e40 100644
>>> --- a/drivers/clk/qcom/clk-alpha-pll.c
>>> +++ b/drivers/clk/qcom/clk-alpha-pll.c
>>> @@ -63,6 +63,8 @@
>>>   #define PLL_OPMODE(p)		((p)->offset + (p)->regs[PLL_OFF_OPMODE])
>>>   #define PLL_FRAC(p)		((p)->offset + (p)->regs[PLL_OFF_FRAC])
>>> +#define GET_PLL_TYPE(pll)	(((pll)->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)
>>> +
>>>   const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = {
>>>   	[CLK_ALPHA_PLL_TYPE_DEFAULT] =  {
>>>   		[PLL_OFF_L_VAL] = 0x04,
>>> @@ -2960,3 +2962,64 @@ const struct clk_ops clk_alpha_pll_regera_ops = {
>>>   	.set_rate = clk_zonda_pll_set_rate,
>>>   };
>>>   EXPORT_SYMBOL_GPL(clk_alpha_pll_regera_ops);
>>> +
>>> +void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
>>> +{
>>> +	const struct clk_init_data *init = pll->clkr.hw.init;
>>> +	const char *name = init->name;
>>> +
>>> +	if (!pll->config || !pll->regs) {
>>> +		pr_err("%s: missing pll config or regs\n", name);
>>> +		return;
>>> +	}
>>
>> Seems like a strange check - you are calling this function in a loop which
>> looks like
>>
>> for (i = 0; i < desc->num_alpha_plls; i++)
>> 	qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
>>
>> Can num_alpha_plls be true but alpha_plls be NULL and why is regmap
>> considered valid ?
>>
>> I think you can drop this check.
>

The regmap check is already performed in qcom_cc_map() before we reach this
point.

It is not possible for num_alpha_plls to be true when alpha_plls is NULL, as
num_alpha_plls is derived using SIZE_OF(alpha_plls).

Including the above check is beneficial to catch any errors in case we missed
adding the necessary config/regs fields to the PLL structure in the CC code.
 
> I think pll->config should be moved to a calling code.
>
Yes, I can move above checks to calling code 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:
>>> +		WARN(1, "%s: invalid pll type\n", name);
>>> +		break;
>>> +	}
>>> +}
>>> +EXPORT_SYMBOL_GPL(qcom_clk_alpha_pll_configure);
>>> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
>>> index 79aca8525262211ae5295245427d4540abf1e09a..7f35aaa7a35d88411beb11fd2be5d5dd5bfbe066 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)
>>> @@ -237,5 +239,6 @@ void clk_stromer_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
>>>   			       const struct alpha_pll_config *config);
>>>   void clk_regera_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
>>>   			     const struct alpha_pll_config *config);
>>> +void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap);
>>>   #endif
>>>
> 

[PATCH v3 05/18] clk: qcom: common: Handle runtime power management in qcom_cc_really_probe

[Jagadeesh Kona]

Add support for runtime power management in qcom_cc_really_probe() to
commonize it across all the clock controllers. The runtime power management
is not required for all clock controllers, hence handle the rpm based on
use_rpm flag in clock controller descriptor.

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

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 9e3380fd718198c9fe63d7361615a91c3ecb3d60..9cbf1c5296dad3ee5477a2f5a445488707663b9d 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>
 
@@ -304,6 +305,16 @@ 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;
+	}
+
 	reset = &cc->reset;
 	reset->rcdev.of_node = dev->of_node;
 	reset->rcdev.ops = &qcom_reset_ops;
@@ -314,23 +325,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;
@@ -341,7 +354,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++) {
@@ -350,14 +363,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 7ace5d7f5836aa81431153ba92d8f14f2ffe8147..9c10bc8c197cd7dfa25ccd245763ad6acb081523 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -38,6 +38,7 @@ struct qcom_cc_desc {
 	const struct qcom_icc_hws_data *icc_hws;
 	size_t num_icc_hws;
 	unsigned int icc_first_node_id;
+	bool use_rpm;
 };
 
 /**

-- 
2.34.1

Re: [PATCH v3 05/18] clk: qcom: common: Handle runtime power management in qcom_cc_really_probe

[Dmitry Baryshkov]

On Thu, Mar 27, 2025 at 03:22:25PM +0530, Jagadeesh Kona wrote:
> Add support for runtime power management in qcom_cc_really_probe() to
> commonize it across all the clock controllers. The runtime power management
> is not required for all clock controllers, hence handle the rpm based on
> use_rpm flag in clock controller descriptor.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>  drivers/clk/qcom/common.c | 37 ++++++++++++++++++++++++++++---------
>  drivers/clk/qcom/common.h |  1 +
>  2 files changed, 29 insertions(+), 9 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: [PATCH v3 05/18] clk: qcom: common: Handle runtime power management in qcom_cc_really_probe

[Bryan O'Donoghue]

On 27/03/2025 09:52, Jagadeesh Kona wrote:
> -		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);

Doesn't look right you're missing the put if register goes wrong

	ret = qcom_cc_icc_register(dev, desc);

	if (ret)
		goto put_rpm;

	return 0;

put_rpm:
	if (desc->us_rpm)
		pm_runtime_put();

	return ret;

Re: [PATCH v3 05/18] clk: qcom: common: Handle runtime power management in qcom_cc_really_probe

[Jagadeesh Kona]

On 3/27/2025 9:28 PM, Bryan O'Donoghue wrote:
> On 27/03/2025 09:52, Jagadeesh Kona wrote:
>> -        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);
> 
> Doesn't look right you're missing the put if register goes wrong
> 

The intention is to call pm_runtime_put() regardless of the return value
from qcom_cc_icc_register(), as it is the final API call. Therefore, the
return type is not checked, and pm_runtime_put() is called in both success
and failure cases before returning the final return code.

Thanks,
Jagadeesh

>     ret = qcom_cc_icc_register(dev, desc);
> 
>     if (ret)
>         goto put_rpm;
> 
>     return 0;
> 
> put_rpm:
>     if (desc->us_rpm)
>         pm_runtime_put();
> 
>     return ret;

[PATCH v3 06/18] clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe

[Jagadeesh Kona]

Add support to configure PLLS and clk registers in qcom_cc_really_probe().
This ensures all required power domains are enabled and kept ON by runtime
PM code in qcom_cc_really_probe() before configuring the PLLS or clock
registers.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 drivers/clk/qcom/common.c | 28 ++++++++++++++++++++++++++++
 drivers/clk/qcom/common.h | 19 +++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 9cbf1c5296dad3ee5477a2f5a445488707663b9d..c4d980c6145834969fada14863360ee81c9aa251 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -14,6 +14,8 @@
 #include <linux/of.h>
 
 #include "common.h"
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
 #include "clk-rcg.h"
 #include "clk-regmap.h"
 #include "reset.h"
@@ -285,6 +287,29 @@ static int qcom_cc_icc_register(struct device *dev,
 						     desc->num_icc_hws, icd);
 }
 
+static void qcom_cc_clk_pll_configure(const struct qcom_cc_desc *desc,
+				      struct regmap *regmap)
+{
+	int i;
+
+	for (i = 0; i < desc->num_alpha_plls; i++)
+		qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
+}
+
+static void qcom_cc_clk_regs_configure(const struct qcom_cc_desc *desc,
+				       struct regmap *regmap)
+{
+	struct qcom_clk_reg_setting *clk_regs = desc->clk_regs;
+	int i;
+
+	for (i = 0; i < desc->num_clk_cbcrs; i++)
+		qcom_branch_set_clk_en(regmap, desc->clk_cbcrs[i]);
+
+	for (i = 0 ; i < desc->num_clk_regs; i++)
+		regmap_update_bits(regmap, clk_regs[i].offset,
+				   clk_regs[i].mask, clk_regs[i].val);
+}
+
 int qcom_cc_really_probe(struct device *dev,
 			 const struct qcom_cc_desc *desc, struct regmap *regmap)
 {
@@ -315,6 +340,9 @@ int qcom_cc_really_probe(struct device *dev,
 			return ret;
 	}
 
+	qcom_cc_clk_pll_configure(desc, regmap);
+	qcom_cc_clk_regs_configure(desc, regmap);
+
 	reset = &cc->reset;
 	reset->rcdev.of_node = dev->of_node;
 	reset->rcdev.ops = &qcom_reset_ops;
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index 9c10bc8c197cd7dfa25ccd245763ad6acb081523..01b1ae52f2dc580350409d6244578944cce571f0 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -25,6 +25,19 @@ struct qcom_icc_hws_data {
 	int clk_id;
 };
 
+/**
+ * struct qcom_clk_reg_setting - Represents miscellaneous clock register settings
+ * @offset: address offset for the clock register
+ * @mask: bit mask indicating the bits to be updated
+ * @val: Encoded value to be set within the specified bit mask
+ *       (e.g., if writing 7 to bits 4-7, mask = 0xF0 and val = 0x70)
+ */
+struct qcom_clk_reg_setting {
+	u32 offset;
+	u32 mask;
+	u32 val;
+};
+
 struct qcom_cc_desc {
 	const struct regmap_config *config;
 	struct clk_regmap **clks;
@@ -38,6 +51,12 @@ struct qcom_cc_desc {
 	const struct qcom_icc_hws_data *icc_hws;
 	size_t num_icc_hws;
 	unsigned int icc_first_node_id;
+	u32 *clk_cbcrs;
+	size_t num_clk_cbcrs;
+	struct clk_alpha_pll **alpha_plls;
+	size_t num_alpha_plls;
+	struct qcom_clk_reg_setting *clk_regs;
+	size_t num_clk_regs;
 	bool use_rpm;
 };
 

-- 
2.34.1

Re: [PATCH v3 06/18] clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe

[Dmitry Baryshkov]

On Thu, Mar 27, 2025 at 03:22:26PM +0530, Jagadeesh Kona wrote:
> Add support to configure PLLS and clk registers in qcom_cc_really_probe().
> This ensures all required power domains are enabled and kept ON by runtime
> PM code in qcom_cc_really_probe() before configuring the PLLS or clock
> registers.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>  drivers/clk/qcom/common.c | 28 ++++++++++++++++++++++++++++
>  drivers/clk/qcom/common.h | 19 +++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 9cbf1c5296dad3ee5477a2f5a445488707663b9d..c4d980c6145834969fada14863360ee81c9aa251 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -14,6 +14,8 @@
>  #include <linux/of.h>
>  
>  #include "common.h"
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
>  #include "clk-rcg.h"
>  #include "clk-regmap.h"
>  #include "reset.h"
> @@ -285,6 +287,29 @@ static int qcom_cc_icc_register(struct device *dev,
>  						     desc->num_icc_hws, icd);
>  }
>  
> +static void qcom_cc_clk_pll_configure(const struct qcom_cc_desc *desc,
> +				      struct regmap *regmap)
> +{
> +	int i;
> +
> +	for (i = 0; i < desc->num_alpha_plls; i++)
> +		qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
> +}
> +
> +static void qcom_cc_clk_regs_configure(const struct qcom_cc_desc *desc,
> +				       struct regmap *regmap)
> +{
> +	struct qcom_clk_reg_setting *clk_regs = desc->clk_regs;
> +	int i;
> +
> +	for (i = 0; i < desc->num_clk_cbcrs; i++)
> +		qcom_branch_set_clk_en(regmap, desc->clk_cbcrs[i]);
> +
> +	for (i = 0 ; i < desc->num_clk_regs; i++)
> +		regmap_update_bits(regmap, clk_regs[i].offset,
> +				   clk_regs[i].mask, clk_regs[i].val);

I think there are other semantic functions which we don't want to
convert to offset-mask-val tuples. See drivers/clk/qcom/clk-branch.h.
I'd suggest to move setup steps to a driver callback. We can improve it
later on if it is found to make sense, but it won't block this series
from being merged.

> +}
> +
>  int qcom_cc_really_probe(struct device *dev,
>  			 const struct qcom_cc_desc *desc, struct regmap *regmap)
>  {
> @@ -315,6 +340,9 @@ int qcom_cc_really_probe(struct device *dev,
>  			return ret;
>  	}
>  
> +	qcom_cc_clk_pll_configure(desc, regmap);
> +	qcom_cc_clk_regs_configure(desc, regmap);
> +
>  	reset = &cc->reset;
>  	reset->rcdev.of_node = dev->of_node;
>  	reset->rcdev.ops = &qcom_reset_ops;
> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
> index 9c10bc8c197cd7dfa25ccd245763ad6acb081523..01b1ae52f2dc580350409d6244578944cce571f0 100644
> --- a/drivers/clk/qcom/common.h
> +++ b/drivers/clk/qcom/common.h
> @@ -25,6 +25,19 @@ struct qcom_icc_hws_data {
>  	int clk_id;
>  };
>  
> +/**
> + * struct qcom_clk_reg_setting - Represents miscellaneous clock register settings
> + * @offset: address offset for the clock register
> + * @mask: bit mask indicating the bits to be updated
> + * @val: Encoded value to be set within the specified bit mask
> + *       (e.g., if writing 7 to bits 4-7, mask = 0xF0 and val = 0x70)
> + */
> +struct qcom_clk_reg_setting {
> +	u32 offset;
> +	u32 mask;
> +	u32 val;
> +};
> +
>  struct qcom_cc_desc {
>  	const struct regmap_config *config;
>  	struct clk_regmap **clks;
> @@ -38,6 +51,12 @@ struct qcom_cc_desc {
>  	const struct qcom_icc_hws_data *icc_hws;
>  	size_t num_icc_hws;
>  	unsigned int icc_first_node_id;
> +	u32 *clk_cbcrs;
> +	size_t num_clk_cbcrs;
> +	struct clk_alpha_pll **alpha_plls;
> +	size_t num_alpha_plls;
> +	struct qcom_clk_reg_setting *clk_regs;
> +	size_t num_clk_regs;
>  	bool use_rpm;
>  };
>  
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 06/18] clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe

[Jagadeesh Kona]

On 3/27/2025 6:20 PM, Dmitry Baryshkov wrote:
> On Thu, Mar 27, 2025 at 03:22:26PM +0530, Jagadeesh Kona wrote:
>> Add support to configure PLLS and clk registers in qcom_cc_really_probe().
>> This ensures all required power domains are enabled and kept ON by runtime
>> PM code in qcom_cc_really_probe() before configuring the PLLS or clock
>> registers.
>>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
>>  drivers/clk/qcom/common.c | 28 ++++++++++++++++++++++++++++
>>  drivers/clk/qcom/common.h | 19 +++++++++++++++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>> index 9cbf1c5296dad3ee5477a2f5a445488707663b9d..c4d980c6145834969fada14863360ee81c9aa251 100644
>> --- a/drivers/clk/qcom/common.c
>> +++ b/drivers/clk/qcom/common.c
>> @@ -14,6 +14,8 @@
>>  #include <linux/of.h>
>>  
>>  #include "common.h"
>> +#include "clk-alpha-pll.h"
>> +#include "clk-branch.h"
>>  #include "clk-rcg.h"
>>  #include "clk-regmap.h"
>>  #include "reset.h"
>> @@ -285,6 +287,29 @@ static int qcom_cc_icc_register(struct device *dev,
>>  						     desc->num_icc_hws, icd);
>>  }
>>  
>> +static void qcom_cc_clk_pll_configure(const struct qcom_cc_desc *desc,
>> +				      struct regmap *regmap)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < desc->num_alpha_plls; i++)
>> +		qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
>> +}
>> +
>> +static void qcom_cc_clk_regs_configure(const struct qcom_cc_desc *desc,
>> +				       struct regmap *regmap)
>> +{
>> +	struct qcom_clk_reg_setting *clk_regs = desc->clk_regs;
>> +	int i;
>> +
>> +	for (i = 0; i < desc->num_clk_cbcrs; i++)
>> +		qcom_branch_set_clk_en(regmap, desc->clk_cbcrs[i]);
>> +
>> +	for (i = 0 ; i < desc->num_clk_regs; i++)
>> +		regmap_update_bits(regmap, clk_regs[i].offset,
>> +				   clk_regs[i].mask, clk_regs[i].val);
> 
> I think there are other semantic functions which we don't want to
> convert to offset-mask-val tuples. See drivers/clk/qcom/clk-branch.h.
> I'd suggest to move setup steps to a driver callback. We can improve it
> later on if it is found to make sense, but it won't block this series
> from being merged.
> 

Yes, there are other wrapper functions as well but they are unused in most
clock controllers. We will check more on how we can improve this in a separate
series.

Thanks,
Jagadeesh

>> +}
>> +
>>  int qcom_cc_really_probe(struct device *dev,
>>  			 const struct qcom_cc_desc *desc, struct regmap *regmap)
>>  {
>> @@ -315,6 +340,9 @@ int qcom_cc_really_probe(struct device *dev,
>>  			return ret;
>>  	}
>>  
>> +	qcom_cc_clk_pll_configure(desc, regmap);
>> +	qcom_cc_clk_regs_configure(desc, regmap);
>> +
>>  	reset = &cc->reset;
>>  	reset->rcdev.of_node = dev->of_node;
>>  	reset->rcdev.ops = &qcom_reset_ops;
>> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
>> index 9c10bc8c197cd7dfa25ccd245763ad6acb081523..01b1ae52f2dc580350409d6244578944cce571f0 100644
>> --- a/drivers/clk/qcom/common.h
>> +++ b/drivers/clk/qcom/common.h
>> @@ -25,6 +25,19 @@ struct qcom_icc_hws_data {
>>  	int clk_id;
>>  };
>>  
>> +/**
>> + * struct qcom_clk_reg_setting - Represents miscellaneous clock register settings
>> + * @offset: address offset for the clock register
>> + * @mask: bit mask indicating the bits to be updated
>> + * @val: Encoded value to be set within the specified bit mask
>> + *       (e.g., if writing 7 to bits 4-7, mask = 0xF0 and val = 0x70)
>> + */
>> +struct qcom_clk_reg_setting {
>> +	u32 offset;
>> +	u32 mask;
>> +	u32 val;
>> +};
>> +
>>  struct qcom_cc_desc {
>>  	const struct regmap_config *config;
>>  	struct clk_regmap **clks;
>> @@ -38,6 +51,12 @@ struct qcom_cc_desc {
>>  	const struct qcom_icc_hws_data *icc_hws;
>>  	size_t num_icc_hws;
>>  	unsigned int icc_first_node_id;
>> +	u32 *clk_cbcrs;
>> +	size_t num_clk_cbcrs;
>> +	struct clk_alpha_pll **alpha_plls;
>> +	size_t num_alpha_plls;
>> +	struct qcom_clk_reg_setting *clk_regs;
>> +	size_t num_clk_regs;
>>  	bool use_rpm;
>>  };
>>  
>>
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH v3 06/18] clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe

[Dmitry Baryshkov]

On Fri, 11 Apr 2025 at 10:14, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>
>
>
> On 3/27/2025 6:20 PM, Dmitry Baryshkov wrote:
> > On Thu, Mar 27, 2025 at 03:22:26PM +0530, Jagadeesh Kona wrote:
> >> Add support to configure PLLS and clk registers in qcom_cc_really_probe().
> >> This ensures all required power domains are enabled and kept ON by runtime
> >> PM code in qcom_cc_really_probe() before configuring the PLLS or clock
> >> registers.
> >>
> >> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> >> ---
> >>  drivers/clk/qcom/common.c | 28 ++++++++++++++++++++++++++++
> >>  drivers/clk/qcom/common.h | 19 +++++++++++++++++++
> >>  2 files changed, 47 insertions(+)
> >>
> >> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> >> index 9cbf1c5296dad3ee5477a2f5a445488707663b9d..c4d980c6145834969fada14863360ee81c9aa251 100644
> >> --- a/drivers/clk/qcom/common.c
> >> +++ b/drivers/clk/qcom/common.c
> >> @@ -14,6 +14,8 @@
> >>  #include <linux/of.h>
> >>
> >>  #include "common.h"
> >> +#include "clk-alpha-pll.h"
> >> +#include "clk-branch.h"
> >>  #include "clk-rcg.h"
> >>  #include "clk-regmap.h"
> >>  #include "reset.h"
> >> @@ -285,6 +287,29 @@ static int qcom_cc_icc_register(struct device *dev,
> >>                                                   desc->num_icc_hws, icd);
> >>  }
> >>
> >> +static void qcom_cc_clk_pll_configure(const struct qcom_cc_desc *desc,
> >> +                                  struct regmap *regmap)
> >> +{
> >> +    int i;
> >> +
> >> +    for (i = 0; i < desc->num_alpha_plls; i++)
> >> +            qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
> >> +}
> >> +
> >> +static void qcom_cc_clk_regs_configure(const struct qcom_cc_desc *desc,
> >> +                                   struct regmap *regmap)
> >> +{
> >> +    struct qcom_clk_reg_setting *clk_regs = desc->clk_regs;
> >> +    int i;
> >> +
> >> +    for (i = 0; i < desc->num_clk_cbcrs; i++)
> >> +            qcom_branch_set_clk_en(regmap, desc->clk_cbcrs[i]);
> >> +
> >> +    for (i = 0 ; i < desc->num_clk_regs; i++)
> >> +            regmap_update_bits(regmap, clk_regs[i].offset,
> >> +                               clk_regs[i].mask, clk_regs[i].val);
> >
> > I think there are other semantic functions which we don't want to
> > convert to offset-mask-val tuples. See drivers/clk/qcom/clk-branch.h.
> > I'd suggest to move setup steps to a driver callback. We can improve it
> > later on if it is found to make sense, but it won't block this series
> > from being merged.
> >
>
> Yes, there are other wrapper functions as well but they are unused in most
> clock controllers. We will check more on how we can improve this in a separate
> series.

Please do it the other way around. Implement a generic callback, then
we can check how to sort things out.

>
> Thanks,
> Jagadeesh
>
> >> +}
> >> +
> >>  int qcom_cc_really_probe(struct device *dev,
> >>                       const struct qcom_cc_desc *desc, struct regmap *regmap)
> >>  {
> >> @@ -315,6 +340,9 @@ int qcom_cc_really_probe(struct device *dev,
> >>                      return ret;
> >>      }
> >>
> >> +    qcom_cc_clk_pll_configure(desc, regmap);
> >> +    qcom_cc_clk_regs_configure(desc, regmap);
> >> +
> >>      reset = &cc->reset;
> >>      reset->rcdev.of_node = dev->of_node;
> >>      reset->rcdev.ops = &qcom_reset_ops;
> >> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
> >> index 9c10bc8c197cd7dfa25ccd245763ad6acb081523..01b1ae52f2dc580350409d6244578944cce571f0 100644
> >> --- a/drivers/clk/qcom/common.h
> >> +++ b/drivers/clk/qcom/common.h
> >> @@ -25,6 +25,19 @@ struct qcom_icc_hws_data {
> >>      int clk_id;
> >>  };
> >>
> >> +/**
> >> + * struct qcom_clk_reg_setting - Represents miscellaneous clock register settings
> >> + * @offset: address offset for the clock register
> >> + * @mask: bit mask indicating the bits to be updated
> >> + * @val: Encoded value to be set within the specified bit mask
> >> + *       (e.g., if writing 7 to bits 4-7, mask = 0xF0 and val = 0x70)
> >> + */
> >> +struct qcom_clk_reg_setting {
> >> +    u32 offset;
> >> +    u32 mask;
> >> +    u32 val;
> >> +};
> >> +
> >>  struct qcom_cc_desc {
> >>      const struct regmap_config *config;
> >>      struct clk_regmap **clks;
> >> @@ -38,6 +51,12 @@ struct qcom_cc_desc {
> >>      const struct qcom_icc_hws_data *icc_hws;
> >>      size_t num_icc_hws;
> >>      unsigned int icc_first_node_id;
> >> +    u32 *clk_cbcrs;
> >> +    size_t num_clk_cbcrs;
> >> +    struct clk_alpha_pll **alpha_plls;
> >> +    size_t num_alpha_plls;
> >> +    struct qcom_clk_reg_setting *clk_regs;
> >> +    size_t num_clk_regs;
> >>      bool use_rpm;
> >>  };
> >>
> >>
> >> --
> >> 2.34.1
> >>
> >



-- 
With best wishes
Dmitry

Re: [PATCH v3 06/18] clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe

[Jagadeesh Kona]

On 4/11/2025 2:21 PM, Dmitry Baryshkov wrote:
> On Fri, 11 Apr 2025 at 10:14, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>
>>
>>
>> On 3/27/2025 6:20 PM, Dmitry Baryshkov wrote:
>>> On Thu, Mar 27, 2025 at 03:22:26PM +0530, Jagadeesh Kona wrote:
>>>> Add support to configure PLLS and clk registers in qcom_cc_really_probe().
>>>> This ensures all required power domains are enabled and kept ON by runtime
>>>> PM code in qcom_cc_really_probe() before configuring the PLLS or clock
>>>> registers.
>>>>
>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>> ---
>>>>  drivers/clk/qcom/common.c | 28 ++++++++++++++++++++++++++++
>>>>  drivers/clk/qcom/common.h | 19 +++++++++++++++++++
>>>>  2 files changed, 47 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>>>> index 9cbf1c5296dad3ee5477a2f5a445488707663b9d..c4d980c6145834969fada14863360ee81c9aa251 100644
>>>> --- a/drivers/clk/qcom/common.c
>>>> +++ b/drivers/clk/qcom/common.c
>>>> @@ -14,6 +14,8 @@
>>>>  #include <linux/of.h>
>>>>
>>>>  #include "common.h"
>>>> +#include "clk-alpha-pll.h"
>>>> +#include "clk-branch.h"
>>>>  #include "clk-rcg.h"
>>>>  #include "clk-regmap.h"
>>>>  #include "reset.h"
>>>> @@ -285,6 +287,29 @@ static int qcom_cc_icc_register(struct device *dev,
>>>>                                                   desc->num_icc_hws, icd);
>>>>  }
>>>>
>>>> +static void qcom_cc_clk_pll_configure(const struct qcom_cc_desc *desc,
>>>> +                                  struct regmap *regmap)
>>>> +{
>>>> +    int i;
>>>> +
>>>> +    for (i = 0; i < desc->num_alpha_plls; i++)
>>>> +            qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
>>>> +}
>>>> +
>>>> +static void qcom_cc_clk_regs_configure(const struct qcom_cc_desc *desc,
>>>> +                                   struct regmap *regmap)
>>>> +{
>>>> +    struct qcom_clk_reg_setting *clk_regs = desc->clk_regs;
>>>> +    int i;
>>>> +
>>>> +    for (i = 0; i < desc->num_clk_cbcrs; i++)
>>>> +            qcom_branch_set_clk_en(regmap, desc->clk_cbcrs[i]);
>>>> +
>>>> +    for (i = 0 ; i < desc->num_clk_regs; i++)
>>>> +            regmap_update_bits(regmap, clk_regs[i].offset,
>>>> +                               clk_regs[i].mask, clk_regs[i].val);
>>>
>>> I think there are other semantic functions which we don't want to
>>> convert to offset-mask-val tuples. See drivers/clk/qcom/clk-branch.h.
>>> I'd suggest to move setup steps to a driver callback. We can improve it
>>> later on if it is found to make sense, but it won't block this series
>>> from being merged.
>>>
>>
>> Yes, there are other wrapper functions as well but they are unused in most
>> clock controllers. We will check more on how we can improve this in a separate
>> series.
> 
> Please do it the other way around. Implement a generic callback, then
> we can check how to sort things out.
> 

Yeah, but since this series doesn't require any misc register settings update, I
will remove the above regmap_update_bits() code for now. I will check further on
this and post a separate series for it.

Thanks,
Jagadeesh

>>
>> Thanks,
>> Jagadeesh
>>
>>>> +}
>>>> +
>>>>  int qcom_cc_really_probe(struct device *dev,
>>>>                       const struct qcom_cc_desc *desc, struct regmap *regmap)
>>>>  {
>>>> @@ -315,6 +340,9 @@ int qcom_cc_really_probe(struct device *dev,
>>>>                      return ret;
>>>>      }
>>>>
>>>> +    qcom_cc_clk_pll_configure(desc, regmap);
>>>> +    qcom_cc_clk_regs_configure(desc, regmap);
>>>> +
>>>>      reset = &cc->reset;
>>>>      reset->rcdev.of_node = dev->of_node;
>>>>      reset->rcdev.ops = &qcom_reset_ops;
>>>> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
>>>> index 9c10bc8c197cd7dfa25ccd245763ad6acb081523..01b1ae52f2dc580350409d6244578944cce571f0 100644
>>>> --- a/drivers/clk/qcom/common.h
>>>> +++ b/drivers/clk/qcom/common.h
>>>> @@ -25,6 +25,19 @@ struct qcom_icc_hws_data {
>>>>      int clk_id;
>>>>  };
>>>>
>>>> +/**
>>>> + * struct qcom_clk_reg_setting - Represents miscellaneous clock register settings
>>>> + * @offset: address offset for the clock register
>>>> + * @mask: bit mask indicating the bits to be updated
>>>> + * @val: Encoded value to be set within the specified bit mask
>>>> + *       (e.g., if writing 7 to bits 4-7, mask = 0xF0 and val = 0x70)
>>>> + */
>>>> +struct qcom_clk_reg_setting {
>>>> +    u32 offset;
>>>> +    u32 mask;
>>>> +    u32 val;
>>>> +};
>>>> +
>>>>  struct qcom_cc_desc {
>>>>      const struct regmap_config *config;
>>>>      struct clk_regmap **clks;
>>>> @@ -38,6 +51,12 @@ struct qcom_cc_desc {
>>>>      const struct qcom_icc_hws_data *icc_hws;
>>>>      size_t num_icc_hws;
>>>>      unsigned int icc_first_node_id;
>>>> +    u32 *clk_cbcrs;
>>>> +    size_t num_clk_cbcrs;
>>>> +    struct clk_alpha_pll **alpha_plls;
>>>> +    size_t num_alpha_plls;
>>>> +    struct qcom_clk_reg_setting *clk_regs;
>>>> +    size_t num_clk_regs;
>>>>      bool use_rpm;
>>>>  };
>>>>
>>>>
>>>> --
>>>> 2.34.1
>>>>
>>>
> 
> 
> 

Re: [PATCH v3 06/18] clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe

[Dmitry Baryshkov]

On Mon, Apr 14, 2025 at 03:39:04PM +0530, Jagadeesh Kona wrote:
> 
> 
> On 4/11/2025 2:21 PM, Dmitry Baryshkov wrote:
> > On Fri, 11 Apr 2025 at 10:14, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
> >>
> >>
> >>
> >> On 3/27/2025 6:20 PM, Dmitry Baryshkov wrote:
> >>> On Thu, Mar 27, 2025 at 03:22:26PM +0530, Jagadeesh Kona wrote:
> >>>> Add support to configure PLLS and clk registers in qcom_cc_really_probe().
> >>>> This ensures all required power domains are enabled and kept ON by runtime
> >>>> PM code in qcom_cc_really_probe() before configuring the PLLS or clock
> >>>> registers.
> >>>>
> >>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> >>>> ---
> >>>>  drivers/clk/qcom/common.c | 28 ++++++++++++++++++++++++++++
> >>>>  drivers/clk/qcom/common.h | 19 +++++++++++++++++++
> >>>>  2 files changed, 47 insertions(+)
> >>>>
> >>>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> >>>> index 9cbf1c5296dad3ee5477a2f5a445488707663b9d..c4d980c6145834969fada14863360ee81c9aa251 100644
> >>>> --- a/drivers/clk/qcom/common.c
> >>>> +++ b/drivers/clk/qcom/common.c
> >>>> @@ -14,6 +14,8 @@
> >>>>  #include <linux/of.h>
> >>>>
> >>>>  #include "common.h"
> >>>> +#include "clk-alpha-pll.h"
> >>>> +#include "clk-branch.h"
> >>>>  #include "clk-rcg.h"
> >>>>  #include "clk-regmap.h"
> >>>>  #include "reset.h"
> >>>> @@ -285,6 +287,29 @@ static int qcom_cc_icc_register(struct device *dev,
> >>>>                                                   desc->num_icc_hws, icd);
> >>>>  }
> >>>>
> >>>> +static void qcom_cc_clk_pll_configure(const struct qcom_cc_desc *desc,
> >>>> +                                  struct regmap *regmap)
> >>>> +{
> >>>> +    int i;
> >>>> +
> >>>> +    for (i = 0; i < desc->num_alpha_plls; i++)
> >>>> +            qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
> >>>> +}
> >>>> +
> >>>> +static void qcom_cc_clk_regs_configure(const struct qcom_cc_desc *desc,
> >>>> +                                   struct regmap *regmap)
> >>>> +{
> >>>> +    struct qcom_clk_reg_setting *clk_regs = desc->clk_regs;
> >>>> +    int i;
> >>>> +
> >>>> +    for (i = 0; i < desc->num_clk_cbcrs; i++)
> >>>> +            qcom_branch_set_clk_en(regmap, desc->clk_cbcrs[i]);
> >>>> +
> >>>> +    for (i = 0 ; i < desc->num_clk_regs; i++)
> >>>> +            regmap_update_bits(regmap, clk_regs[i].offset,
> >>>> +                               clk_regs[i].mask, clk_regs[i].val);
> >>>
> >>> I think there are other semantic functions which we don't want to
> >>> convert to offset-mask-val tuples. See drivers/clk/qcom/clk-branch.h.
> >>> I'd suggest to move setup steps to a driver callback. We can improve it
> >>> later on if it is found to make sense, but it won't block this series
> >>> from being merged.
> >>>
> >>
> >> Yes, there are other wrapper functions as well but they are unused in most
> >> clock controllers. We will check more on how we can improve this in a separate
> >> series.
> > 
> > Please do it the other way around. Implement a generic callback, then
> > we can check how to sort things out.
> > 
> 
> Yeah, but since this series doesn't require any misc register settings update, I
> will remove the above regmap_update_bits() code for now. I will check further on
> this and post a separate series for it.

This way we end up with a non-generic solution with no proposed path.
I'd really suggest using a callback for now to configure all
platform-specific and then post a separte series adding clk_cbcrs and
related functionality.

> 
> Thanks,
> Jagadeesh
> 
> >>
> >> Thanks,
> >> Jagadeesh
> >>
> >>>> +}
> >>>> +
> >>>>  int qcom_cc_really_probe(struct device *dev,
> >>>>                       const struct qcom_cc_desc *desc, struct regmap *regmap)
> >>>>  {
> >>>> @@ -315,6 +340,9 @@ int qcom_cc_really_probe(struct device *dev,
> >>>>                      return ret;
> >>>>      }
> >>>>
> >>>> +    qcom_cc_clk_pll_configure(desc, regmap);
> >>>> +    qcom_cc_clk_regs_configure(desc, regmap);
> >>>> +
> >>>>      reset = &cc->reset;
> >>>>      reset->rcdev.of_node = dev->of_node;
> >>>>      reset->rcdev.ops = &qcom_reset_ops;
> >>>> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
> >>>> index 9c10bc8c197cd7dfa25ccd245763ad6acb081523..01b1ae52f2dc580350409d6244578944cce571f0 100644
> >>>> --- a/drivers/clk/qcom/common.h
> >>>> +++ b/drivers/clk/qcom/common.h
> >>>> @@ -25,6 +25,19 @@ struct qcom_icc_hws_data {
> >>>>      int clk_id;
> >>>>  };
> >>>>
> >>>> +/**
> >>>> + * struct qcom_clk_reg_setting - Represents miscellaneous clock register settings
> >>>> + * @offset: address offset for the clock register
> >>>> + * @mask: bit mask indicating the bits to be updated
> >>>> + * @val: Encoded value to be set within the specified bit mask
> >>>> + *       (e.g., if writing 7 to bits 4-7, mask = 0xF0 and val = 0x70)
> >>>> + */
> >>>> +struct qcom_clk_reg_setting {
> >>>> +    u32 offset;
> >>>> +    u32 mask;
> >>>> +    u32 val;
> >>>> +};
> >>>> +
> >>>>  struct qcom_cc_desc {
> >>>>      const struct regmap_config *config;
> >>>>      struct clk_regmap **clks;
> >>>> @@ -38,6 +51,12 @@ struct qcom_cc_desc {
> >>>>      const struct qcom_icc_hws_data *icc_hws;
> >>>>      size_t num_icc_hws;
> >>>>      unsigned int icc_first_node_id;
> >>>> +    u32 *clk_cbcrs;
> >>>> +    size_t num_clk_cbcrs;
> >>>> +    struct clk_alpha_pll **alpha_plls;
> >>>> +    size_t num_alpha_plls;
> >>>> +    struct qcom_clk_reg_setting *clk_regs;
> >>>> +    size_t num_clk_regs;
> >>>>      bool use_rpm;
> >>>>  };
> >>>>
> >>>>
> >>>> --
> >>>> 2.34.1
> >>>>
> >>>
> > 
> > 
> > 

-- 
With best wishes
Dmitry

[PATCH v3 07/18] 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 runtime power management, 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 | 54 +++++++++++++++------------------------
 1 file changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8450.c b/drivers/clk/qcom/videocc-sm8450.c
index 2e11dcffb6646d47b298c27ef68635a465dd728e..50259374273d04dcf1486ea59915347340b6f2aa 100644
--- a/drivers/clk/qcom/videocc-sm8450.c
+++ b/drivers/clk/qcom/videocc-sm8450.c
@@ -7,7 +7,6 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,sm8450-videocc.h>
@@ -63,6 +62,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 +106,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 +408,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 u32 video_cc_sm8450_critical_cbcrs[] = {
+	0x80e4, /* VIDEO_CC_AHB_CLK */
+	0x8114, /* VIDEO_CC_XO_CLK */
+	0x8130, /* VIDEO_CC_SLEEP_CLK */
+};
+
 static const struct regmap_config video_cc_sm8450_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -423,6 +435,11 @@ static const 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),
+	.alpha_plls = video_cc_sm8450_plls,
+	.num_alpha_plls = ARRAY_SIZE(video_cc_sm8450_plls),
+	.clk_cbcrs = video_cc_sm8450_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(video_cc_sm8450_critical_cbcrs),
+	.use_rpm = true,
 };
 
 static const struct of_device_id video_cc_sm8450_match_table[] = {
@@ -434,23 +451,6 @@ 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);
-		return PTR_ERR(regmap);
-	}
-
 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8475-videocc")) {
 		/* Update VideoCC PLL0 */
 		video_cc_pll0.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE];
@@ -458,23 +458,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_probe(pdev, &video_cc_sm8450_desc);
 }
 
 static struct platform_driver video_cc_sm8450_driver = {

-- 
2.34.1

Re: [PATCH v3 07/18] clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe

[Dmitry Baryshkov]

On Thu, Mar 27, 2025 at 03:22:27PM +0530, Jagadeesh Kona wrote:
> Video PLLs on SM8450/SM8475 require both MMCX and MXC rails to be kept ON
> to configure the PLLs properly. Hence move runtime power management, 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 | 54 +++++++++++++++------------------------
>  1 file changed, 21 insertions(+), 33 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

[PATCH v3 08/18] 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 runtime power management, 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 | 55 ++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 33 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
index fcfe0cade6d0a95e749aabbc2af1174e5a70f0db..9c34ecc8ca5a042104956c48fd577b18929e6023 100644
--- a/drivers/clk/qcom/videocc-sm8550.c
+++ b/drivers/clk/qcom/videocc-sm8550.c
@@ -7,7 +7,6 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,sm8650-videocc.h>
@@ -51,6 +50,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 +82,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 +512,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 u32 video_cc_sm8550_critical_cbcrs[] = {
+	0x80f4, /* VIDEO_CC_AHB_CLK */
+	0x8124, /* VIDEO_CC_XO_CLK */
+	0x8140, /* VIDEO_CC_SLEEP_CLK */
+};
+
 static const struct regmap_config video_cc_sm8550_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -527,6 +539,11 @@ static const 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),
+	.alpha_plls = video_cc_sm8550_plls,
+	.num_alpha_plls = ARRAY_SIZE(video_cc_sm8550_plls),
+	.clk_cbcrs = video_cc_sm8550_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(video_cc_sm8550_critical_cbcrs),
+	.use_rpm = true,
 };
 
 static const struct of_device_id video_cc_sm8550_match_table[] = {
@@ -538,26 +555,7 @@ 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);
-		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 +567,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_critical_cbcrs[2] = 0x8150;
+	}
 
-	return ret;
+	return qcom_cc_probe(pdev, &video_cc_sm8550_desc);
 }
 
 static struct platform_driver video_cc_sm8550_driver = {

-- 
2.34.1

Re: [PATCH v3 08/18] clk: qcom: videocc-sm8550: Move PLL & clk configuration to really probe

[Dmitry Baryshkov]

On Thu, Mar 27, 2025 at 03:22:28PM +0530, Jagadeesh Kona wrote:
> Video PLLs on SM8550/SM8650 require both MMCX and MXC rails to be kept ON
> to configure the PLLs properly. Hence move runtime power management, 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 | 55 ++++++++++++++++-----------------------
>  1 file changed, 22 insertions(+), 33 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

[PATCH v3 09/18] clk: qcom: camcc-sm8450: Move PLL & clk configuration to really probe

[Jagadeesh Kona]

Camera PLLs on SM8450/SM8475 require both MMCX and MXC rails to be
kept ON to configure the PLLs properly. Hence move runtime power
management, 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.

This change also removes the modelling for cam_cc_gdsc_clk and keeps it
always ON from probe since using CLK_IS_CRITICAL will prevent the clock
controller associated power domains from collapsing due to clock framework
invoking clk_pm_runtime_get() during prepare.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 drivers/clk/qcom/camcc-sm8450.c | 85 +++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 45 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sm8450.c b/drivers/clk/qcom/camcc-sm8450.c
index 08982737e4901c0703e19f8dd2d302e24748210c..8656379efa135475f807ab7d3e1b8c88f932d3a4 100644
--- a/drivers/clk/qcom/camcc-sm8450.c
+++ b/drivers/clk/qcom/camcc-sm8450.c
@@ -86,6 +86,7 @@ static const struct alpha_pll_config sm8475_cam_cc_pll0_config = {
 
 static struct clk_alpha_pll cam_cc_pll0 = {
 	.offset = 0x0,
+	.config = &cam_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],
@@ -191,6 +192,7 @@ static const struct alpha_pll_config sm8475_cam_cc_pll1_config = {
 
 static struct clk_alpha_pll cam_cc_pll1 = {
 	.offset = 0x1000,
+	.config = &cam_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],
@@ -257,6 +259,7 @@ static const struct alpha_pll_config sm8475_cam_cc_pll2_config = {
 
 static struct clk_alpha_pll cam_cc_pll2 = {
 	.offset = 0x2000,
+	.config = &cam_cc_pll2_config,
 	.vco_table = rivian_evo_vco,
 	.num_vco = ARRAY_SIZE(rivian_evo_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_RIVIAN_EVO],
@@ -296,6 +299,7 @@ static const struct alpha_pll_config sm8475_cam_cc_pll3_config = {
 
 static struct clk_alpha_pll cam_cc_pll3 = {
 	.offset = 0x3000,
+	.config = &cam_cc_pll3_config,
 	.vco_table = lucid_evo_vco,
 	.num_vco = ARRAY_SIZE(lucid_evo_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_EVO],
@@ -368,6 +372,7 @@ static const struct alpha_pll_config sm8475_cam_cc_pll4_config = {
 
 static struct clk_alpha_pll cam_cc_pll4 = {
 	.offset = 0x4000,
+	.config = &cam_cc_pll4_config,
 	.vco_table = lucid_evo_vco,
 	.num_vco = ARRAY_SIZE(lucid_evo_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_EVO],
@@ -440,6 +445,7 @@ static const struct alpha_pll_config sm8475_cam_cc_pll5_config = {
 
 static struct clk_alpha_pll cam_cc_pll5 = {
 	.offset = 0x5000,
+	.config = &cam_cc_pll5_config,
 	.vco_table = lucid_evo_vco,
 	.num_vco = ARRAY_SIZE(lucid_evo_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_EVO],
@@ -512,6 +518,7 @@ static const struct alpha_pll_config sm8475_cam_cc_pll6_config = {
 
 static struct clk_alpha_pll cam_cc_pll6 = {
 	.offset = 0x6000,
+	.config = &cam_cc_pll6_config,
 	.vco_table = lucid_evo_vco,
 	.num_vco = ARRAY_SIZE(lucid_evo_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_EVO],
@@ -584,6 +591,7 @@ static const struct alpha_pll_config sm8475_cam_cc_pll7_config = {
 
 static struct clk_alpha_pll cam_cc_pll7 = {
 	.offset = 0x7000,
+	.config = &cam_cc_pll7_config,
 	.vco_table = lucid_evo_vco,
 	.num_vco = ARRAY_SIZE(lucid_evo_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_EVO],
@@ -656,6 +664,7 @@ static const struct alpha_pll_config sm8475_cam_cc_pll8_config = {
 
 static struct clk_alpha_pll cam_cc_pll8 = {
 	.offset = 0x8000,
+	.config = &cam_cc_pll8_config,
 	.vco_table = lucid_evo_vco,
 	.num_vco = ARRAY_SIZE(lucid_evo_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_EVO],
@@ -1476,24 +1485,6 @@ static struct clk_rcg2 cam_cc_xo_clk_src = {
 	},
 };
 
-static struct clk_branch cam_cc_gdsc_clk = {
-	.halt_reg = 0x1320c,
-	.halt_check = BRANCH_HALT,
-	.clkr = {
-		.enable_reg = 0x1320c,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "cam_cc_gdsc_clk",
-			.parent_hws = (const struct clk_hw*[]) {
-				&cam_cc_xo_clk_src.clkr.hw,
-			},
-			.num_parents = 1,
-			.flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch cam_cc_bps_ahb_clk = {
 	.halt_reg = 0x1004c,
 	.halt_check = BRANCH_HALT,
@@ -2819,7 +2810,6 @@ static struct clk_regmap *cam_cc_sm8450_clocks[] = {
 	[CAM_CC_CSIPHY4_CLK] = &cam_cc_csiphy4_clk.clkr,
 	[CAM_CC_CSIPHY5_CLK] = &cam_cc_csiphy5_clk.clkr,
 	[CAM_CC_FAST_AHB_CLK_SRC] = &cam_cc_fast_ahb_clk_src.clkr,
-	[CAM_CC_GDSC_CLK] = &cam_cc_gdsc_clk.clkr,
 	[CAM_CC_ICP_AHB_CLK] = &cam_cc_icp_ahb_clk.clkr,
 	[CAM_CC_ICP_CLK] = &cam_cc_icp_clk.clkr,
 	[CAM_CC_ICP_CLK_SRC] = &cam_cc_icp_clk_src.clkr,
@@ -2913,6 +2903,22 @@ static const struct qcom_reset_map cam_cc_sm8450_resets[] = {
 	[CAM_CC_SFE_1_BCR] = { 0x13094 },
 };
 
+static struct clk_alpha_pll *cam_cc_sm8450_plls[] = {
+	&cam_cc_pll0,
+	&cam_cc_pll1,
+	&cam_cc_pll2,
+	&cam_cc_pll3,
+	&cam_cc_pll4,
+	&cam_cc_pll5,
+	&cam_cc_pll6,
+	&cam_cc_pll7,
+	&cam_cc_pll8,
+};
+
+static u32 cam_cc_sm8450_critical_cbcrs[] = {
+	0x1320c, /* CAM_CC_GDSC_CLK */
+};
+
 static const struct regmap_config cam_cc_sm8450_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -3029,6 +3035,11 @@ static const struct qcom_cc_desc cam_cc_sm8450_desc = {
 	.num_resets = ARRAY_SIZE(cam_cc_sm8450_resets),
 	.gdscs = cam_cc_sm8450_gdscs,
 	.num_gdscs = ARRAY_SIZE(cam_cc_sm8450_gdscs),
+	.alpha_plls = cam_cc_sm8450_plls,
+	.num_alpha_plls = ARRAY_SIZE(cam_cc_sm8450_plls),
+	.clk_cbcrs = cam_cc_sm8450_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(cam_cc_sm8450_critical_cbcrs),
+	.use_rpm = true,
 };
 
 static const struct of_device_id cam_cc_sm8450_match_table[] = {
@@ -3040,12 +3051,6 @@ MODULE_DEVICE_TABLE(of, cam_cc_sm8450_match_table);
 
 static int cam_cc_sm8450_probe(struct platform_device *pdev)
 {
-	struct regmap *regmap;
-
-	regmap = qcom_cc_map(pdev, &cam_cc_sm8450_desc);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
-
 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8475-camcc")) {
 		/* Update CAMCC PLL0 */
 		cam_cc_pll0.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE];
@@ -3092,28 +3097,18 @@ static int cam_cc_sm8450_probe(struct platform_device *pdev)
 		cam_cc_pll8_out_even.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE];
 		cam_cc_pll8_out_even.clkr.hw.init = &sm8475_cam_cc_pll8_out_even_init;
 
-		clk_lucid_ole_pll_configure(&cam_cc_pll0, regmap, &sm8475_cam_cc_pll0_config);
-		clk_lucid_ole_pll_configure(&cam_cc_pll1, regmap, &sm8475_cam_cc_pll1_config);
-		clk_rivian_evo_pll_configure(&cam_cc_pll2, regmap, &sm8475_cam_cc_pll2_config);
-		clk_lucid_ole_pll_configure(&cam_cc_pll3, regmap, &sm8475_cam_cc_pll3_config);
-		clk_lucid_ole_pll_configure(&cam_cc_pll4, regmap, &sm8475_cam_cc_pll4_config);
-		clk_lucid_ole_pll_configure(&cam_cc_pll5, regmap, &sm8475_cam_cc_pll5_config);
-		clk_lucid_ole_pll_configure(&cam_cc_pll6, regmap, &sm8475_cam_cc_pll6_config);
-		clk_lucid_ole_pll_configure(&cam_cc_pll7, regmap, &sm8475_cam_cc_pll7_config);
-		clk_lucid_ole_pll_configure(&cam_cc_pll8, regmap, &sm8475_cam_cc_pll8_config);
-	} else {
-		clk_lucid_evo_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
-		clk_lucid_evo_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
-		clk_rivian_evo_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config);
-		clk_lucid_evo_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
-		clk_lucid_evo_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config);
-		clk_lucid_evo_pll_configure(&cam_cc_pll5, regmap, &cam_cc_pll5_config);
-		clk_lucid_evo_pll_configure(&cam_cc_pll6, regmap, &cam_cc_pll6_config);
-		clk_lucid_evo_pll_configure(&cam_cc_pll7, regmap, &cam_cc_pll7_config);
-		clk_lucid_evo_pll_configure(&cam_cc_pll8, regmap, &cam_cc_pll8_config);
+		cam_cc_pll0.config = &sm8475_cam_cc_pll0_config;
+		cam_cc_pll1.config = &sm8475_cam_cc_pll1_config;
+		cam_cc_pll2.config = &sm8475_cam_cc_pll2_config;
+		cam_cc_pll3.config = &sm8475_cam_cc_pll3_config;
+		cam_cc_pll4.config = &sm8475_cam_cc_pll4_config;
+		cam_cc_pll5.config = &sm8475_cam_cc_pll5_config;
+		cam_cc_pll6.config = &sm8475_cam_cc_pll6_config;
+		cam_cc_pll7.config = &sm8475_cam_cc_pll7_config;
+		cam_cc_pll8.config = &sm8475_cam_cc_pll8_config;
 	}
 
-	return qcom_cc_really_probe(&pdev->dev, &cam_cc_sm8450_desc, regmap);
+	return qcom_cc_probe(pdev, &cam_cc_sm8450_desc);
 }
 
 static struct platform_driver cam_cc_sm8450_driver = {

-- 
2.34.1

Re: [PATCH v3 09/18] clk: qcom: camcc-sm8450: Move PLL & clk configuration to really probe

[Dmitry Baryshkov]

On Thu, Mar 27, 2025 at 03:22:29PM +0530, Jagadeesh Kona wrote:
> Camera PLLs on SM8450/SM8475 require both MMCX and MXC rails to be
> kept ON to configure the PLLs properly. Hence move runtime power
> management, 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.
> 
> This change also removes the modelling for cam_cc_gdsc_clk and keeps it
> always ON from probe since using CLK_IS_CRITICAL will prevent the clock
> controller associated power domains from collapsing due to clock framework
> invoking clk_pm_runtime_get() during prepare.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>  drivers/clk/qcom/camcc-sm8450.c | 85 +++++++++++++++++++----------------------
>  1 file changed, 40 insertions(+), 45 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

[PATCH v3 10/18] clk: qcom: camcc-sm8550: Move PLL & clk configuration to really probe

[Jagadeesh Kona]

Camera PLLs on SM8550 require both MMCX and MXC rails to be kept ON to
configure the PLLs properly. Hence move runtime power management, PLL
configuration and enabling 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/camcc-sm8550.c | 81 ++++++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 41 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sm8550.c b/drivers/clk/qcom/camcc-sm8550.c
index 871155783c798fd9245d735642272eae2a2d3465..35158178a2ecba332f0079db3fe5c75b858724bc 100644
--- a/drivers/clk/qcom/camcc-sm8550.c
+++ b/drivers/clk/qcom/camcc-sm8550.c
@@ -7,7 +7,6 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,sm8550-camcc.h>
@@ -74,6 +73,7 @@ static const struct alpha_pll_config cam_cc_pll0_config = {
 
 static struct clk_alpha_pll cam_cc_pll0 = {
 	.offset = 0x0,
+	.config = &cam_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],
@@ -151,6 +151,7 @@ static const struct alpha_pll_config cam_cc_pll1_config = {
 
 static struct clk_alpha_pll cam_cc_pll1 = {
 	.offset = 0x1000,
+	.config = &cam_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],
@@ -201,6 +202,7 @@ static const struct alpha_pll_config cam_cc_pll2_config = {
 
 static struct clk_alpha_pll cam_cc_pll2 = {
 	.offset = 0x2000,
+	.config = &cam_cc_pll2_config,
 	.vco_table = rivian_ole_vco,
 	.num_vco = ARRAY_SIZE(rivian_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_RIVIAN_EVO],
@@ -232,6 +234,7 @@ static const struct alpha_pll_config cam_cc_pll3_config = {
 
 static struct clk_alpha_pll cam_cc_pll3 = {
 	.offset = 0x3000,
+	.config = &cam_cc_pll3_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -286,6 +289,7 @@ static const struct alpha_pll_config cam_cc_pll4_config = {
 
 static struct clk_alpha_pll cam_cc_pll4 = {
 	.offset = 0x4000,
+	.config = &cam_cc_pll4_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -340,6 +344,7 @@ static const struct alpha_pll_config cam_cc_pll5_config = {
 
 static struct clk_alpha_pll cam_cc_pll5 = {
 	.offset = 0x5000,
+	.config = &cam_cc_pll5_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -394,6 +399,7 @@ static const struct alpha_pll_config cam_cc_pll6_config = {
 
 static struct clk_alpha_pll cam_cc_pll6 = {
 	.offset = 0x6000,
+	.config = &cam_cc_pll6_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -448,6 +454,7 @@ static const struct alpha_pll_config cam_cc_pll7_config = {
 
 static struct clk_alpha_pll cam_cc_pll7 = {
 	.offset = 0x7000,
+	.config = &cam_cc_pll7_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -502,6 +509,7 @@ static const struct alpha_pll_config cam_cc_pll8_config = {
 
 static struct clk_alpha_pll cam_cc_pll8 = {
 	.offset = 0x8000,
+	.config = &cam_cc_pll8_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -556,6 +564,7 @@ static const struct alpha_pll_config cam_cc_pll9_config = {
 
 static struct clk_alpha_pll cam_cc_pll9 = {
 	.offset = 0x9000,
+	.config = &cam_cc_pll9_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -610,6 +619,7 @@ static const struct alpha_pll_config cam_cc_pll10_config = {
 
 static struct clk_alpha_pll cam_cc_pll10 = {
 	.offset = 0xa000,
+	.config = &cam_cc_pll10_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -664,6 +674,7 @@ static const struct alpha_pll_config cam_cc_pll11_config = {
 
 static struct clk_alpha_pll cam_cc_pll11 = {
 	.offset = 0xb000,
+	.config = &cam_cc_pll11_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -718,6 +729,7 @@ static const struct alpha_pll_config cam_cc_pll12_config = {
 
 static struct clk_alpha_pll cam_cc_pll12 = {
 	.offset = 0xc000,
+	.config = &cam_cc_pll12_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -3479,6 +3491,27 @@ static const struct qcom_reset_map cam_cc_sm8550_resets[] = {
 	[CAM_CC_SFE_1_BCR] = { 0x133dc },
 };
 
+static struct clk_alpha_pll *cam_cc_sm8550_plls[] = {
+	&cam_cc_pll0,
+	&cam_cc_pll1,
+	&cam_cc_pll2,
+	&cam_cc_pll3,
+	&cam_cc_pll4,
+	&cam_cc_pll5,
+	&cam_cc_pll6,
+	&cam_cc_pll7,
+	&cam_cc_pll8,
+	&cam_cc_pll9,
+	&cam_cc_pll10,
+	&cam_cc_pll11,
+	&cam_cc_pll12,
+};
+
+static u32 cam_cc_sm8550_critical_cbcrs[] = {
+	0x1419c, /* CAM_CC_GDSC_CLK */
+	0x142cc, /* CAM_CC_SLEEP_CLK */
+};
+
 static const struct regmap_config cam_cc_sm8550_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -3495,6 +3528,11 @@ static const struct qcom_cc_desc cam_cc_sm8550_desc = {
 	.num_resets = ARRAY_SIZE(cam_cc_sm8550_resets),
 	.gdscs = cam_cc_sm8550_gdscs,
 	.num_gdscs = ARRAY_SIZE(cam_cc_sm8550_gdscs),
+	.alpha_plls = cam_cc_sm8550_plls,
+	.num_alpha_plls = ARRAY_SIZE(cam_cc_sm8550_plls),
+	.clk_cbcrs = cam_cc_sm8550_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(cam_cc_sm8550_critical_cbcrs),
+	.use_rpm = true,
 };
 
 static const struct of_device_id cam_cc_sm8550_match_table[] = {
@@ -3505,46 +3543,7 @@ MODULE_DEVICE_TABLE(of, cam_cc_sm8550_match_table);
 
 static int cam_cc_sm8550_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, &cam_cc_sm8550_desc);
-	if (IS_ERR(regmap)) {
-		pm_runtime_put(&pdev->dev);
-		return PTR_ERR(regmap);
-	}
-
-	clk_lucid_ole_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
-	clk_rivian_evo_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll5, regmap, &cam_cc_pll5_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll6, regmap, &cam_cc_pll6_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll7, regmap, &cam_cc_pll7_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll8, regmap, &cam_cc_pll8_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll9, regmap, &cam_cc_pll9_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll10, regmap, &cam_cc_pll10_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll11, regmap, &cam_cc_pll11_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll12, regmap, &cam_cc_pll12_config);
-
-	/* Keep some clocks always-on */
-	qcom_branch_set_clk_en(regmap, 0x1419c); /* CAM_CC_GDSC_CLK */
-	qcom_branch_set_clk_en(regmap, 0x142cc); /* CAM_CC_SLEEP_CLK */
-
-	ret = qcom_cc_really_probe(&pdev->dev, &cam_cc_sm8550_desc, regmap);
-
-	pm_runtime_put(&pdev->dev);
-
-	return ret;
+	return qcom_cc_probe(pdev, &cam_cc_sm8550_desc);
 }
 
 static struct platform_driver cam_cc_sm8550_driver = {

-- 
2.34.1

Re: [PATCH v3 10/18] clk: qcom: camcc-sm8550: Move PLL & clk configuration to really probe

[Dmitry Baryshkov]

On Thu, Mar 27, 2025 at 03:22:30PM +0530, Jagadeesh Kona wrote:
> Camera PLLs on SM8550 require both MMCX and MXC rails to be kept ON to
> configure the PLLs properly. Hence move runtime power management, PLL
> configuration and enabling 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/camcc-sm8550.c | 81 ++++++++++++++++++++---------------------
>  1 file changed, 40 insertions(+), 41 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

[PATCH v3 11/18] clk: qcom: camcc-sm8650: Move PLL & clk configuration to really probe

[Jagadeesh Kona]

Camera PLLs on SM8650 require both MMCX and MXC rails to be kept ON
to configure the PLLs properly. Hence move runtime power management,
PLL configuration and enabling 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/camcc-sm8650.c | 79 ++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 41 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sm8650.c b/drivers/clk/qcom/camcc-sm8650.c
index 0ccd6de8ba78a3493f8f853a4330d2676b5743d4..c08b43f27aa20f6c666586bdfd9ade836104151e 100644
--- a/drivers/clk/qcom/camcc-sm8650.c
+++ b/drivers/clk/qcom/camcc-sm8650.c
@@ -7,7 +7,6 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,sm8650-camcc.h>
@@ -72,6 +71,7 @@ static const struct alpha_pll_config cam_cc_pll0_config = {
 
 static struct clk_alpha_pll cam_cc_pll0 = {
 	.offset = 0x0,
+	.config = &cam_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],
@@ -149,6 +149,7 @@ static const struct alpha_pll_config cam_cc_pll1_config = {
 
 static struct clk_alpha_pll cam_cc_pll1 = {
 	.offset = 0x1000,
+	.config = &cam_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],
@@ -199,6 +200,7 @@ static const struct alpha_pll_config cam_cc_pll2_config = {
 
 static struct clk_alpha_pll cam_cc_pll2 = {
 	.offset = 0x2000,
+	.config = &cam_cc_pll2_config,
 	.vco_table = rivian_ole_vco,
 	.num_vco = ARRAY_SIZE(rivian_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_RIVIAN_EVO],
@@ -230,6 +232,7 @@ static const struct alpha_pll_config cam_cc_pll3_config = {
 
 static struct clk_alpha_pll cam_cc_pll3 = {
 	.offset = 0x3000,
+	.config = &cam_cc_pll3_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -284,6 +287,7 @@ static const struct alpha_pll_config cam_cc_pll4_config = {
 
 static struct clk_alpha_pll cam_cc_pll4 = {
 	.offset = 0x4000,
+	.config = &cam_cc_pll4_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -338,6 +342,7 @@ static const struct alpha_pll_config cam_cc_pll5_config = {
 
 static struct clk_alpha_pll cam_cc_pll5 = {
 	.offset = 0x5000,
+	.config = &cam_cc_pll5_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -392,6 +397,7 @@ static const struct alpha_pll_config cam_cc_pll6_config = {
 
 static struct clk_alpha_pll cam_cc_pll6 = {
 	.offset = 0x6000,
+	.config = &cam_cc_pll6_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -446,6 +452,7 @@ static const struct alpha_pll_config cam_cc_pll7_config = {
 
 static struct clk_alpha_pll cam_cc_pll7 = {
 	.offset = 0x7000,
+	.config = &cam_cc_pll7_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -500,6 +507,7 @@ static const struct alpha_pll_config cam_cc_pll8_config = {
 
 static struct clk_alpha_pll cam_cc_pll8 = {
 	.offset = 0x8000,
+	.config = &cam_cc_pll8_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -554,6 +562,7 @@ static const struct alpha_pll_config cam_cc_pll9_config = {
 
 static struct clk_alpha_pll cam_cc_pll9 = {
 	.offset = 0x9000,
+	.config = &cam_cc_pll9_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -631,6 +640,7 @@ static const struct alpha_pll_config cam_cc_pll10_config = {
 
 static struct clk_alpha_pll cam_cc_pll10 = {
 	.offset = 0xa000,
+	.config = &cam_cc_pll10_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -3509,6 +3519,27 @@ static const struct qcom_reset_map cam_cc_sm8650_resets[] = {
 	[CAM_CC_SFE_2_BCR] = { 0x130f4 },
 };
 
+static struct clk_alpha_pll *cam_cc_sm8650_plls[] = {
+	&cam_cc_pll0,
+	&cam_cc_pll1,
+	&cam_cc_pll2,
+	&cam_cc_pll3,
+	&cam_cc_pll4,
+	&cam_cc_pll5,
+	&cam_cc_pll6,
+	&cam_cc_pll7,
+	&cam_cc_pll8,
+	&cam_cc_pll9,
+	&cam_cc_pll10,
+};
+
+static u32 cam_cc_sm8650_critical_cbcrs[] = {
+	0x132ec, /* CAM_CC_GDSC_CLK */
+	0x13308, /* CAM_CC_SLEEP_CLK */
+	0x13314, /* CAM_CC_DRV_XO_CLK */
+	0x13318, /* CAM_CC_DRV_AHB_CLK */
+};
+
 static const struct regmap_config cam_cc_sm8650_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -3525,6 +3556,11 @@ static const struct qcom_cc_desc cam_cc_sm8650_desc = {
 	.num_resets = ARRAY_SIZE(cam_cc_sm8650_resets),
 	.gdscs = cam_cc_sm8650_gdscs,
 	.num_gdscs = ARRAY_SIZE(cam_cc_sm8650_gdscs),
+	.alpha_plls = cam_cc_sm8650_plls,
+	.num_alpha_plls = ARRAY_SIZE(cam_cc_sm8650_plls),
+	.clk_cbcrs = cam_cc_sm8650_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(cam_cc_sm8650_critical_cbcrs),
+	.use_rpm = true,
 };
 
 static const struct of_device_id cam_cc_sm8650_match_table[] = {
@@ -3535,46 +3571,7 @@ MODULE_DEVICE_TABLE(of, cam_cc_sm8650_match_table);
 
 static int cam_cc_sm8650_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, &cam_cc_sm8650_desc);
-	if (IS_ERR(regmap)) {
-		pm_runtime_put(&pdev->dev);
-		return PTR_ERR(regmap);
-	}
-
-	clk_lucid_ole_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
-	clk_rivian_evo_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll5, regmap, &cam_cc_pll5_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll6, regmap, &cam_cc_pll6_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll7, regmap, &cam_cc_pll7_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll8, regmap, &cam_cc_pll8_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll9, regmap, &cam_cc_pll9_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll10, regmap, &cam_cc_pll10_config);
-
-	/* Keep clocks always enabled */
-	qcom_branch_set_clk_en(regmap, 0x13318); /* CAM_CC_DRV_AHB_CLK */
-	qcom_branch_set_clk_en(regmap, 0x13314); /* CAM_CC_DRV_XO_CLK */
-	qcom_branch_set_clk_en(regmap, 0x132ec); /* CAM_CC_GDSC_CLK */
-	qcom_branch_set_clk_en(regmap, 0x13308); /* CAM_CC_SLEEP_CLK */
-
-	ret = qcom_cc_really_probe(&pdev->dev, &cam_cc_sm8650_desc, regmap);
-
-	pm_runtime_put(&pdev->dev);
-
-	return ret;
+	return qcom_cc_probe(pdev, &cam_cc_sm8650_desc);
 }
 
 static struct platform_driver cam_cc_sm8650_driver = {

-- 
2.34.1

[PATCH v3 12/18] clk: qcom: camcc-x1e80100: Move PLL & clk configuration to really probe

[Jagadeesh Kona]

Camera PLLs on X1E80100 require both MMCX and MXC rails to be kept ON
to configure the PLLs properly. Hence move runtime power management,
PLL configuration and enabling 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/camcc-x1e80100.c | 63 +++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/qcom/camcc-x1e80100.c b/drivers/clk/qcom/camcc-x1e80100.c
index b73524ae64b1b2b1ee94ceca88b5f3b46143f20b..1f2e49c4798f33b2204b95665cc977b4a52b549a 100644
--- a/drivers/clk/qcom/camcc-x1e80100.c
+++ b/drivers/clk/qcom/camcc-x1e80100.c
@@ -7,7 +7,6 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
@@ -67,6 +66,7 @@ static const struct alpha_pll_config cam_cc_pll0_config = {
 
 static struct clk_alpha_pll cam_cc_pll0 = {
 	.offset = 0x0,
+	.config = &cam_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],
@@ -144,6 +144,7 @@ static const struct alpha_pll_config cam_cc_pll1_config = {
 
 static struct clk_alpha_pll cam_cc_pll1 = {
 	.offset = 0x1000,
+	.config = &cam_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],
@@ -194,6 +195,7 @@ static const struct alpha_pll_config cam_cc_pll2_config = {
 
 static struct clk_alpha_pll cam_cc_pll2 = {
 	.offset = 0x2000,
+	.config = &cam_cc_pll2_config,
 	.vco_table = rivian_ole_vco,
 	.num_vco = ARRAY_SIZE(rivian_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_RIVIAN_EVO],
@@ -225,6 +227,7 @@ static const struct alpha_pll_config cam_cc_pll3_config = {
 
 static struct clk_alpha_pll cam_cc_pll3 = {
 	.offset = 0x3000,
+	.config = &cam_cc_pll3_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -279,6 +282,7 @@ static const struct alpha_pll_config cam_cc_pll4_config = {
 
 static struct clk_alpha_pll cam_cc_pll4 = {
 	.offset = 0x4000,
+	.config = &cam_cc_pll4_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -333,6 +337,7 @@ static const struct alpha_pll_config cam_cc_pll6_config = {
 
 static struct clk_alpha_pll cam_cc_pll6 = {
 	.offset = 0x6000,
+	.config = &cam_cc_pll6_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -387,6 +392,7 @@ static const struct alpha_pll_config cam_cc_pll8_config = {
 
 static struct clk_alpha_pll cam_cc_pll8 = {
 	.offset = 0x8000,
+	.config = &cam_cc_pll8_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -2418,6 +2424,21 @@ static const struct qcom_reset_map cam_cc_x1e80100_resets[] = {
 	[CAM_CC_SFE_0_BCR] = { 0x1327c },
 };
 
+static struct clk_alpha_pll *cam_cc_x1e80100_plls[] = {
+	&cam_cc_pll0,
+	&cam_cc_pll1,
+	&cam_cc_pll2,
+	&cam_cc_pll3,
+	&cam_cc_pll4,
+	&cam_cc_pll6,
+	&cam_cc_pll8,
+};
+
+static u32 cam_cc_x1e80100_critical_cbcrs[] = {
+	0x13a9c, /* CAM_CC_GDSC_CLK */
+	0x13ab8, /* CAM_CC_SLEEP_CLK */
+};
+
 static const struct regmap_config cam_cc_x1e80100_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -2434,6 +2455,11 @@ static const struct qcom_cc_desc cam_cc_x1e80100_desc = {
 	.num_resets = ARRAY_SIZE(cam_cc_x1e80100_resets),
 	.gdscs = cam_cc_x1e80100_gdscs,
 	.num_gdscs = ARRAY_SIZE(cam_cc_x1e80100_gdscs),
+	.alpha_plls = cam_cc_x1e80100_plls,
+	.num_alpha_plls = ARRAY_SIZE(cam_cc_x1e80100_plls),
+	.clk_cbcrs = cam_cc_x1e80100_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(cam_cc_x1e80100_critical_cbcrs),
+	.use_rpm = true,
 };
 
 static const struct of_device_id cam_cc_x1e80100_match_table[] = {
@@ -2444,40 +2470,7 @@ MODULE_DEVICE_TABLE(of, cam_cc_x1e80100_match_table);
 
 static int cam_cc_x1e80100_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, &cam_cc_x1e80100_desc);
-	if (IS_ERR(regmap)) {
-		pm_runtime_put(&pdev->dev);
-		return PTR_ERR(regmap);
-	}
-
-	clk_lucid_ole_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
-	clk_rivian_evo_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll6, regmap, &cam_cc_pll6_config);
-	clk_lucid_ole_pll_configure(&cam_cc_pll8, regmap, &cam_cc_pll8_config);
-
-	/* Keep clocks always enabled */
-	qcom_branch_set_clk_en(regmap, 0x13a9c); /* CAM_CC_GDSC_CLK */
-	qcom_branch_set_clk_en(regmap, 0x13ab8); /* CAM_CC_SLEEP_CLK */
-
-	ret = qcom_cc_really_probe(&pdev->dev, &cam_cc_x1e80100_desc, regmap);
-
-	pm_runtime_put(&pdev->dev);
-
-	return ret;
+	return qcom_cc_probe(pdev, &cam_cc_x1e80100_desc);
 }
 
 static struct platform_driver cam_cc_x1e80100_driver = {

-- 
2.34.1

Re: [PATCH v3 12/18] clk: qcom: camcc-x1e80100: Move PLL & clk configuration to really probe

[Bryan O'Donoghue]

On 27/03/2025 09:52, Jagadeesh Kona wrote:
> Camera PLLs on X1E80100 require both MMCX and MXC rails to be kept ON
> to configure the PLLs properly. Hence move runtime power management,
> PLL configuration and enabling 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/camcc-x1e80100.c | 63 +++++++++++++++++----------------------
>   1 file changed, 28 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/clk/qcom/camcc-x1e80100.c b/drivers/clk/qcom/camcc-x1e80100.c
> index b73524ae64b1b2b1ee94ceca88b5f3b46143f20b..1f2e49c4798f33b2204b95665cc977b4a52b549a 100644
> --- a/drivers/clk/qcom/camcc-x1e80100.c
> +++ b/drivers/clk/qcom/camcc-x1e80100.c
> @@ -7,7 +7,6 @@
>   #include <linux/mod_devicetable.h>
>   #include <linux/module.h>
>   #include <linux/platform_device.h>
> -#include <linux/pm_runtime.h>
>   #include <linux/regmap.h>
>   
>   #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
> @@ -67,6 +66,7 @@ static const struct alpha_pll_config cam_cc_pll0_config = {
>   
>   static struct clk_alpha_pll cam_cc_pll0 = {
>   	.offset = 0x0,
> +	.config = &cam_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],
> @@ -144,6 +144,7 @@ static const struct alpha_pll_config cam_cc_pll1_config = {
>   
>   static struct clk_alpha_pll cam_cc_pll1 = {
>   	.offset = 0x1000,
> +	.config = &cam_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],
> @@ -194,6 +195,7 @@ static const struct alpha_pll_config cam_cc_pll2_config = {
>   
>   static struct clk_alpha_pll cam_cc_pll2 = {
>   	.offset = 0x2000,
> +	.config = &cam_cc_pll2_config,
>   	.vco_table = rivian_ole_vco,
>   	.num_vco = ARRAY_SIZE(rivian_ole_vco),
>   	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_RIVIAN_EVO],
> @@ -225,6 +227,7 @@ static const struct alpha_pll_config cam_cc_pll3_config = {
>   
>   static struct clk_alpha_pll cam_cc_pll3 = {
>   	.offset = 0x3000,
> +	.config = &cam_cc_pll3_config,
>   	.vco_table = lucid_ole_vco,
>   	.num_vco = ARRAY_SIZE(lucid_ole_vco),
>   	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
> @@ -279,6 +282,7 @@ static const struct alpha_pll_config cam_cc_pll4_config = {
>   
>   static struct clk_alpha_pll cam_cc_pll4 = {
>   	.offset = 0x4000,
> +	.config = &cam_cc_pll4_config,
>   	.vco_table = lucid_ole_vco,
>   	.num_vco = ARRAY_SIZE(lucid_ole_vco),
>   	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
> @@ -333,6 +337,7 @@ static const struct alpha_pll_config cam_cc_pll6_config = {
>   
>   static struct clk_alpha_pll cam_cc_pll6 = {
>   	.offset = 0x6000,
> +	.config = &cam_cc_pll6_config,
>   	.vco_table = lucid_ole_vco,
>   	.num_vco = ARRAY_SIZE(lucid_ole_vco),
>   	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
> @@ -387,6 +392,7 @@ static const struct alpha_pll_config cam_cc_pll8_config = {
>   
>   static struct clk_alpha_pll cam_cc_pll8 = {
>   	.offset = 0x8000,
> +	.config = &cam_cc_pll8_config,
>   	.vco_table = lucid_ole_vco,
>   	.num_vco = ARRAY_SIZE(lucid_ole_vco),
>   	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
> @@ -2418,6 +2424,21 @@ static const struct qcom_reset_map cam_cc_x1e80100_resets[] = {
>   	[CAM_CC_SFE_0_BCR] = { 0x1327c },
>   };
>   
> +static struct clk_alpha_pll *cam_cc_x1e80100_plls[] = {
> +	&cam_cc_pll0,
> +	&cam_cc_pll1,
> +	&cam_cc_pll2,
> +	&cam_cc_pll3,
> +	&cam_cc_pll4,
> +	&cam_cc_pll6,
> +	&cam_cc_pll8,
> +};
> +
> +static u32 cam_cc_x1e80100_critical_cbcrs[] = {
> +	0x13a9c, /* CAM_CC_GDSC_CLK */
> +	0x13ab8, /* CAM_CC_SLEEP_CLK */
> +};
> +
>   static const struct regmap_config cam_cc_x1e80100_regmap_config = {
>   	.reg_bits = 32,
>   	.reg_stride = 4,
> @@ -2434,6 +2455,11 @@ static const struct qcom_cc_desc cam_cc_x1e80100_desc = {
>   	.num_resets = ARRAY_SIZE(cam_cc_x1e80100_resets),
>   	.gdscs = cam_cc_x1e80100_gdscs,
>   	.num_gdscs = ARRAY_SIZE(cam_cc_x1e80100_gdscs),
> +	.alpha_plls = cam_cc_x1e80100_plls,
> +	.num_alpha_plls = ARRAY_SIZE(cam_cc_x1e80100_plls),
> +	.clk_cbcrs = cam_cc_x1e80100_critical_cbcrs,
> +	.num_clk_cbcrs = ARRAY_SIZE(cam_cc_x1e80100_critical_cbcrs),
> +	.use_rpm = true,
>   };
>   
>   static const struct of_device_id cam_cc_x1e80100_match_table[] = {
> @@ -2444,40 +2470,7 @@ MODULE_DEVICE_TABLE(of, cam_cc_x1e80100_match_table);
>   
>   static int cam_cc_x1e80100_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, &cam_cc_x1e80100_desc);
> -	if (IS_ERR(regmap)) {
> -		pm_runtime_put(&pdev->dev);
> -		return PTR_ERR(regmap);
> -	}
> -
> -	clk_lucid_ole_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
> -	clk_lucid_ole_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
> -	clk_rivian_evo_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config);
> -	clk_lucid_ole_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
> -	clk_lucid_ole_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config);
> -	clk_lucid_ole_pll_configure(&cam_cc_pll6, regmap, &cam_cc_pll6_config);
> -	clk_lucid_ole_pll_configure(&cam_cc_pll8, regmap, &cam_cc_pll8_config);
> -
> -	/* Keep clocks always enabled */
> -	qcom_branch_set_clk_en(regmap, 0x13a9c); /* CAM_CC_GDSC_CLK */
> -	qcom_branch_set_clk_en(regmap, 0x13ab8); /* CAM_CC_SLEEP_CLK */
> -
> -	ret = qcom_cc_really_probe(&pdev->dev, &cam_cc_x1e80100_desc, regmap);
> -
> -	pm_runtime_put(&pdev->dev);
> -
> -	return ret;
> +	return qcom_cc_probe(pdev, &cam_cc_x1e80100_desc);
>   }
>   
>   static struct platform_driver cam_cc_x1e80100_driver = {
> 

Thanks for this work.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # Dell Inspiron 
14 Plus 7441

[PATCH v3 13/18] 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>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

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

-- 
2.34.1

[PATCH v3 14/18] 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>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

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

-- 
2.34.1

[PATCH v3 15/18] 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>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@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 818db6ba3b3be99c187512ea4acf2004422f6a18..ad60596b71d25bb0198b26660dc41195a1210a23 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -4959,7 +4959,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 v3 15/18] arm64: dts: qcom: Add MXC power domain to videocc node on SM8650

[Konrad Dybcio]

On 3/27/25 10:52 AM, Jagadeesh Kona wrote:
> 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>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@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 818db6ba3b3be99c187512ea4acf2004422f6a18..ad60596b71d25bb0198b26660dc41195a1210a23 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -4959,7 +4959,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>;

So all other DTs touched in this series reference low_svs in required-opps

Is that an actual requirement? Otherwise since Commit e3e56c050ab6
("soc: qcom: rpmhpd: Make power_on actually enable the domain") we get the
first nonzero state, which can be something like low_svs_d2

Konrad

Re: [PATCH v3 15/18] arm64: dts: qcom: Add MXC power domain to videocc node on SM8650

[Konrad Dybcio]

On 4/1/25 5:27 PM, Konrad Dybcio wrote:
> On 3/27/25 10:52 AM, Jagadeesh Kona wrote:
>> 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>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@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 818db6ba3b3be99c187512ea4acf2004422f6a18..ad60596b71d25bb0198b26660dc41195a1210a23 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> @@ -4959,7 +4959,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>;
> 
> So all other DTs touched in this series reference low_svs in required-opps

actually "all" is wrong on my side, please also consider and if necessary apply
the same change to patch 18

Konrad

> 
> Is that an actual requirement? Otherwise since Commit e3e56c050ab6
> ("soc: qcom: rpmhpd: Make power_on actually enable the domain") we get the
> first nonzero state, which can be something like low_svs_d2
> 
> Konrad

Re: [PATCH v3 15/18] arm64: dts: qcom: Add MXC power domain to videocc node on SM8650

[Jagadeesh Kona]

On 4/1/2025 9:30 PM, Konrad Dybcio wrote:
> On 4/1/25 5:27 PM, Konrad Dybcio wrote:
>> On 3/27/25 10:52 AM, Jagadeesh Kona wrote:
>>> 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>
>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@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 818db6ba3b3be99c187512ea4acf2004422f6a18..ad60596b71d25bb0198b26660dc41195a1210a23 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> @@ -4959,7 +4959,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>;
>>
>> So all other DTs touched in this series reference low_svs in required-opps
> 
> actually "all" is wrong on my side, please also consider and if necessary apply
> the same change to patch 18
> 
> Konrad
>
It is not needed for SM8650. In the initial SM8650 videocc and camcc series,
required-opps was added for MMCX. But it was dropped based on the review comments
in that series, after confirming that minimum non-zero level from cmd-db on MMCX
is > retention on SM8650. And as mentioned here[1], required-opps is not mandatory
for MXC as well.

[1]: https://lore.kernel.org/all/44dad3b5-ea3d-47db-8aca-8f67294fced9@quicinc.com/

Thanks,
Jagadeesh

>>
>> Is that an actual requirement? Otherwise since Commit e3e56c050ab6
>> ("soc: qcom: rpmhpd: Make power_on actually enable the domain") we get the
>> first nonzero state, which can be something like low_svs_d2
>>
>> Konrad

Re: [PATCH v3 15/18] arm64: dts: qcom: Add MXC power domain to videocc node on SM8650

[Jagadeesh Kona]

On 4/1/2025 8:57 PM, Konrad Dybcio wrote:
> On 3/27/25 10:52 AM, Jagadeesh Kona wrote:
>> 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>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@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 818db6ba3b3be99c187512ea4acf2004422f6a18..ad60596b71d25bb0198b26660dc41195a1210a23 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> @@ -4959,7 +4959,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>;
> 
> So all other DTs touched in this series reference low_svs in required-opps
> 
> Is that an actual requirement? Otherwise since Commit e3e56c050ab6
> ("soc: qcom: rpmhpd: Make power_on actually enable the domain") we get the
> first nonzero state, which can be something like low_svs_d2
> 
Yes, commit e3e56c050ab6 enables the power-domain at first non-zero level, but in
some chipsets, the first nonzero state could be retention, which is not sufficient
for clock controller to operate. So required-opps is needed to ensure the rails are
at a level above retention for clock controller to operate. low_svs was choosen since
that is a level that is generally supported across all the chipsets, but low_svs_d2
may not be supported on some chipsets.

And required-opps is not mandatory for MXC power domain due to commit f0cc5f7cb43f
(pmdomain: qcom: rpmhpd: Skip retention level for Power Domains), which ensures MXC
always gets enabled above retention level. But it was added to make number of
required-opps uniform with the number of power domains based on discussion at [1].

[1]: https://lore.kernel.org/all/eoqqz5hyyq6ej5uo6phijbeu5qafbpfxlnreyzzcyfw23pl2yq@ftxnasc6sr2t/#t

Thanks,
Jagadeesh

> Konrad

Re: [PATCH v3 15/18] arm64: dts: qcom: Add MXC power domain to videocc node on SM8650

[Konrad Dybcio]

On 4/11/25 9:16 AM, Jagadeesh Kona wrote:
> 
> 
> On 4/1/2025 8:57 PM, Konrad Dybcio wrote:
>> On 3/27/25 10:52 AM, Jagadeesh Kona wrote:
>>> 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>
>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@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 818db6ba3b3be99c187512ea4acf2004422f6a18..ad60596b71d25bb0198b26660dc41195a1210a23 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> @@ -4959,7 +4959,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>;
>>
>> So all other DTs touched in this series reference low_svs in required-opps
>>
>> Is that an actual requirement? Otherwise since Commit e3e56c050ab6
>> ("soc: qcom: rpmhpd: Make power_on actually enable the domain") we get the
>> first nonzero state, which can be something like low_svs_d2
>>
> Yes, commit e3e56c050ab6 enables the power-domain at first non-zero level, but in
> some chipsets, the first nonzero state could be retention, which is not sufficient
> for clock controller to operate. So required-opps is needed to ensure the rails are
> at a level above retention for clock controller to operate. low_svs was choosen since
> that is a level that is generally supported across all the chipsets, but low_svs_d2
> may not be supported on some chipsets.
> 
> And required-opps is not mandatory for MXC power domain due to commit f0cc5f7cb43f
> (pmdomain: qcom: rpmhpd: Skip retention level for Power Domains), which ensures MXC
> always gets enabled above retention level. But it was added to make number of
> required-opps uniform with the number of power domains based on discussion at [1].
> 
> [1]: https://lore.kernel.org/all/eoqqz5hyyq6ej5uo6phijbeu5qafbpfxlnreyzzcyfw23pl2yq@ftxnasc6sr2t/#t

Alright, thanks for the explanation!

Konrad

[PATCH v3 16/18] arm64: dts: qcom: Add MXC power domain to camcc node on SM8450

[Jagadeesh Kona]

Camcc requires both MMCX and MXC rails to be powered ON to configure
the camera PLLs on SM8450 platform. Hence add MXC power domain to
camcc node on SM8450.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 36a67c679fbaed944d7590528b696635c306da5d..624190c07c59f3e6714f296f1b264d2a88135116 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -3292,8 +3292,10 @@ camcc: clock-controller@ade0000 {
 				 <&rpmhcc RPMH_CXO_CLK>,
 				 <&rpmhcc RPMH_CXO_CLK_A>,
 				 <&sleep_clk>;
-			power-domains = <&rpmhpd RPMHPD_MMCX>;
-			required-opps = <&rpmhpd_opp_low_svs>;
+			power-domains = <&rpmhpd RPMHPD_MMCX>,
+					<&rpmhpd RPMHPD_MXC>;
+			required-opps = <&rpmhpd_opp_low_svs>,
+					<&rpmhpd_opp_low_svs>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;

-- 
2.34.1

Re: [PATCH v3 16/18] arm64: dts: qcom: Add MXC power domain to camcc node on SM8450

[Bryan O'Donoghue]

Your patch titles are in some places missing the soc:

arm64: dts: qcom: In this patch

arm64: dts: qcom: sm8550: In the next patch

Please add the SoC names to the patch titles in your next version

---
bod

Re: [PATCH v3 16/18] arm64: dts: qcom: Add MXC power domain to camcc node on SM8450

[Jagadeesh Kona]

On 3/27/2025 9:33 PM, Bryan O'Donoghue wrote:
> 
> Your patch titles are in some places missing the soc:
> 
> arm64: dts: qcom: In this patch
> 
> arm64: dts: qcom: sm8550: In the next patch
> 
> Please add the SoC names to the patch titles in your next version
> 

Yes, I will add them and keep all commits to be uniform in next series.

Thanks,
Jagadeesh

> ---
> bod
> 

[PATCH v3 17/18] arm64: dts: qcom: sm8550: Additionally manage MXC power domain in camcc

[Jagadeesh Kona]

From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

Camcc requires both MMCX and MXC rails to be powered ON to configure
the camera PLLs on SM8550 platform. Hence add MXC power domain to
camcc node on SM8550. While at it, update SM8550_MMCX macro to RPMHPD_MMCX
to align towards common macros.

Fixes: e271b59e39a6f ("arm64: dts: qcom: sm8550: Add camera clock controller")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
Reviewed-by: Taniya Das <quic_tdas@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 92017caedbbbea12eb2e43f2e9f5bcad0c0ee40c..e9bb077aa9f0b8be28608d4a0345aae7df8cd167 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -3333,8 +3333,10 @@ camcc: clock-controller@ade0000 {
 				 <&bi_tcxo_div2>,
 				 <&bi_tcxo_ao_div2>,
 				 <&sleep_clk>;
-			power-domains = <&rpmhpd SM8550_MMCX>;
-			required-opps = <&rpmhpd_opp_low_svs>;
+			power-domains = <&rpmhpd RPMHPD_MMCX>,
+					<&rpmhpd RPMHPD_MXC>;
+			required-opps = <&rpmhpd_opp_low_svs>,
+					<&rpmhpd_opp_low_svs>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;

-- 
2.34.1

[PATCH v3 18/18] arm64: dts: qcom: Add MXC power domain to camcc node on SM8650

[Jagadeesh Kona]

Camcc requires both MMCX and MXC rails to be powered ON to configure
the camera PLLs on SM8650 platform. Hence add MXC power domain to
camcc node on SM8650.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 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 ad60596b71d25bb0198b26660dc41195a1210a23..a2b3d97abc7f799810e20131d7231608c8757859 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -5072,7 +5072,8 @@ camcc: clock-controller@ade0000 {
 				 <&bi_tcxo_div2>,
 				 <&bi_tcxo_ao_div2>,
 				 <&sleep_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 v3 00/18] clk: qcom: Add support to attach multiple power domains in cc probe

[Rob Herring (Arm)]

On Thu, 27 Mar 2025 15:22:20 +0530, Jagadeesh Kona wrote:
> In recent QCOM 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 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/
> 
> This series is dependent on bindings patch in below Vladimir's series, hence
> included the Vladimir's series patches also in this series and updated them.
> https://lore.kernel.org/all/20250303225521.1780611-1-vladimir.zapolskiy@linaro.org/
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
> Changes in v3:
>  - Updated the videocc bindings patch to add required-opps for MXC power domain [Dmitry]
>    and added Bryan & Rob R/A-By tags received for this patch on v1.
>  - Included the Vladimir's bindings patch for SM8450 camcc bindings to
>    add multiple PD support and updated them to fix the bot warnings.
>  - Moved SC8280XP camcc bindings to SA8775P camcc since SC8280XP only
>    require single MMCX power domain
>  - Split runtime PM and PLL configuration to separate patches [Dmitry]
>  - Removed direct regmap_update_bits to configure clock CBCR's and
>    using clock helpers to configure the CBCR registers [Dmitry, Bryan]
>  - Added new helpers to configure all PLLs & update misc clock
>    register settings from common code [Dmitry, Bryan]
>  - Updated the name of qcom_clk_cfg structure to qcom_clk_reg_setting [Konrad]
>  - Updated the fields in structure from unsigned int to u32 and added
>    val field to this structure [Konrad]
>  - Added a new u32 array for cbcr branch clocks & num_clk_cbcrs fields
>    to maintain the list of critical clock cbcrs in clock controller
>    descriptor [Konrad]
>  - Updated the plls field to alpha_plls in descriptor structure [Konrad]
>  - Added WARN() in PLL configure function if PLL type passed is not
>    supported. The suggestion is to use BUG(), but updated it to
>    WARN() to avoid checkpatch warning. [Bjorn]
>  - Moved the pll configure and helper macros to PLL code from common code [Bjorn]
>  - Updated camcc drivers for SM8450, SM8550, SM8650 and X1E80100 targets
>    with support to configure PLLs from common code and added MXC power
>    domain in corresponding camcc DT nodes. [Bryan]
>  - Added Dmitry and Bryan R-By tags received on videocc DT node changes in v1
>  - Link to v2: https://lore.kernel.org/r/20250306-videocc-pll-multi-pd-voting-v2-0-0cd00612bc0e@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 (15):
>       dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain
>       dt-bindings: clock: qcom: Update sc8280xp camcc bindings
>       clk: qcom: common: Handle runtime power management in qcom_cc_really_probe
>       clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe
>       clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe
>       clk: qcom: videocc-sm8550: Move PLL & clk configuration to really probe
>       clk: qcom: camcc-sm8450: Move PLL & clk configuration to really probe
>       clk: qcom: camcc-sm8550: Move PLL & clk configuration to really probe
>       clk: qcom: camcc-sm8650: Move PLL & clk configuration to really probe
>       clk: qcom: camcc-x1e80100: 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
>       arm64: dts: qcom: Add MXC power domain to camcc node on SM8450
>       arm64: dts: qcom: Add MXC power domain to camcc node on SM8650
> 
> Taniya Das (1):
>       clk: qcom: clk-alpha-pll: Add support for common PLL configuration function
> 
> Vladimir Zapolskiy (2):
>       dt-bindings: clock: qcom: sm8450-camcc: Allow to specify two power domains
>       arm64: dts: qcom: sm8550: Additionally manage MXC power domain in camcc
> 
>  .../bindings/clock/qcom,sa8775p-camcc.yaml         |  2 +
>  .../bindings/clock/qcom,sm8450-camcc.yaml          | 20 +++--
>  .../bindings/clock/qcom,sm8450-videocc.yaml        | 18 +++--
>  arch/arm64/boot/dts/qcom/sm8450.dtsi               | 12 ++-
>  arch/arm64/boot/dts/qcom/sm8550.dtsi               | 12 ++-
>  arch/arm64/boot/dts/qcom/sm8650.dtsi               |  6 +-
>  drivers/clk/qcom/camcc-sm8450.c                    | 85 ++++++++++------------
>  drivers/clk/qcom/camcc-sm8550.c                    | 81 ++++++++++-----------
>  drivers/clk/qcom/camcc-sm8650.c                    | 79 ++++++++++----------
>  drivers/clk/qcom/camcc-x1e80100.c                  | 63 +++++++---------
>  drivers/clk/qcom/clk-alpha-pll.c                   | 63 ++++++++++++++++
>  drivers/clk/qcom/clk-alpha-pll.h                   |  3 +
>  drivers/clk/qcom/common.c                          | 65 ++++++++++++++---
>  drivers/clk/qcom/common.h                          | 20 +++++
>  drivers/clk/qcom/videocc-sm8450.c                  | 54 ++++++--------
>  drivers/clk/qcom/videocc-sm8550.c                  | 55 ++++++--------
>  16 files changed, 377 insertions(+), 261 deletions(-)
> ---
> base-commit: 138cfc44b3c4a5fb800388c6e27be169970fb9f7
> change-id: 20250218-videocc-pll-multi-pd-voting-d614dce910e7
> 
> Best regards,
> --
> Jagadeesh Kona <quic_jkona@quicinc.com>
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


This patch series was applied (using b4) to base:
 Base: base-commit 138cfc44b3c4a5fb800388c6e27be169970fb9f7 not known, ignoring
 Base: attempting to guess base-commit...
 Base: tags/next-20250327 (exact match)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)

New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/qcom/' for 20250327-videocc-pll-multi-pd-voting-v3-0-895fafd62627@quicinc.com:

arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dtb: clock-controller@ad00000: Unevaluated properties are not allowed ('required-opps' was unexpected)
	from schema $id: http://devicetree.org/schemas/clock/qcom,sa8775p-camcc.yaml#
arch/arm64/boot/dts/qcom/sc8280xp-microsoft-arcata.dtb: clock-controller@ad00000: Unevaluated properties are not allowed ('required-opps' was unexpected)
	from schema $id: http://devicetree.org/schemas/clock/qcom,sa8775p-camcc.yaml#
arch/arm64/boot/dts/qcom/sa8295p-adp.dtb: clock-controller@ad00000: Unevaluated properties are not allowed ('required-opps' was unexpected)
	from schema $id: http://devicetree.org/schemas/clock/qcom,sa8775p-camcc.yaml#
arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dtb: clock-controller@ad00000: Unevaluated properties are not allowed ('required-opps' was unexpected)
	from schema $id: http://devicetree.org/schemas/clock/qcom,sa8775p-camcc.yaml#
arch/arm64/boot/dts/qcom/sc8280xp-microsoft-blackrock.dtb: clock-controller@ad00000: Unevaluated properties are not allowed ('required-opps' was unexpected)
	from schema $id: http://devicetree.org/schemas/clock/qcom,sa8775p-camcc.yaml#
arch/arm64/boot/dts/qcom/sc8280xp-crd.dtb: clock-controller@ad00000: Unevaluated properties are not allowed ('required-opps' was unexpected)
	from schema $id: http://devicetree.org/schemas/clock/qcom,sa8775p-camcc.yaml#
arch/arm64/boot/dts/qcom/sa8540p-ride.dtb: clock-controller@ad00000: Unevaluated properties are not allowed ('required-opps' was unexpected)
	from schema $id: http://devicetree.org/schemas/clock/qcom,sa8775p-camcc.yaml#