| Message ID | 20240117102526.557006-4-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: > 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
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.
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
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 --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:
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(+)