PCI: Layerscape: Add Layerscape PCIe driver
PATCH v4 1/1 PCI: Layerscape: Add Layerscape PCIe driver
Add support for Freescale Layerscape PCIe controller. This driver re-uses the designware core code.
---
Change log:
v4:
1. Add 'depends on OF' to Kconfig
v3:
1. Add prefechable mem region and adjust mem region
2. Rename 'pcie-scfg' to 'fsl,pcie-scfg'
3. Change MSI interrupt handler
4. Use module_platform_driver_probe instead of subsys_initcall
v2:
1. Add pcie-scfg to link scfg device node and remove "fsl, ls-pcie" compatible
2. Use regmap API to access scfg.
3. remove ls1021 PCI device ID.
4. remove unused irq pme_irq and ls_pcie_list.
5. Do not set scfg bit reverse mode
.../devicetree/bindings/pci/layerscape-pci.txt | 46 ++++ drivers/pci/host/Kconfig | 8 + drivers/pci/host/Makefile | 1 + drivers/pci/host/pci-layerscape.c | 274 +++++++++++++++++++++ 4 files changed, 329 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/layerscape-pci.txt create mode 100644 drivers/pci/host/pci-layerscape.c
Documentation/devicetree/bindings/pci/layerscape-pci.txt new file +46 -0
@@ -0,0 +1,46 @@ +Freescale Layerscape PCIe controller + +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: should contain the platform identifier such as "fsl,ls1021a-pcie" +- reg: base addresses and lengths of the PCIe controller +- interrupts: A list of interrupt outputs of the controller. Must contain an + entry for each entry in the interrupt-names property. +- interrupt-names: Must include the following entries: + "intr": The interrupt that is asserted for controller interrupts + "msi": The interrupt that is asserted when an MSI is received + "pme": The interrupt that is asserted when PME state changes +- fsl,pcie-scfg: Must include two entries. + The first entry must be a link to the SCFG device node + The second entry must be '0' or '1' based on physical PCIe controller index. + used to get SCFG PEXN registers + +Example: + + pcie@3400000 { + compatible = "fsl,ls1021a-pcie", "snps,dw-pcie"; + reg = <0x00 0x03400000 0x0 0x00010000 /* controller registers */ + 0x40 0x00000000 0x0 0x00002000>; /* configuration space */ + reg-names = "regs", "config"; + interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ + <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH>, /* MSI interrupt */ + <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>; /* PME interrupt */ + interrupt-names = "intr", "msi", "pme"; + fsl,pcie-scfg = <&scfg 0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + num-lanes = <4>; + bus-range = <0x0 0xff>; + ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000 /* downstream I/O */ + 0xc2000000 0x0 0x20000000 0x40 0x20000000 0x0 0x20000000 /* prefetchable memory */ + 0x82000000 0x0 0x40000000 0x40 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */ + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 7>; + interrupt-map = <0000 0 0 1 &gic GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>, + <0000 0 0 2 &gic GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>, + <0000 0 0 3 &gic GIC_SPI 190 IRQ_TYPE_LEVEL_HIGH>, + <0000 0 0 4 &gic GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>; + };
drivers/pci/host/Kconfig +8 -0
@@ -82,4 +82,12 @@ config PCIE_XILINX Say 'Y' here if you want kernel to support the Xilinx AXI PCIe Host Bridge driver. +config PCI_LAYERSCAPE + bool "Freescale Layerscape PCIe controller" + depends on OF + select PCIE_DW + select MFD_SYSCON + help + Say Y here if you want PCIe controller support on Layerscape SoCs. + endmenu
drivers/pci/host/Makefile +1 -0
@@ -10,3 +10,4 @@ obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
drivers/pci/host/pci-layerscape.c new file +274 -0
274 lines not on this page · raw message
Signature · 1 line
1.9.1
Bjorn Helgaas
maintainerPATCH v4 1/1 PCI: Layerscape: Add Layerscape PCIe driver
Quoted 3 lines · Minghuan Lian
Add support for Freescale Layerscape PCIe controller. This driver re-uses the designware core code.
...
This looks pretty good to me. I have a couple trivial comments below. Anybody else have any suggestions? I'd like to get this merged in the next day or two so we can get this into v3.18.
I'd like a MAINTAINERS update, too, so I know who to expect patches and acks from.
Quoted 7 lines
drivers/pci/host/pci-layerscape.c new file +7
-0+static irqreturn_t ls_pcie_msi_irq_handler (int irq,void *data) +{ +struct pcie_port *pp = data; +struct ls_pcie *pcie =to_ls_pcie (pp); +unsigned int msi_irq; + +/* clear the interrupt */
drivers/pci/host/pci-layerscape.c line 102
Quoted 10 lines
drivers/pci/host/pci-layerscape.c new file +10
-0+regmap_write (pcie->scfg,SCFG_SPIMSICLRCR , +MSI_LS1021A_DATA (pcie->index)); + + msi_irq =irq_find_mapping (pp->irq_domain,0 ); +if (!msi_irq) { +/* +* that's weird who triggered this? +* just clear it +*/ +dev_err (pcie->dev,"unexpected MSI\n" );
Scott suggested either using dev_dbg or rate-limiting this dev_err.
drivers/pci/host/pci-layerscape.c line 175
Quoted 7 lines
drivers/pci/host/pci-layerscape.c new file +7
-0+static int ls_add_pcie_port (struct ls_pcie *pcie) +{ +struct pcie_port *pp; +int ret; + +if (!pcie) +return -EINVAL ;
Unnecessary NULL pointer check (we already checked it below).
Bjorn
PATCH v4 1/1 PCI: Layerscape: Add Layerscape PCIe driver
In reply to Minghuan LianQuoted 4 lines · On Sunday 28 September 2014 15:08:27 Minghuan Lian wrote:
Add support for Freescale Layerscape PCIe controller. This driver re-uses the designware core code.
Signed-off-by Minghuan Lian <[email protected]>
I'm still not too happy about the MSI handling here. I think you really need to have a separate driver for the msi controller.
Quoted 8 lines
drivers/pci/host/pci-layerscape.c new file +6
-0+/* SCFG MSI register */ +#define SCFG_SPIMSICR 0x40 +#define SCFG_SPIMSICLRCR 0x90 + +#define MSI_LS1021A_ADDR 0x1570040 +#define MSI_LS1021A_DATA (pex_idx) (0xb3 + pex_idx)From what I can tell, the 'scfg' registers are mostly for MSI, and
the MSI_LS1021A_ADDR that you have hardcoded here is actually part of the scfg area itself. You should never hardwire that in a PCI host driver.
Please make a separate MSI driver instead and use the 'msi-parent' property.
drivers/pci/host/pci-layerscape.c line 84
Quoted 11 lines
drivers/pci/host/pci-layerscape.c new file +11
-0+static u32 ls_pcie_get_msi_addr (struct pcie_port *pp) +{ +return MSI_LS1021A_ADDR ; +} + +static u32 ls_pcie_get_msi_data (struct pcie_port *pp,int pos) +{ +struct ls_pcie *pcie =to_ls_pcie (pp); + +return MSI_LS1021A_DATA (pcie->index); +}
Does this mean you can have only one device on this PCI host that uses MSI?
Quoted 13 lines
drivers/pci/host/pci-layerscape.c new file +13
-0+static irqreturn_t ls_pcie_msi_irq_handler (int irq,void *data) +{ +struct pcie_port *pp = data; +struct ls_pcie *pcie =to_ls_pcie (pp); +unsigned int msi_irq; + +/* clear the interrupt */ +regmap_write (pcie->scfg,SCFG_SPIMSICLRCR , +MSI_LS1021A_DATA (pcie->index)); + + msi_irq =irq_find_mapping (pp->irq_domain,0 ); +if (!msi_irq) { +/*
drivers/pci/host/pci-layerscape.c line 108
Quoted 10 lines
drivers/pci/host/pci-layerscape.c new file +10
-0+* that's weird who triggered this? +* just clear it +*/ +dev_err (pcie->dev,"unexpected MSI\n" ); +return IRQ_NONE ; + } + +generic_handle_irq (msi_irq); +return IRQ_HANDLED ; +}
Since you have only one addr/data value per host controller, does that mean that you can have only one device on this host that uses one interrupt?
How do you ensure that the second user that tries to call pci_enable_msi gets an error? Is there only one internal device attached to the host?
drivers/pci/host/pci-layerscape.c line 158
Quoted 12 lines
drivers/pci/host/pci-layerscape.c new file +12
-0+if (of_device_is_compatible (pcie->dev->of_node,"fsl,ls1021a-pcie" )) { +/* +* LS1021A Workaround for internal TKT228622 +* to fix the INTx hang issue +*/ + val =ioread32 (pcie->dbi +PCIE_STRFMR1 ); + val &=0xffff ; +iowrite32 (val, pcie->dbi +PCIE_STRFMR1 ); + +ls1021a_pcie_msi_fixup (pp); + } +}
"fsl,ls1021a-pcie" is the only one supported by this driver, so the check is useless.
Arnd
PATCH v4 1/1 PCI: Layerscape: Add Layerscape PCIe driver
Hi Arnd,
Sorry for delay in my reply.
Please see my comments inline.
Quoted 7 lines · Arnd Bergmann
Quoted 4 lines · On Sunday 28 September 2014 15:08:27 Minghuan Lian wrote:
Add support for Freescale Layerscape PCIe controller. This driver re-uses the designware core code.
Signed-off-by Minghuan Lian <[email protected]>I'm still not too happy about the MSI handling here. I think you really need to have a separate driver for the msi controller.
[Minghuan] LS1021 does not contains MSI controller.
SCFG contains only one register to implement MSI interrupt support. And I reused pci-designware.c MSI API.
Quoted 11 lines
drivers/pci/host/pci-layerscape.c line 41
Quoted 6 lines
drivers/pci/host/pci-layerscape.c new file +6
-0+/* SCFG MSI register */ +#define SCFG_SPIMSICR 0x40 +#define SCFG_SPIMSICLRCR 0x90 + +#define MSI_LS1021A_ADDR 0x1570040 +#define MSI_LS1021A_DATA (pex_idx) (0xb3 + pex_idx)From what I can tell, the 'scfg' registers are mostly for MSI, and
theMSI_LS1021A_ADDRthat you have hardcoded here is actually part of the scfg area itself. You should never hardwire that in a PCI host driver.
Quoted 2 lines
Please make a separate MSI driver instead and use the 'msi-parent' property.
[Minghuan] There is only one register in SCFG used for MSI. I really do not want to write a separate MSI driver only for one register and two MSI interrupts.
It is really not good to use hardcode MSI_LS1021A_ADDR. I hope to change it when adding LS2085 PCI driver support. SCFG contains some registers used for PCI link status, PCI error interrupt and PCI PME support.
This is the first patch and will continue to improve.
Quoted 5 lines
Quoted 5 lines
drivers/pci/host/pci-layerscape.c new file +5
-0+static u32 ls_pcie_get_msi_addr (struct pcie_port *pp) +{ +return MSI_LS1021A_ADDR ; +} +
Quoted 8 lines
drivers/pci/host/pci-layerscape.c line 84
Quoted 6 lines
drivers/pci/host/pci-layerscape.c new file +6
-0+static u32 ls_pcie_get_msi_data (struct pcie_port *pp,int pos) +{ +struct ls_pcie *pcie =to_ls_pcie (pp); + +return MSI_LS1021A_DATA (pcie->index); +}Does this mean you can have only one device on this PCI host that uses MSI?
[Minghuan] Yes. LS1021A provides only one MSI interrupt for a PCI controller NOT 32.
Quoted 26 lines
Quoted 13 lines
drivers/pci/host/pci-layerscape.c new file +13
-0+static irqreturn_t ls_pcie_msi_irq_handler (int irq,void *data) +{ +struct pcie_port *pp = data; +struct ls_pcie *pcie =to_ls_pcie (pp); +unsigned int msi_irq; + +/* clear the interrupt */ +regmap_write (pcie->scfg,SCFG_SPIMSICLRCR , +MSI_LS1021A_DATA (pcie->index)); + + msi_irq =irq_find_mapping (pp->irq_domain,0 ); +if (!msi_irq) { +/* drivers/pci/host/pci-layerscape.c line 108
Quoted 10 lines
drivers/pci/host/pci-layerscape.c new file +10
-0+* that's weird who triggered this? +* just clear it +*/ +dev_err (pcie->dev,"unexpected MSI\n" ); +return IRQ_NONE ; + } + +generic_handle_irq (msi_irq); +return IRQ_HANDLED ; +}Since you have only one addr/data value per host controller, does that mean that you can have only one device on this host that uses one interrupt?
Quoted 2 lines
How do you ensure that the second user that tries to call
pci_enable_msigets an error? Is there only one internal device attached to the host?
[Minghuan] I used the following code to reserve 31 MSI interrupt. static void ls1021a_pcie_msi_fixup(struct pcie_port *pp)
{
int i;
/*
* LS1021A has only one MSI interrupt
* Set all msi interrupts as used except the first one
*/
for (i = 1; i < MAX_MSI_IRQS; i++)
set_bit(i, pp->msi_irq_in_use);
}Quoted 6 lines
Quoted 6 lines
drivers/pci/host/pci-layerscape.c new file +6
-0+if (of_device_is_compatible (pcie->dev->of_node,"fsl,ls1021a-pcie" )) { +/* +* LS1021A Workaround for internal TKT228622 +* to fix the INTx hang issue +*/ + val =ioread32 (pcie->dbi +PCIE_STRFMR1 );
Quoted 8 lines
drivers/pci/host/pci-layerscape.c line 158
Quoted 6 lines
drivers/pci/host/pci-layerscape.c new file +6
-0+ val &=0xffff ; +iowrite32 (val, pcie->dbi +PCIE_STRFMR1 ); + +ls1021a_pcie_msi_fixup (pp); + } +}"fsl,ls1021a-pcie" is the only one supported by this driver, so the check is useless.
[Minghuan] Ok. I will remove the 'if" and will re-add when adding LS2085 PCI driver.
Quoted 1 line
Arnd