[PATCH] nuvoton-cir: Add support for user configurable wake-up codes

[PATCH] nuvoton-cir: Add support for user configurable wake-up codes

[Antti Seppälä]

This patch introduces module parameters for setting wake-up codes to be
programmed into the hardware FIFO. This allows users to provide custom
IR sample sequences to trigger system wake-up from sleep states.

Usage:
   modprobe nuvoton-cir wake_samples=0x90,0x11,0xa1... (up to 67 bytes)

Here is a summary of module parameters introduced by this patch:
 * wake_samples: FIFO values to compare against received IR codes when
   in sleep state.

 * cmp_deep: Number of bytes the hardware will compare against. This is
   currently autodetected by the driver.

 * cmp_tolerance: The maximum allowed value difference between a single
   wake-up byte and a sample read from IR to be still considered a match.
   Default is 5.

Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
 drivers/media/rc/nuvoton-cir.c | 65 +++++++++++++++++++++++++++++++-----------
 drivers/media/rc/nuvoton-cir.h |  6 ++--
 2 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index 21ee0dc..b11ee43 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -39,6 +39,15 @@
 
 #include "nuvoton-cir.h"
 
+/* debugging module parameter */
+static int debug;
+
+/* Wake up configuration parameters */
+static unsigned char wake_samples[WAKE_FIFO_LEN];
+static unsigned int num_wake_samples;
+static unsigned char cmp_deep;
+static unsigned char cmp_tolerance = CIR_WAKE_CMP_TOLERANCE;
+
 /* write val to config reg */
 static inline void nvt_cr_write(struct nvt_dev *nvt, u8 val, u8 reg)
 {
@@ -418,13 +427,38 @@ static void nvt_cir_regs_init(struct nvt_dev *nvt)
 
 static void nvt_cir_wake_regs_init(struct nvt_dev *nvt)
 {
+	int i;
+	u8 cmp_reg = CIR_WAKE_FIFO_CMP_BYTES;
+	u8 ircon_reg = CIR_WAKE_IRCON_RXEN | CIR_WAKE_IRCON_R |
+		       CIR_WAKE_IRCON_RXINV | CIR_WAKE_IRCON_SAMPLE_PERIOD_SEL;
+	/*
+	 * Enable TX and RX, specific carrier on = low, off = high, and set
+	 * sample period (currently 50us)
+	 */
+	nvt_cir_wake_reg_write(nvt, ircon_reg | CIR_WAKE_IRCON_MODE1,
+			       CIR_WAKE_IRCON);
+
+	/* clear cir wake rx fifo */
+	nvt_clear_cir_wake_fifo(nvt);
+
+	/* Write samples from module parameter to fifo */
+	for (i = 0; i < num_wake_samples; i++)
+		nvt_cir_wake_reg_write(nvt, wake_samples[i],
+				       CIR_WAKE_WR_FIFO_DATA);
+
+	/* Switch cir to wakeup mode and disable fifo writing */
+	nvt_cir_wake_reg_write(nvt, ircon_reg | CIR_WAKE_IRCON_MODE0,
+			       CIR_WAKE_IRCON);
+
 	/* set number of bytes needed for wake from s3 (default 65) */
-	nvt_cir_wake_reg_write(nvt, CIR_WAKE_FIFO_CMP_BYTES,
-			       CIR_WAKE_FIFO_CMP_DEEP);
+	if (cmp_deep)
+		cmp_reg = cmp_deep;
+	else if (num_wake_samples)
+		cmp_reg = num_wake_samples;
+	nvt_cir_wake_reg_write(nvt, cmp_reg, CIR_WAKE_FIFO_CMP_DEEP);
 
 	/* set tolerance/variance allowed per byte during wake compare */
-	nvt_cir_wake_reg_write(nvt, CIR_WAKE_CMP_TOLERANCE,
-			       CIR_WAKE_FIFO_CMP_TOL);
+	nvt_cir_wake_reg_write(nvt, cmp_tolerance, CIR_WAKE_FIFO_CMP_TOL);
 
 	/* set sample limit count (PE interrupt raised when reached) */
 	nvt_cir_wake_reg_write(nvt, CIR_RX_LIMIT_COUNT >> 8, CIR_WAKE_SLCH);
@@ -434,18 +468,6 @@ static void nvt_cir_wake_regs_init(struct nvt_dev *nvt)
 	nvt_cir_wake_reg_write(nvt, CIR_WAKE_FIFOCON_RX_TRIGGER_LEV,
 			       CIR_WAKE_FIFOCON);
 
-	/*
-	 * Enable TX and RX, specific carrier on = low, off = high, and set
-	 * sample period (currently 50us)
-	 */
-	nvt_cir_wake_reg_write(nvt, CIR_WAKE_IRCON_MODE0 | CIR_WAKE_IRCON_RXEN |
-			       CIR_WAKE_IRCON_R | CIR_WAKE_IRCON_RXINV |
-			       CIR_WAKE_IRCON_SAMPLE_PERIOD_SEL,
-			       CIR_WAKE_IRCON);
-
-	/* clear cir wake rx fifo */
-	nvt_clear_cir_wake_fifo(nvt);
-
 	/* clear any and all stray interrupts */
 	nvt_cir_wake_reg_write(nvt, 0xff, CIR_WAKE_IRSTS);
 }
@@ -1232,6 +1254,17 @@ static void nvt_exit(void)
 module_param(debug, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(debug, "Enable debugging output");
 
+module_param_array(wake_samples, byte, &num_wake_samples, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(wake_samples, "FIFO sample bytes triggering wake");
+
+module_param(cmp_deep, byte, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(cmp_deep, "How many bytes need to compare\n"
+			   "\t\t(0 = auto (default))");
+
+module_param(cmp_tolerance, byte, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(cmp_tolerance, "Data tolerance to each wake sample byte\n"
+				"\t\t(default = 5)");
+
 MODULE_DEVICE_TABLE(pnp, nvt_ids);
 MODULE_DESCRIPTION("Nuvoton W83667HG-A & W83677HG-I CIR driver");
 
diff --git a/drivers/media/rc/nuvoton-cir.h b/drivers/media/rc/nuvoton-cir.h
index 07e8310..8209f84 100644
--- a/drivers/media/rc/nuvoton-cir.h
+++ b/drivers/media/rc/nuvoton-cir.h
@@ -31,10 +31,6 @@
 /* platform driver name to register */
 #define NVT_DRIVER_NAME "nuvoton-cir"
 
-/* debugging module parameter */
-static int debug;
-
-
 #define nvt_pr(level, text, ...) \
 	printk(level KBUILD_MODNAME ": " text, ## __VA_ARGS__)
 
@@ -64,6 +60,8 @@ static int debug;
 #define TX_BUF_LEN 256
 #define RX_BUF_LEN 32
 
+#define WAKE_FIFO_LEN 67
+
 struct nvt_dev {
 	struct pnp_dev *pdev;
 	struct rc_dev *rdev;
-- 
1.8.3.2

Re: [PATCH] nuvoton-cir: Add support for user configurable wake-up codes

[Mauro Carvalho Chehab]

Em Fri,  3 Jan 2014 16:18:43 +0200
Antti Seppälä <a.seppala@gmail.com> escreveu:

> This patch introduces module parameters for setting wake-up codes to be
> programmed into the hardware FIFO. This allows users to provide custom
> IR sample sequences to trigger system wake-up from sleep states.
> 
> Usage:
>    modprobe nuvoton-cir wake_samples=0x90,0x11,0xa1... (up to 67 bytes)
> 
> Here is a summary of module parameters introduced by this patch:
>  * wake_samples: FIFO values to compare against received IR codes when
>    in sleep state.
> 
>  * cmp_deep: Number of bytes the hardware will compare against. This is
>    currently autodetected by the driver.
> 
>  * cmp_tolerance: The maximum allowed value difference between a single
>    wake-up byte and a sample read from IR to be still considered a match.
>    Default is 5.

Instead of adding it as a modprobe parameter, the better is to create
an special sysfs node for devices that support wake up.

Btw, there's another device driver also requiring a similar feature.

Regards,
Mauro

> 
> Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
> ---
>  drivers/media/rc/nuvoton-cir.c | 65 +++++++++++++++++++++++++++++++-----------
>  drivers/media/rc/nuvoton-cir.h |  6 ++--
>  2 files changed, 51 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
> index 21ee0dc..b11ee43 100644
> --- a/drivers/media/rc/nuvoton-cir.c
> +++ b/drivers/media/rc/nuvoton-cir.c
> @@ -39,6 +39,15 @@
>  
>  #include "nuvoton-cir.h"
>  
> +/* debugging module parameter */
> +static int debug;
> +
> +/* Wake up configuration parameters */
> +static unsigned char wake_samples[WAKE_FIFO_LEN];
> +static unsigned int num_wake_samples;
> +static unsigned char cmp_deep;
> +static unsigned char cmp_tolerance = CIR_WAKE_CMP_TOLERANCE;
> +
>  /* write val to config reg */
>  static inline void nvt_cr_write(struct nvt_dev *nvt, u8 val, u8 reg)
>  {
> @@ -418,13 +427,38 @@ static void nvt_cir_regs_init(struct nvt_dev *nvt)
>  
>  static void nvt_cir_wake_regs_init(struct nvt_dev *nvt)
>  {
> +	int i;
> +	u8 cmp_reg = CIR_WAKE_FIFO_CMP_BYTES;
> +	u8 ircon_reg = CIR_WAKE_IRCON_RXEN | CIR_WAKE_IRCON_R |
> +		       CIR_WAKE_IRCON_RXINV | CIR_WAKE_IRCON_SAMPLE_PERIOD_SEL;
> +	/*
> +	 * Enable TX and RX, specific carrier on = low, off = high, and set
> +	 * sample period (currently 50us)
> +	 */
> +	nvt_cir_wake_reg_write(nvt, ircon_reg | CIR_WAKE_IRCON_MODE1,
> +			       CIR_WAKE_IRCON);
> +
> +	/* clear cir wake rx fifo */
> +	nvt_clear_cir_wake_fifo(nvt);
> +
> +	/* Write samples from module parameter to fifo */
> +	for (i = 0; i < num_wake_samples; i++)
> +		nvt_cir_wake_reg_write(nvt, wake_samples[i],
> +				       CIR_WAKE_WR_FIFO_DATA);
> +
> +	/* Switch cir to wakeup mode and disable fifo writing */
> +	nvt_cir_wake_reg_write(nvt, ircon_reg | CIR_WAKE_IRCON_MODE0,
> +			       CIR_WAKE_IRCON);
> +
>  	/* set number of bytes needed for wake from s3 (default 65) */
> -	nvt_cir_wake_reg_write(nvt, CIR_WAKE_FIFO_CMP_BYTES,
> -			       CIR_WAKE_FIFO_CMP_DEEP);
> +	if (cmp_deep)
> +		cmp_reg = cmp_deep;
> +	else if (num_wake_samples)
> +		cmp_reg = num_wake_samples;
> +	nvt_cir_wake_reg_write(nvt, cmp_reg, CIR_WAKE_FIFO_CMP_DEEP);
>  
>  	/* set tolerance/variance allowed per byte during wake compare */
> -	nvt_cir_wake_reg_write(nvt, CIR_WAKE_CMP_TOLERANCE,
> -			       CIR_WAKE_FIFO_CMP_TOL);
> +	nvt_cir_wake_reg_write(nvt, cmp_tolerance, CIR_WAKE_FIFO_CMP_TOL);
>  
>  	/* set sample limit count (PE interrupt raised when reached) */
>  	nvt_cir_wake_reg_write(nvt, CIR_RX_LIMIT_COUNT >> 8, CIR_WAKE_SLCH);
> @@ -434,18 +468,6 @@ static void nvt_cir_wake_regs_init(struct nvt_dev *nvt)
>  	nvt_cir_wake_reg_write(nvt, CIR_WAKE_FIFOCON_RX_TRIGGER_LEV,
>  			       CIR_WAKE_FIFOCON);
>  
> -	/*
> -	 * Enable TX and RX, specific carrier on = low, off = high, and set
> -	 * sample period (currently 50us)
> -	 */
> -	nvt_cir_wake_reg_write(nvt, CIR_WAKE_IRCON_MODE0 | CIR_WAKE_IRCON_RXEN |
> -			       CIR_WAKE_IRCON_R | CIR_WAKE_IRCON_RXINV |
> -			       CIR_WAKE_IRCON_SAMPLE_PERIOD_SEL,
> -			       CIR_WAKE_IRCON);
> -
> -	/* clear cir wake rx fifo */
> -	nvt_clear_cir_wake_fifo(nvt);
> -
>  	/* clear any and all stray interrupts */
>  	nvt_cir_wake_reg_write(nvt, 0xff, CIR_WAKE_IRSTS);
>  }
> @@ -1232,6 +1254,17 @@ static void nvt_exit(void)
>  module_param(debug, int, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(debug, "Enable debugging output");
>  
> +module_param_array(wake_samples, byte, &num_wake_samples, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(wake_samples, "FIFO sample bytes triggering wake");
> +
> +module_param(cmp_deep, byte, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(cmp_deep, "How many bytes need to compare\n"
> +			   "\t\t(0 = auto (default))");
> +
> +module_param(cmp_tolerance, byte, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(cmp_tolerance, "Data tolerance to each wake sample byte\n"
> +				"\t\t(default = 5)");
> +
>  MODULE_DEVICE_TABLE(pnp, nvt_ids);
>  MODULE_DESCRIPTION("Nuvoton W83667HG-A & W83677HG-I CIR driver");
>  
> diff --git a/drivers/media/rc/nuvoton-cir.h b/drivers/media/rc/nuvoton-cir.h
> index 07e8310..8209f84 100644
> --- a/drivers/media/rc/nuvoton-cir.h
> +++ b/drivers/media/rc/nuvoton-cir.h
> @@ -31,10 +31,6 @@
>  /* platform driver name to register */
>  #define NVT_DRIVER_NAME "nuvoton-cir"
>  
> -/* debugging module parameter */
> -static int debug;
> -
> -
>  #define nvt_pr(level, text, ...) \
>  	printk(level KBUILD_MODNAME ": " text, ## __VA_ARGS__)
>  
> @@ -64,6 +60,8 @@ static int debug;
>  #define TX_BUF_LEN 256
>  #define RX_BUF_LEN 32
>  
> +#define WAKE_FIFO_LEN 67
> +
>  struct nvt_dev {
>  	struct pnp_dev *pdev;
>  	struct rc_dev *rdev;


-- 

Cheers,
Mauro

[RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Antti Seppälä]

This patch series introduces a simple sysfs file interface for reading
and writing wakeup scancodes to rc drivers.

This is an improved version of my previous patch for nuvoton-cir which
did the same thing via module parameters. This is a more generic
approach allowing other drivers to utilize the interface as well.

I did not port winbond-cir to this method of wakeup scancode setting yet
because I don't have the hardware to test it and I wanted first to get
some comments about how the patch series looks. I did however write a
simple support to read and write scancodes to rc-loopback module.

Antti Seppälä (4):
  rc-core: Add defintions needed for sysfs callback
  rc-core: Add support for reading/writing wakeup scancodes via sysfs
  rc-loopback: Add support for reading/writing wakeup scancodes via
    sysfs
  nuvoton-cir: Add support for reading/writing wakeup scancodes via
    sysfs

 drivers/media/rc/nuvoton-cir.c |  81 ++++++++++++++++++++++++++
 drivers/media/rc/nuvoton-cir.h |   2 +
 drivers/media/rc/rc-loopback.c |  31 ++++++++++
 drivers/media/rc/rc-main.c     | 129 +++++++++++++++++++++++++++++++++++++++++
 include/media/rc-core.h        |  13 +++++
 5 files changed, 256 insertions(+)

-- 
1.8.3.2

[RFC PATCH 1/4] rc-core: Add defintions needed for sysfs callback

[Antti Seppälä]

Introduce a list for wake scancode values and add callback for sysfs
reads/writes.

Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
 include/media/rc-core.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 2f6f1f7..6ee68ac 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -20,6 +20,7 @@
 #include <linux/kfifo.h>
 #include <linux/time.h>
 #include <linux/timer.h>
+#include <linux/list.h>
 #include <media/rc-map.h>
 
 extern int rc_core_debug;
@@ -35,6 +36,16 @@ enum rc_driver_type {
 };
 
 /**
+ * struct rc_wakeup_scancode - represents a single IR code sample
+ * @value: scan code value
+ * @list: linked list pointer
+ */
+struct rc_wakeup_scancode {
+	u8			value;
+	struct list_head	list_item;
+};
+
+/**
  * struct rc_dev - represents a remote control device
  * @dev: driver model's view of this device
  * @input_name: name of the input child device
@@ -84,6 +95,7 @@ enum rc_driver_type {
  *	device doesn't interrupt host until it sees IR pulses
  * @s_learning_mode: enable wide band receiver used for learning
  * @s_carrier_report: enable carrier reports
+ * @s_wakeup_scancodes: set/get IR scancode to wake hardware from sleep states
  */
 struct rc_dev {
 	struct device			dev;
@@ -127,6 +139,7 @@ struct rc_dev {
 	void				(*s_idle)(struct rc_dev *dev, bool enable);
 	int				(*s_learning_mode)(struct rc_dev *dev, int enable);
 	int				(*s_carrier_report) (struct rc_dev *dev, int enable);
+	int				(*s_wakeup_scancodes) (struct rc_dev *dev, struct list_head *scancode_list, int write);
 };
 
 #define to_rc_dev(d) container_of(d, struct rc_dev, dev)
-- 
1.8.3.2

[RFC PATCH 2/4] rc-core: Add support for reading/writing wakeup scancodes via sysfs

[Antti Seppälä]

This patch adds support for /sys/class/rc/rc?/wakeup_scancodes file
which can be used to read/write new wakeup scancodes to/from hardware.

The contents of the scancode file are simply white space separated
bytes.

How to read:
 cat /sys/class/rc/rc?/wakeup_scancodes

How to write:
 echo "0x1 0x2 0x3" > /sys/class/rc/rc?/wakeup_scancodes

Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
 drivers/media/rc/rc-main.c | 129 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 02e2f38..a2a68f3 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -967,6 +967,119 @@ out:
 	return ret;
 }
 
+/**
+ * show_wakeup_scancodes() - shows the current IR wake scancode(s)
+ * @device:	the device descriptor
+ * @mattr:	the device attribute struct (unused)
+ * @buf:	a pointer to the output buffer
+ *
+ * This routine is a callback routine for input read the IR wake scancode(s).
+ * it is trigged by reading /sys/class/rc/rc?/wakeup_scancodes.
+ * It returns the currently active IR wake scancode or empty buffer if wake
+ * scancode is not active.
+ *
+ * dev->lock is taken to guard against races between device
+ * registration, store_wakeup_scancodes and show_wakeup_scancodes.
+ */
+static ssize_t show_wakeup_scancodes(struct device *device,
+				     struct device_attribute *mattr, char *buf)
+{
+	int ret, pos = 0;
+	struct rc_wakeup_scancode *scancode, *next;
+	LIST_HEAD(scancode_list);
+	struct rc_dev *dev = to_rc_dev(device);
+
+	if (!dev || !dev->s_wakeup_scancodes)
+		return -ENODEV;
+
+	mutex_lock(&dev->lock);
+
+	ret = dev->s_wakeup_scancodes(dev, &scancode_list, 0);
+
+	list_for_each_entry_safe_reverse(scancode, next, &scancode_list,
+					 list_item) {
+		pos += scnprintf(buf + pos, PAGE_SIZE - pos, "0x%x ",
+				 scancode->value);
+		list_del(&scancode->list_item);
+		kfree(scancode);
+	}
+	pos += scnprintf(buf + pos, PAGE_SIZE - pos, "\n");
+
+	mutex_unlock(&dev->lock);
+
+	if (ret < 0)
+		return ret;
+
+	return pos;
+}
+
+/**
+ * store_wakeup_scancodes() - changes the current IR wake scancode(s)
+ * @device:	the device descriptor
+ * @mattr:	the device attribute struct (unused)
+ * @buf:	a pointer to the input buffer
+ * @len:	length of the input buffer
+ *
+ * This routine is for changing the IR wake scancode.
+ * It is trigged by writing to /sys/class/rc/rc?/wakeup_scancodes.
+ * Writing bytes separated by white space will pass them to the hardware.
+ * Writing "" (empty) will clear active wake scancode.
+ * Returns -EINVAL if too many values or invalid values were used
+ * otherwise @len.
+ *
+ * dev->lock is taken to guard against races between device
+ * registration, store_wakeup_scancodes and show_wakeup_scancodes.
+ */
+static ssize_t store_wakeup_scancodes(struct device *device,
+				      struct device_attribute *mattr,
+				      const char *data,
+				      size_t len)
+{
+	int ret = 0, error = 0;
+	char *tmp;
+	u8 value;
+	struct rc_wakeup_scancode *scancode, *next;
+	LIST_HEAD(scancode_list);
+	struct rc_dev *dev = to_rc_dev(device);
+
+	if (!dev || !dev->s_wakeup_scancodes)
+		return -ENODEV;
+
+	mutex_lock(&dev->lock);
+
+	while ((tmp = strsep((char **) &data, " ,\t\n")) != NULL) {
+		if (!*tmp)
+			break;
+
+		if (sscanf(tmp, "0x%2hhx", &value) != 1 &&
+		    sscanf(tmp,   "%2hhx", &value) != 1) {
+			error = 1;
+			break;
+		} else {
+			scancode = kmalloc(sizeof(struct rc_wakeup_scancode),
+					   GFP_KERNEL);
+			scancode->value = value;
+			list_add(&scancode->list_item, &scancode_list);
+		}
+	}
+
+	if (error)
+		IR_dprintk(1, "Error parsing value of %s", tmp);
+	else
+		ret = dev->s_wakeup_scancodes(dev, &scancode_list, 1);
+
+	list_for_each_entry_safe(scancode, next, &scancode_list, list_item) {
+		list_del(&scancode->list_item);
+		kfree(scancode);
+	}
+
+	mutex_unlock(&dev->lock);
+	if (ret < 0)
+		return ret;
+
+	return error ? -EINVAL : len;
+}
+
 static void rc_dev_release(struct device *device)
 {
 }
@@ -1019,6 +1132,15 @@ static struct device_type rc_dev_type = {
 	.uevent		= rc_dev_uevent,
 };
 
+static struct device_attribute dev_attr_wakeup_scancodes = {
+	.attr	= {
+		.name = "wakeup_scancodes",
+		.mode = S_IRUGO | S_IWUSR,
+	},
+	.show = show_wakeup_scancodes,
+	.store = store_wakeup_scancodes,
+};
+
 struct rc_dev *rc_allocate_device(void)
 {
 	struct rc_dev *dev;
@@ -1175,6 +1297,13 @@ int rc_register_device(struct rc_dev *dev)
 		dev->enabled_protocols = rc_type;
 	}
 
+	/* Create sysfs entry only if device has wake scancode support */
+	if (dev->s_wakeup_scancodes) {
+		rc = device_create_file(&dev->dev, &dev_attr_wakeup_scancodes);
+		if (rc < 0)
+			goto out_raw;
+	}
+
 	mutex_unlock(&dev->lock);
 
 	IR_dprintk(1, "Registered rc%ld (driver: %s, remote: %s, mode %s)\n",
-- 
1.8.3.2

[RFC PATCH 3/4] rc-loopback: Add support for reading/writing wakeup scancodes via sysfs

[Antti Seppälä]

This patch adds sample support for reading/writing the wakeup scancodes
to rc-loopback device driver.

Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
 drivers/media/rc/rc-loopback.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
index 53d0282..12bd1c5 100644
--- a/drivers/media/rc/rc-loopback.c
+++ b/drivers/media/rc/rc-loopback.c
@@ -26,12 +26,14 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 #include <media/rc-core.h>
 
 #define DRIVER_NAME	"rc-loopback"
 #define dprintk(x...)	if (debug) printk(KERN_INFO DRIVER_NAME ": " x)
 #define RXMASK_REGULAR	0x1
 #define RXMASK_LEARNING	0x2
+#define WAKE_CODE_LEN	0xF
 
 static bool debug;
 
@@ -45,6 +47,7 @@ struct loopback_dev {
 	bool carrierreport;
 	u32 rxcarriermin;
 	u32 rxcarriermax;
+	u8 wake_codes[WAKE_CODE_LEN];
 };
 
 static struct loopback_dev loopdev;
@@ -176,6 +179,33 @@ static int loop_set_carrier_report(struct rc_dev *dev, int enable)
 	return 0;
 }
 
+static int loop_wakeup_scancodes(struct rc_dev *dev,
+				 struct list_head *scancode_list, int write)
+{
+	int i = 0;
+	struct rc_wakeup_scancode *scancode;
+	struct loopback_dev *lodev = dev->priv;
+
+	dprintk("%sing wakeup scancodes\n", write ? "writ" : "read");
+
+	if (write) {
+		list_for_each_entry_reverse(scancode, scancode_list,
+					    list_item) {
+			lodev->wake_codes[i] = scancode->value;
+			if (++i > WAKE_CODE_LEN)
+				return -EINVAL;
+		}
+	} else {
+		for (i = 0; i < WAKE_CODE_LEN; i++) {
+			scancode = kmalloc(sizeof(struct rc_wakeup_scancode),
+					   GFP_KERNEL);
+			scancode->value = lodev->wake_codes[i];
+			list_add(&scancode->list_item, scancode_list);
+		}
+	}
+	return 0;
+}
+
 static int __init loop_init(void)
 {
 	struct rc_dev *rc;
@@ -209,6 +239,7 @@ static int __init loop_init(void)
 	rc->s_idle		= loop_set_idle;
 	rc->s_learning_mode	= loop_set_learning_mode;
 	rc->s_carrier_report	= loop_set_carrier_report;
+	rc->s_wakeup_scancodes	= loop_wakeup_scancodes;
 
 	loopdev.txmask		= RXMASK_REGULAR;
 	loopdev.txcarrier	= 36000;
-- 
1.8.3.2

[RFC PATCH 4/4] nuvoton-cir: Add support for reading/writing wakeup scancodes via sysfs

[Antti Seppälä]

This patch adds support for reading/writing wakeup scancodes via sysfs
to nuvoton-cir hardware.

Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
 drivers/media/rc/nuvoton-cir.c | 81 ++++++++++++++++++++++++++++++++++++++++++
 drivers/media/rc/nuvoton-cir.h |  2 ++
 2 files changed, 83 insertions(+)

diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index 21ee0dc..c1b0cf2 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -531,6 +531,86 @@ static int nvt_set_tx_carrier(struct rc_dev *dev, u32 carrier)
 	return 0;
 }
 
+static int nvt_wakeup_scancodes(struct rc_dev *dev,
+				struct list_head *scancode_list, int write)
+{
+	int i = 0;
+	u8 cnt, reg, reg_learn_mode;
+	unsigned long flags;
+	struct rc_wakeup_scancode *scancode;
+	struct nvt_dev *nvt = dev->priv;
+
+	if (write) {
+		nvt_dbg_wake("%s firing, write", __func__);
+
+		reg = nvt_cir_wake_reg_read(nvt, CIR_WAKE_IRCON);
+		reg_learn_mode = reg & ~CIR_WAKE_IRCON_MODE0;
+		reg_learn_mode |= CIR_WAKE_IRCON_MODE1;
+
+		/* Lock the learn area to prevent racing with wake-isr */
+		spin_lock_irqsave(&nvt->nvt_lock, flags);
+
+		/* Enable fifo writes */
+		nvt_cir_wake_reg_write(nvt, reg_learn_mode, CIR_WAKE_IRCON);
+
+		/* Clear cir wake rx fifo */
+		nvt_clear_cir_wake_fifo(nvt);
+
+		/* Write wake samples to fifo */
+		list_for_each_entry_reverse(scancode, scancode_list,
+					    list_item) {
+			/* Prevent writing too many codes */
+			if (++i > WAKE_FIFO_LEN)
+				break;
+			nvt_cir_wake_reg_write(nvt, scancode->value,
+					       CIR_WAKE_WR_FIFO_DATA);
+		}
+
+		/* Switch cir to wakeup mode and disable fifo writing */
+		nvt_cir_wake_reg_write(nvt, reg, CIR_WAKE_IRCON);
+
+		/* Set number of bytes needed for wake */
+		nvt_cir_wake_reg_write(nvt, i ? i :
+				       CIR_WAKE_FIFO_CMP_BYTES,
+				       CIR_WAKE_FIFO_CMP_DEEP);
+
+		spin_unlock_irqrestore(&nvt->nvt_lock, flags);
+
+		if (i > WAKE_FIFO_LEN)
+			return -EINVAL;
+	} else {
+		nvt_dbg_wake("%s firing, read", __func__);
+
+		/* Scroll to index 0 */
+		while (nvt_cir_wake_reg_read(nvt, CIR_WAKE_RD_FIFO_ONLY_IDX)) {
+			nvt_cir_wake_reg_read(nvt, CIR_WAKE_RD_FIFO_ONLY);
+
+			/* Stuck index guardian */
+			if (++i > 255) {
+				nvt_dbg_wake("Idx reg is stuck!");
+				break;
+			}
+		}
+
+		/* Get size of wake fifo and allocate memory for the bytes */
+		cnt = nvt_cir_wake_reg_read(nvt, CIR_WAKE_FIFO_COUNT);
+
+		for (i = 0; i < cnt; i++) {
+			scancode = kmalloc(sizeof(struct rc_wakeup_scancode),
+					   GFP_KERNEL | GFP_ATOMIC);
+			if (!scancode)
+				return -ENOMEM;
+			scancode->value =
+				nvt_cir_wake_reg_read(nvt,
+						      CIR_WAKE_RD_FIFO_ONLY);
+			list_add(&scancode->list_item, scancode_list);
+		}
+
+		return cnt;
+	}
+
+	return 0;
+}
 /*
  * nvt_tx_ir
  *
@@ -1047,6 +1127,7 @@ static int nvt_probe(struct pnp_dev *pdev, const struct pnp_device_id *dev_id)
 	rdev->close = nvt_close;
 	rdev->tx_ir = nvt_tx_ir;
 	rdev->s_tx_carrier = nvt_set_tx_carrier;
+	rdev->s_wakeup_scancodes = nvt_wakeup_scancodes;
 	rdev->input_name = "Nuvoton w836x7hg Infrared Remote Transceiver";
 	rdev->input_phys = "nuvoton/cir0";
 	rdev->input_id.bustype = BUS_HOST;
diff --git a/drivers/media/rc/nuvoton-cir.h b/drivers/media/rc/nuvoton-cir.h
index 07e8310..24ff630 100644
--- a/drivers/media/rc/nuvoton-cir.h
+++ b/drivers/media/rc/nuvoton-cir.h
@@ -64,6 +64,8 @@ static int debug;
 #define TX_BUF_LEN 256
 #define RX_BUF_LEN 32
 
+#define WAKE_FIFO_LEN 67
+
 struct nvt_dev {
 	struct pnp_dev *pdev;
 	struct rc_dev *rdev;
-- 
1.8.3.2

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Sean Young]

On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote:
> This patch series introduces a simple sysfs file interface for reading
> and writing wakeup scancodes to rc drivers.
> 
> This is an improved version of my previous patch for nuvoton-cir which
> did the same thing via module parameters. This is a more generic
> approach allowing other drivers to utilize the interface as well.
> 
> I did not port winbond-cir to this method of wakeup scancode setting yet
> because I don't have the hardware to test it and I wanted first to get
> some comments about how the patch series looks. I did however write a
> simple support to read and write scancodes to rc-loopback module.

Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup?

This is needed for winbond-cir; I guess this should be another sysfs
file, something like "wakeup_protocol". Even if the nuvoton can only
handle one IR protocol, maybe it should be exported (readonly) via
sysfs?

I'm happy to help with a winbond-cir implementation; I have the hardware.


Sean

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Antti Seppälä]

On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote:
> On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote:
>> This patch series introduces a simple sysfs file interface for reading
>> and writing wakeup scancodes to rc drivers.
>>
>> This is an improved version of my previous patch for nuvoton-cir which
>> did the same thing via module parameters. This is a more generic
>> approach allowing other drivers to utilize the interface as well.
>>
>> I did not port winbond-cir to this method of wakeup scancode setting yet
>> because I don't have the hardware to test it and I wanted first to get
>> some comments about how the patch series looks. I did however write a
>> simple support to read and write scancodes to rc-loopback module.
>
> Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup?
>
> This is needed for winbond-cir; I guess this should be another sysfs
> file, something like "wakeup_protocol". Even if the nuvoton can only
> handle one IR protocol, maybe it should be exported (readonly) via
> sysfs?
>
> I'm happy to help with a winbond-cir implementation; I have the hardware.
>
>
> Sean

Nuvoton-cir doesn't care about the IR protocol because the hardware
compares raw IR pulse lengths and wakes the system if received pulse
is within certain tolerance of the one pre-programmed to the HW. This
approach is agnostic to the used IR protocol.

I glanced over the winbond-cir driver and porting the driver to use
sysfs for wakeup scancodes looks doable. Also a new sysfs entry for
setting the wakeup protocol would indeed be needed... I will take a
closer look at this when I have some more time.

-Antti

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Sean Young]

On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote:
> On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote:
> > On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote:
> >> This patch series introduces a simple sysfs file interface for reading
> >> and writing wakeup scancodes to rc drivers.
> >>
> >> This is an improved version of my previous patch for nuvoton-cir which
> >> did the same thing via module parameters. This is a more generic
> >> approach allowing other drivers to utilize the interface as well.
> >>
> >> I did not port winbond-cir to this method of wakeup scancode setting yet
> >> because I don't have the hardware to test it and I wanted first to get
> >> some comments about how the patch series looks. I did however write a
> >> simple support to read and write scancodes to rc-loopback module.
> >
> > Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup?
> >
> > This is needed for winbond-cir; I guess this should be another sysfs
> > file, something like "wakeup_protocol". Even if the nuvoton can only
> > handle one IR protocol, maybe it should be exported (readonly) via
> > sysfs?
> >
> > I'm happy to help with a winbond-cir implementation; I have the hardware.
> >
> >
> > Sean
> 
> Nuvoton-cir doesn't care about the IR protocol because the hardware
> compares raw IR pulse lengths and wakes the system if received pulse
> is within certain tolerance of the one pre-programmed to the HW. This
> approach is agnostic to the used IR protocol.

Your patch talks about scancodes which is something entirely different.
This should be renamed to something better.

So with the nuvoton you program a set of pulses and spaces; with the
winbond you set the protocol and the scancode. I don't think there is
any shared code here. Maybe it's better to implement the wakeup 
sysfs files in the drivers themselves rather than in rcdev, I guess that
depends on whether there are other devices that implement similar 
functionality.


Sean

> I glanced over the winbond-cir driver and porting the driver to use
> sysfs for wakeup scancodes looks doable. Also a new sysfs entry for
> setting the wakeup protocol would indeed be needed... I will take a
> closer look at this when I have some more time.
> 
> -Antti
> --
> 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: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Antti Seppälä]

On 22 January 2014 18:29, Sean Young <sean@mess.org> wrote:
> On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote:
>> On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote:
>> > On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote:
>> >> This patch series introduces a simple sysfs file interface for reading
>> >> and writing wakeup scancodes to rc drivers.
>> >>
>> >> This is an improved version of my previous patch for nuvoton-cir which
>> >> did the same thing via module parameters. This is a more generic
>> >> approach allowing other drivers to utilize the interface as well.
>> >>
>> >> I did not port winbond-cir to this method of wakeup scancode setting yet
>> >> because I don't have the hardware to test it and I wanted first to get
>> >> some comments about how the patch series looks. I did however write a
>> >> simple support to read and write scancodes to rc-loopback module.
>> >
>> > Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup?
>> >
>> > This is needed for winbond-cir; I guess this should be another sysfs
>> > file, something like "wakeup_protocol". Even if the nuvoton can only
>> > handle one IR protocol, maybe it should be exported (readonly) via
>> > sysfs?
>> >
>> > I'm happy to help with a winbond-cir implementation; I have the hardware.
>> >
>> >
>> > Sean
>>
>> Nuvoton-cir doesn't care about the IR protocol because the hardware
>> compares raw IR pulse lengths and wakes the system if received pulse
>> is within certain tolerance of the one pre-programmed to the HW. This
>> approach is agnostic to the used IR protocol.
>
> Your patch talks about scancodes which is something entirely different.
> This should be renamed to something better.
>

I agree that for the nuvoton my choice of wording (scancode) was a
poor one. Perhaps wakeup_code would suit both drivers?

> So with the nuvoton you program a set of pulses and spaces; with the
> winbond you set the protocol and the scancode. I don't think there is
> any shared code here. Maybe it's better to implement the wakeup
> sysfs files in the drivers themselves rather than in rcdev, I guess that
> depends on whether there are other devices that implement similar
> functionality.
>

The code to be shared is the logic of creating, parsing and formatting
the sysfs file. In the end the drivers are only interested in getting
a bunch of values to write to the hardware.

I was thinking about adding another file (wakeup_protocol sounds good)
which would tell what semantics are used to interpret the contents of
wakeup_code file (rc6, rc5, nec or raw). Would this be a decent
solution?

The other alternative is to push the sysfs handling to individual
drivers. I'm ok with either way. Which one should I pursue?

-Antti

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Antti Palosaari]

On 22.01.2014 21:10, Antti Seppälä wrote:
> On 22 January 2014 18:29, Sean Young <sean@mess.org> wrote:
>> On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote:
>>> On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote:
>>>> On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote:
>>>>> This patch series introduces a simple sysfs file interface for reading
>>>>> and writing wakeup scancodes to rc drivers.
>>>>>
>>>>> This is an improved version of my previous patch for nuvoton-cir which
>>>>> did the same thing via module parameters. This is a more generic
>>>>> approach allowing other drivers to utilize the interface as well.
>>>>>
>>>>> I did not port winbond-cir to this method of wakeup scancode setting yet
>>>>> because I don't have the hardware to test it and I wanted first to get
>>>>> some comments about how the patch series looks. I did however write a
>>>>> simple support to read and write scancodes to rc-loopback module.
>>>>
>>>> Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup?
>>>>
>>>> This is needed for winbond-cir; I guess this should be another sysfs
>>>> file, something like "wakeup_protocol". Even if the nuvoton can only
>>>> handle one IR protocol, maybe it should be exported (readonly) via
>>>> sysfs?
>>>>
>>>> I'm happy to help with a winbond-cir implementation; I have the hardware.
>>>>
>>>>
>>>> Sean
>>>
>>> Nuvoton-cir doesn't care about the IR protocol because the hardware
>>> compares raw IR pulse lengths and wakes the system if received pulse
>>> is within certain tolerance of the one pre-programmed to the HW. This
>>> approach is agnostic to the used IR protocol.
>>
>> Your patch talks about scancodes which is something entirely different.
>> This should be renamed to something better.
>>
>
> I agree that for the nuvoton my choice of wording (scancode) was a
> poor one. Perhaps wakeup_code would suit both drivers?
>
>> So with the nuvoton you program a set of pulses and spaces; with the
>> winbond you set the protocol and the scancode. I don't think there is
>> any shared code here. Maybe it's better to implement the wakeup
>> sysfs files in the drivers themselves rather than in rcdev, I guess that
>> depends on whether there are other devices that implement similar
>> functionality.
>>
>
> The code to be shared is the logic of creating, parsing and formatting
> the sysfs file. In the end the drivers are only interested in getting
> a bunch of values to write to the hardware.
>
> I was thinking about adding another file (wakeup_protocol sounds good)
> which would tell what semantics are used to interpret the contents of
> wakeup_code file (rc6, rc5, nec or raw). Would this be a decent
> solution?
>
> The other alternative is to push the sysfs handling to individual
> drivers. I'm ok with either way. Which one should I pursue?

RTL2832U supports also that function. Could you study it too at least 
check that implementation will be suitable for it?

regards
Antti

-- 
http://palosaari.fi/

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Sean Young]

On Wed, Jan 22, 2014 at 09:10:58PM +0200, Antti Seppälä wrote:
> On 22 January 2014 18:29, Sean Young <sean@mess.org> wrote:
> > On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote:
> >> On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote:
> >> > On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote:
> >> >> This patch series introduces a simple sysfs file interface for reading
> >> >> and writing wakeup scancodes to rc drivers.
> >> >>
> >> >> This is an improved version of my previous patch for nuvoton-cir which
> >> >> did the same thing via module parameters. This is a more generic
> >> >> approach allowing other drivers to utilize the interface as well.
> >> >>
> >> >> I did not port winbond-cir to this method of wakeup scancode setting yet
> >> >> because I don't have the hardware to test it and I wanted first to get
> >> >> some comments about how the patch series looks. I did however write a
> >> >> simple support to read and write scancodes to rc-loopback module.
> >> >
> >> > Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup?
> >> >
> >> > This is needed for winbond-cir; I guess this should be another sysfs
> >> > file, something like "wakeup_protocol". Even if the nuvoton can only
> >> > handle one IR protocol, maybe it should be exported (readonly) via
> >> > sysfs?
> >> >
> >> > I'm happy to help with a winbond-cir implementation; I have the hardware.
> >> >
> >> >
> >> > Sean
> >>
> >> Nuvoton-cir doesn't care about the IR protocol because the hardware
> >> compares raw IR pulse lengths and wakes the system if received pulse
> >> is within certain tolerance of the one pre-programmed to the HW. This
> >> approach is agnostic to the used IR protocol.
> >
> > Your patch talks about scancodes which is something entirely different.
> > This should be renamed to something better.
> >
> 
> I agree that for the nuvoton my choice of wording (scancode) was a
> poor one. Perhaps wakeup_code would suit both drivers?
> 
> > So with the nuvoton you program a set of pulses and spaces; with the
> > winbond you set the protocol and the scancode. I don't think there is
> > any shared code here. Maybe it's better to implement the wakeup
> > sysfs files in the drivers themselves rather than in rcdev, I guess that
> > depends on whether there are other devices that implement similar
> > functionality.
> >
> 
> The code to be shared is the logic of creating, parsing and formatting
> the sysfs file. In the end the drivers are only interested in getting
> a bunch of values to write to the hardware.
> 
> I was thinking about adding another file (wakeup_protocol sounds good)
> which would tell what semantics are used to interpret the contents of
> wakeup_code file (rc6, rc5, nec or raw). Would this be a decent
> solution?

Good idea. I like it.

Sean

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Mauro Carvalho Chehab]

Em Wed, 22 Jan 2014 21:00:24 +0000
Sean Young <sean@mess.org> escreveu:

> On Wed, Jan 22, 2014 at 09:10:58PM +0200, Antti Seppälä wrote:
> > On 22 January 2014 18:29, Sean Young <sean@mess.org> wrote:
> > > On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote:
> > >> On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote:
> > >> > On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote:
> > >> >> This patch series introduces a simple sysfs file interface for reading
> > >> >> and writing wakeup scancodes to rc drivers.
> > >> >>
> > >> >> This is an improved version of my previous patch for nuvoton-cir which
> > >> >> did the same thing via module parameters. This is a more generic
> > >> >> approach allowing other drivers to utilize the interface as well.
> > >> >>
> > >> >> I did not port winbond-cir to this method of wakeup scancode setting yet
> > >> >> because I don't have the hardware to test it and I wanted first to get
> > >> >> some comments about how the patch series looks. I did however write a
> > >> >> simple support to read and write scancodes to rc-loopback module.
> > >> >
> > >> > Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup?
> > >> >
> > >> > This is needed for winbond-cir; I guess this should be another sysfs
> > >> > file, something like "wakeup_protocol". Even if the nuvoton can only
> > >> > handle one IR protocol, maybe it should be exported (readonly) via
> > >> > sysfs?
> > >> >
> > >> > I'm happy to help with a winbond-cir implementation; I have the hardware.
> > >> >
> > >> >
> > >> > Sean
> > >>
> > >> Nuvoton-cir doesn't care about the IR protocol because the hardware
> > >> compares raw IR pulse lengths and wakes the system if received pulse
> > >> is within certain tolerance of the one pre-programmed to the HW. This
> > >> approach is agnostic to the used IR protocol.
> > >
> > > Your patch talks about scancodes which is something entirely different.
> > > This should be renamed to something better.
> > >
> > 
> > I agree that for the nuvoton my choice of wording (scancode) was a
> > poor one. Perhaps wakeup_code would suit both drivers?
> > 
> > > So with the nuvoton you program a set of pulses and spaces; with the
> > > winbond you set the protocol and the scancode. I don't think there is
> > > any shared code here. Maybe it's better to implement the wakeup
> > > sysfs files in the drivers themselves rather than in rcdev, I guess that
> > > depends on whether there are other devices that implement similar
> > > functionality.
> > >
> > 
> > The code to be shared is the logic of creating, parsing and formatting
> > the sysfs file. In the end the drivers are only interested in getting
> > a bunch of values to write to the hardware.
> > 
> > I was thinking about adding another file (wakeup_protocol sounds good)
> > which would tell what semantics are used to interpret the contents of
> > wakeup_code file (rc6, rc5, nec or raw). Would this be a decent
> > solution?
> 
> Good idea. I like it.

Not sure if you saw it, but there's already another patchset proposing
that, that got submitted before this changeset:
	https://patchwork.linuxtv.org/patch/21625/
	
> 
> Sean


-- 

Cheers,
Mauro

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Antti Seppälä]

On 23 January 2014 00:01, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote:
> Not sure if you saw it, but there's already another patchset proposing
> that, that got submitted before this changeset:
>         https://patchwork.linuxtv.org/patch/21625/

I actually didn't notice that until now. Seems quite a similar kind of
approach with even more advanced features than what I had in mind
(namely the scancode filtering and masking).

However it looks like that patchset has the same drawback about not
knowing which protocol to use for the wakeup scancode as was pointed
from my patch.

I think I'll try to come up with a new patch addressing the comments
I've seen so far.

-Antti

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Mauro Carvalho Chehab]

Em Thu, 23 Jan 2014 21:11:09 +0200
Antti Seppälä <a.seppala@gmail.com> escreveu:

> On 23 January 2014 00:01, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote:
> > Not sure if you saw it, but there's already another patchset proposing
> > that, that got submitted before this changeset:
> >         https://patchwork.linuxtv.org/patch/21625/
> 
> I actually didn't notice that until now. Seems quite a similar kind of
> approach with even more advanced features than what I had in mind
> (namely the scancode filtering and masking).
> 
> However it looks like that patchset has the same drawback about not
> knowing which protocol to use for the wakeup scancode as was pointed
> from my patch.

Well, the protocol is important only if there are two or more active
RCs that produce the same IR code on different protocols.

Also, from the sysfs description made by James, it seems clear to me
that the protocol to be used is the current protocol.

I think is an unlikely border case to have some hardware that supports
more than one IR protocols enabled for the wakeup to work, so James
patch looks ok on my eyes.

Also, nothing prevents to add latter a wakeup_filter_protocol sysfs node
to allow to filter the wakeup scancode to a protocol set different than
the one(s) currently enabled.

> I think I'll try to come up with a new patch addressing the comments
> I've seen so far.

I guess I'll merge James approach, as there are a pile of other patches
depending on it. If we need latter to distinguish between current_protocol
and the wakeup one, as I said, a latter patch can add a
"wakeup_filter_protocol" sysfs node to specify it.

Regards,
Mauro

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Antti Seppälä]

On 4 February 2014 19:54, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote:
> Em Thu, 23 Jan 2014 21:11:09 +0200
> Antti Seppälä <a.seppala@gmail.com> escreveu:
>
>> On 23 January 2014 00:01, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote:
>> > Not sure if you saw it, but there's already another patchset proposing
>> > that, that got submitted before this changeset:
>> >         https://patchwork.linuxtv.org/patch/21625/
>>
>> I actually didn't notice that until now. Seems quite a similar kind of
>> approach with even more advanced features than what I had in mind
>> (namely the scancode filtering and masking).
>>
>> However it looks like that patchset has the same drawback about not
>> knowing which protocol to use for the wakeup scancode as was pointed
>> from my patch.
>
> Well, the protocol is important only if there are two or more active
> RCs that produce the same IR code on different protocols.
>
> Also, from the sysfs description made by James, it seems clear to me
> that the protocol to be used is the current protocol.
>
> I think is an unlikely border case to have some hardware that supports
> more than one IR protocols enabled for the wakeup to work, so James
> patch looks ok on my eyes.
>
> Also, nothing prevents to add latter a wakeup_filter_protocol sysfs node
> to allow to filter the wakeup scancode to a protocol set different than
> the one(s) currently enabled.
>
>> I think I'll try to come up with a new patch addressing the comments
>> I've seen so far.
>
> I guess I'll merge James approach, as there are a pile of other patches
> depending on it. If we need latter to distinguish between current_protocol
> and the wakeup one, as I said, a latter patch can add a
> "wakeup_filter_protocol" sysfs node to specify it.
>
> Regards,
> Mauro

Hi Mauro.

How do you want to proceed with the nuvoton wakeup I initially set out to do?

To wake up with nuvoton-cir we need to program several raw ir
pulse/space lengths to the hardware and not a scancode. James's
approach doesn't support this.

To keep things simple maybe module parameters in my first patch
(http://www.mail-archive.com/linux-media@vger.kernel.org/msg69782.html)
are then the best thing to do for nuvoton? Other drivers can probably
adapt to the suggested filter api.

-Antti

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Mauro Carvalho Chehab]

Hi Antti,

Em Wed, 05 Feb 2014 09:03:29 +0200
Antti Seppälä <a.seppala@gmail.com> escreveu:

> On 4 February 2014 19:54, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote:
> > Em Thu, 23 Jan 2014 21:11:09 +0200
> > Antti Seppälä <a.seppala@gmail.com> escreveu:
> >
> >> On 23 January 2014 00:01, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote:
> >> > Not sure if you saw it, but there's already another patchset proposing
> >> > that, that got submitted before this changeset:
> >> >         https://patchwork.linuxtv.org/patch/21625/
> >>
> >> I actually didn't notice that until now. Seems quite a similar kind of
> >> approach with even more advanced features than what I had in mind
> >> (namely the scancode filtering and masking).
> >>
> >> However it looks like that patchset has the same drawback about not
> >> knowing which protocol to use for the wakeup scancode as was pointed
> >> from my patch.
> >
> > Well, the protocol is important only if there are two or more active
> > RCs that produce the same IR code on different protocols.
> >
> > Also, from the sysfs description made by James, it seems clear to me
> > that the protocol to be used is the current protocol.
> >
> > I think is an unlikely border case to have some hardware that supports
> > more than one IR protocols enabled for the wakeup to work, so James
> > patch looks ok on my eyes.
> >
> > Also, nothing prevents to add latter a wakeup_filter_protocol sysfs node
> > to allow to filter the wakeup scancode to a protocol set different than
> > the one(s) currently enabled.
> >
> >> I think I'll try to come up with a new patch addressing the comments
> >> I've seen so far.
> >
> > I guess I'll merge James approach, as there are a pile of other patches
> > depending on it. If we need latter to distinguish between current_protocol
> > and the wakeup one, as I said, a latter patch can add a
> > "wakeup_filter_protocol" sysfs node to specify it.
> >
> > Regards,
> > Mauro
> 
> Hi Mauro.
> 
> How do you want to proceed with the nuvoton wakeup I initially set out to do?
> 
> To wake up with nuvoton-cir we need to program several raw ir
> pulse/space lengths to the hardware and not a scancode. James's
> approach doesn't support this.
> 
> To keep things simple maybe module parameters in my first patch
> (http://www.mail-archive.com/linux-media@vger.kernel.org/msg69782.html)
> are then the best thing to do for nuvoton? Other drivers can probably
> adapt to the suggested filter api.

Well, you're basically telling that you need an IR encoder to convert
from protocol/scancode into pulse/space lengths. 

As we need/want a portable interface that will provide the same API
to  userspace, no matter what hardware, I can see two alternatives:

1) Use a lirc-like API for that, using the IR encoder code in userspace;

2) The same way as we mapped IR decoders, add IR encoders that can be
used either for IR TX or to provide IR wakeup codes.

IMO, (2) is a much more elegant option.

Regards,
Mauro

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[James Hogan]

Hi Antti,

On 05/02/14 07:03, Antti Seppälä wrote:
> To wake up with nuvoton-cir we need to program several raw ir
> pulse/space lengths to the hardware and not a scancode. James's
> approach doesn't support this.

Do the raw pulse/space lengths your hardware requires correspond to a
single IR packet (mapping to a single scancode)?

If so then my API is simply at a higher level of abstraction. I think
this has the following advantages:
* userspace sees a consistent interface at the same level of abstraction
as it already has access to from input subsystem (i.e. scancodes). I.e.
it doesn't need to care which IR device is in use, whether it does
raw/hardware decode, or the details of the timings of the current protocol.
* it supports hardware decoders which filter on the demodulated data
rather than the raw pulse/space lengths.

Of course to support this we'd need some per-protocol code to convert a
scancode back to pulse/space lengths. I'd like to think that code could
be generic, maybe as helper functions which multiple drivers could use,
which could also handle corner cases of the API in a consistent way
(e.g. user providing filter mask covering multiple scancodes, which
presumably pulse/space).

I see I've just crossed emails with Mauro who has just suggested
something similar. I agree that his (2) is the more elegant option.

Cheers
James

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[James Hogan]

On 05/02/14 09:39, James Hogan wrote:
> Hi Antti,
> 
> On 05/02/14 07:03, Antti Seppälä wrote:
>> To wake up with nuvoton-cir we need to program several raw ir
>> pulse/space lengths to the hardware and not a scancode. James's
>> approach doesn't support this.
> 
> Do the raw pulse/space lengths your hardware requires correspond to a
> single IR packet (mapping to a single scancode)?
> 
> If so then my API is simply at a higher level of abstraction. I think
> this has the following advantages:
> * userspace sees a consistent interface at the same level of abstraction
> as it already has access to from input subsystem (i.e. scancodes). I.e.
> it doesn't need to care which IR device is in use, whether it does
> raw/hardware decode, or the details of the timings of the current protocol.
> * it supports hardware decoders which filter on the demodulated data
> rather than the raw pulse/space lengths.
> 
> Of course to support this we'd need some per-protocol code to convert a
> scancode back to pulse/space lengths. I'd like to think that code could
> be generic, maybe as helper functions which multiple drivers could use,
> which could also handle corner cases of the API in a consistent way
> (e.g. user providing filter mask covering multiple scancodes, which
> presumably pulse/space).

hmm, I didn't complete that sentence :(.
I meant:
..., which presumably pulse/space can't really represent very easily).

Cheers
James

> 
> I see I've just crossed emails with Mauro who has just suggested
> something similar. I agree that his (2) is the more elegant option.
> 
> Cheers
> James
> 

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Antti Seppälä]

On 5 February 2014 11:42, James Hogan <james.hogan@imgtec.com> wrote:
> On 05/02/14 09:39, James Hogan wrote:
>> Hi Antti,
>>
>> On 05/02/14 07:03, Antti Seppälä wrote:
>>> To wake up with nuvoton-cir we need to program several raw ir
>>> pulse/space lengths to the hardware and not a scancode. James's
>>> approach doesn't support this.
>>
>> Do the raw pulse/space lengths your hardware requires correspond to a
>> single IR packet (mapping to a single scancode)?
>>
>> If so then my API is simply at a higher level of abstraction. I think
>> this has the following advantages:
>> * userspace sees a consistent interface at the same level of abstraction
>> as it already has access to from input subsystem (i.e. scancodes). I.e.
>> it doesn't need to care which IR device is in use, whether it does
>> raw/hardware decode, or the details of the timings of the current protocol.
>> * it supports hardware decoders which filter on the demodulated data
>> rather than the raw pulse/space lengths.
>>
>> Of course to support this we'd need some per-protocol code to convert a
>> scancode back to pulse/space lengths. I'd like to think that code could
>> be generic, maybe as helper functions which multiple drivers could use,
>> which could also handle corner cases of the API in a consistent way
>> (e.g. user providing filter mask covering multiple scancodes, which
>> presumably pulse/space).
>
> hmm, I didn't complete that sentence :(.
> I meant:
> ..., which presumably pulse/space can't really represent very easily).
>
> Cheers
> James
>
>>
>> I see I've just crossed emails with Mauro who has just suggested
>> something similar. I agree that his (2) is the more elegant option.
>>

Yes, in nuvoton the ir pulses correspond to a scancode (or part of a scancode)

After giving it some thought I agree that using scancodes is the most
elegant way for specifying wakeup commands. Too bad that nuvoton does
not work with scancodes.
I pretty much agree with Mauro that the right solution would be to
write an IR encoder and use it to convert the given scancode back to a
format understood by nuvoton.

Writing IR encoders for all the protocols and an encoder selector
functionality is quite labourous and sadly I don't have time for that
anytime soon. If anyone wants to step up I'd be more than happy to
help though :)

-Antti

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Mauro Carvalho Chehab]

Em Wed, 05 Feb 2014 20:16:04 +0200
Antti Seppälä <a.seppala@gmail.com> escreveu:

> On 5 February 2014 11:42, James Hogan <james.hogan@imgtec.com> wrote:
> > On 05/02/14 09:39, James Hogan wrote:
> >> Hi Antti,
> >>
> >> On 05/02/14 07:03, Antti Seppälä wrote:
> >>> To wake up with nuvoton-cir we need to program several raw ir
> >>> pulse/space lengths to the hardware and not a scancode. James's
> >>> approach doesn't support this.
> >>
> >> Do the raw pulse/space lengths your hardware requires correspond to a
> >> single IR packet (mapping to a single scancode)?
> >>
> >> If so then my API is simply at a higher level of abstraction. I think
> >> this has the following advantages:
> >> * userspace sees a consistent interface at the same level of abstraction
> >> as it already has access to from input subsystem (i.e. scancodes). I.e.
> >> it doesn't need to care which IR device is in use, whether it does
> >> raw/hardware decode, or the details of the timings of the current protocol.
> >> * it supports hardware decoders which filter on the demodulated data
> >> rather than the raw pulse/space lengths.
> >>
> >> Of course to support this we'd need some per-protocol code to convert a
> >> scancode back to pulse/space lengths. I'd like to think that code could
> >> be generic, maybe as helper functions which multiple drivers could use,
> >> which could also handle corner cases of the API in a consistent way
> >> (e.g. user providing filter mask covering multiple scancodes, which
> >> presumably pulse/space).
> >
> > hmm, I didn't complete that sentence :(.
> > I meant:
> > ..., which presumably pulse/space can't really represent very easily).
> >
> > Cheers
> > James
> >
> >>
> >> I see I've just crossed emails with Mauro who has just suggested
> >> something similar. I agree that his (2) is the more elegant option.
> >>
> 
> Yes, in nuvoton the ir pulses correspond to a scancode (or part of a scancode)
> 
> After giving it some thought I agree that using scancodes is the most
> elegant way for specifying wakeup commands. Too bad that nuvoton does
> not work with scancodes.
> I pretty much agree with Mauro that the right solution would be to
> write an IR encoder and use it to convert the given scancode back to a
> format understood by nuvoton.

Ok, as we all agreed, I'll merge the remaining patches from James.

> Writing IR encoders for all the protocols and an encoder selector
> functionality is quite labourous and sadly I don't have time for that
> anytime soon. If anyone wants to step up I'd be more than happy to
> help though :)

I suspect that writing one IR encoder should not be hard, as there
are already some on LIRC userspace.

I would love to have some time to write at least a few IR encoders,
but, unfortunately, I would not have any time soon.

Regards,
Mauro

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[James Hogan]

On 05/02/14 21:21, Mauro Carvalho Chehab wrote:
> Em Wed, 05 Feb 2014 20:16:04 +0200
> Antti Seppälä <a.seppala@gmail.com> escreveu:
> 
>> On 5 February 2014 11:42, James Hogan <james.hogan@imgtec.com> wrote:
>>> On 05/02/14 09:39, James Hogan wrote:
>>>> Hi Antti,
>>>>
>>>> On 05/02/14 07:03, Antti Seppälä wrote:
>>>>> To wake up with nuvoton-cir we need to program several raw ir
>>>>> pulse/space lengths to the hardware and not a scancode. James's
>>>>> approach doesn't support this.
>>>>
>>>> Do the raw pulse/space lengths your hardware requires correspond to a
>>>> single IR packet (mapping to a single scancode)?
>>>>
>>>> If so then my API is simply at a higher level of abstraction. I think
>>>> this has the following advantages:
>>>> * userspace sees a consistent interface at the same level of abstraction
>>>> as it already has access to from input subsystem (i.e. scancodes). I.e.
>>>> it doesn't need to care which IR device is in use, whether it does
>>>> raw/hardware decode, or the details of the timings of the current protocol.
>>>> * it supports hardware decoders which filter on the demodulated data
>>>> rather than the raw pulse/space lengths.
>>>>
>>>> Of course to support this we'd need some per-protocol code to convert a
>>>> scancode back to pulse/space lengths. I'd like to think that code could
>>>> be generic, maybe as helper functions which multiple drivers could use,
>>>> which could also handle corner cases of the API in a consistent way
>>>> (e.g. user providing filter mask covering multiple scancodes, which
>>>> presumably pulse/space).
>>>
>>> hmm, I didn't complete that sentence :(.
>>> I meant:
>>> ..., which presumably pulse/space can't really represent very easily).
>>>
>>> Cheers
>>> James
>>>
>>>>
>>>> I see I've just crossed emails with Mauro who has just suggested
>>>> something similar. I agree that his (2) is the more elegant option.
>>>>
>>
>> Yes, in nuvoton the ir pulses correspond to a scancode (or part of a scancode)
>>
>> After giving it some thought I agree that using scancodes is the most
>> elegant way for specifying wakeup commands. Too bad that nuvoton does
>> not work with scancodes.
>> I pretty much agree with Mauro that the right solution would be to
>> write an IR encoder and use it to convert the given scancode back to a
>> format understood by nuvoton.
> 
> Ok, as we all agreed, I'll merge the remaining patches from James.

Thanks :)

> 
>> Writing IR encoders for all the protocols and an encoder selector
>> functionality is quite labourous and sadly I don't have time for that
>> anytime soon. If anyone wants to step up I'd be more than happy to
>> help though :)
> 
> I suspect that writing one IR encoder should not be hard, as there
> are already some on LIRC userspace.
> 
> I would love to have some time to write at least a few IR encoders,
> but, unfortunately, I would not have any time soon.

For fun, I did some experimentation yesterday evening with adding basic
generic IR encoding (just NEC implemented so far). Encoders can
certainly be a lot simpler than decoders.

I'll submit a few RFC patches later so Antti can see whether it would be
suitable for nuvoton.

Cheers
James

Re: [RFC PATCH 0/4] rc: Adding support for sysfs wakeup scancodes

[Mauro Carvalho Chehab]

Em Thu, 06 Feb 2014 10:46:04 +0000
James Hogan <james.hogan@imgtec.com> escreveu:

> On 05/02/14 21:21, Mauro Carvalho Chehab wrote:
> > Em Wed, 05 Feb 2014 20:16:04 +0200
> > Antti Seppälä <a.seppala@gmail.com> escreveu:
> > 
> >> On 5 February 2014 11:42, James Hogan <james.hogan@imgtec.com> wrote:
> >>> On 05/02/14 09:39, James Hogan wrote:
> >>>> Hi Antti,
> >>>>
> >>>> On 05/02/14 07:03, Antti Seppälä wrote:
> >>>>> To wake up with nuvoton-cir we need to program several raw ir
> >>>>> pulse/space lengths to the hardware and not a scancode. James's
> >>>>> approach doesn't support this.
> >>>>
> >>>> Do the raw pulse/space lengths your hardware requires correspond to a
> >>>> single IR packet (mapping to a single scancode)?
> >>>>
> >>>> If so then my API is simply at a higher level of abstraction. I think
> >>>> this has the following advantages:
> >>>> * userspace sees a consistent interface at the same level of abstraction
> >>>> as it already has access to from input subsystem (i.e. scancodes). I.e.
> >>>> it doesn't need to care which IR device is in use, whether it does
> >>>> raw/hardware decode, or the details of the timings of the current protocol.
> >>>> * it supports hardware decoders which filter on the demodulated data
> >>>> rather than the raw pulse/space lengths.
> >>>>
> >>>> Of course to support this we'd need some per-protocol code to convert a
> >>>> scancode back to pulse/space lengths. I'd like to think that code could
> >>>> be generic, maybe as helper functions which multiple drivers could use,
> >>>> which could also handle corner cases of the API in a consistent way
> >>>> (e.g. user providing filter mask covering multiple scancodes, which
> >>>> presumably pulse/space).
> >>>
> >>> hmm, I didn't complete that sentence :(.
> >>> I meant:
> >>> ..., which presumably pulse/space can't really represent very easily).
> >>>
> >>> Cheers
> >>> James
> >>>
> >>>>
> >>>> I see I've just crossed emails with Mauro who has just suggested
> >>>> something similar. I agree that his (2) is the more elegant option.
> >>>>
> >>
> >> Yes, in nuvoton the ir pulses correspond to a scancode (or part of a scancode)
> >>
> >> After giving it some thought I agree that using scancodes is the most
> >> elegant way for specifying wakeup commands. Too bad that nuvoton does
> >> not work with scancodes.
> >> I pretty much agree with Mauro that the right solution would be to
> >> write an IR encoder and use it to convert the given scancode back to a
> >> format understood by nuvoton.
> > 
> > Ok, as we all agreed, I'll merge the remaining patches from James.
> 
> Thanks :)

Well, Rob asked some changes at the DT binding pathes. I'll tag the
patches as changes requested. Please re-submit the remaining ones after
fixing the pointed issues and getting his ack. Except for that, the
series look OK on my eyes.

> > 
> >> Writing IR encoders for all the protocols and an encoder selector
> >> functionality is quite labourous and sadly I don't have time for that
> >> anytime soon. If anyone wants to step up I'd be more than happy to
> >> help though :)
> > 
> > I suspect that writing one IR encoder should not be hard, as there
> > are already some on LIRC userspace.
> > 
> > I would love to have some time to write at least a few IR encoders,
> > but, unfortunately, I would not have any time soon.
> 
> For fun, I did some experimentation yesterday evening with adding basic
> generic IR encoding (just NEC implemented so far). Encoders can
> certainly be a lot simpler than decoders.

Yes, you don't need to deal with time shifts there, nor with missing
events ;)

Well, using the encoders also for TX (as an another ioctl at LIRC) 
would require some extra bits to handle things like carrier frequency,
duty cycle, etc, but lirc should be able to handle those already.

So, a future patch expanding its usage to lirc would just need to
add the proper binding.

Btw, I just noticed that the LIRC UAPI interface is still at the
wrong place. It is at:
	include/media/lirc.h
but it should be at:
	include/uapi/.../lirc.h

I'm wandering why nobody didn't notice it yet when compiling
Lirc userspace... Perhaps userspace is just keeping a copy of
that file without any process to sync it from the Kernel userspace
headers.

> I'll submit a few RFC patches later so Antti can see whether it would be
> suitable for nuvoton.

Thanks for that!

Regards,
Mauro