Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751832Ab3D1GJa (ORCPT ); Sun, 28 Apr 2013 02:09:30 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:57490 "EHLO e33.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751329Ab3D1GJ2 (ORCPT ); Sun, 28 Apr 2013 02:09:28 -0400 Date: Sun, 28 Apr 2013 14:08:55 +0800 From: Ram Pai To: Bjorn Helgaas Cc: Yinghai Lu , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v3 03/27] PCI: pci resource iterator Message-ID: <20130428060855.GA2700@ram.oc3035372033.ibm.com> Reply-To: Ram Pai References: <1363217302-14383-1-git-send-email-yinghai@kernel.org> <1363217302-14383-4-git-send-email-yinghai@kernel.org> <20130409045113.GA7251@ram.oc3035372033.ibm.com> <20130425035503.GA15749@ram.oc3035372033.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-12-10) X-TM-AS-MML: No X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13042806-2398-0000-0000-000013D1044E Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4436 Lines: 105 On Thu, Apr 25, 2013 at 11:22:59AM -0600, Bjorn Helgaas wrote: > On Wed, Apr 24, 2013 at 9:55 PM, Ram Pai wrote: > > On Wed, Apr 10, 2013 at 09:22:48AM -0600, Bjorn Helgaas wrote: > >> On Mon, Apr 8, 2013 at 10:51 PM, Ram Pai wrote: > >> > On Thu, Apr 04, 2013 at 04:18:01PM -0600, Bjorn Helgaas wrote: > >> >> On Wed, Mar 13, 2013 at 5:27 PM, Yinghai Lu wrote: > >> >> > From: Ram Pai > >> >> > > >> >> > Currently pci_dev structure holds an array of 17 PCI resources; six base > >> >> > BARs, one ROM BAR, four BRIDGE BARs, six sriov BARs. This is wasteful. > >> >> > A bridge device just needs the 4 bridge resources. A non-bridge device > >> >> > just needs the six base resources and one ROM resource. The sriov > >> >> > resources are needed only if the device has SRIOV capability. > >> >> > > ... > >> > I agree. There are two cleanups needed. > >> > > >> > a) pci drivers should not assume the internal organization of the > >> > resources in the struct pci_dev. > >> > >> Do you mean that drivers should not use "pci_dev->resource[i]"? If > >> so, I agree that it would be great if we had an accessor for BARs, but > >> it seems impractical to change all the drivers that use the current > >> style. > > > > Sorry for the delay. Was vacationing. I mean, we cannot let drivers > > assume anything about the how the resources are organized. > > > > The only thing the drivers should know is that there are 6 normal > > resources, 4 bridge resources, 1 ROM resource and 6 iov resources. > > > > Currently the drivers assume that ROM resource follows normal resources > > followed by IOV followed by bridge. These assumptions are making it hard > > to re-organize the layout of resources in struct pci_dev. > > > > I think we need to expose the following interfaces to drivers. > > > > a) return the nth normal resource > > I think this needs to remain "pci_dev->resource[n]", because so many > drivers do this that it would be impractical to change them all. Scanning through the entire kernel tree, I did find about 40 different drivers that are accessing pci_dev->resource[n]. These drivers can be changed to use the newer interface. Out-of-tree drivers can continue to access it directly, but they will break, when the datastructure is eventually re-organized. I was thinking of a interface something like pci_get_std_resource(dev,i) which is implemented internally as #define pci_get_std_resource(dev,i) dev->resource[i] > > > b) return the nth iov resource > > I could imagine a new interface for this, given that I only see a > dozen SR-IOV drivers in the tree. There might be a few out-of-tree, > but there probably aren't many. > > > c) return the rom resource > > There are only about 30 drivers in the tree that reference > PCI_ROM_RESOURCE. Fewer than I expected, but I'd still be hesitant > about make "pci_dev->resource[PCI_ROM_RESOURCE]" stop working. It will work till someday when the datastructure is re-organized. Again the interface will be something like pci_get_rom_resource(dev) which is implemented internally as #define pci_get_std_resource(dev) pci_dev->resource[PCI_ROM_RESOURCE] > > > d) return the nth bridge resource > > I think it's reasonable to have a new interface for this because > bridges are handled almost entirely in the PCI core and architecture > code, and I doubt there are many, if any, drivers that care. > > > e) return the type and index of a given resource, where 'index' is > > the index w.r.t to that resource type; not w.r.t to all > > the resources of the device. > > f) ability to loop through all resources of the given type/types. > > We do loop through resources in the core when we're assigning, fixing > up, etc., and that makes some sense to me. But I actually don't see > the use case for *drivers* to loop through resources. All a driver > knows is "BAR X means Y", and it generally doesn't need to iterate and > do something generic to all of them. Yes mostly true. However I have seen a couple of drivers looping through the resources. An examples is .. yenta_free_resources() RP -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/