Using the kernel ida facilities, we can avoid a lot of unnecessary code and at 
the same
time get rid of lirc_dev_lock in favour of per-device locks (the irctls array 
was used
throughout lirc_dev so this patch necessarily touches a lot of code).

Signed-off-by: David Härdeman <da...@hardeman.nu>
---
 drivers/media/rc/ir-lirc-codec.c        |    9 -
 drivers/media/rc/lirc_dev.c             |  258 ++++++++++++-------------------
 drivers/staging/media/lirc/lirc_zilog.c |   16 +-
 include/media/lirc_dev.h                |   14 --
 4 files changed, 115 insertions(+), 182 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index a30af91710fe..2c1221a61ea1 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -382,7 +382,6 @@ static int ir_lirc_register(struct rc_dev *dev)
 
        snprintf(drv->name, sizeof(drv->name), "ir-lirc-codec (%s)",
                 dev->driver_name);
-       drv->minor = -1;
        drv->features = features;
        drv->data = &dev->raw->lirc;
        drv->rbuf = NULL;
@@ -394,11 +393,9 @@ static int ir_lirc_register(struct rc_dev *dev)
        drv->rdev = dev;
        drv->owner = THIS_MODULE;
 
-       drv->minor = lirc_register_driver(drv);
-       if (drv->minor < 0) {
-               rc = -ENODEV;
+       rc = lirc_register_driver(drv);
+       if (rc < 0)
                goto out;
-       }
 
        dev->raw->lirc.drv = drv;
        dev->raw->lirc.dev = dev;
@@ -413,7 +410,7 @@ static int ir_lirc_unregister(struct rc_dev *dev)
 {
        struct lirc_codec *lirc = &dev->raw->lirc;
 
-       lirc_unregister_driver(lirc->drv->minor);
+       lirc_unregister_driver(lirc->drv);
        kfree(lirc->drv);
        lirc->drv = NULL;
 
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index e44e0b23b883..7db7d4c57991 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -31,23 +31,35 @@
 #include <linux/bitops.h>
 #include <linux/device.h>
 #include <linux/cdev.h>
+#include <linux/idr.h>
 
 #include <media/rc-core.h>
 #include <media/lirc.h>
 #include <media/lirc_dev.h>
 
 #define IRCTL_DEV_NAME "BaseRemoteCtl"
-#define NOPLUG         -1
 #define LOGHEAD                "lirc_dev (%s[%d]): "
 
 static dev_t lirc_base_dev;
 
+/**
+ * struct irctl - lirc per-device structure
+ * @d:                 internal copy of the &struct lirc_driver for the device
+ * @dead:              if the device has gone away
+ * @users:             the number of users of the lirc chardev (currently 
limited to 1)
+ * @mutex:             synchronizes open()/close()/ioctl()/etc calls
+ * @buf:               used to store lirc RX data
+ * @buf_internal:      whether @buf was allocated internally or not
+ * @chunk_size:                @buf stores and read() returns data chunks of 
this size
+ * @dev:               the &struct device for the lirc device
+ * @cdev:              the &struct device for the lirc chardev
+ */
 struct irctl {
        struct lirc_driver d;
-       int attached;
-       int open;
+       bool dead;
+       unsigned users;
 
-       struct mutex irctl_lock;
+       struct mutex mutex;
        struct lirc_buffer *buf;
        bool buf_internal;
        unsigned int chunk_size;
@@ -56,9 +68,9 @@ struct irctl {
        struct cdev cdev;
 };
 
-static DEFINE_MUTEX(lirc_dev_lock);
-
-static struct irctl *irctls[MAX_IRCTL_DEVICES];
+/* Used to keep track of allocated lirc devices */
+#define LIRC_MAX_DEVICES 256
+static DEFINE_IDA(lirc_ida);
 
 /* Only used for sysfs but defined to void otherwise */
 static struct class *lirc_class;
@@ -72,9 +84,6 @@ static void lirc_release(struct device *ld)
                kfree(ir->buf);
        }
 
-       mutex_lock(&lirc_dev_lock);
-       irctls[ir->d.minor] = NULL;
-       mutex_unlock(&lirc_dev_lock);
        kfree(ir);
 }
 
@@ -117,7 +126,18 @@ static int lirc_allocate_buffer(struct irctl *ir)
        return err;
 }
 
-int lirc_register_driver(struct lirc_driver *d)
+/**
+ * lirc_register_driver() - create a new lirc device by registering a driver
+ * @d: the &struct lirc_driver to register
+ *
+ * This function allocates and registers a lirc device for a given
+ * &lirc_driver. The &lirc_driver structure is updated to contain
+ * information about the allocated device (minor, buffer, etc).
+ *
+ * Return: zero on success or a negative error value.
+ */
+int
+lirc_register_driver(struct lirc_driver *d)
 {
        struct irctl *ir;
        int minor;
@@ -138,12 +158,6 @@ int lirc_register_driver(struct lirc_driver *d)
                return -EINVAL;
        }
 
-       if (d->minor >= MAX_IRCTL_DEVICES) {
-               dev_err(d->dev, "minor must be between 0 and %d!\n",
-                                               MAX_IRCTL_DEVICES - 1);
-               return -EBADRQC;
-       }
-
        if (d->code_length < 1 || d->code_length > (BUFLEN * 8)) {
                dev_err(d->dev, "code length must be less than %d bits\n",
                                                                BUFLEN * 8);
@@ -156,49 +170,30 @@ int lirc_register_driver(struct lirc_driver *d)
                return -EBADRQC;
        }
 
-       mutex_lock(&lirc_dev_lock);
-
-       minor = d->minor;
+       d->name[sizeof(d->name)-1] = '\0';
+       if (d->features == 0)
+               d->features = LIRC_CAN_REC_LIRCCODE;
 
-       if (minor < 0) {
-               /* find first free slot for driver */
-               for (minor = 0; minor < MAX_IRCTL_DEVICES; minor++)
-                       if (!irctls[minor])
-                               break;
-               if (minor == MAX_IRCTL_DEVICES) {
-                       dev_err(d->dev, "no free slots for drivers!\n");
-                       err = -ENOMEM;
-                       goto out_lock;
-               }
-       } else if (irctls[minor]) {
-               dev_err(d->dev, "minor (%d) just registered!\n", minor);
-               err = -EBUSY;
-               goto out_lock;
-       }
+       minor = ida_simple_get(&lirc_ida, 0, LIRC_MAX_DEVICES, GFP_KERNEL);
+       if (minor < 0)
+               return minor;
+       d->minor = minor;
 
        ir = kzalloc(sizeof(struct irctl), GFP_KERNEL);
        if (!ir) {
                err = -ENOMEM;
-               goto out_lock;
+               goto out_minor;
        }
 
-       mutex_init(&ir->irctl_lock);
-       irctls[minor] = ir;
-       d->minor = minor;
-
-       /* some safety check 8-) */
-       d->name[sizeof(d->name)-1] = '\0';
-
-       if (d->features == 0)
-               d->features = LIRC_CAN_REC_LIRCCODE;
+       mutex_init(&ir->mutex);
 
        ir->d = *d;
 
        if (LIRC_CAN_REC(d->features)) {
-               err = lirc_allocate_buffer(irctls[minor]);
+               err = lirc_allocate_buffer(ir);
                if (err) {
                        kfree(ir);
-                       goto out_lock;
+                       goto out_minor;
                }
                d->rbuf = ir->buf;
        }
@@ -218,107 +213,82 @@ int lirc_register_driver(struct lirc_driver *d)
        if (err)
                goto out_free_dev;
 
-       ir->attached = 1;
-
        err = device_add(&ir->dev);
        if (err)
                goto out_cdev;
 
-       mutex_unlock(&lirc_dev_lock);
-
        dev_info(ir->d.dev, "lirc_dev: driver %s registered at minor = %d\n",
                 ir->d.name, ir->d.minor);
 
-       return minor;
+       d->lirc_internal = ir;
+       return 0;
 
 out_cdev:
        cdev_del(&ir->cdev);
 out_free_dev:
        put_device(&ir->dev);
-out_lock:
-       mutex_unlock(&lirc_dev_lock);
+out_minor:
+       ida_simple_remove(&lirc_ida, minor);
 
        return err;
 }
 EXPORT_SYMBOL(lirc_register_driver);
 
-int lirc_unregister_driver(int minor)
+/**
+ * lirc_unregister_driver() - remove the lirc device for a given driver
+ * @d: the &struct lirc_driver to unregister
+ *
+ * This function unregisters the lirc device for a given &lirc_driver.
+ */
+void
+lirc_unregister_driver(struct lirc_driver *d)
 {
        struct irctl *ir;
 
-       if (minor < 0 || minor >= MAX_IRCTL_DEVICES) {
-               pr_err("minor (%d) must be between 0 and %d!\n",
-                                       minor, MAX_IRCTL_DEVICES - 1);
-               return -EBADRQC;
-       }
+       if (!d->lirc_internal)
+               return;
 
-       ir = irctls[minor];
-       if (!ir) {
-               pr_err("failed to get irctl\n");
-               return -ENOENT;
-       }
+       ir = d->lirc_internal;
 
-       mutex_lock(&lirc_dev_lock);
-
-       if (ir->d.minor != minor) {
-               dev_err(ir->d.dev, "lirc_dev: minor %d device not registered\n",
-                                                                       minor);
-               mutex_unlock(&lirc_dev_lock);
-               return -ENOENT;
-       }
+       mutex_lock(&ir->mutex);
 
        dev_dbg(ir->d.dev, "lirc_dev: driver %s unregistered from minor = %d\n",
                ir->d.name, ir->d.minor);
 
-       ir->attached = 0;
-       if (ir->open) {
+       ir->dead = true;
+       if (ir->users) {
                dev_dbg(ir->d.dev, LOGHEAD "releasing opened driver\n",
                        ir->d.name, ir->d.minor);
                wake_up_interruptible(&ir->buf->wait_poll);
        }
 
-       mutex_unlock(&lirc_dev_lock);
+       mutex_unlock(&ir->mutex);
 
        device_del(&ir->dev);
        cdev_del(&ir->cdev);
-       put_device(&ir->dev);
-
-       return 0;
+       ida_simple_remove(&lirc_ida, d->minor);
+       d->minor = -1;
+       d->lirc_internal = NULL;
 }
 EXPORT_SYMBOL(lirc_unregister_driver);
 
 int lirc_dev_fop_open(struct inode *inode, struct file *file)
 {
-       struct irctl *ir;
+       struct irctl *ir = container_of(inode->i_cdev, struct irctl, cdev);
        int retval;
 
-       if (iminor(inode) >= MAX_IRCTL_DEVICES) {
-               pr_err("open result for %d is -ENODEV\n", iminor(inode));
-               return -ENODEV;
-       }
-
-       if (mutex_lock_interruptible(&lirc_dev_lock))
-               return -ERESTARTSYS;
+       mutex_lock(&ir->mutex);
 
-       ir = irctls[iminor(inode)];
-       mutex_unlock(&lirc_dev_lock);
-
-       if (!ir) {
-               retval = -ENODEV;
+       if (ir->users) {
+               retval = -EBUSY;
+               mutex_unlock(&ir->mutex);
                goto error;
        }
+       ir->users++;
 
-       dev_dbg(ir->d.dev, LOGHEAD "open called\n", ir->d.name, ir->d.minor);
-
-       if (ir->d.minor == NOPLUG) {
-               retval = -ENODEV;
-               goto error;
-       }
+       mutex_unlock(&ir->mutex);
 
-       if (ir->open) {
-               retval = -EBUSY;
-               goto error;
-       }
+       dev_dbg(ir->d.dev, LOGHEAD "open called\n", ir->d.name, ir->d.minor);
 
        if (ir->d.rdev) {
                retval = rc_open(ir->d.rdev);
@@ -329,8 +299,7 @@ int lirc_dev_fop_open(struct inode *inode, struct file 
*file)
        if (ir->buf)
                lirc_buffer_clear(ir->buf);
 
-       ir->open++;
-
+       file->private_data = ir;
        nonseekable_open(inode, file);
 
        return 0;
@@ -342,22 +311,14 @@ EXPORT_SYMBOL(lirc_dev_fop_open);
 
 int lirc_dev_fop_close(struct inode *inode, struct file *file)
 {
-       struct irctl *ir = irctls[iminor(inode)];
-       int ret;
-
-       if (!ir) {
-               pr_err("called with invalid irctl\n");
-               return -EINVAL;
-       }
+       struct irctl *ir = file->private_data;
 
-       ret = mutex_lock_killable(&lirc_dev_lock);
-       WARN_ON(ret);
+       mutex_lock(&ir->mutex);
 
        rc_close(ir->d.rdev);
+       ir->users--;
 
-       ir->open--;
-       if (!ret)
-               mutex_unlock(&lirc_dev_lock);
+       mutex_unlock(&ir->mutex);
 
        return 0;
 }
@@ -365,26 +326,21 @@ EXPORT_SYMBOL(lirc_dev_fop_close);
 
 unsigned int lirc_dev_fop_poll(struct file *file, poll_table *wait)
 {
-       struct irctl *ir = irctls[iminor(file_inode(file))];
+       struct irctl *ir = file->private_data;
        unsigned int ret;
 
-       if (!ir) {
-               pr_err("called with invalid irctl\n");
-               return POLLERR;
-       }
-
-       if (!ir->attached)
+       if (ir->dead)
                return POLLHUP | POLLERR;
 
-       if (ir->buf) {
-               poll_wait(file, &ir->buf->wait_poll, wait);
+       if (!ir->buf)
+               return POLLERR;
+
+       poll_wait(file, &ir->buf->wait_poll, wait);
 
-               if (lirc_buffer_empty(ir->buf))
-                       ret = 0;
-               else
-                       ret = POLLIN | POLLRDNORM;
-       } else
-               ret = POLLERR;
+       if (lirc_buffer_empty(ir->buf))
+               ret = 0;
+       else
+               ret = POLLIN | POLLRDNORM;
 
        dev_dbg(ir->d.dev, LOGHEAD "poll result = %d\n",
                ir->d.name, ir->d.minor, ret);
@@ -395,25 +351,20 @@ EXPORT_SYMBOL(lirc_dev_fop_poll);
 
 long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
+       struct irctl *ir = file->private_data;
        __u32 mode;
        int result = 0;
-       struct irctl *ir = irctls[iminor(file_inode(file))];
-
-       if (!ir) {
-               pr_err("no irctl found!\n");
-               return -ENODEV;
-       }
 
        dev_dbg(ir->d.dev, LOGHEAD "ioctl called (0x%x)\n",
                ir->d.name, ir->d.minor, cmd);
 
-       if (ir->d.minor == NOPLUG || !ir->attached) {
+       if (ir->dead) {
                dev_err(ir->d.dev, LOGHEAD "ioctl result = -ENODEV\n",
                        ir->d.name, ir->d.minor);
                return -ENODEV;
        }
 
-       mutex_lock(&ir->irctl_lock);
+       mutex_lock(&ir->mutex);
 
        switch (cmd) {
        case LIRC_GET_FEATURES:
@@ -468,7 +419,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
                result = -ENOTTY;
        }
 
-       mutex_unlock(&ir->irctl_lock);
+       mutex_unlock(&ir->mutex);
 
        return result;
 }
@@ -479,16 +430,11 @@ ssize_t lirc_dev_fop_read(struct file *file,
                          size_t length,
                          loff_t *ppos)
 {
-       struct irctl *ir = irctls[iminor(file_inode(file))];
+       struct irctl *ir = file->private_data;
        unsigned char *buf;
        int ret = 0, written = 0;
        DECLARE_WAITQUEUE(wait, current);
 
-       if (!ir) {
-               pr_err("called with invalid irctl\n");
-               return -ENODEV;
-       }
-
        if (!LIRC_CAN_REC(ir->d.features))
                return -EINVAL;
 
@@ -498,11 +444,12 @@ ssize_t lirc_dev_fop_read(struct file *file,
        if (!buf)
                return -ENOMEM;
 
-       if (mutex_lock_interruptible(&ir->irctl_lock)) {
+       if (mutex_lock_interruptible(&ir->mutex)) {
                ret = -ERESTARTSYS;
                goto out_unlocked;
        }
-       if (!ir->attached) {
+
+       if (ir->dead) {
                ret = -ENODEV;
                goto out_locked;
        }
@@ -541,18 +488,18 @@ ssize_t lirc_dev_fop_read(struct file *file,
                                break;
                        }
 
-                       mutex_unlock(&ir->irctl_lock);
+                       mutex_unlock(&ir->mutex);
                        set_current_state(TASK_INTERRUPTIBLE);
                        schedule();
                        set_current_state(TASK_RUNNING);
 
-                       if (mutex_lock_interruptible(&ir->irctl_lock)) {
+                       if (mutex_lock_interruptible(&ir->mutex)) {
                                ret = -ERESTARTSYS;
                                remove_wait_queue(&ir->buf->wait_poll, &wait);
                                goto out_unlocked;
                        }
 
-                       if (!ir->attached) {
+                       if (ir->dead) {
                                ret = -ENODEV;
                                goto out_locked;
                        }
@@ -570,7 +517,7 @@ ssize_t lirc_dev_fop_read(struct file *file,
        remove_wait_queue(&ir->buf->wait_poll, &wait);
 
 out_locked:
-       mutex_unlock(&ir->irctl_lock);
+       mutex_unlock(&ir->mutex);
 
 out_unlocked:
        kfree(buf);
@@ -581,7 +528,8 @@ EXPORT_SYMBOL(lirc_dev_fop_read);
 
 void *lirc_get_pdata(struct file *file)
 {
-       return irctls[iminor(file_inode(file))]->d.data;
+       struct irctl *ir = file->private_data;
+       return ir->d.data;
 }
 EXPORT_SYMBOL(lirc_get_pdata);
 
@@ -596,7 +544,7 @@ static int __init lirc_dev_init(void)
                return PTR_ERR(lirc_class);
        }
 
-       retval = alloc_chrdev_region(&lirc_base_dev, 0, MAX_IRCTL_DEVICES,
+       retval = alloc_chrdev_region(&lirc_base_dev, 0, LIRC_MAX_DEVICES,
                                     IRCTL_DEV_NAME);
        if (retval) {
                class_destroy(lirc_class);
@@ -613,7 +561,7 @@ static int __init lirc_dev_init(void)
 static void __exit lirc_dev_exit(void)
 {
        class_destroy(lirc_class);
-       unregister_chrdev_region(lirc_base_dev, MAX_IRCTL_DEVICES);
+       unregister_chrdev_region(lirc_base_dev, LIRC_MAX_DEVICES);
        pr_info("module unloaded\n");
 }
 
diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 59e05aa1bc55..ffb70dee4547 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -183,11 +183,7 @@ static void release_ir_device(struct kref *ref)
         * ir->open_count ==  0 - happens on final close()
         * ir_lock, tx_ref_lock, rx_ref_lock, all released
         */
-       if (ir->l.minor >= 0) {
-               lirc_unregister_driver(ir->l.minor);
-               ir->l.minor = -1;
-       }
-
+       lirc_unregister_driver(&ir->l);
        if (kfifo_initialized(&ir->rbuf.fifo))
                lirc_buffer_free(&ir->rbuf);
        list_del(&ir->list);
@@ -1382,7 +1378,6 @@ static const struct file_operations lirc_fops = {
 
 static struct lirc_driver lirc_template = {
        .name           = "lirc_zilog",
-       .minor          = -1,
        .code_length    = 13,
        .buffer_size    = BUFLEN / 2,
        .chunk_size     = 2,
@@ -1597,14 +1592,13 @@ static int ir_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
        }
 
        /* register with lirc */
-       ir->l.minor = lirc_register_driver(&ir->l);
-       if (ir->l.minor < 0) {
+       ret = lirc_register_driver(&ir->l);
+       if (ret < 0) {
                dev_err(tx->ir->l.dev,
-                       "%s: lirc_register_driver() failed: %i\n",
-                       __func__, ir->l.minor);
-               ret = -EBADRQC;
+                       "%s: lirc_register_driver failed: %i\n", __func__, ret);
                goto out_put_xx;
        }
+
        dev_info(ir->l.dev,
                 "IR unit on %s (i2c-%d) registered as lirc%d and ready\n",
                 adap->name, adap->nr, ir->l.minor);
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 1f327e25a9be..f7629ff116a9 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -9,7 +9,6 @@
 #ifndef _LINUX_LIRC_DEV_H
 #define _LINUX_LIRC_DEV_H
 
-#define MAX_IRCTL_DEVICES 8
 #define BUFLEN            16
 
 #define mod(n, div) ((n) % (div))
@@ -164,6 +163,8 @@ static inline unsigned int lirc_buffer_write(struct 
lirc_buffer *buf,
  *                     device.
  *
  * @owner:             the module owning this struct
+ *
+ * @lirc_internal:     lirc_dev bookkeeping data, don't touch.
  */
 struct lirc_driver {
        char name[40];
@@ -182,19 +183,12 @@ struct lirc_driver {
        const struct file_operations *fops;
        struct device *dev;
        struct module *owner;
+       void *lirc_internal;
 };
 
-/* following functions can be called ONLY from user context
- *
- * returns negative value on error or minor number
- * of the registered device if success
- * contents of the structure pointed by p is copied
- */
 extern int lirc_register_driver(struct lirc_driver *d);
 
-/* returns negative value on error or 0 if success
-*/
-extern int lirc_unregister_driver(int minor);
+extern void lirc_unregister_driver(struct lirc_driver *d);
 
 /* Returns the private data stored in the lirc_driver
  * associated with the given device file pointer.

Reply via email to