diff mbox series

[1/3] dt-bindings: PCI: Convert Arm Versatile binding to DT schema

Message ID 20191116005240.15722-1-robh@kernel.org
State Superseded
Headers show
Series [1/3] dt-bindings: PCI: Convert Arm Versatile binding to DT schema | expand

Commit Message

Rob Herring Nov. 16, 2019, 12:52 a.m. UTC
Convert the Arm Versatile PCI host binding to a DT schema.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Andrew Murray <andrew.murray@arm.com>
Signed-off-by: Rob Herring <robh@kernel.org>

---
 .../devicetree/bindings/pci/versatile.txt     | 59 ------------
 .../devicetree/bindings/pci/versatile.yaml    | 92 +++++++++++++++++++
 MAINTAINERS                                   |  2 +-
 3 files changed, 93 insertions(+), 60 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pci/versatile.txt
 create mode 100644 Documentation/devicetree/bindings/pci/versatile.yaml

-- 
2.20.1

Comments

Rob Herring Dec. 30, 2019, 11:29 p.m. UTC | #1
On Thu, Dec 12, 2019 at 7:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>

> Hi Rob,

>

> On Sat, Nov 16, 2019 at 1:53 AM Rob Herring <robh@kernel.org> wrote:

> > Convert the generic PCI host binding to DT schema. The derivative Juno,

> > PLDA XpressRICH3-AXI, and Designware ECAM bindings all just vary in

> > their compatible strings. The simplest way to convert those to

> > schema is just add them into the common generic PCI host schema.

> >

> > Cc: Bjorn Helgaas <bhelgaas@google.com>

> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> > Cc: Andrew Murray <andrew.murray@arm.com>

> > Cc: Zhou Wang <wangzhou1@hisilicon.com>

> > Cc: Will Deacon <will@kernel.org>

> > Cc: David Daney <david.daney@cavium.com>

> > Signed-off-by: Rob Herring <robh@kernel.org>

>

> > index 515b2f9542e5..000000000000

> > --- a/Documentation/devicetree/bindings/pci/designware-pcie-ecam.txt

> > +++ /dev/null

>

> > -Example:

> > -

> > -    pcie1: pcie@7f000000 {

> > -        compatible = "socionext,synquacer-pcie-ecam", "snps,dw-pcie-ecam";

> > -        device_type = "pci";

> > -        reg = <0x0 0x7f000000 0x0 0xf00000>;

> > -        bus-range = <0x0 0xe>;

> > -        #address-cells = <3>;

> > -        #size-cells = <2>;

> > -        ranges = <0x1000000 0x00 0x00010000 0x00 0x7ff00000 0x0 0x00010000>,

> > -                 <0x2000000 0x00 0x70000000 0x00 0x70000000 0x0 0x0f000000>,

> > -                 <0x3000000 0x3f 0x00000000 0x3f 0x00000000 0x1 0x00000000>;

> > -

> > -        #interrupt-cells = <0x1>;

> > -        interrupt-map-mask = <0x0 0x0 0x0 0x0>;

>

> An all-zeroes interrupt-map-mask seems to be very common on embedded

> SoCs, where all devices are mapped to a single interrupt.


Indeed.

> However, schemas/pci/pci-bus.yaml says:

>

>   interrupt-map-mask:

>     items:

>       - description: PCI high address cell

>         minimum: 0

>         maximum: 0xf800

>       - description: PCI mid address cell

>         const: 0

>       - description: PCI low address cell

>         const: 0

>       - description: PCI IRQ cell

>         minimum: 1

>         maximum: 7

>

> and thus complains about an all-zeroes mask, e.g.

>

>     arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml:

> pcie@fe000000: interrupt-map-mask:0:3: 0 is less than the minimum of 1


Now fixed.

Thanks,
Rob
Geert Uytterhoeven Dec. 31, 2019, 8:23 a.m. UTC | #2
Hi Rob,

On Tue, Dec 31, 2019 at 12:30 AM Rob Herring <robh@kernel.org> wrote:
> On Thu, Dec 12, 2019 at 7:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> > On Sat, Nov 16, 2019 at 1:53 AM Rob Herring <robh@kernel.org> wrote:

> > > Convert the generic PCI host binding to DT schema. The derivative Juno,

> > > PLDA XpressRICH3-AXI, and Designware ECAM bindings all just vary in

> > > their compatible strings. The simplest way to convert those to

> > > schema is just add them into the common generic PCI host schema.

> > >

> > > Cc: Bjorn Helgaas <bhelgaas@google.com>

> > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> > > Cc: Andrew Murray <andrew.murray@arm.com>

> > > Cc: Zhou Wang <wangzhou1@hisilicon.com>

> > > Cc: Will Deacon <will@kernel.org>

> > > Cc: David Daney <david.daney@cavium.com>

> > > Signed-off-by: Rob Herring <robh@kernel.org>

> >

> > > index 515b2f9542e5..000000000000

> > > --- a/Documentation/devicetree/bindings/pci/designware-pcie-ecam.txt

> > > +++ /dev/null

> >

> > > -Example:

> > > -

> > > -    pcie1: pcie@7f000000 {

> > > -        compatible = "socionext,synquacer-pcie-ecam", "snps,dw-pcie-ecam";

> > > -        device_type = "pci";

> > > -        reg = <0x0 0x7f000000 0x0 0xf00000>;

> > > -        bus-range = <0x0 0xe>;

> > > -        #address-cells = <3>;

> > > -        #size-cells = <2>;

> > > -        ranges = <0x1000000 0x00 0x00010000 0x00 0x7ff00000 0x0 0x00010000>,

> > > -                 <0x2000000 0x00 0x70000000 0x00 0x70000000 0x0 0x0f000000>,

> > > -                 <0x3000000 0x3f 0x00000000 0x3f 0x00000000 0x1 0x00000000>;

> > > -

> > > -        #interrupt-cells = <0x1>;

> > > -        interrupt-map-mask = <0x0 0x0 0x0 0x0>;

> >

> > An all-zeroes interrupt-map-mask seems to be very common on embedded

> > SoCs, where all devices are mapped to a single interrupt.

>

> Indeed.

>

> > However, schemas/pci/pci-bus.yaml says:

> >

> >   interrupt-map-mask:

> >     items:

> >       - description: PCI high address cell

> >         minimum: 0

> >         maximum: 0xf800

> >       - description: PCI mid address cell

> >         const: 0

> >       - description: PCI low address cell

> >         const: 0

> >       - description: PCI IRQ cell

> >         minimum: 1

> >         maximum: 7

> >

> > and thus complains about an all-zeroes mask, e.g.

> >

> >     arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml:

> > pcie@fe000000: interrupt-map-mask:0:3: 0 is less than the minimum of 1

>

> Now fixed.


Thank you, confirmed.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Geert Uytterhoeven Dec. 31, 2019, 2:31 p.m. UTC | #3
Hi Rob,

On Tue, Dec 31, 2019 at 9:23 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Dec 31, 2019 at 12:30 AM Rob Herring <robh@kernel.org> wrote:

> > On Thu, Dec 12, 2019 at 7:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> > > On Sat, Nov 16, 2019 at 1:53 AM Rob Herring <robh@kernel.org> wrote:

> > > > Convert the generic PCI host binding to DT schema. The derivative Juno,

> > > > PLDA XpressRICH3-AXI, and Designware ECAM bindings all just vary in

> > > > their compatible strings. The simplest way to convert those to

> > > > schema is just add them into the common generic PCI host schema.

> > > >

> > > > Cc: Bjorn Helgaas <bhelgaas@google.com>

> > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> > > > Cc: Andrew Murray <andrew.murray@arm.com>

> > > > Cc: Zhou Wang <wangzhou1@hisilicon.com>

> > > > Cc: Will Deacon <will@kernel.org>

> > > > Cc: David Daney <david.daney@cavium.com>

> > > > Signed-off-by: Rob Herring <robh@kernel.org>

> > >

> > > > index 515b2f9542e5..000000000000

> > > > --- a/Documentation/devicetree/bindings/pci/designware-pcie-ecam.txt

> > > > +++ /dev/null

> > >

> > > > -Example:

> > > > -

> > > > -    pcie1: pcie@7f000000 {

> > > > -        compatible = "socionext,synquacer-pcie-ecam", "snps,dw-pcie-ecam";

> > > > -        device_type = "pci";

> > > > -        reg = <0x0 0x7f000000 0x0 0xf00000>;

> > > > -        bus-range = <0x0 0xe>;

> > > > -        #address-cells = <3>;

> > > > -        #size-cells = <2>;

> > > > -        ranges = <0x1000000 0x00 0x00010000 0x00 0x7ff00000 0x0 0x00010000>,

> > > > -                 <0x2000000 0x00 0x70000000 0x00 0x70000000 0x0 0x0f000000>,

> > > > -                 <0x3000000 0x3f 0x00000000 0x3f 0x00000000 0x1 0x00000000>;

> > > > -

> > > > -        #interrupt-cells = <0x1>;

> > > > -        interrupt-map-mask = <0x0 0x0 0x0 0x0>;

> > >

> > > An all-zeroes interrupt-map-mask seems to be very common on embedded

> > > SoCs, where all devices are mapped to a single interrupt.

> >

> > Indeed.

> >

> > > However, schemas/pci/pci-bus.yaml says:

> > >

> > >   interrupt-map-mask:

> > >     items:

> > >       - description: PCI high address cell

> > >         minimum: 0

> > >         maximum: 0xf800

> > >       - description: PCI mid address cell

> > >         const: 0

> > >       - description: PCI low address cell

> > >         const: 0

> > >       - description: PCI IRQ cell

> > >         minimum: 1

> > >         maximum: 7

> > >

> > > and thus complains about an all-zeroes mask, e.g.

> > >

> > >     arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml:

> > > pcie@fe000000: interrupt-map-mask:0:3: 0 is less than the minimum of 1

> >

> > Now fixed.

>

> Thank you, confirmed.


And with latest renesas-drivers, I started seeing:

    arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:
interrupt-map:0: [0, 0, 0, 1] is too short
    arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:
interrupt-map:1: [5, 0, 113, 4] is too short
    arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:
interrupt-map:2: [2048, 0, 0, 1] is too short
    arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:
interrupt-map:3: [5, 0, 113, 4] is too short
    arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:
interrupt-map:4: [4096, 0, 0, 2] is too short
    arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:
interrupt-map:5: [5, 0, 113, 4] is too short

Looks like interrupt-map is split incorrectly: shouldn't each entry have 8
cells?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Rob Herring Dec. 31, 2019, 5:10 p.m. UTC | #4
On Tue, Dec 31, 2019 at 7:31 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>

> Hi Rob,

>

> On Tue, Dec 31, 2019 at 9:23 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> > On Tue, Dec 31, 2019 at 12:30 AM Rob Herring <robh@kernel.org> wrote:

> > > On Thu, Dec 12, 2019 at 7:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> > > > On Sat, Nov 16, 2019 at 1:53 AM Rob Herring <robh@kernel.org> wrote:

> > > > > Convert the generic PCI host binding to DT schema. The derivative Juno,

> > > > > PLDA XpressRICH3-AXI, and Designware ECAM bindings all just vary in

> > > > > their compatible strings. The simplest way to convert those to

> > > > > schema is just add them into the common generic PCI host schema.

> > > > >

> > > > > Cc: Bjorn Helgaas <bhelgaas@google.com>

> > > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> > > > > Cc: Andrew Murray <andrew.murray@arm.com>

> > > > > Cc: Zhou Wang <wangzhou1@hisilicon.com>

> > > > > Cc: Will Deacon <will@kernel.org>

> > > > > Cc: David Daney <david.daney@cavium.com>

> > > > > Signed-off-by: Rob Herring <robh@kernel.org>

> > > >

> > > > > index 515b2f9542e5..000000000000

> > > > > --- a/Documentation/devicetree/bindings/pci/designware-pcie-ecam.txt

> > > > > +++ /dev/null

> > > >

> > > > > -Example:

> > > > > -

> > > > > -    pcie1: pcie@7f000000 {

> > > > > -        compatible = "socionext,synquacer-pcie-ecam", "snps,dw-pcie-ecam";

> > > > > -        device_type = "pci";

> > > > > -        reg = <0x0 0x7f000000 0x0 0xf00000>;

> > > > > -        bus-range = <0x0 0xe>;

> > > > > -        #address-cells = <3>;

> > > > > -        #size-cells = <2>;

> > > > > -        ranges = <0x1000000 0x00 0x00010000 0x00 0x7ff00000 0x0 0x00010000>,

> > > > > -                 <0x2000000 0x00 0x70000000 0x00 0x70000000 0x0 0x0f000000>,

> > > > > -                 <0x3000000 0x3f 0x00000000 0x3f 0x00000000 0x1 0x00000000>;

> > > > > -

> > > > > -        #interrupt-cells = <0x1>;

> > > > > -        interrupt-map-mask = <0x0 0x0 0x0 0x0>;

> > > >

> > > > An all-zeroes interrupt-map-mask seems to be very common on embedded

> > > > SoCs, where all devices are mapped to a single interrupt.

> > >

> > > Indeed.

> > >

> > > > However, schemas/pci/pci-bus.yaml says:

> > > >

> > > >   interrupt-map-mask:

> > > >     items:

> > > >       - description: PCI high address cell

> > > >         minimum: 0

> > > >         maximum: 0xf800

> > > >       - description: PCI mid address cell

> > > >         const: 0

> > > >       - description: PCI low address cell

> > > >         const: 0

> > > >       - description: PCI IRQ cell

> > > >         minimum: 1

> > > >         maximum: 7

> > > >

> > > > and thus complains about an all-zeroes mask, e.g.

> > > >

> > > >     arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml:

> > > > pcie@fe000000: interrupt-map-mask:0:3: 0 is less than the minimum of 1

> > >

> > > Now fixed.

> >

> > Thank you, confirmed.

>

> And with latest renesas-drivers, I started seeing:

>

>     arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:

> interrupt-map:0: [0, 0, 0, 1] is too short

>     arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:

> interrupt-map:1: [5, 0, 113, 4] is too short

>     arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:

> interrupt-map:2: [2048, 0, 0, 1] is too short

>     arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:

> interrupt-map:3: [5, 0, 113, 4] is too short

>     arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:

> interrupt-map:4: [4096, 0, 0, 2] is too short

>     arch/arm/boot/dts/r8a7791-koelsch.dt.yaml: pci@ee0d0000:

> interrupt-map:5: [5, 0, 113, 4] is too short

>

> Looks like interrupt-map is split incorrectly: shouldn't each entry have 8

> cells?


That must be with a current dtc which now splits the array before each
phandle. That works for phandle+args, but not *-map properties. :( I
was trying to avoid a bunch of dts updates to add brackets. I think
for now, I'll just drop the interrupt-map size constraint. It's not
all that accurate anyways as it doesn't look at cell sizes.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/versatile.txt b/Documentation/devicetree/bindings/pci/versatile.txt
deleted file mode 100644
index 0a702b13d2ac..000000000000
--- a/Documentation/devicetree/bindings/pci/versatile.txt
+++ /dev/null
@@ -1,59 +0,0 @@ 
-* ARM Versatile Platform Baseboard PCI interface
-
-PCI host controller found on the ARM Versatile PB board's FPGA.
-
-Required properties:
-- compatible: should contain "arm,versatile-pci" to identify the Versatile PCI
-  controller.
-- reg: base addresses and lengths of the PCI controller. There must be 3
-  entries:
-	- Versatile-specific registers
-	- Self Config space
-	- Config space
-- #address-cells: set to <3>
-- #size-cells: set to <2>
-- device_type: set to "pci"
-- bus-range: set to <0 0xff>
-- ranges: ranges for the PCI memory and I/O regions
-- #interrupt-cells: set to <1>
-- interrupt-map-mask and interrupt-map: standard PCI properties to define
-	the mapping of the PCI interface to interrupt numbers.
-
-Example:
-
-pci-controller@10001000 {
-	compatible = "arm,versatile-pci";
-	device_type = "pci";
-	reg = <0x10001000 0x1000
-	       0x41000000 0x10000
-	       0x42000000 0x100000>;
-	bus-range = <0 0xff>;
-	#address-cells = <3>;
-	#size-cells = <2>;
-	#interrupt-cells = <1>;
-
-	ranges = <0x01000000 0 0x00000000 0x43000000 0 0x00010000   /* downstream I/O */
-		  0x02000000 0 0x50000000 0x50000000 0 0x10000000   /* non-prefetchable memory */
-		  0x42000000 0 0x60000000 0x60000000 0 0x10000000>; /* prefetchable memory */
-
-	interrupt-map-mask = <0x1800 0 0 7>;
-	interrupt-map = <0x1800 0 0 1 &sic 28
-			 0x1800 0 0 2 &sic 29
-			 0x1800 0 0 3 &sic 30
-			 0x1800 0 0 4 &sic 27
-
-			 0x1000 0 0 1 &sic 27
-			 0x1000 0 0 2 &sic 28
-			 0x1000 0 0 3 &sic 29
-			 0x1000 0 0 4 &sic 30
-
-			 0x0800 0 0 1 &sic 30
-			 0x0800 0 0 2 &sic 27
-			 0x0800 0 0 3 &sic 28
-			 0x0800 0 0 4 &sic 29
-
-			 0x0000 0 0 1 &sic 29
-			 0x0000 0 0 2 &sic 30
-			 0x0000 0 0 3 &sic 27
-			 0x0000 0 0 4 &sic 28>;
-};
diff --git a/Documentation/devicetree/bindings/pci/versatile.yaml b/Documentation/devicetree/bindings/pci/versatile.yaml
new file mode 100644
index 000000000000..07a48c27db1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/versatile.yaml
@@ -0,0 +1,92 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/versatile.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM Versatile Platform Baseboard PCI interface
+
+maintainers:
+  - Rob Herring <robh@kernel.org>
+
+description: |+
+  PCI host controller found on the ARM Versatile PB board's FPGA.
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+  compatible:
+    const: arm,versatile-pci
+
+  reg:
+    items:
+      - description: Versatile-specific registers
+      - description: Self Config space
+      - description: Config space
+
+  ranges:
+    maxItems: 3
+
+  "#interrupt-cells": true
+
+  interrupt-map:
+    maxItems: 16
+
+  interrupt-map-mask:
+    items:
+      - const: 0x1800
+      - const: 0
+      - const: 0
+      - const: 7
+
+required:
+  - compatible
+  - reg
+  - ranges
+  - "#interrupt-cells"
+  - interrupt-map
+  - interrupt-map-mask
+
+examples:
+  - |
+    pci@10001000 {
+      compatible = "arm,versatile-pci";
+      device_type = "pci";
+      reg = <0x10001000 0x1000>,
+            <0x41000000 0x10000>,
+            <0x42000000 0x100000>;
+      bus-range = <0 0xff>;
+      #address-cells = <3>;
+      #size-cells = <2>;
+      #interrupt-cells = <1>;
+
+      ranges =
+          <0x01000000 0 0x00000000 0x43000000 0 0x00010000>,  /* downstream I/O */
+          <0x02000000 0 0x50000000 0x50000000 0 0x10000000>,  /* non-prefetchable memory */
+          <0x42000000 0 0x60000000 0x60000000 0 0x10000000>;  /* prefetchable memory */
+
+      interrupt-map-mask = <0x1800 0 0 7>;
+      interrupt-map = <0x1800 0 0 1 &sic 28>,
+          <0x1800 0 0 2 &sic 29>,
+          <0x1800 0 0 3 &sic 30>,
+          <0x1800 0 0 4 &sic 27>,
+
+          <0x1000 0 0 1 &sic 27>,
+          <0x1000 0 0 2 &sic 28>,
+          <0x1000 0 0 3 &sic 29>,
+          <0x1000 0 0 4 &sic 30>,
+
+          <0x0800 0 0 1 &sic 30>,
+          <0x0800 0 0 2 &sic 27>,
+          <0x0800 0 0 3 &sic 28>,
+          <0x0800 0 0 4 &sic 29>,
+
+          <0x0000 0 0 1 &sic 29>,
+          <0x0000 0 0 2 &sic 30>,
+          <0x0000 0 0 3 &sic 27>,
+          <0x0000 0 0 4 &sic 28>;
+    };
+
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index c6c34d04ce95..48a90f0833b8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12401,7 +12401,7 @@  M:	Rob Herring <robh@kernel.org>
 L:	linux-pci@vger.kernel.org
 L:	linux-arm-kernel@lists.infradead.org
 S:	Maintained
-F:	Documentation/devicetree/bindings/pci/versatile.txt
+F:	Documentation/devicetree/bindings/pci/versatile.yaml
 F:	drivers/pci/controller/pci-versatile.c
 
 PCI DRIVER FOR ARMADA 8K