On Tue, Aug 21, 2012 at 04:22:52PM -0700, Yinghai Lu wrote: > On Tue, Aug 21, 2012 at 8:13 AM, Ram Pai <linuxram@xxxxxxxxxx> 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. by exposing idx through the interface, we are exposing the implementation to the enduser. I want the end user not know that the resources are structured as a array. This will help easily restructure resources in the pci_dev structure to whatever implementation we want, linked list or hash or whatever... Why can't the addon resource be hidden behind the interface? something like this? static inline struct resource *pci_next_resource(struct pci_dev *pdev, struct resource *res, int flag) { int i = res? pci_resource_number(pdev, res) : -1; while (++i < PCI_NUM_RESOURCES) { if ((i >= 0 && i < PCI_ROM_RESOURCE) && (flag & PCI_STD_RES)) return pci_get_std_resource(pdev, i); else if ((i == PCI_ROM_RESOURCE) && (flag & PCI_ROM_RES)) return pci_get_rom_resource(pdev); else if ((i <= PCI_IOV_RESOURCE_END) && (flag & PCI_IOV_RES)) return pci_get_sriov_resource(pdev, i-PCI_IOV_RESOURCES); else if ((i <= PCI_BRIDGE_RESOURCE_END) && (flag & PCI_BRIDGE_RES)) return pci_get_bridge_resource(pdev, i-PCI_BRIDGE_RESOURCES); } if (flag & PCI_ADDON_RES) { if ( i == PCI_NUM_RESOURCES) { // return the first element of the addon list; } else { // return the next element in the list; } } return NULL; } -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html