Re: [PATCH v3] PCI/ACPI: xgene: Add ECAM quirk for X-Gene PCIe controller

From: Mark Salter
Date: Thu Dec 01 2016 - 10:08:27 EST


On Wed, 2016-11-30 at 15:42 -0800, Duc Dang wrote:
> PCIe controllers in X-Gene SoCs is not ECAM compliant: software
> needs to configure additional controller's register to address
> device at bus:dev:function.
>
> The quirk will only be applied for X-Gene PCIe MCFG table with
> OEM revison 1, 2, 3 or 4 (PCIe controller v1 and v2 on X-Gene SoCs).
>
> The quirk declares the X-Gene PCIe controller register space as 64KB
> fixed memory resource with name "PCIe CSR". This name will be showed
> next to the resource range in the output of "cat /proc/iomem".
>
> Signed-off-by: Duc Dang <dhdang@xxxxxxx>
> ---
> v3:
> Â - Rebase on top of pci/ecam-v6 tree.
> Â - Use DEFINE_RES_MEM_NAMED to declare controller register space
> Â with name "PCIe CSR"
> v2:
> Â RFC v2: https://patchwork.ozlabs.org/patch/686846/
> v1:
> Â RFC v1: https://patchwork.kernel.org/patch/9337115/
>
> Âdrivers/acpi/pci_mcfg.cÂÂÂÂÂÂ|ÂÂ31 ++++++++
> Âdrivers/pci/host/pci-xgene.c | 165 ++++++++++++++++++++++++++++++++++++++++++-
> Âinclude/linux/pci-ecam.hÂÂÂÂÂ|ÂÂÂ7 ++
> Â3 files changed, 200 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index ac21db3..eb6125b 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -57,6 +57,37 @@ struct mcfg_fixup {
> Â { "QCOMÂÂ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
> Â { "QCOMÂÂ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
> Â { "QCOMÂÂ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
> +
> +#ifdef CONFIG_PCI_XGENE
> +#define XGENE_V1_ECAM_MCFG(rev, seg) \
> + {"APMÂÂÂ", "XGENEÂÂÂ", rev, seg, MCFG_BUS_ANY, \
> + &xgene_v1_pcie_ecam_ops }
> +#define XGENE_V2_1_ECAM_MCFG(rev, seg) \
> + {"APMÂÂÂ", "XGENEÂÂÂ", rev, seg, MCFG_BUS_ANY, \
> + &xgene_v2_1_pcie_ecam_ops }
> +#define XGENE_V2_2_ECAM_MCFG(rev, seg) \
> + {"APMÂÂÂ", "XGENEÂÂÂ", rev, seg, MCFG_BUS_ANY, \
> + &xgene_v2_2_pcie_ecam_ops }
> +
> + /* X-Gene SoC with v1 PCIe controller */
> + XGENE_V1_ECAM_MCFG(1, 0),
> + XGENE_V1_ECAM_MCFG(1, 1),
> + XGENE_V1_ECAM_MCFG(1, 2),
> + XGENE_V1_ECAM_MCFG(1, 3),
> + XGENE_V1_ECAM_MCFG(1, 4),
> + XGENE_V1_ECAM_MCFG(2, 0),
> + XGENE_V1_ECAM_MCFG(2, 1),
> + XGENE_V1_ECAM_MCFG(2, 2),
> + XGENE_V1_ECAM_MCFG(2, 3),
> + XGENE_V1_ECAM_MCFG(2, 4),
> + /* X-Gene SoC with v2.1 PCIe controller */
> + XGENE_V2_1_ECAM_MCFG(3, 0),
> + XGENE_V2_1_ECAM_MCFG(3, 1),
> + /* X-Gene SoC with v2.2 PCIe controller */
> + XGENE_V2_2_ECAM_MCFG(4, 0),
> + XGENE_V2_2_ECAM_MCFG(4, 1),
> + XGENE_V2_2_ECAM_MCFG(4, 2),
> +#endif
> Â};
> Â
> Âstatic char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> index 1de23d7..43d7c69 100644
> --- a/drivers/pci/host/pci-xgene.c
> +++ b/drivers/pci/host/pci-xgene.c
> @@ -27,6 +27,8 @@
> Â#include <linux/of_irq.h>
> Â#include <linux/of_pci.h>
> Â#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> Â#include <linux/platform_device.h>
> Â#include <linux/slab.h>
> Â
> @@ -64,6 +66,7 @@
> Â/* PCIe IP version */
> Â#define XGENE_PCIE_IP_VER_UNKN 0
> Â#define XGENE_PCIE_IP_VER_1 1
> +#define XGENE_PCIE_IP_VER_2 2
> Â
> Âstruct xgene_pcie_port {
> Â struct device_node *node;
> @@ -97,7 +100,15 @@ static inline u32 pcie_bar_low_val(u32 addr, u32 flags)
> Â */
> Âstatic void __iomem *xgene_pcie_get_cfg_base(struct pci_bus *bus)
> Â{
> - struct xgene_pcie_port *port = bus->sysdata;
> + struct pci_config_window *cfg;
> + struct xgene_pcie_port *port;
> +
> + if (acpi_disabled)
> + port = bus->sysdata;
> + else {
> + cfg = bus->sysdata;
> + port = cfg->priv;
> + }
> Â
> Â if (bus->number >= (bus->primary + 1))
> Â return port->cfg_base + AXI_EP_CFG_ACCESS;
> @@ -111,10 +122,18 @@ static void __iomem *xgene_pcie_get_cfg_base(struct pci_bus *bus)
> Â */
> Âstatic void xgene_pcie_set_rtdid_reg(struct pci_bus *bus, uint devfn)
> Â{
> - struct xgene_pcie_port *port = bus->sysdata;
> + struct pci_config_window *cfg;
> + struct xgene_pcie_port *port;
> Â unsigned int b, d, f;
> Â u32 rtdid_val = 0;
> Â
> + if (acpi_disabled)
> + port = bus->sysdata;
> + else {
> + cfg = bus->sysdata;
> + port = cfg->priv;
> + }
> +
> Â b = bus->number;
> Â d = PCI_SLOT(devfn);
> Â f = PCI_FUNC(devfn);
> @@ -158,7 +177,15 @@ static void __iomem *xgene_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
> Âstatic int xgene_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
> Â ÂÂÂÂint where, int size, u32 *val)
> Â{
> - struct xgene_pcie_port *port = bus->sysdata;
> + struct pci_config_window *cfg;
> + struct xgene_pcie_port *port;
> +
> + if (acpi_disabled)
> + port = bus->sysdata;
> + else {
> + cfg = bus->sysdata;
> + port = cfg->priv;
> + }
> Â
> Â if (pci_generic_config_read32(bus, devfn, where & ~0x3, 4, val) !=
> Â ÂÂÂÂPCIBIOS_SUCCESSFUL)
> @@ -189,6 +216,138 @@ static int xgene_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
> Â .write = pci_generic_config_write32,
> Â};
> Â
> +#ifdef CONFIG_ACPI
> +static struct resource xgene_v1_csr_res[] = {
> + [0] = DEFINE_RES_MEM_NAMED(0x1f2b0000UL, SZ_64K, "PCIe CSR"),
> + [1] = DEFINE_RES_MEM_NAMED(0x1f2c0000UL, SZ_64K, "PCIe CSR"),
> + [2] = DEFINE_RES_MEM_NAMED(0x1f2d0000UL, SZ_64K, "PCIe CSR"),
> + [3] = DEFINE_RES_MEM_NAMED(0x1f500000UL, SZ_64K, "PCIe CSR"),
> + [4] = DEFINE_RES_MEM_NAMED(0x1f510000UL, SZ_64K, "PCIe CSR"),
> +};
> +
> +static int xgene_v1_pcie_ecam_init(struct pci_config_window *cfg)
> +{
> + struct acpi_device *adev = to_acpi_device(cfg->parent);
> + struct acpi_pci_root *root = acpi_driver_data(adev);
> + struct device *dev = cfg->parent;
> + struct xgene_pcie_port *port;
> + struct resource *csr;
> +
> + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + csr = &xgene_v1_csr_res[root->segment];

This hard-coded assumption that segment N uses controller N breaks
for m400 where segment 0 is using controller 3.

> + port->csr_base = devm_ioremap_resource(dev, csr);
> + if (IS_ERR(port->csr_base)) {
> + kfree(port);
> + return -ENOMEM;
> + }
> +
> + port->cfg_base = cfg->win;
> + port->version = XGENE_PCIE_IP_VER_1;
> +
> + cfg->priv = port;
> +
> + return 0;
> +}
> +
> +struct pci_ecam_ops xgene_v1_pcie_ecam_ops = {
> + .bus_shiftÂÂÂÂÂÂ= 16,
> + .initÂÂÂÂÂÂÂÂÂÂÂ= xgene_v1_pcie_ecam_init,
> + .pci_opsÂÂÂÂÂÂÂÂ= {
> + .map_busÂÂÂÂÂÂÂÂ= xgene_pcie_map_bus,
> + .readÂÂÂÂÂÂÂÂÂÂÂ= xgene_pcie_config_read32,
> + .writeÂÂÂÂÂÂÂÂÂÂ= pci_generic_config_write,
> + }
> +};
> +
> +static struct resource xgene_v2_1_csr_res[] = {
> + [0] = DEFINE_RES_MEM_NAMED(0x1f2b0000UL, SZ_64K, "PCIe CSR"),
> + [1] = DEFINE_RES_MEM_NAMED(0x1f2c0000UL, SZ_64K, "PCIe CSR"),
> +};
> +
> +static int xgene_v2_1_pcie_ecam_init(struct pci_config_window *cfg)
> +{
> + struct acpi_device *adev = to_acpi_device(cfg->parent);
> + struct acpi_pci_root *root = acpi_driver_data(adev);
> + struct device *dev = cfg->parent;
> + struct xgene_pcie_port *port;
> + struct resource *csr;
> +
> + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + csr = &xgene_v2_1_csr_res[root->segment];
> + port->csr_base = devm_ioremap_resource(dev, csr);
> + if (IS_ERR(port->csr_base)) {
> + kfree(port);
> + return -ENOMEM;
> + }
> +
> + port->cfg_base = cfg->win;
> + port->version = XGENE_PCIE_IP_VER_2;
> +
> + cfg->priv = port;
> +
> + return 0;
> +}
> +
> +struct pci_ecam_ops xgene_v2_1_pcie_ecam_ops = {
> + .bus_shiftÂÂÂÂÂÂ= 16,
> + .initÂÂÂÂÂÂÂÂÂÂÂ= xgene_v2_1_pcie_ecam_init,
> + .pci_opsÂÂÂÂÂÂÂÂ= {
> + .map_busÂÂÂÂÂÂÂÂ= xgene_pcie_map_bus,
> + .readÂÂÂÂÂÂÂÂÂÂÂ= xgene_pcie_config_read32,
> + .writeÂÂÂÂÂÂÂÂÂÂ= pci_generic_config_write,
> + }
> +};
> +
> +static struct resource xgene_v2_2_csr_res[] = {
> + [0] = DEFINE_RES_MEM_NAMED(0x1f2b0000UL, SZ_64K, "PCIe CSR"),
> + [1] = DEFINE_RES_MEM_NAMED(0x1f500000UL, SZ_64K, "PCIe CSR"),
> + [2] = DEFINE_RES_MEM_NAMED(0x1f2d0000UL, SZ_64K, "PCIe CSR"),
> +};
> +
> +static int xgene_v2_2_pcie_ecam_init(struct pci_config_window *cfg)
> +{
> + struct acpi_device *adev = to_acpi_device(cfg->parent);
> + struct acpi_pci_root *root = acpi_driver_data(adev);
> + struct device *dev = cfg->parent;
> + struct xgene_pcie_port *port;
> + struct resource *csr;
> +
> + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + csr = &xgene_v2_2_csr_res[root->segment];
> + port->csr_base = devm_ioremap_resource(dev, csr);
> + if (IS_ERR(port->csr_base)) {
> + kfree(port);
> + return -ENOMEM;
> + }
> +
> + port->cfg_base = cfg->win;
> + port->version = XGENE_PCIE_IP_VER_2;
> +
> + cfg->priv = port;
> +
> + return 0;
> +}
> +
> +struct pci_ecam_ops xgene_v2_2_pcie_ecam_ops = {
> + .bus_shiftÂÂÂÂÂÂ= 16,
> + .initÂÂÂÂÂÂÂÂÂÂÂ= xgene_v2_2_pcie_ecam_init,
> + .pci_opsÂÂÂÂÂÂÂÂ= {
> + .map_busÂÂÂÂÂÂÂÂ= xgene_pcie_map_bus,
> + .readÂÂÂÂÂÂÂÂÂÂÂ= xgene_pcie_config_read32,
> + .writeÂÂÂÂÂÂÂÂÂÂ= pci_generic_config_write,
> + }
> +};
> +#endif
> +
> Âstatic u64 xgene_pcie_set_ib_mask(struct xgene_pcie_port *port, u32 addr,
> Â ÂÂu32 flags, u64 size)
> Â{
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index f5740b7..47ab947 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -62,6 +62,13 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
> Â/* ops for buggy ECAM that supports only 32-bit accesses */
> Âextern struct pci_ecam_ops pci_32b_ops;
> Â
> +/* ECAM ops for known quirks */
> +#ifdef CONFIG_PCI_XGENE
> +extern struct pci_ecam_ops xgene_v1_pcie_ecam_ops;
> +extern struct pci_ecam_ops xgene_v2_1_pcie_ecam_ops;
> +extern struct pci_ecam_ops xgene_v2_2_pcie_ecam_ops;
> +#endif
> +
> Â#ifdef CONFIG_PCI_HOST_GENERIC
> Â/* for DT-based PCI controllers that support ECAM */
> Âint pci_host_common_probe(struct platform_device *pdev,