[PATCH 00/15] lirc_dev fixes and beautification

[PATCH 00/15] lirc_dev fixes and beautification

[Andi Shyti]

Hi,

because I wanted to add three ioctl commands in lirc, I ended up
with the patchset below.

This is a collection of fixes, added functionality, coding rework
and trivial coding style fixes.

The first patch is preparatory to the second, which allows the
user to create a lirc driver without receiver buffer, which is
obvious for transmitters. Besides, even though that buffer could
have been used also by transmitters, drivers might have the need
to handle it separately.

The rest of the patches is a series of coding style and code
rework, as I said, some of them are very trivial, but I sent them
anyway because I was on fire.

Patch 14 is a segfault fix, while the last patch adds the
possibility to send to ioctl the set frequency, get frequency and
set length command.

Thanks,
Andi

Andi Shyti (15):
  lirc_dev: place buffer allocation on separate function
  lirc_dev: allow bufferless driver registration
  lirc_dev: remove unnecessary debug prints
  lirc_dev: replace printk with pr_* or dev_*
  lirc_dev: simplify goto paths
  lirc_dev: do not use goto to create loops
  lirc_dev: simplify if statement in lirc_add_to_buf
  lirc_dev: remove double if ... else statement
  lirc_dev: merge three if statements in only one
  lirc_dev: remove CONFIG_COMPAT precompiler check
  lirc_dev: fix variable constant comparisons
  lirc_dev: fix error return value
  lirc_dev: extremely trivial comment style fix
  lirc_dev: fix potential segfault
  include: lirc: add set length and frequency ioctl options

 drivers/media/rc/lirc_dev.c | 297 +++++++++++++++++++++-----------------------
 include/media/lirc_dev.h    |  12 ++
 include/uapi/linux/lirc.h   |   4 +
 3 files changed, 156 insertions(+), 157 deletions(-)

-- 
2.8.1

[PATCH 01/15] lirc_dev: place buffer allocation on separate function

[Andi Shyti]

During the driver registration, move the buffer allocation on a
separate function.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 57 +++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 92ae190..5716978 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;
-- 
2.8.1

[PATCH 02/15] lirc_dev: allow bufferless driver registration

[Andi Shyti]

Some drivers don't necessarily need to have a FIFO managed buffer
for their transfers. Drivers now should call
lirc_register_bufferless_driver in order to handle the buffer
themselves.

The function works exaclty like lirc_register_driver except of
the buffer allocation.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 44 ++++++++++++++++++++++++++++++++++----------
 include/media/lirc_dev.h    | 12 ++++++++++++
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 5716978..fa562a3 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,8 +388,29 @@ out_lock:
 out:
 	return err;
 }
+
+int lirc_register_driver(struct lirc_driver *d)
+{
+	int err, minor;
+
+	minor = lirc_allocate_driver(d);
+	if (minor < 0)
+		return minor;
+
+	err = lirc_allocate_buffer(irctls[minor]);
+	if (err)
+		lirc_unregister_driver(minor);
+
+	return err ? err : minor;
+}
 EXPORT_SYMBOL(lirc_register_driver);
 
+int lirc_register_bufferless_driver(struct lirc_driver *d)
+{
+	return lirc_allocate_driver(d);
+}
+EXPORT_SYMBOL(lirc_register_bufferless_driver);
+
 int lirc_unregister_driver(int minor)
 {
 	struct irctl *ir;
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 0ab59a5..8bed57a 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -214,6 +214,18 @@ struct lirc_driver {
  */
 extern int lirc_register_driver(struct lirc_driver *d);
 
+/* int lirc_register_bufferless_driver - allocates a lirc bufferless driver
+ * @d: reference to the lirc_driver to initialize
+ *
+ * The difference between lirc_register_driver and
+ * lirc_register_bufferless_driver is that the latter doesn't allocate any
+ * buffer, which means that the driver using the lirc_driver should take care of
+ * it by itself.
+ *
+ * returns 0 on success or a the negative errno number in case of failure.
+ */
+extern int lirc_register_bufferless_driver(struct lirc_driver *d);
+
 /* returns negative value on error or 0 if success
 */
 extern int lirc_unregister_driver(int minor);
-- 
2.8.1

[PATCH 03/15] lirc_dev: remove unnecessary debug prints

[Andi Shyti]

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index fa562a3..ee997ab 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: "
@@ -525,10 +515,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);
@@ -550,8 +536,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);
 
@@ -586,8 +570,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;
 
@@ -683,9 +665,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;
@@ -790,8 +769,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 ? "<fail>" : "<ok>", ret);
 
 	return ret ? ret : written;
 }
@@ -814,8 +791,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;
 
-- 
2.8.1

[PATCH 04/15] lirc_dev: replace printk with pr_* or dev_*

[Andi Shyti]

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 <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 76 +++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 44 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index ee997ab..b11ab5c 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -240,59 +240,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("lirc_dev: 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("%s: dev pointer not filled in!\n", __func__);
 		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 +300,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 +342,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;
 		}
@@ -407,15 +396,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("lirc_dev: %s: minor (%d) must be between 0 and %d!\n",
+				__func__, 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("lirc_dev: %s: failed to get irctl\n", __func__);
 		return -ENOENT;
 	}
 
@@ -424,8 +412,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;
 	}
@@ -467,7 +455,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",
+		pr_err("lirc_dev [%d]: open result = -ENODEV\n",
 		       iminor(inode));
 		return -ENODEV;
 	}
@@ -530,7 +518,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("%s: called with invalid irctl\n", __func__);
 		return -EINVAL;
 	}
 
@@ -566,7 +554,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("%s: called with invalid irctl\n", __func__);
 		return POLLERR;
 	}
 
@@ -597,7 +585,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("lirc_dev: %s: no irctl found!\n", __func__);
 		return -ENODEV;
 	}
 
@@ -605,7 +593,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;
 	}
@@ -682,7 +670,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("%s: called with invalid irctl\n", __func__);
 		return -ENODEV;
 	}
 
@@ -787,7 +775,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("%s: called with invalid irctl\n", __func__);
 		return -ENODEV;
 	}
 
@@ -806,7 +794,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("lirc_dev: class_create failed\n");
 		goto error;
 	}
 
@@ -814,13 +802,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("lirc_dev: 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("lirc_dev: IR Remote Control driver registered, major %d\n",
+							MAJOR(lirc_base_dev));
 
 error:
 	return retval;
@@ -832,7 +820,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("lirc_dev: module unloaded\n");
 }
 
 module_init(lirc_dev_init);
-- 
2.8.1

[PATCH 05/15] lirc_dev: simplify goto paths

[Andi Shyti]

The code can be rearranged so that some goto paths can be removed

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index b11ab5c..400ab80 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -241,52 +241,44 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 
 	if (!d) {
 		pr_err("lirc_dev: driver pointer must be not NULL!\n");
-		err = -EBADRQC;
-		goto out;
+		return -EBADRQC;
 	}
 
 	if (!d->dev) {
 		pr_err("%s: dev pointer not filled in!\n", __func__);
-		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;
 		}
 	}
 
@@ -364,7 +356,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;
 }
 
@@ -793,9 +785,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("lirc_dev: class_create failed\n");
-		goto error;
+		return PTR_ERR(lirc_class);
 	}
 
 	retval = alloc_chrdev_region(&lirc_base_dev, 0, MAX_IRCTL_DEVICES,
@@ -803,15 +794,14 @@ static int __init lirc_dev_init(void)
 	if (retval) {
 		class_destroy(lirc_class);
 		pr_err("lirc_dev: alloc_chrdev_region failed\n");
-		goto error;
+		return retval;
 	}
 
 
 	pr_info("lirc_dev: IR Remote Control driver registered, major %d\n",
 							MAJOR(lirc_base_dev));
 
-error:
-	return retval;
+	return 0;
 }
 
 
-- 
2.8.1

[PATCH 06/15] lirc_dev: do not use goto to create loops

[Andi Shyti]

... use "do .. while" instead.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 400ab80..cc00b9a 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -103,12 +103,11 @@ static int lirc_add_to_buf(struct irctl *ir)
 		 * 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) {
-			got_data++;
-			goto get_data;
-		}
+		do {
+			res = ir->d.add_to_buf(ir->d.data, ir->buf);
+			if (!res)
+				got_data++;
+		} while (!res);
 
 		if (res == -ENODEV)
 			kthread_stop(ir->task);
-- 
2.8.1

[PATCH 07/15] lirc_dev: simplify if statement in lirc_add_to_buf

[Andi Shyti]

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 <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index cc00b9a..d63ff85 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -95,27 +95,26 @@ 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 = 0;
+	int res;
+	int got_data = 0;
 
-		/*
-		 * service the device as long as it is returning
-		 * data and we have space
-		 */
-		do {
-			res = ir->d.add_to_buf(ir->d.data, ir->buf);
-			if (!res)
-				got_data++;
-		} 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 {
+		res = ir->d.add_to_buf(ir->d.data, ir->buf);
+		if (!res)
+			got_data++;
+	} 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
-- 
2.8.1

[PATCH 08/15] lirc_dev: remove double if ... else statement

[Andi Shyti]

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 <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index d63ff85..7dff92c 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -309,13 +309,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';
 
@@ -329,6 +322,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)) {
@@ -337,6 +332,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);
-- 
2.8.1

[PATCH 09/15] lirc_dev: merge three if statements in only one

[Andi Shyti]

The three if statements check the same thing, merge them in only
one statement.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 7dff92c..0c26609 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -269,15 +269,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);
-- 
2.8.1

[PATCH 10/15] lirc_dev: remove CONFIG_COMPAT precompiler check

[Andi Shyti]

There is no need to check in precompilation whether the ioctl is
compat or unlocked, depending on the configuration it will be
called the correct one.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 0c26609..c11cfc0 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -149,9 +149,7 @@ static const struct file_operations lirc_dev_fops = {
 	.write		= lirc_dev_fop_write,
 	.poll		= lirc_dev_fop_poll,
 	.unlocked_ioctl	= lirc_dev_fop_ioctl,
-#ifdef CONFIG_COMPAT
 	.compat_ioctl	= lirc_dev_fop_ioctl,
-#endif
 	.open		= lirc_dev_fop_open,
 	.release	= lirc_dev_fop_close,
 	.llseek		= noop_llseek,
-- 
2.8.1

[PATCH 11/15] lirc_dev: fix variable constant comparisons

[Andi Shyti]

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 <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index c11cfc0..7e5cb85 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -245,13 +245,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;
@@ -282,7 +282,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;
-- 
2.8.1

[PATCH 12/15] lirc_dev: fix error return value

[Andi Shyti]

If ioctl is called, it cannot be a case of invalid system call
number (ENOSYS), that is an operation not permitted (EPERM).
Replace ENOSYS with EPERM.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 7e5cb85..6f3402c 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -587,7 +587,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 = -EPERM;
 			break;
 		}
 
@@ -597,7 +597,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 = -EPERM;
 			break;
 		}
 
@@ -615,7 +615,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 = -EPERM;
 			break;
 		}
 
@@ -624,7 +624,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 = -EPERM;
 			break;
 		}
 
-- 
2.8.1

[PATCH 13/15] lirc_dev: extremely trivial comment style fix

[Andi Shyti]

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 6f3402c..0a3d65d 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -692,7 +692,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) {
-- 
2.8.1

[PATCH 14/15] lirc_dev: fix potential segfault

[Andi Shyti]

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 <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 0a3d65d..58dabdc 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -412,7 +412,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 {
@@ -471,7 +474,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);
@@ -512,7 +516,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);
-- 
2.8.1

[PATCH 15/15] include: lirc: add set length and frequency ioctl options

[Andi Shyti]

The Lirc framework works mainly with receivers, but there is
nothing that prevents us from using it for transmitters as well.

For that we need to have more control on the device frequency to
set (which is a new concept fro LIRC) and we also need to provide
to userspace, as feedback, the values of the used frequency and
length.

Add the LIRC_SET_LENGTH, LIRC_GET_FREQUENCY and
LIRC_SET_FREQUENCY ioctl commands in order to allow the above
mentioned operations.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 include/uapi/linux/lirc.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/lirc.h b/include/uapi/linux/lirc.h
index 4b3ab29..94a0d8c 100644
--- a/include/uapi/linux/lirc.h
+++ b/include/uapi/linux/lirc.h
@@ -106,6 +106,7 @@
 
 /* code length in bits, currently only for LIRC_MODE_LIRCCODE */
 #define LIRC_GET_LENGTH                _IOR('i', 0x0000000f, __u32)
+#define LIRC_SET_LENGTH                _IOW('i', 0x00000010, __u32)
 
 #define LIRC_SET_SEND_MODE             _IOW('i', 0x00000011, __u32)
 #define LIRC_SET_REC_MODE              _IOW('i', 0x00000012, __u32)
@@ -165,4 +166,7 @@
 
 #define LIRC_SET_WIDEBAND_RECEIVER     _IOW('i', 0x00000023, __u32)
 
+#define LIRC_GET_FREQUENCY             _IOR('i', 0x00000024, __u32)
+#define LIRC_SET_FREQUENCY             _IOW('i', 0x00000025, __u32)
+
 #endif
-- 
2.8.1

Re: [PATCH 15/15] include: lirc: add set length and frequency ioctl options

[Sean Young]

On Wed, Jun 29, 2016 at 10:20:44PM +0900, Andi Shyti wrote:
> The Lirc framework works mainly with receivers, but there is
> nothing that prevents us from using it for transmitters as well.

The lirc interface already provides for transmitting IR.

> For that we need to have more control on the device frequency to
> set (which is a new concept fro LIRC) and we also need to provide
> to userspace, as feedback, the values of the used frequency and
> length.

Please can you elaborate on what exactly you mean by frequency and
length.

The carrier frequency can already be set with LIRC_SET_SEND_CARRIER.

> Add the LIRC_SET_LENGTH, LIRC_GET_FREQUENCY and
> LIRC_SET_FREQUENCY ioctl commands in order to allow the above
> mentioned operations.

You're also adding ioctls without any drivers implementing them
unless I missed something.

> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  include/uapi/linux/lirc.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/uapi/linux/lirc.h b/include/uapi/linux/lirc.h
> index 4b3ab29..94a0d8c 100644
> --- a/include/uapi/linux/lirc.h
> +++ b/include/uapi/linux/lirc.h
> @@ -106,6 +106,7 @@
>  
>  /* code length in bits, currently only for LIRC_MODE_LIRCCODE */
>  #define LIRC_GET_LENGTH                _IOR('i', 0x0000000f, __u32)
> +#define LIRC_SET_LENGTH                _IOW('i', 0x00000010, __u32)

The LIRC_GET_LENGTH is specific to LIRCCODE encoding. Why are you
adding it here?

>  
>  #define LIRC_SET_SEND_MODE             _IOW('i', 0x00000011, __u32)
>  #define LIRC_SET_REC_MODE              _IOW('i', 0x00000012, __u32)
> @@ -165,4 +166,7 @@
>  
>  #define LIRC_SET_WIDEBAND_RECEIVER     _IOW('i', 0x00000023, __u32)
>  
> +#define LIRC_GET_FREQUENCY             _IOR('i', 0x00000024, __u32)
> +#define LIRC_SET_FREQUENCY             _IOW('i', 0x00000025, __u32)
> +
>  #endif
> -- 
> 2.8.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 15/15] include: lirc: add set length and frequency ioctl options

[Andi Shyti]

Hi Sean,

> > For that we need to have more control on the device frequency to
> > set (which is a new concept fro LIRC) and we also need to provide
> > to userspace, as feedback, the values of the used frequency and
> > length.
> 
> Please can you elaborate on what exactly you mean by frequency and
> length.
> 
> The carrier frequency can already be set with LIRC_SET_SEND_CARRIER.

yes, I mean carrier's frequency. I didn't understand that
LIRC_SET_SEND_CARRIER was related to the frequency.

> > Add the LIRC_SET_LENGTH, LIRC_GET_FREQUENCY and
> > LIRC_SET_FREQUENCY ioctl commands in order to allow the above
> > mentioned operations.
> 
> You're also adding ioctls without any drivers implementing them
> unless I missed something.

You're right; the first idea was to submit also the device
driver, but then I decided to keep it separate from this
patchset.
Anyway, we can drop this one (it's the last of the series) and,
in case it will be needed after the above comment, I will re-send
it with the driver.

Thanks,
Andi

Re: [PATCH 04/15] lirc_dev: replace printk with pr_* or dev_*

[Joe Perches]

On Wed, 2016-06-29 at 22:20 +0900, Andi Shyti wrote:
> 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:

Adding

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

before any #include would allow these to be prefixed
automatically and allow the embedded prefixes to be removed.
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
[]
> @@ -240,59 +240,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("lirc_dev: driver pointer must be not NULL!\n");
>  		err = -EBADRQC;
>  		goto out;
>  	}

		pr_err("driver pointer must not be NULL!\n");

And typical multiple line statement alignment is to
the open parenthesis.