From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755207Ab1JKQQG (ORCPT ); Tue, 11 Oct 2011 12:16:06 -0400 Received: from smtp-out.google.com ([74.125.121.67]:5768 "EHLO smtp-out.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754859Ab1JKQQB (ORCPT ); Tue, 11 Oct 2011 12:16:01 -0400 DomainKey-Signature: a=rsa-sha1; s=beta; d=google.com; c=nofws; q=dns; h=dkim-signature:mime-version:in-reply-to:references:from:date: message-id:subject:to:cc:content-type:x-system-of-record; b=lj6JW6rg3yc63WqM0z1O9OWTwfUGtuTFm9Ie5Yrk4nu3NQfm1xZtxMM5j1j18LTea JKjJ4tFTXan0cgNzUEASA== MIME-Version: 1.0 In-Reply-To: <1318319295.29415.452.camel@pasglop> References: <1314845309-3277-1-git-send-email-dczhu@mips.com> <1318319295.29415.452.camel@pasglop> From: Bjorn Helgaas Date: Tue, 11 Oct 2011 10:15:34 -0600 Message-ID: Subject: Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources To: Benjamin Herrenschmidt Cc: "Zhu, DengCheng" , "jbarnes@virtuousgeek.org" , "ralf@linux-mips.org" , "monstr@monstr.eu" , "paulus@samba.org" , "davem@davemloft.net" , "tglx@linutronix.de" , "mingo@redhat.com" , "hpa@zytor.com" , "x86@kernel.org" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-mips@linux-mips.org" , "Barzilay, Eyal" , "Fortuna, Zenon" , "dengcheng.zhu@gmail.com" Content-Type: text/plain; charset=ISO-8859-1 X-System-Of-Record: true Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 11, 2011 at 1:48 AM, Benjamin Herrenschmidt wrote: > I must admit I don't completely understand what this patch is about, > other than it will most probably break the way we do resource management > on powerpc :-) > > I don't understand the point about conflicts in scan_slot and I don't > see what you win by "settling down early". Also keep in mind that the > resources read from the device need to be remapped on some archs like > powerpc which we do from a header quirk at the moment. These patches only deal with root bus resources, i.e., the non-architected PCI host bridge windows. They don't have anything to do with normal PCI BARs. MIPS sets up root buses differently than powerpc, so it has a problem that powerpc doesn't have. Here's the original MIPS flow (before this series): pci_scan_bus pci_scan_bus_parented pci_create_bus <-- A create root bus pci_scan_child_bus pci_scan_slot pci_scan_single_device pci_scan_device pci_setup_device pci_fixup_device (pci_fixup_early) <-- B pci_device_add pci_fixup_device (pci_fixup_header) <-- C pcibios_fixup_bus <-- D fill in root bus resources At point A, we allocate a struct pci_bus for the root bus. pci_create_bus() fills in defaults for the resources available on that bus: ioport_resource and iomem_resource, which cover all possible address space. Later at point D, we replace those defaults with the correct resources (hose->io_resource and hose->mem_resource in this MIPS case). The problem is that the root bus resources are wrong during the interval between A and D. Anything that looks at them may break. In the case Deng-Cheng found, the quirk_piix4_acpi() fixup at point C claimed a region, which incorrectly became the child of ioport_resource instead of host->io_resource. Deng-Cheng's patches close this window by basically combining the fixup at D with the root bus creation at A. Powerpc doesn't have the same problem because it calls pci_create_bus() directly so it can fix the root bus resources with pcibios_setup_phb_resources() *before* it scans the bus. Even though powerpc and many other architectures don't have the MIPS problem, I think it's worth changing the code because the existing pattern is poor. In almost all cases, we know what the host bridge apertures are before we create the root bus. It's error-prone to have pci_create_bus() fill in default resources, then rely on the architecture to fix that up later. I think it's better to supply the resources up front. Bjorn