Re: [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI

From: Sean Young
Date: Thu Sep 01 2016 - 17:21:09 EST

Next message: Salil Mehta: "[PATCH for-next 02/10] IB/hns: Register add_gid and del_gid for GID Table management"
Previous message: Salil Mehta: "[PATCH for-next 06/10] IB/hns: Add phy_port for computing GSI/QPN"
In reply to: Andi Shyti: "[PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI"
Next in thread: Andi Shyti: "Re: [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Thanks Andi, this is looking great!

On Fri, Sep 02, 2016 at 02:16:29AM +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 an rc character device. The chardev
> is handled by the LIRC framework and its functionality basically
> provides:
>
> - write: the driver gets a pulse/space signal and translates it
> to a binary signal that will be streamed to the IR led through
> the SPI framework.
> - set frequency: sets the frequency whith which the data should
> be sent. This is handle with ioctl with the
> LIRC_SET_SEND_CARRIER flag (as per lirc documentation)
> - set duty cycle: this is also handled with ioctl with the
> LIRC_SET_SEND_DUTY_CYCLE flag. The driver handles duty cycles
> of 50%, 60%, 70%, 75%, 80% and 90%, calculated on 16bit data.
>
> The character device is created under /dev/lircX name, where X is
> and ID assigned by the LIRC framework.
>
> Example of usage:
>
> fd = open("/dev/lirc0", O_RDWR);
> if (fd < 0)
> return -1;
>
> val = 608000;
> ret = ioctl(fd, LIRC_SET_SEND_CARRIER, &val);
> if (ret < 0)
> return -1;
>
> val = 60;
> ret = ioctl(fd, LIRC_SET_SEND_DUTY_CYCLE, &val);
> if (ret < 0)
> return -1;
>
> n = write(fd, buffer, BUF_LEN);
> if (n < 0 || n != BUF_LEN)
> ret = -1;
>
> close(fd);
>
> Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxx>
> ---
> drivers/media/rc/Kconfig | 9 ++
> drivers/media/rc/Makefile | 1 +
> drivers/media/rc/ir-spi.c | 221 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 231 insertions(+)
> create mode 100644 drivers/media/rc/ir-spi.c
>
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index 370e16e..207dfcc 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..34d5a0c
> --- /dev/null
> +++ b/drivers/media/rc/ir-spi.c
> @@ -0,0 +1,221 @@
> +/*
> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> + * Author: Andi Shyti <andi.shyti@xxxxxxxxxxx>
> + *
> + * 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/delay.h>
> +#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/rc-core.h>
> +
> +#define IR_SPI_DRIVER_NAME "ir-spi"
> +
> +/* pulse value for different duty cycles */
> +#define IR_SPI_PULSE_DC_50 0xff00
> +#define IR_SPI_PULSE_DC_60 0xfc00
> +#define IR_SPI_PULSE_DC_70 0xf800
> +#define IR_SPI_PULSE_DC_75 0xf000
> +#define IR_SPI_PULSE_DC_80 0xc000
> +#define IR_SPI_PULSE_DC_90 0x8000
> +
> +/* duty cycles values */
> +#define IR_SPI_DUTY_CYCLE_50 50
> +#define IR_SPI_DUTY_CYCLE_60 60
> +#define IR_SPI_DUTY_CYCLE_70 70
> +#define IR_SPI_DUTY_CYCLE_75 75
> +#define IR_SPI_DUTY_CYCLE_80 80
> +#define IR_SPI_DUTY_CYCLE_90 90

Not sure what these defines are for. 50 = 50?

> +
> +#define IR_SPI_DEFAULT_FREQUENCY 38000
> +#define IR_SPI_BIT_PER_WORD 8
> +#define IR_SPI_MAX_BUFSIZE 4096
> +
> +struct ir_spi_data {
> + u32 freq;
> + u8 duty_cycle;
> + bool negated;
> +
> + u16 tx_buf[IR_SPI_MAX_BUFSIZE];
> + u16 pulse;
> + u16 space;
> +
> + struct rc_dev *rc;
> + struct spi_device *spi;
> + struct regulator *regulator;
> +};
> +
> +static int ir_spi_tx(struct rc_dev *dev, unsigned int *buffer, unsigned int count)
> +{
> + int i;
> + int ret;
> + unsigned int len = 0;
> + struct ir_spi_data *idata = dev->priv;
> + struct spi_transfer xfer;
> +
> + /* convert the pulse/space signal to raw binary signal */
> + for (i = 0; i < count; i++) {
> + int j;
> + u16 val = ((i+1) % 2) ? idata->pulse : idata->space;
> +
> + if (len + buffer[i] >= IR_SPI_MAX_BUFSIZE)
> + return -EINVAL;
> +
> + /*
> + * the first value in buffer is a pulse, so that 0, 2, 4, ...
> + * contain a pulse duration. On the contrary, 1, 3, 5, ...
> + * contain a space duration.
> + */
> + val = (i % 2) ? idata->space : idata->pulse;
> + for (j = 0; j < buffer[i]; j++)
> + idata->tx_buf[len++] = val;
> + }
> +
> + pr_info("from %u data, we originated %u raw data\n", count, len);
> +
> + memset(&xfer, 0, sizeof(xfer));
> +
> + xfer.speed_hz = idata->freq;
> + xfer.len = len * sizeof(*idata->tx_buf);
> + xfer.tx_buf = idata->tx_buf;
> +
> + ret = regulator_enable(idata->regulator);
> + if (ret)
> + return ret;
> +
> + ret = spi_sync_transfer(idata->spi, &xfer, 1);
> + if (ret)
> + dev_err(&idata->spi->dev, "unable to deliver the signal\n");
> +
> + regulator_disable(idata->regulator);
> +
> + return ret ? ret : len;

On success you should be returning count, not the length of the spi
bitstream.

> +}
> +
> +static int ir_spi_set_tx_carrier(struct rc_dev *dev, u32 carrier)
> +{
> + struct ir_spi_data *idata = dev->priv;
> +
> + if (!carrier)
> + return -EINVAL;
> +
> + idata->freq = carrier;
> +
> + return 0;
> +}
> +
> +static int ir_spi_set_duty_cycle(struct rc_dev *dev, u32 duty_cycle)
> +{
> + struct ir_spi_data *idata = dev->priv;
> +
> + switch (duty_cycle) {
> + case IR_SPI_DUTY_CYCLE_90:
> + idata->pulse = IR_SPI_PULSE_DC_90;
> + break;
> + case IR_SPI_DUTY_CYCLE_80:
> + idata->pulse = IR_SPI_PULSE_DC_80;
> + break;
> + case IR_SPI_DUTY_CYCLE_75:
> + idata->pulse = IR_SPI_PULSE_DC_75;
> + break;
> + case IR_SPI_DUTY_CYCLE_70:
> + idata->pulse = IR_SPI_PULSE_DC_70;
> + break;
> + case IR_SPI_DUTY_CYCLE_60:
> + idata->pulse = IR_SPI_PULSE_DC_60;
> + break;
> + case IR_SPI_DUTY_CYCLE_50:
> + default:
> + idata->pulse = IR_SPI_PULSE_DC_50;
> + }

It seems a bit odd that if you set a duty cycle of (say) 61 get a duty
cycle of 50. Maybe something like:

if (duty_cycle >= 90)
idata->pulse = IR_SPI_PULSE_DC_90;
else if (duty_cycle >= 80)
idata->pulse = IR_SPI_PULSE_DC_80;
else if (duty_cycle >= 75)
idata->pulse = IR_SPI_PULSE_DC_75;
else if (duty_cycle >= 70)
idata->pulse = IR_SPI_PULSE_DC_70;
else if (duty_cycle >= 60)
idata->pulse = IR_SPI_PULSE_DC_60;
else
idata->pulse = IR_SPI_PULSE_DC_50;

> +
> + if (idata->negated) {
> + idata->pulse = ~idata->pulse;
> + idata->space = 0xffff;
> + } else {
> + idata->space = 0;
> + }
> +
> + return 0;
> +}
> +
> +static int ir_spi_probe(struct spi_device *spi)
> +{
> + int ret;
> + u8 dc;
> + 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);
> +
> + idata->rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX);
> + if (!idata->rc)
> + return -ENOMEM;
> +
> + idata->rc->tx_ir = ir_spi_tx;
> + idata->rc->s_tx_carrier = ir_spi_set_tx_carrier;
> + idata->rc->s_tx_duty_cycle = ir_spi_set_duty_cycle;
> + idata->rc->driver_name = IR_SPI_DRIVER_NAME;
> + idata->rc->priv = idata;
> + idata->spi = spi;
> +
> + idata->negated = of_property_read_bool(spi->dev.of_node, "negated");
> + ret = of_property_read_u8(spi->dev.of_node, "duty-cycle", &dc);
> + if (ret)
> + dc = IR_SPI_DUTY_CYCLE_50;
> +
> + ret = ir_spi_set_duty_cycle(idata->rc, dc);
> + if (ret)
> + return ret;
> +
> + idata->freq = IR_SPI_DEFAULT_FREQUENCY;
> +
> + ret = rc_register_device(idata->rc);
> + if (ret)
> + rc_unregister_device(idata->rc);

Surely you mean rc_free_device(idata->rc) here.

> +
> + return ret;
> +}
> +
> +static int ir_spi_remove(struct spi_device *spi)
> +{
> + struct ir_spi_data *idata = spi_get_drvdata(spi);
> +
> + rc_unregister_device(idata->rc);
> +
> + 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@xxxxxxxxxxx>");
> +MODULE_DESCRIPTION("SPI IR LED");
> +MODULE_LICENSE("GPL v2");
> --
> 2.9.3

Next message: Salil Mehta: "[PATCH for-next 02/10] IB/hns: Register add_gid and del_gid for GID Table management"
Previous message: Salil Mehta: "[PATCH for-next 06/10] IB/hns: Add phy_port for computing GSI/QPN"
In reply to: Andi Shyti: "[PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI"
Next in thread: Andi Shyti: "Re: [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]