linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml
@ 2021-05-03 18:52 Corentin Labbe
  2021-05-03 18:52 ` [PATCH 2/2] ARM: dts: gemini: add device_type on pci Corentin Labbe
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Corentin Labbe @ 2021-05-03 18:52 UTC (permalink / raw)
  To: bhelgaas, linus.walleij, robh+dt, ulli.kroll
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-pci, Corentin Labbe

Converts pci/faraday,ftpci100.txt to yaml.
Some change are also made:
- example has wrong interrupts place

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 .../bindings/pci/faraday,ftpci100.txt         | 135 -----------
 .../bindings/pci/faraday,ftpci100.yaml        | 211 ++++++++++++++++++
 2 files changed, 211 insertions(+), 135 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pci/faraday,ftpci100.txt
 create mode 100644 Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml

diff --git a/Documentation/devicetree/bindings/pci/faraday,ftpci100.txt b/Documentation/devicetree/bindings/pci/faraday,ftpci100.txt
deleted file mode 100644
index 5f8cb4962f8d..000000000000
--- a/Documentation/devicetree/bindings/pci/faraday,ftpci100.txt
+++ /dev/null
@@ -1,135 +0,0 @@
-Faraday Technology FTPCI100 PCI Host Bridge
-
-This PCI bridge is found inside that Cortina Systems Gemini SoC platform and
-is a generic IP block from Faraday Technology. It exists in two variants:
-plain and dual PCI. The plain version embeds a cascading interrupt controller
-into the host bridge. The dual version routes the interrupts to the host
-chips interrupt controller.
-
-The host controller appear on the PCI bus with vendor ID 0x159b (Faraday
-Technology) and product ID 0x4321.
-
-Mandatory properties:
-
-- compatible: ranging from specific to generic, should be one of
-  "cortina,gemini-pci", "faraday,ftpci100"
-  "cortina,gemini-pci-dual", "faraday,ftpci100-dual"
-  "faraday,ftpci100"
-  "faraday,ftpci100-dual"
-- reg: memory base and size for the host bridge
-- #address-cells: set to <3>
-- #size-cells: set to <2>
-- #interrupt-cells: set to <1>
-- bus-range: set to <0x00 0xff>
-- device_type, set to "pci"
-- ranges: see pci.txt
-- interrupt-map-mask: see pci.txt
-- interrupt-map: see pci.txt
-- dma-ranges: three ranges for the inbound memory region. The ranges must
-  be aligned to a 1MB boundary, and may be 1MB, 2MB, 4MB, 8MB, 16MB, 32MB, 64MB,
-  128MB, 256MB, 512MB, 1GB or 2GB in size. The memory should be marked as
-  pre-fetchable.
-
-Optional properties:
-- clocks: when present, this should contain the peripheral clock (PCLK) and the
-  PCI clock (PCICLK). If these are not present, they are assumed to be
-  hard-wired enabled and always on. The PCI clock will be 33 or 66 MHz.
-- clock-names: when present, this should contain "PCLK" for the peripheral
-  clock and "PCICLK" for the PCI-side clock.
-
-Mandatory subnodes:
-- For "faraday,ftpci100" a node representing the interrupt-controller inside the
-  host bridge is mandatory. It has the following mandatory properties:
-  - interrupt: see interrupt-controller/interrupts.txt
-  - interrupt-controller: see interrupt-controller/interrupts.txt
-  - #address-cells: set to <0>
-  - #interrupt-cells: set to <1>
-
-I/O space considerations:
-
-The plain variant has 128MiB of non-prefetchable memory space, whereas the
-"dual" variant has 64MiB. Take this into account when describing the ranges.
-
-Interrupt map considerations:
-
-The "dual" variant will get INT A, B, C, D from the system interrupt controller
-and should point to respective interrupt in that controller in its
-interrupt-map.
-
-The code which is the only documentation of how the Faraday PCI (the non-dual
-variant) interrupts assigns the default interrupt mapping/swizzling has
-typically been like this, doing the swizzling on the interrupt controller side
-rather than in the interconnect:
-
-interrupt-map-mask = <0xf800 0 0 7>;
-interrupt-map =
-	<0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
-	<0x4800 0 0 2 &pci_intc 1>,
-	<0x4800 0 0 3 &pci_intc 2>,
-	<0x4800 0 0 4 &pci_intc 3>,
-	<0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */
-	<0x5000 0 0 2 &pci_intc 2>,
-	<0x5000 0 0 3 &pci_intc 3>,
-	<0x5000 0 0 4 &pci_intc 0>,
-	<0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */
-	<0x5800 0 0 2 &pci_intc 3>,
-	<0x5800 0 0 3 &pci_intc 0>,
-	<0x5800 0 0 4 &pci_intc 1>,
-	<0x6000 0 0 1 &pci_intc 3>, /* Slot 12 */
-	<0x6000 0 0 2 &pci_intc 0>,
-	<0x6000 0 0 3 &pci_intc 1>,
-	<0x6000 0 0 4 &pci_intc 2>;
-
-Example:
-
-pci@50000000 {
-	compatible = "cortina,gemini-pci", "faraday,ftpci100";
-	reg = <0x50000000 0x100>;
-	interrupts = <8 IRQ_TYPE_LEVEL_HIGH>, /* PCI A */
-			<26 IRQ_TYPE_LEVEL_HIGH>, /* PCI B */
-			<27 IRQ_TYPE_LEVEL_HIGH>, /* PCI C */
-			<28 IRQ_TYPE_LEVEL_HIGH>; /* PCI D */
-	#address-cells = <3>;
-	#size-cells = <2>;
-	#interrupt-cells = <1>;
-
-	bus-range = <0x00 0xff>;
-	ranges = /* 1MiB I/O space 0x50000000-0x500fffff */
-		 <0x01000000 0 0          0x50000000 0 0x00100000>,
-		 /* 128MiB non-prefetchable memory 0x58000000-0x5fffffff */
-		 <0x02000000 0 0x58000000 0x58000000 0 0x08000000>;
-
-	/* DMA ranges */
-	dma-ranges =
-	/* 128MiB at 0x00000000-0x07ffffff */
-	<0x02000000 0 0x00000000 0x00000000 0 0x08000000>,
-	/* 64MiB at 0x00000000-0x03ffffff */
-	<0x02000000 0 0x00000000 0x00000000 0 0x04000000>,
-	/* 64MiB at 0x00000000-0x03ffffff */
-	<0x02000000 0 0x00000000 0x00000000 0 0x04000000>;
-
-	interrupt-map-mask = <0xf800 0 0 7>;
-	interrupt-map =
-		<0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
-		<0x4800 0 0 2 &pci_intc 1>,
-		<0x4800 0 0 3 &pci_intc 2>,
-		<0x4800 0 0 4 &pci_intc 3>,
-		<0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */
-		<0x5000 0 0 2 &pci_intc 2>,
-		<0x5000 0 0 3 &pci_intc 3>,
-		<0x5000 0 0 4 &pci_intc 0>,
-		<0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */
-		<0x5800 0 0 2 &pci_intc 3>,
-		<0x5800 0 0 3 &pci_intc 0>,
-		<0x5800 0 0 4 &pci_intc 1>,
-		<0x6000 0 0 1 &pci_intc 3>, /* Slot 12 */
-		<0x6000 0 0 2 &pci_intc 0>,
-		<0x6000 0 0 3 &pci_intc 0>,
-		<0x6000 0 0 4 &pci_intc 0>;
-	pci_intc: interrupt-controller {
-		interrupt-parent = <&intcon>;
-		interrupt-controller;
-		#address-cells = <0>;
-		#interrupt-cells = <1>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml b/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml
new file mode 100644
index 000000000000..9be27e71526c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml
@@ -0,0 +1,211 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/faraday,ftpci100.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Faraday Technology FTPCI100 PCI Host Bridge
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+description: |
+    This PCI bridge is found inside that Cortina Systems Gemini SoC platform and
+    is a generic IP block from Faraday Technology. It exists in two variants:
+    plain and dual PCI. The plain version embeds a cascading interrupt controller
+    into the host bridge. The dual version routes the interrupts to the host
+    chips interrupt controller.
+    The host controller appear on the PCI bus with vendor ID 0x159b (Faraday
+    Technology) and product ID 0x4321.
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - const: "cortina,gemini-pci"
+          - const: "faraday,ftpci100"
+      - items:
+          - const: "cortina,gemini-pci-dual"
+          - const: "faraday,ftpci100-dual"
+      - const: "faraday,ftpci100"
+      - const: "faraday,ftpci100-dual"
+
+  reg:
+    minItems: 1
+
+  "#address-cells":
+    const: 3
+
+  "#size-cells":
+    const: 2
+
+  "#interrupt-cells":
+    const: 1
+
+  bus-range:
+    items:
+      - const: 0x00
+      - const: 0xff
+
+  ranges:
+    minItems: 2
+    description: see pci.txt
+
+  dma-ranges:
+    minItems: 3
+    description: |
+      three ranges for the inbound memory region. The ranges must
+      be aligned to a 1MB boundary, and may be 1MB, 2MB, 4MB, 8MB, 16MB, 32MB, 64MB,
+      128MB, 256MB, 512MB, 1GB or 2GB in size. The memory should be marked as
+      pre-fetchable.
+
+  clocks:
+    minItems: 2
+    description: |
+      when present, this should contain the peripheral clock (PCLK) and the
+      PCI clock (PCICLK). If these are not present, they are assumed to be
+      hard-wired enabled and always on. The PCI clock will be 33 or 66 MHz.
+
+  clock-names:
+    items:
+      - const: "PCLK"
+      - const: "PCICLK"
+
+  interrupt-controller:
+    allOf:
+      - $ref: /schemas/interrupt-controller.yaml#
+    type: object
+    properties:
+      interrupts:
+        minItems: 1
+
+      interrupt-controller: true
+
+      "#address-cells":
+        const: 0
+
+      "#interrupt-cells":
+        const: 1
+
+    required:
+      - interrupts
+      - interrupt-controller
+      - "#address-cells"
+      - "#interrupt-cells"
+
+required:
+  - reg
+  - compatible
+  - "#address-cells"
+  - "#size-cells"
+  - "#interrupt-cells"
+  - bus-range
+  - ranges
+  - interrupt-map-mask
+  - interrupt-map
+  - dma-ranges
+
+if:
+  properties:
+    compatible:
+      contains:
+        items:
+          - const: cortina,gemini-pci
+          - const: faraday,ftpci100
+then:
+  required:
+    - interrupt-controller
+
+unevaluatedProperties: false
+
+#I/O space considerations:
+
+#The plain variant has 128MiB of non-prefetchable memory space, whereas the
+#"dual" variant has 64MiB. Take this into account when describing the ranges.
+
+#Interrupt map considerations:
+
+#The "dual" variant will get INT A, B, C, D from the system interrupt controller
+#and should point to respective interrupt in that controller in its
+#interrupt-map.
+
+#The code which is the only documentation of how the Faraday PCI (the non-dual
+#variant) interrupts assigns the default interrupt mapping/swizzling has
+#typically been like this, doing the swizzling on the interrupt controller side
+#rather than in the interconnect:
+
+#interrupt-map-mask = <0xf800 0 0 7>;
+#interrupt-map =
+#	<0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
+#	<0x4800 0 0 2 &pci_intc 1>,
+#	<0x4800 0 0 3 &pci_intc 2>,
+#	<0x4800 0 0 4 &pci_intc 3>,
+#	<0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */
+#	<0x5000 0 0 2 &pci_intc 2>,
+#	<0x5000 0 0 3 &pci_intc 3>,
+#	<0x5000 0 0 4 &pci_intc 0>,
+#	<0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */
+#	<0x5800 0 0 2 &pci_intc 3>,
+#	<0x5800 0 0 3 &pci_intc 0>,
+#	<0x5800 0 0 4 &pci_intc 1>,
+#	<0x6000 0 0 1 &pci_intc 3>, /* Slot 12 */
+#	<0x6000 0 0 2 &pci_intc 0>,
+#	<0x6000 0 0 3 &pci_intc 1>,
+#	<0x6000 0 0 4 &pci_intc 2>;
+
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    pci@50000000 {
+      compatible = "cortina,gemini-pci", "faraday,ftpci100";
+      reg = <0x50000000 0x100>;
+      device_type = "pci";
+      #address-cells = <3>;
+      #size-cells = <2>;
+      #interrupt-cells = <1>;
+
+      bus-range = <0x00 0xff>;
+      ranges = /* 1MiB I/O space 0x50000000-0x500fffff */
+        <0x01000000 0 0          0x50000000 0 0x00100000>,
+        /* 128MiB non-prefetchable memory 0x58000000-0x5fffffff */
+        <0x02000000 0 0x58000000 0x58000000 0 0x08000000>;
+
+      /* DMA ranges */
+      dma-ranges =
+        /* 128MiB at 0x00000000-0x07ffffff */
+        <0x02000000 0 0x00000000 0x00000000 0 0x08000000>,
+        /* 64MiB at 0x00000000-0x03ffffff */
+        <0x02000000 0 0x00000000 0x00000000 0 0x04000000>,
+        /* 64MiB at 0x00000000-0x03ffffff */
+        <0x02000000 0 0x00000000 0x00000000 0 0x04000000>;
+
+      interrupt-map-mask = <0xf800 0 0 7>;
+      interrupt-map =
+        <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
+        <0x4800 0 0 2 &pci_intc 1>,
+        <0x4800 0 0 3 &pci_intc 2>,
+        <0x4800 0 0 4 &pci_intc 3>,
+        <0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */
+        <0x5000 0 0 2 &pci_intc 2>,
+        <0x5000 0 0 3 &pci_intc 3>,
+        <0x5000 0 0 4 &pci_intc 0>,
+        <0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */
+        <0x5800 0 0 2 &pci_intc 3>,
+        <0x5800 0 0 3 &pci_intc 0>,
+        <0x5800 0 0 4 &pci_intc 1>,
+        <0x6000 0 0 1 &pci_intc 3>, /* Slot 12 */
+        <0x6000 0 0 2 &pci_intc 0>,
+        <0x6000 0 0 3 &pci_intc 0>,
+        <0x6000 0 0 4 &pci_intc 0>;
+      pci_intc: interrupt-controller {
+        interrupt-parent = <&intcon>;
+        interrupt-controller;
+        interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <0>;
+        #interrupt-cells = <1>;
+      };
+    };
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] ARM: dts: gemini: add device_type on pci
  2021-05-03 18:52 [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml Corentin Labbe
@ 2021-05-03 18:52 ` Corentin Labbe
  2021-05-06 10:34   ` Linus Walleij
  2021-05-06 10:32 ` [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml Linus Walleij
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Corentin Labbe @ 2021-05-03 18:52 UTC (permalink / raw)
  To: bhelgaas, linus.walleij, robh+dt, ulli.kroll
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-pci, Corentin Labbe

Fixes DT warning on pci node by adding the missing device_type.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 arch/arm/boot/dts/gemini.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/gemini.dtsi b/arch/arm/boot/dts/gemini.dtsi
index 1833b6590d76..31db0be7ec67 100644
--- a/arch/arm/boot/dts/gemini.dtsi
+++ b/arch/arm/boot/dts/gemini.dtsi
@@ -287,6 +287,7 @@ pci@50000000 {
 			clock-names = "PCLK", "PCICLK";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pci_default_pins>;
+			device_type = "pci";
 			#address-cells = <3>;
 			#size-cells = <2>;
 			#interrupt-cells = <1>;
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml
  2021-05-03 18:52 [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml Corentin Labbe
  2021-05-03 18:52 ` [PATCH 2/2] ARM: dts: gemini: add device_type on pci Corentin Labbe
@ 2021-05-06 10:32 ` Linus Walleij
  2021-05-06 17:43 ` Rob Herring
  2021-05-06 20:34 ` Bjorn Helgaas
  3 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2021-05-06 10:32 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: Bjorn Helgaas, Rob Herring, Hans Ulli Kroll,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel, linux-pci

On Mon, May 3, 2021 at 8:52 PM Corentin Labbe <clabbe@baylibre.com> wrote:

> Converts pci/faraday,ftpci100.txt to yaml.
> Some change are also made:
> - example has wrong interrupts place
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>

Looks correct to me!

> +#I/O space considerations:
> +
> +#The plain variant has 128MiB of non-prefetchable memory space, whereas the
> +#"dual" variant has 64MiB. Take this into account when describing the ranges.

(...)

I would just move this in under the top level description. YAML
has this funky syntax for pre-formatted text that you can use,
but I don't know it off the top of my head, Rob?

With that fix:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ARM: dts: gemini: add device_type on pci
  2021-05-03 18:52 ` [PATCH 2/2] ARM: dts: gemini: add device_type on pci Corentin Labbe
@ 2021-05-06 10:34   ` Linus Walleij
  0 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2021-05-06 10:34 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: Bjorn Helgaas, Rob Herring, Hans Ulli Kroll,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel, linux-pci

On Mon, May 3, 2021 at 8:52 PM Corentin Labbe <clabbe@baylibre.com> wrote:

> Fixes DT warning on pci node by adding the missing device_type.
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>

Thanks, patch applied!

This is even required for port-mapped IO-space to work
I think, but nothing uses that so that's why I didn't notice.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml
  2021-05-03 18:52 [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml Corentin Labbe
  2021-05-03 18:52 ` [PATCH 2/2] ARM: dts: gemini: add device_type on pci Corentin Labbe
  2021-05-06 10:32 ` [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml Linus Walleij
@ 2021-05-06 17:43 ` Rob Herring
  2021-05-06 20:34 ` Bjorn Helgaas
  3 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2021-05-06 17:43 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: bhelgaas, linus.walleij, ulli.kroll, devicetree,
	linux-arm-kernel, linux-kernel, linux-pci

On Mon, May 03, 2021 at 06:52:27PM +0000, Corentin Labbe wrote:
> Converts pci/faraday,ftpci100.txt to yaml.
> Some change are also made:
> - example has wrong interrupts place
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  .../bindings/pci/faraday,ftpci100.txt         | 135 -----------
>  .../bindings/pci/faraday,ftpci100.yaml        | 211 ++++++++++++++++++
>  2 files changed, 211 insertions(+), 135 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pci/faraday,ftpci100.txt
>  create mode 100644 Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml


> diff --git a/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml b/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml
> new file mode 100644
> index 000000000000..9be27e71526c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml
> @@ -0,0 +1,211 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/faraday,ftpci100.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Faraday Technology FTPCI100 PCI Host Bridge
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +description: |
> +    This PCI bridge is found inside that Cortina Systems Gemini SoC platform and
> +    is a generic IP block from Faraday Technology. It exists in two variants:
> +    plain and dual PCI. The plain version embeds a cascading interrupt controller
> +    into the host bridge. The dual version routes the interrupts to the host
> +    chips interrupt controller.
> +    The host controller appear on the PCI bus with vendor ID 0x159b (Faraday
> +    Technology) and product ID 0x4321.
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: "cortina,gemini-pci"
> +          - const: "faraday,ftpci100"
> +      - items:
> +          - const: "cortina,gemini-pci-dual"
> +          - const: "faraday,ftpci100-dual"
> +      - const: "faraday,ftpci100"
> +      - const: "faraday,ftpci100-dual"

Don't need quotes.

> +
> +  reg:
> +    minItems: 1
> +

> +  "#address-cells":
> +    const: 3
> +
> +  "#size-cells":
> +    const: 2

Covered by pci-bus.yaml.

> +
> +  "#interrupt-cells":
> +    const: 1
> +
> +  bus-range:
> +    items:
> +      - const: 0x00
> +      - const: 0xff

That's the default range, so drop.

> +
> +  ranges:
> +    minItems: 2
> +    description: see pci.txt

Drop the description.

> +
> +  dma-ranges:
> +    minItems: 3
> +    description: |
> +      three ranges for the inbound memory region. The ranges must
> +      be aligned to a 1MB boundary, and may be 1MB, 2MB, 4MB, 8MB, 16MB, 32MB, 64MB,
> +      128MB, 256MB, 512MB, 1GB or 2GB in size. The memory should be marked as
> +      pre-fetchable.
> +
> +  clocks:
> +    minItems: 2
> +    description: |
> +      when present, this should contain the peripheral clock (PCLK) and the
> +      PCI clock (PCICLK). If these are not present, they are assumed to be
> +      hard-wired enabled and always on. The PCI clock will be 33 or 66 MHz.

Split the description:

items:
  - description: peripheral clock (PCLK)
  - description: PCI bus? clock (PCICLK). The PCI clock will be 33 or 66 MHz.


> +
> +  clock-names:
> +    items:
> +      - const: "PCLK"
> +      - const: "PCICLK"

Drop quotes.

> +
> +  interrupt-controller:
> +    allOf:
> +      - $ref: /schemas/interrupt-controller.yaml#

Drop, this will be applied based on the node name.

> +    type: object
> +    properties:
> +      interrupts:
> +        minItems: 1
> +
> +      interrupt-controller: true
> +
> +      "#address-cells":
> +        const: 0
> +
> +      "#interrupt-cells":
> +        const: 1
> +
> +    required:
> +      - interrupts
> +      - interrupt-controller
> +      - "#address-cells"
> +      - "#interrupt-cells"
> +
> +required:
> +  - reg
> +  - compatible
> +  - "#address-cells"
> +  - "#size-cells"
> +  - "#interrupt-cells"
> +  - bus-range
> +  - ranges
> +  - interrupt-map-mask
> +  - interrupt-map
> +  - dma-ranges

Drop all the ones required in pci-bus.yaml already.

> +
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        items:
> +          - const: cortina,gemini-pci
> +          - const: faraday,ftpci100
> +then:
> +  required:
> +    - interrupt-controller
> +
> +unevaluatedProperties: false
> +
> +#I/O space considerations:
> +
> +#The plain variant has 128MiB of non-prefetchable memory space, whereas the
> +#"dual" variant has 64MiB. Take this into account when describing the ranges.

Could go under 'ranges'?

> +
> +#Interrupt map considerations:

Could go under 'interrupt-map'.

> +
> +#The "dual" variant will get INT A, B, C, D from the system interrupt controller
> +#and should point to respective interrupt in that controller in its
> +#interrupt-map.
> +
> +#The code which is the only documentation of how the Faraday PCI (the non-dual
> +#variant) interrupts assigns the default interrupt mapping/swizzling has
> +#typically been like this, doing the swizzling on the interrupt controller side
> +#rather than in the interconnect:
> +
> +#interrupt-map-mask = <0xf800 0 0 7>;
> +#interrupt-map =
> +#	<0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
> +#	<0x4800 0 0 2 &pci_intc 1>,
> +#	<0x4800 0 0 3 &pci_intc 2>,
> +#	<0x4800 0 0 4 &pci_intc 3>,
> +#	<0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */
> +#	<0x5000 0 0 2 &pci_intc 2>,
> +#	<0x5000 0 0 3 &pci_intc 3>,
> +#	<0x5000 0 0 4 &pci_intc 0>,
> +#	<0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */
> +#	<0x5800 0 0 2 &pci_intc 3>,
> +#	<0x5800 0 0 3 &pci_intc 0>,
> +#	<0x5800 0 0 4 &pci_intc 1>,
> +#	<0x6000 0 0 1 &pci_intc 3>, /* Slot 12 */
> +#	<0x6000 0 0 2 &pci_intc 0>,
> +#	<0x6000 0 0 3 &pci_intc 1>,
> +#	<0x6000 0 0 4 &pci_intc 2>;
> +
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    pci@50000000 {
> +      compatible = "cortina,gemini-pci", "faraday,ftpci100";
> +      reg = <0x50000000 0x100>;
> +      device_type = "pci";
> +      #address-cells = <3>;
> +      #size-cells = <2>;
> +      #interrupt-cells = <1>;
> +
> +      bus-range = <0x00 0xff>;
> +      ranges = /* 1MiB I/O space 0x50000000-0x500fffff */
> +        <0x01000000 0 0          0x50000000 0 0x00100000>,
> +        /* 128MiB non-prefetchable memory 0x58000000-0x5fffffff */
> +        <0x02000000 0 0x58000000 0x58000000 0 0x08000000>;
> +
> +      /* DMA ranges */
> +      dma-ranges =
> +        /* 128MiB at 0x00000000-0x07ffffff */
> +        <0x02000000 0 0x00000000 0x00000000 0 0x08000000>,
> +        /* 64MiB at 0x00000000-0x03ffffff */
> +        <0x02000000 0 0x00000000 0x00000000 0 0x04000000>,
> +        /* 64MiB at 0x00000000-0x03ffffff */
> +        <0x02000000 0 0x00000000 0x00000000 0 0x04000000>;
> +
> +      interrupt-map-mask = <0xf800 0 0 7>;
> +      interrupt-map =
> +        <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
> +        <0x4800 0 0 2 &pci_intc 1>,
> +        <0x4800 0 0 3 &pci_intc 2>,
> +        <0x4800 0 0 4 &pci_intc 3>,
> +        <0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */
> +        <0x5000 0 0 2 &pci_intc 2>,
> +        <0x5000 0 0 3 &pci_intc 3>,
> +        <0x5000 0 0 4 &pci_intc 0>,
> +        <0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */
> +        <0x5800 0 0 2 &pci_intc 3>,
> +        <0x5800 0 0 3 &pci_intc 0>,
> +        <0x5800 0 0 4 &pci_intc 1>,
> +        <0x6000 0 0 1 &pci_intc 3>, /* Slot 12 */
> +        <0x6000 0 0 2 &pci_intc 0>,
> +        <0x6000 0 0 3 &pci_intc 0>,
> +        <0x6000 0 0 4 &pci_intc 0>;
> +      pci_intc: interrupt-controller {
> +        interrupt-parent = <&intcon>;
> +        interrupt-controller;
> +        interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
> +        #address-cells = <0>;
> +        #interrupt-cells = <1>;
> +      };
> +    };
> -- 
> 2.26.3
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml
  2021-05-03 18:52 [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml Corentin Labbe
                   ` (2 preceding siblings ...)
  2021-05-06 17:43 ` Rob Herring
@ 2021-05-06 20:34 ` Bjorn Helgaas
  2021-05-07 10:51   ` Linus Walleij
  3 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2021-05-06 20:34 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: bhelgaas, linus.walleij, robh+dt, ulli.kroll, devicetree,
	linux-arm-kernel, linux-kernel, linux-pci

On Mon, May 03, 2021 at 06:52:27PM +0000, Corentin Labbe wrote:
> Converts pci/faraday,ftpci100.txt to yaml.
> Some change are also made:
> - example has wrong interrupts place

I think it's nicer when content changes are in a separate patch from
format conversion patches.  Otherwise it's really hard to see the
content changes in the patch.

Maybe a preliminary patch could fix whatever is actually broken?

Rob suggested a bunch of things that could be dropped.  Maybe those
could be removed in a second preliminary patch before the conversion?
Or maybe the removals are only possible *because* of the conversion?
I'm not a yaml expert.

Bjorn

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml
  2021-05-06 20:34 ` Bjorn Helgaas
@ 2021-05-07 10:51   ` Linus Walleij
  2021-05-10 18:35     ` Bjorn Helgaas
  0 siblings, 1 reply; 8+ messages in thread
From: Linus Walleij @ 2021-05-07 10:51 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Corentin Labbe, Bjorn Helgaas, Rob Herring, Hans Ulli Kroll,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel, linux-pci

On Thu, May 6, 2021 at 10:34 PM Bjorn Helgaas <helgaas@kernel.org> wrote:

> I think it's nicer when content changes are in a separate patch from
> format conversion patches.  Otherwise it's really hard to see the
> content changes in the patch.
>
> Maybe a preliminary patch could fix whatever is actually broken?
>
> Rob suggested a bunch of things that could be dropped.  Maybe those
> could be removed in a second preliminary patch before the conversion?
> Or maybe the removals are only possible *because* of the conversion?
> I'm not a yaml expert.

A bit of taste is involved. The old .txt bindings are for processing
by human brain power. Those lack regular syntax and strictness
because brains are designed for evolved natural languages.

The YAML on the other hand is a chomsky type-3 strict regular
language and the .yaml file (and includes) defines this strict regular
grammar and as such admits less mistakes. The upside is that
it enforces some order.

In the process of moving to YAML we often discover a slew of
mistakes and the initiative often comes with the ambition to add
or modernize something.

In this case I wouldn't care with stepwise fixing because the
platform is modernized by a handful of people who all know
what is going on, so there is noone to confuse other than the
subsystem maintainer and the result will end up in the same
kernel release anyway.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml
  2021-05-07 10:51   ` Linus Walleij
@ 2021-05-10 18:35     ` Bjorn Helgaas
  0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2021-05-10 18:35 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Corentin Labbe, Bjorn Helgaas, Rob Herring, Hans Ulli Kroll,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, linux-kernel, linux-pci

On Fri, May 07, 2021 at 12:51:39PM +0200, Linus Walleij wrote:
> On Thu, May 6, 2021 at 10:34 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> > I think it's nicer when content changes are in a separate patch from
> > format conversion patches.  Otherwise it's really hard to see the
> > content changes in the patch.
> >
> > Maybe a preliminary patch could fix whatever is actually broken?
> >
> > Rob suggested a bunch of things that could be dropped.  Maybe those
> > could be removed in a second preliminary patch before the conversion?
> > Or maybe the removals are only possible *because* of the conversion?
> > I'm not a yaml expert.
> 
> A bit of taste is involved. The old .txt bindings are for processing
> by human brain power. Those lack regular syntax and strictness
> because brains are designed for evolved natural languages.
> 
> The YAML on the other hand is a chomsky type-3 strict regular
> language and the .yaml file (and includes) defines this strict regular
> grammar and as such admits less mistakes. The upside is that
> it enforces some order.
> 
> In the process of moving to YAML we often discover a slew of
> mistakes and the initiative often comes with the ambition to add
> or modernize something.
> 
> In this case I wouldn't care with stepwise fixing because the
> platform is modernized by a handful of people who all know
> what is going on, so there is noone to confuse other than the
> subsystem maintainer and the result will end up in the same
> kernel release anyway.

Haha, I'm in that large majority of people who lack deep knowledge
of what's going on, so it definitely confuses me :)

I think the stepwise fix would be helpful in making the patches more
accessible to us non-experts, and I know it would save me time in
reviewing.

It may also be useful to people converting other bindings to YAML
because it's more obvious what mistakes need to be fixed in the
process.

Also helpful: changing the subject line to match the existing
convention, e.g.,

  dt-bindings: PCI: ftpci100: Convert faraday,ftpci100 to YAML

Bjorn

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-05-10 18:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-03 18:52 [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml Corentin Labbe
2021-05-03 18:52 ` [PATCH 2/2] ARM: dts: gemini: add device_type on pci Corentin Labbe
2021-05-06 10:34   ` Linus Walleij
2021-05-06 10:32 ` [PATCH 1/2] dt-bindings: pci: convert faraday,ftpci100 to yaml Linus Walleij
2021-05-06 17:43 ` Rob Herring
2021-05-06 20:34 ` Bjorn Helgaas
2021-05-07 10:51   ` Linus Walleij
2021-05-10 18:35     ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).