* [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional @ 2023-01-21 0:01 Christian Marangi 2023-01-21 0:01 ` [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based Christian Marangi ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Christian Marangi @ 2023-01-21 0:01 UTC (permalink / raw) To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel Cc: Christian Marangi The qcom-cpufreq-nvmem driver also supports legacy devices pre-cpr that doesn't have power-domains. When the schema was introduced, it was wrongly set to always require these binding but this is not the case for legacy device that base everything on nvmem cells and multiple microvolt binding providing values based on speedbin, psv and version. Make the power-domain 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> --- .../bindings/cpufreq/qcom-cpufreq-nvmem.yaml | 78 +++++++++++++------ 1 file changed, 54 insertions(+), 24 deletions(-) diff --git a/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml b/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml index 9c086eac6ca7..04aac634d7fc 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,37 +36,64 @@ select: required: - compatible -properties: - cpus: - type: object +if: + properties: + compatible: + contains: + enum: + - qcom,qcs404 - patternProperties: - '^cpu@[0-9a-f]+$': - type: object +then: + properties: + cpus: + type: object - properties: - power-domains: - maxItems: 1 + patternProperties: + '^cpu@[0-9a-f]+$': + type: object + + properties: + power-domains: + maxItems: 1 - power-domain-names: - items: - - const: cpr + power-domain-names: + items: + - const: cpr - required: - - power-domains - - power-domain-names + 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 patternProperties: '^opp-table(-[a-z0-9]+)?$': - if: - properties: - compatible: - const: operating-points-v2-kryo-cpu - then: - patternProperties: - '^opp-?[0-9]+$': - required: - - required-opps + allOf: + - if: + properties: + compatible: + const: operating-points-v2-kryo-cpu + then: + $ref: /schemas/opp/opp-v2-kryo-cpu.yaml# + + - if: + properties: + compatible: + const: operating-points-v2-qcom-level + then: + $ref: /schemas/opp/opp-v2-qcom-level.yaml# + + unevaluatedProperties: false additionalProperties: true -- 2.38.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based 2023-01-21 0:01 [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional Christian Marangi @ 2023-01-21 0:01 ` Christian Marangi 2023-01-22 14:00 ` Krzysztof Kozlowski 2023-01-21 0:01 ` [PATCH 3/3] dt-bindings: opp: opp-v2-kryo-cpu: enlarge opp-supported-hw maximum Christian Marangi 2023-01-22 13:57 ` [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional Krzysztof Kozlowski 2 siblings, 1 reply; 13+ messages in thread From: Christian Marangi @ 2023-01-21 0:01 UTC (permalink / raw) To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel Cc: Christian Marangi The operating-points-v2-kryo-cpu driver supports defining multiple opp-microvolt based on the blown efuses in the soc. It consist of 3 values that are parsed: speedbin, psv and version. They are all appended to the opp-microvolt name and selected by the nvmem driver and loaded dynamically at runtime. Example: opp-microvolt-speed0-pvs0-v0 = <1050000 997500 1102500>; opp-microvolt-speed0-pvs1-v0 = <975000 926250 1023750>; opp-microvolt-speed0-pvs2-v0 = <925000 878750 971250>; opp-microvolt-speed0-pvs3-v0 = <850000 807500 892500>; Add support for this and reject these special binding if we don't have a nvmem-cell to read data from. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- .../devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml index b4947b326773..cea932339faf 100644 --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml @@ -61,6 +61,17 @@ patternProperties: required-opps: true + patternProperties: + '^opp-microvolt-speed[0-9]-pvs[0-9]-v[0-9]$': + description: | + Assign a microvolt value to the opp hz based on the efuses value from + speedbin, pvs and version value read from the provided nvmem cell. + minItems: 1 + maxItems: 8 # Should be enough regulators + items: + minItems: 1 + maxItems: 3 + required: - opp-hz @@ -75,6 +86,11 @@ then: '^opp-?[0-9]+$': required: - opp-supported-hw +else: + patternProperties: + '^opp-?[0-9]+$': + patternProperties: + '^opp-microvolt-speed[0-9]-pvs[0-9]-v[0-9]$': false additionalProperties: false -- 2.38.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based 2023-01-21 0:01 ` [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based Christian Marangi @ 2023-01-22 14:00 ` Krzysztof Kozlowski 2023-01-22 14:15 ` Christian Marangi 0 siblings, 1 reply; 13+ messages in thread From: Krzysztof Kozlowski @ 2023-01-22 14:00 UTC (permalink / raw) To: Christian Marangi, Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On 21/01/2023 01:01, Christian Marangi wrote: > The operating-points-v2-kryo-cpu driver supports defining multiple > opp-microvolt based on the blown efuses in the soc. It consist of 3 > values that are parsed: speedbin, psv and version. They are all > appended to the opp-microvolt name and selected by the nvmem driver and > loaded dynamically at runtime. > > Example: > > opp-microvolt-speed0-pvs0-v0 = <1050000 997500 1102500>; > opp-microvolt-speed0-pvs1-v0 = <975000 926250 1023750>; > opp-microvolt-speed0-pvs2-v0 = <925000 878750 971250>; > opp-microvolt-speed0-pvs3-v0 = <850000 807500 892500>; > > Add support for this and reject these special binding if we don't have a > nvmem-cell to read data from. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > .../devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > index b4947b326773..cea932339faf 100644 > --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > @@ -61,6 +61,17 @@ patternProperties: > > required-opps: true > > + patternProperties: > + '^opp-microvolt-speed[0-9]-pvs[0-9]-v[0-9]$': This does not end with correct unit suffix. Should be opp-speed-.....-microvolt > + description: | > + Assign a microvolt value to the opp hz based on the efuses value from > + speedbin, pvs and vers Where is the DTS change? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based 2023-01-22 14:00 ` Krzysztof Kozlowski @ 2023-01-22 14:15 ` Christian Marangi 2023-01-22 14:17 ` Krzysztof Kozlowski 0 siblings, 1 reply; 13+ messages in thread From: Christian Marangi @ 2023-01-22 14:15 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On Sun, Jan 22, 2023 at 03:00:22PM +0100, Krzysztof Kozlowski wrote: > On 21/01/2023 01:01, Christian Marangi wrote: > > The operating-points-v2-kryo-cpu driver supports defining multiple > > opp-microvolt based on the blown efuses in the soc. It consist of 3 > > values that are parsed: speedbin, psv and version. They are all > > appended to the opp-microvolt name and selected by the nvmem driver and > > loaded dynamically at runtime. > > > > Example: > > > > opp-microvolt-speed0-pvs0-v0 = <1050000 997500 1102500>; > > opp-microvolt-speed0-pvs1-v0 = <975000 926250 1023750>; > > opp-microvolt-speed0-pvs2-v0 = <925000 878750 971250>; > > opp-microvolt-speed0-pvs3-v0 = <850000 807500 892500>; > > > > Add support for this and reject these special binding if we don't have a > > nvmem-cell to read data from. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > .../devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > > index b4947b326773..cea932339faf 100644 > > --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > > +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > > @@ -61,6 +61,17 @@ patternProperties: > > > > required-opps: true > > > > + patternProperties: > > + '^opp-microvolt-speed[0-9]-pvs[0-9]-v[0-9]$': > > This does not end with correct unit suffix. Should be > opp-speed-.....-microvolt > I think I didn't understand this? From opp-v2-base and from what we are using downstream, the named opp-micrvolt works correctly. (speed[0-9]-pvs[0-9]-v[0-9] is the entire name of the named opp-microvolt- binding) This is the reference I always used for the pattern. [1] Here the pattern used by the driver. [2] [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/opp/opp-v2-base.yaml#L209 [2] https://elixir.bootlin.com/linux/latest/source/drivers/cpufreq/qcom-cpufreq-nvmem.c#L238 > > + description: | > > + Assign a microvolt value to the opp hz based on the efuses value from > > + speedbin, pvs and vers > > Where is the DTS change? You mean an additional example that use this additional binding? This may be difficult to add since the current example in this schema is a root one and I can't put multiple root example. Is it acceptable to add a dummy opp table with some comments explaining the dummy node is not supported in the current compatible? (apq8096) > > Best regards, > Krzysztof > -- Ansuel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based 2023-01-22 14:15 ` Christian Marangi @ 2023-01-22 14:17 ` Krzysztof Kozlowski 2023-01-22 14:21 ` Christian Marangi 0 siblings, 1 reply; 13+ messages in thread From: Krzysztof Kozlowski @ 2023-01-22 14:17 UTC (permalink / raw) To: Christian Marangi Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On 22/01/2023 15:15, Christian Marangi wrote: > On Sun, Jan 22, 2023 at 03:00:22PM +0100, Krzysztof Kozlowski wrote: >> On 21/01/2023 01:01, Christian Marangi wrote: >>> The operating-points-v2-kryo-cpu driver supports defining multiple >>> opp-microvolt based on the blown efuses in the soc. It consist of 3 >>> values that are parsed: speedbin, psv and version. They are all >>> appended to the opp-microvolt name and selected by the nvmem driver and >>> loaded dynamically at runtime. >>> >>> Example: >>> >>> opp-microvolt-speed0-pvs0-v0 = <1050000 997500 1102500>; >>> opp-microvolt-speed0-pvs1-v0 = <975000 926250 1023750>; >>> opp-microvolt-speed0-pvs2-v0 = <925000 878750 971250>; >>> opp-microvolt-speed0-pvs3-v0 = <850000 807500 892500>; >>> >>> Add support for this and reject these special binding if we don't have a >>> nvmem-cell to read data from. >>> >>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> >>> --- >>> .../devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml >>> index b4947b326773..cea932339faf 100644 >>> --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml >>> +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml >>> @@ -61,6 +61,17 @@ patternProperties: >>> >>> required-opps: true >>> >>> + patternProperties: >>> + '^opp-microvolt-speed[0-9]-pvs[0-9]-v[0-9]$': >> >> This does not end with correct unit suffix. Should be >> opp-speed-.....-microvolt >> > > I think I didn't understand this? > > From opp-v2-base and from what we are using downstream, the named > opp-micrvolt works correctly. > > (speed[0-9]-pvs[0-9]-v[0-9] is the entire name of the named > opp-microvolt- binding) > > This is the reference I always used for the pattern. [1] > Here the pattern used by the driver. [2] > > [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/opp/opp-v2-base.yaml#L209 > [2] https://elixir.bootlin.com/linux/latest/source/drivers/cpufreq/qcom-cpufreq-nvmem.c#L238 Are you documenting existing property or adding new? Commit msg suggests you add new property, so what do you reference here? How is it related? > >>> + description: | >>> + Assign a microvolt value to the opp hz based on the efuses value from >>> + speedbin, pvs and vers >> >> Where is the DTS change? > > You mean an additional example that use this additional binding? This > may be difficult to add since the current example in this schema is a > root one and I can't put multiple root example. No, I mean, you DTS using it. We do not want empty (unused) bindings... Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based 2023-01-22 14:17 ` Krzysztof Kozlowski @ 2023-01-22 14:21 ` Christian Marangi 2023-01-22 14:31 ` Krzysztof Kozlowski 0 siblings, 1 reply; 13+ messages in thread From: Christian Marangi @ 2023-01-22 14:21 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On Sun, Jan 22, 2023 at 03:17:54PM +0100, Krzysztof Kozlowski wrote: > On 22/01/2023 15:15, Christian Marangi wrote: > > On Sun, Jan 22, 2023 at 03:00:22PM +0100, Krzysztof Kozlowski wrote: > >> On 21/01/2023 01:01, Christian Marangi wrote: > >>> The operating-points-v2-kryo-cpu driver supports defining multiple > >>> opp-microvolt based on the blown efuses in the soc. It consist of 3 > >>> values that are parsed: speedbin, psv and version. They are all > >>> appended to the opp-microvolt name and selected by the nvmem driver and > >>> loaded dynamically at runtime. > >>> > >>> Example: > >>> > >>> opp-microvolt-speed0-pvs0-v0 = <1050000 997500 1102500>; > >>> opp-microvolt-speed0-pvs1-v0 = <975000 926250 1023750>; > >>> opp-microvolt-speed0-pvs2-v0 = <925000 878750 971250>; > >>> opp-microvolt-speed0-pvs3-v0 = <850000 807500 892500>; > >>> > >>> Add support for this and reject these special binding if we don't have a > >>> nvmem-cell to read data from. > >>> > >>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > >>> --- > >>> .../devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 16 ++++++++++++++++ > >>> 1 file changed, 16 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > >>> index b4947b326773..cea932339faf 100644 > >>> --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > >>> +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > >>> @@ -61,6 +61,17 @@ patternProperties: > >>> > >>> required-opps: true > >>> > >>> + patternProperties: > >>> + '^opp-microvolt-speed[0-9]-pvs[0-9]-v[0-9]$': > >> > >> This does not end with correct unit suffix. Should be > >> opp-speed-.....-microvolt > >> > > > > I think I didn't understand this? > > > > From opp-v2-base and from what we are using downstream, the named > > opp-micrvolt works correctly. > > > > (speed[0-9]-pvs[0-9]-v[0-9] is the entire name of the named > > opp-microvolt- binding) > > > > This is the reference I always used for the pattern. [1] > > Here the pattern used by the driver. [2] > > > > [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/opp/opp-v2-base.yaml#L209 > > [2] https://elixir.bootlin.com/linux/latest/source/drivers/cpufreq/qcom-cpufreq-nvmem.c#L238 > > Are you documenting existing property or adding new? Commit msg suggests > you add new property, so what do you reference here? How is it related? > It should have been added from the start when the schema was submitted but I get what you mean with the other question. > > > >>> + description: | > >>> + Assign a microvolt value to the opp hz based on the efuses value from > >>> + speedbin, pvs and vers > >> > >> Where is the DTS change? > > > > You mean an additional example that use this additional binding? This > > may be difficult to add since the current example in this schema is a > > root one and I can't put multiple root example. > > No, I mean, you DTS using it. We do not want empty (unused) bindings... > Ok, will drop this and make it part of the ipq8064 opp series that will use this binding. -- Ansuel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based 2023-01-22 14:21 ` Christian Marangi @ 2023-01-22 14:31 ` Krzysztof Kozlowski 2023-01-22 14:35 ` Christian Marangi 0 siblings, 1 reply; 13+ messages in thread From: Krzysztof Kozlowski @ 2023-01-22 14:31 UTC (permalink / raw) To: Christian Marangi Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On 22/01/2023 15:21, Christian Marangi wrote: > On Sun, Jan 22, 2023 at 03:17:54PM +0100, Krzysztof Kozlowski wrote: >> On 22/01/2023 15:15, Christian Marangi wrote: >>> On Sun, Jan 22, 2023 at 03:00:22PM +0100, Krzysztof Kozlowski wrote: >>>> On 21/01/2023 01:01, Christian Marangi wrote: >>>>> The operating-points-v2-kryo-cpu driver supports defining multiple >>>>> opp-microvolt based on the blown efuses in the soc. It consist of 3 >>>>> values that are parsed: speedbin, psv and version. They are all >>>>> appended to the opp-microvolt name and selected by the nvmem driver and >>>>> loaded dynamically at runtime. >>>>> >>>>> Example: >>>>> >>>>> opp-microvolt-speed0-pvs0-v0 = <1050000 997500 1102500>; >>>>> opp-microvolt-speed0-pvs1-v0 = <975000 926250 1023750>; >>>>> opp-microvolt-speed0-pvs2-v0 = <925000 878750 971250>; >>>>> opp-microvolt-speed0-pvs3-v0 = <850000 807500 892500>; >>>>> >>>>> Add support for this and reject these special binding if we don't have a >>>>> nvmem-cell to read data from. >>>>> >>>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> >>>>> --- >>>>> .../devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 16 ++++++++++++++++ >>>>> 1 file changed, 16 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml >>>>> index b4947b326773..cea932339faf 100644 >>>>> --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml >>>>> +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml >>>>> @@ -61,6 +61,17 @@ patternProperties: >>>>> >>>>> required-opps: true >>>>> >>>>> + patternProperties: >>>>> + '^opp-microvolt-speed[0-9]-pvs[0-9]-v[0-9]$': >>>> >>>> This does not end with correct unit suffix. Should be >>>> opp-speed-.....-microvolt >>>> >>> >>> I think I didn't understand this? >>> >>> From opp-v2-base and from what we are using downstream, the named >>> opp-micrvolt works correctly. >>> >>> (speed[0-9]-pvs[0-9]-v[0-9] is the entire name of the named >>> opp-microvolt- binding) >>> >>> This is the reference I always used for the pattern. [1] >>> Here the pattern used by the driver. [2] >>> >>> [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/opp/opp-v2-base.yaml#L209 >>> [2] https://elixir.bootlin.com/linux/latest/source/drivers/cpufreq/qcom-cpufreq-nvmem.c#L238 >> >> Are you documenting existing property or adding new? Commit msg suggests >> you add new property, so what do you reference here? How is it related? >> > > It should have been added from the start when the schema was submitted > but I get what you mean with the other question. > >>> >>>>> + description: | >>>>> + Assign a microvolt value to the opp hz based on the efuses value from >>>>> + speedbin, pvs and vers >>>> >>>> Where is the DTS change? >>> >>> You mean an additional example that use this additional binding? This >>> may be difficult to add since the current example in this schema is a >>> root one and I can't put multiple root example. >> >> No, I mean, you DTS using it. We do not want empty (unused) bindings... >> > > Ok, will drop this and make it part of the ipq8064 opp series that will use > this binding. You can also link the DTS changes, it's also fine. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based 2023-01-22 14:31 ` Krzysztof Kozlowski @ 2023-01-22 14:35 ` Christian Marangi 0 siblings, 0 replies; 13+ messages in thread From: Christian Marangi @ 2023-01-22 14:35 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On Sun, Jan 22, 2023 at 03:31:14PM +0100, Krzysztof Kozlowski wrote: > On 22/01/2023 15:21, Christian Marangi wrote: > > On Sun, Jan 22, 2023 at 03:17:54PM +0100, Krzysztof Kozlowski wrote: > >> On 22/01/2023 15:15, Christian Marangi wrote: > >>> On Sun, Jan 22, 2023 at 03:00:22PM +0100, Krzysztof Kozlowski wrote: > >>>> On 21/01/2023 01:01, Christian Marangi wrote: > >>>>> The operating-points-v2-kryo-cpu driver supports defining multiple > >>>>> opp-microvolt based on the blown efuses in the soc. It consist of 3 > >>>>> values that are parsed: speedbin, psv and version. They are all > >>>>> appended to the opp-microvolt name and selected by the nvmem driver and > >>>>> loaded dynamically at runtime. > >>>>> > >>>>> Example: > >>>>> > >>>>> opp-microvolt-speed0-pvs0-v0 = <1050000 997500 1102500>; > >>>>> opp-microvolt-speed0-pvs1-v0 = <975000 926250 1023750>; > >>>>> opp-microvolt-speed0-pvs2-v0 = <925000 878750 971250>; > >>>>> opp-microvolt-speed0-pvs3-v0 = <850000 807500 892500>; > >>>>> > >>>>> Add support for this and reject these special binding if we don't have a > >>>>> nvmem-cell to read data from. > >>>>> > >>>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > >>>>> --- > >>>>> .../devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 16 ++++++++++++++++ > >>>>> 1 file changed, 16 insertions(+) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > >>>>> index b4947b326773..cea932339faf 100644 > >>>>> --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > >>>>> +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > >>>>> @@ -61,6 +61,17 @@ patternProperties: > >>>>> > >>>>> required-opps: true > >>>>> > >>>>> + patternProperties: > >>>>> + '^opp-microvolt-speed[0-9]-pvs[0-9]-v[0-9]$': > >>>> > >>>> This does not end with correct unit suffix. Should be > >>>> opp-speed-.....-microvolt > >>>> > >>> > >>> I think I didn't understand this? > >>> > >>> From opp-v2-base and from what we are using downstream, the named > >>> opp-micrvolt works correctly. > >>> > >>> (speed[0-9]-pvs[0-9]-v[0-9] is the entire name of the named > >>> opp-microvolt- binding) > >>> > >>> This is the reference I always used for the pattern. [1] > >>> Here the pattern used by the driver. [2] > >>> > >>> [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/opp/opp-v2-base.yaml#L209 > >>> [2] https://elixir.bootlin.com/linux/latest/source/drivers/cpufreq/qcom-cpufreq-nvmem.c#L238 > >> > >> Are you documenting existing property or adding new? Commit msg suggests > >> you add new property, so what do you reference here? How is it related? > >> > > > > It should have been added from the start when the schema was submitted > > but I get what you mean with the other question. > > > >>> > >>>>> + description: | > >>>>> + Assign a microvolt value to the opp hz based on the efuses value from > >>>>> + speedbin, pvs and vers > >>>> > >>>> Where is the DTS change? > >>> > >>> You mean an additional example that use this additional binding? This > >>> may be difficult to add since the current example in this schema is a > >>> root one and I can't put multiple root example. > >> > >> No, I mean, you DTS using it. We do not want empty (unused) bindings... > >> > > > > Ok, will drop this and make it part of the ipq8064 opp series that will use > > this binding. > > You can also link the DTS changes, it's also fine. > There are some required changes to do on the driver so the opp series still have to be submitted so I guess dropping this is the only way currently. But it's not a problem the real important patch in the series is the first one. -- Ansuel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] dt-bindings: opp: opp-v2-kryo-cpu: enlarge opp-supported-hw maximum 2023-01-21 0:01 [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional Christian Marangi 2023-01-21 0:01 ` [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based Christian Marangi @ 2023-01-21 0:01 ` Christian Marangi 2023-01-22 14:01 ` Krzysztof Kozlowski 2023-01-22 13:57 ` [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional Krzysztof Kozlowski 2 siblings, 1 reply; 13+ messages in thread From: Christian Marangi @ 2023-01-21 0:01 UTC (permalink / raw) To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel Cc: Christian Marangi Enlarge opp-supported-hw maximum value. In recent SoC we started matching more bit and we currently match mask of 112. The old maximum of 7 was good for old SoC that didn't had complex id, but now this is limiting and we need to enlarge it to support more variants. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml index cea932339faf..b4ebaf68b43e 100644 --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml @@ -55,7 +55,7 @@ patternProperties: 1: MSM8996, speedbin 1 2: MSM8996, speedbin 2 3-31: unused - maximum: 0x7 + maximum: 0xff clock-latency-ns: true -- 2.38.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] dt-bindings: opp: opp-v2-kryo-cpu: enlarge opp-supported-hw maximum 2023-01-21 0:01 ` [PATCH 3/3] dt-bindings: opp: opp-v2-kryo-cpu: enlarge opp-supported-hw maximum Christian Marangi @ 2023-01-22 14:01 ` Krzysztof Kozlowski 0 siblings, 0 replies; 13+ messages in thread From: Krzysztof Kozlowski @ 2023-01-22 14:01 UTC (permalink / raw) To: Christian Marangi, Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On 21/01/2023 01:01, Christian Marangi wrote: > Enlarge opp-supported-hw maximum value. In recent SoC we started > matching more bit and we currently match mask of 112. The old maximum of > 7 was good for old SoC that didn't had complex id, but now this is > limiting and we need to enlarge it to support more variants. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > index cea932339faf..b4ebaf68b43e 100644 > --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml > @@ -55,7 +55,7 @@ patternProperties: > 1: MSM8996, speedbin 1 > 2: MSM8996, speedbin 2 Document more bits. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional 2023-01-21 0:01 [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional Christian Marangi 2023-01-21 0:01 ` [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based Christian Marangi 2023-01-21 0:01 ` [PATCH 3/3] dt-bindings: opp: opp-v2-kryo-cpu: enlarge opp-supported-hw maximum Christian Marangi @ 2023-01-22 13:57 ` Krzysztof Kozlowski 2023-01-22 14:05 ` Christian Marangi 2 siblings, 1 reply; 13+ messages in thread From: Krzysztof Kozlowski @ 2023-01-22 13:57 UTC (permalink / raw) To: Christian Marangi, Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On 21/01/2023 01:01, Christian Marangi wrote: > The qcom-cpufreq-nvmem driver also supports legacy devices pre-cpr that > doesn't have power-domains. When the schema was introduced, it was > wrongly set to always require these binding but this is not the case for > legacy device that base everything on nvmem cells and multiple microvolt What is a "legacy device"? Why do you adjust bindings to legacy device? Can't you just fix the DTS on these devices? > binding providing values based on speedbin, psv and version. > > Make the power-domain 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> > --- > .../bindings/cpufreq/qcom-cpufreq-nvmem.yaml | 78 +++++++++++++------ > 1 file changed, 54 insertions(+), 24 deletions(-) > > diff --git a/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml b/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml > index 9c086eac6ca7..04aac634d7fc 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,37 +36,64 @@ select: > required: > - compatible > > -properties: > - cpus: > - type: object > +if: Missing allOf. Fix also the placement - this block goes before additionalPropertes:false. > + properties: > + compatible: > + contains: > + enum: > + - qcom,qcs404 > > - patternProperties: > - '^cpu@[0-9a-f]+$': > - type: object > +then: > + properties: > + cpus: > + type: object > > - properties: > - power-domains: Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional 2023-01-22 13:57 ` [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional Krzysztof Kozlowski @ 2023-01-22 14:05 ` Christian Marangi 2023-01-22 14:13 ` Krzysztof Kozlowski 0 siblings, 1 reply; 13+ messages in thread From: Christian Marangi @ 2023-01-22 14:05 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On Sun, Jan 22, 2023 at 02:57:07PM +0100, Krzysztof Kozlowski wrote: > On 21/01/2023 01:01, Christian Marangi wrote: > > The qcom-cpufreq-nvmem driver also supports legacy devices pre-cpr that > > doesn't have power-domains. When the schema was introduced, it was > > wrongly set to always require these binding but this is not the case for > > legacy device that base everything on nvmem cells and multiple microvolt > > What is a "legacy device"? Why do you adjust bindings to legacy device? > Can't you just fix the DTS on these devices? > With legacy I mean device where cpr (core power reduction) wasn't a thing and qcom used a different way to select the microvolt for the opp. There is nothing in the related DTS to fix since they are not broken. The driver doesn't enforce cpr presence and supports both new and old implementation... Setting the cpr as a required binding was wrong from the start. It was probably done when qcs404 was introduced and they had this bright idea of creating the schema and ignoring the other kind of configuration the driver supports. Since now we want to send opp for ipq8064 that use the old implementation this fixup is required. Probably I should drop the legacy term and just say that the driver supports 2 different configuration and the following schema permits only one? > > binding providing values based on speedbin, psv and version. > > > > Make the power-domain 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> > > --- > > .../bindings/cpufreq/qcom-cpufreq-nvmem.yaml | 78 +++++++++++++------ > > 1 file changed, 54 insertions(+), 24 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml b/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml > > index 9c086eac6ca7..04aac634d7fc 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,37 +36,64 @@ select: > > required: > > - compatible > > > > -properties: > > - cpus: > > - type: object > > +if: > > Missing allOf. Fix also the placement - this block goes before > additionalPropertes:false. > Thanks for the review! Will fix in v2. > > + properties: > > + compatible: > > + contains: > > + enum: > > + - qcom,qcs404 > > > > - patternProperties: > > - '^cpu@[0-9a-f]+$': > > - type: object > > +then: > > + properties: > > + cpus: > > + type: object > > > > - properties: > > - power-domains: > > Best regards, > Krzysztof > -- Ansuel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional 2023-01-22 14:05 ` Christian Marangi @ 2023-01-22 14:13 ` Krzysztof Kozlowski 0 siblings, 0 replies; 13+ messages in thread From: Krzysztof Kozlowski @ 2023-01-22 14:13 UTC (permalink / raw) To: Christian Marangi Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Ilia Lin, Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski, Nishanth Menon, Stephen Boyd, Yassine Oudjana, linux-arm-msm, linux-pm, devicetree, linux-kernel On 22/01/2023 15:05, Christian Marangi wrote: > On Sun, Jan 22, 2023 at 02:57:07PM +0100, Krzysztof Kozlowski wrote: >> On 21/01/2023 01:01, Christian Marangi wrote: >>> The qcom-cpufreq-nvmem driver also supports legacy devices pre-cpr that >>> doesn't have power-domains. When the schema was introduced, it was >>> wrongly set to always require these binding but this is not the case for >>> legacy device that base everything on nvmem cells and multiple microvolt >> >> What is a "legacy device"? Why do you adjust bindings to legacy device? >> Can't you just fix the DTS on these devices? >> > > With legacy I mean device where cpr (core power reduction) wasn't a > thing and qcom used a different way to select the microvolt for the opp. > > There is nothing in the related DTS to fix since they are not broken. > The driver doesn't enforce cpr presence and supports both new and old > implementation... > > Setting the cpr as a required binding was wrong from the start. It was > probably done when qcs404 was introduced and they had this bright idea > of creating the schema and ignoring the other kind of configuration the > driver supports. > > Since now we want to send opp for ipq8064 that use the old > implementation this fixup is required. > > Probably I should drop the legacy term and just say that the driver > supports 2 different configuration and the following schema permits only > one? Yes, it would be clearer. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-01-22 14:35 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-21 0:01 [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional Christian Marangi 2023-01-21 0:01 ` [PATCH 2/3] dt-bindings: opp: opp-v2-kryo-cpu: add opp-microvolt nvmem based Christian Marangi 2023-01-22 14:00 ` Krzysztof Kozlowski 2023-01-22 14:15 ` Christian Marangi 2023-01-22 14:17 ` Krzysztof Kozlowski 2023-01-22 14:21 ` Christian Marangi 2023-01-22 14:31 ` Krzysztof Kozlowski 2023-01-22 14:35 ` Christian Marangi 2023-01-21 0:01 ` [PATCH 3/3] dt-bindings: opp: opp-v2-kryo-cpu: enlarge opp-supported-hw maximum Christian Marangi 2023-01-22 14:01 ` Krzysztof Kozlowski 2023-01-22 13:57 ` [PATCH 1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional Krzysztof Kozlowski 2023-01-22 14:05 ` Christian Marangi 2023-01-22 14:13 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).