On Mon, Jul 21, 2014 at 12:58:44PM -0400, Murali Karicheri wrote:
keystone PCIe controller is based on v3.65 version of the
designware h/w. Main differences are
1. No ATU support
2. Legacy and MSI irq functions are implemented in
application register space
3. MSI interrupts are multiplexed over 8 IRQ lines to the Host
side.
All of the Application register space handing code are organized into
pci-keystone-dw.c and the functions are called from pci-keystone.c
to implement PCI controller driver. Also add necessary DT documentation
for the driver.
Signed-off-by: Murali Karicheri<m-karicheri2@xxxxxx>
Acked-by: Santosh Shilimkar<santosh.shilimkar@xxxxxx>
...
+++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
...
+Note for PCI driver usage
+=========================
+Driver requires pci=pcie_bus_perf in the bootargs for proper functioning.
Whoa, why is this? Special boot args should not be required.
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 21df477..f8bc475 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -46,4 +46,9 @@ config PCI_HOST_GENERIC
Say Y here if you want to support a simple generic PCI host
controller, such as the one emulated by kvmtool.
+config PCI_KEYSTONE
+ bool "TI Keystone PCIe controller"
+ depends on ARCH_KEYSTONE
+ select PCIE_DW
+ select PCIEPORTBUS
It'd be nice to have some help text here. I know, not everybody else does.
+++ b/drivers/pci/host/pci-keystone-dw.c
...
+void ks_dw_pcie_handle_msi_irq(struct keystone_pcie *ks_pcie , int offset)
+{
+ struct pcie_port *pp =&ks_pcie->pp;
+ u32 pending, vector;
+ int src, virq;
+
+ pending = readl(ks_pcie->va_app_base + MSI0_IRQ_STATUS + (offset<< 4));
Blank line here (before the block comment).
+ /*
+ * MSI0, Status bit 0-3 shows vectors 0, 8, 16, 24, MSI1 status bit
+ * shows 1, 9, 17, 25 and so forth
+ */
+ for (src = 0; src< 4; src++) {
+ if (BIT(src)& pending) {
+ vector = offset + (src<< 3);
+ virq = irq_linear_revmap(pp->irq_domain, vector);
+ dev_dbg(pp->dev,
+ "irq: bit %d, vector %d, virq %d\n",
+ src, vector, virq);
+ generic_handle_irq(virq);
+ }
+ }
+}
+
...
+static void __iomem *ks_pcie_cfg_setup(struct keystone_pcie *ks_pcie, u8 bus,
+ unsigned int devfn)
+{
+ u8 device = PCI_SLOT(devfn), function = PCI_FUNC(devfn);
+ struct pcie_port *pp =&ks_pcie->pp;
+ u32 regval;
+
+ if (bus == 0)
+ return pp->dbi_base;
+
+ regval = (bus<< 16) | (device<< 8) | function;
+ /*
+ * Since Bus#1 will be a virtual bus, we need to have TYPE0
+ * access only.
+ * TYPE 1
+ */
+ if (bus != 1)
+ regval |= BIT(24);
+
+ writel(regval, ks_pcie->va_app_base + CFG_SETUP);
+ return pp->va_cfg0_base;
+}
+
+int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+ unsigned int devfn, int where, int size, u32 *val)
+{
+ struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
+ u8 bus_num = bus->number;
+ void __iomem *addr;
+ int ret;
+
+ addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
+ ret = dw_pcie_cfg_read(addr + (where& ~0x3), where, size, val);
This *looks* like it needs a lock to protect against concurrent
ks_pcie_cfg_setup() users, since it writes a register.
+
+ return ret;
Please use the same style as in ks_dw_pcie_wr_other_conf(), i.e., get rid
of "ret".
+}
+
+int ks_dw_pcie_wr_other_conf(struct pcie_port *pp,
+ struct pci_bus *bus, unsigned int devfn, int where,
+ int size, u32 val)
+{
+ struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
+ u8 bus_num = bus->number;
+ void __iomem *addr;
+
+ addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
+
+ return dw_pcie_cfg_write(addr + (where& ~0x3), where, size, val);
+}
+++ b/drivers/pci/host/pci-keystone.c
...
+static struct platform_driver ks_pcie_driver __refdata = {
Why does this need to be __refdata? There are no other occurrences in
drivers/pci.
+ .probe = ks_pcie_probe,
+ .remove = __exit_p(ks_pcie_remove),
+ .driver = {
+ .name = "keystone-pcie",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(ks_pcie_of_match),
+ },
+};
Bjorn