> I like this patch a _lot_ better, nice job.
Thanks.
> Only one comment:
> > + if (error < 0)
> > + return error;
> > + return count;
> > +
> > +
> > + return count;
> > +}
>
> Oops, lost the tabs at the end of the function :)
Duh. Good eyes. Fixed.
> This function will not link up a device to a driver properly within
> the driver core, only with the pci code. So if you do this, the
> driver core still thinks you have a device that is unbound, right?
> Also, the symlinks don't get created from the bus to the device I
> think, correct?
I think you're right.
> Unfortunatly, looking at the driver core real quickly, I don't see a
> simple way to kick the probe cycle off again for all pci devices, but
> I'm probably just missing something somewhere...
I think drivers/base/bus.c: driver_attach() is what we want, which will
walk the list of the bus's devices and run bus_match() which is
pci_bus_match() which will scan for us. Just need to un-static
driver_attach() I expect. Pat, does this sound right?
If that works, probe_each_pci_dev() can go away. I'll play with it some
more.
Thanks,
Matt
--
Matt Domsch
Sr. Software Engineer, Lead Engineer, Architect
Dell Linux Solutions http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com
On Mon, May 05, 2003 at 09:04:35PM -0500, [email protected] wrote:
> > Unfortunatly, looking at the driver core real quickly, I don't see a
> > simple way to kick the probe cycle off again for all pci devices, but
> > I'm probably just missing something somewhere...
>
> I think drivers/base/bus.c: driver_attach() is what we want, which will
> walk the list of the bus's devices and run bus_match() which is
> pci_bus_match() which will scan for us. Just need to un-static
> driver_attach() I expect. Pat, does this sound right?
You can't just call driver_attach(), as the bus semaphore needs to be
locked before doing so. In short, you almost need to duplicate
bus_add_driver(), but not quite :)
Good luck,
greg k-h
> You can't just call driver_attach(), as the bus semaphore needs to be
> locked before doing so.? In short, you almost need to duplicate
> bus_add_driver(), but not quite :)
Right, and it seems to work. I made driver_attach non-static, declared
it extern in pci.h, and call it in pci-driver.c while holding the bus
semaphore and references to the driver and the bus. This also let me
delete my probe_each_pci_dev() function and let the driver core
handle it.
Pat, can you ack the changes to bus.c and device.h please?
Now when an ID is added and a new device is found, the symlink for the
device shows up under sys/bus/pci/drivers/{driver}/.
Before:
/sys/bus/pci/drivers/e100
`-- new_id
After:
/sys/bus/pci/drivers/e100
|-- 00:04.0 -> ../../../../devices/pci0/00:04.0
`-- new_id
Patch below, and at:
http://domsch.com/linux/patches/dynids/linux-2.5/linux-2.5.69-dynids-20030506.patch.bz2
http://domsch.com/linux/patches/dynids/linux-2.5/linux-2.5.69-dynids-20030506.patch.bz2.asc
Documentation/pci.txt | 24 +++
drivers/base/bus.c | 2
drivers/pci/pci-driver.c | 281 ++++++++++++++++++++++++++++++++++++++-----
include/linux/device.h | 1
include/linux/pci-dynids.h | 18 ++
include/linux/pci.h | 16 ++
6 files changed, 312 insertions(+), 30 deletions(-)
BK:
http://mdomsch.bkbits.net/linux-2.5-dynids
Thanks,
Matt
--
Matt Domsch
Sr. Software Engineer
Dell Linux Solutions http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-2.5/Documentation/pci.txt dynids/linux-2.5-dynids/Documentation/pci.txt
--- linux-2.5/Documentation/pci.txt Mon May 5 17:19:05 2003
+++ dynids/linux-2.5-dynids/Documentation/pci.txt Tue May 6 11:16:08 2003
@@ -45,8 +45,6 @@
id_table Pointer to table of device ID's the driver is
interested in. Most drivers should export this
table using MODULE_DEVICE_TABLE(pci,...).
- Set to NULL to call probe() function for every
- PCI device known to the system.
probe Pointer to a probing function which gets called (during
execution of pci_register_driver for already existing
devices or later if a new device gets inserted) for all
@@ -82,6 +80,28 @@
class, Device class to match. The class_mask tells which bits
class_mask of the class are honored during the comparison.
driver_data Data private to the driver.
+
+Most drivers don't need to use the driver_data field. Best practice
+for use of driver_data is to use it as an index into a static list of
+equivalant device types, not to use it as a pointer.
+
+Have a table entry {PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID}
+to have probe() called for every PCI device known to the system.
+
+New PCI IDs may be added to a device driver at runtime by writing
+to the file /sys/bus/pci/drivers/{driver}/new_id. When added, the
+driver will probe for all devices it can support.
+
+echo "vendor device subvendor subdevice class class_mask driver_data" > \
+ /sys/bus/pci/drivers/{driver}/new_id
+where all fields are passed in as hexadecimal values (no leading 0x).
+Users need pass only as many fields as necessary; vendor, device,
+subvendor, and subdevice fields default to PCI_ANY_ID (FFFFFFFF),
+class and classmask fields default to 0, and driver_data defaults to
+0UL. Device drivers must call
+ pci_dynids_set_use_driver_data(pci_driver *, 1)
+in order for the driver_data field to get passed to the driver.
+Otherwise, only a 0 is passed in that field.
When the driver exits, it just calls pci_unregister_driver() and the PCI layer
automatically calls the remove hook for all devices handled by the driver.
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-2.5/drivers/base/bus.c dynids/linux-2.5-dynids/drivers/base/bus.c
--- linux-2.5/drivers/base/bus.c Mon May 5 17:19:17 2003
+++ dynids/linux-2.5-dynids/drivers/base/bus.c Tue May 6 11:16:18 2003
@@ -316,7 +316,7 @@
* Note that we ignore the error from bus_match(), since it's perfectly
* valid for a driver not to bind to any devices.
*/
-static void driver_attach(struct device_driver * drv)
+void driver_attach(struct device_driver * drv)
{
struct bus_type * bus = drv->bus;
struct list_head * entry;
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-2.5/drivers/pci/pci-driver.c dynids/linux-2.5-dynids/drivers/pci/pci-driver.c
--- linux-2.5/drivers/pci/pci-driver.c Mon May 5 17:19:31 2003
+++ dynids/linux-2.5-dynids/drivers/pci/pci-driver.c Tue May 6 11:16:29 2003
@@ -6,6 +6,8 @@
#include <linux/pci.h>
#include <linux/module.h>
#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/pci-dynids.h>
#include "pci.h"
/*
@@ -13,7 +15,29 @@
*/
/**
- * pci_match_device - Tell if a PCI device structure has a matching PCI device id structure
+ * pci_match_one_device - Tell if a PCI device structure has a matching
+ * 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.
+ */
+
+static inline const struct pci_device_id *
+pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
+{
+ if ((id->vendor == PCI_ANY_ID || id->vendor == dev->vendor) &&
+ (id->device == PCI_ANY_ID || id->device == dev->device) &&
+ (id->subvendor == PCI_ANY_ID || id->subvendor == dev->subsystem_vendor) &&
+ (id->subdevice == PCI_ANY_ID || id->subdevice == dev->subsystem_device) &&
+ !((id->class ^ dev->class) & id->class_mask))
+ return id;
+ return NULL;
+}
+
+/**
+ * pci_match_device - Tell if a PCI device structure has a matching
+ * PCI device id structure
* @ids: array of PCI device id structures to search in
* @dev: the PCI device structure to match against
*
@@ -25,17 +49,87 @@
pci_match_device(const struct pci_device_id *ids, const struct pci_dev *dev)
{
while (ids->vendor || ids->subvendor || ids->class_mask) {
- if ((ids->vendor == PCI_ANY_ID || ids->vendor == dev->vendor) &&
- (ids->device == PCI_ANY_ID || ids->device == dev->device) &&
- (ids->subvendor == PCI_ANY_ID || ids->subvendor == dev->subsystem_vendor) &&
- (ids->subdevice == PCI_ANY_ID || ids->subdevice == dev->subsystem_device) &&
- !((ids->class ^ dev->class) & ids->class_mask))
+ if (pci_match_one_device(ids, dev))
return ids;
ids++;
}
return NULL;
}
+/**
+ * pci_device_probe_static()
+ *
+ * returns 0 and sets pci_dev->driver when drv claims pci_dev, else error.
+ */
+static int
+pci_device_probe_static(struct pci_driver *drv, struct pci_dev *pci_dev)
+{
+ int error = -ENODEV;
+ const struct pci_device_id *id;
+
+ if (!drv->id_table)
+ return error;
+ id = pci_match_device(drv->id_table, pci_dev);
+ if (id)
+ error = drv->probe(pci_dev, id);
+ if (error >= 0) {
+ pci_dev->driver = drv;
+ return 0;
+ }
+ return error;
+}
+
+/**
+ * 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.
+ * side-effect: pci_dev->driver is set to drv when drv claims pci_dev.
+ */
+static int
+__pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
+{
+ int error = 0;
+
+ if (!pci_dev->driver && drv->probe) {
+ error = pci_device_probe_static(drv, pci_dev);
+ if (error >= 0)
+ return error;
+
+ error = pci_device_probe_dynamic(drv, pci_dev);
+ }
+ return error;
+}
+
static int pci_device_probe(struct device * dev)
{
int error = 0;
@@ -44,17 +138,9 @@
drv = to_pci_driver(dev->driver);
pci_dev = to_pci_dev(dev);
-
- if (!pci_dev->driver && drv->probe) {
- const struct pci_device_id *id;
-
- id = pci_match_device(drv->id_table, pci_dev);
- if (id)
- error = drv->probe(pci_dev, id);
- if (error >= 0) {
- pci_dev->driver = drv;
- error = 0;
- }
+ if (get_device(dev)) {
+ error = __pci_device_probe(drv, pci_dev);
+ put_device(dev);
}
return error;
}
@@ -101,6 +187,134 @@
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(&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)
+
+static ssize_t
+pci_driver_attr_show(struct kobject * kobj, struct attribute *attr, char *buf)
+{
+ struct device_driver *driver = kobj_to_pci_driver(kobj);
+ struct driver_attribute *dattr = attr_to_driver_attribute(attr);
+ ssize_t ret = 0;
+
+ if (get_driver(driver)) {
+ if (dattr->show)
+ ret = dattr->show(driver, buf);
+ put_driver(driver);
+ }
+ return ret;
+}
+
+static ssize_t
+pci_driver_attr_store(struct kobject * kobj, struct attribute *attr,
+ const char *buf, size_t count)
+{
+ struct device_driver *driver = kobj_to_pci_driver(kobj);
+ struct driver_attribute *dattr = attr_to_driver_attribute(attr);
+ ssize_t ret = 0;
+
+ if (get_driver(driver)) {
+ if (dattr->store)
+ ret = dattr->store(driver, buf, count);
+ put_driver(driver);
+ }
+ return ret;
+}
+
+static struct sysfs_ops pci_driver_sysfs_ops = {
+ .show = pci_driver_attr_show,
+ .store = pci_driver_attr_store,
+};
+static struct kobj_type pci_driver_kobj_type = {
+ .sysfs_ops = &pci_driver_sysfs_ops,
+};
+
+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);
+}
+
/**
* pci_register_driver - register a new pci driver
* @drv: the driver structure to register
@@ -122,9 +336,16 @@
drv->driver.resume = pci_device_resume;
drv->driver.suspend = pci_device_suspend;
drv->driver.remove = pci_device_remove;
+ drv->driver.kobj.ktype = &pci_driver_kobj_type;
+ pci_init_dynids(&drv->dynids);
/* register with core */
count = driver_register(&drv->driver);
+
+ if (count >= 0) {
+ pci_populate_driver_dir(drv);
+ }
+
return count ? count : 1;
}
@@ -180,22 +401,30 @@
*/
static int pci_bus_match(struct device * dev, struct device_driver * drv)
{
- struct pci_dev * pci_dev = to_pci_dev(dev);
+ const struct pci_dev * pci_dev = to_pci_dev(dev);
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)
+ if (!ids)
return 0;
- while (ids->vendor || ids->subvendor || ids->class_mask) {
- if ((ids->vendor == PCI_ANY_ID || ids->vendor == pci_dev->vendor) &&
- (ids->device == PCI_ANY_ID || ids->device == pci_dev->device) &&
- (ids->subvendor == PCI_ANY_ID || ids->subvendor == pci_dev->subsystem_vendor) &&
- (ids->subdevice == PCI_ANY_ID || ids->subdevice == pci_dev->subsystem_device) &&
- !((ids->class ^ pci_dev->class) & ids->class_mask))
+ found_id = pci_match_device(ids, pci_dev);
+ 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;
- ids++;
+ }
}
+ spin_unlock(&pci_drv->dynids.lock);
+
return 0;
}
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-2.5/include/linux/device.h dynids/linux-2.5-dynids/include/linux/device.h
--- linux-2.5/include/linux/device.h Mon May 5 17:19:50 2003
+++ dynids/linux-2.5-dynids/include/linux/device.h Tue May 6 11:16:45 2003
@@ -318,6 +318,7 @@
*/
extern void device_bind_driver(struct device * dev);
extern void device_release_driver(struct device * dev);
+extern void driver_attach(struct device_driver * drv);
/* driverfs interface for exporting device attributes */
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-2.5/include/linux/pci-dynids.h dynids/linux-2.5-dynids/include/linux/pci-dynids.h
--- linux-2.5/include/linux/pci-dynids.h Wed Dec 31 18:00:00 1969
+++ dynids/linux-2.5-dynids/include/linux/pci-dynids.h Tue May 6 11:16:45 2003
@@ -0,0 +1,18 @@
+/*
+ * PCI defines and function prototypes
+ * Copyright 2003 Dell Computer Corporation
+ * by Matt Domsch <[email protected]>
+ */
+
+#ifndef LINUX_PCI_DYNIDS_H
+#define LINUX_PCI_DYNIDS_H
+
+#include <linux/list.h>
+#include <linux/mod_devicetable.h>
+
+struct dynid {
+ struct list_head node;
+ struct pci_device_id id;
+};
+
+#endif
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-2.5/include/linux/pci.h dynids/linux-2.5-dynids/include/linux/pci.h
--- linux-2.5/include/linux/pci.h Mon May 5 17:19:51 2003
+++ dynids/linux-2.5-dynids/include/linux/pci.h Tue May 6 11:16:45 2003
@@ -490,10 +490,16 @@
unsigned long end;
};
+struct pci_dynids {
+ spinlock_t lock; /* protects list, index */
+ struct list_head list; /* for IDs added at runtime */
+ unsigned int use_driver_data:1; /* pci_driver->driver_data is used */
+};
+
struct pci_driver {
struct list_head node;
char *name;
- const struct pci_device_id *id_table; /* NULL if wants all devices */
+ const struct pci_device_id *id_table; /* must be non-NULL for probe to be called */
int (*probe) (struct pci_dev *dev, const struct pci_device_id *id); /* New device inserted */
void (*remove) (struct pci_dev *dev); /* Device removed (NULL if not a hot-plug capable driver) */
int (*save_state) (struct pci_dev *dev, u32 state); /* Save Device Context */
@@ -502,6 +508,7 @@
int (*enable_wake) (struct pci_dev *dev, u32 state, int enable); /* Enable wake event */
struct device_driver driver;
+ struct pci_dynids dynids;
};
#define to_pci_driver(drv) container_of(drv,struct pci_driver, driver)
@@ -702,6 +709,12 @@
struct pci_bus_wrapped *wrapped_parent);
extern int pci_remove_device_safe(struct pci_dev *dev);
+static inline void
+pci_dynids_set_use_driver_data(struct pci_driver *pdrv, int val)
+{
+ pdrv->dynids.use_driver_data = val;
+}
+
#endif /* CONFIG_PCI */
/* Include architecture-dependent settings and functions */
@@ -750,6 +763,7 @@
static inline int scsi_to_pci_dma_dir(unsigned char scsi_dir) { return scsi_dir; }
static inline int pci_find_capability (struct pci_dev *dev, int cap) {return 0; }
static inline const struct pci_device_id *pci_match_device(const struct pci_device_id *ids, const struct pci_dev *dev) { return NULL; }
+static inline void pci_dynids_set_use_driver_data(struct pci_driver *pdrv, int val) { }
/* Power management related routines */
static inline int pci_save_state(struct pci_dev *dev, u32 *buffer) { return 0; }
On Tue, May 06, 2003 at 11:35:17AM -0500, Matt Domsch wrote:
> > You can't just call driver_attach(), as the bus semaphore needs to be
> > locked before doing so.? In short, you almost need to duplicate
> > bus_add_driver(), but not quite :)
>
> Right, and it seems to work. I made driver_attach non-static, declared
> it extern in pci.h, and call it in pci-driver.c while holding the bus
> semaphore and references to the driver and the bus. This also let me
> delete my probe_each_pci_dev() function and let the driver core
> handle it.
Nice, this looks much better. I don't have a problem with this patch
anymore. I'll wait for Pat to ack the driver core changes to see if he
agrees with them before sending this on.
thanks,
greg k-h
On Tue, 6 May 2003, Matt Domsch wrote:
> > You can't just call driver_attach(), as the bus semaphore needs to be
> > locked before doing so.? In short, you almost need to duplicate
> > bus_add_driver(), but not quite :)
>
> Right, and it seems to work. I made driver_attach non-static, declared
> it extern in pci.h, and call it in pci-driver.c while holding the bus
> semaphore and references to the driver and the bus. This also let me
> delete my probe_each_pci_dev() function and let the driver core
> handle it.
>
> Pat, can you ack the changes to bus.c and device.h please?
ACK. I'll add them to my tree.
Thanks,
-pat
On Tue, 13 May 2003, Patrick Mochel wrote:
>
> On Tue, 6 May 2003, Matt Domsch wrote:
>
> > > You can't just call driver_attach(), as the bus semaphore needs to be
> > > locked before doing so.? In short, you almost need to duplicate
> > > bus_add_driver(), but not quite :)
> >
> > Right, and it seems to work. I made driver_attach non-static, declared
> > it extern in pci.h, and call it in pci-driver.c while holding the bus
> > semaphore and references to the driver and the bus. This also let me
> > delete my probe_each_pci_dev() function and let the driver core
> > handle it.
> >
> > Pat, can you ack the changes to bus.c and device.h please?
>
> ACK. I'll add them to my tree.
I take that back, since it's already checked into your BK tree. Go forth
and merge.
-pat