Re: [RFC PATCH v2 ]pci: pci resource iterator

[Yinghai Lu <yinghai@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]

On Tue, Aug 21, 2012 at 8:13 AM, Ram Pai <linuxram@us.ibm.com> wrote:
> PCI: pci resource iterator
>
> Currently pci_dev structure holds an array of 17 PCI resources; six base
> BARs, one ROM BAR, four BRIDGE BARs, six sriov BARs.  This is wasteful.
> A bridge device just needs the 4 bridge resources. A non-bridge device
> just needs the six base resources and one ROM resource. The sriov
> resources are needed only if the device has SRIOV capability.
>
> The pci_dev structure needs to be re-organized to avoid unnecessary
> bloating.  However too much code outside the pci-bus driver, assumes the
> internal details of the pci_dev structure, thus making it hard to
> re-organize the datastructure.
>
> As a first step this patch provides generic methods to access the
> resource structure of the pci_dev.
>
> Once this patch is accepted, I have another 40+ patches that modify all
> the files that directly access the resource structure, to use the new
> methods provided in the first step.
>
> Finally we can re-organize the resource structure in the pci_dev
> structure and correspondingly update the methods.
>
> This patch is compile tested only.
>
> Changelog:
>         Consolidated iterator interface as per Bjorn's suggestion.
>
> +#define for_each_pci_resource(dev, res, flag) \
> +       for (res = pci_next_resource(dev, NULL, flag); res; \
> +                       res = pci_next_resource(dev, res, flag))
> +

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

because some loop body is still using the idx.

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

 PCI: Add addon_resource support for pci_dev

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

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

Thanks

Yinghai

[-- Attachment #2: ram_pci_it_v3.patch --]
[-- Type: application/octet-stream, Size: 3340 bytes --]

From: Ram Pai <linuxram@us.ibm.com>
To: linux-pci@vger.kernel.org
Cc: Yinghai Lu <yinghai@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>
Subject: [RFC PATCH v3]pci: pci resource iterator

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

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

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

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

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

This patch is compile tested only.

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

-v3: add the idx back

Signed-off-by: Ram Pai <linuxram@us.ibm.com>

---
 include/linux/pci.h |   41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -1410,6 +1410,47 @@ static inline struct pci_dev *pci_dev_ge
 	 (pci_resource_end((dev), (bar)) -		\
 	  pci_resource_start((dev), (bar)) + 1))
 
+#define PCI_STD_RES		0x01
+#define PCI_ROM_RES		0x02
+#define PCI_BRIDGE_RES		0x04
+#define PCI_IOV_RES		0x08
+#define PCI_ALL_RES		(PCI_STD_RES | PCI_ROM_RES | PCI_BRIDGE_RES | PCI_IOV_RES)
+#define PCI_NOSTD_RES		(PCI_ALL_RES & ^PCI_STD_RES)
+#define PCI_NOIOV_RES		(PCI_ALL_RES & ^PCI_IOV_RES)
+#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)
+
+static inline struct resource *pci_next_resource(struct pci_dev *pdev,
+			struct resource *res, int flag, int *next_i)
+{
+	int i = *next_i;
+
+	while (++i < PCI_NUM_RESOURCES) {
+		if ((i >= 0 && i < PCI_ROM_RESOURCE) && (flag & PCI_STD_RES))
+			goto found;
+		else if ((i == PCI_ROM_RESOURCE) && (flag & PCI_ROM_RES))
+			goto found;
+		else if ((i <= PCI_IOV_RESOURCE_END) && (flag & PCI_IOV_RES))
+			goto found;
+		else if ((i <= PCI_BRIDGE_RESOURCE_END) && (flag & PCI_BRIDGE_RES))
+			goto found;
+	}
+
+	*next_id = -1;
+	return NULL;
+
+found:
+	*next_id = i;
+	return pci_dev_resource_n(i);
+}
+
+#define for_each_pci_resource(dev, res, i, flag) \
+	for (i = -1, res = pci_next_resource(dev, NULL, flag, &i); res; \
+			res = pci_next_resource(dev, res, flag, &i))
+
 /* Similar to the helpers above, these manipulate per-pci_dev
  * driver-specific data.  They are really just a wrapper around
  * the generic device structure functions of these calls.