[RFC 0/4] rc: ir-raw: Add encode, implement NEC encode

[RFC 0/4] rc: ir-raw: Add encode, implement NEC encode

[James Hogan]

A recent discussion about proposed interfaces for setting up the
hardware wakeup filter lead to the conclusion that it could help to have
the generic capability to encode and modulate scancodes into raw IR
events so that drivers for hardware with a low level wake filter (on the
level of pulse/space durations) can still easily implement the higher
level scancode interface that is proposed.

This patchset is just a quick experiment to suggest how it might work.
I'm not familiar with the hardware that could use it so it could well be
a bit misdesigned in places (e.g. what sort of buffer length would the
hardware have, do we need to support any kind of partial encoding, can
the hardware/driver easily take care of allowing for a margin of
error?).


The first patch adds an encode callback to the existing raw ir handler
struct and a helper function to encode a scancode for a given protocol.

The third patch implements encode for NEC. The modulation is abstracted
to use functions in patch 2 (pulse-distance is used by multiple
protocols).

Finally for debug purposes patch 4 modifies img-ir-raw to loop back the
encoded data when a filter is altered. Should be pretty easy to apply
similarly to any raw ir driver to try it out.

James Hogan (4):
  rc: ir-raw: add scancode encoder callback
  rc: ir-raw: add modulation helpers
  rc: ir-nec-decoder: add encode capability
  DEBUG: rc: img-ir: raw: Add loopback on s_filter

 drivers/media/rc/img-ir/img-ir-raw.c | 30 ++++++++++++
 drivers/media/rc/ir-nec-decoder.c    | 91 ++++++++++++++++++++++++++++++++++++
 drivers/media/rc/ir-raw.c            | 70 +++++++++++++++++++++++++++
 drivers/media/rc/rc-core-priv.h      | 46 ++++++++++++++++++
 include/media/rc-core.h              |  3 ++
 5 files changed, 240 insertions(+)

-- 
1.8.3.2

[RFC 1/4] rc: ir-raw: add scancode encoder callback

[James Hogan]

Add a callback to raw ir handlers for encoding and modulating a scancode
to a set of raw events. This could be used for transmit, or for
converting a wakeup scancode filter to a form that is more suitable for
raw hardware wake up filters.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
 drivers/media/rc/ir-raw.c       | 37 +++++++++++++++++++++++++++++++++++++
 drivers/media/rc/rc-core-priv.h |  2 ++
 include/media/rc-core.h         |  3 +++
 3 files changed, 42 insertions(+)

diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index 79a9cb6..9aea407 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -240,6 +240,43 @@ ir_raw_get_allowed_protocols(void)
 	return protocols;
 }
 
+/**
+ * ir_raw_encode_scancode() - Encode a scancode as raw events
+ *
+ * @protocols:		permitted protocols
+ * @scancode:		scancode filter describing a single scancode
+ * @events:		array of raw events to write into
+ * @max:		max number of raw events
+ *
+ * Attempts to encode the scancode as raw events.
+ *
+ * Returns -EINVAL if the scancode is ambiguous or invalid, or there isn't
+ * enough space in the array to fit the encoding.
+ *
+ * Returns -ENOTSUPP if no compatible encoder is found.
+ */
+int ir_raw_encode_scancode(u64 protocols,
+			   const struct rc_scancode_filter *scancode,
+			   struct ir_raw_event *events, unsigned int max)
+{
+	struct ir_raw_handler *handler;
+	int ret = -ENOTSUPP;
+
+	mutex_lock(&ir_raw_handler_lock);
+	list_for_each_entry(handler, &ir_raw_handler_list, list) {
+		if (handler->protocols & protocols && handler->encode) {
+			ret = handler->encode(protocols, scancode, events, max);
+			if (ret >= 0)
+				break;
+		}
+	}
+	mutex_unlock(&ir_raw_handler_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(ir_raw_encode_scancode);
+
+
 /*
  * Used to (un)register raw event clients
  */
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index dc3b0b7..dfbaad0 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -25,6 +25,8 @@ struct ir_raw_handler {
 
 	u64 protocols; /* which are handled by this handler */
 	int (*decode)(struct rc_dev *dev, struct ir_raw_event event);
+	int (*encode)(u64 protocols, const struct rc_scancode_filter *scancode,
+		      struct ir_raw_event *events, unsigned int max);
 
 	/* These two should only be used by the lirc decoder */
 	int (*raw_register)(struct rc_dev *dev);
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 4a72176..7bd66be 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -234,6 +234,9 @@ int ir_raw_event_store_edge(struct rc_dev *dev, enum raw_event_type type);
 int ir_raw_event_store_with_filter(struct rc_dev *dev,
 				struct ir_raw_event *ev);
 void ir_raw_event_set_idle(struct rc_dev *dev, bool idle);
+int ir_raw_encode_scancode(u64 protocols,
+			   const struct rc_scancode_filter *scancode,
+			   struct ir_raw_event *events, unsigned int max);
 
 static inline void ir_raw_event_reset(struct rc_dev *dev)
 {
-- 
1.8.3.2

[RFC 2/4] rc: ir-raw: add modulation helpers

[James Hogan]

Add IR encoding helpers, particularly for pulse-distance modulation as
used by the NEC protocol.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
 drivers/media/rc/ir-raw.c       | 33 +++++++++++++++++++++++++++++++
 drivers/media/rc/rc-core-priv.h | 44 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index 9aea407..ae7b445 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -240,6 +240,39 @@ ir_raw_get_allowed_protocols(void)
 	return protocols;
 }
 
+int ir_raw_gen_pd(struct ir_raw_event **ev, unsigned int max,
+		  const struct ir_raw_timings_pd *timings,
+		  unsigned int n, unsigned int data)
+{
+	int i;
+
+	i = 2 + n*2;
+	if (timings->header_pulse)
+		i += 2;
+	if (max < i)
+		return -EINVAL;
+
+	if (timings->header_pulse)
+		ir_raw_gen_pulse_space(ev, timings->header_pulse,
+				       timings->header_space);
+
+	if (timings->msb_first) {
+		for (i = n - 1; i >= 0; --i)
+			ir_raw_gen_pulse_space(ev, timings->bit_pulse,
+					timings->bit_space[(data >> i) & 1]);
+	} else {
+		for (i = 0; i < n; ++i, data >>= 1)
+			ir_raw_gen_pulse_space(ev, timings->bit_pulse,
+					timings->bit_space[data & 1]);
+	}
+
+	ir_raw_gen_pulse_space(ev, timings->trailer_pulse,
+			       timings->trailer_space);
+
+	return 0;
+}
+EXPORT_SYMBOL(ir_raw_gen_pd);
+
 /**
  * ir_raw_encode_scancode() - Encode a scancode as raw events
  *
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index dfbaad0..a77ad96 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -153,6 +153,50 @@ static inline bool is_timing_event(struct ir_raw_event ev)
 #define TO_US(duration)			DIV_ROUND_CLOSEST((duration), 1000)
 #define TO_STR(is_pulse)		((is_pulse) ? "pulse" : "space")
 
+/* functions for IR encoders */
+
+static inline void init_ir_raw_event_duration(struct ir_raw_event *ev,
+					      unsigned int pulse,
+					      u32 duration)
+{
+	init_ir_raw_event(ev);
+	ev->duration = duration;
+	ev->pulse = pulse;
+}
+
+static inline void ir_raw_gen_pulse_space(struct ir_raw_event **ev,
+					  unsigned int pulse_width,
+					  unsigned int space_width)
+{
+	init_ir_raw_event_duration((*ev)++, 1, pulse_width);
+	init_ir_raw_event_duration((*ev)++, 0, space_width);
+}
+
+/**
+ * struct ir_raw_timings_pd - pulse-distance modulation timings
+ * @header_pulse:	duration of header pulse in ns (0 for none)
+ * @header_space:	duration of header space in ns
+ * @bit_pulse:		duration of bit pulse in ns
+ * @bit_space:		duration of bit space (for logic 0 and 1) in ns
+ * @trailer_pulse:	duration of trailer pulse in ns
+ * @trailer_space:	duration of trailer space in ns
+ * @msb_first:		1 if most significant bit is sent first
+ */
+struct ir_raw_timings_pd
+{
+	unsigned int header_pulse;
+	unsigned int header_space;
+	unsigned int bit_pulse;
+	unsigned int bit_space[2];
+	unsigned int trailer_pulse;
+	unsigned int trailer_space;
+	unsigned int msb_first:1;
+};
+
+int ir_raw_gen_pd(struct ir_raw_event **ev, unsigned int max,
+		  const struct ir_raw_timings_pd *timings,
+		  unsigned int n, unsigned int data);
+
 /*
  * Routines from rc-raw.c to be used internally and by decoders
  */
-- 
1.8.3.2

[RFC 3/4] rc: ir-nec-decoder: add encode capability

[James Hogan]

Add the capability to encode NEC scancodes as raw events. The
scancode_to_raw is pretty much taken from the img-ir NEC filter()
callback, and modulation uses the pulse distance helper added in a
previous commit.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
 drivers/media/rc/ir-nec-decoder.c | 91 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index 1bab7ea..5083ed6 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -203,9 +203,100 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	return -EINVAL;
 }
 
+/**
+ * ir_nec_scancode_to_raw() - encode an NEC scancode ready for modulation.
+ * @in:		scancode filter describing a single NEC scancode.
+ * @raw:	raw data to be modulated.
+ */
+static int ir_nec_scancode_to_raw(const struct rc_scancode_filter *in,
+				  u32 *raw)
+{
+	unsigned int addr, addr_inv, data, data_inv;
+
+	data = in->data & 0xff;
+
+	if ((in->data | in->mask) & 0xff000000) {
+		/* 32-bit NEC (used by Apple and TiVo remotes) */
+		/* scan encoding: aaAAddDD */
+		if (in->mask != 0xffffffff)
+			return -EINVAL;
+		addr_inv   = (in->data >> 24) & 0xff;
+		addr       = (in->data >> 16) & 0xff;
+		data_inv   = (in->data >>  8) & 0xff;
+	} else if ((in->data | in->mask) & 0x00ff0000) {
+		/* Extended NEC */
+		/* scan encoding AAaaDD */
+		if (in->mask != 0x00ffffff)
+			return -EINVAL;
+		addr       = (in->data >> 16) & 0xff;
+		addr_inv   = (in->data >>  8) & 0xff;
+		data_inv   = data ^ 0xff;
+	} else {
+		/* Normal NEC */
+		/* scan encoding: AADD */
+		if (in->mask != 0x0000ffff)
+			return -EINVAL;
+		addr       = (in->data >>  8) & 0xff;
+		addr_inv   = addr ^ 0xff;
+		data_inv   = data ^ 0xff;
+	}
+
+	/* raw encoding: ddDDaaAA */
+	*raw = data_inv << 24 |
+	       data     << 16 |
+	       addr_inv <<  8 |
+	       addr;
+	return 0;
+}
+
+static struct ir_raw_timings_pd ir_nec_timings = {
+	.header_pulse	= NEC_HEADER_PULSE,
+	.header_space	= NEC_HEADER_SPACE,
+	.bit_pulse	= NEC_BIT_PULSE,
+	.bit_space[0]	= NEC_BIT_0_SPACE,
+	.bit_space[1]	= NEC_BIT_1_SPACE,
+	.trailer_pulse	= NEC_TRAILER_PULSE,
+	.trailer_space	= NEC_TRAILER_SPACE,
+	.msb_first = 0,
+};
+
+/**
+ * ir_nec_encode() - Encode a scancode as a stream of raw events
+ *
+ * @protocols:	allowed protocols
+ * @scancode:	scancode filter describing scancode (helps distinguish between
+ *		protocol subtypes when scancode is ambiguous)
+ * @events:	array of raw ir events to write into
+ * @max:	maximum size of @events
+ *
+ * This function returns -EINVAL if the scancode filter is invalid or matches
+ * multiple scancodes.
+ */
+static int ir_nec_encode(u64 protocols,
+			 const struct rc_scancode_filter *scancode,
+			 struct ir_raw_event *events, unsigned int max)
+{
+	struct ir_raw_event *e = events;
+	int ret;
+	u32 raw;
+
+	/* Convert a NEC scancode to raw NEC data */
+	ret = ir_nec_scancode_to_raw(scancode, &raw);
+	if (ret < 0)
+		return ret;
+
+	/* Modulate the raw data using a pulse distance modulation */
+	ret = ir_raw_gen_pd(&e, max, &ir_nec_timings, NEC_NBITS, raw);
+	if (ret < 0)
+		return ret;
+
+	return e - events;
+}
+
 static struct ir_raw_handler nec_handler = {
 	.protocols	= RC_BIT_NEC,
 	.decode		= ir_nec_decode,
+	.encode		= ir_nec_encode,
 };
 
 static int __init ir_nec_decode_init(void)
-- 
1.8.3.2

[RFC 4/4] DEBUG: rc: img-ir: raw: Add loopback on s_filter

[James Hogan]

Purely for the purposes of debugging the raw IR encode, add the s_filter
callback to the img-ir-raw driver, which instead of setting the filter
just feeds it back through the input device so that it can be verified.
---
 drivers/media/rc/img-ir/img-ir-raw.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/media/rc/img-ir/img-ir-raw.c b/drivers/media/rc/img-ir/img-ir-raw.c
index cfb01d9..272767a 100644
--- a/drivers/media/rc/img-ir/img-ir-raw.c
+++ b/drivers/media/rc/img-ir/img-ir-raw.c
@@ -7,6 +7,7 @@
  * signal edges are reported and decoded by generic software decoders.
  */
 
+#include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <media/rc-core.h>
 #include "img-ir.h"
@@ -95,6 +96,34 @@ void img_ir_setup_raw(struct img_ir_priv *priv)
 	spin_unlock_irq(&priv->lock);
 }
 
+static int img_ir_raw_set_filter(struct rc_dev *dev, enum rc_filter_type type,
+				 struct rc_scancode_filter *sc_filter)
+{
+	struct ir_raw_event *raw;
+	int ret;
+	int i;
+
+	/* fine to disable filter */
+	if (!sc_filter->mask)
+		return 0;
+	
+	raw = kmalloc(512 * sizeof(*raw), GFP_KERNEL);
+	ret = ir_raw_encode_scancode(dev->enabled_protocols, sc_filter, raw,
+				     512);
+	if (ret >= 0) {
+		/* loop back the scancode just for fun! */
+		for (i = 0; i < ret; ++i)
+			ir_raw_event_store(dev, &raw[i]);
+		ir_raw_event_handle(dev);
+
+		ret = 0;
+	}
+
+	kfree(raw);
+
+	return ret;
+}
+
 int img_ir_probe_raw(struct img_ir_priv *priv)
 {
 	struct img_ir_priv_raw *raw = &priv->raw;
@@ -114,6 +143,7 @@ int img_ir_probe_raw(struct img_ir_priv *priv)
 	rdev->map_name = RC_MAP_EMPTY;
 	rdev->input_name = "IMG Infrared Decoder Raw";
 	rdev->driver_type = RC_DRIVER_IR_RAW;
+	rdev->s_filter = img_ir_raw_set_filter;
 
 	/* Register raw decoder */
 	error = rc_register_device(rdev);
-- 
1.8.3.2

Re: [RFC 0/4] rc: ir-raw: Add encode, implement NEC encode

[Antti Seppälä]

On 6 February 2014 21:59, James Hogan <james.hogan@imgtec.com> wrote:
> A recent discussion about proposed interfaces for setting up the
> hardware wakeup filter lead to the conclusion that it could help to have
> the generic capability to encode and modulate scancodes into raw IR
> events so that drivers for hardware with a low level wake filter (on the
> level of pulse/space durations) can still easily implement the higher
> level scancode interface that is proposed.
>
> This patchset is just a quick experiment to suggest how it might work.
> I'm not familiar with the hardware that could use it so it could well be
> a bit misdesigned in places (e.g. what sort of buffer length would the
> hardware have, do we need to support any kind of partial encoding, can
> the hardware/driver easily take care of allowing for a margin of
> error?).
>

Hi James.

Thank you for your patchset. I think it is a good start towards an
interface for encoding scancodes back to ir pulses.

First some elaboration to what support is needed regarding nuvoton:

The buffer length is 67 pulses or spaces. At current sampling rate
maximum length of each individual pulse/space is 6350usec. However if
longer consecutive pulses occur they can be split to multiple smaller
values to represent the original long value. Ir-core can just leave
splitting values to the driver.

Due to the 67 pulse/space limitation I'd say that support for partial
encoding would be needed. Now it is difficult for driver to allocate
enough memory for some of the chattier protocols.

The nuvoton hardware has a special register for setting accepted
margin of error (cmp_tolerance). This is the delta within which each
pulse/space length is still considered a match. So for nuvoton the
margin of error does not need to be worried about.

>
> The first patch adds an encode callback to the existing raw ir handler
> struct and a helper function to encode a scancode for a given protocol.
>

The mechanism used here to encode works fine as long as there is only
one protocol selected. If there are several which all support encoding
then there's no easy way to tell which one will be used to do the
actual encoding.

> The third patch implements encode for NEC. The modulation is abstracted
> to use functions in patch 2 (pulse-distance is used by multiple
> protocols).
>
> Finally for debug purposes patch 4 modifies img-ir-raw to loop back the
> encoded data when a filter is altered. Should be pretty easy to apply
> similarly to any raw ir driver to try it out.
>

I believe we have rc-loopback driver for exactly this purpose. Could
you use it instead? Also adding the scancode filter to it would
demonstrate its usage.

One other thing I noticed while reviewing your patches was that
currently the dev->s_filter callback return value is ignored by
store_filter. It would be useful to return an error to userspace if
scancode storage was not possible for whatever reason.


Nevertheless I decided to use this patchset as a basis and write a
generic rc-5 encoder and use it to support streamzap protocol in
nuvoton. I'll post my patchset shortly for review / discussion.

-Antti

[RFC PATCH 0/3] rc: add RC5-SZ encoder and utilize encoders in nuvoton-cir

[Antti Seppälä]

This patchset is based and built on an earlier series posted by James Hogan
(rc: ir-raw: Add encode, implement NEC encode).

This set extends the series by adding a generic RC-5 encoder and adds support
for it to RC-5-SZ protocol.

In addition nuvoton-cir driver is modified to read wakeup filters from sysfs
and utilize encoding to convert the scancodes to format understood by the
underlying hardware.

Antti Seppälä (3):
  rc-core: Add Manchester encoder (phase encoder) support to rc-core
  ir-rc5-sz: Add ir encoding support
  nuvoton-cir: Add support for writing wakeup samples via sysfs filter
    callback

 drivers/media/rc/ir-raw.c            |  44 +++++++++++++
 drivers/media/rc/ir-rc5-sz-decoder.c |  35 ++++++++++
 drivers/media/rc/nuvoton-cir.c       | 121 +++++++++++++++++++++++++++++++++++
 drivers/media/rc/nuvoton-cir.h       |   1 +
 drivers/media/rc/rc-core-priv.h      |  14 ++++
 include/media/rc-core.h              |   1 +
 6 files changed, 216 insertions(+)

-- 
1.8.3.2

[RFC PATCH 1/3] rc-core: Add Manchester encoder (phase encoder) support to rc-core

[Antti Seppälä]

Adding a simple Manchester encoder to rc-core.
Manchester coding is used by at least RC-5 protocol and its variants.

Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
 drivers/media/rc/ir-raw.c       | 44 +++++++++++++++++++++++++++++++++++++++++
 drivers/media/rc/rc-core-priv.h | 14 +++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index 9d734dd..7fea9ac 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -240,6 +240,50 @@ ir_raw_get_allowed_protocols(void)
 	return protocols;
 }
 
+int ir_raw_gen_manchester(struct ir_raw_event **ev, unsigned int max,
+			  const struct ir_raw_timings_manchester *timings,
+			  unsigned int n, unsigned int data)
+{
+	bool need_pulse;
+	int i, count = 0;
+	i = 1 << (n - 1);
+
+	if (n > max || max < 2)
+		return -EINVAL;
+
+	if (timings->pulse_space_start) {
+		init_ir_raw_event_duration((*ev)++, 1, timings->clock);
+		init_ir_raw_event_duration((*ev), 0, timings->clock);
+		count += 2;
+	} else {
+		init_ir_raw_event_duration((*ev), 1, timings->clock);
+		count++;
+	}
+	i >>= 1;
+
+	while (i > 0) {
+		if (count > max)
+			return -EINVAL;
+
+		need_pulse = !(data & i);
+		if (need_pulse == !!(*ev)->pulse) {
+			(*ev)->duration += timings->clock;
+		} else {
+			init_ir_raw_event_duration(++(*ev), need_pulse,
+						   timings->clock);
+			count++;
+		}
+
+		init_ir_raw_event_duration(++(*ev), !need_pulse,
+					   timings->clock);
+		count++;
+		i >>= 1;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(ir_raw_gen_manchester);
+
 int ir_raw_gen_pd(struct ir_raw_event **ev, unsigned int max,
 		  const struct ir_raw_timings_pd *timings,
 		  unsigned int n, unsigned int data)
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index 3bf8c7b..4a2e2b8 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -173,6 +173,20 @@ static inline void ir_raw_gen_pulse_space(struct ir_raw_event **ev,
 }
 
 /**
+ * struct ir_raw_timings_manchester - manchester coding timings
+ * @pulse_space_start:	1 for starting with pulse (0 for starting with space)
+ * @clock:		duration of each pulse/space in ns
+ */
+struct ir_raw_timings_manchester {
+	unsigned int pulse_space_start:1;
+	unsigned int clock;
+};
+
+int ir_raw_gen_manchester(struct ir_raw_event **ev, unsigned int max,
+			  const struct ir_raw_timings_manchester *timings,
+			  unsigned int n, unsigned int data);
+
+/**
  * struct ir_raw_timings_pd - pulse-distance modulation timings
  * @header_pulse:	duration of header pulse in ns (0 for none)
  * @header_space:	duration of header space in ns
-- 
1.8.3.2

[RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support

[Antti Seppälä]

The encoding in rc5-sz first inserts a pulse and then simply utilizes the
generic Manchester encoder available in rc-core.

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

diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c
index 984e5b9..0d5e552 100644
--- a/drivers/media/rc/ir-rc5-sz-decoder.c
+++ b/drivers/media/rc/ir-rc5-sz-decoder.c
@@ -127,9 +127,44 @@ out:
 	return -EINVAL;
 }
 
+static struct ir_raw_timings_manchester ir_rc5_sz_timings = {
+	.pulse_space_start	= 0,
+	.clock			= RC5_UNIT,
+};
+
+/*
+ * ir_rc5_sz_encode() - Encode a scancode as a stream of raw events
+ *
+ * @protocols:  allowed protocols
+ * @scancode:   scancode filter describing scancode (helps distinguish between
+ *              protocol subtypes when scancode is ambiguous)
+ * @events:     array of raw ir events to write into
+ * @max:        maximum size of @events
+ *
+ * This function returns -EINVAL if the scancode filter is invalid or matches
+ * multiple scancodes. Otherwise the number of ir_raw_events generated is
+ * returned.
+ */
+static int ir_rc5_sz_encode(u64 protocols,
+			    const struct rc_scancode_filter *scancode,
+			    struct ir_raw_event *events, unsigned int max)
+{
+	int ret;
+	struct ir_raw_event *e = events;
+
+	/* RC5-SZ scancode is raw enough for manchester as it is */
+	ret = ir_raw_gen_manchester(&e, max, &ir_rc5_sz_timings, RC5_SZ_NBITS,
+				    scancode->data);
+	if (ret < 0)
+		return ret;
+
+	return e - events;
+}
+
 static struct ir_raw_handler rc5_sz_handler = {
 	.protocols	= RC_BIT_RC5_SZ,
 	.decode		= ir_rc5_sz_decode,
+	.encode		= ir_rc5_sz_encode,
 };
 
 static int __init ir_rc5_sz_decode_init(void)
-- 
1.8.3.2

[RFC PATCH 3/3] nuvoton-cir: Add support for writing wakeup samples via sysfs filter callback

[Antti Seppälä]

Nuvoton-cir utilizes the encoding capabilities of currently active protocols
to convert scancodes from user space to pulse/space format understood by the
underlying hardware.

Samples are then written to the wakeup fifo along with other necessary
configuration to enable wake up functionality.

Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
 drivers/media/rc/nuvoton-cir.c | 121 +++++++++++++++++++++++++++++++++++++++++
 drivers/media/rc/nuvoton-cir.h |   1 +
 include/media/rc-core.h        |   1 +
 3 files changed, 123 insertions(+)

diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index b41e52e..e4a4c5d 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -527,6 +527,126 @@ static int nvt_set_tx_carrier(struct rc_dev *dev, u32 carrier)
 	return 0;
 }
 
+static int nvt_write_wakeup_codes(struct rc_dev *dev,
+				  const u8 *wakeup_sample_buf, int count)
+{
+	int i = 0;
+	u8 reg, reg_learn_mode;
+	unsigned long flags;
+	struct nvt_dev *nvt = dev->priv;
+
+	nvt_dbg_wake("writing wakeup samples");
+
+	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);
+
+	if (count)
+		pr_info("Wake samples (%d) =", count);
+	else
+		pr_info("Wake sample fifo cleared");
+
+	/* Write wake samples to fifo */
+	for (i = 0; i < count; i++) {
+		pr_cont(" %02x", wakeup_sample_buf[i]);
+		nvt_cir_wake_reg_write(nvt, wakeup_sample_buf[i],
+				       CIR_WAKE_WR_FIFO_DATA);
+	}
+	pr_cont("\n");
+
+	/* 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, count ? count :
+			       CIR_WAKE_FIFO_CMP_BYTES,
+			       CIR_WAKE_FIFO_CMP_DEEP);
+
+	spin_unlock_irqrestore(&nvt->nvt_lock, flags);
+
+	return 0;
+}
+
+static int nvt_ir_raw_set_filter(struct rc_dev *dev, enum rc_filter_type type,
+				 struct rc_scancode_filter *sc_filter)
+{
+	u8 *reg_buf;
+	int i, ret, count = 0;
+	unsigned int val;
+	struct ir_raw_event *raw;
+
+	/* Other types are not valid for nuvoton */
+	if (type != RC_FILTER_WAKEUP)
+		return -EINVAL;
+
+	/* Require both mask and data to be set before actually committing */
+	if (!sc_filter->mask || !sc_filter->data)
+		return 0;
+
+	raw = kmalloc(WAKE_FIFO_LEN * sizeof(*raw), GFP_KERNEL);
+	if (!raw)
+		return -ENOMEM;
+
+	ret = ir_raw_encode_scancode(dev->enabled_protocols, sc_filter, raw,
+				     WAKE_FIFO_LEN);
+	if (ret < 0)
+		goto out_raw;
+
+	if (ret > WAKE_FIFO_LEN) {
+		pr_info("Scancode size (%d) will not fit in wake fifo (%d)",
+			ret, WAKE_FIFO_LEN);
+		ret = -EINVAL;
+		goto out_raw;
+	}
+
+	reg_buf = kmalloc(sizeof(*reg_buf) * WAKE_FIFO_LEN, GFP_KERNEL);
+	if (!reg_buf) {
+		ret = -ENOMEM;
+		goto out_raw;
+	}
+
+	/* Inspect the ir samples */
+	for (i = 0; i < ret; ++i) {
+		val = NS_TO_US((raw[i]).duration) / SAMPLE_PERIOD;
+
+		/* Split too large values into several smaller ones */
+		while (val > BUF_LEN_MASK) {
+			if (count > WAKE_FIFO_LEN) {
+				ret = -EINVAL;
+				goto out_reg;
+			}
+			reg_buf[count] = BUF_LEN_MASK;
+			val -= BUF_LEN_MASK;
+			if ((raw[i]).pulse)
+				reg_buf[count] |= BUF_PULSE_BIT;
+			count++;
+		}
+
+		if ((raw[i]).pulse)
+			val |= BUF_PULSE_BIT;
+		reg_buf[count] = val;
+		count++;
+	}
+
+	ret = nvt_write_wakeup_codes(dev, reg_buf, count);
+
+out_reg:
+	kfree(reg_buf);
+out_raw:
+	kfree(raw);
+
+	return ret;
+}
+
 /*
  * nvt_tx_ir
  *
@@ -1043,6 +1163,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_filter = nvt_ir_raw_set_filter;
 	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 e1cf23c..9d0e161 100644
--- a/drivers/media/rc/nuvoton-cir.h
+++ b/drivers/media/rc/nuvoton-cir.h
@@ -63,6 +63,7 @@ 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;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 81cddd3..6b12a1b 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -227,6 +227,7 @@ static inline void init_ir_raw_event(struct ir_raw_event *ev)
 #define US_TO_NS(usec)		((usec) * 1000)
 #define MS_TO_US(msec)		((msec) * 1000)
 #define MS_TO_NS(msec)		((msec) * 1000 * 1000)
+#define NS_TO_US(nsec)		DIV_ROUND_UP(nsec, 1000L)
 
 void ir_raw_event_handle(struct rc_dev *dev);
 int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev);
-- 
1.8.3.2

Re: [RFC 0/4] rc: ir-raw: Add encode, implement NEC encode

[James Hogan]

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

Hi Antti,

On 08/02/14 11:30, Antti Seppälä wrote:
>> The first patch adds an encode callback to the existing raw ir handler
>> struct and a helper function to encode a scancode for a given protocol.
>>
> 
> The mechanism used here to encode works fine as long as there is only
> one protocol selected. If there are several which all support encoding
> then there's no easy way to tell which one will be used to do the
> actual encoding.

True, I suppose it needs a wakeup_protocol sysfs file for that really (I
can't imagine a need or method to wake on multiple protocols, a
demodulating hardware decoder like img-ir can only have one set of
timings at a time, and a raw hardware decoder like nuvoton would seem
unlikely to have multiple wake match buffers - and if it did the sysfs
interface would probably need extending to take multiple
single-protocol/filter sets anyway).

This should probably be done prior to the new sysfs interface reaching
mainline, so that userland can always be expected to write the protocol
prior to the wakeup filter (rather than userland expecting the wake
protocol to follow the current protocol).

>> Finally for debug purposes patch 4 modifies img-ir-raw to loop back the
>> encoded data when a filter is altered. Should be pretty easy to apply
>> similarly to any raw ir driver to try it out.
>>
> 
> I believe we have rc-loopback driver for exactly this purpose. Could
> you use it instead? Also adding the scancode filter to it would
> demonstrate its usage.

True I could have done, I used img-ir simply out of convenience and
familiarity :). Would it make sense to generate an input event when
setting the filter though, or perhaps since the whole point of the
loopback driver is presumably debug it doesn't matter?

To actually add filtering support to loopback would require either:
* raw-decoder/rc-core level scancode filtering for raw ir drivers
* OR loopback driver to encode like nuvoton and fuzzy match the IR signals.

> One other thing I noticed while reviewing your patches was that
> currently the dev->s_filter callback return value is ignored by
> store_filter. It would be useful to return an error to userspace if
> scancode storage was not possible for whatever reason.

Thanks, well spotted, I'll do a fix for that soon.

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH 1/3] rc-core: Add Manchester encoder (phase encoder) support to rc-core

[James Hogan]

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

Hi Antti,

On 08/02/14 12:07, Antti Seppälä wrote:
> Adding a simple Manchester encoder to rc-core.
> Manchester coding is used by at least RC-5 protocol and its variants.
> 
> Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
> ---
>  drivers/media/rc/ir-raw.c       | 44 +++++++++++++++++++++++++++++++++++++++++
>  drivers/media/rc/rc-core-priv.h | 14 +++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
> index 9d734dd..7fea9ac 100644
> --- a/drivers/media/rc/ir-raw.c
> +++ b/drivers/media/rc/ir-raw.c
> @@ -240,6 +240,50 @@ ir_raw_get_allowed_protocols(void)
>  	return protocols;
>  }
>  
> +int ir_raw_gen_manchester(struct ir_raw_event **ev, unsigned int max,
> +			  const struct ir_raw_timings_manchester *timings,
> +			  unsigned int n, unsigned int data)
> +{
> +	bool need_pulse;
> +	int i, count = 0;
> +	i = 1 << (n - 1);
> +
> +	if (n > max || max < 2)
> +		return -EINVAL;
> +
> +	if (timings->pulse_space_start) {
> +		init_ir_raw_event_duration((*ev)++, 1, timings->clock);
> +		init_ir_raw_event_duration((*ev), 0, timings->clock);
> +		count += 2;
> +	} else {
> +		init_ir_raw_event_duration((*ev), 1, timings->clock);
> +		count++;
> +	}
> +	i >>= 1;

If you use pulse_space_start to encode the first bit, did you mean to
discard the highest bit of data?

> +
> +	while (i > 0) {
> +		if (count > max)

if count > max I think you've already overflowed the buffer (max is more
of a max count rather than max buffer index).

> +			return -EINVAL;
> +
> +		need_pulse = !(data & i);
> +		if (need_pulse == !!(*ev)->pulse) {
> +			(*ev)->duration += timings->clock;
> +		} else {
> +			init_ir_raw_event_duration(++(*ev), need_pulse,
> +						   timings->clock);
> +			count++;

I guess you need to check for buffer space here too.

> +		}
> +
> +		init_ir_raw_event_duration(++(*ev), !need_pulse,
> +					   timings->clock);
> +		count++;
> +		i >>= 1;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ir_raw_gen_manchester);

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support

[James Hogan]

Hi Antti,

On 08/02/14 12:07, Antti Seppälä wrote:
> The encoding in rc5-sz first inserts a pulse and then simply utilizes the
> generic Manchester encoder available in rc-core.
> 
> Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
> ---
>  drivers/media/rc/ir-rc5-sz-decoder.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c
> index 984e5b9..0d5e552 100644
> --- a/drivers/media/rc/ir-rc5-sz-decoder.c
> +++ b/drivers/media/rc/ir-rc5-sz-decoder.c
> @@ -127,9 +127,44 @@ out:
>  	return -EINVAL;
>  }
>  
> +static struct ir_raw_timings_manchester ir_rc5_sz_timings = {
> +	.pulse_space_start	= 0,
> +	.clock			= RC5_UNIT,
> +};
> +
> +/*
> + * ir_rc5_sz_encode() - Encode a scancode as a stream of raw events
> + *
> + * @protocols:  allowed protocols
> + * @scancode:   scancode filter describing scancode (helps distinguish between
> + *              protocol subtypes when scancode is ambiguous)
> + * @events:     array of raw ir events to write into
> + * @max:        maximum size of @events
> + *
> + * This function returns -EINVAL if the scancode filter is invalid or matches
> + * multiple scancodes. Otherwise the number of ir_raw_events generated is
> + * returned.
> + */
> +static int ir_rc5_sz_encode(u64 protocols,
> +			    const struct rc_scancode_filter *scancode,
> +			    struct ir_raw_event *events, unsigned int max)
> +{
> +	int ret;
> +	struct ir_raw_event *e = events;

Probably worth checking scancode->mask == 0xfff too?

> +
> +	/* RC5-SZ scancode is raw enough for manchester as it is */
> +	ret = ir_raw_gen_manchester(&e, max, &ir_rc5_sz_timings, RC5_SZ_NBITS,
> +				    scancode->data);
> +	if (ret < 0)
> +		return ret;

I suspect it needs some more space at the end too, to be sure that no
more bits afterwards are accepted.

> +
> +	return e - events;
> +}
> +
>  static struct ir_raw_handler rc5_sz_handler = {
>  	.protocols	= RC_BIT_RC5_SZ,
>  	.decode		= ir_rc5_sz_decode,
> +	.encode		= ir_rc5_sz_encode,
>  };
>  
>  static int __init ir_rc5_sz_decode_init(void)
> 

Cheers
James

Re: [RFC 0/4] rc: ir-raw: Add encode, implement NEC encode

[Antti Seppälä]

On 10 February 2014 11:58, James Hogan <james.hogan@imgtec.com> wrote:
> Hi Antti,
>
> On 08/02/14 11:30, Antti Seppälä wrote:
>>> The first patch adds an encode callback to the existing raw ir handler
>>> struct and a helper function to encode a scancode for a given protocol.
>>>
>>
>> The mechanism used here to encode works fine as long as there is only
>> one protocol selected. If there are several which all support encoding
>> then there's no easy way to tell which one will be used to do the
>> actual encoding.
>
> True, I suppose it needs a wakeup_protocol sysfs file for that really (I
> can't imagine a need or method to wake on multiple protocols, a
> demodulating hardware decoder like img-ir can only have one set of
> timings at a time, and a raw hardware decoder like nuvoton would seem
> unlikely to have multiple wake match buffers - and if it did the sysfs
> interface would probably need extending to take multiple
> single-protocol/filter sets anyway).
>
> This should probably be done prior to the new sysfs interface reaching
> mainline, so that userland can always be expected to write the protocol
> prior to the wakeup filter (rather than userland expecting the wake
> protocol to follow the current protocol).
>

I agree. I think the new sysfs file could pretty much use the existing
show/store_protocols() with the modification that only one protocol
can be active at a time.

>>> Finally for debug purposes patch 4 modifies img-ir-raw to loop back the
>>> encoded data when a filter is altered. Should be pretty easy to apply
>>> similarly to any raw ir driver to try it out.
>>>
>>
>> I believe we have rc-loopback driver for exactly this purpose. Could
>> you use it instead? Also adding the scancode filter to it would
>> demonstrate its usage.
>
> True I could have done, I used img-ir simply out of convenience and
> familiarity :). Would it make sense to generate an input event when
> setting the filter though, or perhaps since the whole point of the
> loopback driver is presumably debug it doesn't matter?
>

Well the purpose of rc-loopback is to provide means to write scripts
for debugging and the driver already loops tx back to rx. I just
thought that it would fit nicely to loop the encoded filter back as
well. It doesn't really matter though. Maybe some printk of the ir
samples will also suffice.

> To actually add filtering support to loopback would require either:
> * raw-decoder/rc-core level scancode filtering for raw ir drivers
> * OR loopback driver to encode like nuvoton and fuzzy match the IR signals.
>

Rc-core level scancode filtering shouldn't be too hard to do right? If
such would exist then it would provide a software fallback to other rc
devices where hardware filtering isn't available. I'd love to see the
sysfs filter and filter_mask files to have an effect on my nuvoton too
:)

But maybe we'll first need to try to get the wakeup finished.

-Antti

Re: [RFC PATCH 1/3] rc-core: Add Manchester encoder (phase encoder) support to rc-core

[Antti Seppälä]

Hi James.

On 10 February 2014 12:25, James Hogan <james.hogan@imgtec.com> wrote:
> Hi Antti,
>
> On 08/02/14 12:07, Antti Seppälä wrote:
>> Adding a simple Manchester encoder to rc-core.
>> Manchester coding is used by at least RC-5 protocol and its variants.
>>
>> Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
>> ---
>>  drivers/media/rc/ir-raw.c       | 44 +++++++++++++++++++++++++++++++++++++++++
>>  drivers/media/rc/rc-core-priv.h | 14 +++++++++++++
>>  2 files changed, 58 insertions(+)
>>
>> diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
>> index 9d734dd..7fea9ac 100644
>> --- a/drivers/media/rc/ir-raw.c
>> +++ b/drivers/media/rc/ir-raw.c
>> @@ -240,6 +240,50 @@ ir_raw_get_allowed_protocols(void)
>>       return protocols;
>>  }
>>
>> +int ir_raw_gen_manchester(struct ir_raw_event **ev, unsigned int max,
>> +                       const struct ir_raw_timings_manchester *timings,
>> +                       unsigned int n, unsigned int data)
>> +{
>> +     bool need_pulse;
>> +     int i, count = 0;
>> +     i = 1 << (n - 1);
>> +
>> +     if (n > max || max < 2)
>> +             return -EINVAL;
>> +
>> +     if (timings->pulse_space_start) {
>> +             init_ir_raw_event_duration((*ev)++, 1, timings->clock);
>> +             init_ir_raw_event_duration((*ev), 0, timings->clock);
>> +             count += 2;
>> +     } else {
>> +             init_ir_raw_event_duration((*ev), 1, timings->clock);
>> +             count++;
>> +     }
>> +     i >>= 1;
>
> If you use pulse_space_start to encode the first bit, did you mean to
> discard the highest bit of data?
>

I did not mean to discard data but to just start the ir pulse with
logic 1 as that first bit is discarded when decoding.

>> +
>> +     while (i > 0) {
>> +             if (count > max)
>
> if count > max I think you've already overflowed the buffer (max is more
> of a max count rather than max buffer index).
>

I guess you're right. I'll address this in next version of my patch.

>> +                     return -EINVAL;
>> +
>> +             need_pulse = !(data & i);
>> +             if (need_pulse == !!(*ev)->pulse) {
>> +                     (*ev)->duration += timings->clock;
>> +             } else {
>> +                     init_ir_raw_event_duration(++(*ev), need_pulse,
>> +                                                timings->clock);
>> +                     count++;
>
> I guess you need to check for buffer space here too.
>

This comment seems also correct. Thank you for reviewing.

>> +             }
>> +
>> +             init_ir_raw_event_duration(++(*ev), !need_pulse,
>> +                                        timings->clock);
>> +             count++;
>> +             i >>= 1;
>> +     }
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL(ir_raw_gen_manchester);
>
> Cheers
> James
>

Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support

[Antti Seppälä]

Hi James.

On 10 February 2014 12:30, James Hogan <james.hogan@imgtec.com> wrote:
> Hi Antti,
>
> On 08/02/14 12:07, Antti Seppälä wrote:
>> The encoding in rc5-sz first inserts a pulse and then simply utilizes the
>> generic Manchester encoder available in rc-core.
>>
>> Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
>> ---
>>  drivers/media/rc/ir-rc5-sz-decoder.c | 35 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c
>> index 984e5b9..0d5e552 100644
>> --- a/drivers/media/rc/ir-rc5-sz-decoder.c
>> +++ b/drivers/media/rc/ir-rc5-sz-decoder.c
>> @@ -127,9 +127,44 @@ out:
>>       return -EINVAL;
>>  }
>>
>> +static struct ir_raw_timings_manchester ir_rc5_sz_timings = {
>> +     .pulse_space_start      = 0,
>> +     .clock                  = RC5_UNIT,
>> +};
>> +
>> +/*
>> + * ir_rc5_sz_encode() - Encode a scancode as a stream of raw events
>> + *
>> + * @protocols:  allowed protocols
>> + * @scancode:   scancode filter describing scancode (helps distinguish between
>> + *              protocol subtypes when scancode is ambiguous)
>> + * @events:     array of raw ir events to write into
>> + * @max:        maximum size of @events
>> + *
>> + * This function returns -EINVAL if the scancode filter is invalid or matches
>> + * multiple scancodes. Otherwise the number of ir_raw_events generated is
>> + * returned.
>> + */
>> +static int ir_rc5_sz_encode(u64 protocols,
>> +                         const struct rc_scancode_filter *scancode,
>> +                         struct ir_raw_event *events, unsigned int max)
>> +{
>> +     int ret;
>> +     struct ir_raw_event *e = events;
>
> Probably worth checking scancode->mask == 0xfff too?
>

I guess so. However if I'm not mistaken this makes all wakeup_filter
writes fail in user space if wakeup_filter_mask is not set. Is that
intended?

>> +
>> +     /* RC5-SZ scancode is raw enough for manchester as it is */
>> +     ret = ir_raw_gen_manchester(&e, max, &ir_rc5_sz_timings, RC5_SZ_NBITS,
>> +                                 scancode->data);
>> +     if (ret < 0)
>> +             return ret;
>
> I suspect it needs some more space at the end too, to be sure that no
> more bits afterwards are accepted.
>

I'm sorry but I'm not sure I completely understood what you meant
here. For RC-5-SZ the entire scancode gets encoded and nothing more.
Do you mean that the encoder should append some ir silence to the end
result to make sure the ir sample has ended?

-Antti

Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support

[James Hogan]

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

Hi Antti,

On Monday 10 February 2014 22:09:33 Antti Seppälä wrote:
> >> +static int ir_rc5_sz_encode(u64 protocols,
> >> +                         const struct rc_scancode_filter *scancode,
> >> +                         struct ir_raw_event *events, unsigned int max)
> >> +{
> >> +     int ret;
> >> +     struct ir_raw_event *e = events;
> > 
> > Probably worth checking scancode->mask == 0xfff too?
> 
> I guess so. However if I'm not mistaken this makes all wakeup_filter
> writes fail in user space if wakeup_filter_mask is not set. Is that
> intended?

Good point, although looking at your patch 3, mask==0 is already permitted 
silently by the driver, which I think would make it okay.

I guess to be safe userland would have to do:
wakeup_filter_mask = 0
wakeup_filter = $value
wakeup_filter_mask = 0xfff

which doesn't sound unreasonable in the absence of a way to update them 
atomically (sysfs files doing more than one thing is frowned upon I believe).

> >> +     /* RC5-SZ scancode is raw enough for manchester as it is */
> >> +     ret = ir_raw_gen_manchester(&e, max, &ir_rc5_sz_timings,
> >> RC5_SZ_NBITS, +                                 scancode->data);
> >> +     if (ret < 0)
> >> +             return ret;
> > 
> > I suspect it needs some more space at the end too, to be sure that no
> > more bits afterwards are accepted.
> 
> I'm sorry but I'm not sure I completely understood what you meant
> here. For RC-5-SZ the entire scancode gets encoded and nothing more.
> Do you mean that the encoder should append some ir silence to the end
> result to make sure the ir sample has ended?

Yeh something like that. Certainly the raw decoders I've looked at expect a 
certain amount of space at the end to avoid decoding part of a longer protocol 
(it's in the pulse distance helper as the trailer space timing). Similarly the 
IMG hardware decoder has register fields for the free-time to require at the 
end of the message.

In fact it becomes a bit awkward for the raw IR driver for the IMG hardware 
which uses edge interrupts, as it has to have a timeout to emit a final repeat 
event after 150ms of inactivity, in order for the raw decoders to accept it 
(unless you hold the button down in which case the repeat code edges result in 
the long space).

Cheers
James

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support

[Antti Seppälä]

On 10 February 2014 22:50, James Hogan <james.hogan@imgtec.com> wrote:
>> > I suspect it needs some more space at the end too, to be sure that no
>> > more bits afterwards are accepted.
>>
>> I'm sorry but I'm not sure I completely understood what you meant
>> here. For RC-5-SZ the entire scancode gets encoded and nothing more.
>> Do you mean that the encoder should append some ir silence to the end
>> result to make sure the ir sample has ended?
>
> Yeh something like that. Certainly the raw decoders I've looked at expect a
> certain amount of space at the end to avoid decoding part of a longer protocol
> (it's in the pulse distance helper as the trailer space timing). Similarly the
> IMG hardware decoder has register fields for the free-time to require at the
> end of the message.
>
> In fact it becomes a bit awkward for the raw IR driver for the IMG hardware
> which uses edge interrupts, as it has to have a timeout to emit a final repeat
> event after 150ms of inactivity, in order for the raw decoders to accept it
> (unless you hold the button down in which case the repeat code edges result in
> the long space).
>

Ok, I understand now.

I suppose I can append some IR silence to the encoded result. The
trailer space timing seems like a good way to do it. I'll create new
version of my patches sometime later.

Are you working on the wakeup protocol selector sysfs interface?

-Antti

Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support

[James Hogan]

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

On Tuesday 11 February 2014 20:14:19 Antti Seppälä wrote:
> On 10 February 2014 22:50, James Hogan <james.hogan@imgtec.com> wrote:
> >> > I suspect it needs some more space at the end too, to be sure that no
> >> > more bits afterwards are accepted.
> >> 
> >> I'm sorry but I'm not sure I completely understood what you meant
> >> here. For RC-5-SZ the entire scancode gets encoded and nothing more.
> >> Do you mean that the encoder should append some ir silence to the end
> >> result to make sure the ir sample has ended?
> > 
> > Yeh something like that. Certainly the raw decoders I've looked at expect
> > a
> > certain amount of space at the end to avoid decoding part of a longer
> > protocol (it's in the pulse distance helper as the trailer space timing).
> > Similarly the IMG hardware decoder has register fields for the free-time
> > to require at the end of the message.
> > 
> > In fact it becomes a bit awkward for the raw IR driver for the IMG
> > hardware
> > which uses edge interrupts, as it has to have a timeout to emit a final
> > repeat event after 150ms of inactivity, in order for the raw decoders to
> > accept it (unless you hold the button down in which case the repeat code
> > edges result in the long space).
> 
> Ok, I understand now.
> 
> I suppose I can append some IR silence to the encoded result. The
> trailer space timing seems like a good way to do it. I'll create new
> version of my patches sometime later.
> 
> Are you working on the wakeup protocol selector sysfs interface?

I gave it a try yesterday, but it's a bit of a work in progress at the moment.
It's also a bit more effort for img-ir to work properly with it, so I'd
probably just limit the allowed wakeup protocols to the enabled normal
protocol at first in img-ir.

Here's what I have (hopefully kmail won't corrupt it), feel free to take and
improve/fix it. I'm not keen on the invasiveness of the
allowed_protos/enabled_protocols change (which isn't complete), but it
should probably be abstracted at some point anyway.

Cheers
James

diff --git a/Documentation/ABI/testing/sysfs-class-rc b/Documentation/ABI/testing/sysfs-class-rc
index c0e1d14..1e4ecc8 100644
--- a/Documentation/ABI/testing/sysfs-class-rc
+++ b/Documentation/ABI/testing/sysfs-class-rc
@@ -61,6 +61,25 @@ Description:
 		an error.
 		This value may be reset to 0 if the current protocol is altered.
 
+What:		/sys/class/rc/rcN/wakeup_protocol
+Date:		Feb 2014
+KernelVersion:	3.15
+Contact:	Mauro Carvalho Chehab <m.chehab@samsung.com>
+Description:
+		Reading this file returns a list of available protocols to use
+		for the wakeup filter, something like:
+		    "rc5 rc6 nec jvc [sony]"
+		The enabled wakeup protocol is shown in [] brackets.
+		Writing "+proto" will add a protocol to the list of enabled
+		wakeup protocols.
+		Writing "-proto" will remove a protocol from the list of enabled
+		wakeup protocols.
+		Writing "proto" will use "proto" for wakeup events.
+		Writing "none" will disable wakeup.
+		Write fails with EINVAL if more than one protocol would be
+		enabled, an unknown protocol name is used, or if wakeup is not
+		supported by the hardware.
+
 What:		/sys/class/rc/rcN/wakeup_filter
 Date:		Jan 2014
 KernelVersion:	3.15
@@ -74,7 +93,7 @@ Description:
 		scancodes which match the filter will wake the system from e.g.
 		suspend to RAM or power off.
 		Otherwise the write will fail with an error.
-		This value may be reset to 0 if the current protocol is altered.
+		This value may be reset to 0 if the wakeup protocol is altered.
 
 What:		/sys/class/rc/rcN/wakeup_filter_mask
 Date:		Jan 2014
@@ -89,4 +108,4 @@ Description:
 		scancodes which match the filter will wake the system from e.g.
 		suspend to RAM or power off.
 		Otherwise the write will fail with an error.
-		This value may be reset to 0 if the current protocol is altered.
+		This value may be reset to 0 if the wakeup protocol is altered.
diff --git a/drivers/media/rc/ir-jvc-decoder.c b/drivers/media/rc/ir-jvc-decoder.c
index 3948138..7fb9467 100644
--- a/drivers/media/rc/ir-jvc-decoder.c
+++ b/drivers/media/rc/ir-jvc-decoder.c
@@ -47,7 +47,7 @@ static int ir_jvc_decode(struct rc_dev *dev, struct ir_raw_event ev)
 {
 	struct jvc_dec *data = &dev->raw->jvc;
 
-	if (!(dev->enabled_protocols & RC_BIT_JVC))
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_JVC))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-mce_kbd-decoder.c b/drivers/media/rc/ir-mce_kbd-decoder.c
index 9f3c9b5..bc93e11 100644
--- a/drivers/media/rc/ir-mce_kbd-decoder.c
+++ b/drivers/media/rc/ir-mce_kbd-decoder.c
@@ -216,7 +216,7 @@ static int ir_mce_kbd_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	unsigned long delay;
 
-	if (!(dev->enabled_protocols & RC_BIT_MCE_KBD))
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_MCE_KBD))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index 5083ed6..f08010d 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -52,7 +52,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u8 address, not_address, command, not_command;
 	bool send_32bits = false;
 
-	if (!(dev->enabled_protocols & RC_BIT_NEC))
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_NEC))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index ae7b445..553937e 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -326,7 +326,7 @@ int ir_raw_event_register(struct rc_dev *dev)
 		return -ENOMEM;
 
 	dev->raw->dev = dev;
-	dev->enabled_protocols = ~0;
+	dev->enabled_protocols[RC_FILTER_NORMAL] = ~0;
 	rc = kfifo_alloc(&dev->raw->kfifo,
 			 sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
 			 GFP_KERNEL);
diff --git a/drivers/media/rc/ir-rc5-decoder.c b/drivers/media/rc/ir-rc5-decoder.c
index 4e53a31..e0e5118 100644
--- a/drivers/media/rc/ir-rc5-decoder.c
+++ b/drivers/media/rc/ir-rc5-decoder.c
@@ -52,7 +52,7 @@ static int ir_rc5_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u8 toggle;
 	u32 scancode;
 
-	if (!(dev->enabled_protocols & (RC_BIT_RC5 | RC_BIT_RC5X)))
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & (RC_BIT_RC5 | RC_BIT_RC5X)))
 		return 0;
 
 	if (!is_timing_event(ev)) {
@@ -128,7 +128,7 @@ again:
 		if (data->wanted_bits == RC5X_NBITS) {
 			/* RC5X */
 			u8 xdata, command, system;
-			if (!(dev->enabled_protocols & RC_BIT_RC5X)) {
+			if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_RC5X)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
@@ -145,7 +145,7 @@ again:
 		} else {
 			/* RC5 */
 			u8 command, system;
-			if (!(dev->enabled_protocols & RC_BIT_RC5)) {
+			if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_RC5)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c
index 865fe84..47de4b3 100644
--- a/drivers/media/rc/ir-rc5-sz-decoder.c
+++ b/drivers/media/rc/ir-rc5-sz-decoder.c
@@ -48,7 +48,7 @@ static int ir_rc5_sz_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u8 toggle, command, system;
 	u32 scancode;
 
-	if (!(dev->enabled_protocols & RC_BIT_RC5_SZ))
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_RC5_SZ))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-rc6-decoder.c b/drivers/media/rc/ir-rc6-decoder.c
index 7cba7d3..e2cee1d 100644
--- a/drivers/media/rc/ir-rc6-decoder.c
+++ b/drivers/media/rc/ir-rc6-decoder.c
@@ -89,7 +89,7 @@ static int ir_rc6_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	u8 toggle;
 
-	if (!(dev->enabled_protocols &
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
 	      (RC_BIT_RC6_0 | RC_BIT_RC6_6A_20 | RC_BIT_RC6_6A_24 |
 	       RC_BIT_RC6_6A_32 | RC_BIT_RC6_MCE)))
 		return 0;
diff --git a/drivers/media/rc/ir-sanyo-decoder.c b/drivers/media/rc/ir-sanyo-decoder.c
index 0a06205..352bd37 100644
--- a/drivers/media/rc/ir-sanyo-decoder.c
+++ b/drivers/media/rc/ir-sanyo-decoder.c
@@ -58,7 +58,7 @@ static int ir_sanyo_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	u8 address, command, not_command;
 
-	if (!(dev->enabled_protocols & RC_BIT_SANYO))
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_SANYO))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-sharp-decoder.c b/drivers/media/rc/ir-sharp-decoder.c
index 4c17be5..e4e66f6 100644
--- a/drivers/media/rc/ir-sharp-decoder.c
+++ b/drivers/media/rc/ir-sharp-decoder.c
@@ -48,7 +48,7 @@ static int ir_sharp_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	struct sharp_dec *data = &dev->raw->sharp;
 	u32 msg, echo, address, command, scancode;
 
-	if (!(dev->enabled_protocols & RC_BIT_SHARP))
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_SHARP))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-sony-decoder.c b/drivers/media/rc/ir-sony-decoder.c
index 29ab9c2..1c586603 100644
--- a/drivers/media/rc/ir-sony-decoder.c
+++ b/drivers/media/rc/ir-sony-decoder.c
@@ -45,7 +45,7 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	u8 device, subdevice, function;
 
-	if (!(dev->enabled_protocols &
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
 	      (RC_BIT_SONY12 | RC_BIT_SONY15 | RC_BIT_SONY20)))
 		return 0;
 
@@ -124,7 +124,8 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
 
 		switch (data->count) {
 		case 12:
-			if (!(dev->enabled_protocols & RC_BIT_SONY12)) {
+			if (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
+							RC_BIT_SONY12)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
@@ -133,7 +134,8 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
 			function  = bitrev8((data->bits >>  4) & 0xFE);
 			break;
 		case 15:
-			if (!(dev->enabled_protocols & RC_BIT_SONY15)) {
+			if (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
+							RC_BIT_SONY15)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
@@ -142,7 +144,8 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
 			function  = bitrev8((data->bits >>  7) & 0xFE);
 			break;
 		case 20:
-			if (!(dev->enabled_protocols & RC_BIT_SONY20)) {
+			if (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
+							RC_BIT_SONY20)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
index 53d0282..b447045 100644
--- a/drivers/media/rc/rc-loopback.c
+++ b/drivers/media/rc/rc-loopback.c
@@ -195,7 +195,7 @@ static int __init loop_init(void)
 	rc->map_name		= RC_MAP_EMPTY;
 	rc->priv		= &loopdev;
 	rc->driver_type		= RC_DRIVER_IR_RAW;
-	rc->allowed_protos	= RC_BIT_ALL;
+	rc->allowed_protos[RC_FILTER_NORMAL]	= RC_BIT_ALL;
 	rc->timeout		= 100 * 1000 * 1000; /* 100 ms */
 	rc->min_timeout		= 1;
 	rc->max_timeout		= UINT_MAX;
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 25b3f8f..4594b61 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -793,13 +793,38 @@ static struct {
 };
 
 /**
- * show_protocols() - shows the current IR protocol(s)
+ * struct rc_filter_attribute - Device attribute relating to a filter type.
+ * @attr:	Device attribute.
+ * @type:	Filter type.
+ * @mask:	false for filter value, true for filter mask.
+ */
+struct rc_filter_attribute {
+	struct device_attribute		attr;
+	enum rc_filter_type		type;
+	bool				mask;
+};
+#define to_rc_filter_attr(a) container_of(a, struct rc_filter_attribute, attr)
+
+#define RC_PROTO_ATTR(_name, _mode, _show, _store, _type)		\
+	struct rc_filter_attribute dev_attr_##_name = {			\
+		.attr = __ATTR(_name, _mode, _show, _store),		\
+		.type = (_type),					\
+	}
+#define RC_FILTER_ATTR(_name, _mode, _show, _store, _type, _mask)	\
+	struct rc_filter_attribute dev_attr_##_name = {			\
+		.attr = __ATTR(_name, _mode, _show, _store),		\
+		.type = (_type),					\
+		.mask = (_mask),					\
+	}
+
+/**
+ * show_protocols() - shows the current/wakeup IR protocol(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 protocol type(s).
- * it is trigged by reading /sys/class/rc/rc?/protocols.
+ * it is trigged by reading /sys/class/rc/rc?/[wakeup_]protocols.
  * It returns the protocol names of supported protocols.
  * Enabled protocols are printed in brackets.
  *
@@ -810,6 +835,7 @@ static ssize_t show_protocols(struct device *device,
 			      struct device_attribute *mattr, char *buf)
 {
 	struct rc_dev *dev = to_rc_dev(device);
+	struct rc_filter_attribute *fattr = to_rc_filter_attr(mattr);
 	u64 allowed, enabled;
 	char *tmp = buf;
 	int i;
@@ -820,11 +846,13 @@ static ssize_t show_protocols(struct device *device,
 
 	mutex_lock(&dev->lock);
 
-	enabled = dev->enabled_protocols;
+	enabled = dev->enabled_protocols[fattr->type];
 	if (dev->driver_type == RC_DRIVER_SCANCODE)
-		allowed = dev->allowed_protos;
+		allowed = dev->allowed_protos[fattr->type];
 	else if (dev->raw)
-		allowed = ir_raw_get_allowed_protocols();
+		allowed = (fattr->type == RC_FILTER_NORMAL)
+			? ir_raw_get_allowed_protocols()
+			: 0;
 	else {
 		mutex_unlock(&dev->lock);
 		return -ENODEV;
@@ -854,14 +882,14 @@ static ssize_t show_protocols(struct device *device,
 }
 
 /**
- * store_protocols() - changes the current IR protocol(s)
+ * store_protocols() - changes the current/wakeup IR protocol(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 protocol type.
- * It is trigged by writing to /sys/class/rc/rc?/protocols.
+ * It is trigged by writing to /sys/class/rc/rc?/[wakeup_]protocols.
  * Writing "+proto" will add a protocol to the list of enabled protocols.
  * Writing "-proto" will remove a protocol from the list of enabled protocols.
  * Writing "proto" will enable only "proto".
@@ -878,6 +906,7 @@ static ssize_t store_protocols(struct device *device,
 			       size_t len)
 {
 	struct rc_dev *dev = to_rc_dev(device);
+	struct rc_filter_attribute *fattr = to_rc_filter_attr(mattr);
 	bool enable, disable;
 	const char *tmp;
 	u64 type;
@@ -896,7 +925,7 @@ static ssize_t store_protocols(struct device *device,
 		ret = -EINVAL;
 		goto out;
 	}
-	type = dev->enabled_protocols;
+	type = dev->enabled_protocols[fattr->type];
 
 	while ((tmp = strsep((char **) &data, " \n")) != NULL) {
 		if (!*tmp)
@@ -945,7 +974,7 @@ static ssize_t store_protocols(struct device *device,
 	}
 
 	if (dev->change_protocol) {
-		rc = dev->change_protocol(dev, &type);
+		rc = dev->change_protocol(dev, fattr->type, &type);
 		if (rc < 0) {
 			IR_dprintk(1, "Error setting protocols to 0x%llx\n",
 				   (long long)type);
@@ -954,7 +983,7 @@ static ssize_t store_protocols(struct device *device,
 		}
 	}
 
-	dev->enabled_protocols = type;
+	dev->enabled_protocols[fattr->type] = type;
 	IR_dprintk(1, "Current protocol(s): 0x%llx\n",
 		   (long long)type);
 
@@ -966,26 +995,6 @@ out:
 }
 
 /**
- * struct rc_filter_attribute - Device attribute relating to a filter type.
- * @attr:	Device attribute.
- * @type:	Filter type.
- * @mask:	false for filter value, true for filter mask.
- */
-struct rc_filter_attribute {
-	struct device_attribute		attr;
-	enum rc_filter_type		type;
-	bool				mask;
-};
-#define to_rc_filter_attr(a) container_of(a, struct rc_filter_attribute, attr)
-
-#define RC_FILTER_ATTR(_name, _mode, _show, _store, _type, _mask)	\
-	struct rc_filter_attribute dev_attr_##_name = {			\
-		.attr = __ATTR(_name, _mode, _show, _store),		\
-		.type = (_type),					\
-		.mask = (_mask),					\
-	}
-
-/**
  * show_filter() - shows the current scancode filter value or mask
  * @device:	the device descriptor
  * @attr:	the device attribute struct
@@ -1118,8 +1127,10 @@ static int rc_dev_uevent(struct device *device, struct kobj_uevent_env *env)
 /*
  * Static device attribute struct with the sysfs attributes for IR's
  */
-static DEVICE_ATTR(protocols, S_IRUGO | S_IWUSR,
-		   show_protocols, store_protocols);
+static RC_PROTO_ATTR(protocols, S_IRUGO | S_IWUSR,
+		     show_protocols, store_protocols, RC_FILTER_NORMAL);
+static RC_PROTO_ATTR(wakeup_protocol, S_IRUGO | S_IWUSR,
+		     show_protocols, store_protocols, RC_FILTER_WAKEUP);
 static RC_FILTER_ATTR(filter, S_IRUGO|S_IWUSR,
 		      show_filter, store_filter, RC_FILTER_NORMAL, false);
 static RC_FILTER_ATTR(filter_mask, S_IRUGO|S_IWUSR,
@@ -1130,7 +1141,8 @@ static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR,
 		      show_filter, store_filter, RC_FILTER_WAKEUP, true);
 
 static struct attribute *rc_dev_attrs[] = {
-	&dev_attr_protocols.attr,
+	&dev_attr_protocols.attr.attr,
+	&dev_attr_wakeup_protocol.attr.attr,
 	&dev_attr_filter.attr.attr,
 	&dev_attr_filter_mask.attr.attr,
 	&dev_attr_wakeup_filter.attr.attr,
@@ -1296,10 +1308,10 @@ int rc_register_device(struct rc_dev *dev)
 
 	if (dev->change_protocol) {
 		u64 rc_type = (1 << rc_map->rc_type);
-		rc = dev->change_protocol(dev, &rc_type);
+		rc = dev->change_protocol(dev, RC_FILTER_NORMAL, &rc_type);
 		if (rc < 0)
 			goto out_raw;
-		dev->enabled_protocols = rc_type;
+		dev->enabled_protocols[RC_FILTER_NORMAL] = rc_type;
 	}
 
 	mutex_unlock(&dev->lock);
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 7bd66be..eb47584 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -124,8 +124,8 @@ struct rc_dev {
 	struct input_dev		*input_dev;
 	enum rc_driver_type		driver_type;
 	bool				idle;
-	u64				allowed_protos;
-	u64				enabled_protocols;
+	u64				allowed_protos[RC_FILTER_MAX];
+	u64				enabled_protocols[RC_FILTER_MAX];
 	u32				users;
 	u32				scanmask;
 	void				*priv;
@@ -142,7 +142,9 @@ struct rc_dev {
 	u32				rx_resolution;
 	u32				tx_resolution;
 	struct rc_scancode_filter	scancode_filters[RC_FILTER_MAX];
-	int				(*change_protocol)(struct rc_dev *dev, u64 *rc_type);
+	int				(*change_protocol)(struct rc_dev *dev,
+							   enum rc_filter_type type,
+							   u64 *rc_type);
 	int				(*open)(struct rc_dev *dev);
 	void				(*close)(struct rc_dev *dev);
 	int				(*s_tx_mask)(struct rc_dev *dev, u32 mask);

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[RFCv2 PATCH 0/3] rc: add RC5-SZ encoder and utilize encoders in nuvoton-cir

[Antti Seppälä]

This patchset is an improved version of an earlier attempt to add RC5-SZ 
encoder to ir-core. The set is based on a series posted by James Hogan 
(rc: ir-raw: Add encode, implement NEC encode).

This set extends the series by adding a generic RC-5 encoder and adds 
support for it to RC-5-SZ protocol.

In addition nuvoton-cir driver is modified to read wakeup filters from 
sysfs and utilize encoding to convert the scancodes to format understood 
by the underlying hardware.

Changes in v2:
 - (Hopefully) fixed all buffer indexing issues
 - Write samples to fifo even if if encoded result won't completely fit
 - Check filter mask in encoder
 - Append some trailing ir silence to the encoded result
 - Some cleanups and readability improvements

Antti Seppälä (3):
  rc-core: Add Manchester encoder (phase encoder) support to rc-core
  ir-rc5-sz: Add ir encoding support
  nuvoton-cir: Add support for writing wakeup samples via sysfs filter callback

 drivers/media/rc/ir-raw.c            |  56 +++++++++++++++++
 drivers/media/rc/ir-rc5-sz-decoder.c |  39 ++++++++++++
 drivers/media/rc/nuvoton-cir.c       | 119 +++++++++++++++++++++++++++++++++++
 drivers/media/rc/nuvoton-cir.h       |   1 +
 drivers/media/rc/rc-core-priv.h      |  16 +++++
 include/media/rc-core.h              |   1 +
 6 files changed, 232 insertions(+)

-- 
1.8.3.2

[RFCv2 PATCH 1/3] rc-core: Add Manchester encoder (phase encoder) support to rc-core

[Antti Seppälä]

Adding a simple Manchester encoder to rc-core.
Manchester coding is used by at least RC-5 protocol and its variants.

Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
 drivers/media/rc/ir-raw.c       | 56 +++++++++++++++++++++++++++++++++++++++++
 drivers/media/rc/rc-core-priv.h | 16 ++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index 9d734dd..b9cc45d 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -240,6 +240,62 @@ ir_raw_get_allowed_protocols(void)
 	return protocols;
 }
 
+int ir_raw_gen_manchester(struct ir_raw_event **ev, unsigned int max,
+			  const struct ir_raw_timings_manchester *timings,
+			  unsigned int n, unsigned int data)
+{
+	bool need_pulse;
+	int i, count = 0;
+	i = 1 << (n - 1);
+
+	if (n > max || max < 3)
+		return -EINVAL;
+
+	if (timings->pulse_space_start) {
+		init_ir_raw_event_duration((*ev)++, 1, timings->clock);
+		init_ir_raw_event_duration((*ev), 0, timings->clock);
+		count += 2;
+	} else {
+		init_ir_raw_event_duration((*ev), 1, timings->clock);
+		count++;
+	}
+	i >>= 1;
+
+	while (i > 0) {
+		need_pulse = !(data & i);
+		if (need_pulse == !!(*ev)->pulse) {
+			(*ev)->duration += timings->clock;
+		} else {
+			init_ir_raw_event_duration(++(*ev), need_pulse,
+						   timings->clock);
+			count++;
+		}
+
+		if (count >= max)
+			return -EINVAL;
+
+		init_ir_raw_event_duration(++(*ev), !need_pulse,
+					   timings->clock);
+		count++;
+		i >>= 1;
+	}
+
+	if (timings->trailer_space) {
+		if (!(*ev)->pulse) {
+			(*ev)->duration += timings->trailer_space;
+		} else if (count < max) {
+			init_ir_raw_event_duration(++(*ev), 0,
+						   timings->trailer_space);
+			count++;
+		} else {
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(ir_raw_gen_manchester);
+
 int ir_raw_gen_pd(struct ir_raw_event **ev, unsigned int max,
 		  const struct ir_raw_timings_pd *timings,
 		  unsigned int n, unsigned int data)
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index 3bf8c7b..df3ab46 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -173,6 +173,22 @@ static inline void ir_raw_gen_pulse_space(struct ir_raw_event **ev,
 }
 
 /**
+ * struct ir_raw_timings_manchester - manchester coding timings
+ * @pulse_space_start:	1 for starting with pulse (0 for starting with space)
+ * @clock:		duration of each pulse/space in ns
+ * @trailer_space:	duration of trailer space in ns
+ */
+struct ir_raw_timings_manchester {
+	unsigned int pulse_space_start:1;
+	unsigned int clock;
+	unsigned int trailer_space;
+};
+
+int ir_raw_gen_manchester(struct ir_raw_event **ev, unsigned int max,
+			  const struct ir_raw_timings_manchester *timings,
+			  unsigned int n, unsigned int data);
+
+/**
  * struct ir_raw_timings_pd - pulse-distance modulation timings
  * @header_pulse:	duration of header pulse in ns (0 for none)
  * @header_space:	duration of header space in ns
-- 
1.8.3.2

[RFCv2 PATCH 2/3] ir-rc5-sz: Add ir encoding support

[Antti Seppälä]

The encoding in rc5-sz first inserts a pulse and then simply utilizes the
generic Manchester encoder available in rc-core.

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

diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c
index 984e5b9..dff609e 100644
--- a/drivers/media/rc/ir-rc5-sz-decoder.c
+++ b/drivers/media/rc/ir-rc5-sz-decoder.c
@@ -127,9 +127,48 @@ out:
 	return -EINVAL;
 }
 
+static struct ir_raw_timings_manchester ir_rc5_sz_timings = {
+	.pulse_space_start	= 0,
+	.clock			= RC5_UNIT,
+	.trailer_space		= RC5_UNIT * 10,
+};
+
+/*
+ * ir_rc5_sz_encode() - Encode a scancode as a stream of raw events
+ *
+ * @protocols:  allowed protocols
+ * @scancode:   scancode filter describing scancode (helps distinguish between
+ *              protocol subtypes when scancode is ambiguous)
+ * @events:     array of raw ir events to write into
+ * @max:        maximum size of @events
+ *
+ * This function returns -EINVAL if the scancode filter is invalid or matches
+ * multiple scancodes. Otherwise the number of ir_raw_events generated is
+ * returned.
+ */
+static int ir_rc5_sz_encode(u64 protocols,
+			    const struct rc_scancode_filter *scancode,
+			    struct ir_raw_event *events, unsigned int max)
+{
+	int ret;
+	struct ir_raw_event *e = events;
+
+	if (scancode->mask != 0xffff)
+		return -EINVAL;
+
+	/* RC5-SZ scancode is raw enough for manchester as it is */
+	ret = ir_raw_gen_manchester(&e, max, &ir_rc5_sz_timings, RC5_SZ_NBITS,
+				    scancode->data);
+	if (ret < 0)
+		return ret;
+
+	return e - events;
+}
+
 static struct ir_raw_handler rc5_sz_handler = {
 	.protocols	= RC_BIT_RC5_SZ,
 	.decode		= ir_rc5_sz_decode,
+	.encode		= ir_rc5_sz_encode,
 };
 
 static int __init ir_rc5_sz_decode_init(void)
-- 
1.8.3.2

[RFCv2 PATCH 3/3] nuvoton-cir: Add support for writing wakeup samples via sysfs filter callback

[Antti Seppälä]

Nuvoton-cir utilizes the encoding capabilities of rc-core to convert
scancodes from user space to pulse/space format understood by the
underlying hardware.

Converted samples are then written to the wakeup fifo along with other
necessary configuration to enable wake up functionality.

Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
 drivers/media/rc/nuvoton-cir.c | 119 +++++++++++++++++++++++++++++++++++++++++
 drivers/media/rc/nuvoton-cir.h |   1 +
 include/media/rc-core.h        |   1 +
 3 files changed, 121 insertions(+)

diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index b41e52e..8be81b5 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -527,6 +527,124 @@ static int nvt_set_tx_carrier(struct rc_dev *dev, u32 carrier)
 	return 0;
 }
 
+static int nvt_write_wakeup_codes(struct rc_dev *dev,
+				  const u8 *wakeup_sample_buf, int count)
+{
+	int i = 0;
+	u8 reg, reg_learn_mode;
+	unsigned long flags;
+	struct nvt_dev *nvt = dev->priv;
+
+	nvt_dbg_wake("writing wakeup samples");
+
+	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);
+
+	if (count > WAKE_FIFO_LEN) {
+		nvt_dbg_wake("HW FIFO too small for all wake samples");
+		count = WAKE_FIFO_LEN;
+	}
+
+	if (count)
+		pr_info("Wake samples (%d) =", count);
+	else
+		pr_info("Wake sample fifo cleared");
+
+	/* Write wake samples to fifo */
+	for (i = 0; i < count; i++) {
+		pr_cont(" %02x", wakeup_sample_buf[i]);
+		nvt_cir_wake_reg_write(nvt, wakeup_sample_buf[i],
+				       CIR_WAKE_WR_FIFO_DATA);
+	}
+	pr_cont("\n");
+
+	/* 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, count ? count :
+			       CIR_WAKE_FIFO_CMP_BYTES,
+			       CIR_WAKE_FIFO_CMP_DEEP);
+
+	spin_unlock_irqrestore(&nvt->nvt_lock, flags);
+
+	return 0;
+}
+
+static int nvt_ir_raw_set_filter(struct rc_dev *dev, enum rc_filter_type type,
+				 struct rc_scancode_filter *sc_filter)
+{
+	u8 *reg_buf;
+	u8 buf_val;
+	int i, ret, count;
+	unsigned int val;
+	const unsigned int BUF_SIZE = 127;
+	struct ir_raw_event *raw;
+
+	/* Other types are not valid for nuvoton */
+	if (type != RC_FILTER_WAKEUP)
+		return -EINVAL;
+
+	/* Require both mask and data to be set before actually committing */
+	if (!sc_filter->mask || !sc_filter->data)
+		return 0;
+
+	raw = kmalloc(BUF_SIZE * sizeof(*raw), GFP_KERNEL);
+	if (!raw)
+		return -ENOMEM;
+
+	ret = ir_raw_encode_scancode(dev->enabled_protocols, sc_filter, raw,
+				     BUF_SIZE);
+	if (ret < 0)
+		goto out_raw;
+
+	reg_buf = kmalloc(sizeof(*reg_buf) * BUF_SIZE, GFP_KERNEL);
+	if (!reg_buf) {
+		ret = -ENOMEM;
+		goto out_raw;
+	}
+
+	/* Inspect the ir samples */
+	for (i = 0, count = 0; i <= ret && count < BUF_SIZE; ++i) {
+		val = NS_TO_US((raw[i]).duration) / SAMPLE_PERIOD;
+
+		/* Split too large values into several smaller ones */
+		while (val > 0 && count < BUF_SIZE) {
+
+			/* Skip last value for better comparison tolerance */
+			if (i == ret && val < BUF_LEN_MASK)
+				break;
+
+			/* Clamp values to BUF_LEN_MASK at most */
+			buf_val = (val > BUF_LEN_MASK) ? BUF_LEN_MASK : val;
+
+			reg_buf[count] = buf_val;
+			val -= buf_val;
+			if ((raw[i]).pulse)
+				reg_buf[count] |= BUF_PULSE_BIT;
+			count++;
+		}
+	}
+
+	ret = nvt_write_wakeup_codes(dev, reg_buf, count);
+
+	kfree(reg_buf);
+out_raw:
+	kfree(raw);
+
+	return ret;
+}
+
 /*
  * nvt_tx_ir
  *
@@ -1043,6 +1161,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_filter = nvt_ir_raw_set_filter;
 	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 e1cf23c..9d0e161 100644
--- a/drivers/media/rc/nuvoton-cir.h
+++ b/drivers/media/rc/nuvoton-cir.h
@@ -63,6 +63,7 @@ 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;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 81cddd3..6b12a1b 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -227,6 +227,7 @@ static inline void init_ir_raw_event(struct ir_raw_event *ev)
 #define US_TO_NS(usec)		((usec) * 1000)
 #define MS_TO_US(msec)		((msec) * 1000)
 #define MS_TO_NS(msec)		((msec) * 1000 * 1000)
+#define NS_TO_US(nsec)		DIV_ROUND_UP(nsec, 1000L)
 
 void ir_raw_event_handle(struct rc_dev *dev);
 int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev);
-- 
1.8.3.2

Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support

[Antti Seppälä]

On 12 February 2014 01:39, James Hogan <james.hogan@imgtec.com> wrote:
> On Tuesday 11 February 2014 20:14:19 Antti Seppälä wrote:
>> Are you working on the wakeup protocol selector sysfs interface?
>
> I gave it a try yesterday, but it's a bit of a work in progress at the moment.
> It's also a bit more effort for img-ir to work properly with it, so I'd
> probably just limit the allowed wakeup protocols to the enabled normal
> protocol at first in img-ir.
>
> Here's what I have (hopefully kmail won't corrupt it), feel free to take and
> improve/fix it. I'm not keen on the invasiveness of the
> allowed_protos/enabled_protocols change (which isn't complete), but it
> should probably be abstracted at some point anyway.
>

Hi James.

In general the approach here looks good. At least I couldn't figure
any easy way to be less intrusive towards drivers/decoders and still
support wakeup filters.

I've just sent a new version of my rc-5-sz encoder patches for review.

I was thinking that once you are finished with the wakeup protocol
sysfs interface maybe you could combine my patchset and your changes
into a single "review patch" series and send it to the list? That
would keep the entire wakeup filter series easier to track.

Br,
-Antti

Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support

[James Hogan]

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

On Sunday 16 February 2014 19:04:01 Antti Seppälä wrote:
> On 12 February 2014 01:39, James Hogan <james.hogan@imgtec.com> wrote:
> > On Tuesday 11 February 2014 20:14:19 Antti Seppälä wrote:
> >> Are you working on the wakeup protocol selector sysfs interface?
> > 
> > I gave it a try yesterday, but it's a bit of a work in progress at the
> > moment. It's also a bit more effort for img-ir to work properly with it,
> > so I'd probably just limit the allowed wakeup protocols to the enabled
> > normal protocol at first in img-ir.
> > 
> > Here's what I have (hopefully kmail won't corrupt it), feel free to take
> > and improve/fix it. I'm not keen on the invasiveness of the
> > allowed_protos/enabled_protocols change (which isn't complete), but it
> > should probably be abstracted at some point anyway.
> 
> In general the approach here looks good. At least I couldn't figure
> any easy way to be less intrusive towards drivers/decoders and still
> support wakeup filters.

Thanks for taking a look (and sorry for the delay getting back to this, 
holiday and sickness got in the way). FYI I've cleaned up my wakeup_protocols 
patches a lot and will probably post tomorrow (after rebasing my patches to 
allow me to test img-ir properly with it).

Cheers
James

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]