From patchwork Mon Aug 28 21:53:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 9926367 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 0B1CD60383 for ; Mon, 28 Aug 2017 21:54:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EFAF4287A5 for ; Mon, 28 Aug 2017 21:54:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E416F287D5; Mon, 28 Aug 2017 21:54:00 +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_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 DAC3E287D0 for ; Mon, 28 Aug 2017 21:53:58 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=giHsXqnnvRjhcqeZelrr1occUH1gHGImNdSMWZWa/Fg=; b=mPoAOUkZicMyzz M1MmFlxY5vl1DjGcroLBGdmReY7DprW+DIjq3/eEFRJKUuj7N3W5xLcE0LcoXfm4g9qqj0OZXs3bI NR70UjJh0ATNpEJ65xL7wwVWT5HE0mkmfJYn/nU3vb/8JkXlFDjGzYjNt2QT1hwdQdInTptBiQuI9 3ZZwkjWd5X/o4a7x2hPh8y6/zIk9khoYI4du7RG5VAlfUMaSsspA1aEWZVgczxuyiih9nyVrcLuB+ xquqGLX6BaD0/Q9xNktJpqg/x/8kHT5PQDZRAnOyOvkLUzxj8l6n+onjHipHpBAppm1KMI51O910P Mbep3EAgbtnGuhNPGWog==; 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 1dmRyR-0000gS-LG; Mon, 28 Aug 2017 21:53:35 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dmRyN-0000Tn-BO for linux-arm-kernel@lists.infradead.org; Mon, 28 Aug 2017 21:53:33 +0000 Received: from localhost (unknown [69.55.156.165]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id C30B720C48; Mon, 28 Aug 2017 21:53:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C30B720C48 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, 28 Aug 2017 16:53:09 -0500 From: Bjorn Helgaas To: Oza Pawandeep Subject: Re: [PATCH v8 0/3] PCI: iproc: SOC specific fixes Message-ID: <20170828215309.GM8154@bhelgaas-glaptop.roam.corp.google.com> References: <1503551066-23212-1-git-send-email-oza.oza@broadcom.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1503551066-23212-1-git-send-email-oza.oza@broadcom.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170828_145331_500928_191B26D7 X-CRM114-Status: GOOD ( 24.46 ) 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: Mark Rutland , devicetree@vger.kernel.org, Scott Branden , Oza Pawandeep , Jon Mason , Ray Jui , linux-kernel@vger.kernel.org, Rob Herring , bcm-kernel-feedback-list@broadcom.com, linux-pci@vger.kernel.org, Bjorn Helgaas , Andy Gospodarek , 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 On Thu, Aug 24, 2017 at 10:34:23AM +0530, Oza Pawandeep wrote: > PCI: iproc: Retry request when CRS returned from EP Above patch adds > support for CRS in PCI RC driver, otherwise if not handled at lower > level, the user space PMD (poll mode drivers) can timeout. > > PCI: iproc: add device shutdown for PCI RC This fixes the issue where > certian PCI endpoints are not getting detected on Stingray SOC after > reboot. > > Changes Since v7: > Factor out the ep config access code. > > Changes Since v6: > Rebased patches on top of Lorenzo's patches. > Bjorn's comments addressed. > now the confg retry returns 0xffffffff as data. > Added reference to PCIe spec and iproc Controller spec in Changelog. > > Changes Since v5: > Ray's comments addressed. > > Changes Since v4: > Bjorn's comments addressed. > > Changes Since v3: > [re-send] > > Changes Since v2: > Fix compilation errors for pcie-iproc-platform.ko which was caught > by kbuild. > > Oza Pawandeep (3): > PCI: iproc: factor-out ep configuration access > PCI: iproc: Retry request when CRS returned from EP > PCI: iproc: add device shutdown for PCI RC > > drivers/pci/host/pcie-iproc-platform.c | 8 ++ > drivers/pci/host/pcie-iproc.c | 143 ++++++++++++++++++++++++++------- > drivers/pci/host/pcie-iproc.h | 1 + > 3 files changed, 124 insertions(+), 28 deletions(-) I applied these to pci/host-iproc for v4.14. Man, this is ugly. I reworked the changelog to try to make it more readable. I also tried to disable the PCI_EXP_RTCAP_CRSVIS bit, which advertises CRS SV support. And I removed what looked like a duplicate pci_generic_config_read32() call. And I added a warning about the fact that we corrupt reads of config registers that happen to contain 0xffff0001. I'm pretty sure I broke something, so please take a look. Incremental diff from your v8 to what's on pci/host-iproc: diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c index cbdabe8a073e..8bd5e544b1c1 100644 --- a/drivers/pci/host/pcie-iproc.c +++ b/drivers/pci/host/pcie-iproc.c @@ -69,7 +69,7 @@ #define APB_ERR_EN BIT(APB_ERR_EN_SHIFT) #define CFG_RETRY_STATUS 0xffff0001 -#define CFG_RETRY_STATUS_TIMEOUT_US 500000 /* 500 milli-seconds. */ +#define CFG_RETRY_STATUS_TIMEOUT_US 500000 /* 500 milliseconds */ /* derive the enum index of the outbound/inbound mapping registers */ #define MAP_REG(base_reg, index) ((base_reg) + (index) * 2) @@ -482,17 +482,21 @@ static unsigned int iproc_pcie_cfg_retry(void __iomem *cfg_data_p) unsigned int data; /* - * As per PCIe spec r3.1, sec 2.3.2, CRS Software - * Visibility only affects config read of the Vendor ID. - * For config write or any other config read the Root must - * automatically re-issue configuration request again as a - * new request. + * As per PCIe spec r3.1, sec 2.3.2, CRS Software Visibility only + * affects config reads of the Vendor ID. For config writes or any + * other config reads, the Root may automatically reissue the + * configuration request again as a new request. * - * For config reads, this hardware returns CFG_RETRY_STATUS data when - * it receives a CRS completion for a config read, regardless of the - * address of the read or the CRS Software Visibility Enable bit. As a + * For config reads, this hardware returns CFG_RETRY_STATUS data + * when it receives a CRS completion, regardless of the address of + * the read or the CRS Software Visibility Enable bit. As a * partial workaround for this, we retry in software any read that * returns CFG_RETRY_STATUS. + * + * Note that a non-Vendor ID config register may have a value of + * CFG_RETRY_STATUS. If we read that, we can't distinguish it from + * a CRS completion, so we will incorrectly retry the read and + * eventually return the wrong data (0xffffffff). */ data = readl(cfg_data_p); while (data == CFG_RETRY_STATUS && timeout--) { @@ -515,10 +519,19 @@ static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn, unsigned int busno = bus->number; void __iomem *cfg_data_p; unsigned int data; + int ret; - /* root complex access. */ - if (busno == 0) - return pci_generic_config_read32(bus, devfn, where, size, val); + /* root complex access */ + if (busno == 0) { + ret = pci_generic_config_read32(bus, devfn, where, size, val); + if (ret != PCIBIOS_SUCCESSFUL) + return ret; + + /* Don't advertise CRS SV support */ + if ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCAP) + *val &= ~(PCI_EXP_RTCAP_CRSVIS << 16); + return PCIBIOS_SUCCESSFUL; + } cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where); @@ -635,7 +648,6 @@ static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn, ret = iproc_pcie_config_read(bus, devfn, where, size, val); else ret = pci_generic_config_read32(bus, devfn, where, size, val); - ret = pci_generic_config_read32(bus, devfn, where, size, val); iproc_pcie_apb_err_disable(bus, false); return ret; @@ -1309,6 +1321,8 @@ static int iproc_pcie_rev_init(struct iproc_pcie *pcie) pcie->ib.nr_regions = ARRAY_SIZE(paxb_v2_ib_map); pcie->ib_map = paxb_v2_ib_map; pcie->need_msi_steer = true; + dev_warn(dev, "reads of config registers that contain %#x return incorrect data\n", + CFG_RETRY_STATUS); break; case IPROC_PCIE_PAXC: regs = iproc_pcie_reg_paxc;