Patchwork PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

login
register
mail settings
Submitter Jason Gunthorpe
Date Sept. 17, 2013, 6:32 p.m.
Message ID <20130917183250.GA21230@obsidianresearch.com>
Download mbox | patch
Permalink /patch/275518/
State Not Applicable
Headers show

Comments

Jason Gunthorpe - Sept. 17, 2013, 6:32 p.m.
Otherwise hotplugging the PEX doesn't work at all since the driver
detects the link state at probe time. Simply replacing the two tests
of haslink with a register read is enough to fix discovery.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/pci/host/pci-mvebu.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)
Thomas Petazzoni - Sept. 17, 2013, 7:05 p.m.
Dear Jason Gunthorpe,

On Tue, 17 Sep 2013 12:32:50 -0600, Jason Gunthorpe wrote:
> Otherwise hotplugging the PEX doesn't work at all since the driver
> detects the link state at probe time. Simply replacing the two tests
> of haslink with a register read is enough to fix discovery.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

I don't have the hardware to test, but this seems to make sense to me.

Thomas
Jason Gunthorpe - Sept. 17, 2013, 7:24 p.m.
On Tue, Sep 17, 2013 at 09:05:20PM +0200, Thomas Petazzoni wrote:
> Dear Jason Gunthorpe,
> 
> On Tue, 17 Sep 2013 12:32:50 -0600, Jason Gunthorpe wrote:
> > Otherwise hotplugging the PEX doesn't work at all since the driver
> > detects the link state at probe time. Simply replacing the two tests
> > of haslink with a register read is enough to fix discovery.
> > 
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> 
> I don't have the hardware to test, but this seems to make sense to me.

FWIW, I have tested this on Kirkwood:

mvebu-pcie pex.1: PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [mem 0xe0000000-0xf0000000]
pci_bus 0000:00: root bus resource [bus 00-ff]
pci_bus 0000:00: root bus resource [io  0x1000-0xffff]
pci 0000:00:01.0: [11ab:7846] type 01 class 0x060400
PCI: bus0: Fast back to back transfers disabled
pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
PCI: bus1: Fast back to back transfers enabled
pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
pci 0000:00:01.0: PCI bridge to [bus 01]
[..]
Freeing unused kernel memory: 1096K (c02ee000 - c0400000)

.. hot plug the device ..

echo 1 > rescan

pci 0000:01:00.0: [170c:0001] type 00 class 0x058000
pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
pci 0000:01:00.0: PME# supported from D0 D1 D2 D3hot
pci_bus 0000:01: busn_res: [bus 01] end is updated to 01
pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xe00fffff]
pci 0000:01:00.0: BAR 0: assigned [mem 0xe0000000-0xe001ffff]
pci 0000:00:01.0: PCI bridge to [bus 01]
pci 0000:00:01.0:   bridge window [mem 0xe0000000-0xe00fffff]
PCI: enabling device 0000:00:01.0 (0140 -> 0143)
PCI: enabling device 0000:01:00.0 (0000 -> 0002)

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thomas Petazzoni - Sept. 17, 2013, 7:50 p.m.
Dear Jason Gunthorpe,

On Tue, 17 Sep 2013 13:24:12 -0600, Jason Gunthorpe wrote:

> > I don't have the hardware to test, but this seems to make sense to me.
> 
> FWIW, I have tested this on Kirkwood:
> 
> mvebu-pcie pex.1: PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [mem 0xe0000000-0xf0000000]
> pci_bus 0000:00: root bus resource [bus 00-ff]
> pci_bus 0000:00: root bus resource [io  0x1000-0xffff]
> pci 0000:00:01.0: [11ab:7846] type 01 class 0x060400
> PCI: bus0: Fast back to back transfers disabled
> pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> PCI: bus1: Fast back to back transfers enabled
> pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
> pci 0000:00:01.0: PCI bridge to [bus 01]
> [..]
> Freeing unused kernel memory: 1096K (c02ee000 - c0400000)
> 
> .. hot plug the device ..
> 
> echo 1 > rescan
> 
> pci 0000:01:00.0: [170c:0001] type 00 class 0x058000
> pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
> pci 0000:01:00.0: PME# supported from D0 D1 D2 D3hot
> pci_bus 0000:01: busn_res: [bus 01] end is updated to 01
> pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xe00fffff]
> pci 0000:01:00.0: BAR 0: assigned [mem 0xe0000000-0xe001ffff]
> pci 0000:00:01.0: PCI bridge to [bus 01]
> pci 0000:00:01.0:   bridge window [mem 0xe0000000-0xe00fffff]
> PCI: enabling device 0000:00:01.0 (0140 -> 0143)
> PCI: enabling device 0000:01:00.0 (0000 -> 0002)

Nice!

Good to see that the changes needed to get PCI hotplug working were not
so large :)

Thomas

Patch

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 729d5a1..f2d61f5 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -115,7 +115,6 @@  struct mvebu_pcie_port {
 	char *name;
 	void __iomem *base;
 	spinlock_t conf_lock;
-	int haslink;
 	u32 port;
 	u32 lane;
 	int devfn;
@@ -552,7 +551,7 @@  static int mvebu_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 	if (bus->number == 0)
 		return mvebu_sw_pci_bridge_write(port, where, size, val);
 
-	if (!port->haslink)
+	if (!mvebu_pcie_link_up(port))
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
 	/*
@@ -594,7 +593,7 @@  static int mvebu_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 	if (bus->number == 0)
 		return mvebu_sw_pci_bridge_read(port, where, size, val);
 
-	if (!port->haslink) {
+	if (!mvebu_pcie_link_up(port)) {
 		*val = 0xffffffff;
 		return PCIBIOS_DEVICE_NOT_FOUND;
 	}
@@ -883,22 +882,11 @@  static int __init mvebu_pcie_probe(struct platform_device *pdev)
 
 		mvebu_pcie_set_local_dev_nr(port, 1);
 
-		if (mvebu_pcie_link_up(port)) {
-			port->haslink = 1;
-			dev_info(&pdev->dev, "PCIe%d.%d: link up\n",
-				 port->port, port->lane);
-		} else {
-			port->haslink = 0;
-			dev_info(&pdev->dev, "PCIe%d.%d: link down\n",
-				 port->port, port->lane);
-		}
-
 		port->clk = of_clk_get_by_name(child, NULL);
 		if (IS_ERR(port->clk)) {
 			dev_err(&pdev->dev, "PCIe%d.%d: cannot get clock\n",
 			       port->port, port->lane);
 			iounmap(port->base);
-			port->haslink = 0;
 			continue;
 		}