Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752864AbbGPNFa (ORCPT ); Thu, 16 Jul 2015 09:05:30 -0400 Received: from foss.arm.com ([217.140.101.70]:43593 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751360AbbGPNF2 (ORCPT ); Thu, 16 Jul 2015 09:05:28 -0400 Date: Thu, 16 Jul 2015 14:06:04 +0100 From: Lorenzo Pieralisi To: David Daney Cc: "linux-arm-kernel@lists.infradead.org" , Catalin Marinas , Will Deacon , Bjorn Helgaas , "linux-pci@vger.kernel.org" , Thomas Gleixner , Jason Cooper , "linux-kernel@vger.kernel.org" , Robert Richter , David Daney Subject: Re: [PATCH 5/5] PCI: Add host drivers for Cavium ThunderX processors. Message-ID: <20150716130604.GB17097@red-moon> References: <1436979285-8177-1-git-send-email-ddaney.cavm@gmail.com> <1436979285-8177-6-git-send-email-ddaney.cavm@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1436979285-8177-6-git-send-email-ddaney.cavm@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 32703 Lines: 999 Hi David, On Wed, Jul 15, 2015 at 05:54:45PM +0100, David Daney wrote: > From: David Daney > -ENOCOMMITLOG > Signed-off-by: David Daney > --- > drivers/pci/host/Kconfig | 12 + > drivers/pci/host/Makefile | 2 + > drivers/pci/host/pcie-thunder-pem.c | 462 ++++++++++++++++++++++++++++++++++++ > drivers/pci/host/pcie-thunder.c | 422 ++++++++++++++++++++++++++++++++ > 4 files changed, 898 insertions(+) > create mode 100644 drivers/pci/host/pcie-thunder-pem.c > create mode 100644 drivers/pci/host/pcie-thunder.c > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index c132bdd..06e26ad 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -145,4 +145,16 @@ config PCIE_IPROC_BCMA > Say Y here if you want to use the Broadcom iProc PCIe controller > through the BCMA bus interface > > +config PCI_THUNDER_PEM > + bool > + > +config PCI_THUNDER > + bool "Thunder PCIe host controller" > + depends on ARM64 || COMPILE_TEST > + depends on OF_PCI > + select PCI_MSI > + select PCI_THUNDER_PEM > + help > + Say Y here if you want internal PCI support on Thunder SoC. > + > endmenu > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile > index 140d66f..a355155 100644 > --- a/drivers/pci/host/Makefile > +++ b/drivers/pci/host/Makefile > @@ -17,3 +17,5 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o > obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o > obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o > obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o > +obj-$(CONFIG_PCI_THUNDER) += pcie-thunder.o > +obj-$(CONFIG_PCI_THUNDER_PEM) += pcie-thunder-pem.o > diff --git a/drivers/pci/host/pcie-thunder-pem.c b/drivers/pci/host/pcie-thunder-pem.c > new file mode 100644 > index 0000000..7861a8a > --- /dev/null > +++ b/drivers/pci/host/pcie-thunder-pem.c > @@ -0,0 +1,462 @@ > +/* > + * PCIe host controller driver for Cavium Thunder SOC > + * > + * Copyright (C) 2014,2015 Cavium Inc. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + */ > +/* #define DEBUG 1 */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define THUNDER_SLI_S2M_REG_ACC_BASE 0x874001000000ull > + > +#define THUNDER_GIC 0x801000000000ull > +#define THUNDER_GICD_SETSPI_NSR 0x801000000040ull > +#define THUNDER_GICD_CLRSPI_NSR 0x801000000048ull > + > +#define THUNDER_GSER_PCIE_MASK 0x01 > + > +#define PEM_CTL_STATUS 0x000 > +#define PEM_RD_CFG 0x030 > +#define P2N_BAR0_START 0x080 > +#define P2N_BAR1_START 0x088 > +#define P2N_BAR2_START 0x090 > +#define BAR_CTL 0x0a8 > +#define BAR2_MASK 0x0b0 > +#define BAR1_INDEX 0x100 > +#define PEM_CFG 0x410 > +#define PEM_ON 0x420 > + > +struct thunder_pem { > + struct list_head list; /* on thunder_pem_buses */ > + bool connected; > + unsigned int id; > + unsigned int sli; > + unsigned int sli_group; > + unsigned int node; > + u64 sli_window_base; > + void __iomem *bar0; > + void __iomem *bar4; > + void __iomem *sli_s2m; > + void __iomem *cfgregion; > + struct pci_bus *bus; > + int vwire_irqs[4]; > + u32 vwire_data[4]; > +}; > + > +static LIST_HEAD(thunder_pem_buses); > + > +static struct pci_device_id thunder_pem_pci_table[] = { > + {PCI_VENDOR_ID_CAVIUM, 0xa020, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > + {0,} > +}; > +MODULE_DEVICE_TABLE(pci, thunder_pem_pci_table); > + > +enum slix_s2m_ctype { > + CTYPE_MEMORY = 0, > + CTYPE_CONFIG = 1, > + CTYPE_IO = 2 > +}; > + > +static u64 slix_s2m_reg_val(unsigned mac, enum slix_s2m_ctype ctype, > + bool merge, bool relaxed, bool snoop, u32 ba_msb) > +{ > + u64 v; > + > + v = (u64)(mac % 3) << 49; > + v |= (u64)ctype << 53; > + if (!merge) > + v |= 1ull << 48; > + if (relaxed) > + v |= 5ull << 40; > + if (!snoop) > + v |= 5ull << 41; > + v |= (u64)ba_msb; > + > + return v; > +} > + > +static u32 thunder_pcierc_config_read(struct thunder_pem *pem, u32 reg, int size) > +{ > + unsigned int val; > + > + writeq(reg & ~3u, pem->bar0 + PEM_RD_CFG); > + val = readq(pem->bar0 + PEM_RD_CFG) >> 32; > + > + if (size == 1) > + val = (val >> (8 * (reg & 3))) & 0xff; > + else if (size == 2) > + val = (val >> (8 * (reg & 3))) & 0xffff; > + > + return val; > +} > + > +static int thunder_pem_read_config(struct pci_bus *bus, unsigned int devfn, > + int reg, int size, u32 *val) > +{ > + void __iomem *addr; > + struct thunder_pem *pem = bus->sysdata; > + unsigned int busnr = bus->number; > + > + if (busnr > 255 || devfn > 255 || reg > 4095) > + return PCIBIOS_DEVICE_NOT_FOUND; > + > + addr = pem->cfgregion + ((busnr << 24) | (devfn << 16) | reg); > + > + switch (size) { > + case 1: > + *val = readb(addr); > + break; > + case 2: > + *val = readw(addr); > + break; > + case 4: > + *val = readl(addr); > + break; > + default: > + return PCIBIOS_BAD_REGISTER_NUMBER; > + } > + > + return PCIBIOS_SUCCESSFUL; > +} > + > +static int thunder_pem_write_config(struct pci_bus *bus, unsigned int devfn, > + int reg, int size, u32 val) > +{ > + void __iomem *addr; > + struct thunder_pem *pem = bus->sysdata; > + unsigned int busnr = bus->number; > + > + if (busnr > 255 || devfn > 255 || reg > 4095) > + return PCIBIOS_DEVICE_NOT_FOUND; > + > + addr = pem->cfgregion + ((busnr << 24) | (devfn << 16) | reg); > + > + switch (size) { > + case 1: > + writeb(val, addr); > + break; > + case 2: > + writew(val, addr); > + break; > + case 4: > + writel(val, addr); > + break; > + default: > + return PCIBIOS_BAD_REGISTER_NUMBER; > + } > + > + return PCIBIOS_SUCCESSFUL; > +} > + > +static struct pci_ops thunder_pem_ops = { > + .read = thunder_pem_read_config, > + .write = thunder_pem_write_config, > +}; Why can't you use generic PCI config accessors (struct gen_pci_cfg_bus_ops) ? > +static struct thunder_pem *thunder_pem_from_dev(struct pci_dev *dev) > +{ > + struct thunder_pem *pem; > + struct pci_bus *bus = dev->bus; > + > + while (!pci_is_root_bus(bus)) > + bus = bus->parent; > + > + list_for_each_entry(pem, &thunder_pem_buses, list) { > + if (pem->bus == bus) > + return pem; > + } > + return NULL; > +} > + > +int thunder_pem_requester_id(struct pci_dev *dev) > +{ > + struct thunder_pem *pem = thunder_pem_from_dev(dev); > + > + if (!pem) > + return -ENODEV; > + > + if (pem->id < 3) > + return ((1 << 16) | > + ((dev)->bus->number << 8) | > + (dev)->devfn); > + > + if (pem->id < 6) > + return ((3 << 16) | > + ((dev)->bus->number << 8) | > + (dev)->devfn); > + > + if (pem->id < 9) > + return ((1 << 19) | (1 << 16) | > + ((dev)->bus->number << 8) | > + (dev)->devfn); > + > + if (pem->id < 12) > + return ((1 << 19) | > + (3 << 16) | > + ((dev)->bus->number << 8) | > + (dev)->devfn); > + return -ENODEV; > +} > + > +static int thunder_pem_pcibios_add_device(struct pci_dev *dev) > +{ > + struct thunder_pem *pem; > + u8 pin; > + > + pem = thunder_pem_from_dev(dev); > + if (!pem) > + return 0; > + > + pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin); > + > + /* Cope with illegal. */ > + if (pin > 4) > + pin = 1; > + > + dev->irq = pin > 0 ? pem->vwire_irqs[pin - 1] : 0; > + > + if (pin) > + dev_dbg(&dev->dev, "assigning IRQ %02d\n", dev->irq); > + > + pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq); Suspiciously similar to pci_fixup_irqs, so use that + proper DT bindings and remove this function (and drop patch 3). > + > + return 0; > +} > + > +static int thunder_pem_pci_probe(struct pci_dev *pdev, > + const struct pci_device_id *ent) > +{ > + struct thunder_pem *pem; > + resource_size_t bar0_start; > + u64 regval; > + u64 sliaddr, pciaddr; > + u32 cfgval; > + int primary_bus; > + int i; > + int ret = 0; > + struct resource *res; > + LIST_HEAD(resources); > + > + set_pcibios_add_device(thunder_pem_pcibios_add_device); This has to go, see above. > + > + pem = devm_kzalloc(&pdev->dev, sizeof(*pem), GFP_KERNEL); > + if (!pem) > + return -ENOMEM; > + > + pci_set_drvdata(pdev, pem); > + > + bar0_start = pci_resource_start(pdev, 0); > + pem->node = (bar0_start >> 44) & 3; > + pem->id = ((bar0_start >> 24) & 7) + (6 * pem->node); > + pem->sli = pem->id % 3; > + pem->sli_group = (pem->id / 3) % 2; > + pem->sli_window_base = 0x880000000000ull | (((u64)pem->node) << 44) | ((u64)pem->sli_group << 40); > + pem->sli_window_base += 0x4000000000 * pem->sli; > + > + ret = pci_enable_device_mem(pdev); > + if (ret) > + goto out; > + > + pem->bar0 = pcim_iomap(pdev, 0, 0x100000); > + if (!pem->bar0) { > + ret = -ENOMEM; > + goto out; > + } > + > + pem->bar4 = pcim_iomap(pdev, 4, 0x100000); > + if (!pem->bar0) { > + ret = -ENOMEM; > + goto out; > + } > + > + sliaddr = THUNDER_SLI_S2M_REG_ACC_BASE | ((u64)pem->node << 44) | ((u64)pem->sli_group << 36); > + > + regval = readq(pem->bar0 + PEM_ON); > + if (!(regval & 1)) { > + dev_notice(&pdev->dev, "PEM%u_ON not set, skipping...\n", pem->id); > + goto out; > + } > + > + regval = readq(pem->bar0 + PEM_CTL_STATUS); > + regval |= 0x10; /* Set Link Enable bit */ > + writeq(regval, pem->bar0 + PEM_CTL_STATUS); > + > + udelay(1000); > + > + cfgval = thunder_pcierc_config_read(pem, 32 * 4, 4); /* PCIERC_CFG032 */ > + > + if (((cfgval >> 29 & 0x1) == 0x0) || ((cfgval >> 27 & 0x1) == 0x1)) { > + dev_notice(&pdev->dev, "PEM%u Link Timeout, skipping...\n", pem->id); > + goto out; > + } > + > + pem->sli_s2m = devm_ioremap(&pdev->dev, sliaddr, 0x1000); > + if (!pem->sli_s2m) { > + ret = -ENOMEM; > + goto out; > + } I would expect most of the configuration above can be moved to FW so that you can reuse the PCI generic host in the kernel. And all these hardcoded magic numbers are quite irritating to start with. > + > + pem->cfgregion = devm_ioremap(&pdev->dev, pem->sli_window_base, 0x100000000ull); > + if (!pem->cfgregion) { > + ret = -ENOMEM; > + goto out; > + } > + regval = slix_s2m_reg_val(pem->sli, CTYPE_CONFIG, false, false, false, 0); > + writeq(regval, pem->sli_s2m + 0x10 * ((0x40 * pem->sli) + 0)); > + > + cfgval = thunder_pcierc_config_read(pem, 6 * 4, 4); /* PCIERC_CFG006 */ > + primary_bus = (cfgval >> 8) & 0xff; > + > + res = kzalloc(sizeof(*res), GFP_KERNEL); > + if (!res) { > + ret = -ENOMEM; > + goto out; > + } > + res->start = primary_bus; > + res->end = 255; > + res->flags = IORESOURCE_BUS; > + pci_add_resource(&resources, res); > + > + > + res = kzalloc(sizeof(*res), GFP_KERNEL); > + if (!res) { > + ret = -ENOMEM; > + goto out; Leaking memory, but that's not the (only) point. These resources should be parsed from DT (of_pci_get_host_bridge_resources(), why it is done in the driver below and not here is beyond my understanding, please explain). > + } > + res->start = 0x100000 * pem->id; > + res->end = res->start + 0x100000 - 1; > + res->flags = IORESOURCE_IO; > + pci_add_resource(&resources, res); > + regval = slix_s2m_reg_val(pem->sli, CTYPE_IO, false, false, false, 0); > + writeq(regval, pem->sli_s2m + 0x10 * ((0x40 * pem->sli) + 1)); > + > + res = kzalloc(sizeof(*res), GFP_KERNEL); > + if (!res) { > + ret = -ENOMEM; > + goto out; Ditto > + } > + pciaddr = 0x10000000ull; > + res->start = pem->sli_window_base + 0x1000000000ull + pciaddr; > + res->end = res->start + 0x1000000000ull - pciaddr - 1; > + res->flags = IORESOURCE_MEM; > + pci_add_resource_offset(&resources, res, res->start - pciaddr); > + for (i = 0; i < 16; i++) { > + regval = slix_s2m_reg_val(pem->sli, CTYPE_MEMORY, false, false, false, i); > + writeq(regval, pem->sli_s2m + 0x10 * ((0x40 * pem->sli) + (0x10 + i))); > + } > + > + res = kzalloc(sizeof(*res), GFP_KERNEL); > + if (!res) { > + ret = -ENOMEM; > + goto out; Ditto > + } > + pciaddr = 0x1000000000ull; > + res->start = pem->sli_window_base + 0x1000000000ull + pciaddr; > + res->end = res->start + 0x1000000000ull - 1; > + res->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH; > + pci_add_resource_offset(&resources, res, res->start - pciaddr); > + for (i = 0; i < 16; i++) { > + regval = slix_s2m_reg_val(pem->sli, CTYPE_MEMORY, true, true, true, i + 0x10); > + writeq(regval, pem->sli_s2m + 0x10 * ((0x40 * pem->sli) + (0x20 + i))); > + } > + > + writeq(0, pem->bar0 + P2N_BAR0_START); > + writeq(0, pem->bar0 + P2N_BAR1_START); > + writeq(0, pem->bar0 + P2N_BAR2_START); > + > + regval = 0x10; /* BAR_CTL[BAR1_SIZ] = 1 (64MB) */ > + regval |= 0x8; /* BAR_CTL[BAR2_ENB] = 1 */ > + writeq(regval, pem->bar0 + BAR_CTL); > + > + /* 1st 4MB region -> GIC registers so 32-bit MSI can reach the GIC. */ > + regval = (THUNDER_GIC + (((u64)pem->node) << 44)) >> 18; > + /* BAR1_INDEX[ADDR_V] = 1 */ > + regval |= 1; > + writeq(regval, pem->bar0 + BAR1_INDEX); > + /* Remaining regions linear mapping to physical address space */ > + for (i = 1; i < 16; i++) { > + regval = (i << 4) | 1; > + writeq(regval, pem->bar0 + BAR1_INDEX + 8 * i); > + } > + > + pem->bus = pci_create_root_bus(&pdev->dev, primary_bus, &thunder_pem_ops, pem, &resources); > + if (!pem->bus) { > + ret = -ENODEV; > + goto err_root_bus; > + } > + pem->bus->is_pcierc = 1; > + list_add_tail(&pem->list, &thunder_pem_buses); > + > + for (i = 0; i < 3; i++) { > + pem->vwire_data[i] = 40 + 4 * pem->id + i; > + pem->vwire_irqs[i] = irq_create_mapping(gic_get_irq_domain(), pem->vwire_data[i]); > + if (!pem->vwire_irqs[i]) { > + dev_err(&pdev->dev, "Error: No irq mapping for %u\n", pem->vwire_data[i]); > + continue; > + } > + irq_set_irq_type(pem->vwire_irqs[i], IRQ_TYPE_LEVEL_HIGH); > + > + writeq(THUNDER_GICD_SETSPI_NSR, pem->bar4 + 0 + (i + 2) * 32); > + writeq(pem->vwire_data[i], pem->bar4 + 8 + (i + 2) * 32); > + writeq(THUNDER_GICD_CLRSPI_NSR, pem->bar4 + 16 + (i + 2) * 32); > + writeq(pem->vwire_data[i], pem->bar4 + 24 + (i + 2) * 32); > + } > + ret = pci_read_config_dword(pdev, 44 * 4, &cfgval); > + if (WARN_ON(ret)) > + goto err_free_root_bus; > + cfgval &= ~0x40000000; /* Clear FUNM */ > + cfgval |= 0x80000000; /* Set MSIXEN */ > + pci_write_config_dword(pdev, 44 * 4, cfgval); > + pem->bus->msi = pdev->bus->msi; > + > + pci_scan_child_bus(pem->bus); > + pci_bus_add_devices(pem->bus); > + pci_assign_unassigned_root_bus_resources(pem->bus); > + > + return 0; > + > +err_free_root_bus: > + pci_remove_root_bus(pem->bus); > +err_root_bus: > + pci_free_resource_list(&resources); > +out: > + return ret; > +} > + > +static void thunder_pem_pci_remove(struct pci_dev *pdev) > +{ > +} > + > +static struct pci_driver thunder_pem_driver = { > + .name = "thunder_pem", > + .id_table = thunder_pem_pci_table, > + .probe = thunder_pem_pci_probe, > + .remove = thunder_pem_pci_remove > +}; > + > +static int __init thunder_pcie_init(void) > +{ > + int ret; > + > + ret = pci_register_driver(&thunder_pem_driver); > + > + return ret; > +} > +module_init(thunder_pcie_init); > + > +static void __exit thunder_pcie_exit(void) > +{ > + pci_unregister_driver(&thunder_pem_driver); > +} > +module_exit(thunder_pcie_exit); > diff --git a/drivers/pci/host/pcie-thunder.c b/drivers/pci/host/pcie-thunder.c > new file mode 100644 > index 0000000..4abab5a > --- /dev/null > +++ b/drivers/pci/host/pcie-thunder.c > @@ -0,0 +1,422 @@ > +/* > + * PCIe host controller driver for Cavium Thunder SOC > + * > + * Copyright (C) 2014, 2015 Cavium Inc. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define PCI_DEVICE_ID_THUNDER_BRIDGE 0xa002 > + > +#define THUNDER_PCIE_BUS_SHIFT 20 > +#define THUNDER_PCIE_DEV_SHIFT 15 > +#define THUNDER_PCIE_FUNC_SHIFT 12 What's THUNDER specific in this ? > +#define THUNDER_ECAM0_CFG_BASE 0x848000000000 > +#define THUNDER_ECAM1_CFG_BASE 0x849000000000 > +#define THUNDER_ECAM2_CFG_BASE 0x84a000000000 > +#define THUNDER_ECAM3_CFG_BASE 0x84b000000000 > +#define THUNDER_ECAM4_CFG_BASE 0x948000000000 > +#define THUNDER_ECAM5_CFG_BASE 0x949000000000 > +#define THUNDER_ECAM6_CFG_BASE 0x94a000000000 > +#define THUNDER_ECAM7_CFG_BASE 0x94b000000000 You are not telling me you are hardcoding config base addresses here, are you ? > +struct thunder_pcie { > + struct device_node *node; > + struct device *dev; > + void __iomem *cfg_base; > + struct msi_controller *msi; > + int ecam; > + bool valid; > +}; > + > +int thunder_pem_requester_id(struct pci_dev *dev); > + > +static atomic_t thunder_pcie_ecam_probed; > + > +static u32 pci_requester_id_ecam(struct pci_dev *dev) > +{ > + return (((pci_domain_nr(dev->bus) >> 2) << 19) | > + ((pci_domain_nr(dev->bus) % 4) << 16) | > + (dev->bus->number << 8) | dev->devfn); > +} > + > +static u32 thunder_pci_requester_id(struct pci_dev *dev, u16 alias) > +{ > + int ret; > + > + ret = thunder_pem_requester_id(dev); > + if (ret >= 0) > + return (u32)ret; > + > + return pci_requester_id_ecam(dev); > +} > + > +/* > + * This bridge is just for the sake of supporting ARI for > + * downstream devices. No resources are attached to it. > + * Copy upstream root bus resources to bridge which aide in > + * resource claiming for downstream devices > + */ > +static void pci_bridge_resource_fixup(struct pci_dev *dev) > +{ > + struct pci_bus *bus; > + int resno; > + > + bus = dev->subordinate; > + for (resno = 0; resno < PCI_BRIDGE_RESOURCE_NUM; resno++) { > + bus->resource[resno] = pci_bus_resource_n(bus->parent, > + PCI_BRIDGE_RESOURCE_NUM + resno); > + } > + > + for (resno = PCI_BRIDGE_RESOURCES; > + resno <= PCI_BRIDGE_RESOURCE_END; resno++) { > + dev->resource[resno].start = dev->resource[resno].end = 0; > + dev->resource[resno].flags = 0; > + } > +} > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_DEVICE_ID_THUNDER_BRIDGE, > + pci_bridge_resource_fixup); Wouldn't pci_read_bridge_bases() do what you need here ? It will be called from core code. If it does not please explain to me what you want to achieve, thank you. > + * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned. > + * Also claim the device's valid resources to set 'res->parent' hierarchy. > + */ > +static void pci_dev_resource_fixup(struct pci_dev *dev) > +{ > + struct resource *res; > + int resno; > + > + /* > + * If the ECAM is not yet probed, we must be in a virtual > + * machine. In that case, don't mark things as > + * IORESOURCE_PCI_FIXED > + */ > + if (!atomic_read(&thunder_pcie_ecam_probed)) > + return; I do not understand the logic here, and to be frank that seems just a royal hack to me regardless. > + for (resno = 0; resno < PCI_NUM_RESOURCES; resno++) > + dev->resource[resno].flags |= IORESOURCE_PCI_FIXED; > + > + for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) { > + res = &dev->resource[resno]; > + if (res->parent || !(res->flags & IORESOURCE_MEM)) > + continue; > + pci_claim_resource(dev, resno); > + } > +} > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, > + pci_dev_resource_fixup); > + > +static void __iomem *thunder_pcie_get_cfg_addr(struct thunder_pcie *pcie, > + unsigned int busnr, > + unsigned int devfn, int reg) > +{ > + return pcie->cfg_base + > + ((busnr << THUNDER_PCIE_BUS_SHIFT) > + | (PCI_SLOT(devfn) << THUNDER_PCIE_DEV_SHIFT) > + | (PCI_FUNC(devfn) << THUNDER_PCIE_FUNC_SHIFT)) + reg; > +} > + > +static int thunder_pcie_read_config(struct pci_bus *bus, unsigned int devfn, > + int reg, int size, u32 *val) > +{ > + struct thunder_pcie *pcie = bus->sysdata; > + void __iomem *addr; > + unsigned int busnr = bus->number; > + > + if (busnr > 255 || devfn > 255 || reg > 4095) > + return PCIBIOS_DEVICE_NOT_FOUND; > + > + addr = thunder_pcie_get_cfg_addr(pcie, busnr, devfn, reg); > + > + switch (size) { > + case 1: > + *val = readb(addr); > + break; > + case 2: > + *val = readw(addr); > + break; > + case 4: > + *val = readl(addr); > + break; > + default: > + return PCIBIOS_BAD_REGISTER_NUMBER; > + } > + > + return PCIBIOS_SUCCESSFUL; > +} > + > +static int thunder_pcie_write_config(struct pci_bus *bus, unsigned int devfn, > + int reg, int size, u32 val) > +{ > + struct thunder_pcie *pcie = bus->sysdata; > + void __iomem *addr; > + unsigned int busnr = bus->number; > + > + if (busnr > 255 || devfn > 255 || reg > 4095) > + return PCIBIOS_DEVICE_NOT_FOUND; > + > + addr = thunder_pcie_get_cfg_addr(pcie, busnr, devfn, reg); > + > + switch (size) { > + case 1: > + writeb(val, addr); > + break; > + case 2: > + writew(val, addr); > + break; > + case 4: > + writel(val, addr); > + break; > + default: > + return PCIBIOS_BAD_REGISTER_NUMBER; > + } > + > + return PCIBIOS_SUCCESSFUL; > +} > + > +static struct pci_ops thunder_pcie_ops = { > + .read = thunder_pcie_read_config, > + .write = thunder_pcie_write_config, > +}; I have already comment on this above. > +static int thunder_pcie_msi_enable(struct thunder_pcie *pcie, > + struct pci_bus *bus) > +{ > + struct device_node *msi_node; > + > + msi_node = of_parse_phandle(pcie->node, "msi-parent", 0); > + if (!msi_node) > + return -ENODEV; > + > + pcie->msi = of_pci_find_msi_chip_by_node(msi_node); > + if (!pcie->msi) > + return -ENODEV; > + > + pcie->msi->dev = pcie->dev; > + bus->msi = pcie->msi; > + > + return 0; > +} > + > +static void thunder_pcie_config(struct thunder_pcie *pcie, u64 addr) > +{ > + atomic_set(&thunder_pcie_ecam_probed, 1); > + set_its_pci_requester_id(thunder_pci_requester_id); > + > + pcie->valid = true; > + > + switch (addr) { > + case THUNDER_ECAM0_CFG_BASE: > + pcie->ecam = 0; > + break; > + case THUNDER_ECAM1_CFG_BASE: > + pcie->ecam = 1; > + break; > + case THUNDER_ECAM2_CFG_BASE: > + pcie->ecam = 2; > + break; > + case THUNDER_ECAM3_CFG_BASE: > + pcie->ecam = 3; > + break; > + case THUNDER_ECAM4_CFG_BASE: > + pcie->ecam = 4; > + break; > + case THUNDER_ECAM5_CFG_BASE: > + pcie->ecam = 5; > + break; > + case THUNDER_ECAM6_CFG_BASE: > + pcie->ecam = 6; > + break; > + case THUNDER_ECAM7_CFG_BASE: > + pcie->ecam = 7; > + break; > + default: > + pcie->valid = false; > + break; > + } My eyes. You are hardcoding config addresses to check their validity and just to set a number (ecam) to use in a dev_info call ? I'll pretend I have not seen this code, please remove it. > +} > + > +static int thunder_pcie_probe(struct platform_device *pdev) > +{ > + struct thunder_pcie *pcie; > + struct resource *cfg_base; > + struct pci_bus *bus; > + int ret = 0; > + LIST_HEAD(res); > + > + pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL); > + if (!pcie) > + return -ENOMEM; > + > + pcie->node = of_node_get(pdev->dev.of_node); > + pcie->dev = &pdev->dev; > + > + /* Get controller's configuration space range */ > + cfg_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + > + thunder_pcie_config(pcie, cfg_base->start); > + > + pcie->cfg_base = devm_ioremap_resource(&pdev->dev, cfg_base); > + if (IS_ERR(pcie->cfg_base)) { > + ret = PTR_ERR(pcie->cfg_base); > + goto err_ioremap; > + } > + > + dev_info(&pdev->dev, "ECAM%d CFG BASE 0x%llx\n", > + pcie->ecam, (u64)cfg_base->start); > + > + ret = of_pci_get_host_bridge_resources(pdev->dev.of_node, > + 0, 255, &res, NULL); > + if (ret) > + goto err_root_bus; > + > + bus = pci_create_root_bus(&pdev->dev, 0, &thunder_pcie_ops, pcie, &res); > + if (!bus) { > + ret = -ENODEV; > + goto err_root_bus; > + } > + > + /* Set reference to MSI chip */ > + ret = thunder_pcie_msi_enable(pcie, bus); > + if (ret) { > + dev_err(&pdev->dev, > + "Unable to set reference to MSI chip: ret=%d\n", ret); > + goto err_msi; > + } > + > + platform_set_drvdata(pdev, pcie); > + > + pci_scan_child_bus(bus); > + pci_bus_add_devices(bus); > + > + return 0; > +err_msi: > + pci_remove_root_bus(bus); > +err_root_bus: > + pci_free_resource_list(&res); > +err_ioremap: > + of_node_put(pcie->node); > + return ret; > +} > + > +static const struct of_device_id thunder_pcie_of_match[] = { > + { .compatible = "cavium,thunder-pcie", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, thunder_pcie_of_match); > + > +static struct platform_driver thunder_pcie_driver = { > + .driver = { > + .name = "thunder-pcie", > + .owner = THIS_MODULE, > + .of_match_table = thunder_pcie_of_match, > + }, > + .probe = thunder_pcie_probe, > +}; > +module_platform_driver(thunder_pcie_driver); > + > +#ifdef CONFIG_ACPI > + > +static int > +thunder_mmcfg_read_config(struct pci_mmcfg_region *cfg, unsigned int busnr, > + unsigned int devfn, int reg, int len, u32 *value) > +{ > + struct thunder_pcie *pcie = cfg->data; > + void __iomem *addr; > + > + if (!pcie->valid) { > + /* Not support for now */ > + pr_err("RC PEM not supported !!!\n"); > + return PCIBIOS_DEVICE_NOT_FOUND; > + } > + > + addr = thunder_pcie_get_cfg_addr(pcie, busnr, devfn, reg); > + > + switch (len) { > + case 1: > + *value = readb(addr); > + break; > + case 2: > + *value = readw(addr); > + break; > + case 4: > + *value = readl(addr); > + break; > + default: > + return PCIBIOS_BAD_REGISTER_NUMBER; > + } > + > + return PCIBIOS_SUCCESSFUL; > +} > + > +static int thunder_mmcfg_write_config(struct pci_mmcfg_region *cfg, > + unsigned int busnr, unsigned int devfn, int reg, int len, > + u32 value) { > + struct thunder_pcie *pcie = cfg->data; > + void __iomem *addr; > + > + if (!pcie->valid) { > + /* Not support for now */ > + pr_err("RC PEM not supported !!!\n"); > + return PCIBIOS_DEVICE_NOT_FOUND; > + } > + > + addr = thunder_pcie_get_cfg_addr(pcie, busnr, devfn, reg); > + > + switch (len) { > + case 1: > + writeb(value, addr); > + break; > + case 2: > + writew(value, addr); > + break; > + case 4: > + writel(value, addr); > + break; > + default: > + return PCIBIOS_BAD_REGISTER_NUMBER; > + } > + > + return PCIBIOS_SUCCESSFUL; > +} Again these duplicate config accessors here. > +static int thunder_acpi_mcfg_fixup(struct acpi_pci_root *root, > + struct pci_mmcfg_region *cfg) > +{ > + struct thunder_pcie *pcie; > + > + pcie = kzalloc(sizeof(*pcie), GFP_KERNEL); > + if (!pcie) > + return -ENOMEM; > + > + pcie->dev = &root->device->dev; > + > + thunder_pcie_config(pcie, cfg->address); > + > + pcie->cfg_base = cfg->virt; > + cfg->data = pcie; > + cfg->read = thunder_mmcfg_read_config; > + cfg->write = thunder_mmcfg_write_config; > + > + return 0; > +} > +DECLARE_ACPI_MCFG_FIXUP("CAVIUM", "THUNDERX", thunder_acpi_mcfg_fixup); > +#endif I must have missed MCFG support for ARM64. if this patch depends on something else you have to tell us, otherwise reviewing it is close to useless. Thanks, Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/