pci: host: new driver for Marvell Armada 7K/8K PCIe controller
cover pci: host: new driver for Marvell Armada 7K/8K PCIe controller
Hello,
This set of two patches add support for the PCIe controller found in the Marvell Armada 7K/8K ARM processors. The first patch adds the Device Tree binding documentation, the second patch the driver itself.
The driver is really simple, since the PCIe controller is based on the Designware design, and only a little bit of vendor-specific glue is needed.
Thanks!
Thomas
Thomas Petazzoni (2):
dt-bindings: pci: add DT binding for Marvell Armada 7K/8K PCIe
controller
pci: host: new driver for Marvell Armada 7K/8K PCIe controller
.../devicetree/bindings/pci/pci-armada8k.txt | 41 ++++ drivers/pci/host/Kconfig | 11 + drivers/pci/host/Makefile | 1 + drivers/pci/host/pcie-armada8k.c | 261 +++++++++++++++++++++ 4 files changed, 314 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/pci-armada8k.txt create mode 100644 drivers/pci/host/pcie-armada8k.c
Signature · 1 line
2.6.4
PATCH 1/2 dt-bindings: pci: add DT binding for Marvell Armada 7K/8K PCIe controller
This commit adds the Device Tree binding documentation that allows to describe the PCIe controller found in Marvell Armada 7K/8K SoCs.
--- .../devicetree/bindings/pci/pci-armada8k.txt | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/pci-armada8k.txt
Documentation/devicetree/bindings/pci/pci-armada8k.txt new file +41 -0
@@ -0,0 +1,41 @@ +* Marvell Armada 7K/8K PCIe interface + +This PCIe host controller is based on the Synopsis Designware PCIe IP +and thus inherits all the common properties defined in designware-pcie.txt. + +Required properties: +- compatible: "marvell,armada8k-pcie" +- reg: must contain two register regions + - the control register region + - the config space region +- reg-names: + - "ctrl" for the control register region + - "config" for the config space region +- interrupts: Interrupt specifier for the PCIe controler +- clock-names: May contain the following entries: + - "main", for the main clock, shared by several PCIe ports + - "port", for the port clock + +Example: + + pcie at f2600000 { + compatible = "marvell,armada8k-pcie", "snps,dw-pcie"; + reg = <0 0xf2600000 0 0x10000>, <0 0xf6f00000 0 0x80000>; + reg-names = "ctrl", "config"; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + device_type = "pci"; + dma-coherent; + + bus-range = <0 0xff>; + ranges = <0x81000000 0 0xf9000000 0 0xf9000000 0 0x10000 /* downstream I/O */ + 0x82000000 0 0xf6000000 0 0xf6000000 0 0xf00000>; /* non-prefetchable memory */ + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &gic 0 GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>; + num-lanes = <1>; + clocks = <&cpm_syscon0 1 14>, <&cpm_syscon0 1 13>; + clock-names = "main", "port"; + status = "disabled"; + };
Signature · 1 line
2.6.4
Rob Herring
maintainerPATCH 1/2 dt-bindings: pci: add DT binding for Marvell Armada 7K/8K PCIe controller
Quoted 8 lines · Thomas Petazzoni
This commit adds the Device Tree binding documentation that allows to describe the PCIe controller found in Marvell Armada 7K/8K SoCs.
Signed-off-by Thomas Petazzoni <[email protected]>--- .../devicetree/bindings/pci/pci-armada8k.txt | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/pci-armada8k.txt
PATCH 1/2 dt-bindings: pci: add DT binding for Marvell Armada 7K/8K PCIe controller
In reply to Thomas PetazzoniDocumentation/devicetree/bindings/pci/pci-armada8k.txt line 33
Quoted 3 lines · On Sunday 27 March 2016 11:30:57 Thomas Petazzoni wrote:
Documentation/devicetree/bindings/pci/pci-armada8k.txt new file +2
-0+ ranges = <0x81000000 0 0xf9000000 0 0xf9000000 0 0x10000 /* downstream I/O */ + 0x82000000 0 0xf6000000 0 0xf6000000 0 0xf00000>; /* non-prefetchable memory */
No 64-bit (prefetchable) MMIO area? Is this a hardware limitation, or did you just forget to add it?
Arnd
PATCH 1/2 dt-bindings: pci: add DT binding for Marvell Armada 7K/8K PCIe controller
Arnd,
Quoted 7 lines · Arnd Bergmann
Documentation/devicetree/bindings/pci/pci-armada8k.txt line 33
Quoted 3 lines · On Sunday 27 March 2016 11:30:57 Thomas Petazzoni wrote:
Documentation/devicetree/bindings/pci/pci-armada8k.txt new file +2
-0+ ranges = <0x81000000 0 0xf9000000 0 0xf9000000 0 0x10000 /* downstream I/O */ + 0x82000000 0 0xf6000000 0 0xf6000000 0 0xf00000>; /* non-prefetchable memory */No 64-bit (prefetchable) MMIO area? Is this a hardware limitation, or did you just forget to add it?
I don't have enough technical documentation at the moment to answer the question. Would it be possible to merge this as-is, and update it later on when we have enough information about 64-bit MMIO area support? It does not affect this new DT binding, since it's purely related to the standard PCI binding.
Would this be OK? I'd prefer to have PCI supported with just the 32 bits non-prefetchable memory rather than no PCI supported at all.
Thanks a lot!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 2/2 pci: host: new driver for Marvell Armada 7K/8K PCIe controller
In reply to Thomas PetazzoniThe Marvell Armada 7K/8K SoCs integrate a PCIe controller from Synopsys. This commit adds a new driver that provides the small glue needed to use the existing Designware driver to make it work on Marvell Armada 7K/8K SoCs.
The MSI support will be enabled at a later point.
--- drivers/pci/host/Kconfig | 11 ++ drivers/pci/host/Makefile | 1 + drivers/pci/host/pcie-armada8k.c | 261 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 273 insertions(+) create mode 100644 drivers/pci/host/pcie-armada8k.c
drivers/pci/host/Kconfig +11 -0
@@ -231,4 +231,15 @@ config PCI_HOST_THUNDER_ECAM help Say Y here if you want ECAM support for CN88XX-Pass-1.x Cavium Thunder SoCs. +config PCIE_ARMADA_8K + bool "Marvell Armada-8K PCIe controller" + depends on ARCH_MVEBU + select PCIE_DW + select PCIEPORTBUS + help + Say Y here if you want to enable PCIe controller support on + Armada-8K SoCs. The PCIe controller on Armada-8K is based on + Designware hardware and therefore the driver re-uses the + Designware core functions to implement the driver. + endmenu
drivers/pci/host/Makefile +1 -0
@@ -28,3 +28,4 @@ obj-$(CONFIG_PCI_HISI) += pcie-hisi.o obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o +obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
drivers/pci/host/pcie-armada8k.c new file +261 -0
261 lines not on this page · raw message
Signature · 1 line
2.6.4
PATCH 2/2 pci: host: new driver for Marvell Armada 7K/8K PCIe controller
drivers/pci/host/pcie-armada8k.c line 164
Quoted 11 lines
drivers/pci/host/pcie-armada8k.c new file +11
-0+static irqreturn_t armada8k_pcie_irq_handler (int irq,void *arg) +{ +struct pcie_port *pp = arg; +struct armada8k_pcie *pcie =to_armada8k_pcie (pp); +void __iomem *base = pcie->base; +u32 val; + + val =readl (base +PCIE_GLOBAL_INT_CAUSE1_REG ); +writel (val, base +PCIE_GLOBAL_INT_CAUSE1_REG ); + +return IRQ_HANDLED ;
Hi Thomas
Maybe a comment as to why you are just throwing them away.
Quoted 16 lines
drivers/pci/host/pcie-armada8k.c new file +16
-0+} + +static struct pcie_host_ops armada8k_pcie_host_ops = { + .link_up = armada8k_pcie_link_up, + .host_init = armada8k_pcie_host_init, +}; + +static int armada8k_pcie_probe (struct platform_device *pdev) +{ +struct armada8k_pcie *pcie; +struct pcie_port *pp; +struct device *dev = &pdev->dev; +struct resource *base; +int ret; + + pcie =devm_kzalloc (dev,sizeof (*pcie),GFP_KERNEL );
drivers/pci/host/pcie-armada8k.c line 190
Quoted 10 lines
drivers/pci/host/pcie-armada8k.c new file +10
-0+if (!pcie) +return -ENOMEM ; + + pcie->main_clk =devm_clk_get (dev,"main" ); +if (!IS_ERR (pcie->main_clk)) +clk_prepare_enable (pcie->main_clk); + + pcie->lane_clk =devm_clk_get (dev,"port" ); +if (!IS_ERR (pcie->lane_clk)) +clk_prepare_enable (pcie->lane_clk);
Any need to handle -EPRODE_DEFERED here?
Andrew
PATCH 2/2 pci: host: new driver for Marvell Armada 7K/8K PCIe controller
Hello,
Quoted 5 lines · Andrew Lunn
Quoted 5 lines
drivers/pci/host/pcie-armada8k.c new file +5
-0+static irqreturn_t armada8k_pcie_irq_handler (int irq,void *arg) +{ +struct pcie_port *pp = arg; +struct armada8k_pcie *pcie =to_armada8k_pcie (pp); +void __iomem *base = pcie->base;
Quoted 8 lines
drivers/pci/host/pcie-armada8k.c line 164
Quoted 6 lines
drivers/pci/host/pcie-armada8k.c new file +6
-0+u32 val; + + val =readl (base +PCIE_GLOBAL_INT_CAUSE1_REG ); +writel (val, base +PCIE_GLOBAL_INT_CAUSE1_REG ); + +return IRQ_HANDLED ;Maybe a comment as to why you are just throwing them away.
I'll have a look into this.
Quoted 13 lines
Quoted 13 lines
drivers/pci/host/pcie-armada8k.c new file +13
-0+static int armada8k_pcie_probe (struct platform_device *pdev) +{ +struct armada8k_pcie *pcie; +struct pcie_port *pp; +struct device *dev = &pdev->dev; +struct resource *base; +int ret; + + pcie =devm_kzalloc (dev,sizeof (*pcie),GFP_KERNEL ); +if (!pcie) +return -ENOMEM ; + + pcie->main_clk =devm_clk_get (dev,"main" );
Quoted 8 lines
drivers/pci/host/pcie-armada8k.c line 190
Quoted 6 lines
drivers/pci/host/pcie-armada8k.c new file +6
-0+if (!IS_ERR (pcie->main_clk)) +clk_prepare_enable (pcie->main_clk); + + pcie->lane_clk =devm_clk_get (dev,"port" ); +if (!IS_ERR (pcie->lane_clk)) +clk_prepare_enable (pcie->lane_clk);Any need to handle -EPRODE_DEFERED here?
Is this needed? The clocks are registered in of_clk_init(), i.e at time_init() time. This is way before the device drivers get probed, no?
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 2/2 pci: host: new driver for Marvell Armada 7K/8K PCIe controller
Hello,
Quoted 12 lines · Thomas Petazzoni
Quoted 11 lines
drivers/pci/host/pcie-armada8k.c line 164
Quoted 9 lines
drivers/pci/host/pcie-armada8k.c new file +9
-0+struct pcie_port *pp = arg; +struct armada8k_pcie *pcie =to_armada8k_pcie (pp); +void __iomem *base = pcie->base; +u32 val; + + val =readl (base +PCIE_GLOBAL_INT_CAUSE1_REG ); +writel (val, base +PCIE_GLOBAL_INT_CAUSE1_REG ); + +return IRQ_HANDLED ;Maybe a comment as to why you are just throwing them away.
Quoted 1 line
I'll have a look into this.
Enabling the INT A-D interrupts is needed for the PCI device drivers to receive interrupts from their respective PCI devices. But then, once you enable the INT A-D interrupts, they are also latched into the PCIe controller, so if you don't acknowledge them in the PCIe controller level, at the first interrupt coming from a PCI device, the system hangs. I've added a comment in the driver about this.
Quoted 2 lines
Is this needed? The clocks are registered in
of_clk_init(), i.e attime_init()time. This is way before the device drivers get probed, no?
The clock drivers are now regular platform drivers, so I'll add the EPROBE_DEFER handling logic.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 2/2 pci: host: new driver for Marvell Armada 7K/8K PCIe controller
In reply to Thomas Petazzonidrivers/pci/host/pcie-armada8k.c line 148
Quoted 8 lines · On Sunday 27 March 2016 11:30:58 Thomas Petazzoni wrote:
drivers/pci/host/pcie-armada8k.c new file +7
-0+/* Wait until the link becomes active again */ +while (timeout) { +if (armada8k_pcie_link_up (pp)) +break ; +udelay (1 ); + timeout--; + }
Why the busy-loop here?
Maybe just do a single msleep(1) here to wait for the link to come up instead?
Arnd
PATCH 2/2 pci: host: new driver for Marvell Armada 7K/8K PCIe controller
Hello,
Quoted 10 lines · Arnd Bergmann
drivers/pci/host/pcie-armada8k.c line 148
Quoted 8 lines · On Sunday 27 March 2016 11:30:58 Thomas Petazzoni wrote:
drivers/pci/host/pcie-armada8k.c new file +7
-0+/* Wait until the link becomes active again */ +while (timeout) { +if (armada8k_pcie_link_up (pp)) +break ; +udelay (1 ); + timeout--; + }
Quoted 4 lines
Why the busy-loop here?
Maybe just do a single msleep(1) here to wait for the link to come up instead?
I've changed the udelay(1) by msleep(1). Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com