From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753106Ab3GWAIw (ORCPT ); Mon, 22 Jul 2013 20:08:52 -0400 Received: from mail-ie0-f176.google.com ([209.85.223.176]:41025 "EHLO mail-ie0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752581Ab3GWAIu (ORCPT ); Mon, 22 Jul 2013 20:08:50 -0400 MIME-Version: 1.0 In-Reply-To: <1372860295-8306-2-git-send-email-mika.westerberg@linux.intel.com> References: <1372860295-8306-1-git-send-email-mika.westerberg@linux.intel.com> <1372860295-8306-2-git-send-email-mika.westerberg@linux.intel.com> From: Bjorn Helgaas Date: Mon, 22 Jul 2013 18:08:28 -0600 Message-ID: Subject: Re: [PATCH v2 1/8] x86/PCI: prevent re-allocation of already existing bridge and ROM resources To: Mika Westerberg Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Jesse Barnes , Yinghai Lu , "Ronciak, John" , "Penner, Miles J" , Bruce Allan , Heikki Krogerus , "Kirill A. Shutemov" , "linux-kernel@vger.kernel.org" , "linux-acpi@vger.kernel.org" , "linux-pci@vger.kernel.org" , "x86@kernel.org" Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 3, 2013 at 8:04 AM, Mika Westerberg wrote: > In hotplug case (especially with Thunderbolt enabled systems) we might need > to call pcibios_resource_survey_bus() several times for a bus. The function > ends up calling pci_claim_resource() for each bridge resource that then > fails claiming that the resource exists already (which it does). Once this > happens the resource is invalidated thus preventing devices behind the > bridge to allocate their resources. > > To fix this we do what has been done in pcibios_allocate_dev_resources() > and check 'parent' of the given resource. If it is non-NULL it means that > the resource has been allocated already and we can skip it. We do the same > for ROM resources as well. > > Signed-off-by: Mika Westerberg This looks reasonable to me. However, the use of "r->parent" to determine whether the resource is already allocated is nothing specific to x86. So I think we might be missing an opportunity to make this more consistent across architectures. I looked at other callers of pci_claim_resource() for bridge and ROM resources, and it looks like we might be able to make similar changes in: frv pcibios_allocate_bus_resources() ia64 pcibios_fixup_resources() (via pcibios_fixup_bridge_resources()) mn10300 pcibios_allocate_bus_resources() mn10300 pcibios_assign_resources() (ROM) mn10300 pcibios_fixup_device_resources() parisc lba_fixup_bus() I really hate the idea of fixing something for x86 but not for other arches, so maybe somebody could take a deeper look at this and see if it would make sense to change the other arches, too. Bjorn > --- > arch/x86/pci/i386.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c > index 94919e3..db6b1ab 100644 > --- a/arch/x86/pci/i386.c > +++ b/arch/x86/pci/i386.c > @@ -210,6 +210,8 @@ static void pcibios_allocate_bridge_resources(struct pci_dev *dev) > r = &dev->resource[idx]; > if (!r->flags) > continue; > + if (r->parent) /* Already allocated */ > + continue; > if (!r->start || pci_claim_resource(dev, idx) < 0) { > /* > * Something is wrong with the region. > @@ -318,6 +320,8 @@ static void pcibios_allocate_dev_rom_resource(struct pci_dev *dev) > r = &dev->resource[PCI_ROM_RESOURCE]; > if (!r->flags || !r->start) > return; > + if (r->parent) /* Already allocated */ > + return; > > if (pci_claim_resource(dev, PCI_ROM_RESOURCE) < 0) { > r->end -= r->start; > -- > 1.8.3.2 >