From: Bjorn Helgaas <bhelgaas@google.com>
To: Ram Pai <linuxram@us.ibm.com>
Cc: Yinghai Lu <yinghai@kernel.org>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 03/27] PCI: pci resource iterator
Date: Thu, 25 Apr 2013 11:22:59 -0600 [thread overview]
Message-ID: <CAErSpo704b0OhSdeRWK+cZ++6RCDvoL7vzqZVEM1WOPp9f7MRQ@mail.gmail.com> (raw)
In-Reply-To: <20130425035503.GA15749@ram.oc3035372033.ibm.com>
On Wed, Apr 24, 2013 at 9:55 PM, Ram Pai <linuxram@us.ibm.com> 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 <linuxram@us.ibm.com> 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 <yinghai@kernel.org> wrote:
>> >> > From: Ram Pai <linuxram@us.ibm.com>
>> >> >
>> >> > 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.
> 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.
> 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.
> Everything else needs to be hidden.
Bjorn
next prev parent reply other threads:[~2013-04-25 17:22 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-13 23:27 [PATCH v3 00/27] PCI: Add for_each_pci_resource and addon_res support Yinghai Lu
2013-03-13 23:27 ` [PATCH v3 01/27] PCI: Add pci_dev_resource_n() Yinghai Lu
2013-03-13 23:27 ` [PATCH v3 02/27] PCI: Add pci_dev_resource_idx() helper Yinghai Lu
2013-04-04 22:00 ` Bjorn Helgaas
2013-03-13 23:27 ` [PATCH v3 03/27] PCI: pci resource iterator Yinghai Lu
2013-04-04 22:18 ` Bjorn Helgaas
2013-04-09 4:51 ` Ram Pai
2013-04-10 15:22 ` Bjorn Helgaas
2013-04-25 3:55 ` Ram Pai
2013-04-25 17:22 ` Bjorn Helgaas [this message]
2013-04-28 6:08 ` Ram Pai
2013-04-10 16:12 ` Yinghai Lu
2013-03-13 23:27 ` [PATCH v3 04/27] PCI: Add is_pci_*_resource_idx() helpers Yinghai Lu
2013-04-04 22:23 ` Bjorn Helgaas
2013-03-13 23:28 ` [PATCH v3 05/27] PCI: Update pci_resource_start etc to use pci_dev_resource_n() Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 06/27] PCI, x86: Use for_each_pci_resource() with pci_allocate_bridge_resources Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 07/27] PCI, x86: Use for_each_pci_resource() with pci_allocate_dev_resources Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 08/27] PCI: Use for_each_pci_resource() with IOV releated functions Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 09/27] PCI, acpiphp: Use for_each_pci_resource() helper Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 10/27] PCI, pciehp: " Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 11/27] PCI: Use for_each_pci_resource() in pci_enable_dev Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 12/27] PCI: Use for_each_pci_resource() in pci_reassigndev Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 13/27] PCI: Use for_each_pci_resource() with pci bar reassign funcs Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 14/27] PCI: Use for_each_pci_resource() in pci_assign_resource Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 15/27] PCI, x86: Use for_each_pci_resource() with noassign_bars Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 16/27] PCI: Use for_each_pci_resource() in pci_dev_driver() Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 17/27] PCI: Use for_each_pci_resource() in pci resource release Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 18/27] PCI: Use for_each_pci_resource() in pci bases reading Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 19/27] PCI, x86: Use for_each_pci_resource() with mrst Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 20/27] PCI, xen: Use for_each_pci_resource() with xen pci Yinghai Lu
2013-03-15 13:35 ` Konrad Rzeszutek Wilk
2013-03-13 23:28 ` [PATCH v3 21/27] PCI: Add addon_resource support for pci devices Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 22/27] PCI: Add helpers to add addon_resource Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 23/27] PCI: Update pci_resource_bar() to support addon_resource Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 24/27] PCI: Assign/update resource to addon_res Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 25/27] PCI: Make piix4 quirk to use addon_res Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 26/27] PCI: Make quirk_io_region " Yinghai Lu
2013-04-04 21:35 ` Bjorn Helgaas
2013-04-10 2:17 ` Yinghai Lu
2013-03-13 23:28 ` [PATCH v3 27/27] PCI: Use addon_fixed_resource with ati fixed resource Yinghai Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAErSpo704b0OhSdeRWK+cZ++6RCDvoL7vzqZVEM1WOPp9f7MRQ@mail.gmail.com \
--to=bhelgaas@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linuxram@us.ibm.com \
--cc=yinghai@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).