Skip to contentSkip to conversation

PCI: tegra: Use generic accessors where possible

PATCH Applied Thierry Reding 3 participants 4 messages Sep 23, 2017 – Oct 11, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 PCI: tegra: Use generic accessors where possible

The Tegra PCI host controller can generate configuration space accesses with byte, word and dword granularity for devices. Only root ports can't have their configuration space accessed in this way.

---
 drivers/pci/host/pci-tegra.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)
drivers/pci/host/pci-tegra.c +22 -2
index 9c40da54f88a..e8e1ddbaabc9 100644
@@ -491,12 +491,32 @@ static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus,
 	return addr;
 }
 
+static int tegra_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
+				  int where, int size, u32 *value)
+{
+	if (bus->number == 0)
+		return pci_generic_config_read32(bus, devfn, where, size,
+						 value);
+
+	return pci_generic_config_read(bus, devfn, where, size, value);
+}
+
+static int tegra_pcie_config_write(struct pci_bus *bus, unsigned int devfn,
+				   int where, int size, u32 value)
+{
+	if (bus->number == 0)
+		return pci_generic_config_write32(bus, devfn, where, size,
+						  value);
+
+	return pci_generic_config_write(bus, devfn, where, size, value);
+}
+
 static struct pci_ops tegra_pcie_ops = {
 	.add_bus = tegra_pcie_add_bus,
 	.remove_bus = tegra_pcie_remove_bus,
 	.map_bus = tegra_pcie_map_bus,
-	.read = pci_generic_config_read32,
-	.write = pci_generic_config_write32,
+	.read = tegra_pcie_config_read,
+	.write = tegra_pcie_config_write,
 };
 
 static unsigned long tegra_pcie_port_get_pex_ctrl(struct tegra_pcie_port *port)
Signature · 1 line
2.14.1
#1 lore raw
Quoted 15 lines · Thierry Reding

The Tegra PCI host controller can generate configuration space accesses with byte, word and dword granularity for devices. Only root ports can't have their configuration space accessed in this way.

---
 drivers/pci/host/pci-tegra.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)
drivers/pci/host/pci-tegra.c +0 -0
index 9c40da54f88a..e8e1ddbaabc9 100644
@@ -491,12 +491,32 @@ static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus,
        return addr;

drivers/pci/host/pci-tegra.c static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus, · line 501

Quoted 10 lines
drivers/pci/host/pci-tegra.c +8 -0
 }
 
+static int tegra_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
+                                 int where, int size, u32 *value)
+{
+       if (bus->number == 0)
+               return pci_generic_config_read32(bus, devfn, where, size,
+                                                value);
+
+       return pci_generic_config_read(bus, devfn, where, size, value);

Since T20, T30 and T124 had issues with 8-bit and 16-bit end point config accesses, generic accessors should be used only for T210

drivers/pci/host/pci-tegra.c static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus, · line 511

Quoted 10 lines
drivers/pci/host/pci-tegra.c +10 -0
+}
+
+static int tegra_pcie_config_write(struct pci_bus *bus, unsigned int devfn,
+                                  int where, int size, u32 value)
+{
+       if (bus->number == 0)
+               return pci_generic_config_write32(bus, devfn, where, size,
+                                                 value);
+
+       return pci_generic_config_write(bus, devfn, where, size, value);

same as above.

Quoted 16 lines
drivers/pci/host/pci-tegra.c +4 -3
+}
+
 static struct pci_ops tegra_pcie_ops = {
        .add_bus = tegra_pcie_add_bus,
        .remove_bus = tegra_pcie_remove_bus,
        .map_bus = tegra_pcie_map_bus,
-       .read = pci_generic_config_read32,
-       .write = pci_generic_config_write32,
+       .read = tegra_pcie_config_read,
+       .write = tegra_pcie_config_write,
 };
 
 static unsigned long tegra_pcie_port_get_pex_ctrl(struct tegra_pcie_port *port)
--

2.14.1

#2 lore raw
Quoted 19 lines · vidya sagar
Quoted 17 lines · Thierry Reding

The Tegra PCI host controller can generate configuration space accesses with byte, word and dword granularity for devices. Only root ports can't have their configuration space accessed in this way.

---
 drivers/pci/host/pci-tegra.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)
drivers/pci/host/pci-tegra.c +0 -0
index 9c40da54f88a..e8e1ddbaabc9 100644
@@ -491,12 +491,32 @@ static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus,
        return addr;
 }
 
Quoted 11 lines

drivers/pci/host/pci-tegra.c static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus, · line 501

Quoted 8 lines
drivers/pci/host/pci-tegra.c +8 -0
+static int tegra_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
+                                 int where, int size, u32 *value)
+{
+       if (bus->number == 0)
+               return pci_generic_config_read32(bus, devfn, where, size,
+                                                value);
+
+       return pci_generic_config_read(bus, devfn, where, size, value);

Since T20, T30 and T124 had issues with 8-bit and 16-bit end point config accesses, generic accessors should be used only for T210

I thought that 8-bit and 16-bit configuration space accesses were only ever problematic for the root ports. I've certainly not seen 8-bit and 16-bit accesses fail on any of the devices I tested on. That is, this patch was tested on TrimSlice (Tegra20), Beaver (Tegra30), Jetson TK1 (Tegra124) and Jetson TX1 (Tegra210) without any issues.

Thierry

#3 lore raw
In reply to Thierry Reding

[+cc vidya]

Quoted 5 lines · Thierry Reding

The Tegra PCI host controller can generate configuration space accesses with byte, word and dword granularity for devices. Only root ports can't have their configuration space accessed in this way.

Applied to pci/host-tegra for v4.15, thanks!

Quoted 46 lines
---
 drivers/pci/host/pci-tegra.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)
drivers/pci/host/pci-tegra.c +22 -2
index 9c40da54f88a..e8e1ddbaabc9 100644
@@ -491,12 +491,32 @@ static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus,
 	return addr;
 }
 
+static int tegra_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
+				  int where, int size, u32 *value)
+{
+	if (bus->number == 0)
+		return pci_generic_config_read32(bus, devfn, where, size,
+						 value);
+
+	return pci_generic_config_read(bus, devfn, where, size, value);
+}
+
+static int tegra_pcie_config_write(struct pci_bus *bus, unsigned int devfn,
+				   int where, int size, u32 value)
+{
+	if (bus->number == 0)
+		return pci_generic_config_write32(bus, devfn, where, size,
+						  value);
+
+	return pci_generic_config_write(bus, devfn, where, size, value);
+}
+
 static struct pci_ops tegra_pcie_ops = {
 	.add_bus = tegra_pcie_add_bus,
 	.remove_bus = tegra_pcie_remove_bus,
 	.map_bus = tegra_pcie_map_bus,
-	.read = pci_generic_config_read32,
-	.write = pci_generic_config_write32,
+	.read = tegra_pcie_config_read,
+	.write = tegra_pcie_config_write,
 };
 
 static unsigned long tegra_pcie_port_get_pex_ctrl(struct tegra_pcie_port *port)
Signature · 1 line
2.14.1