| Message ID | 20210610064134.336781-1-jonathanh@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [V4] PCI: tegra: Fix building Tegra194 PCIe driver | expand |
On Thu, Jun 10, 2021 at 07:41:34AM +0100, Jon Hunter wrote: > Commit 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM > errata") caused a few build regressions for the Tegra194 PCIe driver > which are: > > 1. The Tegra194 PCIe driver can no longer be built as a module. This > was caused by removing the Makefile entry to build the pcie-tegra.c > based upon the CONFIG_PCIE_TEGRA194 option. Therefore, restore this > so that we can build the driver as a module. > 2. 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM > errata") added "#ifdef CONFIG_PCIE_TEGRA194" around the native > driver. But if we set CONFIG_PCIE_TEGRA194=m to build the driver as a > module, autoconf.h contains "#define CONFIG_PCIE_TEGRA194_MODULE 1" > (not "#define CONFIG_PCIE_TEGRA194 1"), so the #ifdef excludes the I think the correct way to write conditionals that are true on =y and =m is to use: #ifdef IS_ENABLED(CONFIG_PCIE_TEGRA194) but that doesn't fix the issue of not getting the quirks included when CONFIG_PCIE_TEGRA194=m. > driver. Given that the ACPI quirk code for Tegra194 is completely > independent of the native Tegra194 PCIe driver, move this code into > its own file so that it can be built independently and we can remove > the "#ifdef CONFIG_PCIE_TEGRA194" in the native driver. Note that > given the native Tegra194 PCIe driver is only used with device-tree, > this will not cause any conflicts. > 3. The below build warnings to be seen with particular kernel > configurations. Fix these by moving these structure definitions to > within the necessary guards. > > drivers/pci/controller/dwc/pcie-tegra194.c:259:18: warning: > ‘event_cntr_data_offset’ defined but not used [-Wunused-const-variable=] > drivers/pci/controller/dwc/pcie-tegra194.c:250:18: warning: > ‘event_cntr_ctrl_offset’ defined but not used [-Wunused-const-variable=] > drivers/pci/controller/dwc/pcie-tegra194.c:243:27: warning: > ‘pcie_gen_freq’ defined but not used [-Wunused-const-variable=] > > Fixes: 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM errata") > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > Changes since V3: > - Moved ACPI quirk code into separate source file > > Changes since V2: > - Update the commit message per Bjorn's feedback > - Moved the structure definitions within the necessary guards as opposed > to wrapping the existing defintions with the appropriate guards. > > Changes since V1: > - Added fixes tag > - Fixed 'defined but not used' compiler warnings > > drivers/pci/controller/dwc/Makefile | 3 +- > .../pci/controller/dwc/pcie-tegra194-acpi.c | 108 ++++++++++++++ > drivers/pci/controller/dwc/pcie-tegra194.c | 138 +++--------------- > 3 files changed, 128 insertions(+), 121 deletions(-) > create mode 100644 drivers/pci/controller/dwc/pcie-tegra194-acpi.c We've had a similar problem with Tegra210 EMC and the EMC frequency table parsing code. The EMC frequency table parsing code needs to be always built-in because it is stashed into a linker section. However we still want the driver to be able to be a loadable module. The only way I know how to achieve that is by splitting up the sources files like you do here, so I this looks like a correct fix: Reviewed-by: Thierry Reding <treding@nvidia.com>
Hi Bjorn, Lorenzo, On 10/06/2021 10:07, Thierry Reding wrote: > On Thu, Jun 10, 2021 at 07:41:34AM +0100, Jon Hunter wrote: >> Commit 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM >> errata") caused a few build regressions for the Tegra194 PCIe driver >> which are: >> >> 1. The Tegra194 PCIe driver can no longer be built as a module. This >> was caused by removing the Makefile entry to build the pcie-tegra.c >> based upon the CONFIG_PCIE_TEGRA194 option. Therefore, restore this >> so that we can build the driver as a module. >> 2. 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM >> errata") added "#ifdef CONFIG_PCIE_TEGRA194" around the native >> driver. But if we set CONFIG_PCIE_TEGRA194=m to build the driver as a >> module, autoconf.h contains "#define CONFIG_PCIE_TEGRA194_MODULE 1" >> (not "#define CONFIG_PCIE_TEGRA194 1"), so the #ifdef excludes the > > I think the correct way to write conditionals that are true on =y and =m > is to use: > > #ifdef IS_ENABLED(CONFIG_PCIE_TEGRA194) > > but that doesn't fix the issue of not getting the quirks included when > CONFIG_PCIE_TEGRA194=m. > >> driver. Given that the ACPI quirk code for Tegra194 is completely >> independent of the native Tegra194 PCIe driver, move this code into >> its own file so that it can be built independently and we can remove >> the "#ifdef CONFIG_PCIE_TEGRA194" in the native driver. Note that >> given the native Tegra194 PCIe driver is only used with device-tree, >> this will not cause any conflicts. >> 3. The below build warnings to be seen with particular kernel >> configurations. Fix these by moving these structure definitions to >> within the necessary guards. >> >> drivers/pci/controller/dwc/pcie-tegra194.c:259:18: warning: >> ‘event_cntr_data_offset’ defined but not used [-Wunused-const-variable=] >> drivers/pci/controller/dwc/pcie-tegra194.c:250:18: warning: >> ‘event_cntr_ctrl_offset’ defined but not used [-Wunused-const-variable=] >> drivers/pci/controller/dwc/pcie-tegra194.c:243:27: warning: >> ‘pcie_gen_freq’ defined but not used [-Wunused-const-variable=] >> >> Fixes: 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM errata") >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> --- >> Changes since V3: >> - Moved ACPI quirk code into separate source file >> >> Changes since V2: >> - Update the commit message per Bjorn's feedback >> - Moved the structure definitions within the necessary guards as opposed >> to wrapping the existing defintions with the appropriate guards. >> >> Changes since V1: >> - Added fixes tag >> - Fixed 'defined but not used' compiler warnings >> >> drivers/pci/controller/dwc/Makefile | 3 +- >> .../pci/controller/dwc/pcie-tegra194-acpi.c | 108 ++++++++++++++ >> drivers/pci/controller/dwc/pcie-tegra194.c | 138 +++--------------- >> 3 files changed, 128 insertions(+), 121 deletions(-) >> create mode 100644 drivers/pci/controller/dwc/pcie-tegra194-acpi.c > > We've had a similar problem with Tegra210 EMC and the EMC frequency > table parsing code. The EMC frequency table parsing code needs to be > always built-in because it is stashed into a linker section. However > we still want the driver to be able to be a loadable module. The only > way I know how to achieve that is by splitting up the sources files > like you do here, so I this looks like a correct fix: > > Reviewed-by: Thierry Reding <treding@nvidia.com> Just a reminder that we need to get this fixed for v5.13. Please let me know if you have any more comments on this fix. Thanks! Jon
Adding Bjorn to the To ... On 14/06/2021 21:37, Jon Hunter wrote: > Hi Bjorn, Lorenzo, > > On 10/06/2021 10:07, Thierry Reding wrote: >> On Thu, Jun 10, 2021 at 07:41:34AM +0100, Jon Hunter wrote: >>> Commit 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM >>> errata") caused a few build regressions for the Tegra194 PCIe driver >>> which are: >>> >>> 1. The Tegra194 PCIe driver can no longer be built as a module. This >>> was caused by removing the Makefile entry to build the pcie-tegra.c >>> based upon the CONFIG_PCIE_TEGRA194 option. Therefore, restore this >>> so that we can build the driver as a module. >>> 2. 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM >>> errata") added "#ifdef CONFIG_PCIE_TEGRA194" around the native >>> driver. But if we set CONFIG_PCIE_TEGRA194=m to build the driver as a >>> module, autoconf.h contains "#define CONFIG_PCIE_TEGRA194_MODULE 1" >>> (not "#define CONFIG_PCIE_TEGRA194 1"), so the #ifdef excludes the >> >> I think the correct way to write conditionals that are true on =y and =m >> is to use: >> >> #ifdef IS_ENABLED(CONFIG_PCIE_TEGRA194) >> >> but that doesn't fix the issue of not getting the quirks included when >> CONFIG_PCIE_TEGRA194=m. >> >>> driver. Given that the ACPI quirk code for Tegra194 is completely >>> independent of the native Tegra194 PCIe driver, move this code into >>> its own file so that it can be built independently and we can remove >>> the "#ifdef CONFIG_PCIE_TEGRA194" in the native driver. Note that >>> given the native Tegra194 PCIe driver is only used with device-tree, >>> this will not cause any conflicts. >>> 3. The below build warnings to be seen with particular kernel >>> configurations. Fix these by moving these structure definitions to >>> within the necessary guards. >>> >>> drivers/pci/controller/dwc/pcie-tegra194.c:259:18: warning: >>> ‘event_cntr_data_offset’ defined but not used [-Wunused-const-variable=] >>> drivers/pci/controller/dwc/pcie-tegra194.c:250:18: warning: >>> ‘event_cntr_ctrl_offset’ defined but not used [-Wunused-const-variable=] >>> drivers/pci/controller/dwc/pcie-tegra194.c:243:27: warning: >>> ‘pcie_gen_freq’ defined but not used [-Wunused-const-variable=] >>> >>> Fixes: 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM errata") >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >>> --- >>> Changes since V3: >>> - Moved ACPI quirk code into separate source file >>> >>> Changes since V2: >>> - Update the commit message per Bjorn's feedback >>> - Moved the structure definitions within the necessary guards as opposed >>> to wrapping the existing defintions with the appropriate guards. >>> >>> Changes since V1: >>> - Added fixes tag >>> - Fixed 'defined but not used' compiler warnings >>> >>> drivers/pci/controller/dwc/Makefile | 3 +- >>> .../pci/controller/dwc/pcie-tegra194-acpi.c | 108 ++++++++++++++ >>> drivers/pci/controller/dwc/pcie-tegra194.c | 138 +++--------------- >>> 3 files changed, 128 insertions(+), 121 deletions(-) >>> create mode 100644 drivers/pci/controller/dwc/pcie-tegra194-acpi.c >> >> We've had a similar problem with Tegra210 EMC and the EMC frequency >> table parsing code. The EMC frequency table parsing code needs to be >> always built-in because it is stashed into a linker section. However >> we still want the driver to be able to be a loadable module. The only >> way I know how to achieve that is by splitting up the sources files >> like you do here, so I this looks like a correct fix: >> >> Reviewed-by: Thierry Reding <treding@nvidia.com> > > > Just a reminder that we need to get this fixed for v5.13. Please let me > know if you have any more comments on this fix. > > Thanks! > Jon >
On Thu, Jun 10, 2021 at 07:41:34AM +0100, Jon Hunter wrote: > Commit 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM > errata") caused a few build regressions for the Tegra194 PCIe driver > which are: > > 1. The Tegra194 PCIe driver can no longer be built as a module. This > was caused by removing the Makefile entry to build the pcie-tegra.c > based upon the CONFIG_PCIE_TEGRA194 option. Therefore, restore this > so that we can build the driver as a module. > 2. 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM > errata") added "#ifdef CONFIG_PCIE_TEGRA194" around the native > driver. But if we set CONFIG_PCIE_TEGRA194=m to build the driver as a > module, autoconf.h contains "#define CONFIG_PCIE_TEGRA194_MODULE 1" > (not "#define CONFIG_PCIE_TEGRA194 1"), so the #ifdef excludes the > driver. Given that the ACPI quirk code for Tegra194 is completely > independent of the native Tegra194 PCIe driver, move this code into > its own file so that it can be built independently and we can remove > the "#ifdef CONFIG_PCIE_TEGRA194" in the native driver. Note that > given the native Tegra194 PCIe driver is only used with device-tree, > this will not cause any conflicts. > 3. The below build warnings to be seen with particular kernel > configurations. Fix these by moving these structure definitions to > within the necessary guards. > > drivers/pci/controller/dwc/pcie-tegra194.c:259:18: warning: > ‘event_cntr_data_offset’ defined but not used [-Wunused-const-variable=] > drivers/pci/controller/dwc/pcie-tegra194.c:250:18: warning: > ‘event_cntr_ctrl_offset’ defined but not used [-Wunused-const-variable=] > drivers/pci/controller/dwc/pcie-tegra194.c:243:27: warning: > ‘pcie_gen_freq’ defined but not used [-Wunused-const-variable=] > > Fixes: 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM errata") > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> Applied with Thierry's reviewed-by and the following subject to for-linus for v5.13, thanks! PCI: tegra194: Fix MCFG quirk build regressions > --- > Changes since V3: > - Moved ACPI quirk code into separate source file > > Changes since V2: > - Update the commit message per Bjorn's feedback > - Moved the structure definitions within the necessary guards as opposed > to wrapping the existing defintions with the appropriate guards. > > Changes since V1: > - Added fixes tag > - Fixed 'defined but not used' compiler warnings > > drivers/pci/controller/dwc/Makefile | 3 +- > .../pci/controller/dwc/pcie-tegra194-acpi.c | 108 ++++++++++++++ > drivers/pci/controller/dwc/pcie-tegra194.c | 138 +++--------------- > 3 files changed, 128 insertions(+), 121 deletions(-) > create mode 100644 drivers/pci/controller/dwc/pcie-tegra194-acpi.c > > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile > index eca805c1a023..9e6ce0dc2f53 100644 > --- a/drivers/pci/controller/dwc/Makefile > +++ b/drivers/pci/controller/dwc/Makefile > @@ -18,6 +18,7 @@ obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o > obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o > obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o > obj-$(CONFIG_PCI_MESON) += pci-meson.o > +obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o > obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o > obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o > > @@ -38,6 +39,6 @@ ifdef CONFIG_ACPI > ifdef CONFIG_PCI_QUIRKS > obj-$(CONFIG_ARM64) += pcie-al.o > obj-$(CONFIG_ARM64) += pcie-hisi.o > -obj-$(CONFIG_ARM64) += pcie-tegra194.o > +obj-$(CONFIG_ARM64) += pcie-tegra194-acpi.o > endif > endif > diff --git a/drivers/pci/controller/dwc/pcie-tegra194-acpi.c b/drivers/pci/controller/dwc/pcie-tegra194-acpi.c > new file mode 100644 > index 000000000000..c2de6ed4d86f > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-tegra194-acpi.c > @@ -0,0 +1,108 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * ACPI quirks for Tegra194 PCIe host controller > + * > + * Copyright (C) 2021 NVIDIA Corporation. > + * > + * Author: Vidya Sagar <vidyas@nvidia.com> > + */ > + > +#include <linux/pci.h> > +#include <linux/pci-acpi.h> > +#include <linux/pci-ecam.h> > + > +#include "pcie-designware.h" > + > +struct tegra194_pcie_ecam { > + void __iomem *config_base; > + void __iomem *iatu_base; > + void __iomem *dbi_base; > +}; > + > +static int tegra194_acpi_init(struct pci_config_window *cfg) > +{ > + struct device *dev = cfg->parent; > + struct tegra194_pcie_ecam *pcie_ecam; > + > + pcie_ecam = devm_kzalloc(dev, sizeof(*pcie_ecam), GFP_KERNEL); > + if (!pcie_ecam) > + return -ENOMEM; > + > + pcie_ecam->config_base = cfg->win; > + pcie_ecam->iatu_base = cfg->win + SZ_256K; > + pcie_ecam->dbi_base = cfg->win + SZ_512K; > + cfg->priv = pcie_ecam; > + > + return 0; > +} > + > +static void atu_reg_write(struct tegra194_pcie_ecam *pcie_ecam, int index, > + u32 val, u32 reg) > +{ > + u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index); > + > + writel(val, pcie_ecam->iatu_base + offset + reg); > +} > + > +static void program_outbound_atu(struct tegra194_pcie_ecam *pcie_ecam, > + int index, int type, u64 cpu_addr, > + u64 pci_addr, u64 size) > +{ > + atu_reg_write(pcie_ecam, index, lower_32_bits(cpu_addr), > + PCIE_ATU_LOWER_BASE); > + atu_reg_write(pcie_ecam, index, upper_32_bits(cpu_addr), > + PCIE_ATU_UPPER_BASE); > + atu_reg_write(pcie_ecam, index, lower_32_bits(pci_addr), > + PCIE_ATU_LOWER_TARGET); > + atu_reg_write(pcie_ecam, index, lower_32_bits(cpu_addr + size - 1), > + PCIE_ATU_LIMIT); > + atu_reg_write(pcie_ecam, index, upper_32_bits(pci_addr), > + PCIE_ATU_UPPER_TARGET); > + atu_reg_write(pcie_ecam, index, type, PCIE_ATU_CR1); > + atu_reg_write(pcie_ecam, index, PCIE_ATU_ENABLE, PCIE_ATU_CR2); > +} > + > +static void __iomem *tegra194_map_bus(struct pci_bus *bus, > + unsigned int devfn, int where) > +{ > + struct pci_config_window *cfg = bus->sysdata; > + struct tegra194_pcie_ecam *pcie_ecam = cfg->priv; > + u32 busdev; > + int type; > + > + if (bus->number < cfg->busr.start || bus->number > cfg->busr.end) > + return NULL; > + > + if (bus->number == cfg->busr.start) { > + if (PCI_SLOT(devfn) == 0) > + return pcie_ecam->dbi_base + where; > + else > + return NULL; > + } > + > + busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) | > + PCIE_ATU_FUNC(PCI_FUNC(devfn)); > + > + if (bus->parent->number == cfg->busr.start) { > + if (PCI_SLOT(devfn) == 0) > + type = PCIE_ATU_TYPE_CFG0; > + else > + return NULL; > + } else { > + type = PCIE_ATU_TYPE_CFG1; > + } > + > + program_outbound_atu(pcie_ecam, 0, type, cfg->res.start, busdev, > + SZ_256K); > + > + return pcie_ecam->config_base + where; > +} > + > +const struct pci_ecam_ops tegra194_pcie_ops = { > + .init = tegra194_acpi_init, > + .pci_ops = { > + .map_bus = tegra194_map_bus, > + .read = pci_generic_config_read, > + .write = pci_generic_config_write, > + } > +}; > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index b19775ab134e..fa6b12cfc043 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -22,8 +22,6 @@ > #include <linux/of_irq.h> > #include <linux/of_pci.h> > #include <linux/pci.h> > -#include <linux/pci-acpi.h> > -#include <linux/pci-ecam.h> > #include <linux/phy/phy.h> > #include <linux/pinctrl/consumer.h> > #include <linux/platform_device.h> > @@ -247,24 +245,6 @@ static const unsigned int pcie_gen_freq[] = { > GEN4_CORE_CLK_FREQ > }; > > -static const u32 event_cntr_ctrl_offset[] = { > - 0x1d8, > - 0x1a8, > - 0x1a8, > - 0x1a8, > - 0x1c4, > - 0x1d8 > -}; > - > -static const u32 event_cntr_data_offset[] = { > - 0x1dc, > - 0x1ac, > - 0x1ac, > - 0x1ac, > - 0x1c8, > - 0x1dc > -}; > - > struct tegra_pcie_dw { > struct device *dev; > struct resource *appl_res; > @@ -313,104 +293,6 @@ struct tegra_pcie_dw_of_data { > enum dw_pcie_device_mode mode; > }; > > -#if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS) > -struct tegra194_pcie_ecam { > - void __iomem *config_base; > - void __iomem *iatu_base; > - void __iomem *dbi_base; > -}; > - > -static int tegra194_acpi_init(struct pci_config_window *cfg) > -{ > - struct device *dev = cfg->parent; > - struct tegra194_pcie_ecam *pcie_ecam; > - > - pcie_ecam = devm_kzalloc(dev, sizeof(*pcie_ecam), GFP_KERNEL); > - if (!pcie_ecam) > - return -ENOMEM; > - > - pcie_ecam->config_base = cfg->win; > - pcie_ecam->iatu_base = cfg->win + SZ_256K; > - pcie_ecam->dbi_base = cfg->win + SZ_512K; > - cfg->priv = pcie_ecam; > - > - return 0; > -} > - > -static void atu_reg_write(struct tegra194_pcie_ecam *pcie_ecam, int index, > - u32 val, u32 reg) > -{ > - u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index); > - > - writel(val, pcie_ecam->iatu_base + offset + reg); > -} > - > -static void program_outbound_atu(struct tegra194_pcie_ecam *pcie_ecam, > - int index, int type, u64 cpu_addr, > - u64 pci_addr, u64 size) > -{ > - atu_reg_write(pcie_ecam, index, lower_32_bits(cpu_addr), > - PCIE_ATU_LOWER_BASE); > - atu_reg_write(pcie_ecam, index, upper_32_bits(cpu_addr), > - PCIE_ATU_UPPER_BASE); > - atu_reg_write(pcie_ecam, index, lower_32_bits(pci_addr), > - PCIE_ATU_LOWER_TARGET); > - atu_reg_write(pcie_ecam, index, lower_32_bits(cpu_addr + size - 1), > - PCIE_ATU_LIMIT); > - atu_reg_write(pcie_ecam, index, upper_32_bits(pci_addr), > - PCIE_ATU_UPPER_TARGET); > - atu_reg_write(pcie_ecam, index, type, PCIE_ATU_CR1); > - atu_reg_write(pcie_ecam, index, PCIE_ATU_ENABLE, PCIE_ATU_CR2); > -} > - > -static void __iomem *tegra194_map_bus(struct pci_bus *bus, > - unsigned int devfn, int where) > -{ > - struct pci_config_window *cfg = bus->sysdata; > - struct tegra194_pcie_ecam *pcie_ecam = cfg->priv; > - u32 busdev; > - int type; > - > - if (bus->number < cfg->busr.start || bus->number > cfg->busr.end) > - return NULL; > - > - if (bus->number == cfg->busr.start) { > - if (PCI_SLOT(devfn) == 0) > - return pcie_ecam->dbi_base + where; > - else > - return NULL; > - } > - > - busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) | > - PCIE_ATU_FUNC(PCI_FUNC(devfn)); > - > - if (bus->parent->number == cfg->busr.start) { > - if (PCI_SLOT(devfn) == 0) > - type = PCIE_ATU_TYPE_CFG0; > - else > - return NULL; > - } else { > - type = PCIE_ATU_TYPE_CFG1; > - } > - > - program_outbound_atu(pcie_ecam, 0, type, cfg->res.start, busdev, > - SZ_256K); > - > - return pcie_ecam->config_base + where; > -} > - > -const struct pci_ecam_ops tegra194_pcie_ops = { > - .init = tegra194_acpi_init, > - .pci_ops = { > - .map_bus = tegra194_map_bus, > - .read = pci_generic_config_read, > - .write = pci_generic_config_write, > - } > -}; > -#endif /* defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS) */ > - > -#ifdef CONFIG_PCIE_TEGRA194 > - > static inline struct tegra_pcie_dw *to_tegra_pcie(struct dw_pcie *pci) > { > return container_of(pci, struct tegra_pcie_dw, pci); > @@ -694,6 +576,24 @@ static struct pci_ops tegra_pci_ops = { > }; > > #if defined(CONFIG_PCIEASPM) > +static const u32 event_cntr_ctrl_offset[] = { > + 0x1d8, > + 0x1a8, > + 0x1a8, > + 0x1a8, > + 0x1c4, > + 0x1d8 > +}; > + > +static const u32 event_cntr_data_offset[] = { > + 0x1dc, > + 0x1ac, > + 0x1ac, > + 0x1ac, > + 0x1c8, > + 0x1dc > +}; > + > static void disable_aspm_l11(struct tegra_pcie_dw *pcie) > { > u32 val; > @@ -2413,5 +2313,3 @@ MODULE_DEVICE_TABLE(of, tegra_pcie_dw_of_match); > MODULE_AUTHOR("Vidya Sagar <vidyas@nvidia.com>"); > MODULE_DESCRIPTION("NVIDIA PCIe host controller driver"); > MODULE_LICENSE("GPL v2"); > - > -#endif /* CONFIG_PCIE_TEGRA194 */ > -- > 2.25.1 >
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile index eca805c1a023..9e6ce0dc2f53 100644 --- a/drivers/pci/controller/dwc/Makefile +++ b/drivers/pci/controller/dwc/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o obj-$(CONFIG_PCI_MESON) += pci-meson.o +obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o @@ -38,6 +39,6 @@ ifdef CONFIG_ACPI ifdef CONFIG_PCI_QUIRKS obj-$(CONFIG_ARM64) += pcie-al.o obj-$(CONFIG_ARM64) += pcie-hisi.o -obj-$(CONFIG_ARM64) += pcie-tegra194.o +obj-$(CONFIG_ARM64) += pcie-tegra194-acpi.o endif endif diff --git a/drivers/pci/controller/dwc/pcie-tegra194-acpi.c b/drivers/pci/controller/dwc/pcie-tegra194-acpi.c new file mode 100644 index 000000000000..c2de6ed4d86f --- /dev/null +++ b/drivers/pci/controller/dwc/pcie-tegra194-acpi.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ACPI quirks for Tegra194 PCIe host controller + * + * Copyright (C) 2021 NVIDIA Corporation. + * + * Author: Vidya Sagar <vidyas@nvidia.com> + */ + +#include <linux/pci.h> +#include <linux/pci-acpi.h> +#include <linux/pci-ecam.h> + +#include "pcie-designware.h" + +struct tegra194_pcie_ecam { + void __iomem *config_base; + void __iomem *iatu_base; + void __iomem *dbi_base; +}; + +static int tegra194_acpi_init(struct pci_config_window *cfg) +{ + struct device *dev = cfg->parent; + struct tegra194_pcie_ecam *pcie_ecam; + + pcie_ecam = devm_kzalloc(dev, sizeof(*pcie_ecam), GFP_KERNEL); + if (!pcie_ecam) + return -ENOMEM; + + pcie_ecam->config_base = cfg->win; + pcie_ecam->iatu_base = cfg->win + SZ_256K; + pcie_ecam->dbi_base = cfg->win + SZ_512K; + cfg->priv = pcie_ecam; + + return 0; +} + +static void atu_reg_write(struct tegra194_pcie_ecam *pcie_ecam, int index, + u32 val, u32 reg) +{ + u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index); + + writel(val, pcie_ecam->iatu_base + offset + reg); +} + +static void program_outbound_atu(struct tegra194_pcie_ecam *pcie_ecam, + int index, int type, u64 cpu_addr, + u64 pci_addr, u64 size) +{ + atu_reg_write(pcie_ecam, index, lower_32_bits(cpu_addr), + PCIE_ATU_LOWER_BASE); + atu_reg_write(pcie_ecam, index, upper_32_bits(cpu_addr), + PCIE_ATU_UPPER_BASE); + atu_reg_write(pcie_ecam, index, lower_32_bits(pci_addr), + PCIE_ATU_LOWER_TARGET); + atu_reg_write(pcie_ecam, index, lower_32_bits(cpu_addr + size - 1), + PCIE_ATU_LIMIT); + atu_reg_write(pcie_ecam, index, upper_32_bits(pci_addr), + PCIE_ATU_UPPER_TARGET); + atu_reg_write(pcie_ecam, index, type, PCIE_ATU_CR1); + atu_reg_write(pcie_ecam, index, PCIE_ATU_ENABLE, PCIE_ATU_CR2); +} + +static void __iomem *tegra194_map_bus(struct pci_bus *bus, + unsigned int devfn, int where) +{ + struct pci_config_window *cfg = bus->sysdata; + struct tegra194_pcie_ecam *pcie_ecam = cfg->priv; + u32 busdev; + int type; + + if (bus->number < cfg->busr.start || bus->number > cfg->busr.end) + return NULL; + + if (bus->number == cfg->busr.start) { + if (PCI_SLOT(devfn) == 0) + return pcie_ecam->dbi_base + where; + else + return NULL; + } + + busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) | + PCIE_ATU_FUNC(PCI_FUNC(devfn)); + + if (bus->parent->number == cfg->busr.start) { + if (PCI_SLOT(devfn) == 0) + type = PCIE_ATU_TYPE_CFG0; + else + return NULL; + } else { + type = PCIE_ATU_TYPE_CFG1; + } + + program_outbound_atu(pcie_ecam, 0, type, cfg->res.start, busdev, + SZ_256K); + + return pcie_ecam->config_base + where; +} + +const struct pci_ecam_ops tegra194_pcie_ops = { + .init = tegra194_acpi_init, + .pci_ops = { + .map_bus = tegra194_map_bus, + .read = pci_generic_config_read, + .write = pci_generic_config_write, + } +}; diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index b19775ab134e..fa6b12cfc043 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -22,8 +22,6 @@ #include <linux/of_irq.h> #include <linux/of_pci.h> #include <linux/pci.h> -#include <linux/pci-acpi.h> -#include <linux/pci-ecam.h> #include <linux/phy/phy.h> #include <linux/pinctrl/consumer.h> #include <linux/platform_device.h> @@ -247,24 +245,6 @@ static const unsigned int pcie_gen_freq[] = { GEN4_CORE_CLK_FREQ }; -static const u32 event_cntr_ctrl_offset[] = { - 0x1d8, - 0x1a8, - 0x1a8, - 0x1a8, - 0x1c4, - 0x1d8 -}; - -static const u32 event_cntr_data_offset[] = { - 0x1dc, - 0x1ac, - 0x1ac, - 0x1ac, - 0x1c8, - 0x1dc -}; - struct tegra_pcie_dw { struct device *dev; struct resource *appl_res; @@ -313,104 +293,6 @@ struct tegra_pcie_dw_of_data { enum dw_pcie_device_mode mode; }; -#if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS) -struct tegra194_pcie_ecam { - void __iomem *config_base; - void __iomem *iatu_base; - void __iomem *dbi_base; -}; - -static int tegra194_acpi_init(struct pci_config_window *cfg) -{ - struct device *dev = cfg->parent; - struct tegra194_pcie_ecam *pcie_ecam; - - pcie_ecam = devm_kzalloc(dev, sizeof(*pcie_ecam), GFP_KERNEL); - if (!pcie_ecam) - return -ENOMEM; - - pcie_ecam->config_base = cfg->win; - pcie_ecam->iatu_base = cfg->win + SZ_256K; - pcie_ecam->dbi_base = cfg->win + SZ_512K; - cfg->priv = pcie_ecam; - - return 0; -} - -static void atu_reg_write(struct tegra194_pcie_ecam *pcie_ecam, int index, - u32 val, u32 reg) -{ - u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index); - - writel(val, pcie_ecam->iatu_base + offset + reg); -} - -static void program_outbound_atu(struct tegra194_pcie_ecam *pcie_ecam, - int index, int type, u64 cpu_addr, - u64 pci_addr, u64 size) -{ - atu_reg_write(pcie_ecam, index, lower_32_bits(cpu_addr), - PCIE_ATU_LOWER_BASE); - atu_reg_write(pcie_ecam, index, upper_32_bits(cpu_addr), - PCIE_ATU_UPPER_BASE); - atu_reg_write(pcie_ecam, index, lower_32_bits(pci_addr), - PCIE_ATU_LOWER_TARGET); - atu_reg_write(pcie_ecam, index, lower_32_bits(cpu_addr + size - 1), - PCIE_ATU_LIMIT); - atu_reg_write(pcie_ecam, index, upper_32_bits(pci_addr), - PCIE_ATU_UPPER_TARGET); - atu_reg_write(pcie_ecam, index, type, PCIE_ATU_CR1); - atu_reg_write(pcie_ecam, index, PCIE_ATU_ENABLE, PCIE_ATU_CR2); -} - -static void __iomem *tegra194_map_bus(struct pci_bus *bus, - unsigned int devfn, int where) -{ - struct pci_config_window *cfg = bus->sysdata; - struct tegra194_pcie_ecam *pcie_ecam = cfg->priv; - u32 busdev; - int type; - - if (bus->number < cfg->busr.start || bus->number > cfg->busr.end) - return NULL; - - if (bus->number == cfg->busr.start) { - if (PCI_SLOT(devfn) == 0) - return pcie_ecam->dbi_base + where; - else - return NULL; - } - - busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) | - PCIE_ATU_FUNC(PCI_FUNC(devfn)); - - if (bus->parent->number == cfg->busr.start) { - if (PCI_SLOT(devfn) == 0) - type = PCIE_ATU_TYPE_CFG0; - else - return NULL; - } else { - type = PCIE_ATU_TYPE_CFG1; - } - - program_outbound_atu(pcie_ecam, 0, type, cfg->res.start, busdev, - SZ_256K); - - return pcie_ecam->config_base + where; -} - -const struct pci_ecam_ops tegra194_pcie_ops = { - .init = tegra194_acpi_init, - .pci_ops = { - .map_bus = tegra194_map_bus, - .read = pci_generic_config_read, - .write = pci_generic_config_write, - } -}; -#endif /* defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS) */ - -#ifdef CONFIG_PCIE_TEGRA194 - static inline struct tegra_pcie_dw *to_tegra_pcie(struct dw_pcie *pci) { return container_of(pci, struct tegra_pcie_dw, pci); @@ -694,6 +576,24 @@ static struct pci_ops tegra_pci_ops = { }; #if defined(CONFIG_PCIEASPM) +static const u32 event_cntr_ctrl_offset[] = { + 0x1d8, + 0x1a8, + 0x1a8, + 0x1a8, + 0x1c4, + 0x1d8 +}; + +static const u32 event_cntr_data_offset[] = { + 0x1dc, + 0x1ac, + 0x1ac, + 0x1ac, + 0x1c8, + 0x1dc +}; + static void disable_aspm_l11(struct tegra_pcie_dw *pcie) { u32 val; @@ -2413,5 +2313,3 @@ MODULE_DEVICE_TABLE(of, tegra_pcie_dw_of_match); MODULE_AUTHOR("Vidya Sagar <vidyas@nvidia.com>"); MODULE_DESCRIPTION("NVIDIA PCIe host controller driver"); MODULE_LICENSE("GPL v2"); - -#endif /* CONFIG_PCIE_TEGRA194 */
Commit 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM errata") caused a few build regressions for the Tegra194 PCIe driver which are: 1. The Tegra194 PCIe driver can no longer be built as a module. This was caused by removing the Makefile entry to build the pcie-tegra.c based upon the CONFIG_PCIE_TEGRA194 option. Therefore, restore this so that we can build the driver as a module. 2. 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM errata") added "#ifdef CONFIG_PCIE_TEGRA194" around the native driver. But if we set CONFIG_PCIE_TEGRA194=m to build the driver as a module, autoconf.h contains "#define CONFIG_PCIE_TEGRA194_MODULE 1" (not "#define CONFIG_PCIE_TEGRA194 1"), so the #ifdef excludes the driver. Given that the ACPI quirk code for Tegra194 is completely independent of the native Tegra194 PCIe driver, move this code into its own file so that it can be built independently and we can remove the "#ifdef CONFIG_PCIE_TEGRA194" in the native driver. Note that given the native Tegra194 PCIe driver is only used with device-tree, this will not cause any conflicts. 3. The below build warnings to be seen with particular kernel configurations. Fix these by moving these structure definitions to within the necessary guards. drivers/pci/controller/dwc/pcie-tegra194.c:259:18: warning: ‘event_cntr_data_offset’ defined but not used [-Wunused-const-variable=] drivers/pci/controller/dwc/pcie-tegra194.c:250:18: warning: ‘event_cntr_ctrl_offset’ defined but not used [-Wunused-const-variable=] drivers/pci/controller/dwc/pcie-tegra194.c:243:27: warning: ‘pcie_gen_freq’ defined but not used [-Wunused-const-variable=] Fixes: 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM errata") Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- Changes since V3: - Moved ACPI quirk code into separate source file Changes since V2: - Update the commit message per Bjorn's feedback - Moved the structure definitions within the necessary guards as opposed to wrapping the existing defintions with the appropriate guards. Changes since V1: - Added fixes tag - Fixed 'defined but not used' compiler warnings drivers/pci/controller/dwc/Makefile | 3 +- .../pci/controller/dwc/pcie-tegra194-acpi.c | 108 ++++++++++++++ drivers/pci/controller/dwc/pcie-tegra194.c | 138 +++--------------- 3 files changed, 128 insertions(+), 121 deletions(-) create mode 100644 drivers/pci/controller/dwc/pcie-tegra194-acpi.c