From 6fa99e1af1981d9a57cc73b5c2f05903f18413c1 Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:13 -0300 Subject: [media] lirc_dev: place buffer allocation on separate function During the driver registration, move the buffer allocation on a separate function. Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 57 +++++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 23 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 92ae1903c010..571697811e4d 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -203,13 +203,41 @@ err_out: return retval; } -int lirc_register_driver(struct lirc_driver *d) +static int lirc_allocate_buffer(struct irctl *ir) { - struct irctl *ir; - int minor; + int err; int bytes_in_key; unsigned int chunk_size; unsigned int buffer_size; + struct lirc_driver *d = &ir->d; + + bytes_in_key = BITS_TO_LONGS(d->code_length) + + (d->code_length % 8 ? 1 : 0); + buffer_size = d->buffer_size ? d->buffer_size : BUFLEN / bytes_in_key; + chunk_size = d->chunk_size ? d->chunk_size : bytes_in_key; + + if (d->rbuf) { + ir->buf = d->rbuf; + } else { + ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL); + if (!ir->buf) + return -ENOMEM; + + err = lirc_buffer_init(ir->buf, chunk_size, buffer_size); + if (err) { + kfree(ir->buf); + return err; + } + } + ir->chunk_size = ir->buf->chunk_size; + + return 0; +} + +int lirc_register_driver(struct lirc_driver *d) +{ + struct irctl *ir; + int minor; int err; if (!d) { @@ -314,26 +342,9 @@ int lirc_register_driver(struct lirc_driver *d) /* some safety check 8-) */ d->name[sizeof(d->name)-1] = '\0'; - bytes_in_key = BITS_TO_LONGS(d->code_length) + - (d->code_length % 8 ? 1 : 0); - buffer_size = d->buffer_size ? d->buffer_size : BUFLEN / bytes_in_key; - chunk_size = d->chunk_size ? d->chunk_size : bytes_in_key; - - if (d->rbuf) { - ir->buf = d->rbuf; - } else { - ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL); - if (!ir->buf) { - err = -ENOMEM; - goto out_lock; - } - err = lirc_buffer_init(ir->buf, chunk_size, buffer_size); - if (err) { - kfree(ir->buf); - goto out_lock; - } - } - ir->chunk_size = ir->buf->chunk_size; + err = lirc_allocate_buffer(ir); + if (err) + goto out_lock; if (d->features == 0) d->features = LIRC_CAN_REC_LIRCCODE; -- cgit v1.2.3 From 70143984dd2c714386ee2d6c0f650bff75873d75 Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:14 -0300 Subject: [media] lirc_dev: allow bufferless driver registration Transmitters don't necessarily need to have a FIFO managed buffer for their transfers. When registering the driver, before allocating the buffer, check whether the device is a transmitter or receiver. Allocate the buffer only for receivers. Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 571697811e4d..154e553b3b67 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -205,12 +205,14 @@ err_out: static int lirc_allocate_buffer(struct irctl *ir) { - int err; + int err = 0; int bytes_in_key; unsigned int chunk_size; unsigned int buffer_size; struct lirc_driver *d = &ir->d; + mutex_lock(&lirc_dev_lock); + bytes_in_key = BITS_TO_LONGS(d->code_length) + (d->code_length % 8 ? 1 : 0); buffer_size = d->buffer_size ? d->buffer_size : BUFLEN / bytes_in_key; @@ -220,21 +222,26 @@ static int lirc_allocate_buffer(struct irctl *ir) ir->buf = d->rbuf; } else { ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL); - if (!ir->buf) - return -ENOMEM; + if (!ir->buf) { + err = -ENOMEM; + goto out; + } err = lirc_buffer_init(ir->buf, chunk_size, buffer_size); if (err) { kfree(ir->buf); - return err; + goto out; } } ir->chunk_size = ir->buf->chunk_size; - return 0; +out: + mutex_unlock(&lirc_dev_lock); + + return err; } -int lirc_register_driver(struct lirc_driver *d) +static int lirc_allocate_driver(struct lirc_driver *d) { struct irctl *ir; int minor; @@ -342,10 +349,6 @@ int lirc_register_driver(struct lirc_driver *d) /* some safety check 8-) */ d->name[sizeof(d->name)-1] = '\0'; - err = lirc_allocate_buffer(ir); - if (err) - goto out_lock; - if (d->features == 0) d->features = LIRC_CAN_REC_LIRCCODE; @@ -385,6 +388,23 @@ out_lock: out: return err; } + +int lirc_register_driver(struct lirc_driver *d) +{ + int minor, err = 0; + + minor = lirc_allocate_driver(d); + if (minor < 0) + return minor; + + if (LIRC_CAN_REC(d->features)) { + err = lirc_allocate_buffer(irctls[minor]); + if (err) + lirc_unregister_driver(minor); + } + + return err ? err : minor; +} EXPORT_SYMBOL(lirc_register_driver); int lirc_unregister_driver(int minor) -- cgit v1.2.3 From fe084c7eff2d89631539bdf941c47ef2290f1649 Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:15 -0300 Subject: [media] lirc_dev: remove unnecessary debug prints Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 25 ------------------------- 1 file changed, 25 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 154e553b3b67..9f20f94a31ae 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -80,8 +80,6 @@ static void lirc_irctl_init(struct irctl *ir) static void lirc_irctl_cleanup(struct irctl *ir) { - dev_dbg(ir->d.dev, LOGHEAD "cleaning up\n", ir->d.name, ir->d.minor); - device_destroy(lirc_class, MKDEV(MAJOR(lirc_base_dev), ir->d.minor)); if (ir->buf != ir->d.rbuf) { @@ -127,9 +125,6 @@ static int lirc_thread(void *irctl) { struct irctl *ir = irctl; - dev_dbg(ir->d.dev, LOGHEAD "poll thread started\n", - ir->d.name, ir->d.minor); - do { if (ir->open) { if (ir->jiffies_to_wait) { @@ -146,9 +141,6 @@ static int lirc_thread(void *irctl) } } while (!kthread_should_stop()); - dev_dbg(ir->d.dev, LOGHEAD "poll thread ended\n", - ir->d.name, ir->d.minor); - return 0; } @@ -277,8 +269,6 @@ static int lirc_allocate_driver(struct lirc_driver *d) goto out; } - dev_dbg(d->dev, "lirc_dev: lirc_register_driver: sample_rate: %d\n", - d->sample_rate); if (d->sample_rate) { if (2 > d->sample_rate || HZ < d->sample_rate) { dev_err(d->dev, "lirc_dev: lirc_register_driver: " @@ -521,10 +511,6 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file) } error: - if (ir) - dev_dbg(ir->d.dev, LOGHEAD "open result = %d\n", - ir->d.name, ir->d.minor, retval); - mutex_unlock(&lirc_dev_lock); nonseekable_open(inode, file); @@ -546,8 +532,6 @@ int lirc_dev_fop_close(struct inode *inode, struct file *file) cdev = ir->cdev; - dev_dbg(ir->d.dev, LOGHEAD "close called\n", ir->d.name, ir->d.minor); - ret = mutex_lock_killable(&lirc_dev_lock); WARN_ON(ret); @@ -582,8 +566,6 @@ unsigned int lirc_dev_fop_poll(struct file *file, poll_table *wait) return POLLERR; } - dev_dbg(ir->d.dev, LOGHEAD "poll called\n", ir->d.name, ir->d.minor); - if (!ir->attached) return POLLERR; @@ -679,9 +661,6 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg) result = -EINVAL; } - dev_dbg(ir->d.dev, LOGHEAD "ioctl result = %d\n", - ir->d.name, ir->d.minor, result); - mutex_unlock(&ir->irctl_lock); return result; @@ -786,8 +765,6 @@ out_locked: out_unlocked: kfree(buf); - dev_dbg(ir->d.dev, LOGHEAD "read result = %s (%d)\n", - ir->d.name, ir->d.minor, ret ? "" : "", ret); return ret ? ret : written; } @@ -810,8 +787,6 @@ ssize_t lirc_dev_fop_write(struct file *file, const char __user *buffer, return -ENODEV; } - dev_dbg(ir->d.dev, LOGHEAD "write called\n", ir->d.name, ir->d.minor); - if (!ir->attached) return -ENODEV; -- cgit v1.2.3 From 3fac031460e31b5ba1da578bf43f81a9c7a751cc Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:16 -0300 Subject: [media] lirc_dev: replace printk with pr_* or dev_* This patch mutes also all the checkpatch warnings related to printk. Reword all the printouts so that the string doesn't need to be split, which fixes the following checkpatch warning: WARNING: quoted string split across lines Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 79 +++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 45 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 9f20f94a31ae..59f4c93cccc4 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -19,6 +19,8 @@ * */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -240,59 +242,51 @@ static int lirc_allocate_driver(struct lirc_driver *d) int err; if (!d) { - printk(KERN_ERR "lirc_dev: lirc_register_driver: " - "driver pointer must be not NULL!\n"); + pr_err("driver pointer must be not NULL!\n"); err = -EBADRQC; goto out; } if (!d->dev) { - printk(KERN_ERR "%s: dev pointer not filled in!\n", __func__); + pr_err("dev pointer not filled in!\n"); err = -EINVAL; goto out; } if (MAX_IRCTL_DEVICES <= d->minor) { - dev_err(d->dev, "lirc_dev: lirc_register_driver: " - "\"minor\" must be between 0 and %d (%d)!\n", - MAX_IRCTL_DEVICES - 1, d->minor); + dev_err(d->dev, "minor must be between 0 and %d!\n", + MAX_IRCTL_DEVICES - 1); err = -EBADRQC; goto out; } if (1 > d->code_length || (BUFLEN * 8) < d->code_length) { - dev_err(d->dev, "lirc_dev: lirc_register_driver: " - "code length in bits for minor (%d) " - "must be less than %d!\n", - d->minor, BUFLEN * 8); + dev_err(d->dev, "code length must be less than %d bits\n", + BUFLEN * 8); err = -EBADRQC; goto out; } if (d->sample_rate) { if (2 > d->sample_rate || HZ < d->sample_rate) { - dev_err(d->dev, "lirc_dev: lirc_register_driver: " - "sample_rate must be between 2 and %d!\n", HZ); + dev_err(d->dev, "invalid %d sample rate\n", + d->sample_rate); err = -EBADRQC; goto out; } if (!d->add_to_buf) { - dev_err(d->dev, "lirc_dev: lirc_register_driver: " - "add_to_buf cannot be NULL when " - "sample_rate is set\n"); + dev_err(d->dev, "add_to_buf not set\n"); err = -EBADRQC; goto out; } } else if (!(d->fops && d->fops->read) && !d->rbuf) { - dev_err(d->dev, "lirc_dev: lirc_register_driver: " - "fops->read and rbuf cannot all be NULL!\n"); + dev_err(d->dev, "fops->read and rbuf are NULL!\n"); err = -EBADRQC; goto out; } else if (!d->rbuf) { if (!(d->fops && d->fops->read && d->fops->poll && d->fops->unlocked_ioctl)) { - dev_err(d->dev, "lirc_dev: lirc_register_driver: " - "neither read, poll nor unlocked_ioctl can be NULL!\n"); + dev_err(d->dev, "undefined read, poll, ioctl\n"); err = -EBADRQC; goto out; } @@ -308,14 +302,12 @@ static int lirc_allocate_driver(struct lirc_driver *d) if (!irctls[minor]) break; if (MAX_IRCTL_DEVICES == minor) { - dev_err(d->dev, "lirc_dev: lirc_register_driver: " - "no free slots for drivers!\n"); + dev_err(d->dev, "no free slots for drivers!\n"); err = -ENOMEM; goto out_lock; } } else if (irctls[minor]) { - dev_err(d->dev, "lirc_dev: lirc_register_driver: " - "minor (%d) just registered!\n", minor); + dev_err(d->dev, "minor (%d) just registered!\n", minor); err = -EBUSY; goto out_lock; } @@ -352,9 +344,8 @@ static int lirc_allocate_driver(struct lirc_driver *d) /* try to fire up polling thread */ ir->task = kthread_run(lirc_thread, (void *)ir, "lirc_dev"); if (IS_ERR(ir->task)) { - dev_err(d->dev, "lirc_dev: lirc_register_driver: " - "cannot run poll thread for minor = %d\n", - d->minor); + dev_err(d->dev, "cannot run thread for minor = %d\n", + d->minor); err = -ECHILD; goto out_sysfs; } @@ -403,15 +394,14 @@ int lirc_unregister_driver(int minor) struct cdev *cdev; if (minor < 0 || minor >= MAX_IRCTL_DEVICES) { - printk(KERN_ERR "lirc_dev: %s: minor (%d) must be between " - "0 and %d!\n", __func__, minor, MAX_IRCTL_DEVICES - 1); + pr_err("minor (%d) must be between 0 and %d!\n", + minor, MAX_IRCTL_DEVICES - 1); return -EBADRQC; } ir = irctls[minor]; if (!ir) { - printk(KERN_ERR "lirc_dev: %s: failed to get irctl struct " - "for minor %d!\n", __func__, minor); + pr_err("failed to get irctl\n"); return -ENOENT; } @@ -420,8 +410,8 @@ int lirc_unregister_driver(int minor) mutex_lock(&lirc_dev_lock); if (ir->d.minor != minor) { - printk(KERN_ERR "lirc_dev: %s: minor (%d) device not " - "registered!\n", __func__, minor); + dev_err(ir->d.dev, "lirc_dev: minor %d device not registered\n", + minor); mutex_unlock(&lirc_dev_lock); return -ENOENT; } @@ -463,8 +453,7 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file) int retval = 0; if (iminor(inode) >= MAX_IRCTL_DEVICES) { - printk(KERN_WARNING "lirc_dev [%d]: open result = -ENODEV\n", - iminor(inode)); + pr_err("open result for %d is -ENODEV\n", iminor(inode)); return -ENODEV; } @@ -526,7 +515,7 @@ int lirc_dev_fop_close(struct inode *inode, struct file *file) int ret; if (!ir) { - printk(KERN_ERR "%s: called with invalid irctl\n", __func__); + pr_err("called with invalid irctl\n"); return -EINVAL; } @@ -562,7 +551,7 @@ unsigned int lirc_dev_fop_poll(struct file *file, poll_table *wait) unsigned int ret; if (!ir) { - printk(KERN_ERR "%s: called with invalid irctl\n", __func__); + pr_err("called with invalid irctl\n"); return POLLERR; } @@ -593,7 +582,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg) struct irctl *ir = irctls[iminor(file_inode(file))]; if (!ir) { - printk(KERN_ERR "lirc_dev: %s: no irctl found!\n", __func__); + pr_err("no irctl found!\n"); return -ENODEV; } @@ -601,7 +590,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg) ir->d.name, ir->d.minor, cmd); if (ir->d.minor == NOPLUG || !ir->attached) { - dev_dbg(ir->d.dev, LOGHEAD "ioctl result = -ENODEV\n", + dev_err(ir->d.dev, LOGHEAD "ioctl result = -ENODEV\n", ir->d.name, ir->d.minor); return -ENODEV; } @@ -678,7 +667,7 @@ ssize_t lirc_dev_fop_read(struct file *file, DECLARE_WAITQUEUE(wait, current); if (!ir) { - printk(KERN_ERR "%s: called with invalid irctl\n", __func__); + pr_err("called with invalid irctl\n"); return -ENODEV; } @@ -783,7 +772,7 @@ ssize_t lirc_dev_fop_write(struct file *file, const char __user *buffer, struct irctl *ir = irctls[iminor(file_inode(file))]; if (!ir) { - printk(KERN_ERR "%s: called with invalid irctl\n", __func__); + pr_err("called with invalid irctl\n"); return -ENODEV; } @@ -802,7 +791,7 @@ static int __init lirc_dev_init(void) lirc_class = class_create(THIS_MODULE, "lirc"); if (IS_ERR(lirc_class)) { retval = PTR_ERR(lirc_class); - printk(KERN_ERR "lirc_dev: class_create failed\n"); + pr_err("class_create failed\n"); goto error; } @@ -810,13 +799,13 @@ static int __init lirc_dev_init(void) IRCTL_DEV_NAME); if (retval) { class_destroy(lirc_class); - printk(KERN_ERR "lirc_dev: alloc_chrdev_region failed\n"); + pr_err("alloc_chrdev_region failed\n"); goto error; } - printk(KERN_INFO "lirc_dev: IR Remote Control driver registered, " - "major %d \n", MAJOR(lirc_base_dev)); + pr_info("IR Remote Control driver registered, major %d\n", + MAJOR(lirc_base_dev)); error: return retval; @@ -828,7 +817,7 @@ static void __exit lirc_dev_exit(void) { class_destroy(lirc_class); unregister_chrdev_region(lirc_base_dev, MAX_IRCTL_DEVICES); - printk(KERN_INFO "lirc_dev: module unloaded\n"); + pr_info("module unloaded\n"); } module_init(lirc_dev_init); -- cgit v1.2.3 From 54fcecafe5737d6987c2e6e203598e4dfb5112b0 Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:17 -0300 Subject: [media] lirc_dev: simplify goto paths The code can be rearranged so that some goto paths can be removed Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 59f4c93cccc4..b11d02682d13 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -243,52 +243,44 @@ static int lirc_allocate_driver(struct lirc_driver *d) if (!d) { pr_err("driver pointer must be not NULL!\n"); - err = -EBADRQC; - goto out; + return -EBADRQC; } if (!d->dev) { pr_err("dev pointer not filled in!\n"); - err = -EINVAL; - goto out; + return -EINVAL; } if (MAX_IRCTL_DEVICES <= d->minor) { dev_err(d->dev, "minor must be between 0 and %d!\n", MAX_IRCTL_DEVICES - 1); - err = -EBADRQC; - goto out; + return -EBADRQC; } if (1 > d->code_length || (BUFLEN * 8) < d->code_length) { dev_err(d->dev, "code length must be less than %d bits\n", BUFLEN * 8); - err = -EBADRQC; - goto out; + return -EBADRQC; } if (d->sample_rate) { if (2 > d->sample_rate || HZ < d->sample_rate) { dev_err(d->dev, "invalid %d sample rate\n", d->sample_rate); - err = -EBADRQC; - goto out; + return -EBADRQC; } if (!d->add_to_buf) { dev_err(d->dev, "add_to_buf not set\n"); - err = -EBADRQC; - goto out; + return -EBADRQC; } } else if (!(d->fops && d->fops->read) && !d->rbuf) { dev_err(d->dev, "fops->read and rbuf are NULL!\n"); - err = -EBADRQC; - goto out; + return -EBADRQC; } else if (!d->rbuf) { if (!(d->fops && d->fops->read && d->fops->poll && d->fops->unlocked_ioctl)) { dev_err(d->dev, "undefined read, poll, ioctl\n"); - err = -EBADRQC; - goto out; + return -EBADRQC; } } @@ -366,7 +358,7 @@ out_sysfs: device_destroy(lirc_class, MKDEV(MAJOR(lirc_base_dev), ir->d.minor)); out_lock: mutex_unlock(&lirc_dev_lock); -out: + return err; } @@ -790,9 +782,8 @@ static int __init lirc_dev_init(void) lirc_class = class_create(THIS_MODULE, "lirc"); if (IS_ERR(lirc_class)) { - retval = PTR_ERR(lirc_class); pr_err("class_create failed\n"); - goto error; + return PTR_ERR(lirc_class); } retval = alloc_chrdev_region(&lirc_base_dev, 0, MAX_IRCTL_DEVICES, @@ -800,15 +791,14 @@ static int __init lirc_dev_init(void) if (retval) { class_destroy(lirc_class); pr_err("alloc_chrdev_region failed\n"); - goto error; + return retval; } pr_info("IR Remote Control driver registered, major %d\n", MAJOR(lirc_base_dev)); -error: - return retval; + return 0; } -- cgit v1.2.3 From 7fe579d29f66e7dee77c50f39947a70a74929f4f Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:18 -0300 Subject: [media] lirc_dev: do not use goto to create loops ... use "do .. while" instead. Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index b11d02682d13..cfa6031eeb72 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -99,18 +99,16 @@ static int lirc_add_to_buf(struct irctl *ir) { if (ir->d.add_to_buf) { int res = -ENODATA; - int got_data = 0; + int got_data = -1; /* * service the device as long as it is returning * data and we have space */ -get_data: - res = ir->d.add_to_buf(ir->d.data, ir->buf); - if (res == 0) { + do { got_data++; - goto get_data; - } + res = ir->d.add_to_buf(ir->d.data, ir->buf); + } while (!res); if (res == -ENODEV) kthread_stop(ir->task); -- cgit v1.2.3 From 19e565397cb92b0484c46c48498e0fe2d2491efa Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:19 -0300 Subject: [media] lirc_dev: simplify if statement in lirc_add_to_buf The whole function is inside an 'if' statement ("if (ir->d.add_to_buf)"). Check the opposite of that statement at the beginning and exit, this way we can have one level less of indentation. Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index cfa6031eeb72..c2826a7b70bd 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -97,26 +97,25 @@ static void lirc_irctl_cleanup(struct irctl *ir) */ static int lirc_add_to_buf(struct irctl *ir) { - if (ir->d.add_to_buf) { - int res = -ENODATA; - int got_data = -1; + int res; + int got_data = -1; - /* - * service the device as long as it is returning - * data and we have space - */ - do { - got_data++; - res = ir->d.add_to_buf(ir->d.data, ir->buf); - } while (!res); + if (!ir->d.add_to_buf) + return 0; - if (res == -ENODEV) - kthread_stop(ir->task); + /* + * service the device as long as it is returning + * data and we have space + */ + do { + got_data++; + res = ir->d.add_to_buf(ir->d.data, ir->buf); + } while (!res); - return got_data ? 0 : res; - } + if (res == -ENODEV) + kthread_stop(ir->task); - return 0; + return got_data ? 0 : res; } /* main function of the polling thread -- cgit v1.2.3 From 6ab86d2aa04375167c0e168eecb672acaf3d991a Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:20 -0300 Subject: [media] lirc_dev: remove double if ... else statement There are two if ... else which check the same thing in different part of the code, they can be merged in a single check. Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index c2826a7b70bd..a8a5116d9669 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -310,13 +310,6 @@ static int lirc_allocate_driver(struct lirc_driver *d) irctls[minor] = ir; d->minor = minor; - if (d->sample_rate) { - ir->jiffies_to_wait = HZ / d->sample_rate; - } else { - /* it means - wait for external event in task queue */ - ir->jiffies_to_wait = 0; - } - /* some safety check 8-) */ d->name[sizeof(d->name)-1] = '\0'; @@ -330,6 +323,8 @@ static int lirc_allocate_driver(struct lirc_driver *d) "lirc%u", ir->d.minor); if (d->sample_rate) { + ir->jiffies_to_wait = HZ / d->sample_rate; + /* try to fire up polling thread */ ir->task = kthread_run(lirc_thread, (void *)ir, "lirc_dev"); if (IS_ERR(ir->task)) { @@ -338,6 +333,9 @@ static int lirc_allocate_driver(struct lirc_driver *d) err = -ECHILD; goto out_sysfs; } + } else { + /* it means - wait for external event in task queue */ + ir->jiffies_to_wait = 0; } err = lirc_cdev_add(ir); -- cgit v1.2.3 From 14db9fc2d4e50d95d7586bc6c54029afbcbdf4a1 Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:21 -0300 Subject: [media] lirc_dev: merge three if statements in only one The three if statements check the same thing, merge them in only one statement. Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index a8a5116d9669..71ff82028c80 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -270,15 +270,10 @@ static int lirc_allocate_driver(struct lirc_driver *d) dev_err(d->dev, "add_to_buf not set\n"); return -EBADRQC; } - } else if (!(d->fops && d->fops->read) && !d->rbuf) { - dev_err(d->dev, "fops->read and rbuf are NULL!\n"); + } else if (!d->rbuf && !(d->fops && d->fops->read && + d->fops->poll && d->fops->unlocked_ioctl)) { + dev_err(d->dev, "undefined read, poll, ioctl\n"); return -EBADRQC; - } else if (!d->rbuf) { - if (!(d->fops && d->fops->read && d->fops->poll && - d->fops->unlocked_ioctl)) { - dev_err(d->dev, "undefined read, poll, ioctl\n"); - return -EBADRQC; - } } mutex_lock(&lirc_dev_lock); -- cgit v1.2.3 From 9675ee5a8e42807ebd33bf4934f235911b5647b0 Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:23 -0300 Subject: [media] lirc_dev: fix variable constant comparisons When comparing a variable with a constant, the comparison should start from the variable and not from the constant. It's also written in the human DNA. Swap the terms of comparisons whenever the constant comes first and fix the following checkpatch warning: WARNING: Comparisons should place the constant on the right side of the test Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 71ff82028c80..2bdd2c0a9268 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -248,13 +248,13 @@ static int lirc_allocate_driver(struct lirc_driver *d) return -EINVAL; } - if (MAX_IRCTL_DEVICES <= d->minor) { + 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 (1 > d->code_length || (BUFLEN * 8) < d->code_length) { + 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); return -EBADRQC; @@ -285,7 +285,7 @@ static int lirc_allocate_driver(struct lirc_driver *d) for (minor = 0; minor < MAX_IRCTL_DEVICES; minor++) if (!irctls[minor]) break; - if (MAX_IRCTL_DEVICES == minor) { + if (minor == MAX_IRCTL_DEVICES) { dev_err(d->dev, "no free slots for drivers!\n"); err = -ENOMEM; goto out_lock; -- cgit v1.2.3 From b408809487e0b80fdd7869f92b5ca3be55923e9d Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:24 -0300 Subject: [media] lirc_dev: fix error return value If ioctl is called, it cannot be a case of invalid system call number (ENOSYS), that is a ENOTTY case which means that the device doesn't support that specific ioctl command. So, replace ENOSYS with ENOTTY. Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 2bdd2c0a9268..64a3d93a21ef 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -585,7 +585,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg) break; case LIRC_GET_REC_MODE: if (!(ir->d.features & LIRC_CAN_REC_MASK)) { - result = -ENOSYS; + result = -ENOTTY; break; } @@ -595,7 +595,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg) break; case LIRC_SET_REC_MODE: if (!(ir->d.features & LIRC_CAN_REC_MASK)) { - result = -ENOSYS; + result = -ENOTTY; break; } @@ -613,7 +613,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg) case LIRC_GET_MIN_TIMEOUT: if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) || ir->d.min_timeout == 0) { - result = -ENOSYS; + result = -ENOTTY; break; } @@ -622,7 +622,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg) case LIRC_GET_MAX_TIMEOUT: if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) || ir->d.max_timeout == 0) { - result = -ENOSYS; + result = -ENOTTY; break; } -- cgit v1.2.3 From 62e92682c08885e91113ba742573e736c85db0e4 Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:25 -0300 Subject: [media] lirc_dev: extremely trivial comment style fix Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 64a3d93a21ef..4be447070fc4 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -690,7 +690,8 @@ ssize_t lirc_dev_fop_read(struct file *file, /* According to the read(2) man page, 'written' can be * returned as less than 'length', instead of blocking * again, returning -EWOULDBLOCK, or returning - * -ERESTARTSYS */ + * -ERESTARTSYS + */ if (written) break; if (file->f_flags & O_NONBLOCK) { -- cgit v1.2.3 From 8e5fa4c67d6387d40e2ae350e96fca939ba5e681 Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:26 -0300 Subject: [media] lirc_dev: fix potential segfault When opening or closing a lirc character device, the framework provides to the user the possibility to keep track of opening or closing of the device by calling two functions: - set_use_inc() when opening the device - set_use_dec() when closing the device if those are not set by the lirc user, the system segfaults. Check the pointer value before calling the above functions. Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 4be447070fc4..a107ab6bcb88 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -411,7 +411,10 @@ int lirc_unregister_driver(int minor) ir->d.name, ir->d.minor); wake_up_interruptible(&ir->buf->wait_poll); mutex_lock(&ir->irctl_lock); - ir->d.set_use_dec(ir->d.data); + + if (ir->d.set_use_dec) + ir->d.set_use_dec(ir->d.data); + module_put(cdev->owner); mutex_unlock(&ir->irctl_lock); } else { @@ -469,7 +472,8 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file) cdev = ir->cdev; if (try_module_get(cdev->owner)) { ir->open++; - retval = ir->d.set_use_inc(ir->d.data); + if (ir->d.set_use_inc) + retval = ir->d.set_use_inc(ir->d.data); if (retval) { module_put(cdev->owner); @@ -510,7 +514,8 @@ int lirc_dev_fop_close(struct inode *inode, struct file *file) ir->open--; if (ir->attached) { - ir->d.set_use_dec(ir->d.data); + if (ir->d.set_use_dec) + ir->d.set_use_dec(ir->d.data); module_put(cdev->owner); } else { lirc_irctl_cleanup(ir); -- cgit v1.2.3 From 273b902a5b1bfd6977a73c4de3eb96db3cb103cb Mon Sep 17 00:00:00 2001 From: Andi Shyti Date: Wed, 6 Jul 2016 06:01:27 -0300 Subject: [media] lirc_dev: use LIRC_CAN_REC() define to check if the device can receive The LIRC_CAN_REC() returns a boolean "flag & LIRC_CAN_REC_MASK" to check whether the device can receive data. Signed-off-by: Andi Shyti Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/lirc_dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/media/rc/lirc_dev.c') diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index a107ab6bcb88..91f9bb87ce68 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -589,7 +589,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg) result = put_user(ir->d.features, (__u32 __user *)arg); break; case LIRC_GET_REC_MODE: - if (!(ir->d.features & LIRC_CAN_REC_MASK)) { + if (LIRC_CAN_REC(ir->d.features)) { result = -ENOTTY; break; } @@ -599,7 +599,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg) (__u32 __user *)arg); break; case LIRC_SET_REC_MODE: - if (!(ir->d.features & LIRC_CAN_REC_MASK)) { + if (LIRC_CAN_REC(ir->d.features)) { result = -ENOTTY; break; } -- cgit v1.2.3