[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f2c79fae-d35c-a17f-8b5c-d2404ff067a0@amazon.com>
Date: Mon, 25 Mar 2019 17:56:35 +0200
From: Jonathan Chocron <jonnyc@...zon.com>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: <linux-pci@...r.kernel.org>, <lorenzo.pieralisi@....com>,
<linux-kernel@...r.kernel.org>, <vaerov@...zon.com>,
<dwmw@...zon.co.uk>, <benh@...nel.crashing.org>,
<alisaidi@...zon.com>, <zeev@...zon.com>, <ronenk@...zon.com>,
<barakw@...zon.com>, Talel Shenhar <talel@...zon.com>
Subject: Re: [PATCH] PCI: al: add pcie-al.c
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@...zon.com wrote:
>> From: Jonathan Chocron <jonnyc@...zon.com>
>>
>> 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@...zon.com>
>> Signed-off-by: Jonathan Chocron <jonnyc@...zon.com>
>> Signed-off-by: Vladimir Aerov <vaerov@...zon.com>
>> Reviewed-by: Benjamin Herrenschmidt <benh@...nel.crashing.org>
>> Reviewed-by: David Woodhouse <dwmw@...zon.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 <jonnyc@...zon.com>
>> +L: linux-pci@...r.kernel.org
>> +S: Maintained
>> +F: drivers/pci/controller/dwc/pcie-al.c
>> +
>> PCIE DRIVER FOR AMLOGIC MESON
>> M: Yue Wang <yue.wang@...ogic.com>
>> L: linux-pci@...r.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 <jonnyc@...zon.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 ..
> */
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
>>
Powered by blists - more mailing lists