pci: Aardvark PCIe controller driver for Marvell Armada 3700

Hello,

This is the third iteration of the patch series that adds a PCIe controller driver for the Aardvark PCI controller, which is used on Marvell Armada 3700.

Changes since v2:

- Fix typo in the DT binding documentation (handled -> handle). Noticed by Bjorn Helgaas.

- Make all macro definitions use lower-case letters for hexadecimal numbers. Suggested by Bjorn Helgaas.

- Remove numerous unused macro definitions. Suggested by Bjorn Helgaas.

- Remove useless parenthesis in macro definitions. Suggested by Bjorn Helgaas.

- Read the root bus number from the Device Tree (through the IORESOURCE_BUS resource) instead of hardcoding bus 0. Suggested by Bjorn Helgaas.

- Rework the "link up" functions in two separate functions:
advk_pcie_link_up() and advk_pcie_wait_for_link(), and use usleep_range() with a number of retries. Suggested by Bjorn Helgaas.

- Fix some comments for consistency.

- Switch from the AXI disabling logic to a different strategy for accessing the configuration space. This avoids the need for synchronization, and therefore solves some stability problems.

- Minor improvements to the advk_pcie_wait_pio() function.

- Make the 'if (where % size)' test earlier in the advk_pcie_wr_conf() function. Suggested by Bjorn Helgaas.

- Use dev_err() instead of pr_err(). Suggested by Bjorn Helgaas.

- Split the irq domain initialization (for legacy interrupts and MSI interrupts into two functions). While at it, also fix a device_node reference count leak when initializing the legacy irq domain. Requested by Bjorn Helgaas.

- Remove the advk_pcie_release_of_pci_ranges() function, and call pci_free_resource_list() directly. Suggested by Bjorn Helgaas.

- Remove MODULE_DEVICE_TABLE(), as the driver is not compiled as a module. Suggested by Bjorn Helgaas.

- Change the "select PCI_MSI" to "depends on PCI_MSI_IRQ_DOMAIN". Suggested by Bjorn Helgaas.

Changes since v1:

- Even if the MSI handling is currently done directly by the PCIe controller, use a msi-parent property to indicate this relationship. This way, if in the future we want to use the MSI capabilities from the GIC interrupt controller rather than the MSI functionality of the PCI controller, it will be possible.

Suggested by Arnd Bergmann.

- Replace the number-of-retry based loops by timeout based loops, using time_before().

Suggested by Arnd Bergmann.

- Simplify the IRQ handling, which actually also fixes a number of issues with the interrupt handling.

Suggested by Marcin Wojtas.

Thanks,

Thomas

Thomas Petazzoni (3):
dt-bindings: add DT binding for the Aardvark PCIe controller
PCI: host: new PCI host controller driver for Aardvark
arm64: dts: marvell: Aardvark PCIe support for Armada 3700

 .../devicetree/bindings/pci/aardvark-pci.txt       |   56 ++
 MAINTAINERS                                        |    7 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts     |    5 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |   25 +
 drivers/pci/host/Kconfig                           |    9 +
 drivers/pci/host/Makefile                          |    1 +
 drivers/pci/host/pci-aardvark.c                    | 1014 ++++++++++++++++++++
 7 files changed, 1117 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/aardvark-pci.txt
 create mode 100644 drivers/pci/host/pci-aardvark.c

2.7.4

This commit adds the documentation for the Device Tree binding used to describe the Aardvark PCIe controller, found on Marvell Armada 3700 ARM64 SoCs.

---
 .../devicetree/bindings/pci/aardvark-pci.txt       | 56 ++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/aardvark-pci.txt

new file mode 100644
index 0000000..bbcd9f4
@@ -0,0 +1,56 @@
+Aardvark PCIe controller
+
+This PCIe controller is used on the Marvell Armada 3700 ARM64 SoC.
+
+The Device Tree node describing an Aardvark PCIe controller must
+contain the following properties:
+
+ - compatible: Should be "marvell,armada-3700-pcie"
+ - reg: range of registers for the PCIe controller
+ - interrupts: the interrupt line of the PCIe controller
+ - #address-cells: set to <3>
+ - #size-cells: set to <2>
+ - device_type: set to "pci"
+ - ranges: ranges for the PCI memory and I/O regions
+ - #interrupt-cells: set to <1>
+ - msi-controller: indicates that the PCIe controller can itself
+   handle MSI interrupts
+ - msi-parent: pointer to the MSI controller to be used
+ - interrupt-map-mask and interrupt-map: standard PCI properties to
+   define the mapping of the PCIe interface to interrupt numbers.
+ - bus-range: PCI bus numbers covered
+
+In addition, the Device Tree describing an Aardvark PCIe controller
+must include a sub-node that describes the legacy interrupt controller
+built into the PCIe controller. This sub-node must have the following
+properties:
+
+ - interrupt-controller
+ - #interrupt-cells: set to <1>
+
+Example:
+
+	pcie0: pcie at d0070000 {
+		compatible = "marvell,armada-3700-pcie";
+		device_type = "pci";
+		status = "disabled";
+		reg = <0 0xd0070000 0 0x20000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		bus-range = <0x00 0xff>;
+		interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+		#interrupt-cells = <1>;
+		msi-controller;
+		msi-parent = <&pcie0>;
+		ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
+			  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
+		interrupt-map-mask = <0 0 0 7>;
+		interrupt-map = <0 0 0 1 &pcie_intc 0>,
+				<0 0 0 2 &pcie_intc 1>,
+				<0 0 0 3 &pcie_intc 2>,
+				<0 0 0 4 &pcie_intc 3>;
+		pcie_intc: interrupt-controller {
+			interrupt-controller;
+			#interrupt-cells = <1>;
+		};
+	};

2.7.4

This commit adds a new driver for a PCIe controller called Aardvark, which is used on the Marvell Armada 3700 ARM64 SoC.

This patch is based on work done by Hezi Shahmoon <[email protected]> and Marcin Wojtas <[email protected]>.

---
 MAINTAINERS                     |    7 +
 drivers/pci/host/Kconfig        |    9 +
 drivers/pci/host/Makefile       |    1 +
 drivers/pci/host/pci-aardvark.c | 1014 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 1031 insertions(+)
 create mode 100644 drivers/pci/host/pci-aardvark.c

index 7304d2e..373215c 100644
@@ -8740,6 +8740,13 @@ L:	linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 F:	drivers/pci/host/*mvebu*
 
+PCI DRIVER FOR AARDVARK (Marvell Armada 3700)
+M:	Thomas Petazzoni <[email protected]>
+L:	linux-pci at vger.kernel.org
+L:	linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	drivers/pci/host/pci-aardvark.c
+
 PCI DRIVER FOR NVIDIA TEGRA
 M:	Thierry Reding <[email protected]>
 L:	linux-tegra at vger.kernel.org

index 5d2374e..c9806c4 100644
@@ -16,6 +16,15 @@ config PCI_MVEBU
 	depends on ARM
 	depends on OF
 
+config PCI_AARDVARK
+	bool "Aardvark PCIe controller"
+	depends on ARCH_MVEBU && ARM64
+	depends on OF
+	depends on PCI_MSI_IRQ_DOMAIN
+	help
+	 Add support for Aardvark 64bit PCIe Host Controller. This
+	 controller is part of the South Bridge of the Marvel Armada
+	 3700 SoC.
 
 config PCIE_XILINX_NWL
 	bool "NWL PCIe Core"

index 9c8698e..66f45b6 100644
@@ -5,6 +5,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
 obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCI_HYPERV) += pci-hyperv.o
 obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
+obj-$(CONFIG_PCI_AARDVARK) += pci-aardvark.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCIE_RCAR) += pcie-rcar.o

2.7.4

This commit adds the SoC-level description of the PCIe controller found on the Marvell Armada 3700, and also enables this PCIe controller on the development board for this SoC.

---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts |  5 +++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   | 25 +++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

index 86110a6..1372e9a6 100644
@@ -76,3 +76,8 @@
 &usb3 {
 	status = "okay";
 };
+
+/* CON17 (PCIe) / CON12 (mini-PCIe) */
+&pcie0 {
+	status = "okay";
+};

index 9e2efb8..8a9cae9 100644
@@ -141,5 +141,30 @@
 				      <0x1d40000 0x40000>; /* GICR */
 			};
 		};
+
+		pcie0: pcie at d0070000 {
+			compatible = "marvell,armada-3700-pcie";
+			device_type = "pci";
+			status = "disabled";
+			reg = <0 0xd0070000 0 0x20000>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			bus-range = <0x00 0xff>;
+			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+			#interrupt-cells = <1>;
+			msi-parent = <&pcie0>;
+			msi-controller;
+			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
+				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
+			interrupt-map-mask = <0 0 0 7>;
+			interrupt-map = <0 0 0 1 &pcie_intc 0>,
+					<0 0 0 2 &pcie_intc 1>,
+					<0 0 0 3 &pcie_intc 2>,
+					<0 0 0 4 &pcie_intc 3>;
+			pcie_intc: interrupt-controller {
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
 	};
 };

2.7.4

Hello Bjorn,

Have you had the chance to look into this third iteration, which hopefully integrates the feedback you made during your review of the second version?

I was hoping to get this in time for 4.8, so if you have any comments, do not hesitate to let me know so that I can address them.

Thanks!

Thomas

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Hi Thomas,

I tested MSI's, Legacy IRQ's and stability problems with configuration space access - everything works fine, so

Best regards,
Marcin

2016-07-07 22:44 GMT+02:00 Thomas Petazzoni
<[email protected]>:

Hi Thomas, I just returned from vacation, so I haven't looked at this yet. Sounds like we'll have a bonus week before v4.7 and I was planning to do two pull requests anyway because of my vacation, so there should still be time.

Bjorn

Hello,

Great, thanks! If there are any things remaining to fix, I should be able to do so early next week.

Thanks,

Thomas

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com