diff mbox series

[3/3] dt-bindings: PCI: ti,j721e-pci-host: Add support for J722S SoC

Message ID 20240117102526.557006-4-s-vadapalli@ti.com (mailing list archive)
State New, archived
Headers show
Series Fix and update ti,j721e-pci-* bindings | expand

Commit Message

s-vadapalli Jan. 17, 2024, 10:25 a.m. UTC
TI's J722S SoC has one instance of a Gen3 Single Lane PCIe controller.
The controller on J722S SoC is similar to the one present on TI's AM64
SoC, with the difference being that the controller on AM64 SoC supports
up to Gen2 link speed while the one on J722S SoC supports Gen3 link speed.

Update the bindings with a new compatible for J722S SoC and enforce checks
for "num-lanes" and "max-link-speed".

Technical Reference Manual of J722S SoC: https://www.ti.com/lit/zip/sprujb3

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 .../devicetree/bindings/pci/ti,j721e-pci-host.yaml  | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Krzysztof Kozlowski Jan. 17, 2024, 10:36 a.m. UTC | #1
On 17/01/2024 11:25, Siddharth Vadapalli wrote:
> TI's J722S SoC has one instance of a Gen3 Single Lane PCIe controller.
> The controller on J722S SoC is similar to the one present on TI's AM64
> SoC, with the difference being that the controller on AM64 SoC supports
> up to Gen2 link speed while the one on J722S SoC supports Gen3 link speed.
> 
> Update the bindings with a new compatible for J722S SoC and enforce checks
> for "num-lanes" and "max-link-speed".
> 
> Technical Reference Manual of J722S SoC: https://www.ti.com/lit/zip/sprujb3
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>  .../devicetree/bindings/pci/ti,j721e-pci-host.yaml  | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> index 005546dc8bd4..b7648f7e73c9 100644
> --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> @@ -14,6 +14,7 @@ properties:
>    compatible:
>      oneOf:
>        - const: ti,j721e-pcie-host
> +      - const: ti,j722s-pcie-host
>        - const: ti,j784s4-pcie-host
>        - description: PCIe controller in AM64
>          items:
> @@ -134,6 +135,18 @@ allOf:
>            minimum: 1
>            maximum: 4
>  
> +  - if:
> +      properties:
> +        compatible:
> +          items:

enum

> +            - const: ti,j722s-pcie-host
> +    then:
> +      properties:
> +        max-link-speed:
> +          const: 3
> +        num-lanes:
> +          const: 1

Similarly to previous patch: What is the point of all this? You have
direct mapping compatible-property, so encode these in the drivers.

Best regards,
Krzysztof
s-vadapalli Jan. 17, 2024, 11:24 a.m. UTC | #2
On 17/01/24 16:06, Krzysztof Kozlowski wrote:
> On 17/01/2024 11:25, Siddharth Vadapalli wrote:
>> TI's J722S SoC has one instance of a Gen3 Single Lane PCIe controller.
>> The controller on J722S SoC is similar to the one present on TI's AM64
>> SoC, with the difference being that the controller on AM64 SoC supports
>> up to Gen2 link speed while the one on J722S SoC supports Gen3 link speed.
>>
>> Update the bindings with a new compatible for J722S SoC and enforce checks
>> for "num-lanes" and "max-link-speed".
>>
>> Technical Reference Manual of J722S SoC: https://www.ti.com/lit/zip/sprujb3
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>  .../devicetree/bindings/pci/ti,j721e-pci-host.yaml  | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
>> index 005546dc8bd4..b7648f7e73c9 100644
>> --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
>> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
>> @@ -14,6 +14,7 @@ properties:
>>    compatible:
>>      oneOf:
>>        - const: ti,j721e-pcie-host
>> +      - const: ti,j722s-pcie-host
>>        - const: ti,j784s4-pcie-host
>>        - description: PCIe controller in AM64
>>          items:
>> @@ -134,6 +135,18 @@ allOf:
>>            minimum: 1
>>            maximum: 4
>>  
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          items:
> 
> enum
> 
>> +            - const: ti,j722s-pcie-host
>> +    then:
>> +      properties:
>> +        max-link-speed:
>> +          const: 3
>> +        num-lanes:
>> +          const: 1
> 
> Similarly to previous patch: What is the point of all this? You have
> direct mapping compatible-property, so encode these in the drivers.

Ok. I will drop patches 1 and 2 of this series and only post v2 of this patch
for adding a new compatible for J722S SoC without any checks for
"max-link-speed" or "num-lanes" for the new compatible.
Krzysztof Kozlowski Jan. 17, 2024, 11:35 a.m. UTC | #3
On 17/01/2024 12:24, Siddharth Vadapalli wrote:
> 
> 
> On 17/01/24 16:06, Krzysztof Kozlowski wrote:
>> On 17/01/2024 11:25, Siddharth Vadapalli wrote:
>>> TI's J722S SoC has one instance of a Gen3 Single Lane PCIe controller.
>>> The controller on J722S SoC is similar to the one present on TI's AM64
>>> SoC, with the difference being that the controller on AM64 SoC supports
>>> up to Gen2 link speed while the one on J722S SoC supports Gen3 link speed.
>>>
>>> Update the bindings with a new compatible for J722S SoC and enforce checks
>>> for "num-lanes" and "max-link-speed".
>>>
>>> Technical Reference Manual of J722S SoC: https://www.ti.com/lit/zip/sprujb3
>>>
>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>> ---
>>>  .../devicetree/bindings/pci/ti,j721e-pci-host.yaml  | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
>>> index 005546dc8bd4..b7648f7e73c9 100644
>>> --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
>>> @@ -14,6 +14,7 @@ properties:
>>>    compatible:
>>>      oneOf:
>>>        - const: ti,j721e-pcie-host
>>> +      - const: ti,j722s-pcie-host
>>>        - const: ti,j784s4-pcie-host
>>>        - description: PCIe controller in AM64
>>>          items:
>>> @@ -134,6 +135,18 @@ allOf:
>>>            minimum: 1
>>>            maximum: 4
>>>  
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          items:
>>
>> enum
>>
>>> +            - const: ti,j722s-pcie-host
>>> +    then:
>>> +      properties:
>>> +        max-link-speed:
>>> +          const: 3
>>> +        num-lanes:
>>> +          const: 1
>>
>> Similarly to previous patch: What is the point of all this? You have
>> direct mapping compatible-property, so encode these in the drivers.
> 
> Ok. I will drop patches 1 and 2 of this series and only post v2 of this patch
> for adding a new compatible for J722S SoC without any checks for
> "max-link-speed" or "num-lanes" for the new compatible.

Without driver change? So how does it solve my comment. I want to move
all these unnecessary properties to the driver.

Best regards,
Krzysztof
s-vadapalli Jan. 17, 2024, 11:41 a.m. UTC | #4
On 17/01/24 17:05, Krzysztof Kozlowski wrote:
> On 17/01/2024 12:24, Siddharth Vadapalli wrote:
>>
>>
>> On 17/01/24 16:06, Krzysztof Kozlowski wrote:
>>> On 17/01/2024 11:25, Siddharth Vadapalli wrote:
>>>> TI's J722S SoC has one instance of a Gen3 Single Lane PCIe controller.
>>>> The controller on J722S SoC is similar to the one present on TI's AM64
>>>> SoC, with the difference being that the controller on AM64 SoC supports
>>>> up to Gen2 link speed while the one on J722S SoC supports Gen3 link speed.
>>>>
>>>> Update the bindings with a new compatible for J722S SoC and enforce checks
>>>> for "num-lanes" and "max-link-speed".
>>>>
>>>> Technical Reference Manual of J722S SoC: https://www.ti.com/lit/zip/sprujb3
>>>>
>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>> ---
>>>>  .../devicetree/bindings/pci/ti,j721e-pci-host.yaml  | 13 +++++++++++++
>>>>  1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
>>>> index 005546dc8bd4..b7648f7e73c9 100644
>>>> --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
>>>> @@ -14,6 +14,7 @@ properties:
>>>>    compatible:
>>>>      oneOf:
>>>>        - const: ti,j721e-pcie-host
>>>> +      - const: ti,j722s-pcie-host
>>>>        - const: ti,j784s4-pcie-host
>>>>        - description: PCIe controller in AM64
>>>>          items:
>>>> @@ -134,6 +135,18 @@ allOf:
>>>>            minimum: 1
>>>>            maximum: 4
>>>>  
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          items:
>>>
>>> enum
>>>
>>>> +            - const: ti,j722s-pcie-host
>>>> +    then:
>>>> +      properties:
>>>> +        max-link-speed:
>>>> +          const: 3
>>>> +        num-lanes:
>>>> +          const: 1
>>>
>>> Similarly to previous patch: What is the point of all this? You have
>>> direct mapping compatible-property, so encode these in the drivers.
>>
>> Ok. I will drop patches 1 and 2 of this series and only post v2 of this patch
>> for adding a new compatible for J722S SoC without any checks for
>> "max-link-speed" or "num-lanes" for the new compatible.
> 
> Without driver change? So how does it solve my comment. I want to move
> all these unnecessary properties to the driver.

Driver support for verifying "num-lanes" is already present. I meant to say that
I will drop the checks in bindings in the v2 patch since "num-lanes" is verified
in driver too. However, "max-link-speed" is not verified either in the driver or
in the bindings prior to this series.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
index 005546dc8bd4..b7648f7e73c9 100644
--- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
+++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
@@ -14,6 +14,7 @@  properties:
   compatible:
     oneOf:
       - const: ti,j721e-pcie-host
+      - const: ti,j722s-pcie-host
       - const: ti,j784s4-pcie-host
       - description: PCIe controller in AM64
         items:
@@ -134,6 +135,18 @@  allOf:
           minimum: 1
           maximum: 4
 
+  - if:
+      properties:
+        compatible:
+          items:
+            - const: ti,j722s-pcie-host
+    then:
+      properties:
+        max-link-speed:
+          const: 3
+        num-lanes:
+          const: 1
+
   - if:
       properties:
         compatible: