Re: Buggy PCI drivers - do not mark pci_device_id as discardabledata

[Matt Domsch]

> > These either need to be marked __devinit and make "new_id" dependant on
> > CONFIG_HOTPLUG

Patch below moves all the new_id code under CONFIG_HOTPLUG.  Tested
with both CONFIG_HOTPLUG enabled and disabled.  No significant code
changes, merely code moving, and in 2 cases, stub functions added.

Please review and apply.

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

--- linux-2.5/drivers/pci/pci-driver.c	Thu Sep  4 16:36:45 2003
+++ linux-2.5-devinit/drivers/pci/pci-driver.c	Wed Sep 10 10:18:55 2003
@@ -19,7 +19,7 @@
  *                        PCI device id structure
  * @id: single PCI device id structure to match
  * @dev: the PCI device structure to match against
- * 
+ *
  * Returns the matching pci_device_id structure or %NULL if there is no match.
  */
 
@@ -35,6 +35,166 @@ pci_match_one_device(const struct pci_de
 	return NULL;
 }
 
+/*
+ * Dynamic device IDs are disabled for !CONFIG_HOTPLUG
+ */
+
+#ifdef CONFIG_HOTPLUG
+/**
+ * pci_device_probe_dynamic()
+ *
+ * Walk the dynamic ID list looking for a match.
+ * returns 0 and sets pci_dev->driver when drv claims pci_dev, else error.
+ */
+static int
+pci_device_probe_dynamic(struct pci_driver *drv, struct pci_dev *pci_dev)
+{
+	int error = -ENODEV;
+	struct list_head *pos;
+	struct dynid *dynid;
+
+	spin_lock(&drv->dynids.lock);
+	list_for_each(pos, &drv->dynids.list) {
+		dynid = list_entry(pos, struct dynid, node);
+		if (pci_match_one_device(&dynid->id, pci_dev)) {
+			spin_unlock(&drv->dynids.lock);
+			error = drv->probe(pci_dev, &dynid->id);
+			if (error >= 0) {
+				pci_dev->driver = drv;
+				return 0;
+			}
+			return error;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+	return error;
+}
+static inline void
+dynid_init(struct dynid *dynid)
+{
+	memset(dynid, 0, sizeof(*dynid));
+	INIT_LIST_HEAD(&dynid->node);
+}
+
+/**
+ * store_new_id
+ * @ pdrv
+ * @ buf
+ * @ count
+ *
+ * Adds a new dynamic pci device ID to this driver,
+ * and causes the driver to probe for all devices again.
+ */
+static inline ssize_t
+store_new_id(struct device_driver * driver, const char * buf, size_t count)
+{
+	struct dynid *dynid;
+	struct bus_type * bus;
+	struct pci_driver *pdrv = to_pci_driver(driver);
+	__u32 vendor=PCI_ANY_ID, device=PCI_ANY_ID, subvendor=PCI_ANY_ID,
+		subdevice=PCI_ANY_ID, class=0, class_mask=0;
+	unsigned long driver_data=0;
+	int fields=0;
+
+	fields = sscanf(buf, "%x %x %x %x %x %x %lux",
+			&vendor, &device, &subvendor, &subdevice,
+			&class, &class_mask, &driver_data);
+	if (fields < 0)
+		return -EINVAL;
+
+	dynid = kmalloc(sizeof(*dynid), GFP_KERNEL);
+	if (!dynid)
+		return -ENOMEM;
+	dynid_init(dynid);
+
+	dynid->id.vendor = vendor;
+	dynid->id.device = device;
+	dynid->id.subvendor = subvendor;
+	dynid->id.subdevice = subdevice;
+	dynid->id.class = class;
+	dynid->id.class_mask = class_mask;
+	dynid->id.driver_data = pdrv->dynids.use_driver_data ?
+		driver_data : 0UL;
+
+	spin_lock(&pdrv->dynids.lock);
+	list_add_tail(&pdrv->dynids.list, &dynid->node);
+	spin_unlock(&pdrv->dynids.lock);
+
+	bus = get_bus(pdrv->driver.bus);
+	if (bus) {
+		if (get_driver(&pdrv->driver)) {
+			down_write(&bus->subsys.rwsem);
+			driver_attach(&pdrv->driver);
+			up_write(&bus->subsys.rwsem);
+			put_driver(&pdrv->driver);
+		}
+		put_bus(bus);
+	}
+
+	return count;
+}
+
+static DRIVER_ATTR(new_id, S_IWUSR, NULL, store_new_id);
+static inline void
+pci_init_dynids(struct pci_dynids *dynids)
+{
+	memset(dynids, 0, sizeof(*dynids));
+	spin_lock_init(&dynids->lock);
+	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);
+}
+
+static int
+pci_create_newid_file(struct pci_driver * drv)
+{
+	int error = 0;
+	if (drv->probe != NULL)
+		error = sysfs_create_file(&drv->driver.kobj,
+					  &driver_attr_new_id.attr);
+	return error;
+}
+
+static int
+pci_bus_match_dynids(const struct pci_dev * pci_dev, const struct pci_driver * pci_drv)
+{
+	struct list_head *pos;
+	struct dynid *dynid;
+
+	spin_lock(&pci_drv->dynids.lock);
+	list_for_each(pos, &pci_drv->dynids.list) {
+		dynid = list_entry(pos, struct dynid, node);
+		if (pci_match_one_device(&dynid->id, pci_dev)) {
+			spin_unlock(&pci_drv->dynids.lock);
+			return 1;
+		}
+	}
+	spin_unlock(&pci_drv->dynids.lock);
+	return 0;
+}
+
+#else /* !CONFIG_HOTPLUG */
+#define pci_device_probe_dynamic(drv,pci_dev) (-ENODEV)
+#define dynid_init(dynid) do {} while (0)
+#define pci_init_dynids(dynids) do {} while (0)
+#define pci_free_dynids(drv) do {} while (0)
+#define pci_create_newid_file(drv) (0)
+#define pci_bus_match_dynids(pci_dev, pci_drv) (0)
+#endif
+
 /**
  * pci_match_device - Tell if a PCI device structure has a matching
  *                    PCI device id structure
@@ -80,36 +240,6 @@ pci_device_probe_static(struct pci_drive
 }
 
 /**
- * pci_device_probe_dynamic()
- * 
- * Walk the dynamic ID list looking for a match.
- * returns 0 and sets pci_dev->driver when drv claims pci_dev, else error.
- */
-static int
-pci_device_probe_dynamic(struct pci_driver *drv, struct pci_dev *pci_dev)
-{		   
-	int error = -ENODEV;
-	struct list_head *pos;
-	struct dynid *dynid;
-
-	spin_lock(&drv->dynids.lock);
-	list_for_each(pos, &drv->dynids.list) {
-		dynid = list_entry(pos, struct dynid, node);
-		if (pci_match_one_device(&dynid->id, pci_dev)) {
-			spin_unlock(&drv->dynids.lock);
-			error = drv->probe(pci_dev, &dynid->id);
-			if (error >= 0) {
-				pci_dev->driver = drv;
-				return 0;
-			}
-			return error;
-		}
-	}
-	spin_unlock(&drv->dynids.lock);
-	return error;
-}
-
-/**
  * __pci_device_probe()
  * 
  * returns 0  on success, else error.
@@ -178,72 +308,6 @@ static int pci_device_resume(struct devi
 	return 0;
 }
 
-static inline void
-dynid_init(struct dynid *dynid)
-{
-	memset(dynid, 0, sizeof(*dynid));
-	INIT_LIST_HEAD(&dynid->node);
-}
-
-/**
- * store_new_id
- * @ pdrv
- * @ buf
- * @ count
- *
- * Adds a new dynamic pci device ID to this driver,
- * and causes the driver to probe for all devices again.
- */
-static inline ssize_t
-store_new_id(struct device_driver * driver, const char * buf, size_t count)
-{
-	struct dynid *dynid;
-	struct bus_type * bus;
-	struct pci_driver *pdrv = to_pci_driver(driver);
-	__u32 vendor=PCI_ANY_ID, device=PCI_ANY_ID, subvendor=PCI_ANY_ID,
-		subdevice=PCI_ANY_ID, class=0, class_mask=0;
-	unsigned long driver_data=0;
-	int fields=0;
-
-	fields = sscanf(buf, "%x %x %x %x %x %x %lux",
-			&vendor, &device, &subvendor, &subdevice,
-			&class, &class_mask, &driver_data);
-	if (fields < 0)
-		return -EINVAL;
-
-	dynid = kmalloc(sizeof(*dynid), GFP_KERNEL);
-	if (!dynid)
-		return -ENOMEM;
-	dynid_init(dynid);
-
-	dynid->id.vendor = vendor;
-	dynid->id.device = device;
-	dynid->id.subvendor = subvendor;
-	dynid->id.subdevice = subdevice;
-	dynid->id.class = class;
-	dynid->id.class_mask = class_mask;
-	dynid->id.driver_data = pdrv->dynids.use_driver_data ?
-		driver_data : 0UL;
-
-	spin_lock(&pdrv->dynids.lock);
-	list_add_tail(&pdrv->dynids.list, &dynid->node);
-	spin_unlock(&pdrv->dynids.lock);
-
-	bus = get_bus(pdrv->driver.bus);
-	if (bus) {
-		if (get_driver(&pdrv->driver)) {
-			down_write(&bus->subsys.rwsem);
-			driver_attach(&pdrv->driver);
-			up_write(&bus->subsys.rwsem);
-			put_driver(&pdrv->driver);
-		}
-		put_bus(bus);
-	}
-	
-	return count;
-}
-
-static DRIVER_ATTR(new_id, S_IWUSR, NULL, store_new_id);
 
 #define kobj_to_pci_driver(obj) container_of(obj, struct device_driver, kobj)
 #define attr_to_driver_attribute(obj) container_of(obj, struct driver_attribute, attr)
@@ -290,38 +354,9 @@ static struct kobj_type pci_driver_kobj_
 static int
 pci_populate_driver_dir(struct pci_driver * drv)
 {
-	int error = 0;
-
-	if (drv->probe != NULL)
-		error = sysfs_create_file(&drv->driver.kobj,
-					  &driver_attr_new_id.attr);
-	return error;
-}
-
-static inline void
-pci_init_dynids(struct pci_dynids *dynids)
-{
-	memset(dynids, 0, sizeof(*dynids));
-	spin_lock_init(&dynids->lock);
-	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);
+	return pci_create_newid_file(drv);
 }
 
-
 /**
  * pci_register_driver - register a new pci driver
  * @drv: the driver structure to register
@@ -411,8 +446,6 @@ static int pci_bus_match(struct device *
 	struct pci_driver * pci_drv = to_pci_driver(drv);
 	const struct pci_device_id * ids = pci_drv->id_table;
 	const struct pci_device_id *found_id;
-	struct list_head *pos;
-	struct dynid *dynid;
 
 	if (!ids)
 		return 0;
@@ -421,17 +454,7 @@ static int pci_bus_match(struct device *
 	if (found_id)
 		return 1;
 
-	spin_lock(&pci_drv->dynids.lock);
-	list_for_each(pos, &pci_drv->dynids.list) {
-		dynid = list_entry(pos, struct dynid, node);
-		if (pci_match_one_device(&dynid->id, pci_dev)) {
-			spin_unlock(&pci_drv->dynids.lock);
-			return 1;
-		}
-	}
-	spin_unlock(&pci_drv->dynids.lock);
-
-	return 0;
+	return pci_bus_match_dynids(pci_dev, pci_drv);
 }
 
 /**

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/