| Message ID | 20230121000146.7809-2-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional | expand |
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
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 >
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
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.
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
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.
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
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(+)