From patchwork Thu Apr 10 20:12:01 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Gunthorpe X-Patchwork-Id: 3964591 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 7A8AABFF02 for ; Thu, 10 Apr 2014 20:12:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9235A20824 for ; Thu, 10 Apr 2014 20:12:57 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7644E20619 for ; Thu, 10 Apr 2014 20:12:56 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WYLL7-0004FP-MZ; Thu, 10 Apr 2014 20:12:49 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WYLL3-0005hx-4H; Thu, 10 Apr 2014 20:12:47 +0000 Received: from bombadil.infradead.org ([2001:1868:205::9]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WYLKy-0005he-7n for linux-arm-kernel@merlin.infradead.org; Thu, 10 Apr 2014 20:12:40 +0000 Received: from quartz.orcorp.ca ([184.70.90.242]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WYLKw-00024r-Qd for linux-arm-kernel@lists.infradead.org; Thu, 10 Apr 2014 20:12:39 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=obsidianresearch.com; s=rsa1; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=1Jnqhlms52Emp3hjF1n+q9xay5aMtQx6C5ajoaZhLwE=; b=wlWviSfw77db0yt07PUdv32B24rrjimo2ILwsnGimYltuwNBEHHNA/KdiktGff6viRzQPXf/sk9exyA9bks4ypXsPq5s9yGxMebLgvme13FHeSk0yxIwwH30paEh+kMGykAEFVSj7PnA/ANwRYqTr4MiwmkP/Je1VC24um+GUqU=; Received: from [10.0.0.161] (helo=jggl.edm.orcorp.ca) by quartz.orcorp.ca with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1WYLKM-0002Cv-3B; Thu, 10 Apr 2014 14:12:02 -0600 Received: from jgg by jggl.edm.orcorp.ca with local (Exim 4.80) (envelope-from ) id 1WYLKL-0001go-N6; Thu, 10 Apr 2014 14:12:01 -0600 Date: Thu, 10 Apr 2014 14:12:01 -0600 From: Jason Gunthorpe To: Thomas Petazzoni Subject: Re: Fixing PCIe issues on Armada XP Message-ID: <20140410201201.GA12661@obsidianresearch.com> References: <20140410181953.50ccfcc3@skate> <20140410165733.GB23104@obsidianresearch.com> <20140410200153.46669e0c@skate> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140410200153.46669e0c@skate> User-Agent: Mutt/1.5.21 (2010-09-15) X-Broken-Reverse-DNS: no host name found for IP address 10.0.0.161 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140410_131238_974619_6328E9FC X-CRM114-Status: GOOD ( 29.57 ) X-Spam-Score: -0.1 (/) Cc: Lior Amsalem , Andrew Lunn , Matthew Minter , Jason Cooper , Tawfik Bayouk , linux-pci@vger.kernel.org, Neil Greatorex , Gerlando Falauto , Ezequiel Garcia , Gregory =?iso-8859-1?Q?Cl=E9ment?= , Willy Tarreau , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Apr 10, 2014 at 08:01:53PM +0200, Thomas Petazzoni wrote: > > Can you run Neil's patch and see if your system behaves the same? > > Specifically that the link eventually goes down after > > mvebu_pcie_set_local_dev_nr ? > > > > I couldn't find any case where the BDF leaks below the TLP layer, and > > the spec is very clear that the assigned BDF can change at run time, > > so I don't have an explanation for why the link reset is happening. > > Do you have a contact at Marvell that might shed some light on that > > behavior? > > There was a potential explanation about the mvebu-soc-id driver that > enables the clock and then disables it, before the pci-mvebu driver. > This is different that what was happening before, where the pci-mvebu > driver was the only one to enable the clock, which was already enabled > by the bootloader. So if the clock takes some time to stabilize, the > introduction of mvebu-soc-id may lead to problems. Oh, that certainly sounds like a potential problem. Disabling the clock will certainly cause 'interesting' effects on the PEX, depending on exactly what it does it could confuse the link partner (trigger a timeout based retrain?). Gating the clock without disabling the Phy first does sound like a bad idea.. Neil, does this do anything for you? Jason diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c index 0d638b7..7b7d19a 100644 --- a/drivers/pci/host/pci-mvebu.c +++ b/drivers/pci/host/pci-mvebu.c @@ -21,6 +21,7 @@ #include #include #include +#include /* * PCIe unit register offsets. @@ -973,6 +974,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; + bool enabled; if (!of_device_is_available(child)) continue; @@ -1044,6 +1046,9 @@ static int mvebu_pcie_probe(struct platform_device *pdev) continue; } + // Does this work on MVEBU? + enabled = __clk_is_enabled(port->clk); + ret = clk_prepare_enable(port->clk); if (ret) continue; @@ -1057,7 +1062,35 @@ static int mvebu_pcie_probe(struct platform_device *pdev) continue; } - mvebu_pcie_set_local_dev_nr(port, 1); + if (!enabled) { + u32 reg; + unsigned int tries; + + /* The clock is being turned on for the first time, do + * a PHY reset + */ + dev_info(&pdev->dev, + "PCIe%d.%d: performing link reset\n", + port->port, port->lane); + reg = mvebu_readl(port, PCIE_CTRL_OFF); + mvebu_writel(port, + reg & ~BIT(30), // Conf_TrainingDisable + PCIE_CTRL_OFF); + do { + udelay(100); // Guess? + } while (mvebu_pcie_link_up(port)); + mvebu_pcie_set_local_dev_nr(port, 1); + mvebu_writel(port, reg | ~BIT(30), PCIE_CTRL_OFF); + + for (tries = 0; + !mvebu_pcie_link_up(port) && tries != 100; tries++) + udelay(100); + } else { + /* We expect the bootloader has setup the port and + * waited for the link to go up + */ + mvebu_pcie_set_local_dev_nr(port, 1); + } port->dn = child; spin_lock_init(&port->conf_lock);