From patchwork Wed Sep 9 11:32:36 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lorenzo Pieralisi X-Patchwork-Id: 7145821 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id E1473BEEC1 for ; Wed, 9 Sep 2015 11:32:33 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4F7E220970 for ; Wed, 9 Sep 2015 11:32:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4ED3820972 for ; Wed, 9 Sep 2015 11:32:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752111AbbIILc3 (ORCPT ); Wed, 9 Sep 2015 07:32:29 -0400 Received: from foss.arm.com ([217.140.101.70]:58013 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752066AbbIILc2 (ORCPT ); Wed, 9 Sep 2015 07:32:28 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2716E75; Wed, 9 Sep 2015 04:32:38 -0700 (PDT) Received: from red-moon (red-moon.cambridge.arm.com [10.1.203.137]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7FFE53F317; Wed, 9 Sep 2015 04:32:25 -0700 (PDT) Date: Wed, 9 Sep 2015 12:32:36 +0100 From: Lorenzo Pieralisi To: Yinghai Lu Cc: Bjorn Helgaas , "oe5hpm@gmail.com" , Ralf Baechle , "James E.J. Bottomley" , Michael Ellerman , Richard Henderson , Benjamin Herrenschmidt , David Howells , Russell King , Tony Luck , "David S. Miller" , Ingo Molnar , Guenter Roeck , Michal Simek , Chris Zankel , "linux-pci@vger.kernel.org" Subject: Re: trouble with PCI: Call pci_read_bridge_bases() from core instead of arch code Message-ID: <20150909113235.GA10991@red-moon> References: <20150902174716.GA6305@red-moon> <20150902203250.GB829@google.com> <20150903100115.GA15308@red-moon> <20150903162140.GG829@google.com> <20150904141903.GA22997@red-moon> <20150904164412.GD22997@red-moon> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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 X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Sep 05, 2015 at 12:53:48AM +0100, Yinghai Lu wrote: > On Fri, Sep 4, 2015 at 9:44 AM, Lorenzo Pieralisi > wrote: > > On Fri, Sep 04, 2015 at 05:00:35PM +0100, Yinghai Lu wrote: > > > > The problem here is not the last retry, it is the first bridge scan. > > > > By moving pci_read_bridge_bases() to core PCI code, if we do not > > vet the bridge apertures (ie claim them and reset them if the claiming > > fails) we end up calling (on ARM) __pci_bus_size_bridges() with apertures > > that can have sizes != 0, which does not make any sense since we are calling > > __pci_bus_size_bridges() to *discover* what the aperture size should > > be on first bridge scan, correct ? > > for x86, in pcibios_allocate_bridge_resources(), we do validate > the bridge resources, and reset size to 1 (strange ?!). > and they are called before pci_asssign_unassigned_resources() > > so arch ARM would support pcibios_allocate_bridge_resources or other > call to do the same thing? > > wonder some arches even claim fails, they still does not want you to > reset it. While finding a fix for this issue, I bumped into another one with current pci_claim_bridge_resource() behaviour. Described and "fixed" (I actually did not fix it, patch below is just there to show what the problem is) in the patch below. Comments welcome, Lorenzo -- >8 -- Subject: [PATCH] PCI: remove dead code in pci_claim_bridge_resource() Commit 8505e729a2f6eb ("PCI: Add pci_claim_bridge_resource() to clip window if necessary") introduced a new API to claim bridge resources. pci_claim_bridge_resource() tries to claim a bridge resource, and if the claiming fails the function tries to clip the resource to make it fit within the parent resource window. If the clipping succeeds the bridge apertures are set-up accordingly and the pci_claim_bridge_resource() tries to claim the resource again. Commit c770cb4cb505 ("PCI: Mark invalid BARs as unassigned") added code that sets the IORESOURCE_UNSET flag on claiming failure. This means that the second resource claiming after window clipping will always fail, since the resource flags contain IORESOURCE_UNSET, previously set on failure by pci_claim_resource(), so the subsequent pci_claim_resource() call ends up spitting a log message and return failure with no chance whatsoever to succeed. This patch removes the second pci_claim_resource() call in pci_claim_bridge_resource() since it basically has no chance to succeed, leaving the current behaviour unchanged. Signed-off-by: Lorenzo Pieralisi Cc: Bjorn Helgaas Cc: Yinghai Lu --- drivers/pci/setup-bus.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 508cc56..2bf4ac1 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -728,14 +728,10 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) break; case 2: pci_setup_bridge_mmio_pref(bridge); - break; default: - return -EINVAL; + break; } - if (pci_claim_resource(bridge, i) == 0) - return 0; /* claimed a smaller window */ - return -EINVAL; }