* [PATCH] PCI: al: add pcie-al.c @ 2019-03-25 11:07 jonnyc 2019-03-25 12:58 ` Bjorn Helgaas 2019-03-26 10:00 ` [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver Jonathan Chocron 0 siblings, 2 replies; 21+ messages in thread From: jonnyc @ 2019-03-25 11:07 UTC (permalink / raw) To: linux-pci Cc: lorenzo.pieralisi, bhelgaas, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw, jonnyc From: Jonathan Chocron <jonnyc@amazon.com> Adding support for Amazon's Annapurna Labs pcie driver. 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. 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. 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. - Adding a new entry in the mcfg quirk array Co-developed-by: Vladimir Aerov <vaerov@amazon.com> Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> Signed-off-by: Vladimir Aerov <vaerov@amazon.com> Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Reviewed-by: David Woodhouse <dwmw@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 <jonnyc@amazon.com> +L: linux-pci@vger.kernel.org +S: Maintained +F: drivers/pci/controller/dwc/pcie-al.c + PCIE DRIVER FOR AMLOGIC MESON M: Yue Wang <yue.wang@Amlogic.com> L: linux-pci@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 <jonnyc@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. + */ + 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] PCI: al: add pcie-al.c 2019-03-25 11:07 [PATCH] PCI: al: add pcie-al.c jonnyc @ 2019-03-25 12:58 ` Bjorn Helgaas 2019-03-25 15:56 ` Jonathan Chocron 2019-03-26 10:00 ` [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver Jonathan Chocron 1 sibling, 1 reply; 21+ messages in thread From: Bjorn Helgaas @ 2019-03-25 12:58 UTC (permalink / raw) To: jonnyc Cc: linux-pci, lorenzo.pieralisi, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw 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, jonnyc@amazon.com wrote: > From: Jonathan Chocron <jonnyc@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 <vaerov@amazon.com> > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > Signed-off-by: Vladimir Aerov <vaerov@amazon.com> > Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Reviewed-by: David Woodhouse <dwmw@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 <jonnyc@amazon.com> > +L: linux-pci@vger.kernel.org > +S: Maintained > +F: drivers/pci/controller/dwc/pcie-al.c > + > PCIE DRIVER FOR AMLOGIC MESON > M: Yue Wang <yue.wang@Amlogic.com> > L: linux-pci@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 <jonnyc@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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] PCI: al: add pcie-al.c 2019-03-25 12:58 ` Bjorn Helgaas @ 2019-03-25 15:56 ` Jonathan Chocron 2019-03-25 17:36 ` Bjorn Helgaas 0 siblings, 1 reply; 21+ messages in thread From: Jonathan Chocron @ 2019-03-25 15:56 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, lorenzo.pieralisi, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw, Talel Shenhar 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@amazon.com wrote: >> From: Jonathan Chocron <jonnyc@amazon.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@amazon.com> >> Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> >> Signed-off-by: Vladimir Aerov <vaerov@amazon.com> >> Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Reviewed-by: David Woodhouse <dwmw@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 <jonnyc@amazon.com> >> +L: linux-pci@vger.kernel.org >> +S: Maintained >> +F: drivers/pci/controller/dwc/pcie-al.c >> + >> PCIE DRIVER FOR AMLOGIC MESON >> M: Yue Wang <yue.wang@Amlogic.com> >> L: linux-pci@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 <jonnyc@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 .. > */ 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 >> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] PCI: al: add pcie-al.c 2019-03-25 15:56 ` Jonathan Chocron @ 2019-03-25 17:36 ` Bjorn Helgaas 0 siblings, 0 replies; 21+ messages in thread From: Bjorn Helgaas @ 2019-03-25 17:36 UTC (permalink / raw) To: Jonathan Chocron Cc: linux-pci, lorenzo.pieralisi, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw, Talel Shenhar On Mon, Mar 25, 2019 at 05:56:35PM +0200, Jonathan Chocron wrote: > On 3/25/19 14:58, Bjorn Helgaas wrote: > > 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. I do still think it should be changed, because there's nothing in the code that refers to PNP0C02. That _CID could be absent or could contain any other ID and it wouldn't make any difference to the driver. But if there's no device with _HID or _CID of AMZN0001, the driver will fail. Bjorn ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-25 11:07 [PATCH] PCI: al: add pcie-al.c jonnyc 2019-03-25 12:58 ` Bjorn Helgaas @ 2019-03-26 10:00 ` Jonathan Chocron 2019-03-26 12:17 ` Lorenzo Pieralisi 2019-03-26 12:55 ` Bjorn Helgaas 1 sibling, 2 replies; 21+ messages in thread From: Jonathan Chocron @ 2019-03-26 10:00 UTC (permalink / raw) To: linux-pci Cc: lorenzo.pieralisi, bhelgaas, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw, jonnyc Adding support for Amazon's Annapurna Labs PCIe driver. 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 an AMZN0001 device. 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. 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 an AMZN0001 device. - Adding a new entry in the mcfg quirk array Co-developed-by: Vladimir Aerov <vaerov@amazon.com> Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> Signed-off-by: Vladimir Aerov <vaerov@amazon.com> Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> --- Changes from v1: - Fix commit message comments (incl. using AMZN0001 instead of PNP0C02) - Use the usual multi-line comment style MAINTAINERS | 6 +++ drivers/acpi/pci_mcfg.c | 12 +++++ drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-al.c | 93 ++++++++++++++++++++++++++++++++++++ include/linux/pci-ecam.h | 1 + 5 files changed, 113 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@amazon.com> +L: linux-pci@vger.kernel.org +S: Maintained +F: drivers/pci/controller/dwc/pcie-al.c + PCIE DRIVER FOR AMLOGIC MESON M: Yue Wang <yue.wang@Amlogic.com> L: linux-pci@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..65a9776c12be --- /dev/null +++ b/drivers/pci/controller/dwc/pcie-al.c @@ -0,0 +1,93 @@ +// 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@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. + */ + 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-26 10:00 ` [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver Jonathan Chocron @ 2019-03-26 12:17 ` Lorenzo Pieralisi 2019-03-26 13:24 ` David Woodhouse 2019-03-27 9:43 ` David Woodhouse 2019-03-26 12:55 ` Bjorn Helgaas 1 sibling, 2 replies; 21+ messages in thread From: Lorenzo Pieralisi @ 2019-03-26 12:17 UTC (permalink / raw) To: Jonathan Chocron Cc: linux-pci, bhelgaas, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw, Gustavo Pimentel, Zhou Wang [+Zhou, Gustavo] On Tue, Mar 26, 2019 at 12:00:55PM +0200, Jonathan Chocron wrote: > Adding support for Amazon's Annapurna Labs PCIe driver. > 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 an AMZN0001 device. > > 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. > 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 an AMZN0001 device. > - Adding a new entry in the mcfg quirk array > > Co-developed-by: Vladimir Aerov <vaerov@amazon.com> > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > Signed-off-by: Vladimir Aerov <vaerov@amazon.com> > Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> Review tags should be given on public mailing lists for public review and I have not seen them (they were already there in v1) so you should drop them. > Changes from v1: > - Fix commit message comments (incl. using AMZN0001 > instead of PNP0C02) > - Use the usual multi-line comment style > > MAINTAINERS | 6 +++ > drivers/acpi/pci_mcfg.c | 12 +++++ > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-al.c | 93 ++++++++++++++++++++++++++++++++++++ > include/linux/pci-ecam.h | 1 + > 5 files changed, 113 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@amazon.com> > +L: linux-pci@vger.kernel.org > +S: Maintained > +F: drivers/pci/controller/dwc/pcie-al.c I do not think we need a maintainer file for that see below, and actually this quirk should be handled by DWC maintainers since it is a DWC quirk, not a platform one. > + > PCIE DRIVER FOR AMLOGIC MESON > M: Yue Wang <yue.wang@Amlogic.com> > L: linux-pci@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..65a9776c12be > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-al.c > @@ -0,0 +1,93 @@ > +// 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@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. > + */ > + 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; > +} This code is basically identical to (apart from the string matching the DBI resource) drivers/pci/controller/pcie-hisi.c because, as you said, that's a DW quirk that is really not platform specific AFAICS. Not that I am really fond of the idea but in practice we can create a quirk that applies to all host controllers DW based, in case they want to boot with ACPI, this patch is basically code duplication. Thanks, Lorenzo > + > +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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-26 12:17 ` Lorenzo Pieralisi @ 2019-03-26 13:24 ` David Woodhouse 2019-03-26 15:58 ` Lorenzo Pieralisi 2019-03-27 9:43 ` David Woodhouse 1 sibling, 1 reply; 21+ messages in thread From: David Woodhouse @ 2019-03-26 13:24 UTC (permalink / raw) To: Lorenzo Pieralisi, Jonathan Chocron Cc: linux-pci, bhelgaas, linux-kernel, vaerov, benh, alisaidi, zeev, ronenk, barakw, Gustavo Pimentel, Zhou Wang [-- Attachment #1: Type: text/plain, Size: 2969 bytes --] On Tue, 2019-03-26 at 12:17 +0000, Lorenzo Pieralisi wrote: > [+Zhou, Gustavo] > > On Tue, Mar 26, 2019 at 12:00:55PM +0200, Jonathan Chocron wrote: > > Adding support for Amazon's Annapurna Labs PCIe driver. > > 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 an AMZN0001 device. > > > > 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. > > 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 an AMZN0001 device. > > - Adding a new entry in the mcfg quirk array > > > > Co-developed-by: Vladimir Aerov <vaerov@amazon.com> > > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > > Signed-off-by: Vladimir Aerov <vaerov@amazon.com> > > Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > > Review tags should be given on public mailing lists for public > review and I have not seen them (they were already there in v1) so > you should drop them. We did that internally. You really don't want me telling engineers to post to the list *first* without running things by me to get the basics right. Not to start with, at least. Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > > Changes from v1: > > - Fix commit message comments (incl. using AMZN0001 > > instead of PNP0C02) > > - Use the usual multi-line comment style > > > > MAINTAINERS | 6 +++ > > drivers/acpi/pci_mcfg.c | 12 +++++ > > drivers/pci/controller/dwc/Makefile | 1 + > > drivers/pci/controller/dwc/pcie-al.c | 93 ++++++++++++++++++++++++++++++++++++ > > include/linux/pci-ecam.h | 1 + > > 5 files changed, 113 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@amazon.com> > > +L: linux-pci@vger.kernel.org > > +S: Maintained > > +F: drivers/pci/controller/dwc/pcie-al.c > > I do not think we need a maintainer file for that see below, and > actually this quirk should be handled by DWC maintainers since it is a > DWC quirk, not a platform one. Many of the others already have this, it seems. It's also fine to drop it, and include it when we add the rest of the Alpine SOC support and a MAINTAINERS entry for that. [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5174 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-26 13:24 ` David Woodhouse @ 2019-03-26 15:58 ` Lorenzo Pieralisi 2019-03-27 9:52 ` David Woodhouse 0 siblings, 1 reply; 21+ messages in thread From: Lorenzo Pieralisi @ 2019-03-26 15:58 UTC (permalink / raw) To: David Woodhouse Cc: Jonathan Chocron, linux-pci, bhelgaas, linux-kernel, vaerov, benh, alisaidi, zeev, ronenk, barakw, Gustavo Pimentel, Zhou Wang On Tue, Mar 26, 2019 at 01:24:41PM +0000, David Woodhouse wrote: > On Tue, 2019-03-26 at 12:17 +0000, Lorenzo Pieralisi wrote: > > [+Zhou, Gustavo] > > > > On Tue, Mar 26, 2019 at 12:00:55PM +0200, Jonathan Chocron wrote: > > > Adding support for Amazon's Annapurna Labs PCIe driver. > > > 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 an AMZN0001 device. > > > > > > 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. > > > 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 an AMZN0001 device. > > > - Adding a new entry in the mcfg quirk array > > > > > > Co-developed-by: Vladimir Aerov <vaerov@amazon.com> > > > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > > > Signed-off-by: Vladimir Aerov <vaerov@amazon.com> > > > Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > > > > Review tags should be given on public mailing lists for public > > review and I have not seen them (they were already there in v1) so > > you should drop them. > > We did that internally. You really don't want me telling engineers to > post to the list *first* without running things by me to get the basics > right. Not to start with, at least. Hi David, I am obviously in favour of internal review and I do not question it was carried out internally, I just kindly ask developers to drop review tags given internally when going to public mailing lists - I understand it is churn for you but I prefer them to be given explicitly. Thanks ! Lorenzo > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > > > > > Changes from v1: > > > - Fix commit message comments (incl. using AMZN0001 > > > instead of PNP0C02) > > > - Use the usual multi-line comment style > > > > > > MAINTAINERS | 6 +++ > > > drivers/acpi/pci_mcfg.c | 12 +++++ > > > drivers/pci/controller/dwc/Makefile | 1 + > > > drivers/pci/controller/dwc/pcie-al.c | 93 ++++++++++++++++++++++++++++++++++++ > > > include/linux/pci-ecam.h | 1 + > > > 5 files changed, 113 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@amazon.com> > > > +L: linux-pci@vger.kernel.org > > > +S: Maintained > > > +F: drivers/pci/controller/dwc/pcie-al.c > > > > I do not think we need a maintainer file for that see below, and > > actually this quirk should be handled by DWC maintainers since it is a > > DWC quirk, not a platform one. > > Many of the others already have this, it seems. > > It's also fine to drop it, and include it when we add the rest of the > Alpine SOC support and a MAINTAINERS entry for that. > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-26 15:58 ` Lorenzo Pieralisi @ 2019-03-27 9:52 ` David Woodhouse 2019-03-27 11:20 ` Lorenzo Pieralisi 0 siblings, 1 reply; 21+ messages in thread From: David Woodhouse @ 2019-03-27 9:52 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: Jonathan Chocron, linux-pci, bhelgaas, linux-kernel, vaerov, benh, alisaidi, zeev, ronenk, barakw, Gustavo Pimentel, Zhou Wang [-- Attachment #1: Type: text/plain, Size: 1244 bytes --] On Tue, 2019-03-26 at 15:58 +0000, Lorenzo Pieralisi wrote: > > We did that internally. You really don't want me telling engineers to > > post to the list *first* without running things by me to get the basics > > right. Not to start with, at least. > > Hi David, > > I am obviously in favour of internal review and I do not question it was > carried out internally, I just kindly ask developers to drop review tags > given internally when going to public mailing lists - I understand it is > churn for you but I prefer them to be given explicitly. Sure, I've provided mine in public now. I will attempt to remember your preference, although I'm not sure I think it's necessary. What's the failure mode we're protecting against here? That my engineers are lying and have *faked* my reviewed-by tag? Don't you think I'd *eat* them if I ever found that happening? What's next? That you only accept such tags in signed email, so that the dishonest engineer in question can't *fake* an email from me to the list? They know I'm afflicted by Exchange so they can always send that fake message with a message-id matching another message they know is already in my inbox, so Exchange will helpfully discard theirs. :) [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5174 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-27 9:52 ` David Woodhouse @ 2019-03-27 11:20 ` Lorenzo Pieralisi 2019-03-27 11:40 ` David Woodhouse 0 siblings, 1 reply; 21+ messages in thread From: Lorenzo Pieralisi @ 2019-03-27 11:20 UTC (permalink / raw) To: David Woodhouse Cc: Jonathan Chocron, linux-pci, bhelgaas, linux-kernel, vaerov, benh, alisaidi, zeev, ronenk, barakw, Gustavo Pimentel, Zhou Wang On Wed, Mar 27, 2019 at 09:52:15AM +0000, David Woodhouse wrote: > On Tue, 2019-03-26 at 15:58 +0000, Lorenzo Pieralisi wrote: > > > We did that internally. You really don't want me telling engineers to > > > post to the list *first* without running things by me to get the basics > > > right. Not to start with, at least. > > > > Hi David, > > > > I am obviously in favour of internal review and I do not question it was > > carried out internally, I just kindly ask developers to drop review tags > > given internally when going to public mailing lists - I understand it is > > churn for you but I prefer them to be given explicitly. > > Sure, I've provided mine in public now. > > I will attempt to remember your preference, although I'm not sure I > think it's necessary. > > What's the failure mode we're protecting against here? That my > engineers are lying and have *faked* my reviewed-by tag? > > Don't you think I'd *eat* them if I ever found that happening? As I wrote above, I did not question the internal review process at all, we do internal review at ARM too in preparation for posting publicly but I think the patches review should take place on public mailing lists and tags should be given accordingly, that's it. You may see it as churn, fair enough, it is not a big deal either. > What's next? That you only accept such tags in signed email, so that > the dishonest engineer in question can't *fake* an email from me to the > list? They know I'm afflicted by Exchange so they can always send that > fake message with a message-id matching another message they know is > already in my inbox, so Exchange will helpfully discard theirs. :) There is nothing next :) - I just would like to see patches discussions and reviews taking place on linux-pci@vger.kernel.org for PCI patches, I do not think I am asking too much. Thanks, Lorenzo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-27 11:20 ` Lorenzo Pieralisi @ 2019-03-27 11:40 ` David Woodhouse 0 siblings, 0 replies; 21+ messages in thread From: David Woodhouse @ 2019-03-27 11:40 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: Jonathan Chocron, linux-pci, bhelgaas, linux-kernel, vaerov, benh, alisaidi, zeev, ronenk, barakw, Gustavo Pimentel, Zhou Wang [-- Attachment #1: Type: text/plain, Size: 3553 bytes --] On Wed, 2019-03-27 at 11:20 +0000, Lorenzo Pieralisi wrote: > On Wed, Mar 27, 2019 at 09:52:15AM +0000, David Woodhouse wrote: > > On Tue, 2019-03-26 at 15:58 +0000, Lorenzo Pieralisi wrote: > > > > We did that internally. You really don't want me telling engineers to > > > > post to the list *first* without running things by me to get the basics > > > > right. Not to start with, at least. > > > > > > Hi David, > > > > > > I am obviously in favour of internal review and I do not question it was > > > carried out internally, I just kindly ask developers to drop review tags > > > given internally when going to public mailing lists - I understand it is > > > churn for you but I prefer them to be given explicitly. > > > > Sure, I've provided mine in public now. > > > > I will attempt to remember your preference, although I'm not sure I > > think it's necessary. > > > > What's the failure mode we're protecting against here? That my > > engineers are lying and have *faked* my reviewed-by tag? > > > > Don't you think I'd *eat* them if I ever found that happening? > > As I wrote above, I did not question the internal review process at all, > we do internal review at ARM too in preparation for posting publicly but > I think the patches review should take place on public mailing lists and > tags should be given accordingly, that's it. > > You may see it as churn, fair enough, it is not a big deal either. Understood. As I said, we will endeavour to comply. Note that we may occasionally forget. Our internal review tooling automatically *adds* those tags, when internal review happens. And we have reduced the "legal" approval process to the point where myself or a handful of others only need to give the nod in that same internal technical review tool, for a patch to be sent upstream. This means that engineers will have to remember to actively *remove* those tags when they're sending PCI patches. We'll try to remember :) > > What's next? That you only accept such tags in signed email, so that > > the dishonest engineer in question can't *fake* an email from me to the > > list? They know I'm afflicted by Exchange so they can always send that > > fake message with a message-id matching another message they know is > > already in my inbox, so Exchange will helpfully discard theirs. :) > > There is nothing next :) - I just would like to see patches discussions > and reviews taking place on linux-pci@vger.kernel.org for PCI patches, > I do not think I am asking too much. Not asking too much at all. We will do as you request. I do think it's pointless — you really *don't* want to see the first rounds of review that happened internally, and once the patch makes it to the list, it doesn't make a lot of difference whether my Reviewed- By: tag is already there or not; you don't get to see the previous iterations of the patch or any of those prior review comments anyway. If *all* there is in my public response is that Reviewed-by: tag, there is literally no benefit to that at all, except that you know the engineer isn't lying. None at all. But it's fine. If it really annoys me, I can set up an autoresponder which sends an empty mail with a 'Reviewed-by:' tag, and my engineers can include a header in their patch submission which triggers that. We can even automate the tooling, to turn the normal Reviewed-by: tag into that header which triggers the response. We will comply with your request, even if we don't understand it :) [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5174 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-26 12:17 ` Lorenzo Pieralisi 2019-03-26 13:24 ` David Woodhouse @ 2019-03-27 9:43 ` David Woodhouse 2019-03-27 11:39 ` Lorenzo Pieralisi 1 sibling, 1 reply; 21+ messages in thread From: David Woodhouse @ 2019-03-27 9:43 UTC (permalink / raw) To: Lorenzo Pieralisi, Jonathan Chocron Cc: linux-pci, bhelgaas, linux-kernel, vaerov, benh, alisaidi, zeev, ronenk, barakw, Gustavo Pimentel, Zhou Wang [-- Attachment #1: Type: text/plain, Size: 2312 bytes --] On Tue, 2019-03-26 at 12:17 +0000, Lorenzo Pieralisi wrote: > This code is basically identical to (apart from the string matching > the DBI resource) > > drivers/pci/controller/pcie-hisi.c > > because, as you said, that's a DW quirk that is really not > platform specific AFAICS. > > Not that I am really fond of the idea but in practice we can > create a quirk that applies to all host controllers DW based, > in case they want to boot with ACPI, this patch is basically > code duplication. Having chatted to Jonny in a little more detail... this isn't quite duplicate code. On the Annapurna implementation we have fixed the alignment constraints so we can just use pci_generic_config_read() directly instead of forcing alignment. We only need to filter out the "ghost" devices on bus zero. There might eventually be scope for some form of consolidation, but it doesn't really seem clear at this point that it would be worth it. There are three separate quirks needed for different versions of the DW controller. One is that the config space of the controller itself shows up multiple times in all slots of bus zero. The second is that bus zero cannot be accessed through ECAM and must be accessed through a separate MMIO region. The third is the requirement for 32-bit alignment of the host controller's config space (through the special MMIO region). Some vendors have managed to work around some of these issues, for example Annapurna fixing the alignment one. It looks like the three DT matches in pci-host-generic.c which use pci_dw_ecam_bus_ops are assuming the hardware suffers only issue #1 from the list above, and not the other two. The Annapurna hardware gives us a new combination, and that's why it isn't quite a duplicate. Again, there may be scope for consolidation in the future but it's not clear what it should look like. Especially as when exposed through DT, both the hisi and al versions seem to do things quite differently and need their own specific code there anyway. There will be a DT variant of the AL driver coming in the near future, but when it's exposed via ACPI in the "as close to ECAM compliant as we could make it in this iteration of the hardware" mode, it's relatively simple so we did that patch first. [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5210 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-27 9:43 ` David Woodhouse @ 2019-03-27 11:39 ` Lorenzo Pieralisi 2019-03-27 12:01 ` David Woodhouse 0 siblings, 1 reply; 21+ messages in thread From: Lorenzo Pieralisi @ 2019-03-27 11:39 UTC (permalink / raw) To: David Woodhouse Cc: Jonathan Chocron, linux-pci, bhelgaas, linux-kernel, vaerov, benh, alisaidi, zeev, ronenk, barakw, Gustavo Pimentel, Zhou Wang On Wed, Mar 27, 2019 at 09:43:26AM +0000, David Woodhouse wrote: > On Tue, 2019-03-26 at 12:17 +0000, Lorenzo Pieralisi wrote: > > This code is basically identical to (apart from the string matching > > the DBI resource) > > > > drivers/pci/controller/pcie-hisi.c > > > > because, as you said, that's a DW quirk that is really not > > platform specific AFAICS. > > > > Not that I am really fond of the idea but in practice we can > > create a quirk that applies to all host controllers DW based, > > in case they want to boot with ACPI, this patch is basically > > code duplication. > > Having chatted to Jonny in a little more detail... this isn't quite > duplicate code. On the Annapurna implementation we have fixed the > alignment constraints so we can just use pci_generic_config_read() > directly instead of forcing alignment. We only need to filter out the > "ghost" devices on bus zero. > > There might eventually be scope for some form of consolidation, but it > doesn't really seem clear at this point that it would be worth it. The pci_ecam_ops.init function can be certainly reused but I agree duplicating it is not a big deal either - I just noticed it and asked. we can merge code as-is and think about writing a common init function if/when needed. > There are three separate quirks needed for different versions of the DW > controller. > > One is that the config space of the controller itself shows up multiple > times in all slots of bus zero. > > The second is that bus zero cannot be accessed through ECAM and must be > accessed through a separate MMIO region. > > The third is the requirement for 32-bit alignment of the host > controller's config space (through the special MMIO region). I missed this one - thanks for clarifying. > Some vendors have managed to work around some of these issues, for > example Annapurna fixing the alignment one. It looks like the three DT > matches in pci-host-generic.c which use pci_dw_ecam_bus_ops are > assuming the hardware suffers only issue #1 from the list above, and > not the other two. > > The Annapurna hardware gives us a new combination, and that's why it > isn't quite a duplicate. Again, there may be scope for consolidation in > the future but it's not clear what it should look like. Especially as > when exposed through DT, both the hisi and al versions seem to do > things quite differently and need their own specific code there anyway. DT PCI host bridge bootstrap is a different story and on that consolidation is all but impossible. I just want to keep MCFG ECAM quirks as simple as possible, code as it stands is horrible enough and I wish I could remove the mechanism in the future rather than adding more to it, hopefully we are getting there. > There will be a DT variant of the AL driver coming in the near future, > but when it's exposed via ACPI in the "as close to ECAM compliant as we > could make it in this iteration of the hardware" mode, it's relatively > simple so we did that patch first. That's fine, no problems with that. Thanks, Lorenzo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-27 11:39 ` Lorenzo Pieralisi @ 2019-03-27 12:01 ` David Woodhouse 0 siblings, 0 replies; 21+ messages in thread From: David Woodhouse @ 2019-03-27 12:01 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: Jonathan Chocron, linux-pci, bhelgaas, linux-kernel, vaerov, benh, alisaidi, zeev, ronenk, barakw, Gustavo Pimentel, Zhou Wang [-- Attachment #1: Type: text/plain, Size: 695 bytes --] On Wed, 2019-03-27 at 11:39 +0000, Lorenzo Pieralisi wrote: > I just want to keep MCFG ECAM quirks as simple as possible, code as it > stands is horrible enough and I wish I could remove the mechanism in > the future rather than adding more to it, hopefully we are getting > there. Obviously I cannot talk about future hardware or even admit that there is such as thing as "future hardware". But all the issues with the DW controller *have* already been worked around by different implementations, in different combinations. And I will be very sad if *we* somehow manage to make a new board which needs a new and different combination of quirks. Actively, percussively, sad. [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5174 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-26 10:00 ` [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver Jonathan Chocron 2019-03-26 12:17 ` Lorenzo Pieralisi @ 2019-03-26 12:55 ` Bjorn Helgaas 2019-03-28 10:55 ` Jonathan Chocron 2019-03-28 11:57 ` [PATCH v3] " Jonathan Chocron 1 sibling, 2 replies; 21+ messages in thread From: Bjorn Helgaas @ 2019-03-26 12:55 UTC (permalink / raw) To: Jonathan Chocron Cc: linux-pci, lorenzo.pieralisi, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw Nits, probably Lorenzo will fix them up unless he sees more substantive things. On Tue, Mar 26, 2019 at 12:00:55PM +0200, Jonathan Chocron wrote: > Adding support for Amazon's Annapurna Labs PCIe driver. Ideally, use "imperative mood", i.e., write it as a command: Add support for Amazon's Annapurna Labs PCIe driver. > 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 an AMZN0001 device. > > 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. > All subordinate buses do support ECAM access. I didn't communicate my point very clearly. The above four lines should either be (1) a single paragraph, wrapped to fill the entire width, or (2) two paragraphs, with a blank line before "All subordinate buses ..." The fact that "... by the driver" ends in the middle of the line suggests that it's the end of the paragraph, but the fact that there's no blank line following suggests that it's not. So it creates an unnecessary hiccup for the reader. > Implementing specific PCI config access functions involves: > - Adding an init function to obtain the Root Port's base address > from an AMZN0001 device. > - Adding a new entry in the mcfg quirk array s/mcfg/MCFG/ since "MCFG" is an ACPI table ID, not a word. Bjorn ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-26 12:55 ` Bjorn Helgaas @ 2019-03-28 10:55 ` Jonathan Chocron 2019-03-28 11:57 ` [PATCH v3] " Jonathan Chocron 1 sibling, 0 replies; 21+ messages in thread From: Jonathan Chocron @ 2019-03-28 10:55 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, lorenzo.pieralisi, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw On 3/26/19 14:55, Bjorn Helgaas wrote: > Nits, probably Lorenzo will fix them up unless he sees more substantive > things. > > On Tue, Mar 26, 2019 at 12:00:55PM +0200, Jonathan Chocron wrote: >> Adding support for Amazon's Annapurna Labs PCIe driver. > Ideally, use "imperative mood", i.e., write it as a command: > > Add support for Amazon's Annapurna Labs PCIe driver. 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 an AMZN0001 device. >> >> 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. >> All subordinate buses do support ECAM access. > I didn't communicate my point very clearly. The above four lines > should either be (1) a single paragraph, wrapped to fill the entire > width, or (2) two paragraphs, with a blank line before "All > subordinate buses ..." > > The fact that "... by the driver" ends in the middle of the line > suggests that it's the end of the paragraph, but the fact that there's > no blank line following suggests that it's not. So it creates an > unnecessary hiccup for the reader. Added a blank line. >> Implementing specific PCI config access functions involves: >> - Adding an init function to obtain the Root Port's base address >> from an AMZN0001 device. >> - Adding a new entry in the mcfg quirk array > s/mcfg/MCFG/ since "MCFG" is an ACPI table ID, not a word. Done. > Bjorn ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-26 12:55 ` Bjorn Helgaas 2019-03-28 10:55 ` Jonathan Chocron @ 2019-03-28 11:57 ` Jonathan Chocron 2019-04-08 22:57 ` Benjamin Herrenschmidt ` (2 more replies) 1 sibling, 3 replies; 21+ messages in thread From: Jonathan Chocron @ 2019-03-28 11:57 UTC (permalink / raw) To: linux-pci Cc: lorenzo.pieralisi, bhelgaas, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw, jonnyc, talel, hanochu, hhhawa Add support for Amazon's Annapurna Labs PCIe driver. 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 an AMZN0001 device. 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. 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 an AMZN0001 device. - Adding a new entry in the MCFG quirk array Co-developed-by: Vladimir Aerov <vaerov@amazon.com> Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> Signed-off-by: Vladimir Aerov <vaerov@amazon.com> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> --- --v2: - Fix commit message comments (incl. using AMZN0001 instead of PNP0C02) - Use the usual multi-line comment style --v3: - Fix additional commit message comments MAINTAINERS | 6 +++ drivers/acpi/pci_mcfg.c | 12 +++++ drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-al.c | 93 ++++++++++++++++++++++++++++++++++++ include/linux/pci-ecam.h | 1 + 5 files changed, 113 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@amazon.com> +L: linux-pci@vger.kernel.org +S: Maintained +F: drivers/pci/controller/dwc/pcie-al.c + PCIE DRIVER FOR AMLOGIC MESON M: Yue Wang <yue.wang@Amlogic.com> L: linux-pci@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..65a9776c12be --- /dev/null +++ b/drivers/pci/controller/dwc/pcie-al.c @@ -0,0 +1,93 @@ +// 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@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. + */ + 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-28 11:57 ` [PATCH v3] " Jonathan Chocron @ 2019-04-08 22:57 ` Benjamin Herrenschmidt 2019-04-16 13:13 ` David Woodhouse 2019-04-16 14:01 ` Lorenzo Pieralisi 2019-04-25 14:08 ` Bjorn Helgaas 2 siblings, 1 reply; 21+ messages in thread From: Benjamin Herrenschmidt @ 2019-04-08 22:57 UTC (permalink / raw) To: Jonathan Chocron, linux-pci Cc: lorenzo.pieralisi, bhelgaas, linux-kernel, vaerov, dwmw, alisaidi, zeev, ronenk, barakw, talel, hanochu, hhhawa On Thu, 2019-03-28 at 13:57 +0200, Jonathan Chocron wrote: > Add support for Amazon's Annapurna Labs PCIe driver. 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 an AMZN0001 device. > > 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. > > 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 > an AMZN0001 device. > - Adding a new entry in the MCFG quirk array > > Co-developed-by: Vladimir Aerov <vaerov@amazon.com> > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > Signed-off-by: Vladimir Aerov <vaerov@amazon.com> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> Late to the party, sorry :-) That kernel.crashing.org email is on its last legs... Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > > --v2: > - Fix commit message comments (incl. using AMZN0001 instead of > PNP0C02) > - Use the usual multi-line comment style > > --v3: > - Fix additional commit message comments > > MAINTAINERS | 6 +++ > drivers/acpi/pci_mcfg.c | 12 +++++ > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-al.c | 93 > ++++++++++++++++++++++++++++++++++++ > include/linux/pci-ecam.h | 1 + > 5 files changed, 113 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@amazon.com> > +L: linux-pci@vger.kernel.org > +S: Maintained > +F: drivers/pci/controller/dwc/pcie-al.c > + > PCIE DRIVER FOR AMLOGIC MESON > M: Yue Wang <yue.wang@Amlogic.com> > L: linux-pci@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..65a9776c12be > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-al.c > @@ -0,0 +1,93 @@ > +// 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@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. > + */ > + 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-04-08 22:57 ` Benjamin Herrenschmidt @ 2019-04-16 13:13 ` David Woodhouse 0 siblings, 0 replies; 21+ messages in thread From: David Woodhouse @ 2019-04-16 13:13 UTC (permalink / raw) To: Benjamin Herrenschmidt, Jonathan Chocron, linux-pci Cc: lorenzo.pieralisi, bhelgaas, linux-kernel, vaerov, alisaidi, zeev, ronenk, barakw, talel, hanochu, hhhawa [-- Attachment #1: Type: text/plain, Size: 1357 bytes --] On Tue, 2019-04-09 at 08:57 +1000, Benjamin Herrenschmidt wrote: > On Thu, 2019-03-28 at 13:57 +0200, Jonathan Chocron wrote: > > Add support for Amazon's Annapurna Labs PCIe driver. 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 an AMZN0001 device. > > > > 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. > > > > 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 > > an AMZN0001 device. > > - Adding a new entry in the MCFG quirk array > > > > Co-developed-by: Vladimir Aerov <vaerov@amazon.com> > > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > > Signed-off-by: Vladimir Aerov <vaerov@amazon.com> > > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > > Late to the party, sorry :-) That kernel.crashing.org email is on its > last legs... > > Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Lorenzo, is there anything else you need from Jonny before this can be applied? Thanks. [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5174 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-28 11:57 ` [PATCH v3] " Jonathan Chocron 2019-04-08 22:57 ` Benjamin Herrenschmidt @ 2019-04-16 14:01 ` Lorenzo Pieralisi 2019-04-25 14:08 ` Bjorn Helgaas 2 siblings, 0 replies; 21+ messages in thread From: Lorenzo Pieralisi @ 2019-04-16 14:01 UTC (permalink / raw) To: Jonathan Chocron, bhelgaas Cc: linux-pci, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw, talel, hanochu, hhhawa On Thu, Mar 28, 2019 at 01:57:56PM +0200, Jonathan Chocron wrote: > Add support for Amazon's Annapurna Labs PCIe driver. 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 an AMZN0001 device. > > 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. > > 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 > an AMZN0001 device. > - Adding a new entry in the MCFG quirk array > > Co-developed-by: Vladimir Aerov <vaerov@amazon.com> > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > Signed-off-by: Vladimir Aerov <vaerov@amazon.com> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > --- > > --v2: > - Fix commit message comments (incl. using AMZN0001 instead of > PNP0C02) > - Use the usual multi-line comment style > > --v3: > - Fix additional commit message comments > > MAINTAINERS | 6 +++ > drivers/acpi/pci_mcfg.c | 12 +++++ > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-al.c | 93 ++++++++++++++++++++++++++++++++++++ > include/linux/pci-ecam.h | 1 + > 5 files changed, 113 insertions(+) > create mode 100644 drivers/pci/controller/dwc/pcie-al.c I think Bjorn can take this if he is OK with it, it is not really necessary to queue it via the controller/dwc tree so: Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > 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@amazon.com> > +L: linux-pci@vger.kernel.org > +S: Maintained > +F: drivers/pci/controller/dwc/pcie-al.c > + > PCIE DRIVER FOR AMLOGIC MESON > M: Yue Wang <yue.wang@Amlogic.com> > L: linux-pci@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..65a9776c12be > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-al.c > @@ -0,0 +1,93 @@ > +// 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@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. > + */ > + 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver 2019-03-28 11:57 ` [PATCH v3] " Jonathan Chocron 2019-04-08 22:57 ` Benjamin Herrenschmidt 2019-04-16 14:01 ` Lorenzo Pieralisi @ 2019-04-25 14:08 ` Bjorn Helgaas 2 siblings, 0 replies; 21+ messages in thread From: Bjorn Helgaas @ 2019-04-25 14:08 UTC (permalink / raw) To: Jonathan Chocron Cc: linux-pci, lorenzo.pieralisi, linux-kernel, vaerov, dwmw, benh, alisaidi, zeev, ronenk, barakw, talel, hanochu, hhhawa On Thu, Mar 28, 2019 at 01:57:56PM +0200, Jonathan Chocron wrote: > Add support for Amazon's Annapurna Labs PCIe driver. 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 an AMZN0001 device. > > 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. > > 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 > an AMZN0001 device. > - Adding a new entry in the MCFG quirk array > > Co-developed-by: Vladimir Aerov <vaerov@amazon.com> > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > Signed-off-by: Vladimir Aerov <vaerov@amazon.com> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> Applied with Ben's reviewed-by and Lorenzo's ack to pci/host/al for v5.2, thanks! > --- > > --v2: > - Fix commit message comments (incl. using AMZN0001 instead of > PNP0C02) > - Use the usual multi-line comment style > > --v3: > - Fix additional commit message comments > > MAINTAINERS | 6 +++ > drivers/acpi/pci_mcfg.c | 12 +++++ > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-al.c | 93 ++++++++++++++++++++++++++++++++++++ > include/linux/pci-ecam.h | 1 + > 5 files changed, 113 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@amazon.com> > +L: linux-pci@vger.kernel.org > +S: Maintained > +F: drivers/pci/controller/dwc/pcie-al.c > + > PCIE DRIVER FOR AMLOGIC MESON > M: Yue Wang <yue.wang@Amlogic.com> > L: linux-pci@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..65a9776c12be > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-al.c > @@ -0,0 +1,93 @@ > +// 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@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. > + */ > + 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2019-04-25 14:08 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-25 11:07 [PATCH] PCI: al: add pcie-al.c jonnyc 2019-03-25 12:58 ` Bjorn Helgaas 2019-03-25 15:56 ` Jonathan Chocron 2019-03-25 17:36 ` Bjorn Helgaas 2019-03-26 10:00 ` [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver Jonathan Chocron 2019-03-26 12:17 ` Lorenzo Pieralisi 2019-03-26 13:24 ` David Woodhouse 2019-03-26 15:58 ` Lorenzo Pieralisi 2019-03-27 9:52 ` David Woodhouse 2019-03-27 11:20 ` Lorenzo Pieralisi 2019-03-27 11:40 ` David Woodhouse 2019-03-27 9:43 ` David Woodhouse 2019-03-27 11:39 ` Lorenzo Pieralisi 2019-03-27 12:01 ` David Woodhouse 2019-03-26 12:55 ` Bjorn Helgaas 2019-03-28 10:55 ` Jonathan Chocron 2019-03-28 11:57 ` [PATCH v3] " Jonathan Chocron 2019-04-08 22:57 ` Benjamin Herrenschmidt 2019-04-16 13:13 ` David Woodhouse 2019-04-16 14:01 ` Lorenzo Pieralisi 2019-04-25 14:08 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).