[{"id":1758801,"web_url":"http://patchwork.ozlabs.org/comment/1758801/","msgid":"<20170828191708.GJ8154@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-08-28T19:17:09","subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Thu, Aug 24, 2017 at 10:34:25AM +0530, Oza Pawandeep wrote:\n> PCIe spec r3.1, sec 2.3.2\n> If CRS software visibility is not enabled, the RC must reissue the\n> config request as a new request.\n> \n> - If CRS software visibility is enabled,\n> - for a config read of Vendor ID, the RC must return 0x0001 data\n> - for all other config reads/writes, the RC must reissue the\n>   request\n> \n> iproc PCIe Controller spec:\n> 4.7.3.3. Retry Status On Configuration Cycle\n> Endpoints are allowed to generate retry status on configuration\n> cycles. In this case, the RC needs to re-issue the request. The IP\n> does not handle this because the number of configuration cycles needed\n> will probably be less than the total number of non-posted operations\n> needed.\n> \n> When a retry status is received on the User RX interface for a\n> configuration request that was sent on the User TX interface,\n> it will be indicated with a completion with the CMPL_STATUS field set\n> to 2=CRS, and the user will have to find the address and data values\n> and send a new transaction on the User TX interface.\n> When the internal configuration space returns a retry status during a\n> configuration cycle (user_cscfg = 1) on the Command/Status interface,\n> the pcie_cscrs will assert with the pcie_csack signal to indicate the\n> CRS status.\n> When the CRS Software Visibility Enable register in the Root Control\n> register is enabled, the IP will return the data value to 0x0001 for\n> the Vendor ID value and 0xffff  (all 1’s) for the rest of the data in\n> the request for reads of offset 0 that return with CRS status.  This\n> is true for both the User RX Interface and for the Command/Status\n> interface.  When CRS Software Visibility is enabled, the CMPL_STATUS\n> field of the completion on the User RX Interface will not be 2=CRS and\n> the pcie_cscrs signal will not assert on the Command/Status interface.\n> \n> Per PCIe r3.1, sec 2.3.2, config requests that receive completions\n> with Configuration Request Retry Status (CRS) should be reissued by\n> the hardware except reads of the Vendor ID when CRS Software\n> Visibility is enabled.\n> \n> This hardware never reissues configuration requests when it receives\n> CRS completions.\n> Note that, neither PCIe host bridge nor PCIe core re-issues the\n> request for any configuration offset.\n> \n> For config reads, this hardware returns CFG_RETRY_STATUS data when\n> it receives a CRS completion for a config read, regardless of the\n> address of the read or the CRS Software Visibility Enable bit.\n\nI can't remember how Stingray handles the CRS Software Visibility Enable\nbit.  Is it a read-only zero?  Is it writable?  Does the hardware look at\nit all (I think not)?","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","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=<UNKNOWN>)","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 3xh1jp6myXz9sN7\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 05:17:16 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751207AbdH1TRL (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 28 Aug 2017 15:17:11 -0400","from mail.kernel.org ([198.145.29.99]:39682 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751190AbdH1TRK (ORCPT <rfc822;linux-pci@vger.kernel.org>);\n\tMon, 28 Aug 2017 15:17:10 -0400","from localhost (unknown [69.55.156.165])\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 135E121A2F;\n\tMon, 28 Aug 2017 19:17:10 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 135E121A2F","Date":"Mon, 28 Aug 2017 14:17:09 -0500","From":"Bjorn Helgaas <helgaas@kernel.org>","To":"Oza Pawandeep <oza.oza@broadcom.com>","Cc":"Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>,\n\tMark Rutland <mark.rutland@arm.com>, Ray Jui <rjui@broadcom.com>,\n\tScott Branden <sbranden@broadcom.com>, Jon Mason <jonmason@broadcom.com>,\n\tbcm-kernel-feedback-list@broadcom.com,\n\tAndy Gospodarek <gospo@broadcom.com>,\n\tlinux-pci@vger.kernel.org, devicetree@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,\n\tOza Pawandeep <oza.pawandeep@gmail.com>","Subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","Message-ID":"<20170828191708.GJ8154@bhelgaas-glaptop.roam.corp.google.com>","References":"<1503551066-23212-1-git-send-email-oza.oza@broadcom.com>\n\t<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1758807,"web_url":"http://patchwork.ozlabs.org/comment/1758807/","msgid":"<CAMSpPPf5LJqvjFdrsR0hXK7oq2n38zCUjEpdmV0m8h6R_oiEXg@mail.gmail.com>","list_archive_url":null,"date":"2017-08-28T19:39:53","subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","submitter":{"id":71219,"url":"http://patchwork.ozlabs.org/api/people/71219/","name":"Oza Pawandeep","email":"oza.oza@broadcom.com"},"content":"On Tue, Aug 29, 2017 at 12:47 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:\n> On Thu, Aug 24, 2017 at 10:34:25AM +0530, Oza Pawandeep wrote:\n>> PCIe spec r3.1, sec 2.3.2\n>> If CRS software visibility is not enabled, the RC must reissue the\n>> config request as a new request.\n>>\n>> - If CRS software visibility is enabled,\n>> - for a config read of Vendor ID, the RC must return 0x0001 data\n>> - for all other config reads/writes, the RC must reissue the\n>>   request\n>>\n>> iproc PCIe Controller spec:\n>> 4.7.3.3. Retry Status On Configuration Cycle\n>> Endpoints are allowed to generate retry status on configuration\n>> cycles. In this case, the RC needs to re-issue the request. The IP\n>> does not handle this because the number of configuration cycles needed\n>> will probably be less than the total number of non-posted operations\n>> needed.\n>>\n>> When a retry status is received on the User RX interface for a\n>> configuration request that was sent on the User TX interface,\n>> it will be indicated with a completion with the CMPL_STATUS field set\n>> to 2=CRS, and the user will have to find the address and data values\n>> and send a new transaction on the User TX interface.\n>> When the internal configuration space returns a retry status during a\n>> configuration cycle (user_cscfg = 1) on the Command/Status interface,\n>> the pcie_cscrs will assert with the pcie_csack signal to indicate the\n>> CRS status.\n>> When the CRS Software Visibility Enable register in the Root Control\n>> register is enabled, the IP will return the data value to 0x0001 for\n>> the Vendor ID value and 0xffff  (all 1’s) for the rest of the data in\n>> the request for reads of offset 0 that return with CRS status.  This\n>> is true for both the User RX Interface and for the Command/Status\n>> interface.  When CRS Software Visibility is enabled, the CMPL_STATUS\n>> field of the completion on the User RX Interface will not be 2=CRS and\n>> the pcie_cscrs signal will not assert on the Command/Status interface.\n>>\n>> Per PCIe r3.1, sec 2.3.2, config requests that receive completions\n>> with Configuration Request Retry Status (CRS) should be reissued by\n>> the hardware except reads of the Vendor ID when CRS Software\n>> Visibility is enabled.\n>>\n>> This hardware never reissues configuration requests when it receives\n>> CRS completions.\n>> Note that, neither PCIe host bridge nor PCIe core re-issues the\n>> request for any configuration offset.\n>>\n>> For config reads, this hardware returns CFG_RETRY_STATUS data when\n>> it receives a CRS completion for a config read, regardless of the\n>> address of the read or the CRS Software Visibility Enable bit.\n>\n> I can't remember how Stingray handles the CRS Software Visibility Enable\n> bit.  Is it a read-only zero?  Is it writable?  Does the hardware look at\n> it all (I think not)?\n\nHW doesnt look it at all, it is \"dont care\" bit.\n\nRegards,\nOza.","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","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=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=broadcom.com header.i=@broadcom.com\n\theader.b=\"DnioOpSc\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xh2D432DZz9s03\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 05:40:04 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751258AbdH1TkB (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 28 Aug 2017 15:40:01 -0400","from mail-pf0-f174.google.com ([209.85.192.174]:36747 \"EHLO\n\tmail-pf0-f174.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751373AbdH1Tjy (ORCPT\n\t<rfc822; linux-pci@vger.kernel.org>); Mon, 28 Aug 2017 15:39:54 -0400","by mail-pf0-f174.google.com with SMTP id z87so3943713pfi.3\n\tfor <linux-pci@vger.kernel.org>; Mon, 28 Aug 2017 12:39:54 -0700 (PDT)","by 10.100.128.26 with HTTP; Mon, 28 Aug 2017 12:39:53 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=broadcom.com; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=TlNTNXhWwrumJntBE7xsHBh3/gFi/sWRexre541BOg8=;\n\tb=DnioOpScQnVoYlWOj6cVyZv8Lxz9TftmdNbcPJpSnFzRakqDs1K9ngvtThXDhamTz2\n\tMitzPbYNR0AIKfDXuTRvQUj0V8YNTRXEUWTusb5o0YQmPkuxSQyw9jwCgVswLUYuBOrw\n\tC9jmyVTDbba7GMSfZ6OkBv4RvrCoz6J1LrYg4=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=TlNTNXhWwrumJntBE7xsHBh3/gFi/sWRexre541BOg8=;\n\tb=qvIBeeYdcjU6CbZGCeM7v3X3ySpDkouq8OcmweVm8hIB6FZ/EeB+CcLpJ8FevnxMz/\n\tc4WK0yV/w7EPcXIQtFCGSdhSmFCKQ/nYhrTFUz6Flc/q4dpxI/ZwbqYO5Py1r6ZXg9uZ\n\tz7d6w9zkxXCwF/rKQH6Jk4+2kKZCWXiqJVxC/xH1mpswhJfjy8rGhddrKpkiy4OHi3Kz\n\tGYcx9mMVrPp3n+VrVM5HaA0Rw4Clo0uGzHiFs/ayEqpX26Y2N6JJCTQ7ujr9MlV/lDIE\n\t90/JGzTLdLTjEFTbNbVSXXFZO/g66vgthEICDb4s2394pHnxbzEMAO5xgZYPSxmkB1Jl\n\tw3Ng==","X-Gm-Message-State":"AHYfb5jacTxpMeDnHD10+lUeh3XkmMXpNRFfZEwhXsYSb2DSS6Yu7V41\n\tfUH4YDQ+Gm4RtXJX6RiiPtMlUBNc/fQ2","X-Received":"by 10.84.217.89 with SMTP id e25mr1987513plj.117.1503949194108; \n\tMon, 28 Aug 2017 12:39:54 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170828191708.GJ8154@bhelgaas-glaptop.roam.corp.google.com>","References":"<1503551066-23212-1-git-send-email-oza.oza@broadcom.com>\n\t<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>\n\t<20170828191708.GJ8154@bhelgaas-glaptop.roam.corp.google.com>","From":"Oza Oza <oza.oza@broadcom.com>","Date":"Tue, 29 Aug 2017 01:09:53 +0530","Message-ID":"<CAMSpPPf5LJqvjFdrsR0hXK7oq2n38zCUjEpdmV0m8h6R_oiEXg@mail.gmail.com>","Subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","To":"Bjorn Helgaas <helgaas@kernel.org>","Cc":"Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>,\n\tMark Rutland <mark.rutland@arm.com>, Ray Jui <rjui@broadcom.com>,\n\tScott Branden <sbranden@broadcom.com>, Jon Mason <jonmason@broadcom.com>,\n\tBCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,\n\tAndy Gospodarek <gospo@broadcom.com>,\n\tlinux-pci <linux-pci@vger.kernel.org>, devicetree@vger.kernel.org,\n\tLinux ARM <linux-arm-kernel@lists.infradead.org>,\n\tlinux-kernel@vger.kernel.org, Oza Pawandeep <oza.pawandeep@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1758813,"web_url":"http://patchwork.ozlabs.org/comment/1758813/","msgid":"<20170828195402.GK8154@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-08-28T19:54:02","subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Tue, Aug 29, 2017 at 01:09:53AM +0530, Oza Oza wrote:\n> On Tue, Aug 29, 2017 at 12:47 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:\n> > On Thu, Aug 24, 2017 at 10:34:25AM +0530, Oza Pawandeep wrote:\n> >> PCIe spec r3.1, sec 2.3.2\n> >> If CRS software visibility is not enabled, the RC must reissue the\n> >> config request as a new request.\n> >>\n> >> - If CRS software visibility is enabled,\n> >> - for a config read of Vendor ID, the RC must return 0x0001 data\n> >> - for all other config reads/writes, the RC must reissue the\n> >>   request\n> >>\n> >> iproc PCIe Controller spec:\n> >> 4.7.3.3. Retry Status On Configuration Cycle\n> >> Endpoints are allowed to generate retry status on configuration\n> >> cycles. In this case, the RC needs to re-issue the request. The IP\n> >> does not handle this because the number of configuration cycles needed\n> >> will probably be less than the total number of non-posted operations\n> >> needed.\n> >>\n> >> When a retry status is received on the User RX interface for a\n> >> configuration request that was sent on the User TX interface,\n> >> it will be indicated with a completion with the CMPL_STATUS field set\n> >> to 2=CRS, and the user will have to find the address and data values\n> >> and send a new transaction on the User TX interface.\n> >> When the internal configuration space returns a retry status during a\n> >> configuration cycle (user_cscfg = 1) on the Command/Status interface,\n> >> the pcie_cscrs will assert with the pcie_csack signal to indicate the\n> >> CRS status.\n> >> When the CRS Software Visibility Enable register in the Root Control\n> >> register is enabled, the IP will return the data value to 0x0001 for\n> >> the Vendor ID value and 0xffff  (all 1’s) for the rest of the data in\n> >> the request for reads of offset 0 that return with CRS status.  This\n> >> is true for both the User RX Interface and for the Command/Status\n> >> interface.  When CRS Software Visibility is enabled, the CMPL_STATUS\n> >> field of the completion on the User RX Interface will not be 2=CRS and\n> >> the pcie_cscrs signal will not assert on the Command/Status interface.\n> >>\n> >> Per PCIe r3.1, sec 2.3.2, config requests that receive completions\n> >> with Configuration Request Retry Status (CRS) should be reissued by\n> >> the hardware except reads of the Vendor ID when CRS Software\n> >> Visibility is enabled.\n> >>\n> >> This hardware never reissues configuration requests when it receives\n> >> CRS completions.\n> >> Note that, neither PCIe host bridge nor PCIe core re-issues the\n> >> request for any configuration offset.\n> >>\n> >> For config reads, this hardware returns CFG_RETRY_STATUS data when\n> >> it receives a CRS completion for a config read, regardless of the\n> >> address of the read or the CRS Software Visibility Enable bit.\n> >\n> > I can't remember how Stingray handles the CRS Software Visibility Enable\n> > bit.  Is it a read-only zero?  Is it writable?  Does the hardware look at\n> > it all (I think not)?\n> \n> HW doesnt look it at all, it is \"dont care\" bit.\n\nSigh, I made the classic mistake of asking more than one question, and\nI guess I'm about to do it again :)  It'll save time if you can answer\nall the questions at once.\n\nLinux enables PCI_EXP_RTCTL_CRSSVE if PCI_EXP_RTCAP says it is\nsupported (see pci_enable_crs()).\n\n  - What does PCI_EXP_RTCAP say?\n\n  - Is PCI_EXP_RTCTL_CRSSVE writable?\n\nWith all the CRS-related work going on, I suspect we may someday want to\nread PCI_EXP_RTCTL_CRSSVE to see if CRS SV is enabled.\n\nBjorn","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","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=<UNKNOWN>)","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 3xh2XT04CTz9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 05:54:17 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751284AbdH1TyF (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 28 Aug 2017 15:54:05 -0400","from mail.kernel.org ([198.145.29.99]:41348 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751218AbdH1TyE (ORCPT <rfc822;linux-pci@vger.kernel.org>);\n\tMon, 28 Aug 2017 15:54:04 -0400","from localhost (unknown [69.55.156.165])\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 8A99521A2F;\n\tMon, 28 Aug 2017 19:54:03 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 8A99521A2F","Date":"Mon, 28 Aug 2017 14:54:02 -0500","From":"Bjorn Helgaas <helgaas@kernel.org>","To":"Oza Oza <oza.oza@broadcom.com>","Cc":"Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>,\n\tMark Rutland <mark.rutland@arm.com>, Ray Jui <rjui@broadcom.com>,\n\tScott Branden <sbranden@broadcom.com>, Jon Mason <jonmason@broadcom.com>,\n\tBCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,\n\tAndy Gospodarek <gospo@broadcom.com>,\n\tlinux-pci <linux-pci@vger.kernel.org>, devicetree@vger.kernel.org,\n\tLinux ARM <linux-arm-kernel@lists.infradead.org>,\n\tlinux-kernel@vger.kernel.org, Oza Pawandeep <oza.pawandeep@gmail.com>","Subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","Message-ID":"<20170828195402.GK8154@bhelgaas-glaptop.roam.corp.google.com>","References":"<1503551066-23212-1-git-send-email-oza.oza@broadcom.com>\n\t<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>\n\t<20170828191708.GJ8154@bhelgaas-glaptop.roam.corp.google.com>\n\t<CAMSpPPf5LJqvjFdrsR0hXK7oq2n38zCUjEpdmV0m8h6R_oiEXg@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<CAMSpPPf5LJqvjFdrsR0hXK7oq2n38zCUjEpdmV0m8h6R_oiEXg@mail.gmail.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1758839,"web_url":"http://patchwork.ozlabs.org/comment/1758839/","msgid":"<CAMSpPPddi7sCk6_tDTZZx2SWetmopOgm23QVXNqjOAX=PNp9iA@mail.gmail.com>","list_archive_url":null,"date":"2017-08-28T20:45:42","subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","submitter":{"id":71219,"url":"http://patchwork.ozlabs.org/api/people/71219/","name":"Oza Pawandeep","email":"oza.oza@broadcom.com"},"content":"On Tue, Aug 29, 2017 at 1:24 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:\n> On Tue, Aug 29, 2017 at 01:09:53AM +0530, Oza Oza wrote:\n>> On Tue, Aug 29, 2017 at 12:47 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:\n>> > On Thu, Aug 24, 2017 at 10:34:25AM +0530, Oza Pawandeep wrote:\n>> >> PCIe spec r3.1, sec 2.3.2\n>> >> If CRS software visibility is not enabled, the RC must reissue the\n>> >> config request as a new request.\n>> >>\n>> >> - If CRS software visibility is enabled,\n>> >> - for a config read of Vendor ID, the RC must return 0x0001 data\n>> >> - for all other config reads/writes, the RC must reissue the\n>> >>   request\n>> >>\n>> >> iproc PCIe Controller spec:\n>> >> 4.7.3.3. Retry Status On Configuration Cycle\n>> >> Endpoints are allowed to generate retry status on configuration\n>> >> cycles. In this case, the RC needs to re-issue the request. The IP\n>> >> does not handle this because the number of configuration cycles needed\n>> >> will probably be less than the total number of non-posted operations\n>> >> needed.\n>> >>\n>> >> When a retry status is received on the User RX interface for a\n>> >> configuration request that was sent on the User TX interface,\n>> >> it will be indicated with a completion with the CMPL_STATUS field set\n>> >> to 2=CRS, and the user will have to find the address and data values\n>> >> and send a new transaction on the User TX interface.\n>> >> When the internal configuration space returns a retry status during a\n>> >> configuration cycle (user_cscfg = 1) on the Command/Status interface,\n>> >> the pcie_cscrs will assert with the pcie_csack signal to indicate the\n>> >> CRS status.\n>> >> When the CRS Software Visibility Enable register in the Root Control\n>> >> register is enabled, the IP will return the data value to 0x0001 for\n>> >> the Vendor ID value and 0xffff  (all 1’s) for the rest of the data in\n>> >> the request for reads of offset 0 that return with CRS status.  This\n>> >> is true for both the User RX Interface and for the Command/Status\n>> >> interface.  When CRS Software Visibility is enabled, the CMPL_STATUS\n>> >> field of the completion on the User RX Interface will not be 2=CRS and\n>> >> the pcie_cscrs signal will not assert on the Command/Status interface.\n>> >>\n>> >> Per PCIe r3.1, sec 2.3.2, config requests that receive completions\n>> >> with Configuration Request Retry Status (CRS) should be reissued by\n>> >> the hardware except reads of the Vendor ID when CRS Software\n>> >> Visibility is enabled.\n>> >>\n>> >> This hardware never reissues configuration requests when it receives\n>> >> CRS completions.\n>> >> Note that, neither PCIe host bridge nor PCIe core re-issues the\n>> >> request for any configuration offset.\n>> >>\n>> >> For config reads, this hardware returns CFG_RETRY_STATUS data when\n>> >> it receives a CRS completion for a config read, regardless of the\n>> >> address of the read or the CRS Software Visibility Enable bit.\n>> >\n>> > I can't remember how Stingray handles the CRS Software Visibility Enable\n>> > bit.  Is it a read-only zero?  Is it writable?  Does the hardware look at\n>> > it all (I think not)?\n>>\n>> HW doesnt look it at all, it is \"dont care\" bit.\n>\n> Sigh, I made the classic mistake of asking more than one question, and\n> I guess I'm about to do it again :)  It'll save time if you can answer\n> all the questions at once.\n>\n> Linux enables PCI_EXP_RTCTL_CRSSVE if PCI_EXP_RTCAP says it is\n> supported (see pci_enable_crs()).\n>\n>   - What does PCI_EXP_RTCAP say?\nit is rea as set: value :0x1\n>\n>   - Is PCI_EXP_RTCTL_CRSSVE writable?\nyes: it is:\nread as 0 and written as 1.\n>\n> With all the CRS-related work going on, I suspect we may someday want to\n> read PCI_EXP_RTCTL_CRSSVE to see if CRS SV is enabled.\n>\n> Bjorn","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","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=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=broadcom.com header.i=@broadcom.com\n\theader.b=\"QIrbfU4F\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xh3jL1hChz9sNn\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 06:47:02 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751258AbdH1Upp (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 28 Aug 2017 16:45:45 -0400","from mail-pg0-f50.google.com ([74.125.83.50]:37358 \"EHLO\n\tmail-pg0-f50.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751182AbdH1Upn (ORCPT\n\t<rfc822; linux-pci@vger.kernel.org>); Mon, 28 Aug 2017 16:45:43 -0400","by mail-pg0-f50.google.com with SMTP id 83so4613430pgb.4\n\tfor <linux-pci@vger.kernel.org>; Mon, 28 Aug 2017 13:45:43 -0700 (PDT)","by 10.100.128.26 with HTTP; Mon, 28 Aug 2017 13:45:42 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=broadcom.com; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=G5uAw1AiOa6bGBlHRADH/F+i2SQLpyXMPhdBS4vxYB0=;\n\tb=QIrbfU4FVupgogjlykcBgBQU/OjUfbSJm6e13x1S+NJBy7yF7J3pp43+qI6/18u0aW\n\txUZ5gGG17gBPcXrhnHIBS3aXxMAzW2iK71Cl0w0LTf0fkwgy8eTrZpdEgY+dAY+7kIEK\n\tc6e4M7SKGpMrbrQi7Ge9kR8gTM8y73T83yLgA=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=G5uAw1AiOa6bGBlHRADH/F+i2SQLpyXMPhdBS4vxYB0=;\n\tb=Laja52F1/QZqTV8A6O6Wy4iy7+2gd8B/b99VSNB5N5ZjxwHhXFr9vqi05gl7XK0mTY\n\t8GeDLwtJRBWUYU66azzCavY3+kJK0J/KcmCUJmHog0/moTX2rK9gBZ7/e94488sEZ/0p\n\tEDEGJFL0XWHICZhVg8DqWltg2gsRUCAYtzuakqOeqIMaEHM4yCu5XpDPAi5qSr+iWWVo\n\t4VHcWH4UEDhVTc4SYhgZFNYpTBcfXl+TxY7lXhRZzlwYXy4dMqZk/dm060dalW54axUY\n\then97diT6drbt923pe4LLvSBYPHS8IOMdxIp/qYApqAkMTdnrm5TIrWfWqKAIoRv+6xg\n\tuVlw==","X-Gm-Message-State":"AHYfb5hfXj91EQoaHT+PAcjFJiBMwWRmGRWk6DWzJ2gpq3qFR20Qqxq4\n\tlXNVIzPlT8oZWu1qUuBJEZxCphNH46Ez","X-Received":"by 10.84.211.99 with SMTP id b90mr2187287pli.450.1503953143379; \n\tMon, 28 Aug 2017 13:45:43 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170828195402.GK8154@bhelgaas-glaptop.roam.corp.google.com>","References":"<1503551066-23212-1-git-send-email-oza.oza@broadcom.com>\n\t<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>\n\t<20170828191708.GJ8154@bhelgaas-glaptop.roam.corp.google.com>\n\t<CAMSpPPf5LJqvjFdrsR0hXK7oq2n38zCUjEpdmV0m8h6R_oiEXg@mail.gmail.com>\n\t<20170828195402.GK8154@bhelgaas-glaptop.roam.corp.google.com>","From":"Oza Oza <oza.oza@broadcom.com>","Date":"Tue, 29 Aug 2017 02:15:42 +0530","Message-ID":"<CAMSpPPddi7sCk6_tDTZZx2SWetmopOgm23QVXNqjOAX=PNp9iA@mail.gmail.com>","Subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","To":"Bjorn Helgaas <helgaas@kernel.org>","Cc":"Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>,\n\tMark Rutland <mark.rutland@arm.com>, Ray Jui <rjui@broadcom.com>,\n\tScott Branden <sbranden@broadcom.com>, Jon Mason <jonmason@broadcom.com>,\n\tBCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,\n\tAndy Gospodarek <gospo@broadcom.com>,\n\tlinux-pci <linux-pci@vger.kernel.org>, devicetree@vger.kernel.org,\n\tLinux ARM <linux-arm-kernel@lists.infradead.org>,\n\tlinux-kernel@vger.kernel.org, Oza Pawandeep <oza.pawandeep@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1758864,"web_url":"http://patchwork.ozlabs.org/comment/1758864/","msgid":"<20170828214753.GL8154@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-08-28T21:47:53","subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Thu, Aug 24, 2017 at 10:34:25AM +0530, Oza Pawandeep wrote:\n> PCIe spec r3.1, sec 2.3.2\n> If CRS software visibility is not enabled, the RC must reissue the\n> config request as a new request.\n> \n> - If CRS software visibility is enabled,\n> - for a config read of Vendor ID, the RC must return 0x0001 data\n> - for all other config reads/writes, the RC must reissue the\n>   request\n> \n> iproc PCIe Controller spec:\n> 4.7.3.3. Retry Status On Configuration Cycle\n> Endpoints are allowed to generate retry status on configuration\n> cycles. In this case, the RC needs to re-issue the request. The IP\n> does not handle this because the number of configuration cycles needed\n> will probably be less than the total number of non-posted operations\n> needed.\n> \n> When a retry status is received on the User RX interface for a\n> configuration request that was sent on the User TX interface,\n> it will be indicated with a completion with the CMPL_STATUS field set\n> to 2=CRS, and the user will have to find the address and data values\n> and send a new transaction on the User TX interface.\n> When the internal configuration space returns a retry status during a\n> configuration cycle (user_cscfg = 1) on the Command/Status interface,\n> the pcie_cscrs will assert with the pcie_csack signal to indicate the\n> CRS status.\n> When the CRS Software Visibility Enable register in the Root Control\n> register is enabled, the IP will return the data value to 0x0001 for\n> the Vendor ID value and 0xffff  (all 1’s) for the rest of the data in\n> the request for reads of offset 0 that return with CRS status.  This\n> is true for both the User RX Interface and for the Command/Status\n> interface.  When CRS Software Visibility is enabled, the CMPL_STATUS\n> field of the completion on the User RX Interface will not be 2=CRS and\n> the pcie_cscrs signal will not assert on the Command/Status interface.\n> \n> Per PCIe r3.1, sec 2.3.2, config requests that receive completions\n> with Configuration Request Retry Status (CRS) should be reissued by\n> the hardware except reads of the Vendor ID when CRS Software\n> Visibility is enabled.\n> \n> This hardware never reissues configuration requests when it receives\n> CRS completions.\n> Note that, neither PCIe host bridge nor PCIe core re-issues the\n> request for any configuration offset.\n> \n> For config reads, this hardware returns CFG_RETRY_STATUS data when\n> it receives a CRS completion for a config read, regardless of the\n> address of the read or the CRS Software Visibility Enable bit.\n> \n> This patch implements iproc_pcie_config_read which gets called for\n> Stingray, if it receives a CRS completion, it retries reading it again.\n> In case of timeout, it returns 0xffffffff.\n> For other iproc based SOC, it falls back to PCI generic APIs.\n> \n> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>\n> \n> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c\n> index 61d9be6..37f4adf 100644\n> --- a/drivers/pci/host/pcie-iproc.c\n> +++ b/drivers/pci/host/pcie-iproc.c\n> @@ -68,6 +68,9 @@\n>  #define APB_ERR_EN_SHIFT             0\n>  #define APB_ERR_EN                   BIT(APB_ERR_EN_SHIFT)\n>  \n> +#define CFG_RETRY_STATUS             0xffff0001\n> +#define CFG_RETRY_STATUS_TIMEOUT_US  500000 /* 500 milli-seconds. */\n> +\n>  /* derive the enum index of the outbound/inbound mapping registers */\n>  #define MAP_REG(base_reg, index)      ((base_reg) + (index) * 2)\n>  \n> @@ -473,6 +476,64 @@ static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,\n>  \treturn (pcie->base + offset);\n>  }\n>  \n> +static unsigned int iproc_pcie_cfg_retry(void __iomem *cfg_data_p)\n> +{\n> +\tint timeout = CFG_RETRY_STATUS_TIMEOUT_US;\n> +\tunsigned int data;\n> +\n> +\t/*\n> +\t * As per PCIe spec r3.1, sec 2.3.2, CRS Software\n> +\t * Visibility only affects config read of the Vendor ID.\n> +\t * For config write or any other config read the Root must\n> +\t * automatically re-issue configuration request again as a\n> +\t * new request.\n> +\t *\n> +\t * For config reads, this hardware returns CFG_RETRY_STATUS data when\n> +\t * it receives a CRS completion for a config read, regardless of the\n> +\t * address of the read or the CRS Software Visibility Enable bit. As a\n> +\t * partial workaround for this, we retry in software any read that\n> +\t * returns CFG_RETRY_STATUS.\n> +\t */\n> +\tdata = readl(cfg_data_p);\n> +\twhile (data == CFG_RETRY_STATUS && timeout--) {\n> +\t\tudelay(1);\n> +\t\tdata = readl(cfg_data_p);\n> +\t}\n> +\n> +\tif (data == CFG_RETRY_STATUS)\n> +\t\tdata = 0xffffffff;\n> +\n> +\treturn data;\n> +}\n> +\n> +static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,\n> +\t\t\t\t    int where, int size, u32 *val)\n> +{\n> +\tstruct iproc_pcie *pcie = iproc_data(bus);\n> +\tunsigned int slot = PCI_SLOT(devfn);\n> +\tunsigned int fn = PCI_FUNC(devfn);\n> +\tunsigned int busno = bus->number;\n> +\tvoid __iomem *cfg_data_p;\n> +\tunsigned int data;\n> +\n> +\t/* root complex access. */\n> +\tif (busno == 0)\n> +\t\treturn pci_generic_config_read32(bus, devfn, where, size, val);\n\nIt sounds like Stingray advertises CRS SV support in its Root Capabilities\nregister.  I think we should mask out PCI_EXP_RTCAP_CRSVIS so we don't\nadvertise it.  That will keep Linux from trying to enable it.  I know the\nhardware doesn't look at PCI_EXP_RTCTL_CRSSVE, but there's no point in\nconfusing users reading the lspci output.\n\nWe did something similar with f09f8735fb9c (\"PCI: xgene: Disable\nConfiguration Request Retry Status for v1 silicon\").\n\nI tried to do this in the patch I pushed to pci/host-iproc.\n\n> +\n> +\tcfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);\n> +\n> +\tif (!cfg_data_p)\n> +\t\treturn PCIBIOS_DEVICE_NOT_FOUND;\n> +\n> +\tdata = iproc_pcie_cfg_retry(cfg_data_p);\n> +\n> +\t*val = data;\n> +\tif (size <= 2)\n> +\t\t*val = (data >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);\n> +\n> +\treturn PCIBIOS_SUCCESSFUL;\n> +}\n> +\n>  /**\n>   * Note access to the configuration registers are protected at the higher layer\n>   * by 'pci_lock' in drivers/pci/access.c\n> @@ -567,8 +628,13 @@ static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,\n>  \t\t\t\t    int where, int size, u32 *val)\n>  {\n>  \tint ret;\n> +\tstruct iproc_pcie *pcie = iproc_data(bus);\n>  \n>  \tiproc_pcie_apb_err_disable(bus, true);\n> +\tif (pcie->type == IPROC_PCIE_PAXB_V2)\n> +\t\tret = iproc_pcie_config_read(bus, devfn, where, size, val);\n> +\telse\n> +\t\tret = pci_generic_config_read32(bus, devfn, where, size, val);\n>  \tret = pci_generic_config_read32(bus, devfn, where, size, val);\n\nThis last pci_generic_config_read32() call looks like a duplicate.\n\n>  \tiproc_pcie_apb_err_disable(bus, false);\n>  \n> -- \n> 1.9.1\n>","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","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=<UNKNOWN>)","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 3xh5531jGmz9s4s\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 07:49:11 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751236AbdH1Vr4 (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 28 Aug 2017 17:47:56 -0400","from mail.kernel.org ([198.145.29.99]:48438 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751182AbdH1Vrz (ORCPT <rfc822;linux-pci@vger.kernel.org>);\n\tMon, 28 Aug 2017 17:47:55 -0400","from localhost (unknown [69.55.156.165])\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 47A9A2199F;\n\tMon, 28 Aug 2017 21:47:54 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 47A9A2199F","Date":"Mon, 28 Aug 2017 16:47:53 -0500","From":"Bjorn Helgaas <helgaas@kernel.org>","To":"Oza Pawandeep <oza.oza@broadcom.com>","Cc":"Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>,\n\tMark Rutland <mark.rutland@arm.com>, Ray Jui <rjui@broadcom.com>,\n\tScott Branden <sbranden@broadcom.com>, Jon Mason <jonmason@broadcom.com>,\n\tbcm-kernel-feedback-list@broadcom.com,\n\tAndy Gospodarek <gospo@broadcom.com>,\n\tlinux-pci@vger.kernel.org, devicetree@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,\n\tOza Pawandeep <oza.pawandeep@gmail.com>","Subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","Message-ID":"<20170828214753.GL8154@bhelgaas-glaptop.roam.corp.google.com>","References":"<1503551066-23212-1-git-send-email-oza.oza@broadcom.com>\n\t<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1759038,"web_url":"http://patchwork.ozlabs.org/comment/1759038/","msgid":"<CAMSpPPcFFULWZ+QTfzMSGhzQx0JboT618Uq6EA0=qPGpAK+h1w@mail.gmail.com>","list_archive_url":null,"date":"2017-08-29T05:32:23","subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","submitter":{"id":71219,"url":"http://patchwork.ozlabs.org/api/people/71219/","name":"Oza Pawandeep","email":"oza.oza@broadcom.com"},"content":"On Tue, Aug 29, 2017 at 3:17 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:\n> On Thu, Aug 24, 2017 at 10:34:25AM +0530, Oza Pawandeep wrote:\n>> PCIe spec r3.1, sec 2.3.2\n>> If CRS software visibility is not enabled, the RC must reissue the\n>> config request as a new request.\n>>\n>> - If CRS software visibility is enabled,\n>> - for a config read of Vendor ID, the RC must return 0x0001 data\n>> - for all other config reads/writes, the RC must reissue the\n>>   request\n>>\n>> iproc PCIe Controller spec:\n>> 4.7.3.3. Retry Status On Configuration Cycle\n>> Endpoints are allowed to generate retry status on configuration\n>> cycles. In this case, the RC needs to re-issue the request. The IP\n>> does not handle this because the number of configuration cycles needed\n>> will probably be less than the total number of non-posted operations\n>> needed.\n>>\n>> When a retry status is received on the User RX interface for a\n>> configuration request that was sent on the User TX interface,\n>> it will be indicated with a completion with the CMPL_STATUS field set\n>> to 2=CRS, and the user will have to find the address and data values\n>> and send a new transaction on the User TX interface.\n>> When the internal configuration space returns a retry status during a\n>> configuration cycle (user_cscfg = 1) on the Command/Status interface,\n>> the pcie_cscrs will assert with the pcie_csack signal to indicate the\n>> CRS status.\n>> When the CRS Software Visibility Enable register in the Root Control\n>> register is enabled, the IP will return the data value to 0x0001 for\n>> the Vendor ID value and 0xffff  (all 1’s) for the rest of the data in\n>> the request for reads of offset 0 that return with CRS status.  This\n>> is true for both the User RX Interface and for the Command/Status\n>> interface.  When CRS Software Visibility is enabled, the CMPL_STATUS\n>> field of the completion on the User RX Interface will not be 2=CRS and\n>> the pcie_cscrs signal will not assert on the Command/Status interface.\n>>\n>> Per PCIe r3.1, sec 2.3.2, config requests that receive completions\n>> with Configuration Request Retry Status (CRS) should be reissued by\n>> the hardware except reads of the Vendor ID when CRS Software\n>> Visibility is enabled.\n>>\n>> This hardware never reissues configuration requests when it receives\n>> CRS completions.\n>> Note that, neither PCIe host bridge nor PCIe core re-issues the\n>> request for any configuration offset.\n>>\n>> For config reads, this hardware returns CFG_RETRY_STATUS data when\n>> it receives a CRS completion for a config read, regardless of the\n>> address of the read or the CRS Software Visibility Enable bit.\n>>\n>> This patch implements iproc_pcie_config_read which gets called for\n>> Stingray, if it receives a CRS completion, it retries reading it again.\n>> In case of timeout, it returns 0xffffffff.\n>> For other iproc based SOC, it falls back to PCI generic APIs.\n>>\n>> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>\n>>\n>> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c\n>> index 61d9be6..37f4adf 100644\n>> --- a/drivers/pci/host/pcie-iproc.c\n>> +++ b/drivers/pci/host/pcie-iproc.c\n>> @@ -68,6 +68,9 @@\n>>  #define APB_ERR_EN_SHIFT             0\n>>  #define APB_ERR_EN                   BIT(APB_ERR_EN_SHIFT)\n>>\n>> +#define CFG_RETRY_STATUS             0xffff0001\n>> +#define CFG_RETRY_STATUS_TIMEOUT_US  500000 /* 500 milli-seconds. */\n>> +\n>>  /* derive the enum index of the outbound/inbound mapping registers */\n>>  #define MAP_REG(base_reg, index)      ((base_reg) + (index) * 2)\n>>\n>> @@ -473,6 +476,64 @@ static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,\n>>       return (pcie->base + offset);\n>>  }\n>>\n>> +static unsigned int iproc_pcie_cfg_retry(void __iomem *cfg_data_p)\n>> +{\n>> +     int timeout = CFG_RETRY_STATUS_TIMEOUT_US;\n>> +     unsigned int data;\n>> +\n>> +     /*\n>> +      * As per PCIe spec r3.1, sec 2.3.2, CRS Software\n>> +      * Visibility only affects config read of the Vendor ID.\n>> +      * For config write or any other config read the Root must\n>> +      * automatically re-issue configuration request again as a\n>> +      * new request.\n>> +      *\n>> +      * For config reads, this hardware returns CFG_RETRY_STATUS data when\n>> +      * it receives a CRS completion for a config read, regardless of the\n>> +      * address of the read or the CRS Software Visibility Enable bit. As a\n>> +      * partial workaround for this, we retry in software any read that\n>> +      * returns CFG_RETRY_STATUS.\n>> +      */\n>> +     data = readl(cfg_data_p);\n>> +     while (data == CFG_RETRY_STATUS && timeout--) {\n>> +             udelay(1);\n>> +             data = readl(cfg_data_p);\n>> +     }\n>> +\n>> +     if (data == CFG_RETRY_STATUS)\n>> +             data = 0xffffffff;\n>> +\n>> +     return data;\n>> +}\n>> +\n>> +static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,\n>> +                                 int where, int size, u32 *val)\n>> +{\n>> +     struct iproc_pcie *pcie = iproc_data(bus);\n>> +     unsigned int slot = PCI_SLOT(devfn);\n>> +     unsigned int fn = PCI_FUNC(devfn);\n>> +     unsigned int busno = bus->number;\n>> +     void __iomem *cfg_data_p;\n>> +     unsigned int data;\n>> +\n>> +     /* root complex access. */\n>> +     if (busno == 0)\n>> +             return pci_generic_config_read32(bus, devfn, where, size, val);\n>\n> It sounds like Stingray advertises CRS SV support in its Root Capabilities\n> register.  I think we should mask out PCI_EXP_RTCAP_CRSVIS so we don't\n> advertise it.  That will keep Linux from trying to enable it.  I know the\n> hardware doesn't look at PCI_EXP_RTCTL_CRSSVE, but there's no point in\n> confusing users reading the lspci output.\n>\n> We did something similar with f09f8735fb9c (\"PCI: xgene: Disable\n> Configuration Request Retry Status for v1 silicon\").\n>\n> I tried to do this in the patch I pushed to pci/host-iproc.\n>\n>> +\n>> +     cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);\n>> +\n>> +     if (!cfg_data_p)\n>> +             return PCIBIOS_DEVICE_NOT_FOUND;\n>> +\n>> +     data = iproc_pcie_cfg_retry(cfg_data_p);\n>> +\n>> +     *val = data;\n>> +     if (size <= 2)\n>> +             *val = (data >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);\n>> +\n>> +     return PCIBIOS_SUCCESSFUL;\n>> +}\n>> +\n>>  /**\n>>   * Note access to the configuration registers are protected at the higher layer\n>>   * by 'pci_lock' in drivers/pci/access.c\n>> @@ -567,8 +628,13 @@ static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,\n>>                                   int where, int size, u32 *val)\n>>  {\n>>       int ret;\n>> +     struct iproc_pcie *pcie = iproc_data(bus);\n>>\n>>       iproc_pcie_apb_err_disable(bus, true);\n>> +     if (pcie->type == IPROC_PCIE_PAXB_V2)\n>> +             ret = iproc_pcie_config_read(bus, devfn, where, size, val);\n>> +     else\n>> +             ret = pci_generic_config_read32(bus, devfn, where, size, val);\n>>       ret = pci_generic_config_read32(bus, devfn, where, size, val);\n>\n> This last pci_generic_config_read32() call looks like a duplicate.\n\nyes indeed.\nI have tested your CRS visibility bit changes; and it works fine.\n\ndo you want me to post new patch-set by removing the duplicate call\nalong with the changes you have made ?\n\nor since, you have already applied patches, with your changes, you\nwill take care of removing this last duplicate call ?\nI think this is the last change for this patch-set, If I did not miss anything.\n\nplease let me know.\n\n>\n>>       iproc_pcie_apb_err_disable(bus, false);\n>>\n>> --\n>> 1.9.1\n>>","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","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=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=broadcom.com header.i=@broadcom.com\n\theader.b=\"Nsfbp2RU\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhHMd5J7fz9t3B\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 15:32:29 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751319AbdH2FcZ (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tTue, 29 Aug 2017 01:32:25 -0400","from mail-pf0-f173.google.com ([209.85.192.173]:34112 \"EHLO\n\tmail-pf0-f173.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751316AbdH2FcY (ORCPT\n\t<rfc822; linux-pci@vger.kernel.org>); Tue, 29 Aug 2017 01:32:24 -0400","by mail-pf0-f173.google.com with SMTP id h75so7450383pfh.1\n\tfor <linux-pci@vger.kernel.org>; Mon, 28 Aug 2017 22:32:24 -0700 (PDT)","by 10.100.128.26 with HTTP; Mon, 28 Aug 2017 22:32:23 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=broadcom.com; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=Vphl5oBNUzzJTmbZRROYu6y9tDpkTrqu+EZ28Vi8jFk=;\n\tb=Nsfbp2RU3HtmGegJPoLTX6nHS2tyeX2fPfKFYd6AURpTNAhrCWVWVyIMixJIRS7wcb\n\tgf+J7v61t57E6s8b/cr5OyeaH7q1pz0AoEQ6SQqiP+7QWnAQCrKtOGZMLHLo8QdczUgs\n\ten21WVx4m9OLKmlvdq0Vw7xlYy9B1wp760vIk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=Vphl5oBNUzzJTmbZRROYu6y9tDpkTrqu+EZ28Vi8jFk=;\n\tb=hIS9/f5z+4Uhvwov0HaCcEpSw5vNihk/5nU/v+MlrD9vZRlLi+Yb82ucvhNl2qlYnb\n\tQxVyohISrCaOyFxDNgb9CDgaCSQgG7YeBIHarlv/QvXV5uFrHSFMVVwtCtDFhRQ87mzv\n\tKUXs/1VTubgjft68L6H/wHrl01thmuxuO35gsycCyQkRAmw/nvyblnNA5SQYNzAcce9L\n\t1EkX3LpxKjHLMJLa6tttFS4I5nXEnvImakdIPQVlyVfrKzlcgzTgXb+hcj8uGanC9gOo\n\tY0VY0Nj0hUcSEWi64CNtxk9o+WCiYEoInxakez0MWzVtYLNACGpUdI0BsbFMh9E5OaAD\n\trv8Q==","X-Gm-Message-State":"AHYfb5gj149zpG3tXcidA8QrZ/5vwzNI6H0wxV/ua7QV4jW+R36AA7nC\n\tTUz0Gh00RgE9LTqhhy0oEBX8XQcfsYbt","X-Received":"by 10.84.175.195 with SMTP id t61mr3613040plb.254.1503984743929; \n\tMon, 28 Aug 2017 22:32:23 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170828214753.GL8154@bhelgaas-glaptop.roam.corp.google.com>","References":"<1503551066-23212-1-git-send-email-oza.oza@broadcom.com>\n\t<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>\n\t<20170828214753.GL8154@bhelgaas-glaptop.roam.corp.google.com>","From":"Oza Oza <oza.oza@broadcom.com>","Date":"Tue, 29 Aug 2017 11:02:23 +0530","Message-ID":"<CAMSpPPcFFULWZ+QTfzMSGhzQx0JboT618Uq6EA0=qPGpAK+h1w@mail.gmail.com>","Subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","To":"Bjorn Helgaas <helgaas@kernel.org>","Cc":"Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>,\n\tMark Rutland <mark.rutland@arm.com>, Ray Jui <rjui@broadcom.com>,\n\tScott Branden <sbranden@broadcom.com>, Jon Mason <jonmason@broadcom.com>,\n\tBCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,\n\tAndy Gospodarek <gospo@broadcom.com>,\n\tlinux-pci <linux-pci@vger.kernel.org>, devicetree@vger.kernel.org,\n\tLinux ARM <linux-arm-kernel@lists.infradead.org>,\n\tlinux-kernel@vger.kernel.org, Oza Pawandeep <oza.pawandeep@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1759632,"web_url":"http://patchwork.ozlabs.org/comment/1759632/","msgid":"<20170829200209.GN8154@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-08-29T20:02:09","subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Tue, Aug 29, 2017 at 11:02:23AM +0530, Oza Oza wrote:\n> On Tue, Aug 29, 2017 at 3:17 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:\n> > On Thu, Aug 24, 2017 at 10:34:25AM +0530, Oza Pawandeep wrote:\n> >> PCIe spec r3.1, sec 2.3.2\n> >> If CRS software visibility is not enabled, the RC must reissue the\n> >> config request as a new request.\n> >>\n> >> - If CRS software visibility is enabled,\n> >> - for a config read of Vendor ID, the RC must return 0x0001 data\n> >> - for all other config reads/writes, the RC must reissue the\n> >>   request\n> >>\n> >> iproc PCIe Controller spec:\n> >> 4.7.3.3. Retry Status On Configuration Cycle\n> >> Endpoints are allowed to generate retry status on configuration\n> >> cycles. In this case, the RC needs to re-issue the request. The IP\n> >> does not handle this because the number of configuration cycles needed\n> >> will probably be less than the total number of non-posted operations\n> >> needed.\n> >>\n> >> When a retry status is received on the User RX interface for a\n> >> configuration request that was sent on the User TX interface,\n> >> it will be indicated with a completion with the CMPL_STATUS field set\n> >> to 2=CRS, and the user will have to find the address and data values\n> >> and send a new transaction on the User TX interface.\n> >> When the internal configuration space returns a retry status during a\n> >> configuration cycle (user_cscfg = 1) on the Command/Status interface,\n> >> the pcie_cscrs will assert with the pcie_csack signal to indicate the\n> >> CRS status.\n> >> When the CRS Software Visibility Enable register in the Root Control\n> >> register is enabled, the IP will return the data value to 0x0001 for\n> >> the Vendor ID value and 0xffff  (all 1’s) for the rest of the data in\n> >> the request for reads of offset 0 that return with CRS status.  This\n> >> is true for both the User RX Interface and for the Command/Status\n> >> interface.  When CRS Software Visibility is enabled, the CMPL_STATUS\n> >> field of the completion on the User RX Interface will not be 2=CRS and\n> >> the pcie_cscrs signal will not assert on the Command/Status interface.\n> >>\n> >> Per PCIe r3.1, sec 2.3.2, config requests that receive completions\n> >> with Configuration Request Retry Status (CRS) should be reissued by\n> >> the hardware except reads of the Vendor ID when CRS Software\n> >> Visibility is enabled.\n> >>\n> >> This hardware never reissues configuration requests when it receives\n> >> CRS completions.\n> >> Note that, neither PCIe host bridge nor PCIe core re-issues the\n> >> request for any configuration offset.\n> >>\n> >> For config reads, this hardware returns CFG_RETRY_STATUS data when\n> >> it receives a CRS completion for a config read, regardless of the\n> >> address of the read or the CRS Software Visibility Enable bit.\n> >>\n> >> This patch implements iproc_pcie_config_read which gets called for\n> >> Stingray, if it receives a CRS completion, it retries reading it again.\n> >> In case of timeout, it returns 0xffffffff.\n> >> For other iproc based SOC, it falls back to PCI generic APIs.\n> >>\n> >> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>\n> >>\n> >> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c\n> >> index 61d9be6..37f4adf 100644\n> >> --- a/drivers/pci/host/pcie-iproc.c\n> >> +++ b/drivers/pci/host/pcie-iproc.c\n> >> @@ -68,6 +68,9 @@\n> >>  #define APB_ERR_EN_SHIFT             0\n> >>  #define APB_ERR_EN                   BIT(APB_ERR_EN_SHIFT)\n> >>\n> >> +#define CFG_RETRY_STATUS             0xffff0001\n> >> +#define CFG_RETRY_STATUS_TIMEOUT_US  500000 /* 500 milli-seconds. */\n> >> +\n> >>  /* derive the enum index of the outbound/inbound mapping registers */\n> >>  #define MAP_REG(base_reg, index)      ((base_reg) + (index) * 2)\n> >>\n> >> @@ -473,6 +476,64 @@ static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,\n> >>       return (pcie->base + offset);\n> >>  }\n> >>\n> >> +static unsigned int iproc_pcie_cfg_retry(void __iomem *cfg_data_p)\n> >> +{\n> >> +     int timeout = CFG_RETRY_STATUS_TIMEOUT_US;\n> >> +     unsigned int data;\n> >> +\n> >> +     /*\n> >> +      * As per PCIe spec r3.1, sec 2.3.2, CRS Software\n> >> +      * Visibility only affects config read of the Vendor ID.\n> >> +      * For config write or any other config read the Root must\n> >> +      * automatically re-issue configuration request again as a\n> >> +      * new request.\n> >> +      *\n> >> +      * For config reads, this hardware returns CFG_RETRY_STATUS data when\n> >> +      * it receives a CRS completion for a config read, regardless of the\n> >> +      * address of the read or the CRS Software Visibility Enable bit. As a\n> >> +      * partial workaround for this, we retry in software any read that\n> >> +      * returns CFG_RETRY_STATUS.\n> >> +      */\n> >> +     data = readl(cfg_data_p);\n> >> +     while (data == CFG_RETRY_STATUS && timeout--) {\n> >> +             udelay(1);\n> >> +             data = readl(cfg_data_p);\n> >> +     }\n> >> +\n> >> +     if (data == CFG_RETRY_STATUS)\n> >> +             data = 0xffffffff;\n> >> +\n> >> +     return data;\n> >> +}\n> >> +\n> >> +static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,\n> >> +                                 int where, int size, u32 *val)\n> >> +{\n> >> +     struct iproc_pcie *pcie = iproc_data(bus);\n> >> +     unsigned int slot = PCI_SLOT(devfn);\n> >> +     unsigned int fn = PCI_FUNC(devfn);\n> >> +     unsigned int busno = bus->number;\n> >> +     void __iomem *cfg_data_p;\n> >> +     unsigned int data;\n> >> +\n> >> +     /* root complex access. */\n> >> +     if (busno == 0)\n> >> +             return pci_generic_config_read32(bus, devfn, where, size, val);\n> >\n> > It sounds like Stingray advertises CRS SV support in its Root Capabilities\n> > register.  I think we should mask out PCI_EXP_RTCAP_CRSVIS so we don't\n> > advertise it.  That will keep Linux from trying to enable it.  I know the\n> > hardware doesn't look at PCI_EXP_RTCTL_CRSSVE, but there's no point in\n> > confusing users reading the lspci output.\n> >\n> > We did something similar with f09f8735fb9c (\"PCI: xgene: Disable\n> > Configuration Request Retry Status for v1 silicon\").\n> >\n> > I tried to do this in the patch I pushed to pci/host-iproc.\n> >\n> >> +\n> >> +     cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);\n> >> +\n> >> +     if (!cfg_data_p)\n> >> +             return PCIBIOS_DEVICE_NOT_FOUND;\n> >> +\n> >> +     data = iproc_pcie_cfg_retry(cfg_data_p);\n> >> +\n> >> +     *val = data;\n> >> +     if (size <= 2)\n> >> +             *val = (data >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);\n> >> +\n> >> +     return PCIBIOS_SUCCESSFUL;\n> >> +}\n> >> +\n> >>  /**\n> >>   * Note access to the configuration registers are protected at the higher layer\n> >>   * by 'pci_lock' in drivers/pci/access.c\n> >> @@ -567,8 +628,13 @@ static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,\n> >>                                   int where, int size, u32 *val)\n> >>  {\n> >>       int ret;\n> >> +     struct iproc_pcie *pcie = iproc_data(bus);\n> >>\n> >>       iproc_pcie_apb_err_disable(bus, true);\n> >> +     if (pcie->type == IPROC_PCIE_PAXB_V2)\n> >> +             ret = iproc_pcie_config_read(bus, devfn, where, size, val);\n> >> +     else\n> >> +             ret = pci_generic_config_read32(bus, devfn, where, size, val);\n> >>       ret = pci_generic_config_read32(bus, devfn, where, size, val);\n> >\n> > This last pci_generic_config_read32() call looks like a duplicate.\n> \n> yes indeed.\n> I have tested your CRS visibility bit changes; and it works fine.\n> \n> do you want me to post new patch-set by removing the duplicate call\n> along with the changes you have made ?\n> \n> or since, you have already applied patches, with your changes, you\n> will take care of removing this last duplicate call ?\n> I think this is the last change for this patch-set, If I did not miss anything.\n> \n> please let me know.\n\nI already removed that duplicate call.  It should be in the next -next.\nLet me know if there's anything wrong with it.\n\nBjorn","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","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=<UNKNOWN>)","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 3xhfgD26C0z9sR9\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 30 Aug 2017 06:02:16 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751249AbdH2UCN (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tTue, 29 Aug 2017 16:02:13 -0400","from mail.kernel.org ([198.145.29.99]:52750 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751186AbdH2UCM (ORCPT <rfc822;linux-pci@vger.kernel.org>);\n\tTue, 29 Aug 2017 16:02:12 -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 54D8E217C3;\n\tTue, 29 Aug 2017 20:02:11 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 54D8E217C3","Date":"Tue, 29 Aug 2017 15:02:09 -0500","From":"Bjorn Helgaas <helgaas@kernel.org>","To":"Oza Oza <oza.oza@broadcom.com>","Cc":"Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>,\n\tMark Rutland <mark.rutland@arm.com>, Ray Jui <rjui@broadcom.com>,\n\tScott Branden <sbranden@broadcom.com>, Jon Mason <jonmason@broadcom.com>,\n\tBCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,\n\tAndy Gospodarek <gospo@broadcom.com>,\n\tlinux-pci <linux-pci@vger.kernel.org>, devicetree@vger.kernel.org,\n\tLinux ARM <linux-arm-kernel@lists.infradead.org>,\n\tlinux-kernel@vger.kernel.org, Oza Pawandeep <oza.pawandeep@gmail.com>","Subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","Message-ID":"<20170829200209.GN8154@bhelgaas-glaptop.roam.corp.google.com>","References":"<1503551066-23212-1-git-send-email-oza.oza@broadcom.com>\n\t<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>\n\t<20170828214753.GL8154@bhelgaas-glaptop.roam.corp.google.com>\n\t<CAMSpPPcFFULWZ+QTfzMSGhzQx0JboT618Uq6EA0=qPGpAK+h1w@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<CAMSpPPcFFULWZ+QTfzMSGhzQx0JboT618Uq6EA0=qPGpAK+h1w@mail.gmail.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1759979,"web_url":"http://patchwork.ozlabs.org/comment/1759979/","msgid":"<CAMSpPPd6_tPe1TtCfM0sbEJdhD9_i_s58KcxoYQkmVUbkROdwA@mail.gmail.com>","list_archive_url":null,"date":"2017-08-30T09:18:33","subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","submitter":{"id":71219,"url":"http://patchwork.ozlabs.org/api/people/71219/","name":"Oza Pawandeep","email":"oza.oza@broadcom.com"},"content":"On Wed, Aug 30, 2017 at 1:32 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:\n> On Tue, Aug 29, 2017 at 11:02:23AM +0530, Oza Oza wrote:\n>> On Tue, Aug 29, 2017 at 3:17 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:\n>> > On Thu, Aug 24, 2017 at 10:34:25AM +0530, Oza Pawandeep wrote:\n>> >> PCIe spec r3.1, sec 2.3.2\n>> >> If CRS software visibility is not enabled, the RC must reissue the\n>> >> config request as a new request.\n>> >>\n>> >> - If CRS software visibility is enabled,\n>> >> - for a config read of Vendor ID, the RC must return 0x0001 data\n>> >> - for all other config reads/writes, the RC must reissue the\n>> >>   request\n>> >>\n>> >> iproc PCIe Controller spec:\n>> >> 4.7.3.3. Retry Status On Configuration Cycle\n>> >> Endpoints are allowed to generate retry status on configuration\n>> >> cycles. In this case, the RC needs to re-issue the request. The IP\n>> >> does not handle this because the number of configuration cycles needed\n>> >> will probably be less than the total number of non-posted operations\n>> >> needed.\n>> >>\n>> >> When a retry status is received on the User RX interface for a\n>> >> configuration request that was sent on the User TX interface,\n>> >> it will be indicated with a completion with the CMPL_STATUS field set\n>> >> to 2=CRS, and the user will have to find the address and data values\n>> >> and send a new transaction on the User TX interface.\n>> >> When the internal configuration space returns a retry status during a\n>> >> configuration cycle (user_cscfg = 1) on the Command/Status interface,\n>> >> the pcie_cscrs will assert with the pcie_csack signal to indicate the\n>> >> CRS status.\n>> >> When the CRS Software Visibility Enable register in the Root Control\n>> >> register is enabled, the IP will return the data value to 0x0001 for\n>> >> the Vendor ID value and 0xffff  (all 1’s) for the rest of the data in\n>> >> the request for reads of offset 0 that return with CRS status.  This\n>> >> is true for both the User RX Interface and for the Command/Status\n>> >> interface.  When CRS Software Visibility is enabled, the CMPL_STATUS\n>> >> field of the completion on the User RX Interface will not be 2=CRS and\n>> >> the pcie_cscrs signal will not assert on the Command/Status interface.\n>> >>\n>> >> Per PCIe r3.1, sec 2.3.2, config requests that receive completions\n>> >> with Configuration Request Retry Status (CRS) should be reissued by\n>> >> the hardware except reads of the Vendor ID when CRS Software\n>> >> Visibility is enabled.\n>> >>\n>> >> This hardware never reissues configuration requests when it receives\n>> >> CRS completions.\n>> >> Note that, neither PCIe host bridge nor PCIe core re-issues the\n>> >> request for any configuration offset.\n>> >>\n>> >> For config reads, this hardware returns CFG_RETRY_STATUS data when\n>> >> it receives a CRS completion for a config read, regardless of the\n>> >> address of the read or the CRS Software Visibility Enable bit.\n>> >>\n>> >> This patch implements iproc_pcie_config_read which gets called for\n>> >> Stingray, if it receives a CRS completion, it retries reading it again.\n>> >> In case of timeout, it returns 0xffffffff.\n>> >> For other iproc based SOC, it falls back to PCI generic APIs.\n>> >>\n>> >> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>\n>> >>\n>> >> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c\n>> >> index 61d9be6..37f4adf 100644\n>> >> --- a/drivers/pci/host/pcie-iproc.c\n>> >> +++ b/drivers/pci/host/pcie-iproc.c\n>> >> @@ -68,6 +68,9 @@\n>> >>  #define APB_ERR_EN_SHIFT             0\n>> >>  #define APB_ERR_EN                   BIT(APB_ERR_EN_SHIFT)\n>> >>\n>> >> +#define CFG_RETRY_STATUS             0xffff0001\n>> >> +#define CFG_RETRY_STATUS_TIMEOUT_US  500000 /* 500 milli-seconds. */\n>> >> +\n>> >>  /* derive the enum index of the outbound/inbound mapping registers */\n>> >>  #define MAP_REG(base_reg, index)      ((base_reg) + (index) * 2)\n>> >>\n>> >> @@ -473,6 +476,64 @@ static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,\n>> >>       return (pcie->base + offset);\n>> >>  }\n>> >>\n>> >> +static unsigned int iproc_pcie_cfg_retry(void __iomem *cfg_data_p)\n>> >> +{\n>> >> +     int timeout = CFG_RETRY_STATUS_TIMEOUT_US;\n>> >> +     unsigned int data;\n>> >> +\n>> >> +     /*\n>> >> +      * As per PCIe spec r3.1, sec 2.3.2, CRS Software\n>> >> +      * Visibility only affects config read of the Vendor ID.\n>> >> +      * For config write or any other config read the Root must\n>> >> +      * automatically re-issue configuration request again as a\n>> >> +      * new request.\n>> >> +      *\n>> >> +      * For config reads, this hardware returns CFG_RETRY_STATUS data when\n>> >> +      * it receives a CRS completion for a config read, regardless of the\n>> >> +      * address of the read or the CRS Software Visibility Enable bit. As a\n>> >> +      * partial workaround for this, we retry in software any read that\n>> >> +      * returns CFG_RETRY_STATUS.\n>> >> +      */\n>> >> +     data = readl(cfg_data_p);\n>> >> +     while (data == CFG_RETRY_STATUS && timeout--) {\n>> >> +             udelay(1);\n>> >> +             data = readl(cfg_data_p);\n>> >> +     }\n>> >> +\n>> >> +     if (data == CFG_RETRY_STATUS)\n>> >> +             data = 0xffffffff;\n>> >> +\n>> >> +     return data;\n>> >> +}\n>> >> +\n>> >> +static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,\n>> >> +                                 int where, int size, u32 *val)\n>> >> +{\n>> >> +     struct iproc_pcie *pcie = iproc_data(bus);\n>> >> +     unsigned int slot = PCI_SLOT(devfn);\n>> >> +     unsigned int fn = PCI_FUNC(devfn);\n>> >> +     unsigned int busno = bus->number;\n>> >> +     void __iomem *cfg_data_p;\n>> >> +     unsigned int data;\n>> >> +\n>> >> +     /* root complex access. */\n>> >> +     if (busno == 0)\n>> >> +             return pci_generic_config_read32(bus, devfn, where, size, val);\n>> >\n>> > It sounds like Stingray advertises CRS SV support in its Root Capabilities\n>> > register.  I think we should mask out PCI_EXP_RTCAP_CRSVIS so we don't\n>> > advertise it.  That will keep Linux from trying to enable it.  I know the\n>> > hardware doesn't look at PCI_EXP_RTCTL_CRSSVE, but there's no point in\n>> > confusing users reading the lspci output.\n>> >\n>> > We did something similar with f09f8735fb9c (\"PCI: xgene: Disable\n>> > Configuration Request Retry Status for v1 silicon\").\n>> >\n>> > I tried to do this in the patch I pushed to pci/host-iproc.\n>> >\n>> >> +\n>> >> +     cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);\n>> >> +\n>> >> +     if (!cfg_data_p)\n>> >> +             return PCIBIOS_DEVICE_NOT_FOUND;\n>> >> +\n>> >> +     data = iproc_pcie_cfg_retry(cfg_data_p);\n>> >> +\n>> >> +     *val = data;\n>> >> +     if (size <= 2)\n>> >> +             *val = (data >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);\n>> >> +\n>> >> +     return PCIBIOS_SUCCESSFUL;\n>> >> +}\n>> >> +\n>> >>  /**\n>> >>   * Note access to the configuration registers are protected at the higher layer\n>> >>   * by 'pci_lock' in drivers/pci/access.c\n>> >> @@ -567,8 +628,13 @@ static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,\n>> >>                                   int where, int size, u32 *val)\n>> >>  {\n>> >>       int ret;\n>> >> +     struct iproc_pcie *pcie = iproc_data(bus);\n>> >>\n>> >>       iproc_pcie_apb_err_disable(bus, true);\n>> >> +     if (pcie->type == IPROC_PCIE_PAXB_V2)\n>> >> +             ret = iproc_pcie_config_read(bus, devfn, where, size, val);\n>> >> +     else\n>> >> +             ret = pci_generic_config_read32(bus, devfn, where, size, val);\n>> >>       ret = pci_generic_config_read32(bus, devfn, where, size, val);\n>> >\n>> > This last pci_generic_config_read32() call looks like a duplicate.\n>>\n>> yes indeed.\n>> I have tested your CRS visibility bit changes; and it works fine.\n>>\n>> do you want me to post new patch-set by removing the duplicate call\n>> along with the changes you have made ?\n>>\n>> or since, you have already applied patches, with your changes, you\n>> will take care of removing this last duplicate call ?\n>> I think this is the last change for this patch-set, If I did not miss anything.\n>>\n>> please let me know.\n>\n> I already removed that duplicate call.  It should be in the next -next.\n> Let me know if there's anything wrong with it.\n>\n> Bjorn\n\nIts all well I think. I see no problem.\nonce they are in, I have to base my PCI hotplug patches on those.\n\nRegards,\nOza.","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","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=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=broadcom.com header.i=@broadcom.com\n\theader.b=\"CKOxnHoJ\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xj0L61YmNz9t2Q\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 30 Aug 2017 19:18:38 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751368AbdH3JSg (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tWed, 30 Aug 2017 05:18:36 -0400","from mail-pg0-f43.google.com ([74.125.83.43]:35873 \"EHLO\n\tmail-pg0-f43.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751298AbdH3JSf (ORCPT\n\t<rfc822; linux-pci@vger.kernel.org>); Wed, 30 Aug 2017 05:18:35 -0400","by mail-pg0-f43.google.com with SMTP id r133so18364817pgr.3\n\tfor <linux-pci@vger.kernel.org>; Wed, 30 Aug 2017 02:18:35 -0700 (PDT)","by 10.100.128.26 with HTTP; Wed, 30 Aug 2017 02:18:33 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=broadcom.com; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=07fknE6ifDKKjkYFvBMog+4UszGDtVas02i62mEe8sk=;\n\tb=CKOxnHoJaPRH6uo0lQb9K8p6NlO+r0DBvvJ0B27xNcAHp3pJeP/0LTOADsT0der1sf\n\tqvfgBZ16BRQWQU48ghFB38eFV65rUIdiUrW6z4G0EGb4ILT78dyl+e/Vh/+BRHcKUnLV\n\tptJAb1a30jwck5jXvts9ZS8soOebdi/i6PG7g=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=07fknE6ifDKKjkYFvBMog+4UszGDtVas02i62mEe8sk=;\n\tb=U8TXbKqfqM2tpyEXkBX6XeGHaIWSx3MrvHIRvCSdJeOTSeY1ZOxjtyUCmMBMMCAQXG\n\tgpctItce+R5nz+P9cefQE9Aux4R8IzFIr7QS8j15Gm0x5xXnEtBq+pKinhrt7Xg8yvdE\n\tFmkCpA+2FFemeWyrm84G/N9YpPZRbmZF9iHfPFFcLQmMAjmYAZv6hqQnP1Wp4B/X/JTl\n\tT/KpaFlDEdRwqvuoQXXY4f22veTIhAbK58gSexRNCmNmQaRAITeIYuXvv+cQmh8OsHu4\n\tNq3Tx/MUAql24G+FR4IKsMRC2VzyukIBy2DcDjaOFLSWm/caNQTMD2OofSR0+GNUnlr5\n\tJdTQ==","X-Gm-Message-State":"AHYfb5ioytGTCeYCJC4eFCamq+5qdDkhqVjCphm6oTkx4b+avUQekBih\n\tNHymLTKTD0oT91co+3MGBDLyommjxwd3","X-Received":"by 10.99.54.12 with SMTP id d12mr886923pga.370.1504084714450;\n\tWed, 30 Aug 2017 02:18:34 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170829200209.GN8154@bhelgaas-glaptop.roam.corp.google.com>","References":"<1503551066-23212-1-git-send-email-oza.oza@broadcom.com>\n\t<1503551066-23212-3-git-send-email-oza.oza@broadcom.com>\n\t<20170828214753.GL8154@bhelgaas-glaptop.roam.corp.google.com>\n\t<CAMSpPPcFFULWZ+QTfzMSGhzQx0JboT618Uq6EA0=qPGpAK+h1w@mail.gmail.com>\n\t<20170829200209.GN8154@bhelgaas-glaptop.roam.corp.google.com>","From":"Oza Oza <oza.oza@broadcom.com>","Date":"Wed, 30 Aug 2017 14:48:33 +0530","Message-ID":"<CAMSpPPd6_tPe1TtCfM0sbEJdhD9_i_s58KcxoYQkmVUbkROdwA@mail.gmail.com>","Subject":"Re: [PATCH v8 2/3] PCI: iproc: retry request when CRS returned from\n\tEP","To":"Bjorn Helgaas <helgaas@kernel.org>","Cc":"Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>,\n\tMark Rutland <mark.rutland@arm.com>, Ray Jui <rjui@broadcom.com>,\n\tScott Branden <sbranden@broadcom.com>, Jon Mason <jonmason@broadcom.com>,\n\tBCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,\n\tAndy Gospodarek <gospo@broadcom.com>,\n\tlinux-pci <linux-pci@vger.kernel.org>, devicetree@vger.kernel.org,\n\tLinux ARM <linux-arm-kernel@lists.infradead.org>,\n\tlinux-kernel@vger.kernel.org, Oza Pawandeep <oza.pawandeep@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}}]