[PATCH] dt-bindings: soc: qcom: qcom,spm: support regulator SAW2 devics

[PATCH] dt-bindings: soc: qcom: qcom,spm: support regulator SAW2 devics

[Dmitry Baryshkov]

Merge qcom,saw2.txt bindings to existing qcom,spm.yaml. This fixes
compatibility of qcom,spm schema with regulator SAW2 devices.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -------------------
 .../bindings/soc/qcom/qcom,spm.yaml           | 44 +++++++++-----
 2 files changed, 30 insertions(+), 72 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
deleted file mode 100644
index c0e3c3a42bea..000000000000
--- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
+++ /dev/null
@@ -1,58 +0,0 @@
-SPM AVS Wrapper 2 (SAW2)
-
-The SAW2 is a wrapper around the Subsystem Power Manager (SPM) and the
-Adaptive Voltage Scaling (AVS) hardware. The SPM is a programmable
-power-controller that transitions a piece of hardware (like a processor or
-subsystem) into and out of low power modes via a direct connection to
-the PMIC. It can also be wired up to interact with other processors in the
-system, notifying them when a low power state is entered or exited.
-
-Multiple revisions of the SAW hardware are supported using these Device Nodes.
-SAW2 revisions differ in the register offset and configuration data. Also, the
-same revision of the SAW in different SoCs may have different configuration
-data due the differences in hardware capabilities. Hence the SoC name, the
-version of the SAW hardware in that SoC and the distinction between cpu (big
-or Little) or cache, may be needed to uniquely identify the SAW register
-configuration and initialization data. The compatible string is used to
-indicate this parameter.
-
-PROPERTIES
-
-- compatible:
-	Usage: required
-	Value type: <string>
-	Definition: Must have
-			"qcom,saw2"
-		    A more specific value could be one of:
-			"qcom,apq8064-saw2-v1.1-cpu"
-			"qcom,msm8226-saw2-v2.1-cpu"
-			"qcom,msm8974-saw2-v2.1-cpu"
-			"qcom,apq8084-saw2-v2.1-cpu"
-
-- reg:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: the first element specifies the base address and size of
-		    the register region. An optional second element specifies
-		    the base address and size of the alias register region.
-
-- regulator:
-	Usage: optional
-	Value type: boolean
-	Definition: Indicates that this SPM device acts as a regulator device
-			device for the core (CPU or Cache) the SPM is attached
-			to.
-
-Example 1:
-
-	power-controller@2099000 {
-		compatible = "qcom,saw2";
-		reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
-		regulator;
-	};
-
-Example 2:
-	saw0: power-controller@f9089000 {
-		compatible = "qcom,apq8084-saw2-v2.1-cpu", "qcom,saw2";
-		reg = <0xf9089000 0x1000>, <0xf9009000 0x1000>;
-	};
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
index f433e6e0a19f..8fe35fde70b8 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
@@ -16,23 +16,33 @@ description: |
 
 properties:
   compatible:
-    items:
-      - enum:
-          - qcom,sdm660-gold-saw2-v4.1-l2
-          - qcom,sdm660-silver-saw2-v4.1-l2
-          - qcom,msm8998-gold-saw2-v4.1-l2
-          - qcom,msm8998-silver-saw2-v4.1-l2
-          - qcom,msm8909-saw2-v3.0-cpu
-          - qcom,msm8916-saw2-v3.0-cpu
-          - qcom,msm8226-saw2-v2.1-cpu
-          - qcom,msm8974-saw2-v2.1-cpu
-          - qcom,apq8084-saw2-v2.1-cpu
-          - qcom,apq8064-saw2-v1.1-cpu
+    oneOf:
       - const: qcom,saw2
+      - items:
+          - enum:
+              - qcom,sdm660-gold-saw2-v4.1-l2
+              - qcom,sdm660-silver-saw2-v4.1-l2
+              - qcom,msm8998-gold-saw2-v4.1-l2
+              - qcom,msm8998-silver-saw2-v4.1-l2
+              - qcom,msm8909-saw2-v3.0-cpu
+              - qcom,msm8916-saw2-v3.0-cpu
+              - qcom,msm8226-saw2-v2.1-cpu
+              - qcom,msm8974-saw2-v2.1-cpu
+              - qcom,apq8084-saw2-v2.1-cpu
+              - qcom,apq8064-saw2-v1.1-cpu
+          - const: qcom,saw2
 
   reg:
-    description: Base address and size of the SPM register region
-    maxItems: 1
+    description: Base address and size of the SPM register region. An optional
+      second element specifies the base address and size of the alias register
+      region.
+    minItems: 1
+    maxItems: 2
+
+  regulator:
+    type: boolean
+    description: Indicates that this SPM device acts as a regulator device
+      device for the core (CPU or Cache) the SPM is attached to.
 
 required:
   - compatible
@@ -79,4 +89,10 @@ examples:
         reg = <0x17912000 0x1000>;
     };
 
+  - |
+    power-controller@2099000 {
+        compatible = "qcom,saw2";
+        reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
+        regulator;
+    };
 ...
-- 
2.35.1

Re: [PATCH] dt-bindings: soc: qcom: qcom,spm: support regulator SAW2 devics

[Krzysztof Kozlowski]

On 01/10/2022 01:14, Dmitry Baryshkov wrote:
> Merge qcom,saw2.txt bindings to existing qcom,spm.yaml. This fixes
> compatibility of qcom,spm schema with regulator SAW2 devices.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -------------------
>  .../bindings/soc/qcom/qcom,spm.yaml           | 44 +++++++++-----

You need to update reference in
Documentation/devicetree/bindings/arm/cpus.yaml

>  2 files changed, 30 insertions(+), 72 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> deleted file mode 100644
> index c0e3c3a42bea..000000000000
> --- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -SPM AVS Wrapper 2 (SAW2)
> -
> -The SAW2 is a wrapper around the Subsystem Power Manager (SPM) and the
> -Adaptive Voltage Scaling (AVS) hardware. The SPM is a programmable
> -power-controller that transitions a piece of hardware (like a processor or
> -subsystem) into and out of low power modes via a direct connection to
> -the PMIC. It can also be wired up to interact with other processors in the
> -system, notifying them when a low power state is entered or exited.
> -
> -Multiple revisions of the SAW hardware are supported using these Device Nodes.
> -SAW2 revisions differ in the register offset and configuration data. Also, the
> -same revision of the SAW in different SoCs may have different configuration
> -data due the differences in hardware capabilities. Hence the SoC name, the
> -version of the SAW hardware in that SoC and the distinction between cpu (big
> -or Little) or cache, may be needed to uniquely identify the SAW register
> -configuration and initialization data. The compatible string is used to
> -indicate this parameter.
> -
> -PROPERTIES
> -
> -- compatible:
> -	Usage: required
> -	Value type: <string>
> -	Definition: Must have
> -			"qcom,saw2"
> -		    A more specific value could be one of:
> -			"qcom,apq8064-saw2-v1.1-cpu"
> -			"qcom,msm8226-saw2-v2.1-cpu"
> -			"qcom,msm8974-saw2-v2.1-cpu"
> -			"qcom,apq8084-saw2-v2.1-cpu"
> -
> -- reg:
> -	Usage: required
> -	Value type: <prop-encoded-array>
> -	Definition: the first element specifies the base address and size of
> -		    the register region. An optional second element specifies
> -		    the base address and size of the alias register region.
> -
> -- regulator:
> -	Usage: optional
> -	Value type: boolean
> -	Definition: Indicates that this SPM device acts as a regulator device
> -			device for the core (CPU or Cache) the SPM is attached
> -			to.
> -
> -Example 1:
> -
> -	power-controller@2099000 {
> -		compatible = "qcom,saw2";
> -		reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
> -		regulator;
> -	};
> -
> -Example 2:
> -	saw0: power-controller@f9089000 {
> -		compatible = "qcom,apq8084-saw2-v2.1-cpu", "qcom,saw2";
> -		reg = <0xf9089000 0x1000>, <0xf9009000 0x1000>;
> -	};
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> index f433e6e0a19f..8fe35fde70b8 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> @@ -16,23 +16,33 @@ description: |
>  
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -          - qcom,sdm660-gold-saw2-v4.1-l2
> -          - qcom,sdm660-silver-saw2-v4.1-l2
> -          - qcom,msm8998-gold-saw2-v4.1-l2
> -          - qcom,msm8998-silver-saw2-v4.1-l2
> -          - qcom,msm8909-saw2-v3.0-cpu
> -          - qcom,msm8916-saw2-v3.0-cpu
> -          - qcom,msm8226-saw2-v2.1-cpu
> -          - qcom,msm8974-saw2-v2.1-cpu
> -          - qcom,apq8084-saw2-v2.1-cpu
> -          - qcom,apq8064-saw2-v1.1-cpu
> +    oneOf:
>        - const: qcom,saw2

I understand old bindings had it, but I don't think we really want to
support the generic compatible on its own. Even old bindings indicated
that there are several differences between SAWs.

Especially confusing is that once qcom,saw2 can be alone and in other
cases must be preceded by specific compatible. IOW, you allow for
apq8064 two cases:

1. qcom,apq8064-saw2-v1.1-cpu, qcom,saw2
2. qcom,saw2

I think we should instead add everywhere specific compatibles.


> +      - items:
> +          - enum:
> +              - qcom,sdm660-gold-saw2-v4.1-l2
> +              - qcom,sdm660-silver-saw2-v4.1-l2
> +              - qcom,msm8998-gold-saw2-v4.1-l2
> +              - qcom,msm8998-silver-saw2-v4.1-l2
> +              - qcom,msm8909-saw2-v3.0-cpu
> +              - qcom,msm8916-saw2-v3.0-cpu
> +              - qcom,msm8226-saw2-v2.1-cpu
> +              - qcom,msm8974-saw2-v2.1-cpu
> +              - qcom,apq8084-saw2-v2.1-cpu
> +              - qcom,apq8064-saw2-v1.1-cpu
> +          - const: qcom,saw2
>  
>    reg:
> -    description: Base address and size of the SPM register region
> -    maxItems: 1
> +    description: Base address and size of the SPM register region. An optional
> +      second element specifies the base address and size of the alias register
> +      region.
> +    minItems: 1
> +    maxItems: 2

And it seems second region is not present on some variants?

> +
> +  regulator:
> +    type: boolean
> +    description: Indicates that this SPM device acts as a regulator device
> +      device for the core (CPU or Cache) the SPM is attached to.
>  
>  required:
>    - compatible
> @@ -79,4 +89,10 @@ examples:
>          reg = <0x17912000 0x1000>;
>      };


Best regards,
Krzysztof

Re: [PATCH] dt-bindings: soc: qcom: qcom,spm: support regulator SAW2 devics

[Dmitry Baryshkov]

On Sun, 2 Oct 2022 at 11:49, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 01/10/2022 01:14, Dmitry Baryshkov wrote:
> > Merge qcom,saw2.txt bindings to existing qcom,spm.yaml. This fixes
> > compatibility of qcom,spm schema with regulator SAW2 devices.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  .../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -------------------
> >  .../bindings/soc/qcom/qcom,spm.yaml           | 44 +++++++++-----
>
> You need to update reference in
> Documentation/devicetree/bindings/arm/cpus.yaml

ack

>
> >  2 files changed, 30 insertions(+), 72 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> >
> > diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> > deleted file mode 100644
> > index c0e3c3a42bea..000000000000
> > --- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt

[skipped]

> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> > index f433e6e0a19f..8fe35fde70b8 100644
> > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> > @@ -16,23 +16,33 @@ description: |
> >
> >  properties:
> >    compatible:
> > -    items:
> > -      - enum:
> > -          - qcom,sdm660-gold-saw2-v4.1-l2
> > -          - qcom,sdm660-silver-saw2-v4.1-l2
> > -          - qcom,msm8998-gold-saw2-v4.1-l2
> > -          - qcom,msm8998-silver-saw2-v4.1-l2
> > -          - qcom,msm8909-saw2-v3.0-cpu
> > -          - qcom,msm8916-saw2-v3.0-cpu
> > -          - qcom,msm8226-saw2-v2.1-cpu
> > -          - qcom,msm8974-saw2-v2.1-cpu
> > -          - qcom,apq8084-saw2-v2.1-cpu
> > -          - qcom,apq8064-saw2-v1.1-cpu
> > +    oneOf:
> >        - const: qcom,saw2
>
> I understand old bindings had it, but I don't think we really want to
> support the generic compatible on its own. Even old bindings indicated
> that there are several differences between SAWs.
>
> Especially confusing is that once qcom,saw2 can be alone and in other
> cases must be preceded by specific compatible. IOW, you allow for
> apq8064 two cases:
>
> 1. qcom,apq8064-saw2-v1.1-cpu, qcom,saw2
> 2. qcom,saw2
>
> I think we should instead add everywhere specific compatibles.

I see your point. Yes, it's probably worth doing that.

Robert, Christian, can you possibly check the version of the SAW2 used
on ipq4019 and ipq8064? It can be read from the SPM block at the
register offset 0xfd0.

> > +      - items:
> > +          - enum:
> > +              - qcom,sdm660-gold-saw2-v4.1-l2
> > +              - qcom,sdm660-silver-saw2-v4.1-l2
> > +              - qcom,msm8998-gold-saw2-v4.1-l2
> > +              - qcom,msm8998-silver-saw2-v4.1-l2
> > +              - qcom,msm8909-saw2-v3.0-cpu
> > +              - qcom,msm8916-saw2-v3.0-cpu
> > +              - qcom,msm8226-saw2-v2.1-cpu
> > +              - qcom,msm8974-saw2-v2.1-cpu
> > +              - qcom,apq8084-saw2-v2.1-cpu
> > +              - qcom,apq8064-saw2-v1.1-cpu
> > +          - const: qcom,saw2
> >
> >    reg:
> > -    description: Base address and size of the SPM register region
> > -    maxItems: 1
> > +    description: Base address and size of the SPM register region. An optional
> > +      second element specifies the base address and size of the alias register
> > +      region.
> > +    minItems: 1
> > +    maxItems: 2
>
> And it seems second region is not present on some variants?

The second region is a bit of a puzzle for me as it doesn't seem to be
used at all.

-- 
With best wishes
Dmitry

Re: [PATCH] dt-bindings: soc: qcom: qcom,spm: support regulator SAW2 devics

[Robert Marko]

On 02. 10. 2022. 14:20, Dmitry Baryshkov wrote:
> On Sun, 2 Oct 2022 at 11:49, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 01/10/2022 01:14, Dmitry Baryshkov wrote:
>>> Merge qcom,saw2.txt bindings to existing qcom,spm.yaml. This fixes
>>> compatibility of qcom,spm schema with regulator SAW2 devices.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>   .../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -------------------
>>>   .../bindings/soc/qcom/qcom,spm.yaml           | 44 +++++++++-----
>> You need to update reference in
>> Documentation/devicetree/bindings/arm/cpus.yaml
> ack
>
>>>   2 files changed, 30 insertions(+), 72 deletions(-)
>>>   delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
>>> deleted file mode 100644
>>> index c0e3c3a42bea..000000000000
>>> --- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> [skipped]
>
>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
>>> index f433e6e0a19f..8fe35fde70b8 100644
>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
>>> @@ -16,23 +16,33 @@ description: |
>>>
>>>   properties:
>>>     compatible:
>>> -    items:
>>> -      - enum:
>>> -          - qcom,sdm660-gold-saw2-v4.1-l2
>>> -          - qcom,sdm660-silver-saw2-v4.1-l2
>>> -          - qcom,msm8998-gold-saw2-v4.1-l2
>>> -          - qcom,msm8998-silver-saw2-v4.1-l2
>>> -          - qcom,msm8909-saw2-v3.0-cpu
>>> -          - qcom,msm8916-saw2-v3.0-cpu
>>> -          - qcom,msm8226-saw2-v2.1-cpu
>>> -          - qcom,msm8974-saw2-v2.1-cpu
>>> -          - qcom,apq8084-saw2-v2.1-cpu
>>> -          - qcom,apq8064-saw2-v1.1-cpu
>>> +    oneOf:
>>>         - const: qcom,saw2
>> I understand old bindings had it, but I don't think we really want to
>> support the generic compatible on its own. Even old bindings indicated
>> that there are several differences between SAWs.
>>
>> Especially confusing is that once qcom,saw2 can be alone and in other
>> cases must be preceded by specific compatible. IOW, you allow for
>> apq8064 two cases:
>>
>> 1. qcom,apq8064-saw2-v1.1-cpu, qcom,saw2
>> 2. qcom,saw2
>>
>> I think we should instead add everywhere specific compatibles.
> I see your point. Yes, it's probably worth doing that.
>
> Robert, Christian, can you possibly check the version of the SAW2 used
> on ipq4019 and ipq8064? It can be read from the SPM block at the
> register offset 0xfd0.

Hi, I completely missed this so far, sorry about that.

I checked from U-boot on multiple SAW blocks on IPQ4019 and it looks to 
be v3.0:
(IPQ40xx) # md.l 0xB0B9FD0 1
0b0b9fd0: 30000000    ...0
(IPQ40xx) # md.l 0xB089FD0 1
0b089fd0: 30000000    ...0
(IPQ40xx) # md.l 0xB099FD0 1
0b099fd0: 30000000    ...0
(IPQ40xx) # md.l 0xB0A9FD0 1
0b0a9fd0: 30000000    ...0

IPQ8064 is a bit weird, both SAW-s from DTS show all zeros:
(IPQ) # md.l 0x2089FD0 1
02089fd0: 00000000    ....
(IPQ) # md.l 0x2099FD0 1
02099fd0: 00000000    ....

However some old datasheet says: 0x02011FD0 APCS_VERSION

(IPQ) # md.l 0x02011FD0 1
02011fd0: 10010000    ....

But It also says that minor and step are both bits 15:0 which makes no 
sense.

Hope this helps,
Robert

>
>>> +      - items:
>>> +          - enum:
>>> +              - qcom,sdm660-gold-saw2-v4.1-l2
>>> +              - qcom,sdm660-silver-saw2-v4.1-l2
>>> +              - qcom,msm8998-gold-saw2-v4.1-l2
>>> +              - qcom,msm8998-silver-saw2-v4.1-l2
>>> +              - qcom,msm8909-saw2-v3.0-cpu
>>> +              - qcom,msm8916-saw2-v3.0-cpu
>>> +              - qcom,msm8226-saw2-v2.1-cpu
>>> +              - qcom,msm8974-saw2-v2.1-cpu
>>> +              - qcom,apq8084-saw2-v2.1-cpu
>>> +              - qcom,apq8064-saw2-v1.1-cpu
>>> +          - const: qcom,saw2
>>>
>>>     reg:
>>> -    description: Base address and size of the SPM register region
>>> -    maxItems: 1
>>> +    description: Base address and size of the SPM register region. An optional
>>> +      second element specifies the base address and size of the alias register
>>> +      region.
>>> +    minItems: 1
>>> +    maxItems: 2
>> And it seems second region is not present on some variants?
> The second region is a bit of a puzzle for me as it doesn't seem to be
> used at all.
>

Re: [PATCH] dt-bindings: soc: qcom: qcom,spm: support regulator SAW2 devics

[Dmitry Baryshkov]

On Wed, 5 Oct 2022 at 16:50, Robert Marko <robimarko@gmail.com> wrote:
>
>
> On 02. 10. 2022. 14:20, Dmitry Baryshkov wrote:
> > On Sun, 2 Oct 2022 at 11:49, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >> On 01/10/2022 01:14, Dmitry Baryshkov wrote:
> >>> Merge qcom,saw2.txt bindings to existing qcom,spm.yaml. This fixes
> >>> compatibility of qcom,spm schema with regulator SAW2 devices.
> >>>
> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> ---
> >>>   .../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -------------------
> >>>   .../bindings/soc/qcom/qcom,spm.yaml           | 44 +++++++++-----
> >> You need to update reference in
> >> Documentation/devicetree/bindings/arm/cpus.yaml
> > ack
> >
> >>>   2 files changed, 30 insertions(+), 72 deletions(-)
> >>>   delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> >>> deleted file mode 100644
> >>> index c0e3c3a42bea..000000000000
> >>> --- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> > [skipped]
> >
> >>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> >>> index f433e6e0a19f..8fe35fde70b8 100644
> >>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> >>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> >>> @@ -16,23 +16,33 @@ description: |
> >>>
> >>>   properties:
> >>>     compatible:
> >>> -    items:
> >>> -      - enum:
> >>> -          - qcom,sdm660-gold-saw2-v4.1-l2
> >>> -          - qcom,sdm660-silver-saw2-v4.1-l2
> >>> -          - qcom,msm8998-gold-saw2-v4.1-l2
> >>> -          - qcom,msm8998-silver-saw2-v4.1-l2
> >>> -          - qcom,msm8909-saw2-v3.0-cpu
> >>> -          - qcom,msm8916-saw2-v3.0-cpu
> >>> -          - qcom,msm8226-saw2-v2.1-cpu
> >>> -          - qcom,msm8974-saw2-v2.1-cpu
> >>> -          - qcom,apq8084-saw2-v2.1-cpu
> >>> -          - qcom,apq8064-saw2-v1.1-cpu
> >>> +    oneOf:
> >>>         - const: qcom,saw2
> >> I understand old bindings had it, but I don't think we really want to
> >> support the generic compatible on its own. Even old bindings indicated
> >> that there are several differences between SAWs.
> >>
> >> Especially confusing is that once qcom,saw2 can be alone and in other
> >> cases must be preceded by specific compatible. IOW, you allow for
> >> apq8064 two cases:
> >>
> >> 1. qcom,apq8064-saw2-v1.1-cpu, qcom,saw2
> >> 2. qcom,saw2
> >>
> >> I think we should instead add everywhere specific compatibles.
> > I see your point. Yes, it's probably worth doing that.
> >
> > Robert, Christian, can you possibly check the version of the SAW2 used
> > on ipq4019 and ipq8064? It can be read from the SPM block at the
> > register offset 0xfd0.
>
> Hi, I completely missed this so far, sorry about that.

No problem.

>
> I checked from U-boot on multiple SAW blocks on IPQ4019 and it looks to
> be v3.0:
> (IPQ40xx) # md.l 0xB0B9FD0 1
> 0b0b9fd0: 30000000    ...0
> (IPQ40xx) # md.l 0xB089FD0 1
> 0b089fd0: 30000000    ...0
> (IPQ40xx) # md.l 0xB099FD0 1
> 0b099fd0: 30000000    ...0
> (IPQ40xx) # md.l 0xB0A9FD0 1
> 0b0a9fd0: 30000000    ...0

Thanks, this looks like v2.3.

>
> IPQ8064 is a bit weird, both SAW-s from DTS show all zeros:
> (IPQ) # md.l 0x2089FD0 1
> 02089fd0: 00000000    ....
> (IPQ) # md.l 0x2099FD0 1
> 02099fd0: 00000000    ....

Hmm, I guess then it didn't have this register. As the rest of
kpss/acc follows apc8064 design, I'd assume that SAW2 also follows
apq8064 with the version 1.1

>
> However some old datasheet says: 0x02011FD0 APCS_VERSION
>
> (IPQ) # md.l 0x02011FD0 1
> 02011fd0: 10010000    ....

This is a part of l2cc, rather than SAW.

>
> But It also says that minor and step are both bits 15:0 which makes no
> sense.
>


-- 
With best wishes
Dmitry

Re: [PATCH] dt-bindings: soc: qcom: qcom,spm: support regulator SAW2 devics

[Christian Marangi]

On Sun, Oct 02, 2022 at 03:20:57PM +0300, Dmitry Baryshkov wrote:
> On Sun, 2 Oct 2022 at 11:49, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 01/10/2022 01:14, Dmitry Baryshkov wrote:
> > > Merge qcom,saw2.txt bindings to existing qcom,spm.yaml. This fixes
> > > compatibility of qcom,spm schema with regulator SAW2 devices.
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  .../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -------------------
> > >  .../bindings/soc/qcom/qcom,spm.yaml           | 44 +++++++++-----
> >
> > You need to update reference in
> > Documentation/devicetree/bindings/arm/cpus.yaml
> 
> ack
> 
> >
> > >  2 files changed, 30 insertions(+), 72 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> > > deleted file mode 100644
> > > index c0e3c3a42bea..000000000000
> > > --- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> 
> [skipped]
> 
> > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> > > index f433e6e0a19f..8fe35fde70b8 100644
> > > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> > > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
> > > @@ -16,23 +16,33 @@ description: |
> > >
> > >  properties:
> > >    compatible:
> > > -    items:
> > > -      - enum:
> > > -          - qcom,sdm660-gold-saw2-v4.1-l2
> > > -          - qcom,sdm660-silver-saw2-v4.1-l2
> > > -          - qcom,msm8998-gold-saw2-v4.1-l2
> > > -          - qcom,msm8998-silver-saw2-v4.1-l2
> > > -          - qcom,msm8909-saw2-v3.0-cpu
> > > -          - qcom,msm8916-saw2-v3.0-cpu
> > > -          - qcom,msm8226-saw2-v2.1-cpu
> > > -          - qcom,msm8974-saw2-v2.1-cpu
> > > -          - qcom,apq8084-saw2-v2.1-cpu
> > > -          - qcom,apq8064-saw2-v1.1-cpu
> > > +    oneOf:
> > >        - const: qcom,saw2
> >
> > I understand old bindings had it, but I don't think we really want to
> > support the generic compatible on its own. Even old bindings indicated
> > that there are several differences between SAWs.
> >
> > Especially confusing is that once qcom,saw2 can be alone and in other
> > cases must be preceded by specific compatible. IOW, you allow for
> > apq8064 two cases:
> >
> > 1. qcom,apq8064-saw2-v1.1-cpu, qcom,saw2
> > 2. qcom,saw2
> >
> > I think we should instead add everywhere specific compatibles.
> 
> I see your point. Yes, it's probably worth doing that.
> 
> Robert, Christian, can you possibly check the version of the SAW2 used
> on ipq4019 and ipq8064? It can be read from the SPM block at the
> register offset 0xfd0.
>

From what I notice from QSDK it seems ipq806x doesn't have a reg to
provide version. (there are many variant of v1 revision)

Anyway these are the values from uboot

(IPQ) # md.l 0x2089FD0
02089fd0: 00000000    ....
(IPQ) # md.l 0x2099FD0
02099fd0: 00000000    ....
(IPQ) # md.l 0x2012FD0
02012fd0: 00000000    ....

> > > +      - items:
> > > +          - enum:
> > > +              - qcom,sdm660-gold-saw2-v4.1-l2
> > > +              - qcom,sdm660-silver-saw2-v4.1-l2
> > > +              - qcom,msm8998-gold-saw2-v4.1-l2
> > > +              - qcom,msm8998-silver-saw2-v4.1-l2
> > > +              - qcom,msm8909-saw2-v3.0-cpu
> > > +              - qcom,msm8916-saw2-v3.0-cpu
> > > +              - qcom,msm8226-saw2-v2.1-cpu
> > > +              - qcom,msm8974-saw2-v2.1-cpu
> > > +              - qcom,apq8084-saw2-v2.1-cpu
> > > +              - qcom,apq8064-saw2-v1.1-cpu
> > > +          - const: qcom,saw2
> > >
> > >    reg:
> > > -    description: Base address and size of the SPM register region
> > > -    maxItems: 1
> > > +    description: Base address and size of the SPM register region. An optional
> > > +      second element specifies the base address and size of the alias register
> > > +      region.
> > > +    minItems: 1
> > > +    maxItems: 2
> >
> > And it seems second region is not present on some variants?
> 
> The second region is a bit of a puzzle for me as it doesn't seem to be
> used at all.
> 
> -- 
> With best wishes
> Dmitry

-- 
	Ansuel

Re: [PATCH] dt-bindings: soc: qcom: qcom,spm: support regulator SAW2 devics

[Rob Herring]

On Sat, 01 Oct 2022 02:14:16 +0300, Dmitry Baryshkov wrote:
> Merge qcom,saw2.txt bindings to existing qcom,spm.yaml. This fixes
> compatibility of qcom,spm schema with regulator SAW2 devices.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -------------------
>  .../bindings/soc/qcom/qcom,spm.yaml           | 44 +++++++++-----
>  2 files changed, 30 insertions(+), 72 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> 

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/soc/qcom/qcom,spm.example.dtb: power-controller@2099000: '#power-domain-cells' is a required property
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/power-domain.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

Re: [PATCH] dt-bindings: soc: qcom: qcom,spm: support regulator SAW2 devics

[Rob Herring]

On Sat, 01 Oct 2022 02:14:16 +0300, Dmitry Baryshkov wrote:
> Merge qcom,saw2.txt bindings to existing qcom,spm.yaml. This fixes
> compatibility of qcom,spm schema with regulator SAW2 devices.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -------------------
>  .../bindings/soc/qcom/qcom,spm.yaml           | 44 +++++++++-----
>  2 files changed, 30 insertions(+), 72 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> 

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/soc/qcom/qcom,spm.example.dtb: power-controller@2099000: '#power-domain-cells' is a required property
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/power-domain.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.