2024-01-17 10:26:51

by Siddharth Vadapalli

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

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 <[email protected]>
---
.../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:
+ - const: ti,j722s-pcie-host
+ then:
+ properties:
+ max-link-speed:
+ const: 3
+ num-lanes:
+ const: 1
+
- if:
properties:
compatible:
--
2.34.1



2024-01-17 10:36:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: PCI: ti,j721e-pci-host: Add support for J722S SoC

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 <[email protected]>
> ---
> .../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


2024-01-17 11:25:22

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: PCI: ti,j721e-pci-host: Add support for J722S SoC



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 <[email protected]>
>> ---
>> .../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.

--
Regards,
Siddharth.

2024-01-17 11:35:31

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: PCI: ti,j721e-pci-host: Add support for J722S SoC

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 <[email protected]>
>>> ---
>>> .../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


2024-01-17 11:42:05

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: PCI: ti,j721e-pci-host: Add support for J722S SoC



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 <[email protected]>
>>>> ---
>>>> .../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.

--
Regards,
Siddharth.