From patchwork Tue Sep 5 17:57:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 9939425 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 D09BD60350 for ; Tue, 5 Sep 2017 17:57:36 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C57F0286D4 for ; Tue, 5 Sep 2017 17:57:36 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B9308289E6; Tue, 5 Sep 2017 17:57:36 +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=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED 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 45E39286D4 for ; Tue, 5 Sep 2017 17:57:35 +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=HWLjN1jFZjBH+D8G5EU651OJVRzZhM7YPqpKSZtEgqA=; b=nPpHMsx6wZxZFo 0TpZmwPBkRALTvSiOUZdEkQMN5ump5JkQ1JvbW2c5RcS+B/bl2AGZcf/LMb17tD2ueGNWm3urhrMq Ym70ZRLTKEAaXjmd6Xl6Wgvc8t7gpwL6cyxC19pXcTVOajhoA3hDZ5/DXxlleiCpXkY6OP1XgSX1/ twnAnuizHjdkLNPylDK2EiBSLs+Xuy17gLQ1iJaorjZk8q8xkTlQTkq9VxchGp7aLqB9T0xPqrAl1 zeZQ5ZHJzg1ZpfbLDk2bEEUWmx4RQf/gVkj7QpUQaXYoi9z1GGc1QIKb0bxXki8tn02byriXKx60a sTZk1oFSWijlmEK9op2g==; 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 1dpI6K-0001lz-Po; Tue, 05 Sep 2017 17:57:28 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dpI6H-0001ir-Gr for linux-arm-kernel@lists.infradead.org; Tue, 05 Sep 2017 17:57:27 +0000 Received: from localhost (unknown [69.71.4.159]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DD13821E92; Tue, 5 Sep 2017 17:57:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DD13821E92 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: Tue, 5 Sep 2017 12:57:01 -0500 From: Bjorn Helgaas To: Arnd Bergmann Subject: Re: [PATCH 2/2] PCI: iproc: fix Stingray CRS defect handling Message-ID: <20170905175701.GB14367@bhelgaas-glaptop.roam.corp.google.com> References: <20170905072004.3959082-1-arnd@arndb.de> <20170905072004.3959082-2-arnd@arndb.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170905072004.3959082-2-arnd@arndb.de> 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-20170905_105725_609233_97819286 X-CRM114-Status: GOOD ( 20.44 ) 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: Lorenzo Pieralisi , Scott Branden , Jon Mason , Ray Jui , linux-kernel@vger.kernel.org, Oza Pawandeep , linux-pci@vger.kernel.org, Bjorn Helgaas , bcm-kernel-feedback-list@broadcom.com, 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 Tue, Sep 05, 2017 at 09:19:45AM +0200, Arnd Bergmann wrote: > The condition that was used to detect the PCI_EXP_RTCAP > flag access is wrong, as pointed out by gcc-8: > > drivers/pci/host/pcie-iproc.c: In function 'iproc_pcie_config_read': > drivers/pci/host/pcie-iproc.c:531:22: error: bitwise comparison always evaluates to false [-Werror=tautological-compare] > if ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCAP) > > This adds the same bit mask to the other end as well, so the > condition is evaluated correctly for any access. > > Fixes: ac8d3e852f75 ("PCI: iproc: Work around Stingray CRS defects") > Signed-off-by: Arnd Bergmann > --- > drivers/pci/host/pcie-iproc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > index d7f4c29aed96..f2df9c2266b9 100644 > --- a/drivers/pci/host/pcie-iproc.c > +++ b/drivers/pci/host/pcie-iproc.c > @@ -528,7 +528,7 @@ static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn, > return ret; > > /* Don't advertise CRS SV support */ > - if ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCAP) > + if ((where & ~0x3) == ((PCI_EXP_CAP + PCI_EXP_RTCAP) & ~0x03)) > *val &= ~(PCI_EXP_RTCAP_CRSVIS << 16); > return PCIBIOS_SUCCESSFUL; > } Definitely looks like a bug. I'm slightly confused because Oza tested this and reported that lspci said CRS SV was not supported. But there used to be an lspci bug related to reporting CRS SV, and there are a lot of executables that don't have the fix: http://git.kernel.org/cgit/utils/pciutils/pciutils.git/commit/?id=1cefd379194e lspci -vvxxx should have enough info to tell for sure. Oza, any chance you could apply the incremental patch (or use my pci/host-iproc branch) and collect that lspci output? I propose the following slightly different fix because (a) the shift in the next line depends on the fact that we read the 32-bit value that contains both PCI_EXP_RTCTL and PCI_EXP_RTCAP so I like the use of PCI_EXP_RTCTL as a hint that it's involved and (b) there's similar code in xgene_pcie_config_read32() that uses the equivalent of PCI_EXP_RTCTL; see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/host/pci-xgene.c?h=v4.13#n191 diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c index 0e29338703bf..9a006cbc3021 100644 --- a/drivers/pci/host/pcie-iproc.c +++ b/drivers/pci/host/pcie-iproc.c @@ -528,7 +528,7 @@ static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn, return ret; /* Don't advertise CRS SV support */ - if ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCAP) + if ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCTL) *val &= ~(PCI_EXP_RTCAP_CRSVIS << 16); return PCIBIOS_SUCCESSFUL; }