From patchwork Fri Aug 18 21:31:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 9909975 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1F9B0600CC for ; Fri, 18 Aug 2017 21:32:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 11BA628C18 for ; Fri, 18 Aug 2017 21:32:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0670528D6E; Fri, 18 Aug 2017 21:32:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, DKIM_VALID, RCVD_IN_DNSWL_LOW autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 6A6C928C18 for ; Fri, 18 Aug 2017 21:32:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:To:From: Subject:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=yL2kkCMA0hx6MpZugJE9Me8LCQr9cPdE9AfSlzJv1bA=; b=g9D/5vWpj4gIES ZUyP5z9RGOg90ab/F9yInaC7JBk7uZaNI0N4iEra4L/1Vu02NYDshUnbpqP+lSlSatA+7WvkxNOfN yWqGCRFDe3s+3ax+TsYmb4boVmd+9JtHBoWwQ9Sk6DY/XtzZgD8QAU6AHzxJbrWb/ximdcmJ8y1Dq AhywJk9dbQG6+Jh9tAOW0FkKdgClMKsn1Al9GjRw1/j2TYC8oFGYipuKjINO9R+ck0NHd9Np9z/Ja pjWwGZfO1AeI67ExaFekS6V4gCsNX+sCTdZIXgnvooG8tw5Rq7W1Ie2hIuP7iSKjemOwG74daqUNL e0p+S7pMyOo2XRzlMaBA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1diosQ-0007FH-Rf; Fri, 18 Aug 2017 21:32:22 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1diosN-0006x3-19 for linux-arm-kernel@lists.infradead.org; Fri, 18 Aug 2017 21:32:20 +0000 Received: from localhost (173-24-227-107.client.mchsi.com [173.24.227.107]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1CCC82170C; Fri, 18 Aug 2017 21:31:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1CCC82170C Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Subject: [PATCH v11 0/4] PCI: Use CRS Software Visibility to wait for device to become ready From: Bjorn Helgaas To: Sinan Kaya Date: Fri, 18 Aug 2017 16:31:56 -0500 Message-ID: <20170818212310.15145.21732.stgit@bhelgaas-glaptop.roam.corp.google.com> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170818_143219_152502_40AB5C69 X-CRM114-Status: GOOD ( 16.78 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-pci@vger.kernel.org, Timur Tabi , linux-kernel@vger.kernel.org, Alex Williamson , linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP This is another rev of Sinan's series to fix an FLR issue seen with an Intel 750 NVMe drive. It seems that the 750 requires more time than we currently allow after the FLR to become ready for use. The biggest remaining issue is that I *think* this does not fix the issue on systems where CRS Software Visibility is not supported or disabled. Differences from v10: - Move 0x0001 test inside pci_bus_wait_crs() - Fix pre-existing "valid response before timeout" bug - Add message if a device becomes ready after we printed "waiting" - Reorder messages so we print "giving up after X" instead of "not ready after X; giving up after X" --- Bjorn Helgaas (1): PCI: Don't ignore valid response before CRS timeout Sinan Kaya (3): PCI: Factor out pci_bus_wait_crs() PCI: Handle CRS ("device not ready") returned by device after FLR PCI: Warn periodically while waiting for device to become ready drivers/pci/pci.c | 19 ++++++++++++++-- drivers/pci/pci.h | 1 + drivers/pci/probe.c | 61 +++++++++++++++++++++++++++++++++------------------ 3 files changed, 57 insertions(+), 24 deletions(-) Incremental diff from Sinan's v10 posting: diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index c853551bc8d1..34c0aa1f37aa 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3824,22 +3824,18 @@ static void pci_flr_wait(struct pci_dev *dev) bool ret; /* - * Don't touch the HW before waiting 100ms. HW has to finish - * within 100ms according to PCI Express Base Specification - * Revision 3.1 Section 6.6.2: Function-Level Reset (FLR). + * Per PCIe r3.1, sec 6.6.2, the device should finish FLR within + * 100ms, but even after that, it may respond to config requests + * with CRS status if it requires more time. */ msleep(100); - if (pci_bus_read_config_dword(dev->bus, dev->devfn, PCI_VENDOR_ID, - &id)) + if (pci_bus_read_config_dword(dev->bus, dev->devfn, PCI_VENDOR_ID, &id)) return; - /* See if we can find a device via CRS first. */ - if ((id & 0xffff) == 0x0001) { - ret = pci_bus_wait_crs(dev->bus, dev->devfn, &id, 60000); - if (ret) - return; - } + ret = pci_bus_wait_crs(dev->bus, dev->devfn, &id, 60000); + if (ret) + return; do { msleep(100); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 1bbe85190183..b0857052c04a 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -235,8 +235,7 @@ enum pci_bar_type { pci_bar_mem64, /* A 64-bit memory BAR */ }; -bool pci_bus_wait_crs(struct pci_bus *bus, int devfn, u32 *l, - int crs_timeout); +bool pci_bus_wait_crs(struct pci_bus *bus, int devfn, u32 *l, int timeout); bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl, int crs_timeout); int pci_setup_device(struct pci_dev *dev); diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 2f4cf7d0e25e..99799cc1de22 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1824,42 +1824,50 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus) } EXPORT_SYMBOL(pci_alloc_dev); -bool pci_bus_wait_crs(struct pci_bus *bus, int devfn, u32 *l, int crs_timeout) +bool pci_bus_wait_crs(struct pci_bus *bus, int devfn, u32 *l, int timeout) { int delay = 1; + if ((*l & 0xffff) != 0x0001) + return true; /* not a CRS completion */ + + if (!timeout) + return false; /* CRS, but caller doesn't want to wait */ + /* - * Configuration Request Retry Status. Some root ports return the - * actual device ID instead of the synthetic ID (0xFFFF) required - * by the PCIe spec. Ignore the device ID and only check for - * (vendor id == 1). + * We got the reserved Vendor ID that indicates a completion with + * Configuration Request Retry Status (CRS). Retry until we get a + * valid Vendor ID or we time out. */ - do { + while ((*l & 0xffff) == 0x0001) { msleep(delay); delay *= 2; - if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, l)) + + if (delay > timeout) { + pr_warn("pci %04x:%02x:%02x.%d: not ready after %dms; giving up\n", + pci_domain_nr(bus), bus->number, + PCI_SLOT(devfn), PCI_FUNC(devfn), delay - 1); return false; + } if (delay >= 1000) - pr_info("pci %04x:%02x:%02x.%d: not responding since %dms still polling\n", + pr_info("pci %04x:%02x:%02x.%d: not ready after %dms; waiting\n", pci_domain_nr(bus), bus->number, - PCI_SLOT(devfn), PCI_FUNC(devfn), delay); + PCI_SLOT(devfn), PCI_FUNC(devfn), delay - 1); - /* Card hasn't responded in 60 seconds? Must be stuck. */ - if (delay > crs_timeout) { - pr_warn("pci %04x:%02x:%02x.%d: not responding %dms timeout reached\n", - pci_domain_nr(bus), bus->number, PCI_SLOT(devfn), - PCI_FUNC(devfn), crs_timeout); + if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, l)) return false; - } - } while ((*l & 0xffff) == 0x0001); + } + if (delay >= 1000) + pr_info("pci %04x:%02x:%02x.%d: ready after %dms\n", + pci_domain_nr(bus), bus->number, + PCI_SLOT(devfn), PCI_FUNC(devfn), delay - 1); return true; } -EXPORT_SYMBOL(pci_bus_wait_crs); bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l, - int crs_timeout) + int timeout) { if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, l)) return false; @@ -1869,20 +1877,7 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l, *l == 0x0000ffff || *l == 0xffff0000) return false; - /* - * Configuration Request Retry Status. Some root ports return the - * actual device ID instead of the synthetic ID (0xFFFF) required - * by the PCIe spec. Ignore the device ID and only check for - * (vendor id == 1). - */ - if ((*l & 0xffff) == 0x0001) { - if (!crs_timeout) - return false; - - return pci_bus_wait_crs(bus, devfn, l, crs_timeout); - } - - return true; + return pci_bus_wait_crs(bus, devfn, l, timeout); } EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);