| Message ID | 20221107204934.32655-2-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6,01/20] dt-bindings: imx6q-pcie: Fix clock names for imx6sx and imx8mq | expand |
On Mon, Nov 07, 2022 at 11:49:15PM +0300, Serge Semin wrote: > Originally as it was defined the legacy bindings the pcie_inbound_axi and > pcie_aux clock names were supposed to be used in the fsl,imx6sx-pcie and > fsl,imx8mq-pcie devices respectively. But the bindings conversion has been > incorrectly so now the fourth clock name is defined as "pcie_inbound_axi > for imx6sx-pcie, pcie_aux for imx8mq-pcie", which is completely wrong. > Let's fix that by conditionally apply the clock-names constraints based on > the compatible string content. > > Fixes: 751ca492f131 ("dt-bindings: PCI: imx6: convert the imx pcie controller to dtschema") > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > --- > > Changelog v5: > - This is a new patch added on the v5 release of the patchset. > --- > .../bindings/pci/fsl,imx6q-pcie.yaml | 47 +++++++++++++++++-- > 1 file changed, 42 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > index 376e739bcad4..ebfe75f1576e 100644 > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > @@ -16,6 +16,47 @@ description: |+ > > allOf: > - $ref: /schemas/pci/snps,dw-pcie.yaml# > + - if: > + properties: > + compatible: > + contains: > + const: fsl,imx6sx-pcie > + then: > + properties: > + clock-names: > + items: > + - const: pcie > + - const: pcie_bus > + - const: pcie_phy > + - const: pcie_inbound_axi > + - if: > + properties: > + compatible: > + contains: > + const: fsl,imx8mq-pcie > + then: > + properties: > + clock-names: > + items: > + - const: pcie > + - const: pcie_bus > + - const: pcie_phy > + - const: pcie_aux > + - if: > + properties: > + compatible: > + not: > + contains: > + enum: > + - fsl,imx6sx-pcie > + - fsl,imx8mq-pcie > + then: > + properties: > + clock-names: > + items: > + - const: pcie > + - const: pcie_bus > + - const: pcie_phy > > properties: > compatible: > @@ -57,11 +98,7 @@ properties: > > clock-names: > minItems: 3 > - items: > - - const: pcie > - - const: pcie_bus > - - const: pcie_phy > - - const: pcie_inbound_axi for imx6sx-pcie, pcie_aux for imx8mq-pcie This should have been just 'enum: [ pcie_inbound_axi, pcie_aux ]' And then do: - if: properties: compatible: contains: const: fsl,imx8mq-pcie then: properties: clock-names: items: - {} - {} - {} - const: pcie_aux And then another if/then with 'maxItems: 3'
On Thu, Nov 10, 2022 at 03:01:04PM -0600, Rob Herring wrote: > On Mon, Nov 07, 2022 at 11:49:15PM +0300, Serge Semin wrote: > > Originally as it was defined the legacy bindings the pcie_inbound_axi and > > pcie_aux clock names were supposed to be used in the fsl,imx6sx-pcie and > > fsl,imx8mq-pcie devices respectively. But the bindings conversion has been > > incorrectly so now the fourth clock name is defined as "pcie_inbound_axi > > for imx6sx-pcie, pcie_aux for imx8mq-pcie", which is completely wrong. > > Let's fix that by conditionally apply the clock-names constraints based on > > the compatible string content. > > > > Fixes: 751ca492f131 ("dt-bindings: PCI: imx6: convert the imx pcie controller to dtschema") > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > > > --- > > > > Changelog v5: > > - This is a new patch added on the v5 release of the patchset. > > --- > > .../bindings/pci/fsl,imx6q-pcie.yaml | 47 +++++++++++++++++-- > > 1 file changed, 42 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > index 376e739bcad4..ebfe75f1576e 100644 > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > @@ -16,6 +16,47 @@ description: |+ > > > > allOf: > > - $ref: /schemas/pci/snps,dw-pcie.yaml# > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: fsl,imx6sx-pcie > > + then: > > + properties: > > + clock-names: > > + items: > > + - const: pcie > > + - const: pcie_bus > > + - const: pcie_phy > > + - const: pcie_inbound_axi > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: fsl,imx8mq-pcie > > + then: > > + properties: > > + clock-names: > > + items: > > + - const: pcie > > + - const: pcie_bus > > + - const: pcie_phy > > + - const: pcie_aux > > + - if: > > + properties: > > + compatible: > > + not: > > + contains: > > + enum: > > + - fsl,imx6sx-pcie > > + - fsl,imx8mq-pcie > > + then: > > + properties: > > + clock-names: > > + items: > > + - const: pcie > > + - const: pcie_bus > > + - const: pcie_phy > > > > properties: > > compatible: > > @@ -57,11 +98,7 @@ properties: > > > > clock-names: > > minItems: 3 > > - items: > > - - const: pcie > > - - const: pcie_bus > > - - const: pcie_phy > > - - const: pcie_inbound_axi for imx6sx-pcie, pcie_aux for imx8mq-pcie > > This should have been just 'enum: [ pcie_inbound_axi, pcie_aux ]' > > And then do: > > - if: > properties: > compatible: > contains: > const: fsl,imx8mq-pcie > then: > properties: > clock-names: > items: > - {} > - {} > - {} > - const: pcie_aux > > > And then another if/then with 'maxItems: 3' Ok. Will fix it in v7. But IMO it looks a bit less descriptive especially with the '{}' pattern and a need to look in two different places to comprehend the whole constraint. I understand though what is an intention of such construction. It's to place as much info into the schema body and isolate the platform-specific constraints in the allOf clause. Pretty neat anyway. -Sergey
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml index 376e739bcad4..ebfe75f1576e 100644 --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml @@ -16,6 +16,47 @@ description: |+ allOf: - $ref: /schemas/pci/snps,dw-pcie.yaml# + - if: + properties: + compatible: + contains: + const: fsl,imx6sx-pcie + then: + properties: + clock-names: + items: + - const: pcie + - const: pcie_bus + - const: pcie_phy + - const: pcie_inbound_axi + - if: + properties: + compatible: + contains: + const: fsl,imx8mq-pcie + then: + properties: + clock-names: + items: + - const: pcie + - const: pcie_bus + - const: pcie_phy + - const: pcie_aux + - if: + properties: + compatible: + not: + contains: + enum: + - fsl,imx6sx-pcie + - fsl,imx8mq-pcie + then: + properties: + clock-names: + items: + - const: pcie + - const: pcie_bus + - const: pcie_phy properties: compatible: @@ -57,11 +98,7 @@ properties: clock-names: minItems: 3 - items: - - const: pcie - - const: pcie_bus - - const: pcie_phy - - const: pcie_inbound_axi for imx6sx-pcie, pcie_aux for imx8mq-pcie + maxItems: 4 num-lanes: const: 1
Originally as it was defined the legacy bindings the pcie_inbound_axi and pcie_aux clock names were supposed to be used in the fsl,imx6sx-pcie and fsl,imx8mq-pcie devices respectively. But the bindings conversion has been incorrectly so now the fourth clock name is defined as "pcie_inbound_axi for imx6sx-pcie, pcie_aux for imx8mq-pcie", which is completely wrong. Let's fix that by conditionally apply the clock-names constraints based on the compatible string content. Fixes: 751ca492f131 ("dt-bindings: PCI: imx6: convert the imx pcie controller to dtschema") Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- Changelog v5: - This is a new patch added on the v5 release of the patchset. --- .../bindings/pci/fsl,imx6q-pcie.yaml | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-)