diff mbox series

[v2,2/2] dt-bindings: opp: opp-v2-kryo-cpu: enlarge opp-supported-hw maximum

Message ID 20230122174548.13758-2-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v2,1/2] dt-bindings: cpufreq: qcom-cpufreq-nvmem: make cpr bindings optional | expand

Commit Message

Christian Marangi Jan. 22, 2023, 5:45 p.m. UTC
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.

Document all the various mask that can be used and limit them to only
reasonable values instead of using a generic maximum limit.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../bindings/opp/opp-v2-kryo-cpu.yaml         | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Comments

Dmitry Baryshkov Jan. 22, 2023, 5:59 p.m. UTC | #1
On Sun, 22 Jan 2023 at 19:46, Christian Marangi <ansuelsmth@gmail.com> 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.
>
> Document all the various mask that can be used and limit them to only
> reasonable values instead of using a generic maximum limit.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../bindings/opp/opp-v2-kryo-cpu.yaml         | 20 +++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> index b4947b326773..908cb0d7695a 100644
> --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> @@ -50,12 +50,28 @@ patternProperties:
>        opp-supported-hw:
>          description: |
>            A single 32 bit bitmap value, representing compatible HW.
> -          Bitmap:
> +          Bitmap for MSM8996 format:
>            0:  MSM8996, speedbin 0
>            1:  MSM8996, speedbin 1
>            2:  MSM8996, speedbin 2
>            3-31:  unused
> -        maximum: 0x7
> +
> +          Bitmap for MSM8996 later revision format:
> +          0:  MSM8996, speedbin 0
> +          1:  MSM8996, speedbin 1
> +          2:  MSM8996, speedbin 2
> +          3:  always set

This is used for speedbin 3

> +          4-31:  unused
> +
> +          Bitmap for MSM8996SG format (speedbin shifted of 4 left):
> +          0-3:  unused
> +          4:  MSM8996SG, speedbin 0
> +          5:  MSM8996SG, speedbin 1
> +          6:  MSM8996SG, speedbin 2
> +          7-31:  unused
> +        enum: [0x1, 0x2, 0x3, 0x4, 0x7,
> +               0x9, 0xd, 0xe, 0xf,
> +               0x10, 0x20, 0x30, 0x70]
>
>        clock-latency-ns: true
>
> --
> 2.38.1
>
Christian Marangi Jan. 22, 2023, 6:04 p.m. UTC | #2
On Sun, Jan 22, 2023 at 07:59:42PM +0200, Dmitry Baryshkov wrote:
> On Sun, 22 Jan 2023 at 19:46, Christian Marangi <ansuelsmth@gmail.com> 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.
> >
> > Document all the various mask that can be used and limit them to only
> > reasonable values instead of using a generic maximum limit.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  .../bindings/opp/opp-v2-kryo-cpu.yaml         | 20 +++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> > index b4947b326773..908cb0d7695a 100644
> > --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> > +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> > @@ -50,12 +50,28 @@ patternProperties:
> >        opp-supported-hw:
> >          description: |
> >            A single 32 bit bitmap value, representing compatible HW.
> > -          Bitmap:
> > +          Bitmap for MSM8996 format:
> >            0:  MSM8996, speedbin 0
> >            1:  MSM8996, speedbin 1
> >            2:  MSM8996, speedbin 2
> >            3-31:  unused
> > -        maximum: 0x7
> > +
> > +          Bitmap for MSM8996 later revision format:
> > +          0:  MSM8996, speedbin 0
> > +          1:  MSM8996, speedbin 1
> > +          2:  MSM8996, speedbin 2
> > +          3:  always set
> 
> This is used for speedbin 3
>

Is it right that 4 bit speedbin is only introduced later? Cause looking
at the current opp-supported-hw for MSM8996SG and MSM8996 originally
(and based on what this Documentation say) there were only 3 bit and
then they started using a 4th bit. Just asking if it's ok to keep the
bitmap split or i should just merge it.

> > +          4-31:  unused
> > +
> > +          Bitmap for MSM8996SG format (speedbin shifted of 4 left):
> > +          0-3:  unused
> > +          4:  MSM8996SG, speedbin 0
> > +          5:  MSM8996SG, speedbin 1
> > +          6:  MSM8996SG, speedbin 2
> > +          7-31:  unused
> > +        enum: [0x1, 0x2, 0x3, 0x4, 0x7,
> > +               0x9, 0xd, 0xe, 0xf,
> > +               0x10, 0x20, 0x30, 0x70]
> >
> >        clock-latency-ns: true
> >
> > --
> > 2.38.1
> >
Dmitry Baryshkov Jan. 22, 2023, 7:15 p.m. UTC | #3
On Sun, 22 Jan 2023 at 20:13, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> On Sun, Jan 22, 2023 at 07:59:42PM +0200, Dmitry Baryshkov wrote:
> > On Sun, 22 Jan 2023 at 19:46, Christian Marangi <ansuelsmth@gmail.com> 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.
> > >
> > > Document all the various mask that can be used and limit them to only
> > > reasonable values instead of using a generic maximum limit.
> > >
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > >  .../bindings/opp/opp-v2-kryo-cpu.yaml         | 20 +++++++++++++++++--
> > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> > > index b4947b326773..908cb0d7695a 100644
> > > --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> > > +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> > > @@ -50,12 +50,28 @@ patternProperties:
> > >        opp-supported-hw:
> > >          description: |
> > >            A single 32 bit bitmap value, representing compatible HW.
> > > -          Bitmap:
> > > +          Bitmap for MSM8996 format:
> > >            0:  MSM8996, speedbin 0
> > >            1:  MSM8996, speedbin 1
> > >            2:  MSM8996, speedbin 2
> > >            3-31:  unused
> > > -        maximum: 0x7
> > > +
> > > +          Bitmap for MSM8996 later revision format:
> > > +          0:  MSM8996, speedbin 0
> > > +          1:  MSM8996, speedbin 1
> > > +          2:  MSM8996, speedbin 2
> > > +          3:  always set
> >
> > This is used for speedbin 3
> >
>
> Is it right that 4 bit speedbin is only introduced later? Cause looking
> at the current opp-supported-hw for MSM8996SG and MSM8996 originally
> (and based on what this Documentation say) there were only 3 bit and
> then they started using a 4th bit. Just asking if it's ok to keep the
> bitmap split or i should just merge it.

I don't think I got the question, please excuse me. Historically
msm8996.dtsi used 0xff as 'all possible platforms' value for the GPU
tables. Lately I fixed the CPU tables, added support for speed-bin 3.
However I left these 0xff in GPU opp table intact. I don't remember if
there was any reason behind that. Anyway this bit isn't always set, it
is set only for the entries which should be selected for MSM8996 speed
bin 3.

>
> > > +          4-31:  unused
> > > +
> > > +          Bitmap for MSM8996SG format (speedbin shifted of 4 left):
> > > +          0-3:  unused
> > > +          4:  MSM8996SG, speedbin 0
> > > +          5:  MSM8996SG, speedbin 1
> > > +          6:  MSM8996SG, speedbin 2
> > > +          7-31:  unused
> > > +        enum: [0x1, 0x2, 0x3, 0x4, 0x7,
> > > +               0x9, 0xd, 0xe, 0xf,
> > > +               0x10, 0x20, 0x30, 0x70]
> > >
> > >        clock-latency-ns: true
> > >
> > > --
> > > 2.38.1
> > >
>
> --
>         Ansuel
Christian Marangi Jan. 22, 2023, 7:19 p.m. UTC | #4
On Sun, Jan 22, 2023 at 09:15:53PM +0200, Dmitry Baryshkov wrote:
> On Sun, 22 Jan 2023 at 20:13, Christian Marangi <ansuelsmth@gmail.com> wrote:
> >
> > On Sun, Jan 22, 2023 at 07:59:42PM +0200, Dmitry Baryshkov wrote:
> > > On Sun, 22 Jan 2023 at 19:46, Christian Marangi <ansuelsmth@gmail.com> 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.
> > > >
> > > > Document all the various mask that can be used and limit them to only
> > > > reasonable values instead of using a generic maximum limit.
> > > >
> > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > > ---
> > > >  .../bindings/opp/opp-v2-kryo-cpu.yaml         | 20 +++++++++++++++++--
> > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> > > > index b4947b326773..908cb0d7695a 100644
> > > > --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> > > > +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
> > > > @@ -50,12 +50,28 @@ patternProperties:
> > > >        opp-supported-hw:
> > > >          description: |
> > > >            A single 32 bit bitmap value, representing compatible HW.
> > > > -          Bitmap:
> > > > +          Bitmap for MSM8996 format:
> > > >            0:  MSM8996, speedbin 0
> > > >            1:  MSM8996, speedbin 1
> > > >            2:  MSM8996, speedbin 2
> > > >            3-31:  unused
> > > > -        maximum: 0x7
> > > > +
> > > > +          Bitmap for MSM8996 later revision format:
> > > > +          0:  MSM8996, speedbin 0
> > > > +          1:  MSM8996, speedbin 1
> > > > +          2:  MSM8996, speedbin 2
> > > > +          3:  always set
> > >
> > > This is used for speedbin 3
> > >
> >
> > Is it right that 4 bit speedbin is only introduced later? Cause looking
> > at the current opp-supported-hw for MSM8996SG and MSM8996 originally
> > (and based on what this Documentation say) there were only 3 bit and
> > then they started using a 4th bit. Just asking if it's ok to keep the
> > bitmap split or i should just merge it.
> 
> I don't think I got the question, please excuse me. Historically
> msm8996.dtsi used 0xff as 'all possible platforms' value for the GPU
> tables. Lately I fixed the CPU tables, added support for speed-bin 3.
> However I left these 0xff in GPU opp table intact. I don't remember if
> there was any reason behind that. Anyway this bit isn't always set, it
> is set only for the entries which should be selected for MSM8996 speed
> bin 3.
>

Question is: Should I merge the 2 format to something like?

          A single 32 bit bitmap value, representing compatible HW.
          Bitmap for MSM8996 format:
          0:  MSM8996, speedbin 0
          1:  MSM8996, speedbin 1
          2:  MSM8996, speedbin 2
          3:  MSM8996, speedbin 3
          4-31:  unused

          Bitmap for MSM8996SG format (speedbin shifted of 4 left):
          0-3:  unused
          4:  MSM8996SG, speedbin 0
          5:  MSM8996SG, speedbin 1
          6:  MSM8996SG, speedbin 2
          7-31:  unused

Or just replace the 'Always set' to speedbin 3?

(by the looks of it seems they started blowing speedbin 3 only in later
revision and initialy there were only 3 speedbin bit)

> >
> > > > +          4-31:  unused
> > > > +
> > > > +          Bitmap for MSM8996SG format (speedbin shifted of 4 left):
> > > > +          0-3:  unused
> > > > +          4:  MSM8996SG, speedbin 0
> > > > +          5:  MSM8996SG, speedbin 1
> > > > +          6:  MSM8996SG, speedbin 2
> > > > +          7-31:  unused
> > > > +        enum: [0x1, 0x2, 0x3, 0x4, 0x7,
> > > > +               0x9, 0xd, 0xe, 0xf,
> > > > +               0x10, 0x20, 0x30, 0x70]
> > > >
> > > >        clock-latency-ns: true
> > > >
> > > > --
> > > > 2.38.1
> > > >
> >
> > --
> >         Ansuel
> 
> 
> 
> -- 
> With best wishes
> Dmitry
Rob Herring (Arm) Jan. 23, 2023, 1:49 p.m. UTC | #5
On Sun, 22 Jan 2023 18:45:48 +0100, 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.
> 
> Document all the various mask that can be used and limit them to only
> reasonable values instead of using a generic maximum limit.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../bindings/opp/opp-v2-kryo-cpu.yaml         | 20 +++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: /: opp-table-0:opp-1401600000:opp-supported-hw:0:0: 5 is not one of [1, 2, 3, 4, 7, 9, 13, 14, 15, 16, 32, 48, 112]
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: /: opp-table-0: Unevaluated properties are not allowed ('compatible', 'nvmem-cells', 'opp-1401600000', 'opp-1593600000', 'opp-307200000', 'opp-shared' were unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: /: opp-table-1:opp-1804800000:opp-supported-hw:0:0: 6 is not one of [1, 2, 3, 4, 7, 9, 13, 14, 15, 16, 32, 48, 112]
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: /: opp-table-1: Unevaluated properties are not allowed ('compatible', 'nvmem-cells', 'opp-1804800000', 'opp-1900800000', 'opp-2150400000', 'opp-307200000', 'opp-shared' were unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: opp-table-0: opp-1401600000:opp-supported-hw:0:0: 5 is not one of [1, 2, 3, 4, 7, 9, 13, 14, 15, 16, 32, 48, 112]
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: opp-table-1: opp-1804800000:opp-supported-hw:0:0: 6 is not one of [1, 2, 3, 4, 7, 9, 13, 14, 15, 16, 32, 48, 112]
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230122174548.13758-2-ansuelsmth@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
index b4947b326773..908cb0d7695a 100644
--- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
+++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
@@ -50,12 +50,28 @@  patternProperties:
       opp-supported-hw:
         description: |
           A single 32 bit bitmap value, representing compatible HW.
-          Bitmap:
+          Bitmap for MSM8996 format:
           0:  MSM8996, speedbin 0
           1:  MSM8996, speedbin 1
           2:  MSM8996, speedbin 2
           3-31:  unused
-        maximum: 0x7
+
+          Bitmap for MSM8996 later revision format:
+          0:  MSM8996, speedbin 0
+          1:  MSM8996, speedbin 1
+          2:  MSM8996, speedbin 2
+          3:  always set
+          4-31:  unused
+
+          Bitmap for MSM8996SG format (speedbin shifted of 4 left):
+          0-3:  unused
+          4:  MSM8996SG, speedbin 0
+          5:  MSM8996SG, speedbin 1
+          6:  MSM8996SG, speedbin 2
+          7-31:  unused
+        enum: [0x1, 0x2, 0x3, 0x4, 0x7,
+               0x9, 0xd, 0xe, 0xf,
+               0x10, 0x20, 0x30, 0x70]
 
       clock-latency-ns: true