diff mbox series

[3/4] PCI: ixp4xx: Add device tree bindings for IXP4xx

Message ID 20210503211649.4109334-4-linus.walleij@linaro.org (mailing list archive)
State Superseded
Headers show
Series IXP4xx PCI rework | expand

Commit Message

Linus Walleij May 3, 2021, 9:16 p.m. UTC
This adds device tree bindings for the Intel IXP4xx
PCI controller which can be used as both host and
option.

Cc: devicetree@vger.kernel.org
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Imre Kaloz <kaloz@openwrt.org>
Cc: Krzysztof Halasa <khalasa@piap.pl>
Cc: Zoltan HERPAI <wigyori@uid0.hu>
Cc: Raylynn Knight <rayknight@me.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
PCI maintainers: mainly looking for a review and ACK (if
you care about DT bindings) the patch will be merged
through ARM SoC.
---
 .../bindings/pci/intel,ixp4xx-pci.yaml        | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml

Comments

Arnd Bergmann May 4, 2021, 12:54 p.m. UTC | #1
On Mon, May 3, 2021 at 11:16 PM Linus Walleij <linus.walleij@linaro.org> wrote:

> +  compatible:
> +    items:
> +      - enum:
> +          - intel,ixp42x-pci
> +          - intel,ixp43x-pci
> +    description: The two supported variants are ixp42x and ixp43x,
> +      though more variants may exist.

These are still wildcard names, better pick a real soc identifier
such as "ixp425" instead of "ixp42x" in case there are differences
after all.

> +        <0x0800 0 0 1 &gpio0 11 3>, /* INT A on slot 1 is irq 11 */
> +        <0x0800 0 0 2 &gpio0 10 3>, /* INT B on slot 1 is irq 10 */
> +        <0x0800 0 0 3 &gpio0 9  3>, /* INT C on slot 1 is irq 9 */
> +        <0x0800 0 0 4 &gpio0 8  3>, /* INT D on slot 1 is irq 8 */
> +        <0x1000 0 0 1 &gpio0 10 3>, /* INT A on slot 2 is irq 10 */
> +        <0x1000 0 0 2 &gpio0 9  3>, /* INT B on slot 2 is irq 9 */
> +        <0x1000 0 0 3 &gpio0 8  3>, /* INT C on slot 2 is irq 8 */
> +        <0x1000 0 0 4 &gpio0 11 3>, /* INT D on slot 2 is irq 11 */
> +        <0x1800 0 0 1 &gpio0 9  3>, /* INT A on slot 3 is irq 9 */
> +        <0x1800 0 0 2 &gpio0 8  3>, /* INT B on slot 3 is irq 8 */
> +        <0x1800 0 0 3 &gpio0 11 3>, /* INT C on slot 3 is irq 11 */
> +        <0x1800 0 0 4 &gpio0 10 3>; /* INT D on slot 3 is irq 10 */

Is this different from the default swizzling rules? You normally
only have to provide the irqs for the bus once.

       Arnd
Rob Herring May 6, 2021, 8:24 p.m. UTC | #2
On Mon, May 03, 2021 at 11:16:48PM +0200, Linus Walleij wrote:
> This adds device tree bindings for the Intel IXP4xx
> PCI controller which can be used as both host and
> option.
> 
> Cc: devicetree@vger.kernel.org
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Imre Kaloz <kaloz@openwrt.org>
> Cc: Krzysztof Halasa <khalasa@piap.pl>
> Cc: Zoltan HERPAI <wigyori@uid0.hu>
> Cc: Raylynn Knight <rayknight@me.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> PCI maintainers: mainly looking for a review and ACK (if
> you care about DT bindings) the patch will be merged
> through ARM SoC.
> ---
>  .../bindings/pci/intel,ixp4xx-pci.yaml        | 96 +++++++++++++++++++
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml b/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml
> new file mode 100644
> index 000000000000..5b6af2f5c2a5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/intel,ixp4xx-pci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel IXP4xx PCI controller
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +description: PCI host controller found in the Intel IXP4xx SoC series.
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - intel,ixp42x-pci
> +          - intel,ixp43x-pci
> +    description: The two supported variants are ixp42x and ixp43x,
> +      though more variants may exist.
> +
> +  reg:
> +    items:
> +      - description: IXP4xx-specific registers
> +
> +  ranges:
> +    maxItems: 2
> +    description: Typically one memory range of 64MB and one IO
> +      space range of 64KB.
> +
> +  dma-ranges:
> +    maxItems: 1
> +    description: The DMA range tells the PCI host which addresses
> +      the RAM is at. It can map only 64MB so if the RAM is bigger
> +      than 64MB the DMA access has to be restricted to these
> +      addresses.
> +
> +  "#interrupt-cells": true
> +
> +  interrupt-map: true
> +
> +  interrupt-map-mask:
> +    items:
> +      - const: 0xf800
> +      - const: 0
> +      - const: 0
> +      - const: 7
> +
> +required:
> +  - compatible
> +  - reg
> +  - ranges

Already required by pci-bus.yaml I think.

> +  - dma-ranges
> +  - "#interrupt-cells"
> +  - interrupt-map
> +  - interrupt-map-mask
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    pci@c0000000 {
> +      compatible = "intel,ixp43x-pci";
> +      reg = <0xc0000000 0x1000>;
> +      #address-cells = <3>;
> +      #size-cells = <2>;
> +      device_type = "pci";
> +      bus-range = <0x00 0xff>;
> +      status = "disabled";

Don't show status in examples. 

I've really got to come up with an examples only schema to check this.

Rob
Linus Walleij May 9, 2021, 9:31 p.m. UTC | #3
On Tue, May 4, 2021 at 2:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, May 3, 2021 at 11:16 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - intel,ixp42x-pci
> > +          - intel,ixp43x-pci
> > +    description: The two supported variants are ixp42x and ixp43x,
> > +      though more variants may exist.
>
> These are still wildcard names, better pick a real soc identifier
> such as "ixp425" instead of "ixp42x" in case there are differences
> after all.

In general I agree. But when not even the vendor think they should
be held apart that is another thing. Even the official Intel
documentation uses these names:
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/ixp42x-product-line-network-processors-datasheet.pdf
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/ixp43x-product-line-network-processors-datasheet.pdf

The differences seem to be very small and related to the NPE
and page 23 in the 42x documentation makes no difference
between them.

I guess I will change it if you insist, but none of the other
drivers have this fine-grained compatible strings.

> > +        <0x0800 0 0 1 &gpio0 11 3>, /* INT A on slot 1 is irq 11 */
> > +        <0x0800 0 0 2 &gpio0 10 3>, /* INT B on slot 1 is irq 10 */
> > +        <0x0800 0 0 3 &gpio0 9  3>, /* INT C on slot 1 is irq 9 */
> > +        <0x0800 0 0 4 &gpio0 8  3>, /* INT D on slot 1 is irq 8 */
> > +        <0x1000 0 0 1 &gpio0 10 3>, /* INT A on slot 2 is irq 10 */
> > +        <0x1000 0 0 2 &gpio0 9  3>, /* INT B on slot 2 is irq 9 */
> > +        <0x1000 0 0 3 &gpio0 8  3>, /* INT C on slot 2 is irq 8 */
> > +        <0x1000 0 0 4 &gpio0 11 3>, /* INT D on slot 2 is irq 11 */
> > +        <0x1800 0 0 1 &gpio0 9  3>, /* INT A on slot 3 is irq 9 */
> > +        <0x1800 0 0 2 &gpio0 8  3>, /* INT B on slot 3 is irq 8 */
> > +        <0x1800 0 0 3 &gpio0 11 3>, /* INT C on slot 3 is irq 11 */
> > +        <0x1800 0 0 4 &gpio0 10 3>; /* INT D on slot 3 is irq 10 */
>
> Is this different from the default swizzling rules? You normally
> only have to provide the irqs for the bus once.

The different board files for ixp4xx does the swizzling in different
ways.

The NSLU2 rotates only the top 3 IRQs and looks like this:

+                       /*
+                        * Taken from NSLU2 PCI boardfile, INT A, B, C
swizzled D constant
+                        * We have slots (IDSEL) 1, 2 and 3.
+                        */
+                       interrupt-map =
+                       /* IDSEL 1 */
+                       <0x0800 0 0 1 &gpio0 11 3>, /* INT A on slot 1
is irq 11 */
+                       <0x0800 0 0 2 &gpio0 10 3>, /* INT B on slot 1
is irq 10 */
+                       <0x0800 0 0 3 &gpio0 9  3>, /* INT C on slot 1
is irq 9 */
+                       <0x0800 0 0 4 &gpio0 8  3>, /* INT D on slot 1
is irq 8 */
+                       /* IDSEL 2 */
+                       <0x1000 0 0 1 &gpio0 10 3>, /* INT A on slot 2
is irq 10 */
+                       <0x1000 0 0 2 &gpio0 9  3>, /* INT B on slot 2
is irq 9 */
+                       <0x1000 0 0 3 &gpio0 11 3>, /* INT C on slot 2
is irq 11 */
+                       <0x1000 0 0 4 &gpio0 8  3>, /* INT D on slot 2
is irq 8 */
+                       /* IDSEL 3 */
+                       <0x1800 0 0 1 &gpio0 9  3>, /* INT A on slot 3
is irq 9 */
+                       <0x1800 0 0 2 &gpio0 11 3>, /* INT B on slot 3
is irq 11 */
+                       <0x1800 0 0 3 &gpio0 10 3>, /* INT C on slot 3
is irq 10 */
+                       <0x1800 0 0 4 &gpio0 8  3>; /* INT D on slot 3
is irq 8 */

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml b/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml
new file mode 100644
index 000000000000..5b6af2f5c2a5
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml
@@ -0,0 +1,96 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/intel,ixp4xx-pci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel IXP4xx PCI controller
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+description: PCI host controller found in the Intel IXP4xx SoC series.
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - intel,ixp42x-pci
+          - intel,ixp43x-pci
+    description: The two supported variants are ixp42x and ixp43x,
+      though more variants may exist.
+
+  reg:
+    items:
+      - description: IXP4xx-specific registers
+
+  ranges:
+    maxItems: 2
+    description: Typically one memory range of 64MB and one IO
+      space range of 64KB.
+
+  dma-ranges:
+    maxItems: 1
+    description: The DMA range tells the PCI host which addresses
+      the RAM is at. It can map only 64MB so if the RAM is bigger
+      than 64MB the DMA access has to be restricted to these
+      addresses.
+
+  "#interrupt-cells": true
+
+  interrupt-map: true
+
+  interrupt-map-mask:
+    items:
+      - const: 0xf800
+      - const: 0
+      - const: 0
+      - const: 7
+
+required:
+  - compatible
+  - reg
+  - ranges
+  - dma-ranges
+  - "#interrupt-cells"
+  - interrupt-map
+  - interrupt-map-mask
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    pci@c0000000 {
+      compatible = "intel,ixp43x-pci";
+      reg = <0xc0000000 0x1000>;
+      #address-cells = <3>;
+      #size-cells = <2>;
+      device_type = "pci";
+      bus-range = <0x00 0xff>;
+      status = "disabled";
+
+      ranges =
+        <0x02000000 0 0x48000000 0x48000000 0 0x04000000>,
+        <0x01000000 0 0x00000000 0x4c000000 0 0x00010000>;
+      dma-ranges =
+        <0x02000000 0 0x00000000 0x00000000 0 0x04000000>;
+
+      #interrupt-cells = <1>;
+      interrupt-map-mask = <0xf800 0 0 7>;
+      interrupt-map =
+        <0x0800 0 0 1 &gpio0 11 3>, /* INT A on slot 1 is irq 11 */
+        <0x0800 0 0 2 &gpio0 10 3>, /* INT B on slot 1 is irq 10 */
+        <0x0800 0 0 3 &gpio0 9  3>, /* INT C on slot 1 is irq 9 */
+        <0x0800 0 0 4 &gpio0 8  3>, /* INT D on slot 1 is irq 8 */
+        <0x1000 0 0 1 &gpio0 10 3>, /* INT A on slot 2 is irq 10 */
+        <0x1000 0 0 2 &gpio0 9  3>, /* INT B on slot 2 is irq 9 */
+        <0x1000 0 0 3 &gpio0 8  3>, /* INT C on slot 2 is irq 8 */
+        <0x1000 0 0 4 &gpio0 11 3>, /* INT D on slot 2 is irq 11 */
+        <0x1800 0 0 1 &gpio0 9  3>, /* INT A on slot 3 is irq 9 */
+        <0x1800 0 0 2 &gpio0 8  3>, /* INT B on slot 3 is irq 8 */
+        <0x1800 0 0 3 &gpio0 11 3>, /* INT C on slot 3 is irq 11 */
+        <0x1800 0 0 4 &gpio0 10 3>; /* INT D on slot 3 is irq 10 */
+    };