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

Nikunj Kela Aug. 29, 2024, 2:23 p.m. UTC | #1
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 | #2
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 | #3
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";
+            };
+        };
+    };
 ...