| Message ID | 20230131151819.16612-2-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | [v5,1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: specify supported opp tables | expand |
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
diff --git a/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml b/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml index 7c42d9439abd..6f5e7904181f 100644 --- a/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml +++ b/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml @@ -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:
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(-)