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.1On 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
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
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
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
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
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
© 2016 - 2025 Red Hat, Inc.