2024-01-17 10:27:05

by Siddharth Vadapalli

[permalink] [raw]
Subject: [PATCH 0/3] Fix and update ti,j721e-pci-* bindings

Hello,

This series fixes the bindings for:
Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml
Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
by updating the checks added for validating and enforcing the
"num-lanes" property for different compatibles. In the commits
which introduced and extended the checks for the "num-lanes"
property, the property was not truly validated but only described.
Therefore, the bindings are being updated to actually validate
the "num-lanes" property. While at it, checks for "max-link-speed"
are also being introduced. The intent of the aforementioned changes
is to update the bindings for a new SoC namely TI's J722S SoC which
has a similar PCIe controller to TI's AM64 SoC, but differs from it
in terms of its support for Gen3 link speeds. For this reason, a new
compatible is being added instead of reusing the one available for
AM64 SoC.

Series is based on linux-next tagged next-20240117.

Regards,
Siddharth.

Siddharth Vadapalli (3):
dt-bindings: PCI: ti,j721e-pci-*: Fix check for num-lanes
dt-bindings: PCI: ti,j721e-pci-*: Add checks for max-link-speed
dt-bindings: PCI: ti,j721e-pci-host: Add support for J722S

.../bindings/pci/ti,j721e-pci-ep.yaml | 34 +++++++++++---
.../bindings/pci/ti,j721e-pci-host.yaml | 47 ++++++++++++++++---
2 files changed, 67 insertions(+), 14 deletions(-)

--
2.34.1



2024-01-17 10:27:11

by Siddharth Vadapalli

[permalink] [raw]
Subject: [PATCH 2/3] dt-bindings: PCI: ti,j721e-pci-*: Add checks for max-link-speed

Extend the existing compatible based checks for validating and enforcing
the "max-link-speed" property.

Signed-off-by: Siddharth Vadapalli <[email protected]>
---
.../devicetree/bindings/pci/ti,j721e-pci-ep.yaml | 8 ++++++++
.../devicetree/bindings/pci/ti,j721e-pci-host.yaml | 8 ++++++++
2 files changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml
index 278e0892f8ac..4839a9574e20 100644
--- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml
@@ -73,6 +73,8 @@ allOf:
- const: ti,j721e-pcie-ep
then:
properties:
+ max-link-speed:
+ const: 2
num-lanes:
const: 1

@@ -84,6 +86,8 @@ allOf:
- const: ti,j721e-pcie-ep
then:
properties:
+ max-link-speed:
+ const: 3
num-lanes:
minimum: 1
maximum: 2
@@ -95,6 +99,8 @@ allOf:
- const: ti,j721e-pcie-ep
then:
properties:
+ max-link-speed:
+ const: 3
num-lanes:
minimum: 1
maximum: 4
@@ -106,6 +112,8 @@ allOf:
- const: ti,j784s4-pcie-ep
then:
properties:
+ max-link-speed:
+ const: 3
num-lanes:
minimum: 1
maximum: 4
diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
index 36bcc8cb7896..005546dc8bd4 100644
--- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
+++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
@@ -102,6 +102,8 @@ allOf:
- const: ti,j721e-pcie-host
then:
properties:
+ max-link-speed:
+ const: 2
num-lanes:
const: 1

@@ -113,6 +115,8 @@ allOf:
- const: ti,j721e-pcie-host
then:
properties:
+ max-link-speed:
+ const: 3
num-lanes:
minimum: 1
maximum: 2
@@ -124,6 +128,8 @@ allOf:
- const: ti,j721e-pcie-host
then:
properties:
+ max-link-speed:
+ const: 3
num-lanes:
minimum: 1
maximum: 4
@@ -135,6 +141,8 @@ allOf:
- const: ti,j784s4-pcie-host
then:
properties:
+ max-link-speed:
+ const: 3
num-lanes:
minimum: 1
maximum: 4
--
2.34.1


2024-01-17 10:36:08

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: PCI: ti,j721e-pci-*: Add checks for max-link-speed

On 17/01/2024 11:25, Siddharth Vadapalli wrote:
> Extend the existing compatible based checks for validating and enforcing
> the "max-link-speed" property.

Based on what? Driver or hardware? Your entire change suggests you
should just drop it from the binding, because this can be deduced from
compatible.

Best regards,
Krzysztof


2024-01-17 10:58:54

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: PCI: ti,j721e-pci-*: Add checks for max-link-speed



On 17/01/24 16:05, Krzysztof Kozlowski wrote:
> On 17/01/2024 11:25, Siddharth Vadapalli wrote:
>> Extend the existing compatible based checks for validating and enforcing
>> the "max-link-speed" property.
>
> Based on what? Driver or hardware? Your entire change suggests you

Hardware. The PCIe controller on AM64 SoC supports up to Gen2 link speed while
the PCIe controllers on other SoCs support Gen3 link speed.

> should just drop it from the binding, because this can be deduced from
> compatible.

Could you please clarify? Isn't the addition of the checks for "max-link-speed"
identical to the checks which were added for "num-lanes", both of which are
Hardware specific?

--
Regards,
Siddharth.

2024-01-17 11:00:45

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: PCI: ti,j721e-pci-*: Add checks for max-link-speed

On 17/01/2024 11:58, Siddharth Vadapalli wrote:
> On 17/01/24 16:05, Krzysztof Kozlowski wrote:
>> On 17/01/2024 11:25, Siddharth Vadapalli wrote:
>>> Extend the existing compatible based checks for validating and enforcing
>>> the "max-link-speed" property.
>>
>> Based on what? Driver or hardware? Your entire change suggests you
>
> Hardware. The PCIe controller on AM64 SoC supports up to Gen2 link speed while
> the PCIe controllers on other SoCs support Gen3 link speed.
>
>> should just drop it from the binding, because this can be deduced from
>> compatible.
>
> Could you please clarify? Isn't the addition of the checks for "max-link-speed"
> identical to the checks which were added for "num-lanes", both of which are
> Hardware specific?

Compatible defines these values, at least what it looks like from the patch.

Best regards,
Krzysztof


2024-01-17 11:16:31

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: PCI: ti,j721e-pci-*: Add checks for max-link-speed



On 17/01/24 16:30, Krzysztof Kozlowski wrote:
> On 17/01/2024 11:58, Siddharth Vadapalli wrote:
>> On 17/01/24 16:05, Krzysztof Kozlowski wrote:
>>> On 17/01/2024 11:25, Siddharth Vadapalli wrote:
>>>> Extend the existing compatible based checks for validating and enforcing
>>>> the "max-link-speed" property.
>>>
>>> Based on what? Driver or hardware? Your entire change suggests you
>>
>> Hardware. The PCIe controller on AM64 SoC supports up to Gen2 link speed while
>> the PCIe controllers on other SoCs support Gen3 link speed.
>>
>>> should just drop it from the binding, because this can be deduced from
>>> compatible.
>>
>> Could you please clarify? Isn't the addition of the checks for "max-link-speed"
>> identical to the checks which were added for "num-lanes", both of which are
>> Hardware specific?
>
> Compatible defines these values, at least what it looks like from the patch.

In this patch, I have added checks for the "max-link-speed" property in the same
section that "num-lanes" is being evaluated. The values for "max-link-speed" are
based on the Hardware support and this patch is validating the "max-link-speed"
property in the device-tree nodes for the devices against the Hardware supported
values which this patch is adding in the corresponding section. Kindly let me
know if I misunderstood what you meant to convey.

--
Regards,
Siddharth.

2024-01-17 11:20:02

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: PCI: ti,j721e-pci-*: Add checks for max-link-speed

On 17/01/2024 12:15, Siddharth Vadapalli wrote:
>
>
> On 17/01/24 16:30, Krzysztof Kozlowski wrote:
>> On 17/01/2024 11:58, Siddharth Vadapalli wrote:
>>> On 17/01/24 16:05, Krzysztof Kozlowski wrote:
>>>> On 17/01/2024 11:25, Siddharth Vadapalli wrote:
>>>>> Extend the existing compatible based checks for validating and enforcing
>>>>> the "max-link-speed" property.
>>>>
>>>> Based on what? Driver or hardware? Your entire change suggests you
>>>
>>> Hardware. The PCIe controller on AM64 SoC supports up to Gen2 link speed while
>>> the PCIe controllers on other SoCs support Gen3 link speed.
>>>
>>>> should just drop it from the binding, because this can be deduced from
>>>> compatible.
>>>
>>> Could you please clarify? Isn't the addition of the checks for "max-link-speed"
>>> identical to the checks which were added for "num-lanes", both of which are
>>> Hardware specific?
>>
>> Compatible defines these values, at least what it looks like from the patch.
>
> In this patch, I have added checks for the "max-link-speed" property in the same
> section that "num-lanes" is being evaluated.

I know what you did in patch. I read it.

> The values for "max-link-speed" are
> based on the Hardware support and this patch is validating the "max-link-speed"
> property in the device-tree nodes for the devices against the Hardware supported
> values which this patch is adding in the corresponding section. Kindly let me
> know if I misunderstood what you meant to convey.

Nothing of this is relevant.

I used two entirely different wordings for this and you still don't get
it, so I don't know if I have third one.

Maybe this:
Move it to driver match data.

So three entirely different wordings for the same. I don't have fourth...

Best regards,
Krzysztof


2024-01-17 11:22:59

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: PCI: ti,j721e-pci-*: Add checks for max-link-speed



On 17/01/24 16:49, Krzysztof Kozlowski wrote:
> On 17/01/2024 12:15, Siddharth Vadapalli wrote:
>>
>>
>> On 17/01/24 16:30, Krzysztof Kozlowski wrote:
>>> On 17/01/2024 11:58, Siddharth Vadapalli wrote:
>>>> On 17/01/24 16:05, Krzysztof Kozlowski wrote:
>>>>> On 17/01/2024 11:25, Siddharth Vadapalli wrote:
>>>>>> Extend the existing compatible based checks for validating and enforcing
>>>>>> the "max-link-speed" property.
>>>>>
>>>>> Based on what? Driver or hardware? Your entire change suggests you
>>>>
>>>> Hardware. The PCIe controller on AM64 SoC supports up to Gen2 link speed while
>>>> the PCIe controllers on other SoCs support Gen3 link speed.
>>>>
>>>>> should just drop it from the binding, because this can be deduced from
>>>>> compatible.
>>>>
>>>> Could you please clarify? Isn't the addition of the checks for "max-link-speed"
>>>> identical to the checks which were added for "num-lanes", both of which are
>>>> Hardware specific?
>>>
>>> Compatible defines these values, at least what it looks like from the patch.
>>
>> In this patch, I have added checks for the "max-link-speed" property in the same
>> section that "num-lanes" is being evaluated.
>
> I know what you did in patch. I read it.
>
>> The values for "max-link-speed" are
>> based on the Hardware support and this patch is validating the "max-link-speed"
>> property in the device-tree nodes for the devices against the Hardware supported
>> values which this patch is adding in the corresponding section. Kindly let me
>> know if I misunderstood what you meant to convey.
>
> Nothing of this is relevant.
>
> I used two entirely different wordings for this and you still don't get
> it, so I don't know if I have third one.
>
> Maybe this:
> Move it to driver match data.

Ok. I will drop the checks for "max-link-speed" and move them to the driver. But
I wonder why the checks for "num-lanes" are needed in the first place when they
could be in the driver as well.

>
> So three entirely different wordings for the same. I don't have fourth...
>
> Best regards,
> Krzysztof
>

--
Regards,
Siddharth.

2024-01-17 11:34:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: PCI: ti,j721e-pci-*: Add checks for max-link-speed

On 17/01/2024 12:22, Siddharth Vadapalli wrote:
>
>
> On 17/01/24 16:49, Krzysztof Kozlowski wrote:
>> On 17/01/2024 12:15, Siddharth Vadapalli wrote:
>>>
>>>
>>> On 17/01/24 16:30, Krzysztof Kozlowski wrote:
>>>> On 17/01/2024 11:58, Siddharth Vadapalli wrote:
>>>>> On 17/01/24 16:05, Krzysztof Kozlowski wrote:
>>>>>> On 17/01/2024 11:25, Siddharth Vadapalli wrote:
>>>>>>> Extend the existing compatible based checks for validating and enforcing
>>>>>>> the "max-link-speed" property.
>>>>>>
>>>>>> Based on what? Driver or hardware? Your entire change suggests you
>>>>>
>>>>> Hardware. The PCIe controller on AM64 SoC supports up to Gen2 link speed while
>>>>> the PCIe controllers on other SoCs support Gen3 link speed.
>>>>>
>>>>>> should just drop it from the binding, because this can be deduced from
>>>>>> compatible.
>>>>>
>>>>> Could you please clarify? Isn't the addition of the checks for "max-link-speed"
>>>>> identical to the checks which were added for "num-lanes", both of which are
>>>>> Hardware specific?
>>>>
>>>> Compatible defines these values, at least what it looks like from the patch.
>>>
>>> In this patch, I have added checks for the "max-link-speed" property in the same
>>> section that "num-lanes" is being evaluated.
>>
>> I know what you did in patch. I read it.
>>
>>> The values for "max-link-speed" are
>>> based on the Hardware support and this patch is validating the "max-link-speed"
>>> property in the device-tree nodes for the devices against the Hardware supported
>>> values which this patch is adding in the corresponding section. Kindly let me
>>> know if I misunderstood what you meant to convey.
>>
>> Nothing of this is relevant.
>>
>> I used two entirely different wordings for this and you still don't get
>> it, so I don't know if I have third one.
>>
>> Maybe this:
>> Move it to driver match data.
>
> Ok. I will drop the checks for "max-link-speed" and move them to the driver. But
> I wonder why the checks for "num-lanes" are needed in the first place when they
> could be in the driver as well.

Yes, that's why I commented in your next patch.

Best regards,
Krzysztof