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 <david@xxxxxxxxxxx>
---
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.
