[PATCH 0/2] dt-bindings: clock: qcom: Fix SM8450 videocc, camcc power domains support

[PATCH 0/2] dt-bindings: clock: qcom: Fix SM8450 videocc, camcc power domains support

[Jagadeesh Kona]

The SM8450 videocc, camcc bindings are updated to support multiple
power domains in below series

https://lore.kernel.org/all/20250530-videocc-pll-multi-pd-voting-v5-0-02303b3a582d@quicinc.com/

However the bindings has to be compatible with older DTS files that
have only single power domain, hence fix the SM8450 videocc, camcc
bindings to support both single and multiple power domains.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
Jagadeesh Kona (2):
      dt-bindings: clock: qcom,sm8450-videocc: Add minItems property
      dt-bindings: clock: qcom,sm8450-camcc: Add minItems property

 Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml   | 2 ++
 Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 2 ++
 2 files changed, 4 insertions(+)
---
base-commit: 4325743c7e209ae7845293679a4de94b969f2bef
change-id: 20250617-sm8450-videocc-camcc-bindings-single-pd-fix-d9b60a2a0d9d

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

[PATCH 1/2] dt-bindings: clock: qcom,sm8450-videocc: Add minItems property

[Jagadeesh Kona]

Add minItems as 1 for power-domains and required-opps properties
to allow this binding to be compatible with both single and multiple
power domains.

This fixes:

arch/arm64/boot/dts/qcom/sm8450-hdk.dtb: clock-controller@aaf0000
(qcom,sm8450-videocc): power-domains: [[106, 6]] is too short

arch/arm64/boot/dts/qcom/sm8450-hdk.dtb: clock-controller@aaf0000
(qcom,sm8450-videocc): required-opps: [[55]] is too short

Fixes: 1a42f4d4bb92 ("dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506141133.AEQRFOWe-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202506151907.LcLf1RIB-lkp@intel.com/
Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
index 0d99178332cb99d3f02f50605e19b9b26e3ec807..e9143887c1739c28d3cfc97e6678ab12231472a6 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
@@ -32,6 +32,7 @@ properties:
       - description: Video AHB clock from GCC
 
   power-domains:
+    minItems: 1
     description:
       Power domains required for the clock controller to operate
     items:
@@ -39,6 +40,7 @@ properties:
       - description: MXC power domain
 
   required-opps:
+    minItems: 1
     description:
       OPP nodes that describe required performance points on power domains
     items:

-- 
2.34.1

[PATCH 2/2] dt-bindings: clock: qcom,sm8450-camcc: Add minItems property

[Jagadeesh Kona]

Add minItems as 1 for power-domains and required-opps properties
to allow this binding to be compatible with both single and multiple
power domains.

This fixes:

arch/arm64/boot/dts/qcom/sm8450-hdk.dtb: clock-controller@ade0000
(qcom,sm8450-camcc): power-domains: [[106, 6]] is too short

arch/arm64/boot/dts/qcom/sm8450-hdk.dtb: clock-controller@ade0000
(qcom,sm8450-camcc): required-opps: [[55]] is too short

Fixes: a02a8f8cb7f6 ("dt-bindings: clock: qcom,sm8450-camcc: Allow to specify two power domains")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506151907.LcLf1RIB-lkp@intel.com/
Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
index c1e06f39431e68a3cd2f6c2dba84be2a3c143bb1..6a530270e5533977ac11863703156122b93690cd 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
@@ -35,6 +35,7 @@ properties:
       - description: Sleep clock source
 
   power-domains:
+    minItems: 1
     description:
       Power domains required for the clock controller to operate
     items:
@@ -42,6 +43,7 @@ properties:
       - description: MXC power domain
 
   required-opps:
+    minItems: 1
     description:
       OPP nodes that describe required performance points on power domains
     items:

-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: clock: qcom,sm8450-videocc: Add minItems property

[Krzysztof Kozlowski]

On 17/06/2025 21:07, Jagadeesh Kona wrote:
> Add minItems as 1 for power-domains and required-opps properties
> to allow this binding to be compatible with both single and multiple
> power domains.

This is your hardware, so you know how it works thus I expect here
arguments why this is correct from the hardware point of view. Without
this, it is impossible to judge whether this is a correct change.

If I overlook this now, it will be used in discussions by other qcom
engineers, so unfortunately you see, you need to prepare perfect commits
now...

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: clock: qcom,sm8450-videocc: Add minItems property

[Jagadeesh Kona]

On 6/18/2025 11:56 AM, Krzysztof Kozlowski wrote:
> On 17/06/2025 21:07, Jagadeesh Kona wrote:
>> Add minItems as 1 for power-domains and required-opps properties
>> to allow this binding to be compatible with both single and multiple
>> power domains.
> 
> This is your hardware, so you know how it works thus I expect here
> arguments why this is correct from the hardware point of view. Without
> this, it is impossible to judge whether this is a correct change.
> 
> If I overlook this now, it will be used in discussions by other qcom
> engineers, so unfortunately you see, you need to prepare perfect commits
> now...
>

These clk controllers mainly require MMCX power domain to be enabled to access
the clock registers. But to configure the cam & video PLLs in probe, an additional
MXC power domain also needs to be enabled.

Since the initial DTS changes only added MMCX power domain, this change is required
to be backward compatible with older DTS and avoid ABI breakage as discussed in below
thread.

https://lore.kernel.org/all/cc737a89-77e0-43bc-8766-2c8e9cce1863@quicinc.com/#t

Thanks,
Jagadeesh
 
> Best regards,
> Krzysztof

Re: [PATCH 1/2] dt-bindings: clock: qcom,sm8450-videocc: Add minItems property

[Krzysztof Kozlowski]

On 19/06/2025 12:20, Jagadeesh Kona wrote:
> 
> 
> On 6/18/2025 11:56 AM, Krzysztof Kozlowski wrote:
>> On 17/06/2025 21:07, Jagadeesh Kona wrote:
>>> Add minItems as 1 for power-domains and required-opps properties
>>> to allow this binding to be compatible with both single and multiple
>>> power domains.
>>
>> This is your hardware, so you know how it works thus I expect here
>> arguments why this is correct from the hardware point of view. Without
>> this, it is impossible to judge whether this is a correct change.
>>
>> If I overlook this now, it will be used in discussions by other qcom
>> engineers, so unfortunately you see, you need to prepare perfect commits
>> now...
>>
> 
> These clk controllers mainly require MMCX power domain to be enabled to access
> the clock registers. But to configure the cam & video PLLs in probe, an additional
> MXC power domain also needs to be enabled.


Then your patch is not correct. Anyway, you should explain the hardware
in commit msg, why this domain is optional in the hardware.

> 
> Since the initial DTS changes only added MMCX power domain, this change is required
> to be backward compatible with older DTS and avoid ABI breakage as discussed in below
> thread.


So you send incorrect hardware description allowing something which will
not work? Or how exactly?

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: clock: qcom,sm8450-videocc: Add minItems property

[Konrad Dybcio]

On 6/20/25 7:56 AM, Krzysztof Kozlowski wrote:
> On 19/06/2025 12:20, Jagadeesh Kona wrote:
>>
>>
>> On 6/18/2025 11:56 AM, Krzysztof Kozlowski wrote:
>>> On 17/06/2025 21:07, Jagadeesh Kona wrote:
>>>> Add minItems as 1 for power-domains and required-opps properties
>>>> to allow this binding to be compatible with both single and multiple
>>>> power domains.
>>>
>>> This is your hardware, so you know how it works thus I expect here
>>> arguments why this is correct from the hardware point of view. Without
>>> this, it is impossible to judge whether this is a correct change.
>>>
>>> If I overlook this now, it will be used in discussions by other qcom
>>> engineers, so unfortunately you see, you need to prepare perfect commits
>>> now...
>>>
>>
>> These clk controllers mainly require MMCX power domain to be enabled to access
>> the clock registers. But to configure the cam & video PLLs in probe, an additional
>> MXC power domain also needs to be enabled.
> 
> 
> Then your patch is not correct. Anyway, you should explain the hardware
> in commit msg, why this domain is optional in the hardware.
> 
>>
>> Since the initial DTS changes only added MMCX power domain, this change is required
>> to be backward compatible with older DTS and avoid ABI breakage as discussed in below
>> thread.
> 
> 
> So you send incorrect hardware description allowing something which will
> not work? Or how exactly?

So I think there's a mistake in understanding the backwards compatibility
paradigm here.

There exists a single, objectively correct and represented in hardware,
list of required power-domains and the commit that caused the schema
validation errors was essentially "align YAML with reality" which should
be coupled with an immediate DT update to match and we forget about the
incomplete past

Konrad 

Re: [PATCH 1/2] dt-bindings: clock: qcom,sm8450-videocc: Add minItems property

[Dmitry Baryshkov]

On Fri, Jun 20, 2025 at 07:39:06PM +0200, Konrad Dybcio wrote:
> On 6/20/25 7:56 AM, Krzysztof Kozlowski wrote:
> > On 19/06/2025 12:20, Jagadeesh Kona wrote:
> >>
> >>
> >> On 6/18/2025 11:56 AM, Krzysztof Kozlowski wrote:
> >>> On 17/06/2025 21:07, Jagadeesh Kona wrote:
> >>>> Add minItems as 1 for power-domains and required-opps properties
> >>>> to allow this binding to be compatible with both single and multiple
> >>>> power domains.
> >>>
> >>> This is your hardware, so you know how it works thus I expect here
> >>> arguments why this is correct from the hardware point of view. Without
> >>> this, it is impossible to judge whether this is a correct change.
> >>>
> >>> If I overlook this now, it will be used in discussions by other qcom
> >>> engineers, so unfortunately you see, you need to prepare perfect commits
> >>> now...
> >>>
> >>
> >> These clk controllers mainly require MMCX power domain to be enabled to access
> >> the clock registers. But to configure the cam & video PLLs in probe, an additional
> >> MXC power domain also needs to be enabled.
> > 
> > 
> > Then your patch is not correct. Anyway, you should explain the hardware
> > in commit msg, why this domain is optional in the hardware.
> > 
> >>
> >> Since the initial DTS changes only added MMCX power domain, this change is required
> >> to be backward compatible with older DTS and avoid ABI breakage as discussed in below
> >> thread.
> > 
> > 
> > So you send incorrect hardware description allowing something which will
> > not work? Or how exactly?
> 
> So I think there's a mistake in understanding the backwards compatibility
> paradigm here.
> 
> There exists a single, objectively correct and represented in hardware,
> list of required power-domains and the commit that caused the schema
> validation errors was essentially "align YAML with reality" which should
> be coupled with an immediate DT update to match and we forget about the
> incomplete past

I'd second that. Let's make sure that the _driver_ works with old DT.
But we don't have to support old DT in schema.

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] dt-bindings: clock: qcom,sm8450-videocc: Add minItems property

[Jagadeesh Kona]

On 6/21/2025 3:04 AM, Dmitry Baryshkov wrote:
> On Fri, Jun 20, 2025 at 07:39:06PM +0200, Konrad Dybcio wrote:
>> On 6/20/25 7:56 AM, Krzysztof Kozlowski wrote:
>>> On 19/06/2025 12:20, Jagadeesh Kona wrote:
>>>>
>>>>
>>>> On 6/18/2025 11:56 AM, Krzysztof Kozlowski wrote:
>>>>> On 17/06/2025 21:07, Jagadeesh Kona wrote:
>>>>>> Add minItems as 1 for power-domains and required-opps properties
>>>>>> to allow this binding to be compatible with both single and multiple
>>>>>> power domains.
>>>>>
>>>>> This is your hardware, so you know how it works thus I expect here
>>>>> arguments why this is correct from the hardware point of view. Without
>>>>> this, it is impossible to judge whether this is a correct change.
>>>>>
>>>>> If I overlook this now, it will be used in discussions by other qcom
>>>>> engineers, so unfortunately you see, you need to prepare perfect commits
>>>>> now...
>>>>>
>>>>
>>>> These clk controllers mainly require MMCX power domain to be enabled to access
>>>> the clock registers. But to configure the cam & video PLLs in probe, an additional
>>>> MXC power domain also needs to be enabled.
>>>
>>>
>>> Then your patch is not correct. Anyway, you should explain the hardware
>>> in commit msg, why this domain is optional in the hardware.
>>>
>>>>
>>>> Since the initial DTS changes only added MMCX power domain, this change is required
>>>> to be backward compatible with older DTS and avoid ABI breakage as discussed in below
>>>> thread.
>>>
>>>
>>> So you send incorrect hardware description allowing something which will
>>> not work? Or how exactly?
>>

The initial videocc with single power domain works fine, but in that case the
PLL's work with default sub-optimal configuration settings and below PLL
warning during bootup was reported during video iris testing. 

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

The multi PD support helps to fix the above warning.  


>> So I think there's a mistake in understanding the backwards compatibility
>> paradigm here.
>>
>> There exists a single, objectively correct and represented in hardware,
>> list of required power-domains and the commit that caused the schema
>> validation errors was essentially "align YAML with reality" which should
>> be coupled with an immediate DT update to match and we forget about the
>> incomplete past
> 
> I'd second that. Let's make sure that the _driver_ works with old DT.
> But we don't have to support old DT in schema.
> 

Yes, The driver code is already backward compatible with the older DT.

Thanks,
Jagadeesh