Skip to contentSkip to conversation

PCI and PCI Hotplug changes and fixes for 2.5.70

PATCH In review Greg KH 3 participants 24 messages Jun 5, 2003

Subsystems (3)
lore
Share & tools
lore

Greg KH

author
#0 lore raw

PATCH 1/1 PCI and PCI Hotplug changes and fixes for 2.5.70

Hi,

Here's some PCI and PCI Hotplug changes that are against the latest 2.5.70 bk tree. They contain the following:
- dynamic id bug fixes from Matt Domsch
- move drivers/hotplug/ to drivers/pci/hotplug/ I should have done this originally, and I'm sorry for ignoring everyone who told me about this, you were right :)
- Because of the PCI Hotplug drivers moving, I was able to remove about 70 lines from include/linux/pci.h, which should speed up everyone's compile time some unmeasurable amount :)
- Removed some exported PCI core variables and functions that didn't need to be exported (no one was using them.)
- Next step in the pci device list locking changes, now the pci device's reference count is incremented when the pci driver is probed, and decremented when the driver is done with the device.
- removed all users of pci_for_each_dev() which was a macro that directly accessed the pci device lists. This was replaced with a call to pci_find_device() which will be later changed to properly lock access to the pci device lists.
- Some "hand made" pci functions were removed from the PCI hotplug drivers and now they call the proper PCI core functions.

Because of the pci_for_each_dev() changes, these patches touch a lot of different files all over the tree, sorry, but it's necessary for us to get proper pci device locking.

On the plus side, I ended up removing 58 lines of code from the tree :)

Please pull from:

	bk://kernel.bkbits.net/gregkh/linux/pci-2.5

thanks,

greg k-h

p.s. I'll send these as patches in response to this email to lkml for those who want to see them.

 drivers/hotplug/Kconfig                   |  120 -
 drivers/hotplug/Makefile                  |   44 
 drivers/hotplug/acpiphp.h                 |  262 --
 drivers/hotplug/acpiphp_core.c            |  505 ----
 drivers/hotplug/acpiphp_glue.c            | 1335 ------------
 drivers/hotplug/acpiphp_pci.c             |  510 ----
 drivers/hotplug/acpiphp_res.c             |  699 ------
 drivers/hotplug/cpci_hotplug.h            |  100 
 drivers/hotplug/cpci_hotplug_core.c       |  919 --------
 drivers/hotplug/cpci_hotplug_pci.c        |  647 ------
 drivers/hotplug/cpcihp_generic.c          |  290 --
 drivers/hotplug/cpcihp_zt5550.c           |  306 --
 drivers/hotplug/cpcihp_zt5550.h           |   79 
 drivers/hotplug/cpqphp.h                  |  912 --------
 drivers/hotplug/cpqphp_core.c             | 1541 --------------
 drivers/hotplug/cpqphp_ctrl.c             | 3084 ------------------------------
 drivers/hotplug/cpqphp_nvram.c            |  667 ------
 drivers/hotplug/cpqphp_nvram.h            |   57 
 drivers/hotplug/cpqphp_pci.c              | 1548 ---------------
 drivers/hotplug/cpqphp_sysfs.c            |  143 -
 drivers/hotplug/ibmphp.h                  |  772 -------
 drivers/hotplug/ibmphp_core.c             | 1417 -------------
 drivers/hotplug/ibmphp_ebda.c             | 1228 -----------
 drivers/hotplug/ibmphp_hpc.c              | 1228 -----------
 drivers/hotplug/ibmphp_pci.c              | 1758 -----------------
 drivers/hotplug/ibmphp_res.c              | 2157 --------------------
 drivers/hotplug/pci_hotplug.h             |  146 -
 drivers/hotplug/pci_hotplug_core.c        |  666 ------
 drivers/hotplug/pcihp_skeleton.c          |  432 ----
 arch/alpha/kernel/sys_sio.c               |    4 
 arch/arm/kernel/bios32.c                  |    4 
 arch/i386/Kconfig                         |    2 
 arch/i386/kernel/cpu/cpufreq/gx-suspmod.c |    4 
 arch/i386/pci/i386.c                      |    8 
 arch/i386/pci/irq.c                       |   11 
 arch/ia64/Kconfig                         |    2 
 arch/ia64/hp/common/sba_iommu.c           |    4 
 arch/ia64/sn/io/pci_bus_cvlink.c          |    2 
 arch/ia64/sn/io/pciba.c                   |    8 
 arch/ia64/sn/io/sn2/pci_bus_cvlink.c      |    2 
 arch/m68knommu/Kconfig                    |    2 
 arch/mips/ddb5074/pci.c                   |    8 
 arch/mips/ddb5476/pci.c                   |    8 
 arch/mips/ddb5xxx/ddb5477/pci.c           |    4 
 arch/mips/mips-boards/generic/pci.c       |    4 
 arch/mips/sni/pci.c                       |    4 
 arch/mips64/mips-boards/generic/pci.c     |    4 
 arch/mips64/sgi-ip32/ip32-pci.c           |   10 
 arch/ppc/kernel/pci.c                     |   12 
 arch/ppc/platforms/chrp_pci.c             |    4 
 arch/ppc/platforms/gemini_pci.c           |    4 
 arch/ppc/platforms/pmac_pci.c             |    8 
 arch/ppc/platforms/prep_pci.c             |    7 
 arch/ppc64/Kconfig                        |    9 
 arch/ppc64/kernel/iSeries_pci.c           |    4 
 arch/ppc64/kernel/pSeries_pci.c           |    4 
 arch/ppc64/kernel/pci.c                   |    4 
 arch/ppc64/kernel/pci_dma.c               |    4 
 arch/sh/kernel/pci-sh7751.c               |    8 
 arch/v850/Kconfig                         |    2 
 arch/v850/kernel/rte_mb_a_pci.c           |    4 
 arch/x86_64/Kconfig                       |    2 
 arch/x86_64/kernel/bluesmoke.c            |    4 
 arch/x86_64/kernel/pci-gart.c             |    3 
 arch/x86_64/pci/irq.c                     |   11 
 arch/x86_64/pci/x86-64.c                  |    8 
 drivers/Makefile                          |    1 
 drivers/acpi/pci_irq.c                    |    2 
 drivers/char/agp/amd-k8-agp.c             |    4 
 drivers/char/agp/generic.c                |    8 
 drivers/char/agp/isoch.c                  |    4 
 drivers/char/hw_random.c                  |    4 
 drivers/char/watchdog/amd7xx_tco.c        |    3 
 drivers/char/watchdog/i810-tco.c          |    4 
 drivers/hotplug/acpiphp_glue.c            |   26 
 drivers/hotplug/ibmphp_core.c             |   38 
 drivers/ide/pci/cs5530.c                  |   19 
 drivers/ide/pci/hpt366.c                  |    4 
 drivers/ide/pci/pdc202xx_new.c            |    4 
 drivers/ide/setup-pci.c                   |    4 
 drivers/macintosh/via-pmu.c               |   12 
 drivers/message/fusion/mptbase.c          |    7 
 drivers/message/i2o/i2o_core.c            |   12 
 drivers/net/e100/e100_main.c              |   47 
 drivers/net/e1000/e1000_main.c            |    2 
 drivers/net/ixgb/ixgb_main.c              |    2 
 drivers/parport/parport_pc.c              |    4 
 drivers/pci/Makefile                      |    3 
 drivers/pci/access.c                      |    3 
 drivers/pci/bus.c                         |    3 
 drivers/pci/hotplug.c                     |    8 
 drivers/pci/hotplug/Kconfig               |  120 +
 drivers/pci/hotplug/Makefile              |   44 
 drivers/pci/hotplug/acpiphp.h             |  262 ++
 drivers/pci/hotplug/acpiphp_core.c        |  505 ++++
 drivers/pci/hotplug/acpiphp_glue.c        | 1336 ++++++++++++
 drivers/pci/hotplug/acpiphp_pci.c         |  511 ++++
 drivers/pci/hotplug/acpiphp_res.c         |  699 ++++++
 drivers/pci/hotplug/cpci_hotplug.h        |  100 
 drivers/pci/hotplug/cpci_hotplug_core.c   |  919 ++++++++
 drivers/pci/hotplug/cpci_hotplug_pci.c    |  651 ++++++
 drivers/pci/hotplug/cpcihp_generic.c      |  290 ++
 drivers/pci/hotplug/cpcihp_zt5550.c       |  306 ++
 drivers/pci/hotplug/cpcihp_zt5550.h       |   79 
 drivers/pci/hotplug/cpqphp.h              |  912 ++++++++
 drivers/pci/hotplug/cpqphp_core.c         | 1541 ++++++++++++++
 drivers/pci/hotplug/cpqphp_ctrl.c         | 3084 ++++++++++++++++++++++++++++++
 drivers/pci/hotplug/cpqphp_nvram.c        |  667 ++++++
 drivers/pci/hotplug/cpqphp_nvram.h        |   57 
 drivers/pci/hotplug/cpqphp_pci.c          | 1549 +++++++++++++++
 drivers/pci/hotplug/cpqphp_sysfs.c        |  143 +
 drivers/pci/hotplug/ibmphp.h              |  772 +++++++
 drivers/pci/hotplug/ibmphp_core.c         | 1428 +++++++++++++
 drivers/pci/hotplug/ibmphp_ebda.c         | 1228 +++++++++++
 drivers/pci/hotplug/ibmphp_hpc.c          | 1228 +++++++++++
 drivers/pci/hotplug/ibmphp_pci.c          | 1758 +++++++++++++++++
 drivers/pci/hotplug/ibmphp_res.c          | 2157 ++++++++++++++++++++
 drivers/pci/hotplug/pci_hotplug.h         |  146 +
 drivers/pci/hotplug/pci_hotplug_core.c    |  666 ++++++
 drivers/pci/hotplug/pcihp_skeleton.c      |  432 ++++
 drivers/pci/pci-driver.c                  |   29 
 drivers/pci/pci.c                         |   16 
 drivers/pci/pci.h                         |   56 
 drivers/pci/pool.c                        |    2 
 drivers/pci/probe.c                       |    6 
 drivers/pci/proc.c                        |    5 
 drivers/pci/search.c                      |    4 
 drivers/pci/setup-irq.c                   |    4 
 drivers/pci/setup-res.c                   |    2 
 drivers/pnp/resource.c                    |    4 
 drivers/video/pm2fb.c                     |    4 
 drivers/video/sis/sis_main.c              |  105 -
 include/linux/pci.h                       |   75 
 sound/core/memalloc.c                     |   16 
 sound/oss/esssolo1.c                      |   16 
 sound/oss/maestro.c                       |    8 
 sound/oss/via82cxxx_audio.c               |   11 
 sound/pci/rme9652/hammerfall_mem.c        |   28 
 138 files changed, 23960 insertions(+), 24018 deletions(-)

-----

Greg Kroah-Hartman:
o PCI: remove usage of pci_for_each_dev() in arch/ppc64/kernel/pci.c
o PCI: finally remove pci_for_each_dev() now that all users of it are gone
o PCI: remove usage of pci_for_each_dev() in arch/x86_64/pci/x86-64.c
o PCI: remove usage of pci_for_each_dev() in arch/x86_64/pci/irq.c
o PCI: remove usage of pci_for_each_dev() in arch/x86_64/kernel/pci-gart.c
o PCI: remove usage of pci_for_each_dev() in arch/x86_64/kernel/bluesmoke.c
o PCI: remove usage of pci_for_each_dev() in arch/v850/kernel/rte_mb_a_pci.c
o PCI: remove usage of pci_for_each_dev() in arch/sh/kernel/pci-sh7751.c
o PCI: remove usage of pci_for_each_dev() in arch/ppc64/kernel/pci_dma.c
o PCI: remove usage of pci_for_each_dev() in arch/ppc64/kernel/pSeries_pci.c
o PCI: remove usage of pci_for_each_dev() in arch/ppc64/kernel/iSeries_pci.c
o PCI: remove usage of pci_for_each_dev() in arch/ppc/platforms/prep_pci.c
o PCI: remove usage of pci_for_each_dev() in arch/ppc/platforms/pmac_pci.c
o PCI: remove usage of pci_for_each_dev() in arch/ppc/platforms/gemini_pci.c
o PCI: remove usage of pci_for_each_dev() in arch/ppc/platforms/chrp_pci.c
o PCI: remove usage of pci_for_each_dev() in arch/ppc/kernel/pci.c
o PCI: remove usage of pci_for_each_dev() in arch/mips64/sgi-ip32/ip32-pci.c
o PCI: remove usage of pci_for_each_dev() in arch/mips64/mips-boards/generic/pci.c
o PCI: remove usage of pci_for_each_dev() in arch/mips/sni/pci.c
o PCI: remove usage of pci_for_each_dev() in arch/mips/mips-boards/generic/pci.c
o PCI: remove usage of pci_for_each_dev() in arch/mips/ddb5xxx/ddb5477/pci.c
o PCI: remove usage of pci_for_each_dev() in arch/mips/ddb5476/pci.c
o PCI: remove usage of pci_for_each_dev() in arch/mips/ddb5074/pci.c
o PCI: remove usage of pci_for_each_dev() in arch/ia64/sn/io/sn2/pci_bus_cvlink.c
o PCI: remove usage of pci_for_each_dev() in arch/ia64/sn/io/pciba.c
o PCI: remove usage of pci_for_each_dev() in arch/ia64/sn/io/pci_bus_cvlink.c
o PCI: remove usage of pci_for_each_dev() in arch/ia64/hp/common/sba_iommu.c
o PCI: remove usage of pci_for_each_dev() in arch/arm/kernel/bios32.c
o PCI: remove usage of pci_for_each_dev() in arch/alpha/kernel/sys_sio.c
o PCI: remove usage of pci_for_each_dev() in arch/i386/pci/i386.c
o PCI: remove usage of pci_for_each_dev() in arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
o PCI: remove usage of pci_for_each_dev() in arch/i386/pci/irq.c
o PCI: remove usage of pci_for_each_dev() in drivers/video/sis/sis_main.c
o PCI: remove usage of pci_for_each_dev() in drivers/video/pm2fb.c
o PCI: remove usage of pci_for_each_dev() in drivers/pnp/resource.c
o PCI: remove usage of pci_for_each_dev() in drivers/pci/setup-irq.c
o PCI: remove usage of pci_for_each_dev() in drivers/pci/search.c
o PCI: remove usage of pci_for_each_dev() in drivers/pci/proc.c
o PCI: remove usage of pci_for_each_dev() in drivers/pci/pci.c
o PCI: remove usage of pci_for_each_dev() in drivers/pci/hotplug/ibmphp_core.c
o PCI: remove usage of pci_for_each_dev() in drivers/parport/parport_pc.c
o PCI: remove usage of pci_for_each_dev() in drivers/net/ixgb/ixgb_main.c
o PCI: remove usage of pci_for_each_dev() in drivers/net/e1000/e1000_main.c
o PCI: remove usage of pci_for_each_dev() in drivers/net/e100/e100_main.c
o PCI: remove usage of pci_for_each_dev() in drivers/message/i2o/i2o_core.c
o PCI: remove usage of pci_for_each_dev() in drivers/message/fusion/mptbase.c
o PCI: remove usage of pci_for_each_dev() in drivers/macintosh/via-pmu.c
o PCI: remove usage of pci_for_each_dev() in drivers/ide/setup-pci.c
o PCI: remove usage of pci_for_each_dev() in drivers/ide/pci/pdc202xx_new.c
o PCI: remove usage of pci_for_each_dev() in drivers/ide/pci/hp2366.c
o PCI: remove usage of pci_for_each_dev() in drivers/ide/pci/cs5530.c
o PCI: remove usage of pci_for_each_dev() in drivers/char/watchdog/i810-tco.c
o PCI: remove usage of pci_for_each_dev() in drivers/char/watchdog/amd7xx_tco.c
o PCI: remove usage of pci_for_each_dev() in drivers/char/hw_random.c
o PCI: remove usage of pci_for_each_dev() in drivers/char/agp/isoch.c
o PCI: remove usage of pci_for_each_dev() in drivers/char/agp/generic.c
o PCI: remove usage of pci_for_each_dev() in drivers/char/agp/amd-k8-agp.c
o PCI: remove usage of pci_for_each_dev() in drivers/acpi/pci_irq.c
o PCI: remove usage of pci_for_each_dev() in sound/pci/rme9652/hammerfall_mem.c
o PCI: remove usage of pci_for_each_dev() in sound/oss/via82cxxx_audio.c
o PCI: remove usage of pci_for_each_dev() in sound/oss/maestro.c
o PCI: remove usage of pci_for_each_dev() in sound/oss/esssolo1.c
o PCI: remove usage of pci_for_each_dev() in sound/core/memalloc.c
o PCI: Grab reference count on pci_dev if the pci driver binds to the device
o PCI: Move more functions out of include/linux/pci.h that don't need to be there
o PCI: remove CONFIG_PROC_FS checks in .c files
o PCI: Remove a lot of PCI core only functions from include/linux/pci.h
o PCI Hotplug: move drivers/hotplug/* to drivers/pci/hotplug/*
o IBM PCI hotplug: remove direct access of pci_devices variable
o IBM PCI hotplug: remove hand made pci_find_bus function
o ACPI PCI Hotplug: remove hand made pci_find_bus function
o PCI: make pools_lock and pci_lock static
o PCI: make pci_setup_device(), pci_alloc_primary_bus() and pci_alloc_primary_bus_parented() static

Matt Domsch:
o dynids: free dynids on driver unload
o dynids: use list_add_tail

author
#1 lore raw

PATCH 1/1 PCI and PCI Hotplug changes and fixes for 2.5.70

ChangeSet 1.1229.25.1, 2003/05/28 15:54:26-05:00, [email protected]

dynids: use list_add_tail

instead of list_add, such that later entries come later in the scanned list.

 drivers/pci/pci-driver.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
drivers/pci/pci-driver.c +1 -1
diff -Nru a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
@@ -235,7 +235,7 @@
 		driver_data : 0UL;
 
 	spin_lock(&pdrv->dynids.lock);
-	list_add(&pdrv->dynids.list, &dynid->node);
+	list_add_tail(&pdrv->dynids.list, &dynid->node);
 	spin_unlock(&pdrv->dynids.lock);
 
 	bus = get_bus(pdrv->driver.bus);
author
#2 lore raw

ChangeSet 1.1229.25.2, 2003/05/28 16:37:35-05:00, [email protected]

dynids: free dynids on driver unload

 drivers/pci/pci-driver.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+)
drivers/pci/pci-driver.c +17 -0
diff -Nru a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
@@ -315,6 +315,22 @@
 	INIT_LIST_HEAD(&dynids->list);
 }
 
+static void
+pci_free_dynids(struct pci_driver *drv)
+{
+	struct list_head *pos, *n;
+	struct dynid *dynid;
+
+	spin_lock(&drv->dynids.lock);
+	list_for_each_safe(pos, n, &drv->dynids.list) {
+		dynid = list_entry(pos, struct dynid, node);
+		list_del(&dynid->node);
+		kfree(dynid);
+	}
+	spin_unlock(&drv->dynids.lock);
+}
+
+
 /**
  * pci_register_driver - register a new pci driver
  * @drv: the driver structure to register
@@ -363,6 +379,7 @@
 pci_unregister_driver(struct pci_driver *drv)
 {
 	driver_unregister(&drv->driver);
+	pci_free_dynids(drv);
 }
 
 static struct pci_driver pci_compat_driver = {
author
#3 lore raw

ChangeSet 1.1254.4.2, 2003/06/03 16:41:07-07:00, [email protected]

[PATCH] PCI: make pci_setup_device(), pci_alloc_primary_bus() and pci_alloc_primary_bus_parented() static

No one is calling these functions.

 drivers/pci/probe.c |    5 ++---
 include/linux/pci.h |    6 ------
 2 files changed, 2 insertions(+), 9 deletions(-)
drivers/pci/probe.c +2 -3
diff -Nru a/drivers/pci/probe.c b/drivers/pci/probe.c
@@ -396,7 +396,7 @@
  * Returns 0 on success and -1 if unknown type of device (not normal, bridge
  * or CardBus).
  */
-int pci_setup_device(struct pci_dev * dev)
+static int pci_setup_device(struct pci_dev * dev)
 {
 	u32 class;
 
@@ -641,7 +641,7 @@
 	return 0;
 }
 
-struct pci_bus * __devinit pci_alloc_primary_bus_parented(struct device *parent, int bus)
+static struct pci_bus * __devinit pci_alloc_primary_bus_parented(struct device *parent, int bus)
 {
 	struct pci_bus *b;
 
@@ -692,7 +692,6 @@
 EXPORT_SYMBOL(pci_root_buses);
 
 #ifdef CONFIG_HOTPLUG
-EXPORT_SYMBOL(pci_setup_device);
 EXPORT_SYMBOL(pci_add_new_bus);
 EXPORT_SYMBOL(pci_do_scan_bus);
 EXPORT_SYMBOL(pci_scan_slot);
include/linux/pci.h +0 -6
diff -Nru a/include/linux/pci.h b/include/linux/pci.h
@@ -546,11 +546,6 @@
 {
 	return pci_scan_bus_parented(NULL, bus, ops, sysdata);
 }
-struct pci_bus *pci_alloc_primary_bus_parented(struct device * parent, int bus);
-static inline struct pci_bus *pci_alloc_primary_bus(int bus)
-{
-	return pci_alloc_primary_bus_parented(NULL, bus);
-}
 int pci_scan_slot(struct pci_bus *bus, int devfn);
 void pci_bus_add_devices(struct pci_bus *bus);
 int pci_proc_attach_device(struct pci_dev *dev);
@@ -561,7 +556,6 @@
 char *pci_class_name(u32 class);
 void pci_read_bridge_bases(struct pci_bus *child);
 struct resource *pci_find_parent_resource(const struct pci_dev *dev, struct resource *res);
-int pci_setup_device(struct pci_dev *dev);
 int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
 extern struct pci_dev *pci_get_dev(struct pci_dev *dev);
 extern void pci_put_dev(struct pci_dev *dev);
author
#4 lore raw

ChangeSet 1.1254.4.3, 2003/06/03 16:41:35-07:00, [email protected]

[PATCH] PCI: make pools_lock and pci_lock static.

No one is using them outside the pci core.

 drivers/pci/access.c |    3 +--
 drivers/pci/pool.c   |    2 +-
 include/linux/pci.h  |    2 --
 3 files changed, 2 insertions(+), 5 deletions(-)
drivers/pci/access.c +1 -2
diff -Nru a/drivers/pci/access.c b/drivers/pci/access.c
@@ -7,7 +7,7 @@
  * configuration space.
  */
 
-spinlock_t pci_lock = SPIN_LOCK_UNLOCKED;
+static spinlock_t pci_lock = SPIN_LOCK_UNLOCKED;
 
 /*
  *  Wrappers for all PCI configuration access functions.  They just check
@@ -60,4 +60,3 @@
 EXPORT_SYMBOL(pci_bus_write_config_byte);
 EXPORT_SYMBOL(pci_bus_write_config_word);
 EXPORT_SYMBOL(pci_bus_write_config_dword);
-EXPORT_SYMBOL(pci_lock);
drivers/pci/pool.c +1 -1
diff -Nru a/drivers/pci/pool.c b/drivers/pci/pool.c
@@ -31,7 +31,7 @@
 #define	POOL_TIMEOUT_JIFFIES	((100 /* msec */ * HZ) / 1000)
 #define	POOL_POISON_BYTE	0xa7
 
-DECLARE_MUTEX (pools_lock);
+static DECLARE_MUTEX (pools_lock);
 
 static ssize_t
 show_pools (struct device *dev, char *buf)
include/linux/pci.h +0 -2
diff -Nru a/include/linux/pci.h b/include/linux/pci.h
@@ -606,8 +606,6 @@
 	return pci_bus_write_config_dword (dev->bus, dev->devfn, where, val);
 }
 
-extern spinlock_t pci_lock;
-
 int pci_enable_device(struct pci_dev *dev);
 int pci_enable_device_bars(struct pci_dev *dev, int mask);
 void pci_disable_device(struct pci_dev *dev);
author
#5 lore raw

ChangeSet 1.1254.4.4, 2003/06/03 17:08:13-07:00, [email protected]

[PATCH] ACPI PCI Hotplug: remove hand made pci_find_bus function

 drivers/hotplug/acpiphp_glue.c |   26 +-------------------------
 1 files changed, 1 insertion(+), 25 deletions(-)
drivers/hotplug/acpiphp_glue.c +1 -25
diff -Nru a/drivers/hotplug/acpiphp_glue.c b/drivers/hotplug/acpiphp_glue.c
@@ -281,30 +281,6 @@
 	return AE_OK;
 }
 
-
-/* find pci_bus structure associated to specific bus number */
-static struct pci_bus *find_pci_bus(const struct list_head *list, int bus)
-{
-	const struct list_head *l;
-
-	list_for_each (l, list) {
-		struct pci_bus *b = pci_bus_b(l);
-		if (b->number == bus)
-			return b;
-
-		if (!list_empty(&b->children)) {
-			/* XXX recursive call */
-			b = find_pci_bus(&b->children, bus);
-
-			if (b)
-				return b;
-		}
-	}
-
-	return NULL;
-}
-
-
 /* decode ACPI 2.0 _HPP hot plug parameters */
 static void decode_hpp(struct acpiphp_bridge *bridge)
 {
@@ -409,7 +385,7 @@
 	bridge->seg = seg;
 	bridge->bus = bus;
 
-	bridge->pci_bus = find_pci_bus(&pci_root_buses, bus);
+	bridge->pci_bus = pci_find_bus(bus);
 
 	bridge->res_lock = SPIN_LOCK_UNLOCKED;
 
author
#6 lore raw

ChangeSet 1.1254.4.5, 2003/06/03 17:08:26-07:00, [email protected]

[PATCH] IBM PCI hotplug: remove hand made pci_find_bus function.

 drivers/hotplug/ibmphp_core.c |   25 +++----------------------
 1 files changed, 3 insertions(+), 22 deletions(-)
drivers/hotplug/ibmphp_core.c +3 -22
diff -Nru a/drivers/hotplug/ibmphp_core.c b/drivers/hotplug/ibmphp_core.c
@@ -724,25 +724,6 @@
 	return NULL;
 }
 
-/* This routine is to find the pci_bus from kernel structures.
- * Parameters: bus number
- * Returns : pci_bus *  or NULL if not found
- */
-static struct pci_bus *ibmphp_find_bus (u8 busno)
-{
-	const struct list_head *tmp;
-	struct pci_bus *bus;
-	debug ("inside %s, busno = %x \n", __FUNCTION__, busno);
-
-	list_for_each (tmp, &pci_root_buses) {
-		bus = (struct pci_bus *) pci_bus_b (tmp);
-		if (bus)
-			if (bus->number == busno)
-				return bus;
-	}
-	return NULL;
-}
-
 /*************************************************************
  * This routine frees up memory used by struct slot, including
  * the pointers to pci_func, bus, hotplug_slot, controller,
@@ -810,7 +791,7 @@
 	struct pci_dev *dev;
 	u16 l;
 
-	if (ibmphp_find_bus (busno) || !(ibmphp_find_same_bus_num (busno)))
+	if (pci_find_bus(busno) || !(ibmphp_find_same_bus_num (busno)))
 		return 1;
 
 	bus = kmalloc (sizeof (*bus), GFP_KERNEL);
@@ -855,7 +836,7 @@
 		func->dev = pci_find_slot (func->busno, PCI_DEVFN(func->device, func->function));
 
 	if (func->dev == NULL) {
-		struct pci_bus *bus = ibmphp_find_bus (func->busno);
+		struct pci_bus *bus = pci_find_bus(func->busno);
 		if (!bus)
 			return 0;
 
@@ -1377,7 +1358,7 @@
 		goto exit;
 	}
 
-	bus = ibmphp_find_bus (0);
+	bus = pci_find_bus(0);
 	if (!bus) {
 		err ("Can't find the root pci bus, can not continue\n");
 		rc = -ENODEV;
author
#7 lore raw

ChangeSet 1.1254.4.6, 2003/06/03 19:36:54-07:00, [email protected]

[PATCH] IBM PCI hotplug: remove direct access of pci_devices variable.

 drivers/hotplug/ibmphp_core.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)
drivers/hotplug/ibmphp_core.c +5 -8
diff -Nru a/drivers/hotplug/ibmphp_core.c b/drivers/hotplug/ibmphp_core.c
@@ -897,7 +897,6 @@
 	int rc;
 	u8 speed;
 	u8 cmd = 0x0;
-	const struct list_head *tmp;
 	struct pci_dev * dev;
 	int retval;
 
@@ -945,13 +944,11 @@
 				cmd = HPC_BUS_100PCIXMODE;
 				break;
 			case BUS_SPEED_133:
-				/* This is to take care of the bug in CIOBX chip*/
-				list_for_each (tmp, &pci_devices) {
-					dev = (struct pci_dev *) pci_dev_g (tmp);
-					if (dev) {
-						if ((dev->vendor == 0x1166) && (dev->device == 0x0101))
-							ibmphp_hpc_writeslot (slot_cur, HPC_BUS_100PCIXMODE);
-					}
+				/* This is to take care of the bug in CIOBX chip */
+				pci_for_each_dev(dev) {
+					if ((dev->vendor == PCI_VENDOR_ID_SERVERWORKS) &&
+					    (dev->device == 0x0101))
+						ibmphp_hpc_writeslot (slot_cur, HPC_BUS_100PCIXMODE);
 				}
 				cmd = HPC_BUS_133PCIXMODE;
 				break;
author
#8 lore raw

ChangeSet 1.1254.4.8, 2003/06/03 22:48:45-07:00, [email protected]

[PATCH] PCI: Remove a lot of PCI core only functions from include/linux/pci.h

 drivers/pci/bus.c                      |    1 
 drivers/pci/hotplug/acpiphp_glue.c     |    1 
 drivers/pci/hotplug/acpiphp_pci.c      |    1 
 drivers/pci/hotplug/cpci_hotplug_pci.c |    1 
 drivers/pci/hotplug/cpqphp_pci.c       |    1 
 drivers/pci/hotplug/ibmphp_core.c      |    1 
 drivers/pci/pci.h                      |   44 ++++++++++++++++++++++++++++
 drivers/pci/setup-res.c                |    2 -
 include/linux/pci.h                    |   51 ---------------------------------
 9 files changed, 50 insertions(+), 53 deletions(-)
drivers/pci/bus.c +0 -1
diff -Nru a/drivers/pci/bus.c b/drivers/pci/bus.c
@@ -129,6 +129,5 @@
 	}
 }
 
-EXPORT_SYMBOL(pci_bus_alloc_resource);
 EXPORT_SYMBOL(pci_bus_add_devices);
 EXPORT_SYMBOL(pci_enable_bridges);
drivers/pci/hotplug/acpiphp_glue.c +1 -0
diff -Nru a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
@@ -34,6 +34,7 @@
 #include <linux/init.h>
 #include <asm/semaphore.h>
 
+#include "../pci.h"
 #include "pci_hotplug.h"
 #include "acpiphp.h"
 
drivers/pci/hotplug/acpiphp_pci.c +1 -0
diff -Nru a/drivers/pci/hotplug/acpiphp_pci.c b/drivers/pci/hotplug/acpiphp_pci.c
@@ -35,6 +35,7 @@
 #include <linux/pci.h>
 #include <linux/init.h>
 #include <linux/acpi.h>
+#include "../pci.h"
 #include "pci_hotplug.h"
 #include "acpiphp.h"
 
drivers/pci/hotplug/cpci_hotplug_pci.c +1 -0
diff -Nru a/drivers/pci/hotplug/cpci_hotplug_pci.c b/drivers/pci/hotplug/cpci_hotplug_pci.c
@@ -28,6 +28,7 @@
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/proc_fs.h>
+#include "../pci.h"
 #include "pci_hotplug.h"
 #include "cpci_hotplug.h"
 
drivers/pci/hotplug/cpqphp_pci.c +1 -0
diff -Nru a/drivers/pci/hotplug/cpqphp_pci.c b/drivers/pci/hotplug/cpqphp_pci.c
@@ -34,6 +34,7 @@
 #include <linux/workqueue.h>
 #include <linux/proc_fs.h>
 #include <linux/pci.h>
+#include "../pci.h"
 #include "cpqphp.h"
 #include "cpqphp_nvram.h"
 #include "../../../arch/i386/pci/pci.h"	/* horrible hack showing how processor dependent we are... */
drivers/pci/hotplug/ibmphp_core.c +1 -0
diff -Nru a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
@@ -35,6 +35,7 @@
 #include <linux/delay.h>
 #include <linux/wait.h>
 #include <linux/smp_lock.h>
+#include "../pci.h"
 #include "../../../arch/i386/pci/pci.h"	/* for struct irq_routing_table */
 #include "ibmphp.h"
 
drivers/pci/pci.h +44 -0

47 lines not on this page · raw message

drivers/pci/setup-res.c +1 -1
diff -Nru a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
@@ -23,7 +23,7 @@
 #include <linux/ioport.h>
 #include <linux/cache.h>
 #include <linux/slab.h>
-
+#include "pci.h"
 
 #define DEBUG_CONFIG 0
 #if DEBUG_CONFIG
include/linux/pci.h +0 -51

83 lines not on this page · raw message

author
#9 lore raw

ChangeSet 1.1254.4.9, 2003/06/03 23:33:47-07:00, [email protected]

[PATCH] PCI: remove CONFIG_PROC_FS checks in .c files.

 drivers/pci/bus.c                      |    2 --
 drivers/pci/hotplug.c                  |    8 --------
 drivers/pci/hotplug/cpci_hotplug_pci.c |    3 +--
 drivers/pci/pci.h                      |    8 ++++++++
 drivers/pci/proc.c                     |    1 -
 5 files changed, 9 insertions(+), 13 deletions(-)
drivers/pci/bus.c +0 -2
diff -Nru a/drivers/pci/bus.c b/drivers/pci/bus.c
@@ -94,9 +94,7 @@
 
 		device_add(&dev->dev);
 		list_add_tail(&dev->global_list, &pci_devices);
-#ifdef CONFIG_PROC_FS
 		pci_proc_attach_device(dev);
-#endif
 		pci_create_sysfs_dev_files(dev);
 
 	}
drivers/pci/hotplug/cpci_hotplug_pci.c +1 -2
diff -Nru a/drivers/pci/hotplug/cpci_hotplug_pci.c b/drivers/pci/hotplug/cpci_hotplug_pci.c
@@ -398,9 +398,8 @@
 	child = pci_find_bus(max + 1);
 	if (!child)
 		return -ENODEV;
-#ifdef CONFIG_PROC_FS
 	pci_proc_attach_bus(child);
-#endif
+
 	/*
 	 * Update parent bridge's subordinate field if there were more bridges
 	 * behind the bridge that was scanned.
drivers/pci/hotplug.c +0 -8
diff -Nru a/drivers/pci/hotplug.c b/drivers/pci/hotplug.c
@@ -190,9 +190,7 @@
 	list_del(&dev->bus_list);
 	list_del(&dev->global_list);
 	pci_free_resources(dev);
-#ifdef CONFIG_PROC_FS
 	pci_proc_detach_device(dev);
-#endif
 	return 0;
 }
 EXPORT_SYMBOL(pci_remove_device_safe);
@@ -237,10 +235,7 @@
 		struct pci_bus *b = dev->subordinate;
 
 		pci_remove_behind_bridge(dev);
-
-#ifdef CONFIG_PROC_FS
 		pci_proc_detach_bus(b);
-#endif
 
 		list_del(&b->node);
 		kfree(b);
@@ -251,10 +246,7 @@
 	list_del(&dev->bus_list);
 	list_del(&dev->global_list);
 	pci_free_resources(dev);
-#ifdef CONFIG_PROC_FS
 	pci_proc_detach_device(dev);
-#endif
-
 	pci_put_dev(dev);
 }
 
drivers/pci/pci.h +8 -0
diff -Nru a/drivers/pci/pci.h b/drivers/pci/pci.h
@@ -9,10 +9,18 @@
 				  void (*alignf)(void *, struct resource *,
 					  	 unsigned long, unsigned long),
 				  void *alignf_data);
+/* PCI /proc functions */
+#ifdef CONFIG_PROC_FS
 extern int pci_proc_attach_device(struct pci_dev *dev);
 extern int pci_proc_detach_device(struct pci_dev *dev);
 extern int pci_proc_attach_bus(struct pci_bus *bus);
 extern int pci_proc_detach_bus(struct pci_bus *bus);
+#else
+static inline int pci_proc_attach_device(struct pci_dev *dev) { return 0; }
+static inline int pci_proc_detach_device(struct pci_dev *dev) { return 0; }
+static inline int pci_proc_attach_bus(struct pci_bus *bus) { return 0; }
+static inline int pci_proc_detach_bus(struct pci_bus *bus) { return 0; }
+#endif
 
 /* Functions for PCI Hotplug drivers to use */
 extern struct pci_bus * pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev, int busnr);
drivers/pci/proc.c +0 -1
diff -Nru a/drivers/pci/proc.c b/drivers/pci/proc.c
@@ -599,7 +599,6 @@
 
 #ifdef CONFIG_HOTPLUG
 EXPORT_SYMBOL(pci_proc_attach_device);
-EXPORT_SYMBOL(pci_proc_detach_device);
 EXPORT_SYMBOL(pci_proc_attach_bus);
 EXPORT_SYMBOL(pci_proc_detach_bus);
 EXPORT_SYMBOL(proc_bus_pci_dir);
author
#10 lore raw

ChangeSet 1.1254.4.10, 2003/06/04 00:02:59-07:00, [email protected]

[PATCH] PCI: Move more functions out of include/linux/pci.h that don't need to be there.

 drivers/pci/pci.h   |    4 ++++
 include/linux/pci.h |    5 -----
 2 files changed, 4 insertions(+), 5 deletions(-)
drivers/pci/pci.h +4 -0
diff -Nru a/drivers/pci/pci.h b/drivers/pci/pci.h
@@ -27,6 +27,10 @@
 extern unsigned int pci_do_scan_bus(struct pci_bus *bus);
 extern void pci_remove_bus_device(struct pci_dev *dev);
 extern int pci_remove_device_safe(struct pci_dev *dev);
+extern unsigned char pci_max_busnr(void);
+extern unsigned char pci_bus_max_busnr(struct pci_bus *bus);
+extern int pci_bus_find_capability (struct pci_bus *bus, unsigned int devfn, int cap);
+extern struct pci_bus *pci_find_bus(unsigned char busnr);
 
 struct pci_dev_wrapped {
 	struct pci_dev	*dev;
include/linux/pci.h +0 -5
diff -Nru a/include/linux/pci.h b/include/linux/pci.h
@@ -563,12 +563,8 @@
 				 unsigned int ss_vendor, unsigned int ss_device,
 				 const struct pci_dev *from);
 struct pci_dev *pci_find_class (unsigned int class, const struct pci_dev *from);
-struct pci_bus *pci_find_bus(unsigned char busnr);
 struct pci_dev *pci_find_slot (unsigned int bus, unsigned int devfn);
-unsigned char pci_bus_max_busnr(struct pci_bus* bus);
-unsigned char pci_max_busnr(void);
 int pci_find_capability (struct pci_dev *dev, int cap);
-int pci_bus_find_capability (struct pci_bus *bus, unsigned int devfn, int cap);
 
 int pci_bus_read_config_byte (struct pci_bus *bus, unsigned int devfn, int where, u8 *val);
 int pci_bus_read_config_word (struct pci_bus *bus, unsigned int devfn, int where, u16 *val);
@@ -620,7 +616,6 @@
 int pci_enable_wake(struct pci_dev *dev, u32 state, int enable);
 
 /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
-
 void pci_bus_assign_resources(struct pci_bus *bus);
 void pci_bus_size_bridges(struct pci_bus *bus);
 int pci_claim_resource(struct pci_dev *, int);
author
#11 lore raw

ChangeSet 1.1254.4.11, 2003/06/04 10:29:42-07:00, [email protected]

[PATCH] PCI: Grab reference count on pci_dev if the pci driver binds to the device.

And remember to decrement the count after remove() is called.

 drivers/pci/pci-driver.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/pci/pci-driver.c +6 -4
diff -Nru a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
@@ -138,10 +138,11 @@
 
 	drv = to_pci_driver(dev->driver);
 	pci_dev = to_pci_dev(dev);
-	if (get_device(dev)) {
-		error = __pci_device_probe(drv, pci_dev);
-		put_device(dev);
-	}
+	pci_get_dev(pci_dev);
+	error = __pci_device_probe(drv, pci_dev);
+	if (error)
+		pci_put_dev(pci_dev);
+
 	return error;
 }
 
@@ -155,6 +156,7 @@
 			drv->remove(pci_dev);
 		pci_dev->driver = NULL;
 	}
+	pci_put_dev(pci_dev);
 	return 0;
 }
 
#12 lore raw
In reply to Greg KH

drivers/pci/bus.c line 133

Quoted 10 lines · Greg KH

diff -Nru a/drivers/pci/bus.c b/drivers/pci/bus.c

drivers/pci/bus.c +0 -1
@@ -129,6 +129,5 @@
 	}
 }
 
-EXPORT_SYMBOL(pci_bus_alloc_resource);
 EXPORT_SYMBOL(pci_bus_add_devices);
 EXPORT_SYMBOL(pci_enable_bridges);

Please don't remove this one. Its there for stuff like:

drivers/pcmcia/cardbus.c

struct resource *
cb_alloc_io(unsigned long base, unsigned long num,
unsigned long align, char *name, socket_info_t *s)
{
...

                ret = pci_bus_alloc_resource(bridge->bus, res, num, 1,
                                             min, 0, cardbus_adjust, &ca);
}

ie, so cardbus can decently allocate resources from the parent bus.

(Unfortunately there's a huge pile of other changes which the above cardbus code relies on, and I fear that its getting rather late to get them into 2.5.)

Signature · 2 lines
Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html
author
#13 lore raw
Quoted 8 lines · Russell King
Quoted 7 lines · Greg KH

diff -Nru a/drivers/pci/bus.c b/drivers/pci/bus.c

drivers/pci/bus.c +0 -0
@@ -129,6 +129,5 @@
 	}
 }
 
Quoted 7 lines

drivers/pci/bus.c line 133

Quoted 3 lines
drivers/pci/bus.c +0 -1
-EXPORT_SYMBOL(pci_bus_alloc_resource);
 EXPORT_SYMBOL(pci_bus_add_devices);
 EXPORT_SYMBOL(pci_enable_bridges);

Please don't remove this one. Its there for stuff like:

drivers/pcmcia/cardbus.c

Sorry, I don't see that in the current kernel version of cardbus.c, otherwise I would not have moved it out. Feel free to put it back, if you need it for any future cardbus changes.

Oh, and I was looking at cardbus.c when doing these changes and saw this old comment:

/*
* This file is going away. Cardbus handling has been re-written to be
* more of a PCI bridge thing, and the PCI code basically does all the
* resource handling. This has wrappers to make the rest of the PCMCIA
* subsystem not notice that it's not here any more.
*
* Linus, Jan 2000
*/

Is that ever going to happen? Just curious, because if so, I can remove some more functions from pci.h :)

thanks,

greg k-h

#14 lore raw
Quoted 15 lines · Greg KH
Quoted 14 lines · Russell King

drivers/pci/bus.c line 133

Quoted 10 lines · Greg KH

diff -Nru a/drivers/pci/bus.c b/drivers/pci/bus.c

drivers/pci/bus.c +0 -1
@@ -129,6 +129,5 @@
 	}
 }
 
-EXPORT_SYMBOL(pci_bus_alloc_resource);
 EXPORT_SYMBOL(pci_bus_add_devices);
 EXPORT_SYMBOL(pci_enable_bridges);

Please don't remove this one. Its there for stuff like:

Quoted 5 lines
Quoted 1 line

drivers/pcmcia/cardbus.c

Sorry, I don't see that in the current kernel version of cardbus.c, otherwise I would not have moved it out. Feel free to put it back, if you need it for any future cardbus changes.

Well, if people are going to remove stuff, I'm not going to be bothered to even submit the changes. I've got enough to do at the moment to go around putting stuff back into the kernel which I've already submitted.

Quoted 11 lines

/*
* This file is going away. Cardbus handling has been re-written to be
* more of a PCI bridge thing, and the PCI code basically does all the
* resource handling. This has wrappers to make the rest of the PCMCIA
* subsystem not notice that it's not here any more.
*
* Linus, Jan 2000
*/

Is that ever going to happen? Just curious, because if so, I can remove some more functions from pci.h :)

It doesn't look like it.

Signature · 2 lines
Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html
author
#15 lore raw
In reply to Greg KH
Quoted 3 lines · Greg KH

p.s. I'll send these as patches in response to this email to lkml for those who want to see them.

I don't think everyone really wants to see all 63 different pci_for_each_dev() removal patches and the huge drivers/hotplug/* move patch on the mailing lists, so I didn't post them here. But they are available, along with all of the patches in this series on your favoriate kernel.org mirror at:

	kernel.org/pub/linux/kernel/people/gregkh/pci/2.5/*2.5.70*

if people really want to take a look at them.

thanks,

greg k-h

#16 lore raw
Quoted 5 lines · Greg KH
Quoted 2 lines

p.s. I'll send these as patches in response to this email to lkml for those who want to see them.

I don't think everyone really wants to see all 63 different pci_for_each_dev() removal patches

I'm puzzled why you did..

- pci_for_each_dev(device)
+ while ((device = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, device)) != NULL)

when you could have just added whatever locking pci_find_device() does to pci_for_each_dev() You'd then not have had to touch any of these drivers, and it'd look a damn sight better to look at IMO.

Dave

author
#17 lore raw
Quoted 13 lines · Dave Jones
Quoted 5 lines · Greg KH
Quoted 2 lines

p.s. I'll send these as patches in response to this email to lkml for those who want to see them.

I don't think everyone really wants to see all 63 different pci_for_each_dev() removal patches

I'm puzzled why you did..

(quoted diff) +1 -1
-	pci_for_each_dev(device)
+	while ((device = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, device)) != NULL)
Quoted 3 lines

when you could have just added whatever locking pci_find_device() does to pci_for_each_dev() You'd then not have had to touch any of these drivers, and it'd look a damn sight better to look at IMO.

pci_for_each_dev() is currently a macro, not a function, and I'm trying to get rid of all public access to the pci lists. The majority of pci drivers use the pci_find_device() function in just the way that I converted the few remaining users of pci_for_each_dev() to (yeah, "few" is a relative number, but check out how many people call pci_find_device()...)

I guess I could create this to clean it up a bit:
#define pci_find_all_devices(dev) pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)

but that's really not that much of a change...

thanks,

greg k-h

#18 lore raw
Quoted 14 lines · Greg KH
Quoted 6 lines
(quoted diff) +1 -1
-	pci_for_each_dev(device)
+	while ((device = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, device)) != NULL)

when you could have just added whatever locking pci_find_device() does to pci_for_each_dev() You'd then not have had to touch any of these drivers, and it'd look a damn sight better to look at IMO.

pci_for_each_dev() is currently a macro, not a function, and I'm trying to get rid of all public access to the pci lists. The majority of pci drivers use the pci_find_device() function in just the way that I converted the few remaining users of pci_for_each_dev() to (yeah, "few" is a relative number, but check out how many people call pci_find_device()...)

Quoted 4 lines

I guess I could create this to clean it up a bit:
#define pci_find_all_devices(dev) pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)

but that's really not that much of a change...

so why not..

#define pci_for_each_dev(dev) \
	while ((device = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, device)) != NULL)

?

Seems to be the same change you made tree-wide, with minimal interruption to drivers.

Dave

author
#19 lore raw
Quoted 23 lines · Dave Jones
Quoted 14 lines · Greg KH
Quoted 6 lines
(quoted diff) +1 -1
-	pci_for_each_dev(device)
+	while ((device = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, device)) != NULL)

when you could have just added whatever locking pci_find_device() does to pci_for_each_dev() You'd then not have had to touch any of these drivers, and it'd look a damn sight better to look at IMO.

pci_for_each_dev() is currently a macro, not a function, and I'm trying to get rid of all public access to the pci lists. The majority of pci drivers use the pci_find_device() function in just the way that I converted the few remaining users of pci_for_each_dev() to (yeah, "few" is a relative number, but check out how many people call pci_find_device()...)

Quoted 4 lines

I guess I could create this to clean it up a bit:
#define pci_find_all_devices(dev) pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)

but that's really not that much of a change...

so why not..

Quoted 7 lines
#define pci_for_each_dev(dev) \
	while ((device = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, device)) != NULL)

?

Seems to be the same change you made tree-wide, with minimal interruption to drivers.

But that would have changed the way that pci_for_each_dev() works. It would require that dev=NULL before the function is called. And having the same function work subtly different on different kernel versions would not be the best thing. Getting rid of it entirely was the better option, and now that Linus has pulled it, we don't have to worry about it anymore :)

thanks,

greg k-h

#20 lore raw
Quoted 5 lines · Greg KH
Quoted 5 lines

so why not..

#define pci_for_each_dev(dev) \
	while ((device = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, device)) != NULL)
Quoted 7 lines
Quoted 4 lines

?

Seems to be the same change you made tree-wide, with minimal interruption to drivers.

But that would have changed the way that pci_for_each_dev() works. It would require that dev=NULL before the function is called.

trivial to fix.

Quoted 3 lines

And having
the same function work subtly different on different kernel versions would not be the best thing.

ditto.

Quoted 3 lines

Getting rid of it entirely was the better
option, and now that Linus has pulled it, we don't have to worry about it anymore :)

The fact that a tree-wide 'cleanup' like this goes in just a few hours after its posted before chance to comment is another argument, but concentrating on the technical point here, I still think this is a step backwards.

Dave

#21 lore raw
Quoted 8 lines · Dave Jones
Quoted 3 lines

Getting rid of it entirely was the better
option, and now that Linus has pulled it, we don't have to worry about it anymore :)

The fact that a tree-wide 'cleanup' like this goes in just a few hours after its posted before chance to comment is another argument, but concentrating on the technical point here, I still think this is a step backwards.

Indeed; especially when some people who need to comment aren't on US time.

Signature · 2 lines
Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html
author
#22 lore raw
In reply to Dave Jones
Quoted 5 lines · Dave Jones

The fact that a tree-wide 'cleanup' like this goes in just a few hours after its posted before chance to comment is another argument, but concentrating on the technical point here, I still think this is a step backwards.

Why? I just got rid of a function (well macro) that isn't even needed (as proven by replacing it with an existing function.) That's technically a good thing :)

Now I can agree that some of those replacements could be done with a different function call, as almost none of the replacements in the driver/* tree really want to walk all of the pci devices in the tree. They usually just want to walk all devices of a type of pci device (be it capability, or other trait.) I'd be glad to take changes of this sort in the future.

thanks,

greg k-h

#23 lore raw
Quoted 8 lines · Greg KH
Quoted 4 lines

The fact that a tree-wide 'cleanup' like this goes in just a few hours after its posted before chance to comment is another argument, but concentrating on the technical point here, I still think this is a step backwards.

Why? I just got rid of a function (well macro) that isn't even needed (as proven by replacing it with an existing function.) That's technically a good thing :)

Not when that replacement reduces readability, which in the case of agpgart is all I care about wrt these changes.

Quoted 6 lines

Now I can agree that some of those replacements could be done with a different function call, as almost none of the replacements in the driver/* tree really want to walk all of the pci devices in the tree. They usually just want to walk all devices of a type of pci device (be it capability, or other trait.) I'd be glad to take changes of this sort in the future.

Ok, for the ones I'm interested in, (agpgart), I don't see how things can get much cleaner than they used to be.

07 - hammer. Needs to walk the whole list, matching pci devices on
bus 0, func 3, slots >23 & <32
This set of rules is so specialised, I see it hard to concieve how a generic helper function for the pci layer could be written.
08 - generic. Needs to know about every AGP device on the bus.
ok, a for_each_agp_dev may actually make life easier here,
but as its the only place this happens, consider it inlined, using pci_for_each_dev
09 - isoch. Could also use a 'for_each_agp_dev', but to be honest,
it shouldn't be scanning at all, but being passed what it needs.

For the time being, I'm actually tempted to hack up a 'for_each_agp_dev' for the latter two, but 07 still bugs me.

Dave