From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0DF7AC43461 for ; Tue, 8 Sep 2020 22:10:34 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AECE120936 for ; Tue, 8 Sep 2020 22:10:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="W+yoJnK0"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="lC8imgfc" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AECE120936 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.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: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:References:List-Owner; bh=EQlu8BE8eb7p2WHCRvC8dqMGn3/ARgmZ167EG9F0B0M=; b=W+yoJnK0PliKVcKgAZsbeRBG/ ov42px2eCRix03kxDK7lk9mZJuyYNO3mHrCb7166VmSGfojFjdkReDaeiwbbTUgHrkuFrDk0PaRSN B5Yldt44+63OUkgPJOZtAIIoOY8dVz3EDpEEWjPMRIV97qmh4m8bWk6K2i3y1RsSZyf2iv+VDVRxz SN/EwzWu0fHxldF+Khu1LeKLrjvWwx/4cu+cf0ZtKkXQWnIH2cA2EVaNSO+XbgaHVhHEccu1bSB+9 uawjl7CNKkx4Yuo8zD9iWQlg6z2fVNoTA2hoVZcOVumqqdEhAVfs9VkwuaGWTx3isZJsyHLTGVSMH zO2vYeS1A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFlnE-0002Xn-M7; Tue, 08 Sep 2020 22:08:48 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFlnB-0002XI-Nh for linux-arm-kernel@lists.infradead.org; Tue, 08 Sep 2020 22:08:46 +0000 Received: from localhost (35.sub-72-107-115.myvzw.com [72.107.115.35]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id CFFC52087D; Tue, 8 Sep 2020 22:08:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599602925; bh=zwjBUwOjGTfEbZT1gPvFnKgEk0SrYJo/S2MVMcETl3E=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=lC8imgfcgq2pSFqv1Kx1AvIUXr85ZofyR35S3lwEryhZWHW59UHA8kbspaVynKSQj t2gKD+mZjVwQts/yIt+4zaerE3uOC6+CyVIHjebZ1t7hLJ1/AamgncrfPD/Iv+2h0H VloXTVShLvvGz72s3+E/8WrtVccq8DLKw8Zd08jY= Date: Tue, 8 Sep 2020 17:08:43 -0500 From: Bjorn Helgaas To: Lorenzo Pieralisi Subject: Re: [PATCH] PCI: rockchip: Fix bus checks in rockchip_pcie_valid_device() Message-ID: <20200908220843.GA643026@bjorn-Precision-5520> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200908100231.GA22909@e121166-lin.cambridge.arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200908_180845_895365_81926DD9 X-CRM114-Status: GOOD ( 29.61 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Rob Herring , Samuel Dionne-Riel , linux-pci@vger.kernel.org, Shawn Lin , Bjorn Helgaas , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Sep 08, 2020 at 11:02:31AM +0100, Lorenzo Pieralisi wrote: > On Fri, Sep 04, 2020 at 03:09:04PM +0100, Lorenzo Pieralisi wrote: > > The root bus checks rework in: > > > > commit d84c572de1a3 ("PCI: rockchip: Use pci_is_root_bus() to check if bus is root bus") > > > > caused a regression whereby in rockchip_pcie_valid_device() if > > the bus parameter is the root bus and the dev value == 0 the > > function should return 1 (ie true) without checking if the > > bus->parent pointer is a root bus because that triggers a NULL > > pointer dereference. > > > > Fix this by streamlining the root bus detection. > > > > Fixes: d84c572de1a3 ("PCI: rockchip: Use pci_is_root_bus() to check if bus is root bus") > > Reported-by: Samuel Dionne-Riel > > Signed-off-by: Lorenzo Pieralisi > > Cc: Bjorn Helgaas > > Cc: Rob Herring > > Cc: Shawn Lin > > --- > > drivers/pci/controller/pcie-rockchip-host.c | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > Hi Bjorn, > > this is a fix for a patch we merged in the last merge window, can > we send it for one of the upcoming -rcX please ? Sure. I added Samuel's tested-by and put this on for-linus for v5.9. But is there any chance we can figure out a way to make all these "valid_device" functions look more similar? They're a real potpourri of styles: - Most return bool, a couple return int. - Some take PCI_SLOT(devfn); others take devfn. - Some reject "devfn > 0", others reject only "dev > 0". Maybe this is a real difference, I dunno. - A few do unusual things that *look* like pci_is_root_bus(): bus->primary == to_pci_host_bridge(bus->bridge)->busnr bus->number == cfg->busr.start bus->number == pcie->root_bus_nr - Some check for a negated condition first ("!pci_is_root_bus()"), i.e., I always prefer something like this: if (pci_is_root_bus(bus)) return devfn == 0; return pcie_link_up(); over this (from nwl_pcie_valid_device()): if (!pci_is_root_bus(bus)) { if (!pcie_link_up()) return false; } else if (devfn > 0) return false; return true; - About half check whether the link is up. - The comments are pointlessly different. > > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c > > index 0bb2fb3e8a0b..9705059523a6 100644 > > --- a/drivers/pci/controller/pcie-rockchip-host.c > > +++ b/drivers/pci/controller/pcie-rockchip-host.c > > @@ -71,16 +71,13 @@ static void rockchip_pcie_update_txcredit_mui(struct rockchip_pcie *rockchip) > > static int rockchip_pcie_valid_device(struct rockchip_pcie *rockchip, > > struct pci_bus *bus, int dev) > > { > > - /* access only one slot on each root port */ > > - if (pci_is_root_bus(bus) && dev > 0) > > - return 0; > > - > > /* > > - * do not read more than one device on the bus directly attached > > + * Access only one slot on each root port. > > + * Do not read more than one device on the bus directly attached > > * to RC's downstream side. > > */ > > - if (pci_is_root_bus(bus->parent) && dev > 0) > > - return 0; > > + if (pci_is_root_bus(bus) || pci_is_root_bus(bus->parent)) > > + return dev == 0; > > > > return 1; > > } > > -- > > 2.26.1 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel