[{"id":1763541,"web_url":"http://patchwork.ozlabs.org/comment/1763541/","msgid":"<20170905175701.GB14367@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-09-05T17:57:01","subject":"Re: [PATCH 2/2] PCI: iproc: fix Stingray CRS defect handling","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Tue, Sep 05, 2017 at 09:19:45AM +0200, Arnd Bergmann wrote:\n> The condition that was used to detect the PCI_EXP_RTCAP\n> flag access is wrong, as pointed out by gcc-8:\n> \n> drivers/pci/host/pcie-iproc.c: In function 'iproc_pcie_config_read':\n> drivers/pci/host/pcie-iproc.c:531:22: error: bitwise comparison always evaluates to false [-Werror=tautological-compare]\n> if ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCAP)\n> \n> This adds the same bit mask to the other end as well, so the\n> condition is evaluated correctly for any access.\n> \n> Fixes: ac8d3e852f75 (\"PCI: iproc: Work around Stingray CRS defects\")\n> Signed-off-by: Arnd Bergmann \n> ---\n> drivers/pci/host/pcie-iproc.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c\n> index d7f4c29aed96..f2df9c2266b9 100644\n> --- a/drivers/pci/host/pcie-iproc.c\n> +++ b/drivers/pci/host/pcie-iproc.c\n> @@ -528,7 +528,7 @@ static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,\n> \t\t\treturn ret;\n> \n> \t\t/* Don't advertise CRS SV support */\n> -\t\tif ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCAP)\n> +\t\tif ((where & ~0x3) == ((PCI_EXP_CAP + PCI_EXP_RTCAP) & ~0x03))\n> \t\t\t*val &= ~(PCI_EXP_RTCAP_CRSVIS << 16);\n> \t\treturn PCIBIOS_SUCCESSFUL;\n> \t}\n\nDefinitely looks like a bug. I'm slightly confused because Oza tested\nthis and reported that lspci said CRS SV was not supported. But there\nused to be an lspci bug related to reporting CRS SV, and there are a\nlot of executables that don't have the fix:\n\nhttp://git.kernel.org/cgit/utils/pciutils/pciutils.git/commit/?id=1cefd379194e\n\nlspci -vvxxx should have enough info to tell for sure. Oza, any\nchance you could apply the incremental patch (or use my pci/host-iproc\nbranch) and collect that lspci output?\n\nI propose the following slightly different fix because (a) the shift\nin the next line depends on the fact that we read the 32-bit value\nthat contains both PCI_EXP_RTCTL and PCI_EXP_RTCAP so I like the use\nof PCI_EXP_RTCTL as a hint that it's involved and (b) there's similar\ncode in xgene_pcie_config_read32() that uses the equivalent of\nPCI_EXP_RTCTL; see\n\nhttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/host/pci-xgene.c?h=v4.13#n191\n\ndiff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c\nindex 0e29338703bf..9a006cbc3021 100644\n--- a/drivers/pci/host/pcie-iproc.c\n+++ b/drivers/pci/host/pcie-iproc.c\n@@ -528,7 +528,7 @@ static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,\n \t\t\treturn ret;\n \n \t\t/* Don't advertise CRS SV support */\n-\t\tif ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCAP)\n+\t\tif ((where & ~0x3) == PCI_EXP_CAP + PCI_EXP_RTCTL)\n \t\t\t*val &= ~(PCI_EXP_RTCAP_CRSVIS << 16);\n \t\treturn PCIBIOS_SUCCESSFUL;\n \t}","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 3xmvYb0K5Mz9s7g\n\tfor ;\n\tWed, 6 Sep 2017 03:57:07 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752136AbdIER5F (ORCPT );\n\tTue, 5 Sep 2017 13:57:05 -0400","from mail.kernel.org ([198.145.29.99]:57962 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1752129AbdIER5E (ORCPT );\n\tTue, 5 Sep 2017 13:57:04 -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 DD13821E92;\n\tTue, 5 Sep 2017 17:57:03 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org DD13821E92","Date":"Tue, 5 Sep 2017 12:57:01 -0500","From":"Bjorn Helgaas ","To":"Arnd Bergmann ","Cc":"Bjorn Helgaas , Ray Jui ,\n\tScott Branden , Jon Mason ,\n\tbcm-kernel-feedback-list@broadcom.com,\n\tOza Pawandeep ,\n\tLorenzo Pieralisi ,\n\tlinux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tlinux-kernel@vger.kernel.org","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>\n\t<20170905072004.3959082-2-arnd@arndb.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170905072004.3959082-2-arnd@arndb.de>","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"}}]