Re: [PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers

[Vladimir Oltean <olteanv@gmail.com>]

On Wed, Dec 09, 2020 at 04:40:52PM +0100, Michael Walle wrote:
> Hopefully my mail client won't mess up the output that much.

I can reproduce on my LS1028A as well. The following fixes the bug for
me. I did not follow the discussion and see if it is helpful for others.
I don't understand how the bug came to be. There might be more to it
than what I'm seeing. If it's just what I'm seeing, then the patch was
pretty broken to begin with.

-----------------------------[cut here]-----------------------------
From b184da4088c9d39d25fee2486941cdf77688a409 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Wed, 9 Dec 2020 22:17:32 +0200
Subject: [PATCH] PCI: fix invalid window size for the ECAM config space

The blamed commit forgot that pci_ecam_create() calculates the size of
the window for the ECAM's config space based on the spacing between two
buses. The drivers whose .bus_shift from struct pci_ecam_ops was changed
to zero in this commit are now using this invalid value for bus_shift
in calculating the window size.

Before (broken):
pci_ecam_create: remapping config space from addr 0x1f0000000, bus_range 0x1, bsz 0x1
After (fixed/restored):
pci_ecam_create: remapping config space from addr 0x1f0000000, bus_range 0x1, bsz 0x100000

Fixes: f3c07cf6924e ("PCI: Unify ECAM constants in native PCI Express drivers")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/pci/ecam.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index 59f91d434859..9fda0d49bc93 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -28,11 +28,19 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
 		struct resource *cfgres, struct resource *busr,
 		const struct pci_ecam_ops *ops)
 {
+	unsigned int bus_shift = ops->bus_shift;
 	struct pci_config_window *cfg;
 	unsigned int bus_range, bus_range_max, bsz;
 	struct resource *conflict;
 	int i, err;
 
+	/*
+	 * struct pci_ecam_ops may omit specifying bus_shift
+	 * if it is as per spec
+	 */
+	if (!bus_shift)
+		bus_shift = PCIE_ECAM_BUS_SHIFT;
+
 	if (busr->start > busr->end)
 		return ERR_PTR(-EINVAL);
 
@@ -46,14 +54,14 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
 	cfg->busr.end = busr->end;
 	cfg->busr.flags = IORESOURCE_BUS;
 	bus_range = resource_size(&cfg->busr);
-	bus_range_max = resource_size(cfgres) >> ops->bus_shift;
+	bus_range_max = resource_size(cfgres) >> bus_shift;
 	if (bus_range > bus_range_max) {
 		bus_range = bus_range_max;
 		cfg->busr.end = busr->start + bus_range - 1;
 		dev_warn(dev, "ECAM area %pR can only accommodate %pR (reduced from %pR desired)\n",
 			 cfgres, &cfg->busr, busr);
 	}
-	bsz = 1 << ops->bus_shift;
+	bsz = 1 << bus_shift;
 
 	cfg->res.start = cfgres->start;
 	cfg->res.end = cfgres->end;
-----------------------------[cut here]-----------------------------

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel