{"id":805195,"url":"http://patchwork.ozlabs.org/api/1.2/patches/805195/?format=json","web_url":"http://patchwork.ozlabs.org/project/linux-pci/patch/20170823222436.GH8498@bhelgaas-glaptop.roam.corp.google.com/","project":{"id":28,"url":"http://patchwork.ozlabs.org/api/1.2/projects/28/?format=json","name":"Linux PCI development","link_name":"linux-pci","list_id":"linux-pci.vger.kernel.org","list_email":"linux-pci@vger.kernel.org","web_url":null,"scm_url":null,"webscm_url":null,"list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20170823222436.GH8498@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-08-23T22:24:36","name":"[V12,4/5] PCI: Handle CRS (\"device not ready\") returned by device after FLR","commit_ref":null,"pull_url":null,"state":"not-applicable","archived":false,"hash":"052131cdf8f87da3b32802f05e0361a053ad3b4b","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/1.2/people/67298/?format=json","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/linux-pci/patch/20170823222436.GH8498@bhelgaas-glaptop.roam.corp.google.com/mbox/","series":[],"comments":"http://patchwork.ozlabs.org/api/patches/805195/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/805195/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=helgaas@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xd26Y2NR6z9t2m\n\tfor ;\n\tThu, 24 Aug 2017 08:24:53 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751186AbdHWWYl (ORCPT );\n\tWed, 23 Aug 2017 18:24:41 -0400","from mail.kernel.org ([198.145.29.99]:44518 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751143AbdHWWYj (ORCPT );\n\tWed, 23 Aug 2017 18:24:39 -0400","from localhost (unknown [69.71.4.159])\n\t(using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits))\n\t(No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 2371A21A1C;\n\tWed, 23 Aug 2017 22:24:38 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 2371A21A1C","Date":"Wed, 23 Aug 2017 17:24:36 -0500","From":"Bjorn Helgaas ","To":"Sinan Kaya ","Cc":"linux-pci@vger.kernel.org, timur@codeaurora.org,\n\talex.williamson@redhat.com, linux-arm-msm@vger.kernel.org,\n\tBjorn Helgaas ,\n\tlinux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org","Subject":"Re: [PATCH V12 4/5] PCI: Handle CRS (\"device not ready\") returned by\n\tdevice after FLR","Message-ID":"<20170823222436.GH8498@bhelgaas-glaptop.roam.corp.google.com>","References":"<1503464171-6471-1-git-send-email-okaya@codeaurora.org>\n\t<1503464171-6471-4-git-send-email-okaya@codeaurora.org>\n\t<20170823213838.GG8498@bhelgaas-glaptop.roam.corp.google.com>\n\t<0ef4c01d-75bc-17dc-8926-2f9647cfeec8@codeaurora.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<0ef4c01d-75bc-17dc-8926-2f9647cfeec8@codeaurora.org>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pci@vger.kernel.org"},"content":"On Wed, Aug 23, 2017 at 05:51:19PM -0400, Sinan Kaya wrote:\n> On 8/23/2017 5:38 PM, Bjorn Helgaas wrote:\n> > If we increase the timeout, is there still value in adding the\n> > pci_bus_wait_crs() stuff? I'm not sure there is.\n> \n> I agree increasing the timeout is more than enough for FLR case. \n\nIf that's the case, I think there's no value in adding additional\ncomplexity to the path. If we increase the timeout, we might pretty\nit up a little along the lines of the patch below.\n\n> However, I was considering the wait and pending functions as a utility\n> that I can reuse towards fixing CRS in other conditions like secondary\n> bus reset and potentially PM.\n> \n> I'm planning to have a CRS session in the Linux Plumbers Conference \n> to talk about CRS use cases. \n\nGreat idea. I'm kind of confused on its value in general myself. And\nthere's now a new mechanism (Function Readiness Status) that I don't\nthink we have any support for at all.\n\n> I was going to follow up this series with secondary bus reset next once\n> this goes in. \n\nI think I'm OK with everything except the pci_flr_wait() change. The\nrest of it makes sense (although I don't think there are any users\noutside probe.c, so maybe should be static for now).\n\n> I'm unable to test PM. I can't promise how I fix/test it.","diff":"diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c\nindex af0cc3456dc1..b04c99e60b77 100644\n--- a/drivers/pci/pci.c\n+++ b/drivers/pci/pci.c\n@@ -3811,27 +3811,50 @@ int pci_wait_for_pending_transaction(struct pci_dev *dev)\n }\n EXPORT_SYMBOL(pci_wait_for_pending_transaction);\n \n-/*\n- * We should only need to wait 100ms after FLR, but some devices take longer.\n- * Wait for up to 1000ms for config space to return something other than -1.\n- * Intel IGD requires this when an LCD panel is attached. We read the 2nd\n- * dword because VFs don't implement the 1st dword.\n- */\n static void pci_flr_wait(struct pci_dev *dev)\n {\n-\tint i = 0;\n+\tint delay = 1, timeout = 60000;\n \tu32 id;\n \n-\tdo {\n-\t\tmsleep(100);\n+\t/*\n+\t * Per PCIe r3.1, sec 6.6.2, a device must complete an FLR within\n+\t * 100ms, but may silently discard requests while the FLR is in\n+\t * progress. Wait 100ms before trying to access the device.\n+\t */\n+\tmsleep(100);\n+\n+\t/*\n+\t * After 100ms, the device should not silently discard config\n+\t * requests, but it may still indicate that it needs more time by\n+\t * responding to them with CRS completions. The Root Port will\n+\t * generally synthesize ~0 data to complete the read (except when\n+\t * CRS SV is enabled and the read was for the Vendor ID; in that\n+\t * case it synthesizes 0x0001 data).\n+\t *\n+\t * Wait for the device to return a non-CRS completion. Read the\n+\t * Command register instead of Vendor ID so we don't have to\n+\t * contend with the CRS SV value.\n+\t */\n+\tpci_read_config_dword(dev, PCI_COMMAND, &id);\n+\twhile (id == ~0) {\n+\t\tif (delay > timeout) {\n+\t\t\tdev_warn(&dev->dev, \"not ready %dms after FLR; giving up\\n\",\n+\t\t\t\t delay - 1);\n+\t\t\treturn;\n+\t\t}\n+\n+\t\tif (delay > 1000)\n+\t\t\tdev_info(&dev->dev, \"not ready %dms after FLR; waiting\\n\",\n+\t\t\t\t delay - 1);\n+\n+\t\tmsleep(delay);\n+\t\tdelay *= 2;\n+\n \t\tpci_read_config_dword(dev, PCI_COMMAND, &id);\n-\t} while (i++ < 10 && id == ~0);\n+\t}\n \n-\tif (id == ~0)\n-\t\tdev_warn(&dev->dev, \"Failed to return from FLR\\n\");\n-\telse if (i > 1)\n-\t\tdev_info(&dev->dev, \"Required additional %dms to return from FLR\\n\",\n-\t\t\t (i - 1) * 100);\n+\tif (delay > 1000)\n+\t\tdev_info(&dev->dev, \"ready %dms after FLR\\n\", delay - 1);\n }\n \n /**\n","prefixes":["V12","4/5"]}