Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: neil@fatboyfat.co.uk (Neil Greatorex)
To: linux-arm-kernel@lists.infradead.org
Subject: Intel I350 mini-PCIe card (igb) on Mirabox (mvebu / Armada 370)
Date: Fri, 28 Mar 2014 01:03:55 +0000 (GMT)	[thread overview]
Message-ID: <alpine.DEB.2.10.1403280056250.4521@vroombuntu> (raw)
In-Reply-To: <20140327044054.GA22681@obsidianresearch.com>

Jason, Thomas,

With the help you've given, I think I've identified what the problem is. 
It is basically what Thomas suggested the problem was (that he'd seen with 
the Armada 38x). The call to mvebu_pcie_set_local_dev_nr causes the PCIe 
link to reset. In my tests with this Intel card, it takes ~25ms for the 
link to go down, and then another ~13ms for it to come back up.

I have put together a patch below that I would be interested to get your 
opinions on and also for you to test against any cards you may have. I 
only have the one mini PCIe card so it's not a great test!

With my card, it is now detected correctly, and there are no issues when 
rescanning the PCI bus.

Cheers,
Neil


>From a0ca1552e737c18b5651fe1f27ade76f512ca191 Mon Sep 17 00:00:00 2001
From: Neil Greatorex <neil@fatboyfat.co.uk>
Date: Fri, 28 Mar 2014 00:35:04 +0000
Subject: [PATCH] PCI: mvebu: Wait for PCIe link to reset after setting
  device number

After setting the local device nr, the PCIe link may go down briefly. If 
we don't wait for the link to come back up we may miss that device when we 
scan the bus (assuming it is not present).

This patch waits up to 40ms for the link to go down, and if it did go down,
waits up to 100ms for it to come back up. If the link was down to begin with,
it will not wait at all.

Signed-off-by: Neil Greatorex <neil@fatboyfat.co.uk>
---
  drivers/pci/host/pci-mvebu.c | 57 
++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 57 insertions(+)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 0e79665..afd0dce 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -891,6 +891,7 @@ static int mvebu_pcie_probe(struct platform_device 
*pdev)
  	for_each_child_of_node(pdev->dev.of_node, child) {
  		struct mvebu_pcie_port *port = &pcie->ports[i];
  		enum of_gpio_flags flags;
+		u32 link_present, retry_count = 0;

  		if (!of_device_is_available(child))
  			continue;
@@ -975,8 +976,64 @@ static int mvebu_pcie_probe(struct platform_device 
*pdev)
  			continue;
  		}

+		/* After setting the local dev number, the PCI-E link may go
+		 * down for a period of time. If we don't wait for it to come
+		 * back up, then we risk missing enumerating that device when
+		 * we scan the bus. However, if the link was down to begin with,
+		 * there's no point waiting, so check now.
+		 */
+		link_present = mvebu_pcie_link_up(port);
+
  		mvebu_pcie_set_local_dev_nr(port, 1);

+		if (link_present) {
+			/* In testing, the link took ~25ms to go down, and
+			 * another ~15ms to come back up, so wait ~40ms for the
+			 * link to go down, and then up to ~100ms for it to come
+			 * back up. If the link doesn't go down after ~40ms, then
+			 * it probably won't go down at all, so carry on.
+			 */
+			if (mvebu_pcie_link_up(port)) {
+				while (retry_count < 40) {
+					if (!mvebu_pcie_link_up(port))
+						break;
+					retry_count++;
+					mdelay(1);
+				}
+
+				if (retry_count == 40) {
+					dev_dbg(&pdev->dev,
+						"PCIe%d.%d: after setting dev nr, link stayed up\n",
+						port->port, port->lane);
+				} else {
+					dev_info(&pdev->dev,
+						 "PCIe%d.%d: after setting dev nr, link went down after %d polls\n",
+						 port->port, port->lane, retry_count);
+				}
+			}
+
+			retry_count = 0;
+
+			if (!mvebu_pcie_link_up(port)) {
+				while (retry_count < 100) {
+					if (mvebu_pcie_link_up(port))
+						break;
+					retry_count++;
+					mdelay(1);
+				}
+
+				if (retry_count == 100) {
+					dev_info(&pdev->dev,
+						"PCIe%d.%d: after setting dev nr, link failed to come back up (timeout)\n",
+						port->port, port->lane);
+				} else {
+					dev_info(&pdev->dev,
+						 "PCIe%d.%d: after setting dev nr, link came back up after %d polls ",
+						 port->port, port->lane, retry_count);
+				}
+			}
+		}
+
  		port->dn = child;
  		spin_lock_init(&port->conf_lock);
  		mvebu_sw_pci_bridge_init(port);
-- 
1.8.3.2

  reply	other threads:[~2014-03-28  1:03 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-25 20:07 Intel I350 mini-PCIe card (igb) on Mirabox (mvebu / Armada 370) Neil Greatorex
2014-03-25 20:20 ` Thomas Petazzoni
2014-03-25 21:03   ` Willy Tarreau
2014-03-25 20:22 ` Jason Gunthorpe
2014-03-25 20:36   ` Thomas Petazzoni
2014-03-25 21:12     ` Jason Gunthorpe
2014-03-25 21:23       ` Thomas Petazzoni
2014-03-25 22:03     ` Neil Greatorex
2014-03-25 22:24       ` Jason Gunthorpe
2014-03-25 22:35         ` Jason Gunthorpe
2014-03-26 19:31           ` Neil Greatorex
2014-03-26 20:12             ` Jason Gunthorpe
2014-03-26 20:34               ` Neil Greatorex
2014-03-26 21:42                 ` Jason Gunthorpe
2014-03-26 21:52                   ` Thomas Petazzoni
2014-03-27  0:29                   ` Neil Greatorex
2014-03-27  4:40                     ` Jason Gunthorpe
2014-03-28  1:03                       ` Neil Greatorex [this message]
2014-03-28  2:04                         ` Jason Gunthorpe
2014-04-04 13:19                         ` Neil Greatorex
2014-04-05 17:32                           ` Willy Tarreau
2014-04-05 17:34                           ` Thomas Petazzoni
2014-04-05 18:04                             ` Willy Tarreau
2014-04-05 18:55                               ` Neil Greatorex
2014-04-05 19:03                                 ` Willy Tarreau
2014-04-05 19:00                             ` Neil Greatorex
2014-04-06 15:34                               ` Neil Greatorex
2014-04-06 17:43                                 ` Willy Tarreau
2014-04-08 15:13                                 ` Thomas Petazzoni
2014-04-08 15:40                                   ` Thomas Petazzoni
2014-04-08 15:55                                     ` Thomas Petazzoni
2014-04-08 16:02                                       ` Matthew Minter
2014-04-08 17:14                                       ` Jason Gunthorpe
2014-04-08 17:53                                         ` Willy Tarreau
2014-04-08 18:08                                           ` Jason Gunthorpe
2014-04-08 18:15                                             ` Thomas Petazzoni
2014-04-08 18:40                                               ` Jason Gunthorpe
2014-04-08 19:15                                             ` Willy Tarreau
2014-04-08 19:21                                               ` Jason Gunthorpe
2014-04-08 20:17                                                 ` Matthew Minter
2014-04-08 21:50                                                   ` Thomas Petazzoni
2014-04-08 20:19                                                 ` Neil Greatorex
2014-04-08 20:43                                                 ` Willy Tarreau
2014-04-08 18:01                                         ` Thomas Petazzoni
2014-04-08 18:22                                           ` Jason Gunthorpe
2014-04-08 18:32                                             ` Thomas Petazzoni
2014-04-08 15:53                                   ` Willy Tarreau
2014-04-08 16:00                                     ` Thomas Petazzoni
2014-04-08 16:05                                       ` Willy Tarreau
2014-04-06 18:58                           ` Willy Tarreau
2014-04-06 19:11                             ` Thomas Petazzoni
2014-04-06 21:57                             ` Neil Greatorex
2014-04-06 22:04                               ` Willy Tarreau
2014-04-06 22:16                               ` Thomas Petazzoni
2014-04-07  0:50                                 ` Neil Greatorex
2014-04-07 17:41                               ` Jason Gunthorpe
2014-04-07 19:41                                 ` Neil Greatorex
2014-04-07 20:48                                   ` Jason Gunthorpe
2014-04-07 21:58                                     ` Neil Greatorex
2014-04-08  6:28                                       ` Willy Tarreau
2014-04-08  6:40                                       ` Willy Tarreau
2014-04-08 10:53                                         ` Matthew Minter
2014-04-08 12:31                                           ` Matthew Minter
2014-04-08 12:36                                             ` Willy Tarreau
2014-04-08 14:43                                               ` Thomas Petazzoni
2014-04-08 14:52                                                 ` Matthew Minter
2014-04-08 14:53                                                 ` Willy Tarreau
2014-04-08 15:25                                                   ` Thomas Petazzoni
2014-04-08 17:56                                             ` Willy Tarreau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.10.1403280056250.4521@vroombuntu \
    --to=neil@fatboyfat.co.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox