PCI: Add for_each_pci_resource and addon_res support
Subsystems (6)
v3 cover PCI: Add for_each_pci_resource and addon_res support
First part is for_each_pci_resource support:
Now pci device resource iteration is done via "for (i=0...)" open code. That make code hard to read esp when only bridge or sriov resources are involved.
We want to replace those open code with for_each_pci_resource() to make the code more readable.
Also will add addon_resource handling, and need to make addon resource to be treated as normal PCI resources during iteration, add for_each_pci_resource will make that support transition more simple.
To make for_each_pci_resource more efficient. We will use preset bitmap of different type for searching next idx.
Second parts is addon_res support:
Now there is some non normal pci resources other than standard,rom, sriov, bridges.
Some could be same as standard reg, but using different position. Some could have own way to read/write to them.
Kernel are using different way to hack those resources like abusing pci bridge resource spot on non bridge pci device.
Add addon_resources list in pci_dev for those non-standard resources. With this patch, will treat addon-resource like standard resources with special ops.
At last replace some quirk_io with addon_res.
could get from
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-for-each-res-addon-v3
on top of v3.9-rc2
-v3: rebase on of v3.9-rc2
Thanks
Yinghai Lu
Ram Pai (1):
PCI: pci resource iterator
Yinghai Lu (26):
PCI: Add pci_dev_resource_n()
PCI: Add pci_dev_resource_idx() helper
PCI: Add is_pci_*_resource_idx() helpers
PCI: Update pci_resource_start etc to use pci_dev_resource_n() PCI, x86: Use for_each_pci_resource() with pci_allocate_bridge_resources PCI, x86: Use for_each_pci_resource() with pci_allocate_dev_resources
PCI: Use for_each_pci_resource() with IOV releated functions PCI, acpiphp: Use for_each_pci_resource() helper
PCI, pciehp: Use for_each_pci_resource() helper
PCI: Use for_each_pci_resource() in pci_enable_dev
PCI: Use for_each_pci_resource() in pci_reassigndev
PCI: Use for_each_pci_resource() with pci bar reassign funcs
PCI: Use for_each_pci_resource() in pci_assign_resource
PCI, x86: Use for_each_pci_resource() with noassign_bars
PCI: Use for_each_pci_resource() in pci_dev_driver()
PCI: Use for_each_pci_resource() in pci resource release
PCI: Use for_each_pci_resource() in pci bases reading
PCI, x86: Use for_each_pci_resource() with mrst
PCI, xen: Use for_each_pci_resource() with xen pci
PCI: Add addon_resource support for pci devices
PCI: Add helpers to add addon_resource
PCI: Update pci_resource_bar() to support addon_resource
PCI: Assign/update resource to addon_res
PCI: Make piix4 quirk to use addon_res
PCI: Make quirk_io_region to use addon_res
PCI: Use addon_fixed_resource with ati fixed resource
arch/x86/pci/common.c | 3 +- arch/x86/pci/i386.c | 58 +++----- arch/x86/pci/mrst.c | 7 +- drivers/pci/hotplug/acpiphp_glue.c | 4 +- drivers/pci/hotplug/pciehp_hpc.c | 5 +- drivers/pci/iov.c | 31 +++-- drivers/pci/pci-driver.c | 6 +- drivers/pci/pci.c | 27 ++-- drivers/pci/probe.c | 176 ++++++++++++++++++++++++- drivers/pci/quirks.c | 256 +++++++++++++++++++++--------------- drivers/pci/remove.c | 5 +- drivers/pci/setup-bus.c | 28 ++-- drivers/pci/setup-res.c | 38 ++++-- drivers/pci/xen-pcifront.c | 4 +- include/linux/pci.h | 79 ++++++++++- 15 files changed, 508 insertions(+), 219 deletions(-)
Signature · 1 line
1.7.10.4
PATCH v3 1/27 PCI: Add pci_dev_resource_n()
Now pci device resource iteration is done via "for (i=0...)" open code. That make code hard to read esp when only bridge or sriov resources are involved.
We want to replace those open code with for_each_pci_resource(). Also want to add addon_resource handling, and need to make addon resource to be treated as normal PCI resources during iteration.
Add pci_device_resource_n() instead of using dev->resources[n]. Use it with for_each_pci_resource macro and addon_resource support.
-v2: Add EXPORT_SYMBOL to find building error found by Gary Hade
--- drivers/pci/probe.c | 9 +++++++++ include/linux/pci.h | 2 ++ 2 files changed, 11 insertions(+)
drivers/pci/probe.c +9 -0
@@ -105,6 +105,15 @@ static int __init pcibus_class_init(void) }postcore_initcall (pcibus_class_init); +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 ; +} +EXPORT_SYMBOL (pci_dev_resource_n); +static u64 pci_size (u64 base,u64 maxbase,u64 mask) {u64 size = mask & maxbase;/* Find the significant bits */
include/linux/pci.h +2 -0
@@ -338,6 +338,8 @@ struct pci_dev {size_t romlen;/* Length of ROM if it's not from the BAR */ }; +struct resource *pci_dev_resource_n (struct pci_dev *dev,int n); +static inline struct pci_dev *pci_physfn (struct pci_dev *dev) {#ifdef CONFIG_PCI_IOV
Signature · 1 line
1.7.10.4
PATCH v3 2/27 PCI: Add pci_dev_resource_idx() helper
In reply to Yinghai LuUse resource pointer to get index in pci resources array/list.
-v2: export symbol for acpiphp compiling error, found by
Steven Newbury <[email protected]>
--- drivers/pci/probe.c | 9 +++++++++ include/linux/pci.h | 1 + 2 files changed, 10 insertions(+)
drivers/pci/probe.c +9 -0
@@ -114,6 +114,15 @@ struct resource *pci_dev_resource_n(struct pci_dev *dev, int n) }EXPORT_SYMBOL (pci_dev_resource_n); +int pci_dev_resource_idx (struct pci_dev *dev,struct resource *res) +{ +if (res >= dev->resource && + res <= dev->resource + (PCI_NUM_RESOURCES -1 )) +return res - dev->resource; + +return -1 ; +} +static u64 pci_size (u64 base,u64 maxbase,u64 mask) {u64 size = mask & maxbase;/* Find the significant bits */
include/linux/pci.h +1 -0
@@ -339,6 +339,7 @@ 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);static inline struct pci_dev *pci_physfn (struct pci_dev *dev) {
Signature · 1 line
1.7.10.4
Bjorn Helgaas
maintainerRe: [PATCH v3 02/27] PCI: Add pci_dev_resource_idx() helper
Quoted 17 lines · Yinghai Lu
Use resource pointer to get index in pci resources array/list.
-v2: export symbol for acpiphp compiling error, found by
Steven Newbury <[email protected]>Signed-off-by Yinghai Lu <[email protected]>--- drivers/pci/probe.c | 9 +++++++++ include/linux/pci.h | 1 + 2 files changed, 10 insertions(+)drivers/pci/probe.c +0
-0@@ -114,6 +114,15 @@ struct resource *pci_dev_resource_n(struct pci_dev *dev, int n) }
drivers/pci/probe.c struct resource *pci_dev_resource_n(struct pci_dev *dev, int n) · line 124
Quoted 10 lines
drivers/pci/probe.c +8
-0EXPORT_SYMBOL (pci_dev_resource_n); +int pci_dev_resource_idx (struct pci_dev *dev,struct resource *res) +{ +if (res >= dev->resource && + res <= dev->resource + (PCI_NUM_RESOURCES -1 )) +return res - dev->resource; + +return -1 ; +}
I'm dubious about the whole idea of a resource *index*. I'd like to get away from that concept completely.
Some uses of this are just for printing and are obviously unnecessary (get_res_add_size()).
Other places we pass around the index, e.g., reassign_resources_sorted() passes it to pci_assign_resource() and pci_reassign_resource(), and eventually we just pass the index to pci_dev_resource_n() to get back what we started with. I'd rather just pass around a pointer instead of this half pointer/half index strategy.
We might need a "struct pci_resource" or something that contains the type (BAR, IOV, bridge window, etc), a BAR number, etc. There's already a "struct pci_resource" in cpqphp, but that's isolated to cpqphp and could be easily changed if you wanted that name.
It's going to be very confusing to have a "struct pci_dev_resource" and a "pci_dev_resource_n()" that have nothing to do with each other.
Quoted 18 lines
drivers/pci/probe.c +0
-0static u64 pci_size (u64 base,u64 maxbase,u64 mask) {u64 size = mask & maxbase;/* Find the significant bits */ include/linux/pci.h +1
-1@@ -339,6 +339,7 @@ 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);static inline struct pci_dev *pci_physfn (struct pci_dev *dev) { --1.7.10.4
PATCH v3 3/27 PCI: pci resource iterator
In reply to Yinghai LuFrom: Ram Pai <[email protected]>
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 <[email protected]> - Yinghai Lu
--- drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 24 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+)
drivers/pci/probe.c +48 -0
@@ -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 */
include/linux/pci.h +24 -0
@@ -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
Signature · 1 line
1.7.10.4
Bjorn Helgaas
maintainerRe: [PATCH v3 03/27] PCI: pci resource iterator
Quoted 116 lines · Yinghai Lu
From: Ram Pai <[email protected]>
Currently
pci_devstructure 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_devstructure needs to be re-organized to avoid unnecessary bloating. However too much code outside the pci-bus driver, assumes the internal details of thepci_devstructure, 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_devstructure and correspondingly update the methods.(quoted diff) +0
-4-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 <[email protected]> - Yinghai LuSigned-off-by Ram Pai <[email protected]>Signed-off-by Yinghai Lu <[email protected]>--- drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 24 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+)drivers/pci/probe.c +48
-0@@ -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 */ include/linux/pci.h +17
-0@@ -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); +
include/linux/pci.h struct pci_dev { · line 370
Quoted 10 lines
include/linux/pci.h +7
-0+#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
Re: [PATCH v3 03/27] PCI: pci resource iterator
Quoted 126 lines · Bjorn Helgaas
Quoted 125 lines · Yinghai Lu
From: Ram Pai <[email protected]>
Currently
pci_devstructure 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_devstructure needs to be re-organized to avoid unnecessary bloating. However too much code outside the pci-bus driver, assumes the internal details of thepci_devstructure, 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_devstructure and correspondingly update the methods.(quoted diff) +0
-4-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 <[email protected]> - Yinghai LuSigned-off-by Ram Pai <[email protected]>Signed-off-by Yinghai Lu <[email protected]>--- drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 24 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+)drivers/pci/probe.c +48
-0@@ -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 */ include/linux/pci.h +24
-0@@ -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) {
Quoted 8 lines
include/linux/pci.h struct pci_dev { · line 370
Quoted 1 line
include/linux/pci.h +0
-0#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 infor_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
Bjorn Helgaas
maintainerRe: [PATCH v3 03/27] PCI: pci resource iterator
Quoted 136 lines · Ram Pai
Quoted 126 lines · Bjorn Helgaas
Quoted 125 lines · Yinghai Lu
From: Ram Pai <[email protected]>
Currently
pci_devstructure 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_devstructure needs to be re-organized to avoid unnecessary bloating. However too much code outside the pci-bus driver, assumes the internal details of thepci_devstructure, 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_devstructure and correspondingly update the methods.(quoted diff) +0
-4-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 <[email protected]> - Yinghai LuSigned-off-by Ram Pai <[email protected]>Signed-off-by Yinghai Lu <[email protected]>--- drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 24 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+)drivers/pci/probe.c +48
-0@@ -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 */ include/linux/pci.h +24
-0@@ -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) {Quoted 8 lines
include/linux/pci.h struct pci_dev { · line 370
Quoted 1 line
include/linux/pci.h +0
-0#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 infor_each_pci_resources()itself cleaner to implement.
Quoted 4 lines
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.
Quoted 3 lines
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
Re: [PATCH v3 03/27] PCI: pci resource iterator
Quoted 139 lines · Bjorn Helgaas
Quoted 138 lines · Ram Pai
Quoted 126 lines · Bjorn Helgaas
Quoted 125 lines · Yinghai Lu
From: Ram Pai <[email protected]>
Currently
pci_devstructure 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_devstructure needs to be re-organized to avoid unnecessary bloating. However too much code outside the pci-bus driver, assumes the internal details of thepci_devstructure, 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_devstructure and correspondingly update the methods.(quoted diff) +0
-4-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 <[email protected]> - Yinghai LuSigned-off-by Ram Pai <[email protected]>Signed-off-by Yinghai Lu <[email protected]>--- drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 24 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+)drivers/pci/probe.c +48
-0@@ -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 */ include/linux/pci.h +24
-0@@ -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) {Quoted 8 lines
include/linux/pci.h struct pci_dev { · line 370
Quoted 1 line
include/linux/pci.h +0
-0#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 infor_each_pci_resources()itself cleaner to implement.I agree. There are two cleanups needed.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 4 lines
The number of places that actually look at *non-BAR*
pci_devresources, 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.
Quoted 8 lines
Quoted 4 lines
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
Signature · 1 line
Ram Pai
Bjorn Helgaas
maintainerRe: [PATCH v3 03/27] PCI: pci resource iterator
Quoted 12 lines · Ram Pai
Quoted 11 lines · Bjorn Helgaas
Quoted 10 lines · Ram Pai
Quoted 9 lines · Bjorn Helgaas
Quoted 8 lines · Yinghai Lu
From: Ram Pai <[email protected]>
Currently
pci_devstructure 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.
...
Quoted 16 lines
Quoted 9 lines
Quoted 4 lines
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.
Quoted 7 lines
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.
Quoted 1 line
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.
Quoted 1 line
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.
Quoted 1 line
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.
Quoted 4 lines
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.
Quoted 1 line
Everything else needs to be hidden.
Bjorn
Re: [PATCH v3 03/27] PCI: pci resource iterator
Quoted 34 lines · Bjorn Helgaas
Quoted 12 lines · Ram Pai
Quoted 11 lines · Bjorn Helgaas
Quoted 10 lines · Ram Pai
Quoted 9 lines · Bjorn Helgaas
Quoted 8 lines · Yinghai Lu
From: Ram Pai <[email protected]>
Currently
pci_devstructure 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....
Quoted 20 lines
Quoted 9 lines
Quoted 4 lines
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.
Quoted 6 lines
Quoted 3 lines
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]
Quoted 7 lines
Quoted 2 lines
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.
Quoted 5 lines
Quoted 1 line
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]
Quoted 12 lines
Quoted 2 lines
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.
Quoted 4 lines
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.
Quoted 5 lines
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
Re: [PATCH v3 03/27] PCI: pci resource iterator
In reply to Bjorn HelgaasQuoted 126 lines · Bjorn Helgaas
Quoted 125 lines · Yinghai Lu
From: Ram Pai <[email protected]>
Currently
pci_devstructure 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_devstructure needs to be re-organized to avoid unnecessary bloating. However too much code outside the pci-bus driver, assumes the internal details of thepci_devstructure, 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_devstructure and correspondingly update the methods.(quoted diff) +0
-4-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 <[email protected]> - Yinghai LuSigned-off-by Ram Pai <[email protected]>Signed-off-by Yinghai Lu <[email protected]>--- drivers/pci/probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 24 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+)drivers/pci/probe.c +48
-0@@ -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 */ include/linux/pci.h +24
-0@@ -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) {
Quoted 8 lines
include/linux/pci.h struct pci_dev { · line 370
Quoted 1 line
include/linux/pci.h +0
-0#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 infor_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.
Quoted 4 lines
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
PATCH v3 4/27 PCI: Add is_pci_*_resource_idx() helpers
In reply to Yinghai LuAccording to resource pointer find out if the resource is some type resource like bridge, sriov, or std.
--- include/linux/pci.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
include/linux/pci.h +23 -0
@@ -92,6 +92,29 @@ enum { DEVICE_COUNT_RESOURCE = PCI_NUM_RESOURCES, }; +static inline bool is_pci_std_resource_idx (int i) +{ +return i >=PCI_STD_RESOURCES && i <=PCI_STD_RESOURCE_END ; +} + +static inline bool is_pci_rom_resource_idx (int i) +{ +return i ==PCI_ROM_RESOURCE ; +} + +static inline bool is_pci_iov_resource_idx (int i) +{ +#ifdef CONFIG_PCI_IOV +return i >=PCI_IOV_RESOURCES && i <=PCI_IOV_RESOURCE_END ; +#endif +return false; +} + +static inline bool is_pci_bridge_resource_idx (int i) +{ +return i >=PCI_BRIDGE_RESOURCES && i <=PCI_BRIDGE_RESOURCE_END ; +} +typedef int __bitwise pci_power_t ;#define PCI_D0 ((pci_power_t __force) 0)
Signature · 1 line
1.7.10.4
Bjorn Helgaas
maintainerRe: [PATCH v3 04/27] PCI: Add is_pci_*_resource_idx() helpers
Quoted 32 lines · Yinghai Lu
According to resource pointer find out if the resource is some type resource like bridge, sriov, or std.
Signed-off-by Yinghai Lu <[email protected]>--- include/linux/pci.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)include/linux/pci.h +16
-0@@ -92,6 +92,29 @@ enum { DEVICE_COUNT_RESOURCE = PCI_NUM_RESOURCES, }; +static inline bool is_pci_std_resource_idx (int i) +{ +return i >=PCI_STD_RESOURCES && i <=PCI_STD_RESOURCE_END ; +} + +static inline bool is_pci_rom_resource_idx (int i) +{ +return i ==PCI_ROM_RESOURCE ; +} + +static inline bool is_pci_iov_resource_idx (int i) +{ +#ifdef CONFIG_PCI_IOV +return i >=PCI_IOV_RESOURCES && i <=PCI_IOV_RESOURCE_END ; +#endif +return false;
include/linux/pci.h enum { · line 120
Quoted 10 lines
include/linux/pci.h +7
-0+} + +static inline bool is_pci_bridge_resource_idx (int i) +{ +return i >=PCI_BRIDGE_RESOURCES && i <=PCI_BRIDGE_RESOURCE_END ; +} +typedef int __bitwise pci_power_t ;#define PCI_D0 ((pci_power_t __force) 0)
1) I don't like adding more "_idx()" interfaces.
2) The only one of these that's even used is "is_pci_bridge_resource_idx()"
3) I think adding a wrapper struct with a "type" or "flags" field would make this trivial, e.g., "pres->flags & PCI_RESOURCE_WINDOW" or something.
PATCH v3 5/27 PCI: Update pci_resource_start etc to use pci_dev_resource_n()
In reply to Yinghai LuNow pci_resource_start is using dev->resource[n] directly. Replace it with pci_dev_resource[n] to prepare for addon resource support.
--- include/linux/pci.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
include/linux/pci.h +3 -3
@@ -1488,9 +1488,9 @@ static inline struct pci_dev *pci_dev_get(struct pci_dev *dev)/* these helpers provide future and backwards compatibility * for accessing popular PCI BAR info */ -#define pci_resource_start (dev, bar) ((dev)->resource[(bar)].start) -#define pci_resource_end (dev, bar) ((dev)->resource[(bar)].end) -#define pci_resource_flags (dev, bar) ((dev)->resource[(bar)].flags) +#define pci_resource_start (dev, bar) (pci_dev_resource_n(dev, (bar))->start) +#define pci_resource_end (dev, bar) (pci_dev_resource_n(dev, (bar))->end) +#define pci_resource_flags (dev, bar) (pci_dev_resource_n(dev, (bar))->flags)#define pci_resource_len (dev,bar) \ ((pci_resource_start((dev), (bar)) == 0 && \ pci_resource_end((dev), (bar)) == \
Signature · 1 line
1.7.10.4
PATCH v3 6/27 PCI, x86: Use for_each_pci_resource() with pci_allocate_bridge_resources
In reply to Yinghai Lu--- arch/x86/pci/i386.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/x86/pci/i386.c +1 -2
@@ -206,8 +206,7 @@ static void pcibios_allocate_bridge_resources(struct pci_dev *dev)int idx;struct resource *r; -for (idx =PCI_BRIDGE_RESOURCES ; idx <PCI_NUM_RESOURCES ; idx++) { - r = &dev->resource[idx]; +for_each_pci_resource (dev, r, idx,PCI_BRIDGE_RES ) {if (!r->flags)continue ;if (!r->start ||pci_claim_resource (dev, idx) <0 ) {
Signature · 1 line
1.7.10.4
PATCH v3 7/27 PCI, x86: Use for_each_pci_resource() with pci_allocate_dev_resources
In reply to Yinghai LuWe can replace two layer loop with for_each_pci_resource(), and make the code more readable.
--- arch/x86/pci/i386.c | 55 +++++++++++++++++++-------------------------------- 1 file changed, 20 insertions(+), 35 deletions(-)
arch/x86/pci/i386.c +20 -35
@@ -233,49 +233,34 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)pcibios_allocate_bus_resources (child); } -struct pci_check_idx_range { -int start; -int end; -}; -static void pcibios_allocate_dev_resources (struct pci_dev *dev,int pass) { -int idx, disabled, i; +int idx, disabled;u16 command;struct resource *r; -struct pci_check_idx_range idx_range[] = { - {PCI_STD_RESOURCES ,PCI_STD_RESOURCE_END }, -#ifdef CONFIG_PCI_IOV - {PCI_IOV_RESOURCES ,PCI_IOV_RESOURCE_END }, -#endif - }; -pci_read_config_word (dev,PCI_COMMAND , &command); -for (i =0 ; i <ARRAY_SIZE (idx_range); i++) -for (idx = idx_range[i].start; idx <= idx_range[i].end; idx++) { - r = &dev->resource[idx]; -if (r->parent)/* Already allocated */ -continue ; -if (!r->start)/* Address not assigned at all */ -continue ; -if (r->flags &IORESOURCE_IO ) - disabled = !(command &PCI_COMMAND_IO ); -else - disabled = !(command &PCI_COMMAND_MEMORY ); -if (pass == disabled) { -dev_dbg (&dev->dev, -"BAR %d: reserving %pr (d=%d, p=%d)\n" , - idx, r, disabled, pass); -if (pci_claim_resource (dev, idx) <0 ) { -/* We'll assign a new address later */ -pcibios_save_fw_addr (dev, - idx, r->start); - r->end -= r->start; - r->start =0 ; - } +for_each_pci_resource (dev, r, idx,PCI_NOROM_RES & ~PCI_BRIDGE_RES ) { +if (r->parent)/* Already allocated */ +continue ; +if (!r->start)/* Address not assigned at all */ +continue ; +if (r->flags &IORESOURCE_IO ) + disabled = !(command &PCI_COMMAND_IO ); +else + disabled = !(command &PCI_COMMAND_MEMORY ); +if (pass == disabled) { +dev_dbg (&dev->dev, +"BAR %d: reserving %pr (d=%d, p=%d)\n" , + idx, r, disabled, pass); +if (pci_claim_resource (dev, idx) <0 ) { +/* We'll assign a new address later */ +pcibios_save_fw_addr (dev, idx, r->start); + r->end -= r->start; + r->start =0 ; } } + }if (!pass) { r = &dev->resource[PCI_ROM_RESOURCE ];if (r->flags &IORESOURCE_ROM_ENABLE ) {
Signature · 1 line
1.7.10.4
PATCH v3 8/27 PCI: Use for_each_pci_resource() with IOV releated functions
In reply to Yinghai Lu--- drivers/pci/iov.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
drivers/pci/iov.c +15 -16
@@ -93,17 +93,20 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)pci_setup_device (virtfn); virtfn->dev.parent = dev->dev.parent; -for (i =0 ; i <PCI_SRIOV_NUM_BARS ; i++) { - res = dev->resource +PCI_IOV_RESOURCES + i; +for_each_pci_resource (dev, res, i,PCI_IOV_RES ) { +struct resource *virtfn_res; +if (!res->parent)continue ; - virtfn->resource[i].name =pci_name (virtfn); - virtfn->resource[i].flags = res->flags; + + virtfn_res =pci_dev_resource_n (virtfn, i -PCI_IOV_RESOURCES ); + virtfn_res->name =pci_name (virtfn); + virtfn_res->flags = res->flags; size =resource_size (res);do_div (size, iov->total_VFs); - virtfn->resource[i].start = res->start + size * id; - virtfn->resource[i].end = virtfn->resource[i].start + size -1 ; - rc =request_resource (res, &virtfn->resource[i]); + virtfn_res->start = res->start + size * id; + virtfn_res->end = virtfn_res->start + size -1 ; + rc =request_resource (res, virtfn_res);BUG_ON (rc); }
@@ -305,9 +308,8 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)return -EIO ; nres =0 ; -for (i =0 ; i <PCI_SRIOV_NUM_BARS ; i++) { - bars |= (1 << (i +PCI_IOV_RESOURCES )); - res = dev->resource +PCI_IOV_RESOURCES + i; +for_each_pci_resource (dev, res, i,PCI_IOV_RES ) { + bars |=1 << i;if (res->parent) nres++; }
@@ -465,10 +467,9 @@ found:pci_write_config_dword (dev, pos +PCI_SRIOV_SYS_PGSIZE , pgsz); nres =0 ; -for (i =0 ; i <PCI_SRIOV_NUM_BARS ; i++) { - res = dev->resource +PCI_IOV_RESOURCES + i; +for_each_pci_resource (dev, res, i,PCI_IOV_RES ) { i +=__pci_read_base (dev, pci_bar_unknown, res, - pos +PCI_SRIOV_BAR + i *4 ); + pos +PCI_SRIOV_BAR + (i -PCI_IOV_RESOURCES ) *4 );if (!res->flags)continue ;if (resource_size (res) & (PAGE_SIZE -1 )) {
@@ -511,10 +512,8 @@ found:return 0 ; failed: -for (i =0 ; i <PCI_SRIOV_NUM_BARS ; i++) { - res = dev->resource +PCI_IOV_RESOURCES + i; +for_each_pci_resource (dev, res, i,PCI_IOV_RES ) res->flags =0 ; - }return rc; }
Signature · 1 line
1.7.10.4
PATCH v3 9/27 PCI, acpiphp: Use for_each_pci_resource() helper
In reply to Yinghai Lu--- drivers/pci/hotplug/acpiphp_glue.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/hotplug/acpiphp_glue.c +2 -2
@@ -1085,10 +1085,10 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)struct pci_dev *dev;int i;unsigned long type_mask =IORESOURCE_IO |IORESOURCE_MEM ; +struct resource *res;list_for_each_entry (dev, &bus->devices, bus_list) { -for (i=0 ; i<PCI_BRIDGE_RESOURCES ; i++) { -struct resource *res = &dev->resource[i]; +for_each_pci_resource (dev, res, i,PCI_NOBRIDGE_RES ) {if ((res->flags & type_mask) && !res->start && res->end) {/* Could not assign a required resources
Signature · 1 line
1.7.10.4
PATCH v3 10/27 PCI, pciehp: Use for_each_pci_resource() helper
In reply to Yinghai Lu--- drivers/pci/hotplug/pciehp_hpc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/pci/hotplug/pciehp_hpc.c +3 -2
@@ -807,6 +807,7 @@ static inline void dbg_ctrl(struct controller *ctrl)int i;u16 reg16;struct pci_dev *pdev = ctrl->pcie->port; +struct resource *res;if (!pciehp_debug)return ;
@@ -822,11 +823,11 @@ static inline void dbg_ctrl(struct controller *ctrl) pdev->subsystem_vendor);ctrl_info (ctrl," PCIe Cap offset : 0x%02x\n" ,pci_pcie_cap (pdev)); -for (i =0 ; i <DEVICE_COUNT_RESOURCE ; i++) { +for_each_pci_resource (pdev, res, i,PCI_ALL_RES ) {if (!pci_resource_len (pdev, i))continue ;ctrl_info (ctrl," PCI resource [%d] : %pR\n" , - i, &pdev->resource[i]); + i, res); }ctrl_info (ctrl,"Slot Capabilities : 0x%08x\n" , ctrl->slot_cap);ctrl_info (ctrl," Physical Slot Number : %d\n" ,PSN (ctrl));
Signature · 1 line
1.7.10.4
PATCH v3 11/27 PCI: Use for_each_pci_resource() in pci_enable_dev
In reply to Yinghai Lu--- drivers/pci/pci.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
drivers/pci/pci.c +8 -7
@@ -440,8 +440,9 @@ static void pci_restore_bars(struct pci_dev *dev) {int i; +struct resource *res; -for (i =0 ; i <PCI_BRIDGE_RESOURCES ; i++) +for_each_pci_resource (dev, res, i,PCI_NOBRIDGE_RES )pci_update_resource (dev, i); }
@@ -1153,6 +1154,7 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags) {int err;int i, bars =0 ; +struct resource *res;/* * Power state could be unknown at this point, either due to a fresh
@@ -1170,12 +1172,11 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)return 0 ;/* already enabled */ /* only skip sriov related */ -for (i =0 ; i <=PCI_ROM_RESOURCE ; i++) -if (dev->resource[i].flags & flags) - bars |= (1 << i); -for (i =PCI_BRIDGE_RESOURCES ; i <DEVICE_COUNT_RESOURCE ; i++) -if (dev->resource[i].flags & flags) +for_each_pci_resource (dev, res, i,PCI_NOIOV_RES ) { +/* TODO: check i with bits of bars */ +if (res->flags & flags) bars |= (1 << i); + } err =do_pci_enable_device (dev, bars);if (err <0 )
@@ -2557,7 +2558,7 @@ static int __pci_request_region(struct pci_dev *pdev, int bar, const char *res_n err_out:dev_warn (&pdev->dev,"BAR %d: can't reserve %pR\n" , bar, - &pdev->resource[bar]); +pci_dev_resource_n (pdev, bar));return -EBUSY ; }
Signature · 1 line
1.7.10.4
PATCH v3 12/27 PCI: Use for_each_pci_resource() in pci_reassigndev
In reply to Yinghai Lu--- drivers/pci/pci.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/pci/pci.c +2 -3
@@ -3782,7 +3782,7 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev) command &= ~PCI_COMMAND_MEMORY ;pci_write_config_word (dev,PCI_COMMAND , command); -for (i =0 ; i <PCI_BRIDGE_RESOURCES ; i++) { +for_each_pci_resource (dev, r, i,PCI_NOBRIDGE_RES ) { r = &dev->resource[i];if (!(r->flags &IORESOURCE_MEM ))continue ;
@@ -3802,8 +3802,7 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)*/ if (dev->hdr_type ==PCI_HEADER_TYPE_BRIDGE && (dev->class >>8 ) ==PCI_CLASS_BRIDGE_PCI ) { -for (i =PCI_BRIDGE_RESOURCES ; i <PCI_NUM_RESOURCES ; i++) { - r = &dev->resource[i]; +for_each_pci_resource (dev, r, i,PCI_BRIDGE_RES ) {if (!(r->flags &IORESOURCE_MEM ))continue ; r->end =resource_size (r) -1 ;
Signature · 1 line
1.7.10.4
PATCH v3 13/27 PCI: Use for_each_pci_resource() with pci bar reassign funcs
In reply to Yinghai LuReplace those open code, and make code more readable.
--- drivers/pci/setup-bus.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-)
drivers/pci/setup-bus.c +12 -16
@@ -106,7 +106,7 @@ static resource_size_t get_res_add_size(struct list_head *head,list_for_each_entry (dev_res, head, list) {if (dev_res->res == res) { -int idx = res - &dev_res->dev->resource[0 ]; +int idx =pci_dev_resource_idx (dev_res->dev, res); dev_printk(KERN_DEBUG, &dev_res->dev->dev, "res[%d]=%pR get_res_add_size add_size %llx\n",
@@ -124,15 +124,13 @@ static resource_size_t get_res_add_size(struct list_head *head,static void pdev_sort_resources (struct pci_dev *dev,struct list_head *head) {int i; +struct resource *r; -for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { -struct resource *r; +for_each_pci_resource (dev, r, i,PCI_ALL_RES ) {struct pci_dev_resource *dev_res, *tmp;resource_size_t r_align;struct list_head *n; - r = &dev->resource[i]; -if (r->flags &IORESOURCE_PCI_FIXED )continue ;
@@ -237,7 +235,7 @@ static void reassign_resources_sorted(struct list_head *realloc_head,if (!found_match)/* just skip */ continue ; - idx = res - &add_res->dev->resource[0 ]; + idx =pci_dev_resource_idx (add_res->dev, res); add_size = add_res->add_size;if (!resource_size (res)) { res->start = add_res->start;
@@ -280,7 +278,7 @@ static void assign_requested_resources_sorted(struct list_head *head,list_for_each_entry (dev_res, head, list) { res = dev_res->res; - idx = res - &dev_res->dev->resource[0 ]; + idx =pci_dev_resource_idx (dev_res->dev, res);if (resource_size (res) &&pci_assign_resource (dev_res->dev, idx)) {if (fail_head) {
@@ -757,9 +755,9 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, io_align = min_align =window_alignment (bus,IORESOURCE_IO );list_for_each_entry (dev, &bus->devices, bus_list) {int i; +struct resource *r; -for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { -struct resource *r = &dev->resource[i]; +for_each_pci_resource (dev, r, i,PCI_ALL_RES ) {unsigned long r_size;if (r->parent || !(r->flags &IORESOURCE_IO ))
@@ -870,9 +868,9 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,list_for_each_entry (dev, &bus->devices, bus_list) {int i; +struct resource *r; -for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { -struct resource *r = &dev->resource[i]; +for_each_pci_resource (dev, r, i,PCI_ALL_RES ) {resource_size_t r_size;if (r->parent || (r->flags & mask) != type)
@@ -1196,9 +1194,7 @@ static void pci_bridge_release_resources(struct pci_bus *bus,IORESOURCE_PREFETCH ; dev = bus->self; -for (idx =PCI_BRIDGE_RESOURCES ; idx <=PCI_BRIDGE_RESOURCE_END ; - idx++) { - r = &dev->resource[idx]; +for_each_pci_resource (dev, r, idx,PCI_BRIDGE_RES ) {if ((r->flags & type_mask) != type)continue ;if (!r->parent)
@@ -1369,9 +1365,9 @@ static void __init pci_realloc_detect(void)for_each_pci_dev (dev) {int i; +struct resource *r; -for (i =PCI_IOV_RESOURCES ; i <=PCI_IOV_RESOURCE_END ; i++) { -struct resource *r = &dev->resource[i]; +for_each_pci_resource (dev, r, i,PCI_IOV_RES ) {/* Not assigned, or rejected by kernel ? */ if (r->flags && !r->start) {
Signature · 1 line
1.7.10.4
PATCH v3 14/27 PCI: Use for_each_pci_resource() in pci_assign_resource
In reply to Yinghai LuAlso replace dev->resource[n] with pci_dev_resource_n().
--- drivers/pci/setup-res.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/pci/setup-res.c +6 -8
@@ -110,7 +110,7 @@ void pci_update_resource(struct pci_dev *dev, int resno)int pci_claim_resource (struct pci_dev *dev,int resource) { -struct resource *res = &dev->resource[resource]; +struct resource *res =pci_dev_resource_n (dev, resource);struct resource *root, *conflict; root =pci_find_parent_resource (dev, res);
@@ -200,7 +200,7 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,static int __pci_assign_resource (struct pci_bus *bus,struct pci_dev *dev,int resno,resource_size_t size,resource_size_t align) { -struct resource *res = dev->resource + resno; +struct resource *res =pci_dev_resource_n (dev, resno);resource_size_t min;int ret;
@@ -227,7 +227,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,static int _pci_assign_resource (struct pci_dev *dev,int resno,resource_size_t size,resource_size_t min_align) { -struct resource *res = dev->resource + resno; +struct resource *res =pci_dev_resource_n (dev, resno);struct pci_bus *bus;int ret;char *type;
@@ -259,7 +259,7 @@ static int _pci_assign_resource(struct pci_dev *dev, int resno,int pci_assign_resource (struct pci_dev *dev,int resno) { -struct resource *res = dev->resource + resno; +struct resource *res =pci_dev_resource_n (dev, resno);resource_size_t align, size;struct pci_bus *bus;int ret;
@@ -295,7 +295,7 @@ int pci_assign_resource(struct pci_dev *dev, int resno)int pci_reassign_resource (struct pci_dev *dev,int resno,resource_size_t addsize,resource_size_t min_align) { -struct resource *res = dev->resource + resno; +struct resource *res =pci_dev_resource_n (dev, resno);resource_size_t new_size;int ret;
@@ -326,12 +326,10 @@ int pci_enable_resources(struct pci_dev *dev, int mask)pci_read_config_word (dev,PCI_COMMAND , &cmd); old_cmd = cmd; -for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { +for_each_pci_resource (dev, r, i,PCI_ALL_RES ) {if (!(mask & (1 << i)))continue ; - r = &dev->resource[i]; -if (!(r->flags & (IORESOURCE_IO |IORESOURCE_MEM )))continue ; if ((i == PCI_ROM_RESOURCE) &&
Signature · 1 line
1.7.10.4
PATCH v3 15/27 PCI, x86: Use for_each_pci_resource() with noassign_bars
In reply to Yinghai LuReplace those open code, and make code more readable.
--- arch/x86/pci/common.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/x86/pci/common.c +1 -2
@@ -136,8 +136,7 @@ static void pcibios_fixup_device_resources(struct pci_dev *dev)* resource so the kernel doesn't attmept to assign * it later on in pci_assign_unassigned_resources */ -for (bar =0 ; bar <=PCI_STD_RESOURCE_END ; bar++) { - bar_r = &dev->resource[bar]; +for_each_pci_resource (dev, bar_r, bar,PCI_NOIOV_RES & ~PCI_BRIDGE_RES & ~PCI_ROM_RES ) {if (bar_r->start ==0 && bar_r->end !=0 ) { bar_r->flags =0 ; bar_r->end =0 ;
Signature · 1 line
1.7.10.4
PATCH v3 16/27 PCI: Use for_each_pci_resource() in pci_dev_driver()
In reply to Yinghai LuReplace those open code, and make code more readable.
--- drivers/pci/pci-driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/pci/pci-driver.c +4 -2
@@ -1170,9 +1170,11 @@ pci_dev_driver(const struct pci_dev *dev)if (dev->driver)return dev->driver;else { +struct resource *res;int i; -for (i=0 ; i<=PCI_ROM_RESOURCE ; i++) -if (dev->resource[i].flags &IORESOURCE_BUSY ) + +for_each_pci_resource ((struct pci_dev *)dev, res, i,PCI_NOIOV_RES & ~PCI_BRIDGE_RES ) +if (res->flags &IORESOURCE_BUSY )return &pci_compat_driver; }return NULL ;
Signature · 1 line
1.7.10.4
PATCH v3 17/27 PCI: Use for_each_pci_resource() in pci resource release
In reply to Yinghai LuReplace those open code, and make code more readable.
--- drivers/pci/remove.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/pci/remove.c +2 -3
@@ -6,15 +6,14 @@static void pci_free_resources (struct pci_dev *dev) {int i; +struct resource *res;msi_remove_pci_irq_vectors (dev);pci_cleanup_rom (dev); -for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { -struct resource *res = dev->resource + i; +for_each_pci_resource (dev, res, i,PCI_ALL_RES )if (res->parent)release_resource (res); - } }static void pci_stop_dev (struct pci_dev *dev)
Signature · 1 line
1.7.10.4
PATCH v3 18/27 PCI: Use for_each_pci_resource() in pci bases reading
In reply to Yinghai LuReplace those open code, and make code more readable.
--- drivers/pci/probe.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/pci/probe.c +4 -2
@@ -354,9 +354,11 @@ out:static void pci_read_bases (struct pci_dev *dev,unsigned int howmany,int rom) {unsigned int pos, reg; +struct resource *res; -for (pos =0 ; pos < howmany; pos++) { -struct resource *res = &dev->resource[pos]; +for_each_pci_resource (dev, res, pos,PCI_STD_RES ) { +if (pos >= howmany) +break ; reg =PCI_BASE_ADDRESS_0 + (pos <<2 ); pos +=__pci_read_base (dev, pci_bar_unknown, res, reg); }
Signature · 1 line
1.7.10.4
PATCH v3 19/27 PCI, x86: Use for_each_pci_resource() with mrst
In reply to Yinghai Lu--- arch/x86/pci/mrst.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
arch/x86/pci/mrst.c +4 -3
@@ -280,6 +280,7 @@ static void pci_fixed_bar_fixup(struct pci_dev *dev)unsigned long offset;u32 size;int i; +struct resource *res;if (!pci_soc_mode)return ;
@@ -294,10 +295,10 @@ static void pci_fixed_bar_fixup(struct pci_dev *dev) PCI_DEVFN(2, 2) == dev->devfn)return ; -for (i =0 ; i <PCI_ROM_RESOURCE ; i++) { +for_each_pci_resource (dev, res, i,PCI_STD_RES ) {pci_read_config_dword (dev, offset +8 + (i *4 ), &size); - dev->resource[i].end = dev->resource[i].start + size -1 ; - dev->resource[i].flags |=IORESOURCE_PCI_FIXED ; + res->end = res->start + size -1 ; + res->flags |=IORESOURCE_PCI_FIXED ; } }DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_INTEL ,PCI_ANY_ID , pci_fixed_bar_fixup);
Signature · 1 line
1.7.10.4
PATCH v3 20/27 PCI, xen: Use for_each_pci_resource() with xen pci
In reply to Yinghai Lu--- drivers/pci/xen-pcifront.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pci/xen-pcifront.c +1 -3
@@ -395,9 +395,7 @@ static int pcifront_claim_resource(struct pci_dev *dev, void *data)int i;struct resource *r; -for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { - r = &dev->resource[i]; - +for_each_pci_resource (dev, r, i,PCI_ALL_RES ) {if (!r->parent && r->start && r->flags) {dev_info (&pdev->xdev->dev,"claiming resource %s/%d\n" ,pci_name (dev), i);
Signature · 1 line
1.7.10.4
Re: [PATCH v3 20/27] PCI, xen: Use for_each_pci_resource() with xen pci
Quoted 2 lines · Yinghai Lu
Signed-off-by Yinghai Lu <[email protected]>Cc Konrad Rzeszutek Wilk <[email protected]>
Quoted 23 lines
--- drivers/pci/xen-pcifront.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)drivers/pci/xen-pcifront.c +1
-3@@ -395,9 +395,7 @@ static int pcifront_claim_resource(struct pci_dev *dev, void *data)int i;struct resource *r; -for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { - r = &dev->resource[i]; - +for_each_pci_resource (dev, r, i,PCI_ALL_RES ) {if (!r->parent && r->start && r->flags) {dev_info (&pdev->xdev->dev,"claiming resource %s/%d\n" ,pci_name (dev), i);Signature · 1 line
1.7.10.4
PATCH v3 21/27 PCI: Add addon_resource support for pci devices
In reply to Yinghai LuNow there is some non normal pci resources other than standard,rom, sriov, bridges.
Some could be same as standard reg, but using different position. Some could have own way to read/write to them.
Kernel are using different way to hack those resources like abusing pci bridge resource spot on non bridge pci device.
Add addon_resources list in pci_dev for those non-standard resources. With this patch, will treat addon-resource like standard resources with special ops.
--- drivers/pci/probe.c | 38 +++++++++++++++++++++++++++++++++++++- include/linux/pci.h | 20 +++++++++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-)
drivers/pci/probe.c +37 -1
@@ -107,22 +107,53 @@ postcore_initcall(pcibus_class_init);struct resource *pci_dev_resource_n (struct pci_dev *dev,int n) { -if (n >=0 && n <PCI_NUM_RESOURCES ) +struct pci_dev_addon_resource *addon_res; + +if (n <0 ) +return NULL ; + +if (n <PCI_NUM_RESOURCES )return &dev->resource[n]; + n -=PCI_NUM_RESOURCES ; +list_for_each_entry (addon_res, &dev->addon_resources, list) { +if (n-- ==0 ) +return &addon_res->res; + } +return NULL ; }EXPORT_SYMBOL (pci_dev_resource_n);int pci_dev_resource_idx (struct pci_dev *dev,struct resource *res) { +struct pci_dev_addon_resource *addon_res; +int n; +if (res >= dev->resource && res <= dev->resource + (PCI_NUM_RESOURCES -1 ))return res - dev->resource; + n =PCI_NUM_RESOURCES ; +list_for_each_entry (addon_res, &dev->addon_resources, list) { +if (res == &addon_res->res) +return n; + n++; + } +return -1 ; } +static void pci_release_dev_addon_resource (struct pci_dev *dev) +{ +struct pci_dev_addon_resource *addon_res, *tmp; + +list_for_each_entry_safe (addon_res, tmp, &dev->addon_resources, list) { +list_del (&addon_res->list); +kfree (addon_res); + } +} +static void __init_res_idx_mask (unsigned long *mask,int flag) {bitmap_zero (mask,PCI_NUM_RESOURCES );
@@ -167,6 +198,9 @@ int pci_next_resource_idx(int i, int flag)if (i <PCI_NUM_RESOURCES )return i; +if (flag &PCI_ADDON_RES ) +return i; +return -1 ; }EXPORT_SYMBOL (pci_next_resource_idx);
@@ -1199,6 +1233,7 @@ static void pci_release_dev(struct device *dev) pci_dev =to_pci_dev (dev);pci_release_capabilities (pci_dev);pci_release_of_node (pci_dev); +pci_release_dev_addon_resource (pci_dev);kfree (pci_dev); }
@@ -1276,6 +1311,7 @@ struct pci_dev *alloc_pci_dev(void)return NULL ;INIT_LIST_HEAD (&dev->bus_list); +INIT_LIST_HEAD (&dev->addon_resources);return dev; }
include/linux/pci.h +19 -1
@@ -308,6 +308,7 @@ struct pci_dev {*/ unsigned int irq;struct resource resource[DEVICE_COUNT_RESOURCE ];/* I/O and memory regions + expansion ROMs */ +struct list_head addon_resources;/* addon I/O and memory resource */ bool match_driver;/* Skip attaching driver */ /* These fields are used by common fixups */
@@ -361,6 +362,21 @@ struct pci_dev {size_t romlen;/* Length of ROM if it's not from the BAR */ }; +struct resource_ops { +int (*read)(struct pci_dev *dev,struct resource *res,int addr); +int (*write)(struct pci_dev *dev,struct resource *res,int addr); +}; + +struct pci_dev_addon_resource { +struct list_head list; +int reg_addr; +int size; +struct resource res; +struct resource_ops *ops; +}; +#define to_pci_dev_addon_resource (n) \ + container_of(n, struct pci_dev_addon_resource, res) +struct resource *pci_dev_resource_n (struct pci_dev *dev,int n);int pci_dev_resource_idx (struct pci_dev *dev,struct resource *res);
@@ -369,8 +385,10 @@ int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res);#define PCI_IOV_RES (1<<2)#define PCI_BRIDGE_RES (1<<3)#define PCI_RES_BLOCK_NUM 4 +/* addon does not need use bitmap ...*/ +#define PCI_ADDON_RES (1<<4) -#define PCI_ALL_RES (PCI_STD_RES | PCI_ROM_RES | PCI_BRIDGE_RES | PCI_IOV_RES) +#define PCI_ALL_RES (PCI_STD_RES | PCI_ROM_RES | PCI_BRIDGE_RES | PCI_IOV_RES | PCI_ADDON_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)
Signature · 1 line
1.7.10.4
PATCH v3 22/27 PCI: Add helpers to add addon_resource
In reply to Yinghai LuAdd helper to put add_on resources into pci devices resource list.
-v2: change one function to static according to Fengguang.
--- drivers/pci/probe.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 5 ++++ 2 files changed, 73 insertions(+)
drivers/pci/probe.c +68 -0
@@ -144,6 +144,74 @@ int pci_dev_resource_idx(struct pci_dev *dev, struct resource *res)return -1 ; } +static struct pci_dev_addon_resource *pci_alloc_dev_addon_resource ( +struct pci_dev *dev,int addr,char *name) +{ +struct pci_dev_addon_resource *addon_res; + + addon_res =kzalloc (sizeof (*addon_res),GFP_KERNEL ); + +if (!addon_res) +return NULL ; + + addon_res->reg_addr = addr; + +if (name) + addon_res->res.name = name; +else + addon_res->res.name =pci_name (dev); + +list_add_tail (&addon_res->list, &dev->addon_resources); + +return addon_res; +} + +struct pci_dev_addon_resource *pci_add_dev_addon_fixed_resource ( +struct pci_dev *dev,int start,int size,int flags, +int addr,char *name) +{ +struct pci_dev_addon_resource *addon_res; +struct resource *res; + + addon_res =pci_alloc_dev_addon_resource (dev, addr, name); +if (addon_res) +return NULL ; + + res = &addon_res->res; + res->start = start; + res->end = start + size -1 ; + res->flags = flags |IORESOURCE_PCI_FIXED ; + +dev_printk (KERN_DEBUG , &dev->dev, +"addon fixed resource %s %pR added\n" , res->name, res); + +return addon_res; +} + +struct pci_dev_addon_resource *pci_add_dev_addon_resource (struct pci_dev *dev, +int addr,int size,struct resource_ops *ops,char *name) +{ +struct pci_dev_addon_resource *addon_res; +struct resource *res; + + addon_res =pci_alloc_dev_addon_resource (dev, addr, name); +if (!addon_res) +return NULL ; + + res = &addon_res->res; +if (ops) { + addon_res->ops = ops; + addon_res->size = size; + ops->read (dev, res, addr); + }else +__pci_read_base (dev, pci_bar_unknown, res, addr); + +dev_printk (KERN_DEBUG , &dev->dev, +"addon resource %s %pR added\n" , res->name, res); + +return addon_res; +} +static void pci_release_dev_addon_resource (struct pci_dev *dev) {struct pci_dev_addon_resource *addon_res, *tmp;
include/linux/pci.h +5 -0
@@ -379,6 +379,11 @@ struct pci_dev_addon_resource {struct resource *pci_dev_resource_n (struct pci_dev *dev,int n);int pci_dev_resource_idx (struct pci_dev *dev,struct resource *res); +struct pci_dev_addon_resource *pci_add_dev_addon_fixed_resource ( +struct pci_dev *dev,int start,int size,int flags, +int addr,char *name); +struct pci_dev_addon_resource *pci_add_dev_addon_resource (struct pci_dev *dev, +int addr,int size,struct resource_ops *ops,char *name);#define PCI_STD_RES (1<<0)#define PCI_ROM_RES (1<<1)
Signature · 1 line
1.7.10.4
PATCH v3 23/27 PCI: Update pci_resource_bar() to support addon_resource
In reply to Yinghai LuNeed to loop addon resource list to retrieve reg_addr in it.
--- drivers/pci/pci.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/pci/pci.c +7 -0
@@ -3613,6 +3613,13 @@ int pci_resource_bar(struct pci_dev *dev, int resno, enum pci_bar_type *type) reg =pci_iov_resource_bar (dev, resno, type);if (reg)return reg; + }else if (resno >=PCI_NUM_RESOURCES ) { +struct resource *res =pci_dev_resource_n (dev, resno); + +if (res) { + *type = pci_bar_unknown; +return to_pci_dev_addon_resource (res)->reg_addr; + } }dev_err (&dev->dev,"BAR %d: invalid resource\n" , resno);
Signature · 1 line
1.7.10.4
PATCH v3 24/27 PCI: Assign/update resource to addon_res
In reply to Yinghai LuExtend pci_update_resource to check resno and call addon resources ops to update bar for it.
--- drivers/pci/setup-res.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
drivers/pci/setup-res.c +18 -6
@@ -35,7 +35,7 @@ void pci_update_resource(struct pci_dev *dev, int resno)u32 new, check, mask;int reg;enum pci_bar_type type; -struct resource *res = dev->resource + resno; +struct resource *res =pci_dev_resource_n (dev, resno);/* * Ignore resources for unimplemented BARs and unused resource slots
@@ -52,6 +52,21 @@ void pci_update_resource(struct pci_dev *dev, int resno)if (res->flags &IORESOURCE_PCI_FIXED )return ; +if (resno >=PCI_NUM_RESOURCES ) { +struct pci_dev_addon_resource *addon_res; + + addon_res =to_pci_dev_addon_resource (res); + reg = addon_res->reg_addr; +if (addon_res->ops) { + addon_res->ops->write (dev, res, reg); +return ; + } + }else + reg =pci_resource_bar (dev, resno, &type); + +if (!reg) +return ; +pcibios_resource_to_bus (dev, ®ion, res); new = region.start | (res->flags &PCI_REGION_FLAG_MASK );
@@ -60,9 +75,6 @@ void pci_update_resource(struct pci_dev *dev, int resno)else mask = (u32 )PCI_BASE_ADDRESS_MEM_MASK ; - reg =pci_resource_bar (dev, resno, &type); -if (!reg) -return ;if (type != pci_bar_unknown) {if (!(res->flags &IORESOURCE_ROM_ENABLE ))return ;
@@ -286,7 +298,7 @@ int pci_assign_resource(struct pci_dev *dev, int resno)if (!ret) { res->flags &= ~IORESOURCE_STARTALIGN ;dev_info (&dev->dev,"BAR %d: assigned %pR\n" , resno, res); -if (resno <PCI_BRIDGE_RESOURCES ) +if (!is_pci_bridge_resource_idx (resno))pci_update_resource (dev, resno); }return ret;
@@ -311,7 +323,7 @@ int pci_reassign_resource(struct pci_dev *dev, int resno, resource_size_t addsizif (!ret) { res->flags &= ~IORESOURCE_STARTALIGN ;dev_info (&dev->dev,"BAR %d: reassigned %pR\n" , resno, res); -if (resno <PCI_BRIDGE_RESOURCES ) +if (!is_pci_bridge_resource_idx (resno))pci_update_resource (dev, resno); }return ret;
Signature · 1 line
1.7.10.4
PATCH v3 25/27 PCI: Make piix4 quirk to use addon_res
In reply to Yinghai LuAfter they are put in add-on resources, they will be safely claimed later.
--- drivers/pci/quirks.c | 93 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 77 insertions(+), 16 deletions(-)
drivers/pci/quirks.c +77 -16
@@ -383,14 +383,14 @@ static void quirk_ali7101_acpi(struct pci_dev *dev) }DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M7101 , quirk_ali7101_acpi); -static void piix4_io_quirk (struct pci_dev *dev,const char *name,unsigned int port,unsigned int enable) +static int piix4_read_io (struct pci_dev *dev,struct resource *res,int port) {u32 devres;u32 mask, size, base; +struct pci_bus_region bus_region;pci_read_config_dword (dev, port, &devres); -if ((devres & enable) != enable) -return ; + mask = (devres >>16 ) &15 ; base = devres &0xffff ; size =16 ;
@@ -400,23 +400,53 @@ static void piix4_io_quirk(struct pci_dev *dev, const char *name, unsigned int pbreak ; size = bit; } -/* -* For now we only print it out. Eventually we'll want to -* reserve it (at least if it's in the 0x1000+ range), but -* let's get enough confirmation reports first. -*/ base &= -size; -dev_info (&dev->dev,"%s PIO at %04x-%04x\n" , name, base, base + size -1 ); + + bus_region.start = base; + bus_region.end = base + size -1 ; + res->flags |=IORESOURCE_IO ; +pcibios_bus_to_resource (dev, res, &bus_region); +dev_info (&dev->dev,"PIO at %pR\n" , res); + +return 0 ; } +static int piix4_write_io (struct pci_dev *dev,struct resource *res,int port) +{ +u32 devres; +struct pci_bus_region bus_region; + +pcibios_resource_to_bus (dev, &bus_region, res); -static void piix4_mem_quirk (struct pci_dev *dev,const char *name,unsigned int port,unsigned int enable) +pci_read_config_dword (dev, port, &devres); + devres &=0xffff0000 ; + devres |= bus_region.start &0xffff ; +pci_write_config_dword (dev, port, devres); + +return 0 ; +} +static struct resource_ops piix4_io_ops = { + .read = piix4_read_io, + .write = piix4_write_io, +}; +static void piix4_io_quirk (struct pci_dev *dev,char *name,unsigned int port, +unsigned int enable) {u32 devres; -u32 mask, size, base;pci_read_config_dword (dev, port, &devres);if ((devres & enable) != enable)return ; + +pci_add_dev_addon_resource (dev, port,0 , &piix4_io_ops, name); +} + +static int piix4_read_mem (struct pci_dev *dev,struct resource *res,int port) +{ +u32 devres; +u32 mask, size, base; +struct pci_bus_region bus_region; + +pci_read_config_dword (dev, port, &devres); base = devres &0xffff0000 ; mask = (devres &0x3f ) <<16 ; size =128 <<16 ;
@@ -426,12 +456,43 @@ static void piix4_mem_quirk(struct pci_dev *dev, const char *name, unsigned intbreak ; size = bit; } -/* -* For now we only print it out. Eventually we'll want to -* reserve it, but let's get enough confirmation reports first. -*/ base &= -size; -dev_info (&dev->dev,"%s MMIO at %04x-%04x\n" , name, base, base + size -1 ); + bus_region.start = base; + bus_region.end = base + size -1 ; + res->flags |=IORESOURCE_MEM ; +pcibios_bus_to_resource (dev, res, &bus_region); +dev_info (&dev->dev,"MMIO at %pR\n" , res); + +return 0 ; +} +static int piix4_write_mem (struct pci_dev *dev,struct resource *res,int port) +{ +u32 devres; +struct pci_bus_region bus_region; + +pcibios_resource_to_bus (dev, &bus_region, res); + +pci_read_config_dword (dev, port, &devres); + devres &=0x0000ffff ; + devres |= bus_region.start &0xffff0000 ; +pci_write_config_dword (dev, port, devres); + +return 0 ; +} +static struct resource_ops piix4_mem_ops = { + .read = piix4_read_mem, + .write = piix4_write_mem, +}; +static void piix4_mem_quirk (struct pci_dev *dev,char *name,unsigned int port, +unsigned int enable) +{ +u32 devres; + +pci_read_config_dword (dev, port, &devres); +if ((devres & enable) != enable) +return ; + +pci_add_dev_addon_resource (dev, port,0 , &piix4_mem_ops, name); }/*
Signature · 1 line
1.7.10.4
PATCH v3 26/27 PCI: Make quirk_io_region to use addon_res
In reply to Yinghai LuAfter they are put in add-on resources, they will be safely claimed later.
--- drivers/pci/quirks.c | 155 +++++++++++++++++++++++--------------------------- 1 file changed, 70 insertions(+), 85 deletions(-)
drivers/pci/quirks.c +70 -85
@@ -324,29 +324,61 @@ static void quirk_cs5536_vsa(struct pci_dev *dev) }DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_AMD ,PCI_DEVICE_ID_AMD_CS5536_ISA , quirk_cs5536_vsa); -static void quirk_io_region (struct pci_dev *dev,unsigned region, -unsigned size,int nr,const char *name) +static int quirk_read_io (struct pci_dev *dev,struct resource *res,int port) { - region &= ~(size-1 ); -if (region) { -struct pci_bus_region bus_region; -struct resource *res = dev->resource + nr; +struct pci_bus_region bus_region; +u16 region; +unsigned size =to_pci_dev_addon_resource (res)->size; - res->name =pci_name (dev); - res->start = region; - res->end = region + size -1 ; - res->flags =IORESOURCE_IO ; +pci_read_config_word (dev, port, ®ion); + region &= ~(size -1 ); -/* Convert from PCI bus to resource space. */ - bus_region.start = res->start; - bus_region.end = res->end; -pcibios_bus_to_resource (dev, res, &bus_region); +/* Convert from PCI bus to resource space. */ + bus_region.start = region; + bus_region.end = region + size -1 ; -if (pci_claim_resource (dev, nr) ==0 ) -dev_info (&dev->dev,"quirk: %pR claimed by %s\n" , - res, name); - } -} +pcibios_bus_to_resource (dev, res, &bus_region); + + res->flags |=IORESOURCE_IO ; +dev_info (&dev->dev,"PIO at %pR\n" , res); + +return 0 ; +} +static int quirk_write_io (struct pci_dev *dev,struct resource *res,int port) +{ +struct pci_bus_region bus_region; +u16 region; +unsigned size =to_pci_dev_addon_resource (res)->size; + +pci_read_config_word (dev, port, ®ion); + region &= size -1 ; + +/* Convert to PCI bus space. */ +pcibios_resource_to_bus (dev, &bus_region, res); + region |= bus_region.start & (~(size -1 )); + +pci_write_config_word (dev, port, region); + +return 0 ; +} +static struct resource_ops quirk_io_ops = { + .read = quirk_read_io, + .write = quirk_write_io, +}; + +static void quirk_io_region (struct pci_dev *dev,int port, +unsigned size,char *name) +{ +u16 region; + +pci_read_config_word (dev, port, ®ion); + region &= size -1 ; + +if (!region) +return ; + +pci_add_dev_addon_resource (dev, port, size, &quirk_io_ops, name); +}/* * ATI Northbridge setups MCE the processor if you even
@@ -374,12 +406,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_RS100, quirk_ati_*/ static void quirk_ali7101_acpi (struct pci_dev *dev) { -u16 region; - -pci_read_config_word (dev,0xE0 , ®ion); -quirk_io_region (dev, region,64 ,PCI_BRIDGE_RESOURCES ,"ali7101 ACPI" ); -pci_read_config_word (dev,0xE2 , ®ion); -quirk_io_region (dev, region,32 ,PCI_BRIDGE_RESOURCES +1 ,"ali7101 SMB" ); +quirk_io_region (dev,0xE0 ,64 ,"ali7101 ACPI" ); +quirk_io_region (dev,0xE2 ,32 ,"ali7101 SMB" ); }DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_AL ,PCI_DEVICE_ID_AL_M7101 , quirk_ali7101_acpi);
@@ -503,12 +531,10 @@ static void piix4_mem_quirk(struct pci_dev *dev, char *name, unsigned int port,*/ static void quirk_piix4_acpi (struct pci_dev *dev) { -u32 region, res_a; +u32 res_a; -pci_read_config_dword (dev,0x40 , ®ion); -quirk_io_region (dev, region,64 ,PCI_BRIDGE_RESOURCES ,"PIIX4 ACPI" ); -pci_read_config_dword (dev,0x90 , ®ion); -quirk_io_region (dev, region,16 ,PCI_BRIDGE_RESOURCES +1 ,"PIIX4 SMB" ); +quirk_io_region (dev,0x40 ,64 ,"PIIX4 ACPI" ); +quirk_io_region (dev,0x90 ,16 ,"PIIX4 SMB" );/* Device resource A has enables for some of the other ones */ pci_read_config_dword (dev,0x5c , &res_a);
@@ -552,7 +578,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82443MX_3, qui*/ static void quirk_ich4_lpc_acpi (struct pci_dev *dev) { -u32 region;u8 enable;/*
@@ -564,22 +589,12 @@ static void quirk_ich4_lpc_acpi(struct pci_dev *dev)*/ pci_read_config_byte (dev,ICH_ACPI_CNTL , &enable); -if (enable &ICH4_ACPI_EN ) { -pci_read_config_dword (dev,ICH_PMBASE , ®ion); - region &=PCI_BASE_ADDRESS_IO_MASK ; -if (region >=PCIBIOS_MIN_IO ) -quirk_io_region (dev, region,128 ,PCI_BRIDGE_RESOURCES , -"ICH4 ACPI/GPIO/TCO" ); - } +if (enable &ICH4_ACPI_EN ) +quirk_io_region (dev,ICH_PMBASE ,128 ,"ICH4 ACPI/GPIO/TCO" );pci_read_config_byte (dev,ICH4_GPIO_CNTL , &enable); -if (enable &ICH4_GPIO_EN ) { -pci_read_config_dword (dev,ICH4_GPIOBASE , ®ion); - region &=PCI_BASE_ADDRESS_IO_MASK ; -if (region >=PCIBIOS_MIN_IO ) -quirk_io_region (dev, region,64 , -PCI_BRIDGE_RESOURCES +1 ,"ICH4 GPIO" ); - } +if (enable &ICH4_GPIO_EN ) +quirk_io_region (dev,ICH4_GPIOBASE ,64 ,"ICH4 GPIO" ); }DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_INTEL ,PCI_DEVICE_ID_INTEL_82801AA_0 , quirk_ich4_lpc_acpi);DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_INTEL ,PCI_DEVICE_ID_INTEL_82801AB_0 , quirk_ich4_lpc_acpi);
@@ -594,26 +609,15 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB_1, quistatic void ich6_lpc_acpi_gpio (struct pci_dev *dev) { -u32 region;u8 enable;pci_read_config_byte (dev,ICH_ACPI_CNTL , &enable); -if (enable &ICH6_ACPI_EN ) { -pci_read_config_dword (dev,ICH_PMBASE , ®ion); - region &=PCI_BASE_ADDRESS_IO_MASK ; -if (region >=PCIBIOS_MIN_IO ) -quirk_io_region (dev, region,128 ,PCI_BRIDGE_RESOURCES , -"ICH6 ACPI/GPIO/TCO" ); - } +if (enable &ICH6_ACPI_EN ) +quirk_io_region (dev,ICH_PMBASE ,128 ,"ICH6 ACPI/GPIO/TCO" );pci_read_config_byte (dev,ICH6_GPIO_CNTL , &enable); -if (enable &ICH6_GPIO_EN ) { -pci_read_config_dword (dev,ICH6_GPIOBASE , ®ion); - region &=PCI_BASE_ADDRESS_IO_MASK ; -if (region >=PCIBIOS_MIN_IO ) -quirk_io_region (dev, region,64 , -PCI_BRIDGE_RESOURCES +1 ,"ICH6 GPIO" ); - } +if (enable &ICH6_GPIO_EN ) +quirk_io_region (dev,ICH6_GPIOBASE ,64 ,"ICH6 GPIO" ); }static void ich6_lpc_generic_decode (struct pci_dev *dev,unsigned reg,const char *name,int dynsize)
@@ -711,13 +715,8 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH10_1, qui*/ static void quirk_vt82c586_acpi (struct pci_dev *dev) { -u32 region; - -if (dev->revision &0x10 ) { -pci_read_config_dword (dev,0x48 , ®ion); - region &=PCI_BASE_ADDRESS_IO_MASK ; -quirk_io_region (dev, region,256 ,PCI_BRIDGE_RESOURCES ,"vt82c586 ACPI" ); - } +if (dev->revision &0x10 ) +quirk_io_region (dev,0x48 ,256 ,"vt82c586 ACPI" ); }DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_VIA ,PCI_DEVICE_ID_VIA_82C586_3 , quirk_vt82c586_acpi);
@@ -729,18 +728,11 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_3, quirk_vt*/ static void quirk_vt82c686_acpi (struct pci_dev *dev) { -u16 hm; -u32 smb; -quirk_vt82c586_acpi (dev); -pci_read_config_word (dev,0x70 , &hm); - hm &=PCI_BASE_ADDRESS_IO_MASK ; -quirk_io_region (dev, hm,128 ,PCI_BRIDGE_RESOURCES +1 ,"vt82c686 HW-mon" ); +quirk_io_region (dev,0x70 ,128 ,"vt82c686 HW-mon" ); -pci_read_config_dword (dev,0x90 , &smb); - smb &=PCI_BASE_ADDRESS_IO_MASK ; -quirk_io_region (dev, smb,16 ,PCI_BRIDGE_RESOURCES +2 ,"vt82c686 SMB" ); +quirk_io_region (dev,0x90 ,16 ,"vt82c686 SMB" ); }DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_VIA ,PCI_DEVICE_ID_VIA_82C686_4 , quirk_vt82c686_acpi);
@@ -751,15 +743,8 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_4, quirk_vt*/ static void quirk_vt8235_acpi (struct pci_dev *dev) { -u16 pm, smb; - -pci_read_config_word (dev,0x88 , &pm); - pm &=PCI_BASE_ADDRESS_IO_MASK ; -quirk_io_region (dev, pm,128 ,PCI_BRIDGE_RESOURCES ,"vt8235 PM" ); - -pci_read_config_word (dev,0xd0 , &smb); - smb &=PCI_BASE_ADDRESS_IO_MASK ; -quirk_io_region (dev, smb,16 ,PCI_BRIDGE_RESOURCES +1 ,"vt8235 SMB" ); +quirk_io_region (dev,0x88 ,128 ,"vt8235 PM" ); +quirk_io_region (dev,0xd0 ,16 ,"vt8235 SMB" ); }DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_VIA ,PCI_DEVICE_ID_VIA_8235 , quirk_vt8235_acpi);
Signature · 1 line
1.7.10.4
Bjorn Helgaas
maintainerRe: [PATCH v3 26/27] PCI: Make quirk_io_region to use addon_res
Quoted 94 lines · Yinghai Lu
After they are put in add-on resources, they will be safely claimed later.
Signed-off-by Yinghai Lu <[email protected]>--- drivers/pci/quirks.c | 155 +++++++++++++++++++++++--------------------------- 1 file changed, 70 insertions(+), 85 deletions(-)drivers/pci/quirks.c +51
-19@@ -324,29 +324,61 @@ static void quirk_cs5536_vsa(struct pci_dev *dev) }DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_AMD ,PCI_DEVICE_ID_AMD_CS5536_ISA , quirk_cs5536_vsa); -static void quirk_io_region (struct pci_dev *dev,unsigned region, -unsigned size,int nr,const char *name) +static int quirk_read_io (struct pci_dev *dev,struct resource *res,int port) { - region &= ~(size-1 ); -if (region) { -struct pci_bus_region bus_region; -struct resource *res = dev->resource + nr; +struct pci_bus_region bus_region; +u16 region; +unsigned size =to_pci_dev_addon_resource (res)->size; - res->name =pci_name (dev); - res->start = region; - res->end = region + size -1 ; - res->flags =IORESOURCE_IO ; +pci_read_config_word (dev, port, ®ion); + region &= ~(size -1 ); -/* Convert from PCI bus to resource space. */ - bus_region.start = res->start; - bus_region.end = res->end; -pcibios_bus_to_resource (dev, res, &bus_region); +/* Convert from PCI bus to resource space. */ + bus_region.start = region; + bus_region.end = region + size -1 ; -if (pci_claim_resource (dev, nr) ==0 ) -dev_info (&dev->dev,"quirk: %pR claimed by %s\n" , - res, name); - } -} +pcibios_bus_to_resource (dev, res, &bus_region); + + res->flags |=IORESOURCE_IO ; +dev_info (&dev->dev,"PIO at %pR\n" , res); + +return 0 ; +} +static int quirk_write_io (struct pci_dev *dev,struct resource *res,int port) +{ +struct pci_bus_region bus_region; +u16 region; +unsigned size =to_pci_dev_addon_resource (res)->size; + +pci_read_config_word (dev, port, ®ion); + region &= size -1 ; + +/* Convert to PCI bus space. */ +pcibios_resource_to_bus (dev, &bus_region, res); + region |= bus_region.start & (~(size -1 )); + +pci_write_config_word (dev, port, region); + +return 0 ; +} +static struct resource_ops quirk_io_ops = { + .read = quirk_read_io, + .write = quirk_write_io, +}; + +static void quirk_io_region (struct pci_dev *dev,int port, +unsigned size,char *name) +{ +u16 region; + +pci_read_config_word (dev, port, ®ion); + region &= size -1 ; + +if (!region) +return ; + +pci_add_dev_addon_resource (dev, port, size, &quirk_io_ops, name); +}/* * ATI Northbridge setups MCE the processor if you even @@ -374,12 +406,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_RS100, quirk_ati_*/
drivers/pci/quirks.c DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_RS100, quirk_ati_ · line 410
Quoted 10 lines
drivers/pci/quirks.c +2
-6static void quirk_ali7101_acpi (struct pci_dev *dev) { -u16 region; - -pci_read_config_word (dev,0xE0 , ®ion); -quirk_io_region (dev, region,64 ,PCI_BRIDGE_RESOURCES ,"ali7101 ACPI" ); -pci_read_config_word (dev,0xE2 , ®ion); -quirk_io_region (dev, region,32 ,PCI_BRIDGE_RESOURCES +1 ,"ali7101 SMB" ); +quirk_io_region (dev,0xE0 ,64 ,"ali7101 ACPI" ); +quirk_io_region (dev,0xE2 ,32 ,"ali7101 SMB" );
This patch has two changes that need to be separated:
1) Refactoring quirk_io_region() so the pci_read_config_word() is
done by quirk_io_region() rather than the caller.
2) Whatever pci_dev resource changes we end up making.
I think part 1 can be done at any time and is probably not controversial.
Bjorn
Re: [PATCH v3 26/27] PCI: Make quirk_io_region to use addon_res
Quoted 7 lines · Bjorn Helgaas
This patch has two changes that need to be separated:
1) Refactoring
quirk_io_region()so thepci_read_config_word()is
done byquirk_io_region()rather than the caller.
2) Whateverpci_devresource changes we end up making.I think part 1 can be done at any time and is probably not controversial.
ok, will put that separated one to be first one in the next reversion of patchset.
Thanks
Yinghai
PATCH v3 27/27 PCI: Use addon_fixed_resource with ati fixed resource
In reply to Yinghai LuAfter they are put in addon_res list, they will be safely claimed later.
--- drivers/pci/quirks.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/pci/quirks.c +5 -3
@@ -386,10 +386,12 @@ static void quirk_io_region(struct pci_dev *dev, int port,*/ static void quirk_ati_exploding_mce (struct pci_dev *dev) { -dev_info (&dev->dev,"ATI Northbridge, reserving I/O ports 0x3b0 to 0x3bb\n" ); +dev_info (&dev->dev,"ATI Northbridge, will reserve I/O ports 0x3b0 to 0x3bb\n" );/* Mae rhaid i ni beidio ag edrych ar y lleoliadiau I/O hyn */ -request_region (0x3b0 ,0x0C ,"RadeonIGP" ); -request_region (0x3d3 ,0x01 ,"RadeonIGP" ); +pci_add_dev_addon_fixed_resource (dev,0x3B0 ,0x0C ,IORESOURCE_IO ,0 , +"RadeonIGP" ); +pci_add_dev_addon_fixed_resource (dev,0x3d3 ,0x01 ,IORESOURCE_IO ,0 , +"RadeonIGP" ); }DECLARE_PCI_FIXUP_FINAL (PCI_VENDOR_ID_ATI ,PCI_DEVICE_ID_ATI_RS100 , quirk_ati_exploding_mce);
Signature · 1 line
1.7.10.4