Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754199AbdHURov (ORCPT ); Mon, 21 Aug 2017 13:44:51 -0400 Received: from mail.kernel.org ([198.145.29.99]:41836 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754000AbdHURou (ORCPT ); Mon, 21 Aug 2017 13:44:50 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 69B1F21456 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Date: Mon, 21 Aug 2017 12:44:47 -0500 From: Bjorn Helgaas To: Sinan Kaya Cc: Bjorn Helgaas , 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 Subject: Re: [PATCH v11 1/4] PCI: Don't ignore valid response before CRS timeout Message-ID: <20170821174447.GA28977@bhelgaas-glaptop.roam.corp.google.com> References: <20170818212310.15145.21732.stgit@bhelgaas-glaptop.roam.corp.google.com> <20170818213203.15145.36487.stgit@bhelgaas-glaptop.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2890 Lines: 81 On Mon, Aug 21, 2017 at 10:02:55AM -0400, Sinan Kaya wrote: > On 8/18/2017 5:32 PM, Bjorn Helgaas wrote: > > While waiting for a device to become ready (i.e., to return a non-CRS > > completion to a read of its Vendor ID), if we got a valid response to the > > very last read before timing out, we printed a warning and gave up on the > > device even though it was actually ready. > > > > For a typical 60s timeout, we wait about 65s (it's not exact because of the > > exponential backoff), but we treated devices that became ready between 33s > > and 65s as though they failed. > > > > Move the Device ID read later so we check whether the device is ready > > immediately, before checking for a timeout. > > > > Signed-off-by: Bjorn Helgaas > > --- > > drivers/pci/probe.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index c31310db0404..08ea844ac4ba 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -1849,15 +1849,16 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l, > > > > msleep(delay); > > delay *= 2; > > - if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, l)) > > - return false; > > - /* Card hasn't responded in 60 seconds? Must be stuck. */ > > + > > There is still a problem here. We'll wait some time above and return without checking if > we actually found the card or not. > > > if (delay > crs_timeout) { > > printk(KERN_WARNING "pci %04x:%02x:%02x.%d: not responding\n", > > pci_domain_nr(bus), bus->number, PCI_SLOT(devfn), > > PCI_FUNC(devfn)); > > return false; > > } > > + > > + if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, l)) > > + return false; > > } > > > > return true; > > > > > > Here is another shot at it. Sorry, I don't have a diff syntax. I made up the function > by copy paste. Yep, this is much better, thanks! > while ((*l & 0xffff) == 0x0001) { > if (!crs_timeout) > return false; > > /* Card hasn't responded in 60 seconds? Must be stuck. */ > if (delay > crs_timeout) { > printk(KERN_WARNING "pci %04x:%02x:%02x.%d: not responding\n", > pci_domain_nr(bus), bus->number, PCI_SLOT(devfn), > PCI_FUNC(devfn)); > return false; > } > msleep(delay); > delay *= 2; > if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, l)) > return false; > } > > -- > Sinan Kaya > Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. > Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel