Hi Dmitry,
Thanks for the feedback. See my comments below:
On 03/01/17 07:51, Dmitry Torokhov wrote:
Hi Hans,
On Thu, Dec 15, 2016 at 02:02:07PM +0100, Hans Verkuil wrote:
From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
This driver supports the RainShadow Tech USB HDMI CEC adapter.
See: http://rainshadowtech.com/HdmiCecUsb.html
Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
---
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@xxxxxxxxxxxxxxx
S: Maintained
F: drivers/video/fbdev/aty/aty128fb.c
+RAINSHADOW-CEC DRIVER
+M: Hans Verkuil <hverkuil@xxxxxxxxx>
+L: linux-media@xxxxxxxxxxxxxxx
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/usb/rainshadow-cec/*
+
RALINK MIPS ARCHITECTURE
M: John Crispin <john@xxxxxxxxxxx>
L: linux-mips@xxxxxxxxxxxxxx
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 <hverkuil@xxxxxxxxx
+ *
+ * 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 <linux/completion.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+#include <linux/time.h>
+#include <linux/delay.h>
+#include <linux/ctype.h>
+
+#include <media/cec.h>
+
+MODULE_AUTHOR("Hans Verkuil <hverkuil@xxxxxxxxx>");
+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.
OK.
+ 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.
I reworked this. This was in fact buggy, mainly because this adapter has the bad
behavior of echoing the CEC messages it transmits. So I would get a REC and a STA
message in quick succession.
+ } else if (data == '?') {
+ rain->idx = 0;
+ rain->started = true;
+ return IRQ_HANDLED;
+ }
+
+ if (data == '\n') {
Should it be a "switch (data) {" ?
Done. That's indeed better.
+ 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.
Certainly not. inputattach knows nothing about CEC, it just sets the right
line discipline.
You don't want to have part of the adapter setup in inputattach and another
part in the CEC driver, that would be impossible to maintain and keep in sync.
Ideally inputattach wouldn't be needed at all, but to my knowledge it is still
not possible to do without this userspace component. Some work was done for
TTY slave devices, but I am not aware of anything being merged. Please let me
know if I am wrong.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
