diff mbox

[RFC] methods to access resources of a struct pci_dev

Message ID CAE9FiQW08aoQBiy+37x-VMYrG-OunqzfuRpB1JhDAgNL3AxEuw@mail.gmail.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Yinghai Lu Aug. 16, 2012, 4:11 a.m. UTC 
On Wed, Aug 15, 2012 at 8:26 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> On Wed, Aug 15, 2012 at 03:25:06PM -0600, Bjorn Helgaas wrote:
>
> I am fine with this approach. I have never encountered the need for 'no'
> based iterator like 'for_each_pci_dev_noiov_resource' or
> 'for_each_pci_dev_base_norom_resource'. While abstracting the code and
> replacing explicit references to the resources in various peices of code
> including the drivers, I just encountered the need for the 'yes' based
> iterators like the one that I added.
>
> However if there is a need for 'no' based iterators, it should be easy
> to incorporate them using flags. Something like
>
> for_each_pci_resource(dev, res, i, flags)
>
> where flags can be
> #define PCI_STD_RES 0x01
> #define PCI_ROM_RES 0x02
> #define PCI_BRIDGE_RES 0x04
> #define PCI_IOV_RES 0x08
> #define PCI_ALL_RES PCI_STD_RES|PCI_ROM_RES|PCI_BRIDGE_RES|PCI_IOV_RES
> #define PCI_NOSTD_RES PCI_ALL_RES&(^PCI_STD_RES)
> #define PCI_NOIOV_RES PCI_ALL_RES&(^PCI_IOV_RES)
> so on and so forth
>
> Yinghai if you are ok with this approach, let me code up all the
> iterators. You can incorporate your patches based on those iterators and
> I can change all my 40+ patches that change various driver sources to
> use this iterator.

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

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

From cd192f0ed93203ef6bac2a44c138899190fb5793 Mon Sep 17 00:00:00 2001
From: Yinghai Lu <yinghai@kernel.org>
Date: Tue, 26 Jun 2012 17:02:04 -0700
Subject: [PATCH] PCI: Add for_each_resource helpers to make resource loop
 easier.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 include/linux/pci.h |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

 {
 #ifdef CONFIG_PCI_IOV

Comments

Ram Pai Aug. 16, 2012, 4:41 a.m. UTC | #1 
On Wed, Aug 15, 2012 at 09:11:03PM -0700, Yinghai Lu wrote:
> On Wed, Aug 15, 2012 at 8:26 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> > On Wed, Aug 15, 2012 at 03:25:06PM -0600, Bjorn Helgaas wrote:
> >
> > I am fine with this approach. I have never encountered the need for 'no'
> > based iterator like 'for_each_pci_dev_noiov_resource' or
> > 'for_each_pci_dev_base_norom_resource'. While abstracting the code and
> > replacing explicit references to the resources in various peices of code
> > including the drivers, I just encountered the need for the 'yes' based
> > iterators like the one that I added.
> >
> > However if there is a need for 'no' based iterators, it should be easy
> > to incorporate them using flags. Something like
> >
> > for_each_pci_resource(dev, res, i, flags)
> >
> > where flags can be
> > #define PCI_STD_RES 0x01
> > #define PCI_ROM_RES 0x02
> > #define PCI_BRIDGE_RES 0x04
> > #define PCI_IOV_RES 0x08
> > #define PCI_ALL_RES PCI_STD_RES|PCI_ROM_RES|PCI_BRIDGE_RES|PCI_IOV_RES
> > #define PCI_NOSTD_RES PCI_ALL_RES&(^PCI_STD_RES)
> > #define PCI_NOIOV_RES PCI_ALL_RES&(^PCI_IOV_RES)
> > so on and so forth
> >
> > Yinghai if you are ok with this approach, let me code up all the
> > iterators. You can incorporate your patches based on those iterators and
> > I can change all my 40+ patches that change various driver sources to
> > use this iterator.
> 
> Do you mean that you will have updated patch for
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=cd192f0ed93203ef6bac2a44c138899190fb5793
> ?

Yes.

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

Ok. Will have it your way soon.
RP

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

Patch

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 77778cb..dd577e3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -363,6 +363,61 @@  struct pci_dev {

 struct resource *pci_dev_resource_n(struct pci_dev *dev, int n);

+#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 (i = 0;							\
+	     (res = pci_dev_resource_n(dev, i)) || i < PCI_NUM_RESOURCES; \
+	     i++)
+/* exclude bridge resources */
+#define for_each_pci_dev_nobridge_resource(dev, res, i)			\
+	for (i = 0;							\
+	     (res = pci_dev_resource_n(dev, i)) || i < PCI_NUM_RESOURCES; \
+	     i = (i != (PCI_BRIDGE_RESOURCES - 1)) ? (i+1) : PCI_NUM_RESOURCES)
+/* exclude bridge and IOV resources */
+#define for_each_pci_dev_base_resource(dev, res, i)			\
+	for (i = 0;							\
+	     (res = pci_dev_resource_n(dev, i)) || i < PCI_NUM_RESOURCES; \
+	     i = (i != PCI_ROM_RESOURCE) ? (i+1) : PCI_NUM_RESOURCES)
+/* exclude ROM and bridge and IOV resources */
+#define for_each_pci_dev_base_norom_resource(dev, res, i)		\
+	for (i = 0;							\
+	     (res = pci_dev_resource_n(dev, i)) || i < PCI_NUM_RESOURCES; \
+	     i = (i != (PCI_ROM_RESOURCE-1)) ? (i+1) : PCI_NUM_RESOURCES)
+/* exclude ROM and bridge resources */
+#define for_each_pci_dev_base_iov_norom_resource(dev, res, i)		\
+	for (i = 0;							\
+	     (res = pci_dev_resource_n(dev, i)) || i < PCI_NUM_RESOURCES; \
+	     i = (i != (PCI_ROM_RESOURCE-1) && i !=
(PCI_BRIDGE_RESOURCES-1)) ? (i+1) : \
+	           ((i == PCI_ROM_RESOURCE-1) ? (PCI_ROM_RESOURCE+1) :
PCI_NUM_RESOURCES))
+/* exclude IOV resources */
+#define for_each_pci_dev_noiov_resource(dev, res, i)			\
+	for (i = 0;							\
+	     (res = pci_dev_resource_n(dev, i)) || i < PCI_NUM_RESOURCES; \
+	     i = (i != PCI_ROM_RESOURCE) ? (i+1) : PCI_BRIDGE_RESOURCES)
+/* only std resources */
+#define for_each_pci_dev_std_resource(dev, res, i)			\
+	for (i = PCI_STD_RESOURCES;					\
+	   (res = pci_dev_resource_n(dev, i)) && i < (PCI_STD_RESOURCE_END+1); \
+	     i++)
+/* only IOV resources */
+#define for_each_pci_dev_iov_resource(dev, res, i)			\
+	for (i = PCI_IOV_RESOURCES;					\
+	   (res = pci_dev_resource_n(dev, i)) && i < (PCI_IOV_RESOURCE_END+1); \
+	     i++)
+/* only bridge resources */
+#define for_each_pci_dev_bridge_resource(dev, res, i)			\
+	for (i = PCI_BRIDGE_RESOURCES;					\
+	(res = pci_dev_resource_n(dev, i)) && i < (PCI_BRIDGE_RESOURCE_END+1); \
+	     i++)
+/* only addon resources */
+#define for_each_pci_dev_addon_resource(dev, res, i)			\
+	for (i = PCI_NUM_RESOURCES;					\
+	     (res = pci_dev_resource_n(dev, i));			\
+	     i++)
+
 static inline struct pci_dev *pci_physfn(struct pci_dev *dev)