methods to access resources of a struct pci_dev

PCI: methods to access resources of struct pci_dev

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.

The pci_dev structure needs to be re-organized to avoid unnecessary bloating.  However too much code outside the pci-bus driver, assumes the internal details of the pci_dev structure, thus making it hard to re-organize the datastructure.

As a first step this patch provides generic methods to access the resource structure of the pci_dev.

Once this patch is accepted, I have another 40+ patches that modify all the files that directly access the resource structure, to use the new methods provided in the first step.

Finally we can re-organize the resource structure in the pci_dev structure and correspondingly update the methods.

Quoted 20 lines · Ram Pai

PCI: methods to access resources of struct pci_dev

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.

The pci_dev structure needs to be re-organized to avoid unnecessary bloating.  However too much code outside the pci-bus driver, assumes the internal details of the pci_dev structure, thus making it hard to re-organize the datastructure.

As a first step this patch provides generic methods to access the resource structure of the pci_dev.

Once this patch is accepted, I have another 40+ patches that modify all the files that directly access the resource structure, to use the new methods provided in the first step.

Quoted 2 lines

Finally we can re-organize the resource structure in the pci_dev structure and correspondingly update the methods.

I have patchset on this, please check

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-for-each-res-addon

Quoted 5 lines · Yinghai Lu

I have patchset on this, please check

  git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
 for-pci-for-each-res-addon

Amazing, when are you pushing those patches in?  Looks like you have patches for everything :)

Do you also have patches that change all the places that directly access the ->resource structure?

Also does Bjorn pull regularly from your tree?  Is making patches against your tree the right approach?

Quoted 20 lines · Ram Pai

PCI: methods to access resources of struct pci_dev

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.

The pci_dev structure needs to be re-organized to avoid unnecessary bloating. However too much code outside the pci-bus driver, assumes the internal details of the pci_dev structure, thus making it hard to re-organize the datastructure.

As a first step this patch provides generic methods to access the resource structure of the pci_dev.

Once this patch is accepted, I have another 40+ patches that modify all the files that directly access the resource structure, to use the new methods provided in the first step.

Quoted 2 lines

Finally we can re-organize the resource structure in the pci_dev structure and correspondingly update the methods.

I have patchset on this, please check

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-for-each-res-addon

Quoted 21 lines · Yinghai Lu

Quoted 20 lines · Ram Pai

PCI: methods to access resources of struct pci_dev

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.

The pci_dev structure needs to be re-organized to avoid unnecessary bloating. However too much code outside the pci-bus driver, assumes the internal details of the pci_dev structure, thus making it hard to re-organize the datastructure.

As a first step this patch provides generic methods to access the resource structure of the pci_dev.

Once this patch is accepted, I have another 40+ patches that modify all the files that directly access the resource structure, to use the new methods provided in the first step.

Quoted 7 lines

Quoted 2 lines

Finally we can re-organize the resource structure in the pci_dev structure and correspondingly update the methods.

I have patchset on this, please check

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-for-each-res-addon

I don't like the pci_dev.resource[] table very well either, and I don't like the fact that drivers access it directly. So I do think we need some sort of abstraction that lets us change the way we store the resources without affecting the callers.

Both of these (Ram's patch:
http://marc.info/?l=linux-pci&m=133999604128815&w=2 and Yinghai's
patch: http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=cd192f0ed93203ef6bac2a44c138899190fb5793)
have a similar approach of adding many iterators.

Yinghai's adds:
for_each_pci_dev_all_resource
for_each_pci_dev_nobridge_resource
for_each_pci_dev_base_resource
for_each_pci_dev_base_norom_resource
for_each_pci_dev_base_iov_norom_resource
for_each_pci_dev_noiov_resource
for_each_pci_dev_std_resource
for_each_pci_dev_iov_resource
for_each_pci_dev_bridge_resource
for_each_pci_dev_addon_resource

Ram's adds:
for_each_resource
for_each_std_resource
for_each_std_and_rom_resource
for_each_sriov_resource
for_each_bridge_resource

It seems like we have a combinatorial explosion of iterators based on various orthogonal selectors (base, rom, iov, bridge window). That doesn't seem understandable or maintainable to me.

I wonder if we could get by with *one* iterator, and select the resources of interest either by supplying a bitmask of things we care about, or by doing similar filtering in the caller.

On Wed, Aug 15, 2012 at 03:25:06PM -0600, Bjorn Helgaas wrote:

I am fine with this approach. I have never encountered the need for 'no' based iterator like 'for_each_pci_dev_noiov_resource' or 'for_each_pci_dev_base_norom_resource'. While abstracting the code and replacing explicit references to the resources in various peices of code including the drivers, I just encountered the need for the 'yes' based iterators like the one that I added.

However if there is a need for 'no' based iterators, it should be easy to incorporate them using flags. Something like

for_each_pci_resource(dev, res, i, flags)

where flags can be
#define PCI_STD_RES 0x01
#define PCI_ROM_RES 0x02
#define PCI_BRIDGE_RES 0x04
#define PCI_IOV_RES 0x08
#define PCI_ALL_RES PCI_STD_RES|PCI_ROM_RES|PCI_BRIDGE_RES|PCI_IOV_RES #define PCI_NOSTD_RES PCI_ALL_RES&(^PCI_STD_RES)
#define PCI_NOIOV_RES PCI_ALL_RES&(^PCI_IOV_RES)
so on and so forth

Yinghai if you are ok with this approach, let me code up all the iterators. You can incorporate your patches based on those iterators and I can change all my 40+ patches that change various driver sources to use this iterator.

From cd192f0ed93203ef6bac2a44c138899190fb5793 Mon Sep 17 00:00:00 2001

Quoted 24 lines · Ram Pai

On Wed, Aug 15, 2012 at 03:25:06PM -0600, Bjorn Helgaas wrote:

I am fine with this approach. I have never encountered the need for 'no' based iterator like 'for_each_pci_dev_noiov_resource' or 'for_each_pci_dev_base_norom_resource'. While abstracting the code and replacing explicit references to the resources in various peices of code including the drivers, I just encountered the need for the 'yes' based iterators like the one that I added.

However if there is a need for 'no' based iterators, it should be easy to incorporate them using flags. Something like

for_each_pci_resource(dev, res, i, flags)

where flags can be
#define PCI_STD_RES 0x01
#define PCI_ROM_RES 0x02
#define PCI_BRIDGE_RES 0x04
#define PCI_IOV_RES 0x08
#define PCI_ALL_RES PCI_STD_RES|PCI_ROM_RES|PCI_BRIDGE_RES|PCI_IOV_RES #define PCI_NOSTD_RES PCI_ALL_RES&(^PCI_STD_RES)
#define PCI_NOIOV_RES PCI_ALL_RES&(^PCI_IOV_RES)
so on and so forth

Quoted 4 lines

Yinghai if you are ok with this approach, let me code up all the iterators. You can incorporate your patches based on those iterators and I can change all my 40+ patches that change various driver sources to use this iterator.

Do you mean that you will have updated patch for
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=cd192f0ed93203ef6bac2a44c138899190fb5793
?

if it is that case, i am ok, and then I could use scripts to update following patches.

PCI: pci resource iterator

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.

The pci_dev structure needs to be re-organized to avoid unnecessary bloating. However too much code outside the pci-bus driver, assumes the internal details of the pci_dev structure, thus making it hard to re-organize the datastructure.

As a first step this patch provides generic methods to access the resource structure of the pci_dev.

Once this patch is accepted, I have another 40+ patches that modify all the files that directly access the resource structure, to use the new methods provided in the first step.

Finally we can re-organize the resource structure in the pci_dev structure and correspondingly update the methods.

This patch is compile tested only.

Changelog:
Consolidated iterator interface as per Bjorn's suggestion.

Quoted 28 lines · Ram Pai

PCI: pci resource iterator

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.

The pci_dev structure needs to be re-organized to avoid unnecessary bloating. However too much code outside the pci-bus driver, assumes the internal details of the pci_dev structure, thus making it hard to re-organize the datastructure.

As a first step this patch provides generic methods to access the resource structure of the pci_dev.

Once this patch is accepted, I have another 40+ patches that modify all the files that directly access the resource structure, to use the new methods provided in the first step.

Finally we can re-organize the resource structure in the pci_dev structure and correspondingly update the methods.

This patch is compile tested only.

Changelog:
Consolidated iterator interface as per Bjorn's suggestion.

Review

Quoted 4 lines

(quoted diff) +4 -0

+#define for_each_pci_resource(dev, res, flag) \
+       for (res = pci_next_resource(dev, NULL, flag); res; \
+                       res = pci_next_resource(dev, res, flag))
+

We may need to keep the idx, so we could make the converting more granularity.

because some loop body is still using the idx.

also there is some abusing pci bridge resource as addon resources. and we need to remove the abusing at first ---
that is addressed by:
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=5eb48c3c998257386f67a7570778872ec600138f

PCI: Add addon_resource support for pci_dev

and later we may remove the idx in the for_each_pci_resource()

Please check updated version of your patch that keep the idx.

Quoted 1 line · Yinghai Lu

On Tue, Aug 21, 2012 at 8:13 AM, Ram Pai <[email protected]> wrote:

by exposing idx through the interface, we are exposing the implementation to the enduser. I want the end user not know that the resources are structured as a array. This will help easily restructure resources in the pci_dev structure to whatever implementation we want, linked list or hash or whatever...

Why can't the addon resource be hidden behind the interface? something like this?

Quoted 10 lines · Ram Pai

Quoted 1 line · Yinghai Lu

On Tue, Aug 21, 2012 at 8:13 AM, Ram Pai <[email protected]> wrote:

by exposing idx through the interface, we are exposing the implementation to the enduser. I want the end user not know that the resources are structured as a array. This will help easily restructure resources in the pci_dev structure to whatever implementation we want, linked list or hash or whatever...

Why can't the addon resource be hidden behind the interface? something like this?

Quoted 1 line · Yinghai Lu

keep the idx would make thing simple for referencing and converting.

pci_dev_resource_n() would be wrapper to from idx to real resource pointer. we just need to change &dev->resource[i] referencing to pci_dev_resource_n(dev, i) instead.

We did that before for irq_desc storage converting, and now we have
for_each_irq_desc(irq, desc)

later for the resource allocation, I will change resource member from

           struct resource resource[DEVICE_COUNT_RESOURCE];
 to
           struct resource std_res[7];
           struct resource *iov_res;
           struct resource *bridge_res;

and allocate iov_res and bridge array as needed.

anyway keep the overalll idx is good way to go and follow.

Hi, Ram and Bjorn,

Quoted 9 lines · Ram Pai

(quoted diff) +9 -0

+static inline int pci_next_resource_idx(int i, int flag)
+{
+       while (++i < PCI_NUM_RESOURCES) {
+               if ((i >= 0 && i < PCI_ROM_RESOURCE && (flag & PCI_STD_RES)) ||
+                   (i == PCI_ROM_RESOURCE && (flag & PCI_ROM_RES)) ||
+#ifdef CONFIG_PCI_IOV
+                   (i <= PCI_IOV_RESOURCE_END && (flag & PCI_IOV_RES)) ||
+#endif
+                   (i <= PCI_BRIDGE_RESOURCE_END && (flag & PCI_BRIDGE_RES)))

Review

Quoted 4 lines

(quoted diff) +4 -0

+                       return i;
+       }
+       return -1;
+}

no, you can not merge them.
when it start as -1, and user only need the bridge resource, it will loop from 0 to 16.

I optimized it more to skip some searching. please check v5 and v6. v5 will store aside the mask, and use the bit map later. v6 will still to do the local checking, but will skip some ++i loop.

Quoted 12 lines · Yinghai Lu

Hi, Ram and Bjorn,

Quoted 9 lines · Ram Pai

(quoted diff) +9 -0

+static inline int pci_next_resource_idx(int i, int flag)
+{
+       while (++i < PCI_NUM_RESOURCES) {
+               if ((i >= 0 && i < PCI_ROM_RESOURCE && (flag & PCI_STD_RES)) ||
+                   (i == PCI_ROM_RESOURCE && (flag & PCI_ROM_RES)) ||
+#ifdef CONFIG_PCI_IOV
+                   (i <= PCI_IOV_RESOURCE_END && (flag & PCI_IOV_RES)) ||
+#endif
+                   (i <= PCI_BRIDGE_RESOURCE_END && (flag & PCI_BRIDGE_RES)))

Quoted 8 lines

drivers/pci/probe.c static int __init pcibus_class_init(void) · line 186

Quoted 4 lines

drivers/pci/probe.c +4 -0

+                       return i;
+       }
+       return -1;
+}

no, you can not merge them.
when it start as -1, and user only need the bridge resource, it will loop from 0 to 16.

True. But the logic by itself is not wrong. It is sub-optimal.

Quoted 4 lines

I optimized it more to skip some searching. please check v5 and v6. v5 will store aside the mask, and use the bit map later. v6 will still to do the local checking, but will skip some ++i loop.

:) The v5 patch; the bitmask one, took me some time to understand. But yes it does a good job.

There is one thing I dont like in that patch. You have to call to pci_dev_resource_n(), but that function is not there anywhere in upstream tree. Its only in your tree. Can we bring pci_dev_resource_n() into this patch and make it self-sustained?

Also, please find below, one other patch proposal, which improves upon your v5 patch. Instead of having a array of bitmask, this patch maintains a single dimensional array with an entry for each resource. Each entry captures its type; ie PCI_RES_*, and index of the starting location of the next resource type. Using this information one can quickly cut down on the number of useless iterations. It also does compile time initialization of the array, unlike the v5 patch which does runtime initialization.

Please decide which version you want and lets take the next step. I think we have iterated over this interface quite a lot. Time to move on :)

---------------------------------------------------------------------

Signed-off-by Ram Pai <[email protected]>

drivers/pci/probe.c +15 -0

index 5e1ca3c..d95a31c 100644
@@ -146,6 +146,54 @@ static int __init pcibus_class_init(void)
 }
 postcore_initcall(pcibus_class_init);
 
+
+#define PCI_RES_SHIFT 4
+/*
+ * capture the starting index of the next resource type
+ * as well as the type of the resource
+ */
+int res_idx[PCI_NUM_RESOURCES] = {
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES   */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+1 */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+2 */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+3 */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+4 */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+5 */
+(PCI_IOV_RESOURCES << PCI_RES_SHIFT) | PCI_ROM_RES, /* PCI_ROM_RESOURCE */
+(PCI_BRIDGE_RESOURCES << PCI_RES_SHIFT) | PCI_IOV_RES, /* PCI_IOV_START */

drivers/pci/probe.c +12 -0

+       return -1;
+}
+
+struct resource *pci_dev_resource_n(struct pci_dev *dev, int n)
+{
+       if (n >= 0 && n < PCI_NUM_RESOURCES)
+               return &dev->resource[n];
+
+       return NULL;
+}
+
+
 static u64 pci_size(u64 base, u64 maxbase, u64 mask)
 {
        u64 size = mask & maxbase;      /* Find the significant bits */

include/linux/pci.h +19 -0

index e444f5b..adae706 100644
@@ -1351,6 +1351,25 @@ static inline int pci_domain_nr(struct pci_bus *bus)
         (pci_resource_end((dev), (bar)) -              \
          pci_resource_start((dev), (bar)) + 1))
 
+#define PCI_STD_RES            (1<<0)
+#define PCI_ROM_RES            (1<<1)
+#define PCI_BRIDGE_RES         (1<<2)
+#define PCI_IOV_RES            (1<<3)
+#define PCI_ALL_RES    (PCI_STD_RES|PCI_ROM_RES|PCI_BRIDGE_RES|PCI_IOV_RES)
+#define PCI_NOSTD_RES          (PCI_ALL_RES & ~PCI_STD_RES)
+#define PCI_NOIOV_RES          (PCI_ALL_RES & ~PCI_IOV_RES)
+#define PCI_NOROM_RES          (PCI_ALL_RES & ~PCI_ROM_RES)
+#define PCI_NOBRIDGE_RES       (PCI_ALL_RES & ~PCI_BRIDGE_RES)
+#define PCI_STD_ROM_RES                (PCI_STD_RES | PCI_ROM_RES)
+#define PCI_STD_IOV_RES                (PCI_STD_RES | PCI_IOV_RES)
+#define PCI_STD_ROM_IOV_RES    (PCI_NOBRIDGE_RES)
+#define for_each_pci_resource(dev, res, flag) \
+       for (i = pci_next_resource_idx(-1, flag), \
+                       res = pci_dev_resource_n(dev, i); \
+            res; \
+            i = pci_next_resource_idx(i, flag), \
+                       res = pci_dev_resource_n(dev, i))
+
 /* Similar to the helpers above, these manipulate per-pci_dev
  * driver-specific data.  They are really just a wrapper around
  * the generic device structure functions of these calls.

Quoted 18 lines · Ram Pai

Quoted 17 lines · Yinghai Lu

Hi, Ram and Bjorn,

Quoted 13 lines · Ram Pai

(quoted diff) +13 -0

+static inline int pci_next_resource_idx(int i, int flag)
+{
+       while (++i < PCI_NUM_RESOURCES) {
+               if ((i >= 0 && i < PCI_ROM_RESOURCE && (flag & PCI_STD_RES)) ||
+                   (i == PCI_ROM_RESOURCE && (flag & PCI_ROM_RES)) ||
+#ifdef CONFIG_PCI_IOV
+                   (i <= PCI_IOV_RESOURCE_END && (flag & PCI_IOV_RES)) ||
+#endif
+                   (i <= PCI_BRIDGE_RESOURCE_END && (flag & PCI_BRIDGE_RES)))
+                       return i;
+       }
+       return -1;
+}

Quoted 36 lines

Please decide which version you want and lets take the next step. I think we have iterated over this interface quite a lot. Time to move on :)

---------------------------------------------------------------------

Signed-off-by Ram Pai <[email protected]>

drivers/pci/probe.c +19 -0

index 5e1ca3c..d95a31c 100644
@@ -146,6 +146,54 @@ static int __init pcibus_class_init(void)
 }
 postcore_initcall(pcibus_class_init);
 
+
+#define PCI_RES_SHIFT 4
+/*
+ * capture the starting index of the next resource type
+ * as well as the type of the resource
+ */
+int res_idx[PCI_NUM_RESOURCES] = {
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES   */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+1 */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+2 */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+3 */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+4 */
+(PCI_ROM_RESOURCE << PCI_RES_SHIFT) | PCI_STD_RES, /* PCI_STD_RESOURCES+5 */
+(PCI_IOV_RESOURCES << PCI_RES_SHIFT) | PCI_ROM_RES, /* PCI_ROM_RESOURCE */
+(PCI_BRIDGE_RESOURCES << PCI_RES_SHIFT) | PCI_IOV_RES, /* PCI_IOV_START */
+(PCI_BRIDGE_RESOURCES << PCI_RES_SHIFT) | PCI_IOV_RES, /* PCI_IOV_START+1 */
+(PCI_BRIDGE_RESOURCES << PCI_RES_SHIFT) | PCI_IOV_RES, /* PCI_IOV_START+2 */
+(PCI_BRIDGE_RESOURCES << PCI_RES_SHIFT) | PCI_IOV_RES, /* PCI_IOV_START+3 */
+(PCI_BRIDGE_RESOURCES << PCI_RES_SHIFT) | PCI_IOV_RES, /* PCI_IOV_START+4 */

drivers/pci/probe.c static int __init pcibus_class_init(void) · line 173

Quoted 6 lines

drivers/pci/probe.c +6 -0

+(PCI_BRIDGE_RESOURCES << PCI_RES_SHIFT) | PCI_IOV_RES, /* PCI_IOV_START+5 */
+(PCI_NUM_RESOURCES << PCI_RES_SHIFT) | PCI_BRIDGE_RES, /* PCI_BRIDGE_START */
+(PCI_NUM_RESOURCES << PCI_RES_SHIFT) | PCI_BRIDGE_RES, /* PCI_BRIDGE_START+1 */
+(PCI_NUM_RESOURCES << PCI_RES_SHIFT) | PCI_BRIDGE_RES, /* PCI_BRIDGE_START+2 */
+(PCI_NUM_RESOURCES << PCI_RES_SHIFT) | PCI_BRIDGE_RES, /* PCI_BRIDGE_START+3 */
+};

how about some one change the some type resource number?

better have some marco helper.

drivers/pci/probe.c static int __init pcibus_class_init(void) · line 184

Quoted 11 lines

drivers/pci/probe.c +11 -0

+#define res_type(idx)  (res_idx[idx] &  ((0x1 << PCI_RES_SHIFT) - 1))
+#define res_next(idx)  (res_idx[idx] >> PCI_RES_SHIFT)
+
+int pci_next_resource_idx(int i, int flag)
+{
+       i++;
+       while (i < PCI_NUM_RESOURCES) {
+               if (res_type(i) & flag)
+                       return i;
+               i = res_next(i);
+       }

here still have some two extra loop when only need to loop STD + BRIDGE.

On Mon, Sep 03, 2012 at 01:07:46AM -0700, Yinghai Lu wrote:

Anyway I am ok with either patch.

Quoted 3 lines · Ram Pai

On Mon, Sep 03, 2012 at 01:07:46AM -0700, Yinghai Lu wrote:

Anyway I am ok with either patch.

please check -v7.

Quoted 3 lines · Yinghai Lu

Quoted 2 lines · Ram Pai

On Mon, Sep 03, 2012 at 01:07:46AM -0700, Yinghai Lu wrote:

Quoted 3 lines

Quoted 1 line

Anyway I am ok with either patch.

please check -v7.

Looks good to me. I am inlining the patch for others to comment.
BTW: your patch that introduces pci_dev_resource_n() will have to be applied before applying this patch.

Quoted 4 lines · Ram Pai

Quoted 3 lines · Yinghai Lu

Quoted 2 lines · Ram Pai

On Mon, Sep 03, 2012 at 01:07:46AM -0700, Yinghai Lu wrote:

Quoted 9 lines

Quoted 3 lines

Quoted 1 line

Anyway I am ok with either patch.

please check -v7.

Looks good to me. I am inlining the patch for others to comment.
BTW: your patch that introduces pci_dev_resource_n() will have to be applied before applying this patch.

just updated for-pci-for-each-res-addon branch...

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-for-each-res-addon

now it only depends on pci-root-bus-hotplug now, and pci-root-bus-hotplug only depends on pci/next without busn-alloc.

So after Bjorn accept pci-root-bus-hotplug, we could push that to him.

Please check for-pci-for-each-addon branch.
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-for-each-res-addon

Checked and the patch looks good.

But later you have added another patch which introduces lot many macros ...

Will that patch also be pushed to Bjorn? I thought Bjorn resisted the idea of having multiple such iterators..