| Message ID | 1363217302-14383-4-git-send-email-yinghai@kernel.org |
|---|---|
| State | Changes Requested |
| Headers | show |
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. > > 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. > > Finally we can re-organize the resource structure in the pci_dev > structure and correspondingly update the methods. > > -v2: Consolidated iterator interface as per Bjorn's suggestion. > -v3: Add the idx back - Yinghai Lu > -v7: Change to use bitmap for searching - Yinghai Lu > -v8: Fix acpiphp module compiling error that is found by > Steven Newbury <steve@snewbury.org.uk> - Yinghai Lu > > Signed-off-by: Ram Pai <linuxram@us.ibm.com> > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > --- > drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/pci.h | 24 ++++++++++++++++++++++++ > 2 files changed, 72 insertions(+) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 1df75f7..ac751a6 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -123,6 +123,54 @@ int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res) > return -1; > } > > +static void __init_res_idx_mask(unsigned long *mask, int flag) > +{ > + bitmap_zero(mask, PCI_NUM_RESOURCES); > + if (flag & PCI_STD_RES) > + bitmap_set(mask, PCI_STD_RESOURCES, > + PCI_STD_RESOURCE_END - PCI_STD_RESOURCES + 1); > + if (flag & PCI_ROM_RES) > + bitmap_set(mask, PCI_ROM_RESOURCE, 1); > +#ifdef CONFIG_PCI_IOV > + if (flag & PCI_IOV_RES) > + bitmap_set(mask, PCI_IOV_RESOURCES, > + PCI_IOV_RESOURCE_END - PCI_IOV_RESOURCES + 1); > +#endif > + if (flag & PCI_BRIDGE_RES) > + bitmap_set(mask, PCI_BRIDGE_RESOURCES, > + PCI_BRIDGE_RESOURCE_END - PCI_BRIDGE_RESOURCES + 1); > +} > + > +static DECLARE_BITMAP(res_idx_mask[1 << PCI_RES_BLOCK_NUM], PCI_NUM_RESOURCES); > +static int __init pci_res_idx_mask_init(void) > +{ > + int i; > + > + for (i = 0; i < (1 << PCI_RES_BLOCK_NUM); i++) > + __init_res_idx_mask(res_idx_mask[i], i); > + > + return 0; > +} > +postcore_initcall(pci_res_idx_mask_init); > + > +static inline unsigned long *get_res_idx_mask(int flag) > +{ > + return res_idx_mask[flag & ((1 << PCI_RES_BLOCK_NUM) - 1)]; > +} > + > +int pci_next_resource_idx(int i, int flag) > +{ > + i++; > + if (i < PCI_NUM_RESOURCES) > + i = find_next_bit(get_res_idx_mask(flag), PCI_NUM_RESOURCES, i); > + > + if (i < PCI_NUM_RESOURCES) > + return i; > + > + return -1; > +} > +EXPORT_SYMBOL(pci_next_resource_idx); > + > static u64 pci_size(u64 base, u64 maxbase, u64 mask) > { > u64 size = mask & maxbase; /* Find the significant bits */ > diff --git a/include/linux/pci.h b/include/linux/pci.h > index aefff8b..127a856 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -341,6 +341,30 @@ struct pci_dev { > struct resource *pci_dev_resource_n(struct pci_dev *dev, int n); > int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res); > > +#define PCI_STD_RES (1<<0) > +#define PCI_ROM_RES (1<<1) > +#define PCI_IOV_RES (1<<2) > +#define PCI_BRIDGE_RES (1<<3) > +#define PCI_RES_BLOCK_NUM 4 > + > +#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_STD_RES | PCI_ROM_RES | PCI_IOV_RES) > + > +int pci_next_resource_idx(int i, int flag); > + > +#define for_each_pci_resource(dev, res, i, 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)) > + > static inline struct pci_dev *pci_physfn(struct pci_dev *dev) > { > #ifdef CONFIG_PCI_IOV I think this turned out to be a disaster, with all the bitmaps and helper functions. Filtering in the bodies of the for_each_pci_resource() users has *got* to be better. That probably requires a wrapper struct around the struct resource. Or possibly having a wrapper struct with a "type" or "flags" field would make filtering in for_each_pci_resources() itself cleaner to implement. I think it would also help if we got rid of all the "PCI_NO*_RES" definitions and just had the users explicitly specify what they *do* want. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > > > > 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. > > > > Finally we can re-organize the resource structure in the pci_dev > > structure and correspondingly update the methods. > > > > -v2: Consolidated iterator interface as per Bjorn's suggestion. > > -v3: Add the idx back - Yinghai Lu > > -v7: Change to use bitmap for searching - Yinghai Lu > > -v8: Fix acpiphp module compiling error that is found by > > Steven Newbury <steve@snewbury.org.uk> - Yinghai Lu > > > > Signed-off-by: Ram Pai <linuxram@us.ibm.com> > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > > --- > > drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/pci.h | 24 ++++++++++++++++++++++++ > > 2 files changed, 72 insertions(+) > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index 1df75f7..ac751a6 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -123,6 +123,54 @@ int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res) > > return -1; > > } > > > > +static void __init_res_idx_mask(unsigned long *mask, int flag) > > +{ > > + bitmap_zero(mask, PCI_NUM_RESOURCES); > > + if (flag & PCI_STD_RES) > > + bitmap_set(mask, PCI_STD_RESOURCES, > > + PCI_STD_RESOURCE_END - PCI_STD_RESOURCES + 1); > > + if (flag & PCI_ROM_RES) > > + bitmap_set(mask, PCI_ROM_RESOURCE, 1); > > +#ifdef CONFIG_PCI_IOV > > + if (flag & PCI_IOV_RES) > > + bitmap_set(mask, PCI_IOV_RESOURCES, > > + PCI_IOV_RESOURCE_END - PCI_IOV_RESOURCES + 1); > > +#endif > > + if (flag & PCI_BRIDGE_RES) > > + bitmap_set(mask, PCI_BRIDGE_RESOURCES, > > + PCI_BRIDGE_RESOURCE_END - PCI_BRIDGE_RESOURCES + 1); > > +} > > + > > +static DECLARE_BITMAP(res_idx_mask[1 << PCI_RES_BLOCK_NUM], PCI_NUM_RESOURCES); > > +static int __init pci_res_idx_mask_init(void) > > +{ > > + int i; > > + > > + for (i = 0; i < (1 << PCI_RES_BLOCK_NUM); i++) > > + __init_res_idx_mask(res_idx_mask[i], i); > > + > > + return 0; > > +} > > +postcore_initcall(pci_res_idx_mask_init); > > + > > +static inline unsigned long *get_res_idx_mask(int flag) > > +{ > > + return res_idx_mask[flag & ((1 << PCI_RES_BLOCK_NUM) - 1)]; > > +} > > + > > +int pci_next_resource_idx(int i, int flag) > > +{ > > + i++; > > + if (i < PCI_NUM_RESOURCES) > > + i = find_next_bit(get_res_idx_mask(flag), PCI_NUM_RESOURCES, i); > > + > > + if (i < PCI_NUM_RESOURCES) > > + return i; > > + > > + return -1; > > +} > > +EXPORT_SYMBOL(pci_next_resource_idx); > > + > > static u64 pci_size(u64 base, u64 maxbase, u64 mask) > > { > > u64 size = mask & maxbase; /* Find the significant bits */ > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index aefff8b..127a856 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -341,6 +341,30 @@ struct pci_dev { > > struct resource *pci_dev_resource_n(struct pci_dev *dev, int n); > > int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res); > > > > +#define PCI_STD_RES (1<<0) > > +#define PCI_ROM_RES (1<<1) > > +#define PCI_IOV_RES (1<<2) > > +#define PCI_BRIDGE_RES (1<<3) > > +#define PCI_RES_BLOCK_NUM 4 > > + > > +#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_STD_RES | PCI_ROM_RES | PCI_IOV_RES) > > + > > +int pci_next_resource_idx(int i, int flag); > > + > > +#define for_each_pci_resource(dev, res, i, 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)) > > + > > static inline struct pci_dev *pci_physfn(struct pci_dev *dev) > > { > > #ifdef CONFIG_PCI_IOV > > I think this turned out to be a disaster, with all the bitmaps and > helper functions. Filtering in the bodies of the > for_each_pci_resource() users has *got* to be better. That probably > requires a wrapper struct around the struct resource. Or possibly > having a wrapper struct with a "type" or "flags" field would make > filtering in for_each_pci_resources() itself cleaner to implement. I agree. There are two cleanups needed. a) pci drivers should not assume the internal organization of the resources in the struct pci_dev. b) The type of a resource has to be determined based on some information internal to the resource; possibly a flag, instead of the relative position of the resource in some array. My original patch was aimed at (a); to make the organization of the resources transparent to the drivers, and then move to (b) where the type of the resource would be determined based on some flag, instead of its relative location. Unfortuately the current newer version has got morphed significantly, that I think is not clean enough. RP -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. >> > >> > 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. >> > >> > Finally we can re-organize the resource structure in the pci_dev >> > structure and correspondingly update the methods. >> > >> > -v2: Consolidated iterator interface as per Bjorn's suggestion. >> > -v3: Add the idx back - Yinghai Lu >> > -v7: Change to use bitmap for searching - Yinghai Lu >> > -v8: Fix acpiphp module compiling error that is found by >> > Steven Newbury <steve@snewbury.org.uk> - Yinghai Lu >> > >> > Signed-off-by: Ram Pai <linuxram@us.ibm.com> >> > Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> > --- >> > drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ >> > include/linux/pci.h | 24 ++++++++++++++++++++++++ >> > 2 files changed, 72 insertions(+) >> > >> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> > index 1df75f7..ac751a6 100644 >> > --- a/drivers/pci/probe.c >> > +++ b/drivers/pci/probe.c >> > @@ -123,6 +123,54 @@ int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res) >> > return -1; >> > } >> > >> > +static void __init_res_idx_mask(unsigned long *mask, int flag) >> > +{ >> > + bitmap_zero(mask, PCI_NUM_RESOURCES); >> > + if (flag & PCI_STD_RES) >> > + bitmap_set(mask, PCI_STD_RESOURCES, >> > + PCI_STD_RESOURCE_END - PCI_STD_RESOURCES + 1); >> > + if (flag & PCI_ROM_RES) >> > + bitmap_set(mask, PCI_ROM_RESOURCE, 1); >> > +#ifdef CONFIG_PCI_IOV >> > + if (flag & PCI_IOV_RES) >> > + bitmap_set(mask, PCI_IOV_RESOURCES, >> > + PCI_IOV_RESOURCE_END - PCI_IOV_RESOURCES + 1); >> > +#endif >> > + if (flag & PCI_BRIDGE_RES) >> > + bitmap_set(mask, PCI_BRIDGE_RESOURCES, >> > + PCI_BRIDGE_RESOURCE_END - PCI_BRIDGE_RESOURCES + 1); >> > +} >> > + >> > +static DECLARE_BITMAP(res_idx_mask[1 << PCI_RES_BLOCK_NUM], PCI_NUM_RESOURCES); >> > +static int __init pci_res_idx_mask_init(void) >> > +{ >> > + int i; >> > + >> > + for (i = 0; i < (1 << PCI_RES_BLOCK_NUM); i++) >> > + __init_res_idx_mask(res_idx_mask[i], i); >> > + >> > + return 0; >> > +} >> > +postcore_initcall(pci_res_idx_mask_init); >> > + >> > +static inline unsigned long *get_res_idx_mask(int flag) >> > +{ >> > + return res_idx_mask[flag & ((1 << PCI_RES_BLOCK_NUM) - 1)]; >> > +} >> > + >> > +int pci_next_resource_idx(int i, int flag) >> > +{ >> > + i++; >> > + if (i < PCI_NUM_RESOURCES) >> > + i = find_next_bit(get_res_idx_mask(flag), PCI_NUM_RESOURCES, i); >> > + >> > + if (i < PCI_NUM_RESOURCES) >> > + return i; >> > + >> > + return -1; >> > +} >> > +EXPORT_SYMBOL(pci_next_resource_idx); >> > + >> > static u64 pci_size(u64 base, u64 maxbase, u64 mask) >> > { >> > u64 size = mask & maxbase; /* Find the significant bits */ >> > diff --git a/include/linux/pci.h b/include/linux/pci.h >> > index aefff8b..127a856 100644 >> > --- a/include/linux/pci.h >> > +++ b/include/linux/pci.h >> > @@ -341,6 +341,30 @@ struct pci_dev { >> > struct resource *pci_dev_resource_n(struct pci_dev *dev, int n); >> > int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res); >> > >> > +#define PCI_STD_RES (1<<0) >> > +#define PCI_ROM_RES (1<<1) >> > +#define PCI_IOV_RES (1<<2) >> > +#define PCI_BRIDGE_RES (1<<3) >> > +#define PCI_RES_BLOCK_NUM 4 >> > + >> > +#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_STD_RES | PCI_ROM_RES | PCI_IOV_RES) >> > + >> > +int pci_next_resource_idx(int i, int flag); >> > + >> > +#define for_each_pci_resource(dev, res, i, 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)) >> > + >> > static inline struct pci_dev *pci_physfn(struct pci_dev *dev) >> > { >> > #ifdef CONFIG_PCI_IOV >> >> I think this turned out to be a disaster, with all the bitmaps and >> helper functions. Filtering in the bodies of the >> for_each_pci_resource() users has *got* to be better. That probably >> requires a wrapper struct around the struct resource. Or possibly >> having a wrapper struct with a "type" or "flags" field would make >> filtering in for_each_pci_resources() itself cleaner to implement. > > 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. The number of places that actually look at *non-BAR* pci_dev resources, e.g., the places that look at bridge windows and SR-IOV BARs, should be pretty small, and it seems reasonable to change them. > b) The type of a resource has to be determined based on some > information internal to the resource; possibly a flag, > instead of the relative position of the resource in some array. Yes, I agree. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 4, 2013 at 3:18 PM, Bjorn Helgaas <bhelgaas@google.com> 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. >> >> 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. >> >> Finally we can re-organize the resource structure in the pci_dev >> structure and correspondingly update the methods. >> >> -v2: Consolidated iterator interface as per Bjorn's suggestion. >> -v3: Add the idx back - Yinghai Lu >> -v7: Change to use bitmap for searching - Yinghai Lu >> -v8: Fix acpiphp module compiling error that is found by >> Steven Newbury <steve@snewbury.org.uk> - Yinghai Lu >> >> Signed-off-by: Ram Pai <linuxram@us.ibm.com> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> --- >> drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/pci.h | 24 ++++++++++++++++++++++++ >> 2 files changed, 72 insertions(+) >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index 1df75f7..ac751a6 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -123,6 +123,54 @@ int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res) >> return -1; >> } >> >> +static void __init_res_idx_mask(unsigned long *mask, int flag) >> +{ >> + bitmap_zero(mask, PCI_NUM_RESOURCES); >> + if (flag & PCI_STD_RES) >> + bitmap_set(mask, PCI_STD_RESOURCES, >> + PCI_STD_RESOURCE_END - PCI_STD_RESOURCES + 1); >> + if (flag & PCI_ROM_RES) >> + bitmap_set(mask, PCI_ROM_RESOURCE, 1); >> +#ifdef CONFIG_PCI_IOV >> + if (flag & PCI_IOV_RES) >> + bitmap_set(mask, PCI_IOV_RESOURCES, >> + PCI_IOV_RESOURCE_END - PCI_IOV_RESOURCES + 1); >> +#endif >> + if (flag & PCI_BRIDGE_RES) >> + bitmap_set(mask, PCI_BRIDGE_RESOURCES, >> + PCI_BRIDGE_RESOURCE_END - PCI_BRIDGE_RESOURCES + 1); >> +} >> + >> +static DECLARE_BITMAP(res_idx_mask[1 << PCI_RES_BLOCK_NUM], PCI_NUM_RESOURCES); >> +static int __init pci_res_idx_mask_init(void) >> +{ >> + int i; >> + >> + for (i = 0; i < (1 << PCI_RES_BLOCK_NUM); i++) >> + __init_res_idx_mask(res_idx_mask[i], i); >> + >> + return 0; >> +} >> +postcore_initcall(pci_res_idx_mask_init); >> + >> +static inline unsigned long *get_res_idx_mask(int flag) >> +{ >> + return res_idx_mask[flag & ((1 << PCI_RES_BLOCK_NUM) - 1)]; >> +} >> + >> +int pci_next_resource_idx(int i, int flag) >> +{ >> + i++; >> + if (i < PCI_NUM_RESOURCES) >> + i = find_next_bit(get_res_idx_mask(flag), PCI_NUM_RESOURCES, i); >> + >> + if (i < PCI_NUM_RESOURCES) >> + return i; >> + >> + return -1; >> +} >> +EXPORT_SYMBOL(pci_next_resource_idx); >> + >> static u64 pci_size(u64 base, u64 maxbase, u64 mask) >> { >> u64 size = mask & maxbase; /* Find the significant bits */ >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index aefff8b..127a856 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -341,6 +341,30 @@ struct pci_dev { >> struct resource *pci_dev_resource_n(struct pci_dev *dev, int n); >> int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res); >> >> +#define PCI_STD_RES (1<<0) >> +#define PCI_ROM_RES (1<<1) >> +#define PCI_IOV_RES (1<<2) >> +#define PCI_BRIDGE_RES (1<<3) >> +#define PCI_RES_BLOCK_NUM 4 >> + >> +#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_STD_RES | PCI_ROM_RES | PCI_IOV_RES) >> + >> +int pci_next_resource_idx(int i, int flag); >> + >> +#define for_each_pci_resource(dev, res, i, 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)) >> + >> static inline struct pci_dev *pci_physfn(struct pci_dev *dev) >> { >> #ifdef CONFIG_PCI_IOV > > I think this turned out to be a disaster, with all the bitmaps and > helper functions. Filtering in the bodies of the > for_each_pci_resource() users has *got* to be better. That probably > requires a wrapper struct around the struct resource. Or possibly > having a wrapper struct with a "type" or "flags" field would make > filtering in for_each_pci_resources() itself cleaner to implement. Do you mean have extra struct that will wrapper resource ? and the struct will have type that will tell that resource is PCI bridge resource or other std/sriov resource? then that struct need to have BAR in it. that looks like addon_res that is added in this patchset. but for the std/bridge/sriov, idx is used all the way and in all over the drivers. > > I think it would also help if we got rid of all the "PCI_NO*_RES" > definitions and just had the users explicitly specify what they *do* > want. yes, remove PCI_NO*_RES should be more clean Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > >> > > >> > 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. > >> > > >> > Finally we can re-organize the resource structure in the pci_dev > >> > structure and correspondingly update the methods. > >> > > >> > -v2: Consolidated iterator interface as per Bjorn's suggestion. > >> > -v3: Add the idx back - Yinghai Lu > >> > -v7: Change to use bitmap for searching - Yinghai Lu > >> > -v8: Fix acpiphp module compiling error that is found by > >> > Steven Newbury <steve@snewbury.org.uk> - Yinghai Lu > >> > > >> > Signed-off-by: Ram Pai <linuxram@us.ibm.com> > >> > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > >> > --- > >> > drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > >> > include/linux/pci.h | 24 ++++++++++++++++++++++++ > >> > 2 files changed, 72 insertions(+) > >> > > >> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > >> > index 1df75f7..ac751a6 100644 > >> > --- a/drivers/pci/probe.c > >> > +++ b/drivers/pci/probe.c > >> > @@ -123,6 +123,54 @@ int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res) > >> > return -1; > >> > } > >> > > >> > +static void __init_res_idx_mask(unsigned long *mask, int flag) > >> > +{ > >> > + bitmap_zero(mask, PCI_NUM_RESOURCES); > >> > + if (flag & PCI_STD_RES) > >> > + bitmap_set(mask, PCI_STD_RESOURCES, > >> > + PCI_STD_RESOURCE_END - PCI_STD_RESOURCES + 1); > >> > + if (flag & PCI_ROM_RES) > >> > + bitmap_set(mask, PCI_ROM_RESOURCE, 1); > >> > +#ifdef CONFIG_PCI_IOV > >> > + if (flag & PCI_IOV_RES) > >> > + bitmap_set(mask, PCI_IOV_RESOURCES, > >> > + PCI_IOV_RESOURCE_END - PCI_IOV_RESOURCES + 1); > >> > +#endif > >> > + if (flag & PCI_BRIDGE_RES) > >> > + bitmap_set(mask, PCI_BRIDGE_RESOURCES, > >> > + PCI_BRIDGE_RESOURCE_END - PCI_BRIDGE_RESOURCES + 1); > >> > +} > >> > + > >> > +static DECLARE_BITMAP(res_idx_mask[1 << PCI_RES_BLOCK_NUM], PCI_NUM_RESOURCES); > >> > +static int __init pci_res_idx_mask_init(void) > >> > +{ > >> > + int i; > >> > + > >> > + for (i = 0; i < (1 << PCI_RES_BLOCK_NUM); i++) > >> > + __init_res_idx_mask(res_idx_mask[i], i); > >> > + > >> > + return 0; > >> > +} > >> > +postcore_initcall(pci_res_idx_mask_init); > >> > + > >> > +static inline unsigned long *get_res_idx_mask(int flag) > >> > +{ > >> > + return res_idx_mask[flag & ((1 << PCI_RES_BLOCK_NUM) - 1)]; > >> > +} > >> > + > >> > +int pci_next_resource_idx(int i, int flag) > >> > +{ > >> > + i++; > >> > + if (i < PCI_NUM_RESOURCES) > >> > + i = find_next_bit(get_res_idx_mask(flag), PCI_NUM_RESOURCES, i); > >> > + > >> > + if (i < PCI_NUM_RESOURCES) > >> > + return i; > >> > + > >> > + return -1; > >> > +} > >> > +EXPORT_SYMBOL(pci_next_resource_idx); > >> > + > >> > static u64 pci_size(u64 base, u64 maxbase, u64 mask) > >> > { > >> > u64 size = mask & maxbase; /* Find the significant bits */ > >> > diff --git a/include/linux/pci.h b/include/linux/pci.h > >> > index aefff8b..127a856 100644 > >> > --- a/include/linux/pci.h > >> > +++ b/include/linux/pci.h > >> > @@ -341,6 +341,30 @@ struct pci_dev { > >> > struct resource *pci_dev_resource_n(struct pci_dev *dev, int n); > >> > int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res); > >> > > >> > +#define PCI_STD_RES (1<<0) > >> > +#define PCI_ROM_RES (1<<1) > >> > +#define PCI_IOV_RES (1<<2) > >> > +#define PCI_BRIDGE_RES (1<<3) > >> > +#define PCI_RES_BLOCK_NUM 4 > >> > + > >> > +#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_STD_RES | PCI_ROM_RES | PCI_IOV_RES) > >> > + > >> > +int pci_next_resource_idx(int i, int flag); > >> > + > >> > +#define for_each_pci_resource(dev, res, i, 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)) > >> > + > >> > static inline struct pci_dev *pci_physfn(struct pci_dev *dev) > >> > { > >> > #ifdef CONFIG_PCI_IOV > >> > >> I think this turned out to be a disaster, with all the bitmaps and > >> helper functions. Filtering in the bodies of the > >> for_each_pci_resource() users has *got* to be better. That probably > >> requires a wrapper struct around the struct resource. Or possibly > >> having a wrapper struct with a "type" or "flags" field would make > >> filtering in for_each_pci_resources() itself cleaner to implement. > > > > 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 b) return the nth iov resource c) return the rom resource d) return the nth bridge resource 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. Everything else needs to be hidden. > > The number of places that actually look at *non-BAR* pci_dev > resources, e.g., the places that look at bridge windows and SR-IOV > BARs, should be pretty small, and it seems reasonable to change them. Unfortunately, the bridge windows and SRIOV windows are currently inter-twined with normal windows. Just cleaning up that code still leaves some ugly hacks around. > > > b) The type of a resource has to be determined based on some > > information internal to the resource; possibly a flag, > > instead of the relative position of the resource in some array. > > Yes, I agree. > > Bjorn
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 25, 2013 at 11:22:59AM -0600, Bjorn Helgaas wrote: > 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. 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-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 1df75f7..ac751a6 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -123,6 +123,54 @@ int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res) return -1; } +static void __init_res_idx_mask(unsigned long *mask, int flag) +{ + bitmap_zero(mask, PCI_NUM_RESOURCES); + if (flag & PCI_STD_RES) + bitmap_set(mask, PCI_STD_RESOURCES, + PCI_STD_RESOURCE_END - PCI_STD_RESOURCES + 1); + if (flag & PCI_ROM_RES) + bitmap_set(mask, PCI_ROM_RESOURCE, 1); +#ifdef CONFIG_PCI_IOV + if (flag & PCI_IOV_RES) + bitmap_set(mask, PCI_IOV_RESOURCES, + PCI_IOV_RESOURCE_END - PCI_IOV_RESOURCES + 1); +#endif + if (flag & PCI_BRIDGE_RES) + bitmap_set(mask, PCI_BRIDGE_RESOURCES, + PCI_BRIDGE_RESOURCE_END - PCI_BRIDGE_RESOURCES + 1); +} + +static DECLARE_BITMAP(res_idx_mask[1 << PCI_RES_BLOCK_NUM], PCI_NUM_RESOURCES); +static int __init pci_res_idx_mask_init(void) +{ + int i; + + for (i = 0; i < (1 << PCI_RES_BLOCK_NUM); i++) + __init_res_idx_mask(res_idx_mask[i], i); + + return 0; +} +postcore_initcall(pci_res_idx_mask_init); + +static inline unsigned long *get_res_idx_mask(int flag) +{ + return res_idx_mask[flag & ((1 << PCI_RES_BLOCK_NUM) - 1)]; +} + +int pci_next_resource_idx(int i, int flag) +{ + i++; + if (i < PCI_NUM_RESOURCES) + i = find_next_bit(get_res_idx_mask(flag), PCI_NUM_RESOURCES, i); + + if (i < PCI_NUM_RESOURCES) + return i; + + return -1; +} +EXPORT_SYMBOL(pci_next_resource_idx); + static u64 pci_size(u64 base, u64 maxbase, u64 mask) { u64 size = mask & maxbase; /* Find the significant bits */ diff --git a/include/linux/pci.h b/include/linux/pci.h index aefff8b..127a856 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -341,6 +341,30 @@ struct pci_dev { struct resource *pci_dev_resource_n(struct pci_dev *dev, int n); int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res); +#define PCI_STD_RES (1<<0) +#define PCI_ROM_RES (1<<1) +#define PCI_IOV_RES (1<<2) +#define PCI_BRIDGE_RES (1<<3) +#define PCI_RES_BLOCK_NUM 4 + +#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_STD_RES | PCI_ROM_RES | PCI_IOV_RES) + +int pci_next_resource_idx(int i, int flag); + +#define for_each_pci_resource(dev, res, i, 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)) + static inline struct pci_dev *pci_physfn(struct pci_dev *dev) { #ifdef CONFIG_PCI_IOV