diff mbox series

[16/22] dt-bindings: qcom: geni-se: document support for SA8255P

Message ID 20240828203721.2751904-17-quic_nkela@quicinc.com
State New
Headers show
Series arm64: qcom: Introduce SA8255p Ride platform | expand

Commit Message

Nikunj Kela Aug. 28, 2024, 8:37 p.m. UTC
Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on
SA8255p.

Clocks are being managed by the firmware VM and not required on
SA8255p Linux VM hence removing it from required list.

CC: Praveen Talari <quic_ptalari@quicinc.com>
Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
---
 .../bindings/soc/qcom/qcom,geni-se.yaml       | 47 +++++++++++++++++--
 1 file changed, 43 insertions(+), 4 deletions(-)

Comments

Rob Herring (Arm) Aug. 28, 2024, 11:44 p.m. UTC | #1
On Wed, 28 Aug 2024 13:37:15 -0700, Nikunj Kela wrote:
> Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on
> SA8255p.
> 
> Clocks are being managed by the firmware VM and not required on
> SA8255p Linux VM hence removing it from required list.
> 
> CC: Praveen Talari <quic_ptalari@quicinc.com>
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> ---
>  .../bindings/soc/qcom/qcom,geni-se.yaml       | 47 +++++++++++++++++--
>  1 file changed, 43 insertions(+), 4 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub']
	from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000: 'clock-names' is a required property
	from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000: Unevaluated properties are not allowed ('compatible' was unexpected)
	from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart']
	from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:power-domains: [[4294967295, 4], [4294967295, 4]] is too long
	from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000: 'clock-names' is a required property
	from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000: Unevaluated properties are not allowed ('compatible', 'power-domain-names', 'power-domains' were unexpected)
	from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml#
Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: /example-1/soc/geniqup@9c0000/i2c@984000: failed to match any schema with compatible: ['qcom,sa8255p-geni-i2c']
Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: /example-1/soc/geniqup@9c0000/serial@990000: failed to match any schema with compatible: ['qcom,sa8255p-geni-uart']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240828203721.2751904-17-quic_nkela@quicinc.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.
Krzysztof Kozlowski Aug. 29, 2024, 7:42 a.m. UTC | #2
On Wed, Aug 28, 2024 at 01:37:15PM -0700, Nikunj Kela wrote:
> Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on
> SA8255p.
> 
> Clocks are being managed by the firmware VM and not required on
> SA8255p Linux VM hence removing it from required list.
> 
> CC: Praveen Talari <quic_ptalari@quicinc.com>
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> ---
>  .../bindings/soc/qcom/qcom,geni-se.yaml       | 47 +++++++++++++++++--
>  1 file changed, 43 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
> index 7b031ef09669..40e3a3e045da 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
> @@ -22,17 +22,16 @@ properties:
>      enum:
>        - qcom,geni-se-qup
>        - qcom,geni-se-i2c-master-hub
> +      - qcom,sa8255p-geni-se-qup

Same problems. If you decide to use generic compatibles, it means it
covers all devices. Otherwise it does not make any sense.

>  
>    reg:
>      description: QUP wrapper common register address and length.
>      maxItems: 1
>  
>    clock-names:
> -    minItems: 1

Huh?

Best regards,
Krzysztof
Nikunj Kela Aug. 29, 2024, 2:23 p.m. UTC | #3
On 8/29/2024 12:42 AM, Krzysztof Kozlowski wrote:
> On Wed, Aug 28, 2024 at 01:37:15PM -0700, Nikunj Kela wrote:
>> Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on
>> SA8255p.
>>
>> Clocks are being managed by the firmware VM and not required on
>> SA8255p Linux VM hence removing it from required list.
>>
>> CC: Praveen Talari <quic_ptalari@quicinc.com>
>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>> ---
>>  .../bindings/soc/qcom/qcom,geni-se.yaml       | 47 +++++++++++++++++--
>>  1 file changed, 43 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
>> index 7b031ef09669..40e3a3e045da 100644
>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
>> @@ -22,17 +22,16 @@ properties:
>>      enum:
>>        - qcom,geni-se-qup
>>        - qcom,geni-se-i2c-master-hub
>> +      - qcom,sa8255p-geni-se-qup
> Same problems. If you decide to use generic compatibles, it means it
> covers all devices. Otherwise it does not make any sense.

Hi Krzysztof,

SA8255p platform is not compatible with generic ones. At the time
generic compatibles were added, no one thought of such platform will
appear in future. Please advise what should we do in this case?

Thanks,

-Nikunj

>>  
>>    reg:
>>      description: QUP wrapper common register address and length.
>>      maxItems: 1
>>  
>>    clock-names:
>> -    minItems: 1
> Huh?
>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Aug. 30, 2024, 9:58 a.m. UTC | #4
On 29/08/2024 16:23, Nikunj Kela wrote:
> 
> On 8/29/2024 12:42 AM, Krzysztof Kozlowski wrote:
>> On Wed, Aug 28, 2024 at 01:37:15PM -0700, Nikunj Kela wrote:
>>> Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on
>>> SA8255p.
>>>
>>> Clocks are being managed by the firmware VM and not required on
>>> SA8255p Linux VM hence removing it from required list.
>>>
>>> CC: Praveen Talari <quic_ptalari@quicinc.com>
>>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>>> ---
>>>  .../bindings/soc/qcom/qcom,geni-se.yaml       | 47 +++++++++++++++++--
>>>  1 file changed, 43 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
>>> index 7b031ef09669..40e3a3e045da 100644
>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
>>> @@ -22,17 +22,16 @@ properties:
>>>      enum:
>>>        - qcom,geni-se-qup
>>>        - qcom,geni-se-i2c-master-hub
>>> +      - qcom,sa8255p-geni-se-qup
>> Same problems. If you decide to use generic compatibles, it means it
>> covers all devices. Otherwise it does not make any sense.
> 
> Hi Krzysztof,
> 
> SA8255p platform is not compatible with generic ones. At the time
> generic compatibles were added, no one thought of such platform will

That's kind of obvious and expected yet these were added...

> appear in future. Please advise what should we do in this case?

I don't know. We keep telling - do not use generic compatibles, because
you will have something like this, but people use generic compatibles -
so what can I say? I told you so?

Can we get agreement that using generic compatibles is a wrong idea? Or
sort of promise - we won't use them? Or policy? I don't know, we can
move on assuming this was a mistake 8 years ago, approaches evolve,
reviews change, but I am just afraid I will be repeating the same to
several future contributions and every time come with long arguments
exhausting my energy - don't add generic compatibles.

If devices are not compatible, I suggest different bindings.

Best regards,
Krzysztof
Nikunj Kela Aug. 30, 2024, 2:55 p.m. UTC | #5
On 8/30/2024 2:58 AM, Krzysztof Kozlowski wrote:
> On 29/08/2024 16:23, Nikunj Kela wrote:
>> On 8/29/2024 12:42 AM, Krzysztof Kozlowski wrote:
>>> On Wed, Aug 28, 2024 at 01:37:15PM -0700, Nikunj Kela wrote:
>>>> Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on
>>>> SA8255p.
>>>>
>>>> Clocks are being managed by the firmware VM and not required on
>>>> SA8255p Linux VM hence removing it from required list.
>>>>
>>>> CC: Praveen Talari <quic_ptalari@quicinc.com>
>>>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>>>> ---
>>>>  .../bindings/soc/qcom/qcom,geni-se.yaml       | 47 +++++++++++++++++--
>>>>  1 file changed, 43 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
>>>> index 7b031ef09669..40e3a3e045da 100644
>>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
>>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
>>>> @@ -22,17 +22,16 @@ properties:
>>>>      enum:
>>>>        - qcom,geni-se-qup
>>>>        - qcom,geni-se-i2c-master-hub
>>>> +      - qcom,sa8255p-geni-se-qup
>>> Same problems. If you decide to use generic compatibles, it means it
>>> covers all devices. Otherwise it does not make any sense.
>> Hi Krzysztof,
>>
>> SA8255p platform is not compatible with generic ones. At the time
>> generic compatibles were added, no one thought of such platform will
> That's kind of obvious and expected yet these were added...
>
>> appear in future. Please advise what should we do in this case?
> I don't know. We keep telling - do not use generic compatibles, because
> you will have something like this, but people use generic compatibles -
> so what can I say? I told you so?
>
> Can we get agreement that using generic compatibles is a wrong idea? Or
> sort of promise - we won't use them? Or policy? I don't know, we can
> move on assuming this was a mistake 8 years ago, approaches evolve,
> reviews change, but I am just afraid I will be repeating the same to
> several future contributions and every time come with long arguments
> exhausting my energy - don't add generic compatibles.
>
> If devices are not compatible, I suggest different bindings.
>
> Best regards,
> Krzysztof

Hi Krzysztof,

I will bring your concerns (raised above) to Qualcomm leads' attention.
Thank you for your feedback and support.

Thanks,

-Nikunj

>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
index 7b031ef09669..40e3a3e045da 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
@@ -22,17 +22,16 @@  properties:
     enum:
       - qcom,geni-se-qup
       - qcom,geni-se-i2c-master-hub
+      - qcom,sa8255p-geni-se-qup
 
   reg:
     description: QUP wrapper common register address and length.
     maxItems: 1
 
   clock-names:
-    minItems: 1
     maxItems: 2
 
   clocks:
-    minItems: 1
     maxItems: 2
 
   "#address-cells":
@@ -57,8 +56,6 @@  properties:
 required:
   - compatible
   - reg
-  - clock-names
-  - clocks
   - "#address-cells"
   - "#size-cells"
   - ranges
@@ -83,6 +80,17 @@  patternProperties:
     $ref: /schemas/serial/qcom,serial-geni-qcom.yaml#
 
 allOf:
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: qcom,sa8255p-geni-se-qup
+    then:
+      required:
+        - clocks
+        - clock-names
+
   - if:
       properties:
         compatible:
@@ -162,4 +170,35 @@  examples:
         };
     };
 
+  - |
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        geniqup@9c0000 {
+            compatible = "qcom,sa8255p-geni-se-qup";
+            reg = <0 0x9c0000 0 0x6000>;
+            #address-cells = <2>;
+            #size-cells = <2>;
+            ranges;
+
+            i2c1: i2c@984000 {
+                compatible = "qcom,sa8255p-geni-i2c";
+                reg = <0 0x984000 0 0x4000>;
+                interrupts = <GIC_SPI 551 IRQ_TYPE_LEVEL_HIGH>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+                power-domains = <&scmi9_pd 1>;
+            };
+
+            uart4: serial@990000 {
+                compatible = "qcom,sa8255p-geni-uart";
+                reg = <0 0x990000 0 0x4000>;
+                interrupts = <GIC_SPI 531 IRQ_TYPE_LEVEL_HIGH>;
+                power-domains = <&scmi11_pd 4>, <&scmi11_dvfs 4>;
+                power-domain-names = "power", "perf";
+            };
+        };
+    };
 ...