Hi Jonathan,
Looks good to me. Looks like this hardware is sooooo close to Just
Working with the generic driver; it's too bad we have to add more ECAM
quirks, but sometimes life gives us lemons. Trivial comments below.
Lorenzo may have additional comments.
The subject should be something like:
PCI: al: Add Amazon Annapurna Labs PCIe host controller driver
On Mon, Mar 25, 2019 at 01:07:20PM +0200, jon...@amazon.com wrote:
> From: Jonathan Chocron <jon...@amazon.com>
>
> Adding support for Amazon's Annapurna Labs pcie driver.
s/pcie/PCIe/
> The HW controller is based on Designware's IP.
>
> The HW doesn't support accessing the Root port's config space via
> ECAM, so we obtain its base address via a PNP0C02 device.
s/port's/Port's/
I think you're actually looking for a AMZN0001 device, not a PNP0C02
device. Your firmware might have a PNP0C02 _HID and AMZN0001 _CID, but
that's not relevant here since you're only filtering by AMZN0001.
> Furthermore, the DesignWare PCIe controller doesn't filter out
> config transactions sent to devices 1 and up on its bus, so they
> are filtered by the driver.
Separate paragraphs with blank lines.
> All subordinate buses do support ECAM access.
>
> Implementing specific PCI config access functions involves:
> - Adding an init function to obtain the Root Port's base address
> from a PNP0C02 device.
s/PNP0C02/AMZN0001/
> - Adding a new entry in the mcfg quirk array
>
> Co-developed-by: Vladimir Aerov <vae...@amazon.com>
> Signed-off-by: Jonathan Chocron <jon...@amazon.com>
> Signed-off-by: Vladimir Aerov <vae...@amazon.com>
> Reviewed-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> Reviewed-by: David Woodhouse <d...@amazon.co.uk>
> ---
> MAINTAINERS | 6 +++
> drivers/acpi/pci_mcfg.c | 12 +++++
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-al.c | 92
> ++++++++++++++++++++++++++++++++++++
> include/linux/pci-ecam.h | 1 +
> 5 files changed, 112 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pcie-al.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 32d444476a90..7a17017f9f82 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11769,6 +11769,12 @@ T: git
> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/
> S: Supported
> F: drivers/pci/controller/
>
> +PCIE DRIVER FOR ANNAPURNA LABS
> +M: Jonathan Chocron <jon...@amazon.com>
> +L: linux-...@vger.kernel.org
> +S: Maintained
> +F: drivers/pci/controller/dwc/pcie-al.c
> +
> PCIE DRIVER FOR AMLOGIC MESON
> M: Yue Wang <yue.w...@amlogic.com>
> L: linux-...@vger.kernel.org
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index a4e8432fc2fb..b42be067fb83 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -52,6 +52,18 @@ struct mcfg_fixup {
> static struct mcfg_fixup mcfg_quirks[] = {
> /* { OEM_ID, OEM_TABLE_ID, REV, SEGMENT, BUS_RANGE, ops, cfgres }, */
>
> +#define AL_ECAM(table_id, rev, seg, ops) \
> + { "AMAZON", table_id, rev, seg, MCFG_BUS_ANY, ops }
> +
> + AL_ECAM("GRAVITON", 0, 0, &al_pcie_ops),
> + AL_ECAM("GRAVITON", 0, 1, &al_pcie_ops),
> + AL_ECAM("GRAVITON", 0, 2, &al_pcie_ops),
> + AL_ECAM("GRAVITON", 0, 3, &al_pcie_ops),
> + AL_ECAM("GRAVITON", 0, 4, &al_pcie_ops),
> + AL_ECAM("GRAVITON", 0, 5, &al_pcie_ops),
> + AL_ECAM("GRAVITON", 0, 6, &al_pcie_ops),
> + AL_ECAM("GRAVITON", 0, 7, &al_pcie_ops),
> +
> #define QCOM_ECAM32(seg) \
> { "QCOM ", "QDF2432 ", 1, seg, MCFG_BUS_ANY, &pci_32b_ops }
>
> diff --git a/drivers/pci/controller/dwc/Makefile
> b/drivers/pci/controller/dwc/Makefile
> index 7bcdcdf5024e..1ea773c0070d 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -28,5 +28,6 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> # depending on whether ACPI, the DT driver, or both are enabled.
>
> ifdef CONFIG_PCI
> +obj-$(CONFIG_ARM64) += pcie-al.o
> obj-$(CONFIG_ARM64) += pcie-hisi.o
> endif
> diff --git a/drivers/pci/controller/dwc/pcie-al.c
> b/drivers/pci/controller/dwc/pcie-al.c
> new file mode 100644
> index 000000000000..019497c2c714
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-al.c
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe host controller driver for Amazon's Annapurna Labs IP (used in chips
> + * such as Graviton and Alpine)
> + *
> + * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
> + *
> + * Author: Jonathan Chocron <jon...@amazon.com>
> + */
> +
> +#include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> +#include <linux/pci-acpi.h>
> +#include "../../pci.h"
> +
> +#if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS)
> +
> +struct al_pcie_acpi {
> + void __iomem *dbi_base;
> +};
> +
> +static void __iomem *al_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
> + int where)
> +{
> + struct pci_config_window *cfg = bus->sysdata;
> + struct al_pcie_acpi *pcie = cfg->priv;
> + void __iomem *dbi_base = pcie->dbi_base;
> +
> + if (bus->number == cfg->busr.start) {
> + /* The DW PCIe core doesn't filter out transactions to other
> + * devices/functions on the primary bus num, so we do this here.
> + */
Use usual multi-line comment style:
/*
* text ..
*/
> + if (PCI_SLOT(devfn) > 0)
> + return NULL;
> + else
> + return dbi_base + where;
> + }
> +
> + return pci_ecam_map_bus(bus, devfn, where);
> +}
> +
> +static int al_pcie_init(struct pci_config_window *cfg)
> +{
> + struct device *dev = cfg->parent;
> + struct acpi_device *adev = to_acpi_device(dev);
> + struct acpi_pci_root *root = acpi_driver_data(adev);
> + struct al_pcie_acpi *al_pcie;
> + struct resource *res;
> + int ret;
> +
> + al_pcie = devm_kzalloc(dev, sizeof(*al_pcie), GFP_KERNEL);
> + if (!al_pcie)
> + return -ENOMEM;
> +
> + res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL);
> + if (!res)
> + return -ENOMEM;
> +
> + ret = acpi_get_rc_resources(dev, "AMZN0001", root->segment, res);
> + if (ret) {
> + dev_err(dev, "can't get rc dbi base address for SEG %d\n",
> + root->segment);
> + return ret;
> + }
> +
> + dev_dbg(dev, "Root port dbi res: %pR\n", res);
> +
> + al_pcie->dbi_base = devm_pci_remap_cfg_resource(dev, res);
> + if (IS_ERR(al_pcie->dbi_base)) {
> + long err = PTR_ERR(al_pcie->dbi_base);
> +
> + dev_err(dev, "couldn't remap dbi base %pR (err:%ld)\n",
> + res, err);
> + return err;
> + }
> +
> + cfg->priv = al_pcie;
> +
> + return 0;
> +}
> +
> +struct pci_ecam_ops al_pcie_ops = {
> + .bus_shift = 20,
> + .init = al_pcie_init,
> + .pci_ops = {
> + .map_bus = al_pcie_map_bus,
> + .read = pci_generic_config_read,
> + .write = pci_generic_config_write,
> + }
> +};
> +
> +#endif /* defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS) */
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index 29efa09d686b..a73164c85e78 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -56,6 +56,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus,
> unsigned int devfn,
> extern struct pci_ecam_ops pci_thunder_ecam_ops; /* Cavium ThunderX 1.x */
> extern struct pci_ecam_ops xgene_v1_pcie_ecam_ops; /* APM X-Gene PCIe v1 */
> extern struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */
> +extern struct pci_ecam_ops al_pcie_ops; /* Amazon Annapurna Labs PCIe */
> #endif
>
> #ifdef CONFIG_PCI_HOST_COMMON
> --
> 1.9.1
>
