diff mbox

[RFC,v3] pci: pci resource iterator

Message ID 20120904032717.GD2438@ram-ThinkPad-T61 (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Ram Pai Sept. 4, 2012, 3:27 a.m. UTC 
On Mon, Sep 03, 2012 at 11:20:45AM -0700, Yinghai Lu wrote:
> On Mon, Sep 3, 2012 at 2:08 AM, Ram Pai <linuxram@us.ibm.com> wrote:
> > On Mon, Sep 03, 2012 at 01:07:46AM -0700, Yinghai Lu wrote:
> >
> > Anyway I am ok with either patch.
> 
> please check -v7.

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


From: Ram Pai <linuxram@us.ibm.com>
To: linux-pci@vger.kernel.org
Subject: [PATCH v7]pci: pci resource iterator

Currently pci_dev structure holds an array of 17 PCI resources; six base
BARs, one ROM BAR, four BRIDGE BARs, six sriov BARs.  This is wasteful.
A bridge device just needs the 4 bridge resources. A non-bridge device
just needs the six base resources and one ROM resource. The sriov
resources are needed only if the device has SRIOV capability.

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

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

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

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/probe.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |   24 ++++++++++++++++++++++++
 2 files changed, 71 insertions(+)


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Yinghai Lu Sept. 18, 2012, 12:03 a.m. UTC | #1 
On Mon, Sep 3, 2012 at 8:27 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> On Mon, Sep 03, 2012 at 11:20:45AM -0700, Yinghai Lu wrote:
>> On Mon, Sep 3, 2012 at 2:08 AM, Ram Pai <linuxram@us.ibm.com> wrote:
>> > On Mon, Sep 03, 2012 at 01:07:46AM -0700, Yinghai Lu wrote:
>> >
>> > Anyway I am ok with either patch.
>>
>> please check -v7.
>
> Looks good to me. I am inlining the patch for others to comment.
> BTW: your patch that introduces pci_dev_resource_n() will have
> to be applied before applying this patch.
>
>

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

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

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

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

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

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ram Pai Sept. 21, 2012, 6:18 a.m. UTC | #2 
On Mon, Sep 17, 2012 at 05:03:56PM -0700, Yinghai Lu wrote:
> On Mon, Sep 3, 2012 at 8:27 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> > On Mon, Sep 03, 2012 at 11:20:45AM -0700, Yinghai Lu wrote:
> >> On Mon, Sep 3, 2012 at 2:08 AM, Ram Pai <linuxram@us.ibm.com> wrote:
> >> > On Mon, Sep 03, 2012 at 01:07:46AM -0700, Yinghai Lu wrote:
> >> >
> >> > Anyway I am ok with either patch.
> >>
> >> please check -v7.
> >
> > Looks good to me. I am inlining the patch for others to comment.
> > BTW: your patch that introduces pci_dev_resource_n() will have
> > to be applied before applying this patch.
> >
> >
> 
> just updated for-pci-for-each-res-addon branch...
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-for-each-res-addon
> 
> now it only depends on pci-root-bus-hotplug now, and
> pci-root-bus-hotplug only depends on pci/next without busn-alloc.
> 
> So after Bjorn accept pci-root-bus-hotplug, we could push that to him.
> 
> Please check for-pci-for-each-addon branch.
> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
> for-pci-for-each-res-addon

Checked and the patch looks good. 

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

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

This is the patch I am talking about:

commit a37fbc4685ab086581f9842630c997ca78186ea4
Author: Yinghai Lu <yinghai@kernel.org>
Date:   Mon Sep 3 00:32:00 2012 -0700

PCI: Add for_each_resource helpers to make resource loop easier.
 
+#define resno_is_for_bridge(n)                                        \ 
+       ((n) >= PCI_BRIDGE_RESOURCES && (n) <= PCI_BRIDGE_RESOURCE_END)
+
+/* all (include bridge) resources */
+#define for_each_pci_dev_all_resource(dev, res, i)                     \
	+ for_each_pci_resource(dev, res, i, PCI_ALL_RES)
	.......
	......
Yinghai Lu Sept. 21, 2012, 6:27 a.m. UTC | #3 
On Thu, Sep 20, 2012 at 11:18 PM, Ram Pai <linuxram@us.ibm.com> wrote:

> Checked and the patch looks good.
>
> But later you have added another patch which introduces lot many macros ...
>
> Will that patch also be pushed to Bjorn? I thought Bjorn resisted the
> idea of having multiple such iterators..

no. I removed that patches 3 days ago...

please check it again.

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

-Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -360,6 +360,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
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -123,6 +123,53 @@  int pci_dev_resource_idx(struct pci_dev
 	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;
+}
+
 static u64 pci_size(u64 base, u64 maxbase, u64 mask)
 {
 	u64 size = mask & maxbase;	/* Find the significant bits */