[v5,2/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional
Commit Message
The qcom-cpufreq-nvmem driver supports 2 kind of devices:
- pre-cpr that doesn't have power-domains and base everything on nvmem
cells and multiple named microvolt bindings.
Doesn't need required-opp binding in the opp nodes as they are only
used for genpd based devices.
- cpr-based that require power-domain in the cpu nodes and use various
source to decide the correct voltage and freq
Require required-opp binding since they need to be linked to the
related opp-level.
When the schema was introduced, it was wrongly set to always require these
binding but this is not the case for pre-cpr devices.
Make the power-domain and the required-opp optional and set them required
only for qcs404 based devices.
Fixes: ec24d1d55469 ("dt-bindings: opp: Convert qcom-nvmem-cpufreq to DT schema")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v5:
- Swap patch 1 and patch 2 to fix dt_check_warning on single
Changes v4:
- Explain why required-opp needs to be conditional
- Split additional ref part
Changes v3:
- No change
Changes v2:
- Reword commit description
- Fix condition order
- Add allOf
.../bindings/cpufreq/qcom-cpufreq-nvmem.yaml | 74 +++++++++++--------
1 file changed, 44 insertions(+), 30 deletions(-)
Comments
On 31/01/2023 16:18, Christian Marangi wrote:
> The qcom-cpufreq-nvmem driver supports 2 kind of devices:
> - pre-cpr that doesn't have power-domains and base everything on nvmem
> cells and multiple named microvolt bindings.
> Doesn't need required-opp binding in the opp nodes as they are only
> used for genpd based devices.
> - cpr-based that require power-domain in the cpu nodes and use various
> source to decide the correct voltage and freq
> Require required-opp binding since they need to be linked to the
> related opp-level.
>
> When the schema was introduced, it was wrongly set to always require these
> binding but this is not the case for pre-cpr devices.
>
> Make the power-domain and the required-opp optional and set them required
> only for qcs404 based devices.
>
> Fixes: ec24d1d55469 ("dt-bindings: opp: Convert qcom-nvmem-cpufreq to DT schema")
Fixes go as first patches in the series.
Best regards,
Krzysztof
On Wed, Feb 01, 2023 at 09:20:39AM +0100, Krzysztof Kozlowski wrote:
> On 31/01/2023 16:18, Christian Marangi wrote:
> > The qcom-cpufreq-nvmem driver supports 2 kind of devices:
> > - pre-cpr that doesn't have power-domains and base everything on nvmem
> > cells and multiple named microvolt bindings.
> > Doesn't need required-opp binding in the opp nodes as they are only
> > used for genpd based devices.
> > - cpr-based that require power-domain in the cpu nodes and use various
> > source to decide the correct voltage and freq
> > Require required-opp binding since they need to be linked to the
> > related opp-level.
> >
> > When the schema was introduced, it was wrongly set to always require these
> > binding but this is not the case for pre-cpr devices.
> >
> > Make the power-domain and the required-opp optional and set them required
> > only for qcs404 based devices.
> >
> > Fixes: ec24d1d55469 ("dt-bindings: opp: Convert qcom-nvmem-cpufreq to DT schema")
>
> Fixes go as first patches in the series.
>
Hi,
this is problematic. This documentation is a bit special.
v4 had this patch as first but this cause error with make
dt_binding_check as the schema will be effectively empty (as it will
have only if condition)
This is why I pushed v5 that swap this with the second patch and first
add non conditional stuff to the schema and only with the second patch
makes them conditional.
Any hint to handle this corner case? I'm having some diffiulties due to
how special this is but we really need this fix since it's blocking the
introduction of opp table for ipq806x and ipq807x (as the schema is
currently flawed)
On 08/02/2023 01:09, Christian Marangi wrote:
> On Wed, Feb 01, 2023 at 09:20:39AM +0100, Krzysztof Kozlowski wrote:
>> On 31/01/2023 16:18, Christian Marangi wrote:
>>> The qcom-cpufreq-nvmem driver supports 2 kind of devices:
>>> - pre-cpr that doesn't have power-domains and base everything on nvmem
>>> cells and multiple named microvolt bindings.
>>> Doesn't need required-opp binding in the opp nodes as they are only
>>> used for genpd based devices.
>>> - cpr-based that require power-domain in the cpu nodes and use various
>>> source to decide the correct voltage and freq
>>> Require required-opp binding since they need to be linked to the
>>> related opp-level.
>>>
>>> When the schema was introduced, it was wrongly set to always require these
>>> binding but this is not the case for pre-cpr devices.
>>>
>>> Make the power-domain and the required-opp optional and set them required
>>> only for qcs404 based devices.
>>>
>>> Fixes: ec24d1d55469 ("dt-bindings: opp: Convert qcom-nvmem-cpufreq to DT schema")
>>
>> Fixes go as first patches in the series.
>>
>
> Hi,
> this is problematic. This documentation is a bit special.
>
> v4 had this patch as first but this cause error with make
> dt_binding_check as the schema will be effectively empty (as it will
> have only if condition)
>
> This is why I pushed v5 that swap this with the second patch and first
> add non conditional stuff to the schema and only with the second patch
> makes them conditional.
>
> Any hint to handle this corner case? I'm having some diffiulties due to
> how special this is but we really need this fix since it's blocking the
> introduction of opp table for ipq806x and ipq807x (as the schema is
> currently flawed)
Let's then drop fixes tag, because it will only confuse any backporters.
Best regards,
Krzysztof
@@ -17,6 +17,9 @@ description: |
on the CPU OPP in use. The CPUFreq driver sets the CPR power domain level
according to the required OPPs defined in the CPU OPP tables.
+ For old implementation efuses are parsed to select the correct opp table and
+ voltage and CPR is not supported/used.
+
select:
properties:
compatible:
@@ -33,26 +36,6 @@ select:
required:
- compatible
-properties:
- cpus:
- type: object
-
- patternProperties:
- '^cpu@[0-9a-f]+$':
- type: object
-
- properties:
- power-domains:
- maxItems: 1
-
- power-domain-names:
- items:
- - const: cpr
-
- required:
- - power-domains
- - power-domain-names
-
patternProperties:
'^opp-table(-[a-z0-9]+)?$':
allOf:
@@ -63,16 +46,6 @@ patternProperties:
then:
$ref: /schemas/opp/opp-v2-kryo-cpu.yaml#
- - if:
- properties:
- compatible:
- const: operating-points-v2-kryo-cpu
- then:
- patternProperties:
- '^opp-?[0-9]+$':
- required:
- - required-opps
-
- if:
properties:
compatible:
@@ -82,6 +55,47 @@ patternProperties:
unevaluatedProperties: false
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,qcs404
+
+ then:
+ properties:
+ cpus:
+ type: object
+
+ patternProperties:
+ '^cpu@[0-9a-f]+$':
+ type: object
+
+ properties:
+ power-domains:
+ maxItems: 1
+
+ power-domain-names:
+ items:
+ - const: cpr
+
+ required:
+ - power-domains
+ - power-domain-names
+
+ patternProperties:
+ '^opp-table(-[a-z0-9]+)?$':
+ if:
+ properties:
+ compatible:
+ const: operating-points-v2-kryo-cpu
+ then:
+ patternProperties:
+ '^opp-?[0-9]+$':
+ required:
+ - required-opps
+
additionalProperties: true
examples: