[PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[Andi Shyti]

The ir-spi is a simple device driver which supports the
connection between an IR LED and the MOSI line of an SPI device.

The driver, indeed, uses the SPI framework to stream the raw data
provided by userspace through a character device. The chardev is
handled by the LIRC framework and its functionality basically
provides:

 - raw write: data to be sent to the SPI and then streamed to the
   MOSI line;
 - set frequency: sets the frequency whith which the data should
   be sent;
 - set length: sets the data length. This information is
   optional, if the length is set, then userspace should send raw
   data only with that length; while if the length is set to '0',
   then the driver will figure out himself the length of the data
   based on the length of the data written on the character
   device.
   The latter is not recommended, though, as the driver, at
   any write, allocates and deallocates a buffer where the data
   from userspace are stored.

The driver provides three feedback commands:

 - get length: reads the length set and (as mentioned), if the
   length is '0' it will be calculated at any write
 - get frequency: the driver reports the frequency. If userpace
   doesn't set the frequency, the driver will use a default value
   of 38000Hz.

The character device is created under /dev/lircX name, where X is
and ID assigned by the LIRC framework.

Example of usage:

        int fd, ret;
        ssize_t n;
        uint32_t val = 0;

        fd = open("/dev/lirc0", O_RDWR);
        if (fd < 0) {
                fprintf(stderr, "unable to open the device\n");
                return -1;
        }

        /* ioctl set frequency and length parameters */
        val = 6430;
        ret = ioctl(fd, LIRC_SET_LENGTH, &val);
        if (ret < 0)
                fprintf(stderr, "LIRC_SET_LENGTH failed\n");
        val = 608000;
        ret = ioctl(fd, LIRC_SET_FREQUENCY, &val);
        if (ret < 0)
                fprintf(stderr, "LIRC_SET_FREQUENCY failed\n");

        /* read back length and frequency parameters */
        ret = ioctl(fd, LIRC_GET_LENGTH, &val);
        if (ret < 0)
                fprintf(stderr, "LIRC_GET_LENGTH failed\n");
        else
                fprintf(stdout, "legnth = %u\n", val);

        ret = ioctl(fd, LIRC_GET_FREQUENCY, &val);
        if (ret < 0)
                fprintf(stderr, "LIRC_GET_FREQUENCY failed\n");
        else
                fprintf(stdout, "frequency = %u\n", val);

        /* write data to device */
        n = write(fd, b, 6430);
        if (n < 0) {
                fprintf(stderr, "unable to write to the device\n");
                ret = -1;
        } else if (n != 6430) {
                fprintf(stderr, "failed to write everything, wrote %ld instead\n", n);
                ret = -1;
        } else {
                fprintf(stdout, "written all the %ld data\n", n);
        }

        close(fd);

The driver supports multi task access, but all the processes
which hold the driver should use the same length and frequency
parameters.

Change-Id: I323d7dd4a56d6dcf48f2c695293822eb04bdb85f
Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 Documentation/devicetree/bindings/media/spi-ir.txt |  24 ++
 drivers/media/rc/Kconfig                           |   9 +
 drivers/media/rc/Makefile                          |   1 +
 drivers/media/rc/ir-spi.c                          | 301 +++++++++++++++++++++
 4 files changed, 335 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
 create mode 100644 drivers/media/rc/ir-spi.c

diff --git a/Documentation/devicetree/bindings/media/spi-ir.txt b/Documentation/devicetree/bindings/media/spi-ir.txt
new file mode 100644
index 0000000..2232d92
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/spi-ir.txt
@@ -0,0 +1,24 @@
+Device tree bindings for IR LED connected through SPI bus which is used as
+remote controller.
+
+The IR LED switch is connected to the MOSI line of the SPI device and the data
+are delivered thourgh that.
+
+Required properties:
+	- compatible: should be "ir-spi"
+
+Optional properties:
+	- irled,switch: specifies the gpio switch which enables the irled
+
+Example:
+
+        irled@0 {
+                compatible = "ir-spi";
+                reg = <0x0>;
+                spi-max-frequency = <5000000>;
+                irled,switch = <&gpr3 3 0>;
+
+                controller-data {
+                        samsung,spi-feedback-delay = <0>;
+                };
+        };
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index bd4d685..dacaa29 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -261,6 +261,15 @@ config IR_REDRAT3
 	   To compile this driver as a module, choose M here: the
 	   module will be called redrat3.
 
+config IR_SPI
+	tristate "SPI connected IR LED"
+	depends on SPI && LIRC
+	---help---
+	  Say Y if you want to use an IR LED connected through SPI bus.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ir-spi.
+
 config IR_STREAMZAP
 	tristate "Streamzap PC Remote IR Receiver"
 	depends on USB_ARCH_HAS_HCD
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 379a5c0..1417c8d 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
 obj-$(CONFIG_IR_ENE) += ene_ir.o
 obj-$(CONFIG_IR_REDRAT3) += redrat3.o
 obj-$(CONFIG_IR_RX51) += ir-rx51.o
+obj-$(CONFIG_IR_SPI) += ir-spi.o
 obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
 obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
 obj-$(CONFIG_RC_LOOPBACK) += rc-loopback.o
diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
new file mode 100644
index 0000000..6eb14e9
--- /dev/null
+++ b/drivers/media/rc/ir-spi.c
@@ -0,0 +1,301 @@
+/*
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Author: Andi Shyti <andi.shyti@samsung.it>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * SPI driven IR LED device driver
+ */
+
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <media/lirc_dev.h>
+
+#define IR_SPI_DRIVER_NAME		"ir-spi"
+
+#define IR_SPI_DEFAULT_FREQUENCY	38000
+#define IR_SPI_BIT_PER_WORD		    8
+
+struct ir_spi_data {
+	u16 nusers;
+	int power_gpio;
+
+	u8 *buffer;
+
+	struct lirc_driver lirc_driver;
+	struct spi_device *spi;
+	struct spi_transfer xfer;
+	struct mutex mutex;
+	struct regulator *regulator;
+};
+
+static ssize_t ir_spi_chardev_write(struct file *file,
+					const char __user *buffer,
+					size_t length, loff_t *offset)
+{
+	struct ir_spi_data *idata = file->private_data;
+	bool please_free = false;
+	int ret = 0;
+
+	if (idata->xfer.len && (idata->xfer.len != length))
+		return -EINVAL;
+
+	mutex_lock(&idata->mutex);
+
+	if (!idata->xfer.len) {
+		idata->buffer = kmalloc(length, GFP_KERNEL);
+
+		if (!idata->buffer) {
+			ret = -ENOMEM;
+			goto out_unlock;
+		}
+
+		idata->xfer.len = length;
+		please_free = true;
+	}
+
+	if (copy_from_user(idata->buffer, buffer, length)) {
+		ret = -EFAULT;
+		goto out_free;
+	}
+
+	ret = regulator_enable(idata->regulator);
+	if (ret) {
+		dev_err(&idata->spi->dev, "failed to power on the LED\n");
+		goto out_free;
+	}
+
+	idata->xfer.tx_buf = idata->buffer;
+
+	ret = spi_sync_transfer(idata->spi, &idata->xfer, 1);
+	if (ret)
+		dev_err(&idata->spi->dev, "unable to deliver the signal\n");
+
+	regulator_disable(idata->regulator);
+
+out_free:
+	if (please_free) {
+		kfree(idata->buffer);
+		idata->xfer.len = 0;
+		idata->buffer = NULL;
+	}
+
+out_unlock:
+	mutex_unlock(&idata->mutex);
+
+	return ret ? ret : length;
+}
+
+static int ir_spi_chardev_open(struct inode *inode, struct file *file)
+{
+	struct ir_spi_data *idata = lirc_get_pdata(file);
+
+	if (unlikely(idata->nusers >= SHRT_MAX)) {
+		dev_err(&idata->spi->dev, "device busy\n");
+		return -EBUSY;
+	}
+
+	file->private_data = idata;
+
+	mutex_lock(&idata->mutex);
+	idata->nusers++;
+	mutex_unlock(&idata->mutex);
+
+	return 0;
+}
+
+static int ir_spi_chardev_close(struct inode *inode, struct file *file)
+{
+	struct ir_spi_data *idata = lirc_get_pdata(file);
+
+	mutex_lock(&idata->mutex);
+	idata->nusers--;
+
+	/*
+	 * check if someone else is using the driver,
+	 * if not, then:
+	 *
+	 *  - reset length and frequency values to default
+	 *  - shut down the LED
+	 *  - free the buffer (NULL or ZERO_SIZE_PTR are noop)
+	 */
+	if (!idata->nusers) {
+		idata->xfer.len = 0;
+		idata->xfer.speed_hz = IR_SPI_DEFAULT_FREQUENCY;
+
+		kfree(idata->buffer);
+		idata->buffer = NULL;
+	}
+
+	mutex_unlock(&idata->mutex);
+
+	return 0;
+}
+
+static long ir_spi_chardev_ioctl(struct file *file, unsigned int cmd,
+						unsigned long arg)
+{
+	__u32 p;
+	s32 ret;
+	struct ir_spi_data *idata = file->private_data;
+
+	switch (cmd) {
+	case LIRC_GET_FEATURES:
+		return put_user(idata->lirc_driver.features,
+					(__u32 __user *) arg);
+
+	case LIRC_GET_LENGTH:
+		return put_user(idata->xfer.len, (__u32 __user *) arg);
+
+	case LIRC_SET_LENGTH: {
+		void *new;
+
+		ret = get_user(p, (__u32 __user *) arg);
+		if (ret)
+			return ret;
+
+		/*
+		 * the user is trying to set the same
+		 * length of the current value
+		 */
+		if (idata->xfer.len == p)
+			return 0;
+
+		/*
+		 * multiple users should use the driver with the
+		 * length, otherwise return EPERM same data
+		 */
+		if (idata->nusers > 1)
+			return -EPERM;
+
+		/*
+		 * if the buffer is already allocated, reallocate it with the
+		 * desired value. If the desired value is 0, then the buffer is
+		 * freed from krealloc()
+		 */
+		if (idata->xfer.len)
+			new = krealloc(idata->buffer, p, GFP_KERNEL);
+		else
+			new = kmalloc(p, GFP_KERNEL);
+
+		if (!new)
+			return -ENOMEM;
+
+		mutex_lock(&idata->mutex);
+		idata->buffer = new;
+		idata->xfer.len = p;
+		mutex_unlock(&idata->mutex);
+
+		return 0;
+	}
+
+	case LIRC_GET_SEND_CARRIER:
+		return put_user(idata->xfer.speed_hz, (__u32 __user *) arg);
+
+	case LIRC_SET_SEND_CARRIER:
+		ret = get_user(p, (__u32 __user *) arg);
+		if (ret)
+			return ret;
+
+		/*
+		 * The frequency cannot be obviously set to '0',
+		 * while, as in the case of the data length,
+		 * multiple users should use the driver with the same
+		 * frequency value, otherwise return EPERM
+		 */
+		if (!p || ((idata->nusers > 1) && p != idata->xfer.speed_hz))
+			return -EPERM;
+
+		mutex_lock(&idata->mutex);
+		idata->xfer.speed_hz = p;
+		mutex_unlock(&idata->mutex);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static const struct file_operations ir_spi_fops = {
+	.owner   = THIS_MODULE,
+	.read    = lirc_dev_fop_read,
+	.write   = ir_spi_chardev_write,
+	.poll    = lirc_dev_fop_poll,
+	.open    = ir_spi_chardev_open,
+	.release = ir_spi_chardev_close,
+	.llseek  = noop_llseek,
+	.unlocked_ioctl = ir_spi_chardev_ioctl,
+	.compat_ioctl   = ir_spi_chardev_ioctl,
+};
+
+static int ir_spi_probe(struct spi_device *spi)
+{
+	struct ir_spi_data *idata;
+
+	idata = devm_kzalloc(&spi->dev, sizeof(*idata), GFP_KERNEL);
+	if (!idata)
+		return -ENOMEM;
+
+	idata->regulator = devm_regulator_get(&spi->dev, "irda_regulator");
+	if (IS_ERR(idata->regulator))
+		return PTR_ERR(idata->regulator);
+
+	snprintf(idata->lirc_driver.name, sizeof(idata->lirc_driver.name),
+							IR_SPI_DRIVER_NAME);
+	idata->lirc_driver.features    = LIRC_CAN_SEND_RAW;
+	idata->lirc_driver.code_length = 1;
+	idata->lirc_driver.fops        = &ir_spi_fops;
+	idata->lirc_driver.dev         = &spi->dev;
+	idata->lirc_driver.data        = idata;
+	idata->lirc_driver.owner       = THIS_MODULE;
+	idata->lirc_driver.minor       = -1;
+
+	idata->lirc_driver.minor = lirc_register_driver(&idata->lirc_driver);
+	if (idata->lirc_driver.minor < 0) {
+		dev_err(&spi->dev, "unable to generate character device\n");
+		return idata->lirc_driver.minor;
+	}
+
+	mutex_init(&idata->mutex);
+
+	idata->spi = spi;
+
+	idata->xfer.bits_per_word = IR_SPI_BIT_PER_WORD;
+	idata->xfer.speed_hz = IR_SPI_DEFAULT_FREQUENCY;
+
+	return 0;
+}
+
+static int ir_spi_remove(struct spi_device *spi)
+{
+	struct ir_spi_data *idata = spi_get_drvdata(spi);
+
+	lirc_unregister_driver(idata->lirc_driver.minor);
+
+	return 0;
+}
+
+static const struct of_device_id ir_spi_of_match[] = {
+	{ .compatible = "ir-spi" },
+	{},
+};
+
+static struct spi_driver ir_spi_driver = {
+	.probe = ir_spi_probe,
+	.remove = ir_spi_remove,
+	.driver = {
+		.name = IR_SPI_DRIVER_NAME,
+		.of_match_table = ir_spi_of_match,
+	},
+};
+
+module_spi_driver(ir_spi_driver);
+
+MODULE_AUTHOR("Andi Shyti <andi.shyti@samsung.com>");
+MODULE_DESCRIPTION("SPI IR LED");
+MODULE_LICENSE("GPL v2");
-- 
2.8.1

Re: [PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[Sean Young]

On Fri, Jul 01, 2016 at 05:33:42PM +0900, Andi Shyti wrote:
> The ir-spi is a simple device driver which supports the
> connection between an IR LED and the MOSI line of an SPI device.
> 
> The driver, indeed, uses the SPI framework to stream the raw data
> provided by userspace through a character device. The chardev is
> handled by the LIRC framework and its functionality basically
> provides:
> 
>  - raw write: data to be sent to the SPI and then streamed to the
>    MOSI line;
>  - set frequency: sets the frequency whith which the data should
>    be sent;
>  - set length: sets the data length. This information is
>    optional, if the length is set, then userspace should send raw
>    data only with that length; while if the length is set to '0',
>    then the driver will figure out himself the length of the data
>    based on the length of the data written on the character
>    device.
>    The latter is not recommended, though, as the driver, at
>    any write, allocates and deallocates a buffer where the data
>    from userspace are stored.
> 
> The driver provides three feedback commands:
> 
>  - get length: reads the length set and (as mentioned), if the
>    length is '0' it will be calculated at any write
>  - get frequency: the driver reports the frequency. If userpace
>    doesn't set the frequency, the driver will use a default value
>    of 38000Hz.

This interface is not compatible with other lirc devices; there is no
way of determining whether this is a regular lirc device or this new
flavour you've invented.

Also I don't see what justifies this new interface. This can be 
implemented in rc-core in less lines of code and it will be entirely 
compatible with existing user-space.


Sean

Re: [PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[Andi Shyti]

Hi Sean,

> > The ir-spi is a simple device driver which supports the
> > connection between an IR LED and the MOSI line of an SPI device.
> > 
> > The driver, indeed, uses the SPI framework to stream the raw data
> > provided by userspace through a character device. The chardev is
> > handled by the LIRC framework and its functionality basically
> > provides:
> > 
> >  - raw write: data to be sent to the SPI and then streamed to the
> >    MOSI line;
> >  - set frequency: sets the frequency whith which the data should
> >    be sent;
> >  - set length: sets the data length. This information is
> >    optional, if the length is set, then userspace should send raw
> >    data only with that length; while if the length is set to '0',
> >    then the driver will figure out himself the length of the data
> >    based on the length of the data written on the character
> >    device.
> >    The latter is not recommended, though, as the driver, at
> >    any write, allocates and deallocates a buffer where the data
> >    from userspace are stored.
> > 
> > The driver provides three feedback commands:
> > 
> >  - get length: reads the length set and (as mentioned), if the
> >    length is '0' it will be calculated at any write
> >  - get frequency: the driver reports the frequency. If userpace
> >    doesn't set the frequency, the driver will use a default value
> >    of 38000Hz.
> 
> This interface is not compatible with other lirc devices; there is no
> way of determining whether this is a regular lirc device or this new
> flavour you've invented.

except of the set length and get length which I'm using a bit
freely because I am dealing with devices that exchange always the
same amount of data, so that I don't need (in my case) to
pre-allocate or overallocate or runtime allocate. I don't
understand what else I invented :)

This is a simple driver which is driving an LED connected through
SPI and userspace writes raw data in it (LIRC_CAN_SEND_RAW).

> Also I don't see what justifies this new interface. This can be 
> implemented in rc-core in less lines of code and it will be entirely 
> compatible with existing user-space.

Also here I'm getting a bit confused. When I started writing
this, I didn't even know of the existence of a remote controlling
framework, but then I run across this:

"LIRC is a package that allows you to decode and send infra-red
signals of many (but not all) commonly used remote controls. "

taken from lirc.org: my case is exactly falling into this
description.

Am I missing anything?

Thanks,
Andi

Re: [PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[Sean Young]

On Fri, Jul 01, 2016 at 09:30:35PM +0900, Andi Shyti wrote:
> Hi Sean,
> 
> > > The ir-spi is a simple device driver which supports the
> > > connection between an IR LED and the MOSI line of an SPI device.
> > > 
> > > The driver, indeed, uses the SPI framework to stream the raw data
> > > provided by userspace through a character device. The chardev is
> > > handled by the LIRC framework and its functionality basically
> > > provides:
> > > 
> > >  - raw write: data to be sent to the SPI and then streamed to the
> > >    MOSI line;
> > >  - set frequency: sets the frequency whith which the data should
> > >    be sent;
> > >  - set length: sets the data length. This information is
> > >    optional, if the length is set, then userspace should send raw
> > >    data only with that length; while if the length is set to '0',
> > >    then the driver will figure out himself the length of the data
> > >    based on the length of the data written on the character
> > >    device.
> > >    The latter is not recommended, though, as the driver, at
> > >    any write, allocates and deallocates a buffer where the data
> > >    from userspace are stored.
> > > 
> > > The driver provides three feedback commands:
> > > 
> > >  - get length: reads the length set and (as mentioned), if the
> > >    length is '0' it will be calculated at any write
> > >  - get frequency: the driver reports the frequency. If userpace
> > >    doesn't set the frequency, the driver will use a default value
> > >    of 38000Hz.
> > 
> > This interface is not compatible with other lirc devices; there is no
> > way of determining whether this is a regular lirc device or this new
> > flavour you've invented.
> 
> except of the set length and get length which I'm using a bit
> freely because I am dealing with devices that exchange always the
> same amount of data, so that I don't need (in my case) to
> pre-allocate or overallocate or runtime allocate. I don't
> understand what else I invented :)

Other than the LIRC_{GET,SET}_LENGTH it might very well be compatible;
you're reusing LIRC_GET_LENGTH for a different purpose.

Is the kmalloc() really that costly that it needs to be avoided for
each transmit?

> This is a simple driver which is driving an LED connected through
> SPI and userspace writes raw data in it (LIRC_CAN_SEND_RAW).

And some odd ioctl.

> > Also I don't see what justifies this new interface. This can be 
> > implemented in rc-core in less lines of code and it will be entirely 
> > compatible with existing user-space.
> 
> Also here I'm getting a bit confused. When I started writing
> this, I didn't even know of the existence of a remote controlling
> framework, but then I run across this:
> 
> "LIRC is a package that allows you to decode and send infra-red
> signals of many (but not all) commonly used remote controls. "
> 
> taken from lirc.org: my case is exactly falling into this
> description.
> 
> Am I missing anything?

See drivers/staging/media/lirc/TODO: "All drivers should either be 
ported to ir-core, or dropped entirely".  ir-core has since been renamed 
to rc-core; it is uses for non-IR purposes like cec.

lirc exists as the user-space ABI but not it is not the preferred 
framework for kernel space.

There is one problem here. rc-core does not provide very well for
transmit-only hardware, so rc-core needs some modifications. This is
what I suggest to make it work:

1. in include/media/rc-core.h add a new entry to the enum rc_driver_type
   called "RC_DRIVER_IR_RAW_TX_ONLY" (or something like that).
2. rc_allocate_device() needs an argument "enum rc_driver_type"; in the
   case it would not allocate an input device. All drivers needs to
   pass in this argument.
3. rc_register_device() and rc_unregister_device() should not execute
   anything with to do with input devices or key maps for tx only
   devices.
4. ir_lirc_register() should not set the LIRC_CAN_REC_MODE2 feature
   or allocate an input buffer in the case of TX only device.

With these changes all you need to do in ir-spi is:

	struct rc_dev *rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX_ONLY);
	strcpy(rc->name, "IR SPI");
        rc->s_tx_carrier = ir_spi_set_tx_carrier; // write function
        rc->tx_ir = ir_spi_tx; // write function
        rc->driver_name = "ir-spi";

	rc_register_driver(rc);


I'm happy to help. 


Sean

Re: [PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[Andi Shyti]

Hi Sean,

> > > Also I don't see what justifies this new interface. This can be 
> > > implemented in rc-core in less lines of code and it will be entirely 
> > > compatible with existing user-space.
> > 
> > Also here I'm getting a bit confused. When I started writing
> > this, I didn't even know of the existence of a remote controlling
> > framework, but then I run across this:
> > 
> > "LIRC is a package that allows you to decode and send infra-red
> > signals of many (but not all) commonly used remote controls. "
> > 
> > taken from lirc.org: my case is exactly falling into this
> > description.
> > 
> > Am I missing anything?
> 
> See drivers/staging/media/lirc/TODO: "All drivers should either be 
> ported to ir-core, or dropped entirely".  ir-core has since been renamed 
> to rc-core; it is uses for non-IR purposes like cec.
> 
> lirc exists as the user-space ABI but not it is not the preferred 
> framework for kernel space.

I missed this part and now what you say makes sense.

> I'm happy to help. 

I will do as you recommend and thanks a lot, appreciated :)

Andi

Re: [PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

Hi,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.7-rc5 next-20160701]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andi-Shyti/rc-ir-spi-add-support-for-IR-LEDs-connected-with-SPI/20160702-102955
base:   git://linuxtv.org/media_tree.git master
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/media/rc/ir-spi.c: In function 'ir_spi_chardev_ioctl':
>> drivers/media/rc/ir-spi.c:156:7: error: 'LIRC_SET_LENGTH' undeclared (first use in this function)
     case LIRC_SET_LENGTH: {
          ^~~~~~~~~~~~~~~
   drivers/media/rc/ir-spi.c:156:7: note: each undeclared identifier is reported only once for each function it appears in

vim +/LIRC_SET_LENGTH +156 drivers/media/rc/ir-spi.c

   150			return put_user(idata->lirc_driver.features,
   151						(__u32 __user *) arg);
   152	
   153		case LIRC_GET_LENGTH:
   154			return put_user(idata->xfer.len, (__u32 __user *) arg);
   155	
 > 156		case LIRC_SET_LENGTH: {
   157			void *new;
   158	
   159			ret = get_user(p, (__u32 __user *) arg);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 55044 bytes --]

Re: [PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[kbuild test robot]

Hi,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.7-rc5 next-20160701]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andi-Shyti/rc-ir-spi-add-support-for-IR-LEDs-connected-with-SPI/20160702-102955
base:   git://linuxtv.org/media_tree.git master
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/compiler.h:232:8: sparse: attribute 'no_sanitize_address': unknown attribute
   drivers/media/rc/ir-spi.c:156:14: sparse: undefined identifier 'LIRC_SET_LENGTH'
>> drivers/media/rc/ir-spi.c:156:14: sparse: incompatible types for 'case' statement
   drivers/media/rc/ir-spi.c:156:14: sparse: Expected constant expression in case statement
   drivers/media/rc/ir-spi.c: In function 'ir_spi_chardev_ioctl':
   drivers/media/rc/ir-spi.c:156:7: error: 'LIRC_SET_LENGTH' undeclared (first use in this function)
     case LIRC_SET_LENGTH: {
          ^~~~~~~~~~~~~~~
   drivers/media/rc/ir-spi.c:156:7: note: each undeclared identifier is reported only once for each function it appears in

vim +/case +156 drivers/media/rc/ir-spi.c

   140	
   141	static long ir_spi_chardev_ioctl(struct file *file, unsigned int cmd,
   142							unsigned long arg)
   143	{
   144		__u32 p;
   145		s32 ret;
   146		struct ir_spi_data *idata = file->private_data;
   147	
   148		switch (cmd) {
   149		case LIRC_GET_FEATURES:
   150			return put_user(idata->lirc_driver.features,
   151						(__u32 __user *) arg);
   152	
   153		case LIRC_GET_LENGTH:
   154			return put_user(idata->xfer.len, (__u32 __user *) arg);
   155	
 > 156		case LIRC_SET_LENGTH: {
   157			void *new;
   158	
   159			ret = get_user(p, (__u32 __user *) arg);
   160			if (ret)
   161				return ret;
   162	
   163			/*
   164			 * the user is trying to set the same

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[Rob Herring]

On Fri, Jul 01, 2016 at 05:33:42PM +0900, Andi Shyti wrote:
> The ir-spi is a simple device driver which supports the
> connection between an IR LED and the MOSI line of an SPI device.

Please split the binding from the driver.


> The driver, indeed, uses the SPI framework to stream the raw data
> provided by userspace through a character device. The chardev is
> handled by the LIRC framework and its functionality basically
> provides:
> 
>  - raw write: data to be sent to the SPI and then streamed to the
>    MOSI line;
>  - set frequency: sets the frequency whith which the data should
>    be sent;
>  - set length: sets the data length. This information is
>    optional, if the length is set, then userspace should send raw
>    data only with that length; while if the length is set to '0',
>    then the driver will figure out himself the length of the data
>    based on the length of the data written on the character
>    device.
>    The latter is not recommended, though, as the driver, at
>    any write, allocates and deallocates a buffer where the data
>    from userspace are stored.
> 
> The driver provides three feedback commands:
> 
>  - get length: reads the length set and (as mentioned), if the
>    length is '0' it will be calculated at any write
>  - get frequency: the driver reports the frequency. If userpace
>    doesn't set the frequency, the driver will use a default value
>    of 38000Hz.
> 
> The character device is created under /dev/lircX name, where X is
> and ID assigned by the LIRC framework.
> 
> Example of usage:
> 
>         int fd, ret;
>         ssize_t n;
>         uint32_t val = 0;
> 
>         fd = open("/dev/lirc0", O_RDWR);
>         if (fd < 0) {
>                 fprintf(stderr, "unable to open the device\n");
>                 return -1;
>         }
> 
>         /* ioctl set frequency and length parameters */
>         val = 6430;
>         ret = ioctl(fd, LIRC_SET_LENGTH, &val);
>         if (ret < 0)
>                 fprintf(stderr, "LIRC_SET_LENGTH failed\n");
>         val = 608000;
>         ret = ioctl(fd, LIRC_SET_FREQUENCY, &val);
>         if (ret < 0)
>                 fprintf(stderr, "LIRC_SET_FREQUENCY failed\n");
> 
>         /* read back length and frequency parameters */
>         ret = ioctl(fd, LIRC_GET_LENGTH, &val);
>         if (ret < 0)
>                 fprintf(stderr, "LIRC_GET_LENGTH failed\n");
>         else
>                 fprintf(stdout, "legnth = %u\n", val);
> 
>         ret = ioctl(fd, LIRC_GET_FREQUENCY, &val);
>         if (ret < 0)
>                 fprintf(stderr, "LIRC_GET_FREQUENCY failed\n");
>         else
>                 fprintf(stdout, "frequency = %u\n", val);
> 
>         /* write data to device */
>         n = write(fd, b, 6430);
>         if (n < 0) {
>                 fprintf(stderr, "unable to write to the device\n");
>                 ret = -1;
>         } else if (n != 6430) {
>                 fprintf(stderr, "failed to write everything, wrote %ld instead\n", n);
>                 ret = -1;
>         } else {
>                 fprintf(stdout, "written all the %ld data\n", n);
>         }
> 
>         close(fd);
> 
> The driver supports multi task access, but all the processes
> which hold the driver should use the same length and frequency
> parameters.
> 
> Change-Id: I323d7dd4a56d6dcf48f2c695293822eb04bdb85f
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  Documentation/devicetree/bindings/media/spi-ir.txt |  24 ++
>  drivers/media/rc/Kconfig                           |   9 +
>  drivers/media/rc/Makefile                          |   1 +
>  drivers/media/rc/ir-spi.c                          | 301 +++++++++++++++++++++
>  4 files changed, 335 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
>  create mode 100644 drivers/media/rc/ir-spi.c
> 
> diff --git a/Documentation/devicetree/bindings/media/spi-ir.txt b/Documentation/devicetree/bindings/media/spi-ir.txt
> new file mode 100644
> index 0000000..2232d92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/spi-ir.txt
> @@ -0,0 +1,24 @@
> +Device tree bindings for IR LED connected through SPI bus which is used as
> +remote controller.

Do said devices have part numbers? Seems kind of generic and I've never 
seen such device.

> +The IR LED switch is connected to the MOSI line of the SPI device and the data
> +are delivered thourgh that.
> +
> +Required properties:
> +	- compatible: should be "ir-spi"
> +
> +Optional properties:
> +	- irled,switch: specifies the gpio switch which enables the irled

Just "switch-gpios"

> +
> +Example:
> +
> +        irled@0 {
> +                compatible = "ir-spi";
> +                reg = <0x0>;
> +                spi-max-frequency = <5000000>;
> +                irled,switch = <&gpr3 3 0>;
> +
> +                controller-data {

This is part of the controller binding? Omit that from the example.

> +                        samsung,spi-feedback-delay = <0>;
> +                };
> +        };

Re: [PATCH] [media] rc: ir-spi: add support for IR LEDs connected with SPI

[Andi Shyti]

Hi Rob,

> > The ir-spi is a simple device driver which supports the
> > connection between an IR LED and the MOSI line of an SPI device.
> 
> Please split the binding from the driver.

OK!

> > +Device tree bindings for IR LED connected through SPI bus which is used as
> > +remote controller.
> 
> Do said devices have part numbers? Seems kind of generic and I've never 
> seen such device.

No, it doesn't, this is a simple irled driven by the SPI MOSI
line that works as a remote controller.

It looked strange to me as well when I heard of it the first
time, but it will be present in the upcoming tm2/tm2e patchset 
(i.e. Samsung Note 4 and Note 4 edge).

> > +Optional properties:
> > +	- irled,switch: specifies the gpio switch which enables the irled
> 
> Just "switch-gpios"

OK!

> > +                controller-data {
> 
> This is part of the controller binding? Omit that from the example.

OK!

Thanks,
Andi