Re: [PATCH] PCI: al: add pcie-al.c

From: Jonathan Chocron
Date: Mon Mar 25 2019 - 11:56:45 EST

On 3/25/19 14:58, Bjorn Helgaas wrote:
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
Done.
On Mon, Mar 25, 2019 at 01:07:20PM +0200, jonnyc@xxxxxxxxxx wrote:
From: Jonathan Chocron <jonnyc@xxxxxxxxxx>

Adding support for Amazon's Annapurna Labs pcie driver.
s/pcie/PCIe/

Done.
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/
Done.

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.

Actually, the _HID is AMZN0001 and the _CID is PNP0C02, so that's why

I used PNP0C02 (to state that it is of this "type"). Please let me know if

you still think it should be changed, so I'll integrate it into v2.

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.
Done.

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 <vaerov@xxxxxxxxxx>
Signed-off-by: Jonathan Chocron <jonnyc@xxxxxxxxxx>
Signed-off-by: Vladimir Aerov <vaerov@xxxxxxxxxx>
Reviewed-by: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
Reviewed-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
---
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 <jonnyc@xxxxxxxxxx>
+L: linux-pci@xxxxxxxxxxxxxxx
+S: Maintained
+F: drivers/pci/controller/dwc/pcie-al.c
+
PCIE DRIVER FOR AMLOGIC MESON
M: Yue Wang <yue.wang@xxxxxxxxxxx>
L: linux-pci@xxxxxxxxxxxxxxx
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 <jonnyc@xxxxxxxxxx>
+ */
+
+#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 ..
*/
Done.
+ 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