[PATCH v2 1/4] dt-bindings: firmware: scm: Add QDU1000/QRU1000 compatibles

Melody Olvera posted 4 patches 3 years, 5 months ago
There is a newer version of this series
[PATCH v2 1/4] dt-bindings: firmware: scm: Add QDU1000/QRU1000 compatibles
Posted by Melody Olvera 3 years, 5 months ago
Add compatibles for scm driver for QDU1000 and QRU1000 platforms.

Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
---
 .../devicetree/bindings/firmware/qcom,scm.yaml   | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
index c5b76c9f7ad0..47083f47f109 100644
--- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
@@ -38,6 +38,8 @@ properties:
           - qcom,scm-msm8994
           - qcom,scm-msm8996
           - qcom,scm-msm8998
+          - qcom,scm-qdu1000
+          - qcom,scm-qru1000
           - qcom,scm-sc7180
           - qcom,scm-sc7280
           - qcom,scm-sc8280xp
@@ -80,6 +82,20 @@ properties:
     description: TCSR hardware block
 
 allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,scm-qdu1000
+              - qcom,scm-qru1000
+    then:
+      properties:
+        '#reset-cells':
+          maxItems: 1
+
+      required:
+        - '#reset-cells'
   - if:
       properties:
         compatible:
-- 
2.38.0
Re: [PATCH v2 1/4] dt-bindings: firmware: scm: Add QDU1000/QRU1000 compatibles
Posted by Krzysztof Kozlowski 3 years, 5 months ago
On 14/10/2022 18:11, Melody Olvera wrote:
> Add compatibles for scm driver for QDU1000 and QRU1000 platforms.
> 
> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
> ---
>  .../devicetree/bindings/firmware/qcom,scm.yaml   | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> index c5b76c9f7ad0..47083f47f109 100644
> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> @@ -38,6 +38,8 @@ properties:
>            - qcom,scm-msm8994
>            - qcom,scm-msm8996
>            - qcom,scm-msm8998
> +          - qcom,scm-qdu1000
> +          - qcom,scm-qru1000

Why exactly we are no using qdu1000 as fallback? That was the
recommendation in previous discussion.

Patch is still incomplete - you still do no have proper changes in allOf
for the clocks. If you want to say that this SoC does not take any
clocks as input, then they should not be allowed.

Best regards,
Krzysztof
Re: [PATCH v2 1/4] dt-bindings: firmware: scm: Add QDU1000/QRU1000 compatibles
Posted by Melody Olvera 3 years, 5 months ago

On 10/15/2022 6:34 AM, Krzysztof Kozlowski wrote:
> On 14/10/2022 18:11, Melody Olvera wrote:
>> Add compatibles for scm driver for QDU1000 and QRU1000 platforms.
>>
>> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
>> ---
>>  .../devicetree/bindings/firmware/qcom,scm.yaml   | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>> index c5b76c9f7ad0..47083f47f109 100644
>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>> @@ -38,6 +38,8 @@ properties:
>>            - qcom,scm-msm8994
>>            - qcom,scm-msm8996
>>            - qcom,scm-msm8998
>> +          - qcom,scm-qdu1000
>> +          - qcom,scm-qru1000
> Why exactly we are no using qdu1000 as fallback? That was the
> recommendation in previous discussion.
Will use only qdu; I think I misunderstood the outcome of that discussion.
>
> Patch is still incomplete - you still do no have proper changes in allOf
> for the clocks. If you want to say that this SoC does not take any
> clocks as input, then they should not be allowed.
That is what I'm trying to say; it seems most of our most recent SoCs (sm8*) don't have any
clocks associated with the scm. Does it make sense to remove the minItems earlier
in the binding, or is there something else that would communicate this in allOf better?

Thanks,
Melody
Re: [PATCH v2 1/4] dt-bindings: firmware: scm: Add QDU1000/QRU1000 compatibles
Posted by Krzysztof Kozlowski 3 years, 5 months ago
On 19/10/2022 14:08, Melody Olvera wrote:
> 
> 
> On 10/15/2022 6:34 AM, Krzysztof Kozlowski wrote:
>> On 14/10/2022 18:11, Melody Olvera wrote:
>>> Add compatibles for scm driver for QDU1000 and QRU1000 platforms.
>>>
>>> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
>>> ---
>>>  .../devicetree/bindings/firmware/qcom,scm.yaml   | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>> index c5b76c9f7ad0..47083f47f109 100644
>>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>> @@ -38,6 +38,8 @@ properties:
>>>            - qcom,scm-msm8994
>>>            - qcom,scm-msm8996
>>>            - qcom,scm-msm8998
>>> +          - qcom,scm-qdu1000
>>> +          - qcom,scm-qru1000
>> Why exactly we are no using qdu1000 as fallback? That was the
>> recommendation in previous discussion.
> Will use only qdu; I think I misunderstood the outcome of that discussion.

Actually, I think I commented about this in wrong patch. I think the
outcome was to use two compatibles for most of the cases, but as a
fallback, so:

QDU: "qcom,qdu1000-rpmhpd"
QRU: "qcom,qru1000-rpmhpd", "qcom,qdu1000-rpmhpd"
(or skip entirely second if you do not customize QRU in DTSI)

However here we already have a fallback, so these are fine:

"qcom,scm-qdu1000", "qcom,scm"
"qcom,scm-qru1000", "qcom,scm"

Still assuming you customize them in DTSI... which does not seem the
case, right?

>>
>> Patch is still incomplete - you still do no have proper changes in allOf
>> for the clocks. If you want to say that this SoC does not take any
>> clocks as input, then they should not be allowed.
> That is what I'm trying to say; it seems most of our most recent SoCs (sm8*) don't have any
> clocks associated with the scm. Does it make sense to remove the minItems earlier
> in the binding, or is there something else that would communicate this in allOf better?
> 


Then disallow clocks for your variant:

  - if:
     ....
    then:
     ...
      clocks: false
      clock-names: false

Best regards,
Krzysztof
Re: [PATCH v2 1/4] dt-bindings: firmware: scm: Add QDU1000/QRU1000 compatibles
Posted by Melody Olvera 3 years, 5 months ago

On 10/20/2022 5:35 AM, Krzysztof Kozlowski wrote:
> On 19/10/2022 14:08, Melody Olvera wrote:
>>
>> On 10/15/2022 6:34 AM, Krzysztof Kozlowski wrote:
>>> On 14/10/2022 18:11, Melody Olvera wrote:
>>>> Add compatibles for scm driver for QDU1000 and QRU1000 platforms.
>>>>
>>>> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
>>>> ---
>>>>  .../devicetree/bindings/firmware/qcom,scm.yaml   | 16 ++++++++++++++++
>>>>  1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>>> index c5b76c9f7ad0..47083f47f109 100644
>>>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>>> @@ -38,6 +38,8 @@ properties:
>>>>            - qcom,scm-msm8994
>>>>            - qcom,scm-msm8996
>>>>            - qcom,scm-msm8998
>>>> +          - qcom,scm-qdu1000
>>>> +          - qcom,scm-qru1000
>>> Why exactly we are no using qdu1000 as fallback? That was the
>>> recommendation in previous discussion.
>> Will use only qdu; I think I misunderstood the outcome of that discussion.
> Actually, I think I commented about this in wrong patch. I think the
> outcome was to use two compatibles for most of the cases, but as a
> fallback, so:
>
> QDU: "qcom,qdu1000-rpmhpd"
> QRU: "qcom,qru1000-rpmhpd", "qcom,qdu1000-rpmhpd"
> (or skip entirely second if you do not customize QRU in DTSI)
>
> However here we already have a fallback, so these are fine:
>
> "qcom,scm-qdu1000", "qcom,scm"
> "qcom,scm-qru1000", "qcom,scm"
>
> Still assuming you customize them in DTSI... which does not seem the
> case, right?
Yeah dtsi is largely shared between RU and DU. It probably makes more sense to
drop RU compat string all together unless there is a significant difference.
>>> Patch is still incomplete - you still do no have proper changes in allOf
>>> for the clocks. If you want to say that this SoC does not take any
>>> clocks as input, then they should not be allowed.
>> That is what I'm trying to say; it seems most of our most recent SoCs (sm8*) don't have any
>> clocks associated with the scm. Does it make sense to remove the minItems earlier
>> in the binding, or is there something else that would communicate this in allOf better?
>>
>
> Then disallow clocks for your variant:
>
>   - if:
>      ....
>     then:
>      ...
>       clocks: false
>       clock-names: false
Got it; thanks.

Thanks,
Melody