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: 3964581 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 85D259F336 for ; Thu, 10 Apr 2014 20:12:44 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A0D8220829 for ; Thu, 10 Apr 2014 20:12:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CFE3720824 for ; Thu, 10 Apr 2014 20:12:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965159AbaDJUMl (ORCPT ); Thu, 10 Apr 2014 16:12:41 -0400 Received: from quartz.orcorp.ca ([184.70.90.242]:46648 "EHLO quartz.orcorp.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935282AbaDJUMi (ORCPT ); Thu, 10 Apr 2014 16:12:38 -0400 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 Cc: Neil Greatorex , Willy Tarreau , Matthew Minter , Gerlando Falauto , linux-arm-kernel@lists.infradead.org, Jason Cooper , Gregory =?iso-8859-1?Q?Cl=E9ment?= , Ezequiel Garcia , Andrew Lunn , linux-pci@vger.kernel.org, Tawfik Bayouk , Lior Amsalem 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 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham 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);