From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-pf0-f193.google.com ([209.85.192.193]:33682 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756329AbdACGvI (ORCPT ); Tue, 3 Jan 2017 01:51:08 -0500 Date: Mon, 2 Jan 2017 22:51:05 -0800 From: Dmitry Torokhov To: Hans Verkuil Cc: linux-media@vger.kernel.org, linux-input , Hans Verkuil Subject: Re: [PATCH 2/2] rainshadow-cec: new RainShadow Tech HDMI CEC driver Message-ID: <20170103065105.GC5743@dtor-ws> References: <20161215130207.12913-1-hverkuil@xs4all.nl> <20161215130207.12913-3-hverkuil@xs4all.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161215130207.12913-3-hverkuil@xs4all.nl> Sender: linux-media-owner@vger.kernel.org List-ID: Hi Hans, On Thu, Dec 15, 2016 at 02:02:07PM +0100, Hans Verkuil wrote: > From: Hans Verkuil > > This driver supports the RainShadow Tech USB HDMI CEC adapter. > > See: http://rainshadowtech.com/HdmiCecUsb.html > > Signed-off-by: Hans Verkuil > --- > MAINTAINERS | 7 + > drivers/media/usb/Kconfig | 1 + > drivers/media/usb/Makefile | 1 + > drivers/media/usb/rainshadow-cec/Kconfig | 10 + > drivers/media/usb/rainshadow-cec/Makefile | 1 + > drivers/media/usb/rainshadow-cec/rainshadow-cec.c | 344 ++++++++++++++++++++++ > 6 files changed, 364 insertions(+) > create mode 100644 drivers/media/usb/rainshadow-cec/Kconfig > create mode 100644 drivers/media/usb/rainshadow-cec/Makefile > create mode 100644 drivers/media/usb/rainshadow-cec/rainshadow-cec.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 52cc077..78ebc5d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10069,6 +10069,13 @@ L: linux-fbdev@vger.kernel.org > S: Maintained > F: drivers/video/fbdev/aty/aty128fb.c > > +RAINSHADOW-CEC DRIVER > +M: Hans Verkuil > +L: linux-media@vger.kernel.org > +T: git git://linuxtv.org/media_tree.git > +S: Maintained > +F: drivers/media/usb/rainshadow-cec/* > + > RALINK MIPS ARCHITECTURE > M: John Crispin > L: linux-mips@linux-mips.org > diff --git a/drivers/media/usb/Kconfig b/drivers/media/usb/Kconfig > index c9644b6..b24e753 100644 > --- a/drivers/media/usb/Kconfig > +++ b/drivers/media/usb/Kconfig > @@ -63,6 +63,7 @@ endif > if MEDIA_CEC_SUPPORT > comment "USB HDMI CEC adapters" > source "drivers/media/usb/pulse8-cec/Kconfig" > +source "drivers/media/usb/rainshadow-cec/Kconfig" > endif > > endif #MEDIA_USB_SUPPORT > diff --git a/drivers/media/usb/Makefile b/drivers/media/usb/Makefile > index 0f15e33..738b993 100644 > --- a/drivers/media/usb/Makefile > +++ b/drivers/media/usb/Makefile > @@ -25,3 +25,4 @@ obj-$(CONFIG_VIDEO_USBTV) += usbtv/ > obj-$(CONFIG_VIDEO_GO7007) += go7007/ > obj-$(CONFIG_DVB_AS102) += as102/ > obj-$(CONFIG_USB_PULSE8_CEC) += pulse8-cec/ > +obj-$(CONFIG_USB_RAINSHADOW_CEC) += rainshadow-cec/ > diff --git a/drivers/media/usb/rainshadow-cec/Kconfig b/drivers/media/usb/rainshadow-cec/Kconfig > new file mode 100644 > index 0000000..447291b > --- /dev/null > +++ b/drivers/media/usb/rainshadow-cec/Kconfig > @@ -0,0 +1,10 @@ > +config USB_RAINSHADOW_CEC > + tristate "RainShadow Tech HDMI CEC" > + depends on USB_ACM && MEDIA_CEC_SUPPORT > + select SERIO > + select SERIO_SERPORT > + ---help--- > + This is a cec driver for the RainShadow Tech HDMI CEC device. > + > + To compile this driver as a module, choose M here: the > + module will be called rainshadow-cec. > diff --git a/drivers/media/usb/rainshadow-cec/Makefile b/drivers/media/usb/rainshadow-cec/Makefile > new file mode 100644 > index 0000000..a79fbc7 > --- /dev/null > +++ b/drivers/media/usb/rainshadow-cec/Makefile > @@ -0,0 +1 @@ > +obj-$(CONFIG_USB_RAINSHADOW_CEC) += rainshadow-cec.o > diff --git a/drivers/media/usb/rainshadow-cec/rainshadow-cec.c b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c > new file mode 100644 > index 0000000..dc7f287 > --- /dev/null > +++ b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c > @@ -0,0 +1,344 @@ > +/* > + * RainShadow Tech HDMI CEC driver > + * > + * Copyright 2016 Hans Verkuil + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version of 2 of the License, or (at your > + * option) any later version. See the file COPYING in the main directory of > + * this archive for more details. > + */ > + > +/* > + * Notes: > + * > + * The higher level protocols are currently disabled. This can be added > + * later, similar to how this is done for the Pulse Eight CEC driver. > + * > + * Documentation of the protocol is available here: > + * > + * http://rainshadowtech.com/doc/HDMICECtoUSBandRS232v2.0.pdf > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +MODULE_AUTHOR("Hans Verkuil "); > +MODULE_DESCRIPTION("RainShadow Tech HDMI CEC driver"); > +MODULE_LICENSE("GPL"); > + > +static int debug; > +module_param(debug, int, 0644); > +MODULE_PARM_DESC(debug, "debug level (0-1)"); > + > +#define DATA_SIZE 256 > + > +struct rain { > + struct device *dev; > + struct serio *serio; > + struct cec_adapter *adap; > + struct completion cmd_done; > + struct work_struct work; > + struct cec_msg rx_msg; > + char data[DATA_SIZE]; > + char buf[DATA_SIZE]; > + unsigned int idx; > + bool started; > + struct mutex write_lock; > +}; > + > +static void rain_irq_work_handler(struct work_struct *work) > +{ > + struct rain *rain = > + container_of(work, struct rain, work); > + struct cec_msg *msg = &rain->rx_msg; > + const char *data = rain->data + 3; > + int stat = -1; > + > + msg->len = 0; > + for (; *data; data++) { > + if (!isxdigit(*data)) > + continue; > + if (isxdigit(data[0]) && isxdigit(data[1])) { > + if (msg->len == CEC_MAX_MSG_SIZE) > + break; > + hex2bin(msg->msg + msg->len, data, 2); > + msg->len++; > + data++; > + continue; > + } > + if (!data[1]) > + stat = hex_to_bin(data[0]); > + break; > + } > + > + if (rain->data[0] == 'R') { > + if (stat == 1 || stat == 2) > + cec_received_msg(rain->adap, msg); > + return; > + } > + > + switch (stat) { > + case 1: > + cec_transmit_done(rain->adap, CEC_TX_STATUS_OK, > + 0, 0, 0, 0); > + break; > + case 2: > + cec_transmit_done(rain->adap, CEC_TX_STATUS_NACK, > + 0, 1, 0, 0); > + break; > + default: > + cec_transmit_done(rain->adap, CEC_TX_STATUS_LOW_DRIVE, > + 0, 0, 0, 1); > + break; > + } > +} > + > +static irqreturn_t rain_interrupt(struct serio *serio, unsigned char data, > + unsigned int flags) > +{ > + struct rain *rain = serio_get_drvdata(serio); > + > + if (!rain->started && data != '?') > + return IRQ_HANDLED; > + if (data == '\r') { > + rain->buf[rain->idx] = '\0'; > + if (debug) > + dev_info(rain->dev, "received: %s\n", rain->buf); Just dev_dbg() please. I think everyone is using dynamic debug. > + strcpy(rain->data, rain->buf); > + if (!memcmp(rain->data, "REC", 3) || > + !memcmp(rain->data, "STA", 3)) > + schedule_work(&rain->work); > + else > + complete(&rain->cmd_done); > + rain->idx = 0; > + rain->started = false; > + return IRQ_HANDLED; This relies on work to complete before you receive the next message, which is not always the case as far as I can understand. > + } else if (data == '?') { > + rain->idx = 0; > + rain->started = true; > + return IRQ_HANDLED; > + } > + > + if (data == '\n') { Should it be a "switch (data) {" ? > + rain->idx = 0; > + rain->started = false; > + return IRQ_HANDLED; > + } > + if (rain->idx >= DATA_SIZE - 1) { > + dev_dbg(rain->dev, > + "throwing away %d bytes of garbage\n", rain->idx); > + rain->idx = 0; > + } > + rain->buf[rain->idx++] = data; > + return IRQ_HANDLED; > +} > + > +static void rain_disconnect(struct serio *serio) > +{ > + struct rain *rain = serio_get_drvdata(serio); > + > + cec_unregister_adapter(rain->adap); > + dev_info(&serio->dev, "disconnected\n"); > + serio_close(serio); > + serio_set_drvdata(serio, NULL); > + kfree(rain); > +} > + > +static int rain_send(struct rain *rain, const char *command) > +{ > + int err = serio_write(rain->serio, '!'); > + > + if (debug) > + dev_info(rain->dev, "send: %s\n", command); > + while (!err && *command) > + err = serio_write(rain->serio, *command++); > + if (!err) > + err = serio_write(rain->serio, '~'); > + > + return err; > +} > + > +static int rain_send_and_wait(struct rain *rain, > + const char *cmd, const char *reply) > +{ > + int err; > + > + init_completion(&rain->cmd_done); > + > + mutex_lock(&rain->write_lock); > + err = rain_send(rain, cmd); > + if (err) > + goto err; > + > + if (!wait_for_completion_timeout(&rain->cmd_done, HZ)) { > + err = -ETIMEDOUT; > + goto err; > + } > + if (reply && strncmp(rain->data, reply, strlen(reply))) { > + if (debug) > + dev_info(rain->dev, > + "transmit of '%s': received '%s' instead of '%s'\n", > + cmd, rain->data, reply); > + err = -EIO; > + } > +err: > + mutex_unlock(&rain->write_lock); > + return err; > +} > + > +static int rain_setup(struct rain *rain, struct serio *serio, > + struct cec_log_addrs *log_addrs, u16 *pa) > +{ > + u16 res; > + int err; > + > + err = rain_send_and_wait(rain, "R", "REV"); > + if (err) > + return err; > + dev_info(rain->dev, "Firmware version %s\n", rain->data + 4); > + > + err = rain_send_and_wait(rain, "Q 1", "QTY"); > + if (err) > + return err; > + err = rain_send_and_wait(rain, "c0000", "CFG"); > + if (err) > + return err; > + return rain_send_and_wait(rain, "A F 0000", "ADR"); Given that this is instantiated via serport + inputattach the setup sequence should be done in inputattach, not kernel. > +} > + > +static int rain_cec_adap_enable(struct cec_adapter *adap, bool enable) > +{ > + return 0; > +} > + > +static int rain_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr) > +{ > + struct rain *rain = adap->priv; > + u8 cmd[16]; > + > + if (log_addr == CEC_LOG_ADDR_INVALID) > + log_addr = CEC_LOG_ADDR_UNREGISTERED; > + snprintf(cmd, sizeof(cmd), "A %x", log_addr); > + return rain_send_and_wait(rain, cmd, "ADR"); > +} > + > +static int rain_cec_adap_transmit(struct cec_adapter *adap, u8 attempts, > + u32 signal_free_time, struct cec_msg *msg) > +{ > + struct rain *rain = adap->priv; > + char cmd[2 * CEC_MAX_MSG_SIZE + 16]; > + unsigned int i; > + int err; > + > + if (msg->len == 1) { > + snprintf(cmd, sizeof(cmd), "x%x", cec_msg_destination(msg)); > + } else { > + char hex[3]; > + > + snprintf(cmd, sizeof(cmd), "x%x %02x ", > + cec_msg_destination(msg), msg->msg[1]); > + for (i = 2; i < msg->len; i++) { > + snprintf(hex, sizeof(hex), "%02x", msg->msg[i]); > + strncat(cmd, hex, sizeof(cmd)); > + } > + } > + mutex_lock(&rain->write_lock); > + err = rain_send(rain, cmd); > + mutex_unlock(&rain->write_lock); > + return err; > +} > + > +static const struct cec_adap_ops rain_cec_adap_ops = { > + .adap_enable = rain_cec_adap_enable, > + .adap_log_addr = rain_cec_adap_log_addr, > + .adap_transmit = rain_cec_adap_transmit, > +}; > + > +static int rain_connect(struct serio *serio, struct serio_driver *drv) > +{ > + u32 caps = CEC_CAP_TRANSMIT | CEC_CAP_LOG_ADDRS | CEC_CAP_PHYS_ADDR | > + CEC_CAP_PASSTHROUGH | CEC_CAP_RC | CEC_CAP_MONITOR_ALL; > + struct rain *rain; > + int err = -ENOMEM; > + struct cec_log_addrs log_addrs = {}; > + u16 pa = CEC_PHYS_ADDR_INVALID; > + > + rain = kzalloc(sizeof(*rain), GFP_KERNEL); > + > + if (!rain) > + return -ENOMEM; > + > + rain->serio = serio; > + rain->adap = cec_allocate_adapter(&rain_cec_adap_ops, rain, > + "HDMI CEC", caps, 1); > + err = PTR_ERR_OR_ZERO(rain->adap); > + if (err < 0) > + goto free_device; > + > + rain->dev = &serio->dev; > + serio_set_drvdata(serio, rain); > + INIT_WORK(&rain->work, rain_irq_work_handler); > + mutex_init(&rain->write_lock); > + > + err = serio_open(serio, drv); > + if (err) > + goto delete_adap; > + > + err = rain_setup(rain, serio, &log_addrs, &pa); > + if (err) > + goto close_serio; > + > + err = cec_register_adapter(rain->adap, &serio->dev); > + if (err < 0) > + goto close_serio; > + > + rain->dev = &rain->adap->devnode.dev; > + return 0; > + > +close_serio: > + serio_close(serio); > +delete_adap: > + cec_delete_adapter(rain->adap); > + serio_set_drvdata(serio, NULL); > +free_device: > + kfree(rain); > + return err; > +} > + > +static struct serio_device_id rain_serio_ids[] = { > + { > + .type = SERIO_RS232, > + .proto = SERIO_RAINSHADOW_CEC, > + .id = SERIO_ANY, > + .extra = SERIO_ANY, > + }, > + { 0 } > +}; > + > +MODULE_DEVICE_TABLE(serio, rain_serio_ids); > + > +static struct serio_driver rain_drv = { > + .driver = { > + .name = "rainshadow-cec", > + }, > + .description = "RainShadow Tech HDMI CEC driver", > + .id_table = rain_serio_ids, > + .interrupt = rain_interrupt, > + .connect = rain_connect, > + .disconnect = rain_disconnect, > +}; > + > +module_serio_driver(rain_drv); > -- > 2.10.2 > Thanks. -- Dmitry