| Message ID | 20240117102526.557006-3-s-vadapalli@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | Fix and update ti,j721e-pci-* bindings | expand |
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
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?
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
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.
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
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 >
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
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
Extend the existing compatible based checks for validating and enforcing the "max-link-speed" property. Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> --- .../devicetree/bindings/pci/ti,j721e-pci-ep.yaml | 8 ++++++++ .../devicetree/bindings/pci/ti,j721e-pci-host.yaml | 8 ++++++++ 2 files changed, 16 insertions(+)