From: =?utf-8?q?Kristian_H=C3=B8gsberg?= Subject: [PATCH 9/21] firewire: Switch cdev code over to use register_chrdev and keep a list of devices. Date: Wed, 7 Mar 2007 12:12:44 -0500 The old mechanism kept a struct cdev for each fw device, but fops->release would reference this struct after the device got freed in some cases. Signed-off-by: Kristian Høgsberg Signed-off-by: Stefan Richter --- drivers/firewire/fw-device-cdev.c | 5 ++- drivers/firewire/fw-device.c | 56 +++++++++++++++++++++++------------- drivers/firewire/fw-device.h | 5 ++- drivers/firewire/fw-transaction.c | 7 ++++ 4 files changed, 51 insertions(+), 22 deletions(-) diff --git a/drivers/firewire/fw-device-cdev.c b/drivers/firewire/fw-device-cdev.c index d9f3bb2..54ef27b 100644 --- a/drivers/firewire/fw-device-cdev.c +++ b/drivers/firewire/fw-device-cdev.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include "fw-transaction.h" @@ -103,7 +104,9 @@ static int fw_device_op_open(struct inode *inode, struct file *file) struct client *client; unsigned long flags; - device = container_of(inode->i_cdev, struct fw_device, cdev); + device = fw_device_from_devt(inode->i_rdev); + if (device == NULL) + return -ENODEV; client = kzalloc(sizeof *client, GFP_KERNEL); if (client == NULL) diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index ccc05e5..b24090a 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "fw-transaction.h" #include "fw-topology.h" #include "fw-device.h" @@ -407,14 +408,31 @@ static int shutdown_unit(struct device *device, void *data) return 0; } +static DEFINE_IDR(fw_device_idr); +int fw_cdev_major; + +struct fw_device *fw_device_from_devt(dev_t devt) +{ + struct fw_device *device; + + down_read(&fw_bus_type.subsys.rwsem); + device = idr_find(&fw_device_idr, MINOR(devt)); + up_read(&fw_bus_type.subsys.rwsem); + + return device; +} + static void fw_device_shutdown(struct work_struct *work) { struct fw_device *device = container_of(work, struct fw_device, work.work); + int minor = MINOR(device->device.devt); + + down_write(&fw_bus_type.subsys.rwsem); + idr_remove(&fw_device_idr, minor); + up_write(&fw_bus_type.subsys.rwsem); device_remove_file(&device->device, &config_rom_attribute); - cdev_del(&device->cdev); - unregister_chrdev_region(device->device.devt, 1); device_for_each_child(&device->device, NULL, shutdown_unit); device_unregister(&device->device); } @@ -434,9 +452,9 @@ static void fw_device_shutdown(struct work_struct *work) static void fw_device_init(struct work_struct *work) { - static atomic_t serial = ATOMIC_INIT(-1); struct fw_device *device = container_of(work, struct fw_device, work.work); + int minor, err; /* All failure paths here set node->data to NULL, so that we * don't try to do device_for_each_child() on a kfree()'d @@ -456,28 +474,24 @@ static void fw_device_init(struct work_struct *work) return; } + err = -ENOMEM; + down_write(&fw_bus_type.subsys.rwsem); + if (idr_pre_get(&fw_device_idr, GFP_KERNEL)) + err = idr_get_new(&fw_device_idr, device, &minor); + up_write(&fw_bus_type.subsys.rwsem); + if (err < 0) + goto error; + device->device.bus = &fw_bus_type; device->device.release = fw_device_release; device->device.parent = device->card->device; + device->device.devt = MKDEV(fw_cdev_major, minor); snprintf(device->device.bus_id, sizeof device->device.bus_id, - "fw%d", atomic_inc_return(&serial)); - - if (alloc_chrdev_region(&device->device.devt, 0, 1, "fw")) { - fw_error("Failed to register char device region.\n"); - goto error; - } - - cdev_init(&device->cdev, &fw_device_ops); - device->cdev.owner = THIS_MODULE; - kobject_set_name(&device->cdev.kobj, device->device.bus_id); - if (cdev_add(&device->cdev, device->device.devt, 1)) { - fw_error("Failed to register char device.\n"); - goto error; - } + "fw%d", minor); if (device_add(&device->device)) { fw_error("Failed to add device.\n"); - goto error; + goto error_with_cdev; } if (device_create_file(&device->device, &config_rom_attribute) < 0) { @@ -513,9 +527,11 @@ static void fw_device_init(struct work_struct *work) error_with_device: device_del(&device->device); + error_with_cdev: + down_write(&fw_bus_type.subsys.rwsem); + idr_remove(&fw_device_idr, minor); + up_write(&fw_bus_type.subsys.rwsem); error: - cdev_del(&device->cdev); - unregister_chrdev_region(device->device.devt, 1); put_device(&device->device); } diff --git a/drivers/firewire/fw-device.h b/drivers/firewire/fw-device.h index 4f731c2..1a3655b 100644 --- a/drivers/firewire/fw-device.h +++ b/drivers/firewire/fw-device.h @@ -39,7 +39,7 @@ struct fw_device { int generation; struct fw_card *card; struct device device; - struct cdev cdev; + struct list_head link; struct list_head client_list; __be32 *config_rom; size_t config_rom_length; @@ -59,6 +59,9 @@ int fw_device_enable_phys_dma(struct fw_device *device); void fw_device_cdev_update(struct fw_device *device); +struct fw_device *fw_device_from_devt(dev_t devt); +extern int fw_cdev_major; + struct fw_unit { struct device device; u32 *directory; diff --git a/drivers/firewire/fw-transaction.c b/drivers/firewire/fw-transaction.c index 8e2b945..3052698 100644 --- a/drivers/firewire/fw-transaction.c +++ b/drivers/firewire/fw-transaction.c @@ -761,6 +761,12 @@ static int __init fw_core_init(void) if (retval < 0) return retval; + fw_cdev_major = register_chrdev(0, "firewire", &fw_device_ops); + if (fw_cdev_major < 0) { + bus_unregister(&fw_bus_type); + return fw_cdev_major; + } + /* Add the vendor textual descriptor. */ retval = fw_core_add_descriptor(&vendor_id_descriptor); BUG_ON(retval < 0); @@ -772,6 +778,7 @@ static int __init fw_core_init(void) static void __exit fw_core_cleanup(void) { + unregister_chrdev(fw_cdev_major, "firewire"); bus_unregister(&fw_bus_type); }