[PATCH v2 0/9] rc: Add IR encode based wakeup filtering

[PATCH v2 0/9] rc: Add IR encode based wakeup filtering

[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.

I posted an RFC patchset showing how this could work, and Antti Seppälä
posted additional patches to support rc5-sz and nuvoton-cir. This
patchset improves the original RFC patches and combines & updates
Antti's patches.

I'm happy these patches are a good start at tackling the problem, as
long as Antti is happy with them and they work for him of course.

Future work could include:
 - Encoders for more protocols.
 - Carrier signal events (no use unless a driver makes use of it).

Patch 1 adds the new encode API.
Patches 2-3 adds some modulation helpers.
Patches 4-6 adds some raw encode implementations.
Patch 7 adds some rc-core support for encode based wakeup filtering.
Patch 8 adds debug loopback of encoded scancode when filter set.
Patch 9 (untested) adds encode based wakeup filtering to nuvoton-cir.

Changes in v2:

Patchset:
 - Alter encode API to return -ENOBUFS when there isn't enough buffer
   space. When this occurs all buffer contents must have been written
   with the partial encoding of the scancode. This is to allow drivers
   such as nuvoton-cir to provide a shorter buffer and still get a
   useful partial encoding for the wakeup pattern.
 - Added RC-5 & RC-5X encoder.
 - Add encode_wakeup support which keeps allowed wakeup protocols in
   sync with registered encoders.

Pulse-distance modulation helper:
 - Update ir_raw_gen_pd() with a kerneldoc comment and individual buffer
   full checks rather than a single one at the beginning, in order to
   support -ENOBUFS properly.
 - Update ir_raw_gen_pulse_space() to check the number of free slots and
   fill as many as possible before returning -ENOBUFS.
 - Fix brace placement for timings struct.

Manchester encoding helper:
 - Add kerneldoc comment.
 - Add individual buffer full checks, in order to support -ENOBUFS
   properly.
 - Make i unsigned to theoretically support all 32bits of data.
 - Increment *ev at end so caller can calculate correct number of
   events (during the loop *ev points to the last written event to allow
   it to be extended in length).
 - Make start/leader pulse optional, continuing from (*ev)[-1] if
   disabled. This helps support rc-5x which has a space in the middle of
   the bits.

rc5-sz encoder:
 - Turn ir_rc5_sz_encode() comment into kerneldoc and update to reflect
   new API.
 - Be more flexible around accepted scancode masks, as long as all the
   important bits are set (0x2fff) and none of the unimportant bits are
   set in the data. Also mask off the unimportant bits before passing to
   ir_raw_gen_manchester().
 - Explicitly enable leader bit in Manchester modulation timings.

rc-loopback:
 - Move img-ir-raw test code to rc-loopback.
 - Set encode_wakeup so that the set of allowed wakeup protocols matches
   the set of raw IR encoders.

nuvoton-cir:
 - Change reference to rc_dev::enabled_protocols to
   enabled_protocols[type] since it has been converted to an array.
 - Fix IR encoding buffer loop condition to be i < ret rather than i <=
   ret. The return value of ir_raw_encode_scancode is the number of
   events rather than the last event.
 - Set encode_wakeup so that the set of allowed wakeup protocols matches
   the set of raw IR encoders.

Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: David Härdeman <david@hardeman.nu>
Cc: Jarod Wilson <jarod@redhat.com>
Cc: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Cc: Hans Verkuil <hans.verkuil@cisco.com>

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

James Hogan (6):
  rc: ir-raw: Add scancode encoder callback
  rc: ir-raw: Add pulse-distance modulation helper
  rc: ir-nec-decoder: Add encode capability
  rc: ir-rc5-decoder: Add encode capability
  rc: rc-core: Add support for encode_wakeup drivers
  rc: rc-loopback: Add loopback of filter scancodes

 drivers/media/rc/ir-nec-decoder.c    |  93 +++++++++++++++++
 drivers/media/rc/ir-raw.c            | 191 +++++++++++++++++++++++++++++++++++
 drivers/media/rc/ir-rc5-decoder.c    | 103 +++++++++++++++++++
 drivers/media/rc/ir-rc5-sz-decoder.c |  45 +++++++++
 drivers/media/rc/nuvoton-cir.c       | 123 ++++++++++++++++++++++
 drivers/media/rc/nuvoton-cir.h       |   1 +
 drivers/media/rc/rc-core-priv.h      |  85 ++++++++++++++++
 drivers/media/rc/rc-loopback.c       |  39 +++++++
 drivers/media/rc/rc-main.c           |  11 +-
 include/media/rc-core.h              |   7 ++
 10 files changed, 695 insertions(+), 3 deletions(-)

-- 
1.8.3.2

[PATCH v2 1/9] 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@albanarts.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: David Härdeman <david@hardeman.nu>
---
Changes in v2:
 - Alter encode API to return -ENOBUFS when there isn't enough buffer
   space. When this occurs all buffer contents must have been written
   with the partial encoding of the scancode. This is to allow drivers
   such as nuvoton-cir to provide a shorter buffer and still get a
   useful partial encoding for the wakeup pattern.
---
 drivers/media/rc/ir-raw.c       | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/media/rc/rc-core-priv.h |  2 ++
 include/media/rc-core.h         |  3 +++
 3 files changed, 43 insertions(+)

diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index 763c9d1..01adc10 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -240,6 +240,44 @@ 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:	The number of events written.
+ *		-ENOBUFS if there isn't enough space in the array to fit the
+ *		encoding. In this case all @max events will have been written.
+ *		-EINVAL if the scancode is ambiguous or invalid, or if no
+ *		compatible encoder was 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 = -EINVAL;
+
+	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 || ret == -ENOBUFS)
+				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 da536c9..8afb971 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 0b9f890..8c64f9e 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -275,6 +275,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

[PATCH v2 2/9] rc: ir-raw: Add pulse-distance modulation helper

[James Hogan]

Add IR encoding helper for pulse-distance modulation as used by the NEC
protocol.

Signed-off-by: James Hogan <james@albanarts.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: David Härdeman <david@hardeman.nu>
---
Changes in v2:
 - Alter encode API to return -ENOBUFS when there isn't enough buffer
   space. When this occurs all buffer contents must have been written
   with the partial encoding of the scancode. This is to allow drivers
   such as nuvoton-cir to provide a shorter buffer and still get a
   useful partial encoding for the wakeup pattern.
 - Update ir_raw_gen_pd() with a kerneldoc comment and individual buffer
   full checks rather than a single one at the beginning, in order to
   support -ENOBUFS properly.
 - Update ir_raw_gen_pulse_space() to check the number of free slots and
   fill as many as possible before returning -ENOBUFS.
 - Fix brace placement for timings struct.
---
 drivers/media/rc/ir-raw.c       | 56 ++++++++++++++++++++++++++++++++++++
 drivers/media/rc/rc-core-priv.h | 63 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index 01adc10..f8fe10e 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -241,6 +241,62 @@ ir_raw_get_allowed_protocols(void)
 }
 
 /**
+ * ir_raw_gen_pd() - Encode data to raw events with pulse-distance modulation.
+ * @ev:		Pointer to pointer to next free event. *@ev is incremented for
+ *		each raw event filled.
+ * @max:	Maximum number of raw events to fill.
+ * @timings:	Pulse distance modulation timings.
+ * @n:		Number of bits of data.
+ * @data:	Data bits to encode.
+ *
+ * Encodes the @n least significant bits of @data using pulse-distance
+ * modulation with the timing characteristics described by @timings, writing up
+ * to @max raw IR events using the *@ev pointer.
+ *
+ * Returns:	0 on success.
+ *		-ENOBUFS if there isn't enough space in the array to fit the
+ *		full encoded data. In this case all @max events will have been
+ *		written.
+ */
+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;
+	int ret;
+
+	if (timings->header_pulse) {
+		ret = ir_raw_gen_pulse_space(ev, &max, timings->header_pulse,
+					     timings->header_space);
+		if (ret)
+			return ret;
+	}
+
+	if (timings->msb_first) {
+		for (i = n - 1; i >= 0; --i) {
+			ret = ir_raw_gen_pulse_space(ev, &max,
+					timings->bit_pulse,
+					timings->bit_space[(data >> i) & 1]);
+			if (ret)
+				return ret;
+		}
+	} else {
+		for (i = 0; i < n; ++i, data >>= 1) {
+			ret = ir_raw_gen_pulse_space(ev, &max,
+					timings->bit_pulse,
+					timings->bit_space[data & 1]);
+			if (ret)
+				return ret;
+		}
+	}
+
+	ret = ir_raw_gen_pulse_space(ev, &max, timings->trailer_pulse,
+				     timings->trailer_space);
+	return ret;
+}
+EXPORT_SYMBOL(ir_raw_gen_pd);
+
+/**
  * ir_raw_encode_scancode() - Encode a scancode as raw events
  *
  * @protocols:		permitted protocols
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index 8afb971..b55ae24 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -153,6 +153,69 @@ 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;
+}
+
+/**
+ * ir_raw_gen_pulse_space() - generate pulse and space raw events.
+ * @ev:			Pointer to pointer to next free raw event.
+ *			Will be incremented for each raw event written.
+ * @max:		Pointer to number of raw events available in buffer.
+ *			Will be decremented for each raw event written.
+ * @pulse_width:	Width of pulse in ns.
+ * @space_width:	Width of space in ns.
+ *
+ * Returns:	0 on success.
+ *		-ENOBUFS if there isn't enough buffer space to write both raw
+ *		events. In this case @max events will have been written.
+ */
+static inline int ir_raw_gen_pulse_space(struct ir_raw_event **ev,
+					 unsigned int *max,
+					 unsigned int pulse_width,
+					 unsigned int space_width)
+{
+	if (!*max)
+		return -ENOBUFS;
+	init_ir_raw_event_duration((*ev)++, 1, pulse_width);
+	if (!--*max)
+		return -ENOBUFS;
+	init_ir_raw_event_duration((*ev)++, 0, space_width);
+	--*max;
+	return 0;
+}
+
+/**
+ * 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

[PATCH v2 3/9] rc: ir-raw: Add Manchester encoder (phase encoder) helper

[James Hogan]

From: Antti Seppälä <a.seppala@gmail.com>

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>
Signed-off-by: James Hogan <james@albanarts.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: David Härdeman <david@hardeman.nu>
---
Changes in v2 (James Hogan):
 - Alter encode API to return -ENOBUFS when there isn't enough buffer
   space. When this occurs all buffer contents must have been written
   with the partial encoding of the scancode. This is to allow drivers
   such as nuvoton-cir to provide a shorter buffer and still get a
   useful partial encoding for the wakeup pattern.
 - Add kerneldoc comment.
 - Add individual buffer full checks, in order to support -ENOBUFS
   properly.
 - Make i unsigned to theoretically support all 32bits of data.
 - Increment *ev at end so caller can calculate correct number of
   events (during the loop *ev points to the last written event to allow
   it to be extended in length).
 - Make start/leader pulse optional, continuing from (*ev)[-1] if
   disabled. This helps support rc-5x which has a space in the middle of
   the bits.
---
 drivers/media/rc/ir-raw.c       | 82 +++++++++++++++++++++++++++++++++++++++++
 drivers/media/rc/rc-core-priv.h | 19 ++++++++++
 2 files changed, 101 insertions(+)

diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index f8fe10e..4310e82 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -241,6 +241,88 @@ ir_raw_get_allowed_protocols(void)
 }
 
 /**
+ * ir_raw_gen_manchester() - Encode data with Manchester (bi-phase) modulation.
+ * @ev:		Pointer to pointer to next free event. *@ev is incremented for
+ *		each raw event filled.
+ * @max:	Maximum number of raw events to fill.
+ * @timings:	Manchester modulation timings.
+ * @n:		Number of bits of data.
+ * @data:	Data bits to encode.
+ *
+ * Encodes the @n least significant bits of @data using Manchester (bi-phase)
+ * modulation with the timing characteristics described by @timings, writing up
+ * to @max raw IR events using the *@ev pointer.
+ *
+ * Returns:	0 on success.
+ *		-ENOBUFS if there isn't enough space in the array to fit the
+ *		full encoded data. In this case all @max events will have been
+ *		written.
+ */
+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;
+	unsigned int i;
+	int ret = -ENOBUFS;
+	i = 1 << (n - 1);
+
+	if (timings->leader) {
+		if (!max--)
+			return ret;
+		if (timings->pulse_space_start) {
+			init_ir_raw_event_duration((*ev)++, 1, timings->clock);
+
+			if (!max--)
+				return ret;
+			init_ir_raw_event_duration((*ev), 0, timings->clock);
+		} else {
+			init_ir_raw_event_duration((*ev), 1, timings->clock);
+		}
+		i >>= 1;
+	} else {
+		/* continue existing signal */
+		--(*ev);
+	}
+	/* from here on *ev will point to the last event rather than the next */
+
+	while (i > 0) {
+		need_pulse = !(data & i);
+		if (need_pulse == !!(*ev)->pulse) {
+			(*ev)->duration += timings->clock;
+		} else {
+			if (!max--)
+				goto nobufs;
+			init_ir_raw_event_duration(++(*ev), need_pulse,
+						   timings->clock);
+		}
+
+		if (!max--)
+			goto nobufs;
+		init_ir_raw_event_duration(++(*ev), !need_pulse,
+					   timings->clock);
+		i >>= 1;
+	}
+
+	if (timings->trailer_space) {
+		if (!(*ev)->pulse)
+			(*ev)->duration += timings->trailer_space;
+		else if (!max--)
+			goto nobufs;
+		else
+			init_ir_raw_event_duration(++(*ev), 0,
+						   timings->trailer_space);
+	}
+
+	ret = 0;
+nobufs:
+	/* point to the next event rather than last event before returning */
+	++(*ev);
+	return ret;
+}
+EXPORT_SYMBOL(ir_raw_gen_manchester);
+
+/**
  * ir_raw_gen_pd() - Encode data to raw events with pulse-distance modulation.
  * @ev:		Pointer to pointer to next free event. *@ev is incremented for
  *		each raw event filled.
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index b55ae24..c45b797 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -193,6 +193,25 @@ static inline int ir_raw_gen_pulse_space(struct ir_raw_event **ev,
 }
 
 /**
+ * struct ir_raw_timings_manchester - Manchester coding timings
+ * @leader:		1 if a leader bit is required (see @pulse_space_start)
+ *			0 if continuing existing signal
+ * @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 leader:1;
+	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

[PATCH v2 4/9] 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@albanarts.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: David Härdeman <david@hardeman.nu>
---
Changes in v2:
 - Update kerneldoc return values to reflect new API with -ENOBUFS
   return value when buffer is full and only a partial message was
   encoded.
---
 drivers/media/rc/ir-nec-decoder.c | 93 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index 9de1791..813fa6b 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -203,9 +203,102 @@ 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
+ *
+ * Returns:	The number of events written.
+ *		-ENOBUFS if there isn't enough space in the array to fit the
+ *		encoding. In this case all @max events will have been written.
+ *		-EINVAL if the scancode is ambiguous or invalid.
+ */
+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

[PATCH v2 5/9] rc: ir-rc5-decoder: Add encode capability

[James Hogan]

Add the capability to encode RC-5 and RC-5X scancodes as raw events. The
protocol is chosen based on the specified protocol mask, and whether all
the required bits are set in the scancode mask, and none of the unused
bits are set in the scancode data. For example a scancode filter with
bit 16 set in both data and mask is unambiguously RC-5X.

The Manchester modulation helper is used, and for RC-5X it is used twice
with two sets of timings, the first with a short trailer space for the
space in the middle, and the second with no leader so that it can
continue the space.

Signed-off-by: James Hogan <james@albanarts.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: David Härdeman <david@hardeman.nu>
---
I've mostly reverse engineered RC-5X from the decoder, but it seems to
work in loopback. Here's some debug output:

RC-5X:
evbug: Event. Dev: input0, Type: 4, Code: 4, Value: 65793
evbug: Event. Dev: input0, Type: 0, Code: 0, Value: 0
 _   __   _   _   _   _    _      _   _   _   _   _    __   _   _   _   _    _
| | |  | | | | | | | | |  | |    | | | | | | | | | |  |  | | | | | | | | |  | |          |
| |_|  |_| |_| |_| |_| |__| |____| |_| |_| |_| |_| |__|  |_| |_| |_| |_| |__| |__________|
      1                  1      3                    1  1                  1            8
 8 8  7 8 8 8 8 8 8 8 8  7 8    5 8 8 8 8 8 8 8 8 8  7  7 8 8 8 8 8 8 8 8  7 8          8
 8 8  7 8 8 8 8 8 8 8 8  7 8    5 8 8 8 8 8 8 8 8 8  7  7 8 8 8 8 8 8 8 8  7 8          8
 9 9  8 9 9 9 9 9 9 9 9  8 9    6 9 9 9 9 9 9 9 9 9  8  8 9 9 9 9 9 9 9 9  8 9          9
   1  0   0   0   0   0  1      X 0   0   0   0   0  1  0   0   0   0   0  1            E rc-5

RC-5:
evbug: Event. Dev: input0, Type: 4, Code: 4, Value: 257
evbug: Event. Dev: input0, Type: 0, Code: 0, Value: 0
 _   __   _   _   _   _    __   _   _   _   _    _
| | |  | | | | | | | | |  |  | | | | | | | | |  | |          |
| |_|  |_| |_| |_| |_| |__|  |_| |_| |_| |_| |__| |__________|
      1                  1  1                  1            8
 8 8  7 8 8 8 8 8 8 8 8  7  7 8 8 8 8 8 8 8 8  7 8          8
 8 8  7 8 8 8 8 8 8 8 8  7  7 8 8 8 8 8 8 8 8  7 8          8
 9 9  8 9 9 9 9 9 9 9 9  8  8 9 9 9 9 9 9 9 9  8 9          9
   1  0   0   0   0   0  1  0   0   0   0   0  1            E rc-5
---
 drivers/media/rc/ir-rc5-decoder.c | 103 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/drivers/media/rc/ir-rc5-decoder.c b/drivers/media/rc/ir-rc5-decoder.c
index 4295d9b..2eed2fe 100644
--- a/drivers/media/rc/ir-rc5-decoder.c
+++ b/drivers/media/rc/ir-rc5-decoder.c
@@ -171,9 +171,112 @@ out:
 	return -EINVAL;
 }
 
+static struct ir_raw_timings_manchester ir_rc5_timings = {
+	.leader			= 1,
+	.pulse_space_start	= 0,
+	.clock			= RC5_UNIT,
+	.trailer_space		= RC5_UNIT * 10,
+};
+
+static struct ir_raw_timings_manchester ir_rc5x_timings[2] = {
+	{
+		.leader			= 1,
+		.pulse_space_start	= 0,
+		.clock			= RC5_UNIT,
+		.trailer_space		= RC5X_SPACE,
+	},
+	{
+		.clock			= RC5_UNIT,
+		.trailer_space		= RC5_UNIT * 10,
+	},
+};
+
+static int ir_rc5_validate_filter(const struct rc_scancode_filter *scancode,
+				  unsigned int important_bits)
+{
+	/* all important bits of scancode should be set in mask */
+	if (~scancode->mask & important_bits)
+		return -EINVAL;
+	/* extra bits in mask should be zero in data */
+	if (scancode->mask & scancode->data & ~important_bits)
+		return -EINVAL;
+	return 0;
+}
+
+/**
+ * ir_rc5_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
+ *
+ * Returns:	The number of events written.
+ *		-ENOBUFS if there isn't enough space in the array to fit the
+ *		encoding. In this case all @max events will have been written.
+ *		-EINVAL if the scancode is ambiguous or invalid.
+ */
+static int ir_rc5_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;
+	unsigned int data, xdata, command, commandx, system;
+
+	/* Detect protocol and convert scancode to raw data */
+	if (protocols & RC_BIT_RC5 &&
+	    !ir_rc5_validate_filter(scancode, 0x1f7f)) {
+		/* decode scancode */
+		command  = (scancode->data & 0x003f) >> 0;
+		commandx = (scancode->data & 0x0040) >> 6;
+		system   = (scancode->data & 0x1f00) >> 8;
+		/* encode data */
+		data = !commandx << 12 | system << 6 | command;
+
+		/* Modulate the data */
+		ret = ir_raw_gen_manchester(&e, max, &ir_rc5_timings, RC5_NBITS,
+					    data);
+		if (ret < 0)
+			return ret;
+	} else if (protocols & RC_BIT_RC5X &&
+		   !ir_rc5_validate_filter(scancode, 0x1f7f3f)) {
+		/* decode scancode */
+		xdata    = (scancode->data & 0x00003f) >> 0;
+		command  = (scancode->data & 0x003f00) >> 8;
+		commandx = (scancode->data & 0x004000) >> 14;
+		system   = (scancode->data & 0x1f0000) >> 16;
+		/* commandx and system overlap, bits must match when encoded */
+		if (commandx == (system & 0x1))
+			return -EINVAL;
+		/* encode data */
+		data = 1 << 18 | system << 12 | command << 6 | xdata;
+
+		/* Modulate the data */
+		ret = ir_raw_gen_manchester(&e, max, &ir_rc5x_timings[0],
+					CHECK_RC5X_NBITS,
+					data >> (RC5X_NBITS-CHECK_RC5X_NBITS));
+		if (ret < 0)
+			return ret;
+		ret = ir_raw_gen_manchester(&e, max - (e - events),
+					&ir_rc5x_timings[1],
+					RC5X_NBITS - CHECK_RC5X_NBITS,
+					data);
+		if (ret < 0)
+			return ret;
+
+	} else {
+		return -EINVAL;
+	}
+
+	return e - events;
+}
+
 static struct ir_raw_handler rc5_handler = {
 	.protocols	= RC_BIT_RC5 | RC_BIT_RC5X,
 	.decode		= ir_rc5_decode,
+	.encode		= ir_rc5_encode,
 };
 
 static int __init ir_rc5_decode_init(void)
-- 
1.8.3.2

[PATCH v2 6/9] rc: ir-rc5-sz-decoder: Add ir encoding support

[James Hogan]

From: Antti Seppälä <a.seppala@gmail.com>

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>
Signed-off-by: James Hogan <james@albanarts.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: David Härdeman <david@hardeman.nu>
---
Changes in v2 (James Hogan):
 - Turn ir_rc5_sz_encode() comment into kerneldoc and update to reflect
   new API, with the -ENOBUFS return value for when the buffer is full
   and only a partial message was encoded.
 - Be more flexible around accepted scancode masks, as long as all the
   important bits are set (0x2fff) and none of the unimportant bits are
   set in the data. Also mask off the unimportant bits before passing to
   ir_raw_gen_manchester().
 - Explicitly enable leader bit in Manchester modulation timings.
---
 drivers/media/rc/ir-rc5-sz-decoder.c | 45 ++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c
index dc18b74..a342a4f 100644
--- a/drivers/media/rc/ir-rc5-sz-decoder.c
+++ b/drivers/media/rc/ir-rc5-sz-decoder.c
@@ -127,9 +127,54 @@ out:
 	return -EINVAL;
 }
 
+static struct ir_raw_timings_manchester ir_rc5_sz_timings = {
+	.leader			= 1,
+	.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
+ *
+ * Returns:	The number of events written.
+ *		-ENOBUFS if there isn't enough space in the array to fit the
+ *		encoding. In this case all @max events will have been written.
+ *		-EINVAL if the scancode is ambiguous or invalid.
+ */
+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;
+
+	/* all important bits of scancode should be set in mask */
+	if (~scancode->mask & 0x2fff)
+		return -EINVAL;
+	/* extra bits in mask should be zero in data */
+	if (scancode->mask & scancode->data & ~0x2fff)
+		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 & 0x2fff);
+	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

[PATCH v2 7/9] rc: rc-core: Add support for encode_wakeup drivers

[James Hogan]

Add support in rc-core for drivers which implement the wakeup scancode
filter by encoding the scancode using the raw IR encoders. This is by
way of rc_dev::encode_wakeup which should be set to true to make the
allowed wakeup protocols the same as the set of raw IR encoders.

As well as updating the sysfs interface to know which wakeup protocols
are allowed for encode_wakeup drivers, also ensure that the IR
decoders/encoders are loaded when an encode_wakeup driver is registered.

Signed-off-by: James Hogan <james@albanarts.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: David Härdeman <david@hardeman.nu>
---
Changes in v2:
 - New patch
---
 drivers/media/rc/ir-raw.c       | 15 +++++++++++++++
 drivers/media/rc/rc-core-priv.h |  1 +
 drivers/media/rc/rc-main.c      | 11 ++++++++---
 include/media/rc-core.h         |  3 +++
 4 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index 4310e82..d8ad81c 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -30,6 +30,7 @@ static LIST_HEAD(ir_raw_client_list);
 static DEFINE_MUTEX(ir_raw_handler_lock);
 static LIST_HEAD(ir_raw_handler_list);
 static u64 available_protocols;
+static u64 encode_protocols;
 
 static int ir_raw_event_thread(void *data)
 {
@@ -240,6 +241,16 @@ ir_raw_get_allowed_protocols(void)
 	return protocols;
 }
 
+/* used internally by the sysfs interface */
+u64 ir_raw_get_encode_protocols(void)
+{
+	u64 protocols;
+	mutex_lock(&ir_raw_handler_lock);
+	protocols = encode_protocols;
+	mutex_unlock(&ir_raw_handler_lock);
+	return protocols;
+}
+
 /**
  * ir_raw_gen_manchester() - Encode data with Manchester (bi-phase) modulation.
  * @ev:		Pointer to pointer to next free event. *@ev is incremented for
@@ -498,6 +509,8 @@ int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler)
 		list_for_each_entry(raw, &ir_raw_client_list, list)
 			ir_raw_handler->raw_register(raw->dev);
 	available_protocols |= ir_raw_handler->protocols;
+	if (ir_raw_handler->encode)
+		encode_protocols |= ir_raw_handler->protocols;
 	mutex_unlock(&ir_raw_handler_lock);
 
 	return 0;
@@ -514,6 +527,8 @@ void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler)
 		list_for_each_entry(raw, &ir_raw_client_list, list)
 			ir_raw_handler->raw_unregister(raw->dev);
 	available_protocols &= ~ir_raw_handler->protocols;
+	if (ir_raw_handler->encode)
+		encode_protocols &= ~ir_raw_handler->protocols;
 	mutex_unlock(&ir_raw_handler_lock);
 }
 EXPORT_SYMBOL(ir_raw_handler_unregister);
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index c45b797..767ef69 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -239,6 +239,7 @@ int ir_raw_gen_pd(struct ir_raw_event **ev, unsigned int max,
  * Routines from rc-raw.c to be used internally and by decoders
  */
 u64 ir_raw_get_allowed_protocols(void);
+u64 ir_raw_get_encode_protocols(void);
 int ir_raw_event_register(struct rc_dev *dev);
 void ir_raw_event_unregister(struct rc_dev *dev);
 int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler);
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 99697aa..712a2d7 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -857,8 +857,10 @@ static ssize_t show_protocols(struct device *device,
 	mutex_lock(&dev->lock);
 
 	enabled = dev->enabled_protocols[fattr->type];
-	if (dev->driver_type == RC_DRIVER_SCANCODE ||
-	    fattr->type == RC_FILTER_WAKEUP)
+	if (dev->encode_wakeup && fattr->type == RC_FILTER_WAKEUP)
+		allowed = ir_raw_get_encode_protocols();
+	else if (dev->driver_type == RC_DRIVER_SCANCODE ||
+		 fattr->type == RC_FILTER_WAKEUP)
 		allowed = dev->allowed_protocols[fattr->type];
 	else if (dev->raw)
 		allowed = ir_raw_get_allowed_protocols();
@@ -1350,13 +1352,16 @@ int rc_register_device(struct rc_dev *dev)
 		path ? path : "N/A");
 	kfree(path);
 
-	if (dev->driver_type == RC_DRIVER_IR_RAW) {
+	if (dev->driver_type == RC_DRIVER_IR_RAW || dev->encode_wakeup) {
 		/* Load raw decoders, if they aren't already */
 		if (!raw_init) {
 			IR_dprintk(1, "Loading raw decoders\n");
 			ir_raw_init();
 			raw_init = true;
 		}
+	}
+
+	if (dev->driver_type == RC_DRIVER_IR_RAW) {
 		rc = ir_raw_event_register(dev);
 		if (rc < 0)
 			goto out_input;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 8c64f9e..2d81d6c 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -73,6 +73,8 @@ enum rc_filter_type {
  * @input_dev: the input child device used to communicate events to userspace
  * @driver_type: specifies if protocol decoding is done in hardware or software
  * @idle: used to keep track of RX state
+ * @encode_wakeup: wakeup filtering uses IR encode API, therefore the allowed
+ *	wakeup protocols is the set of all raw encoders
  * @allowed_protocols: bitmask with the supported RC_BIT_* protocols for each
  *	filter type
  * @enabled_protocols: bitmask with the enabled RC_BIT_* protocols for each
@@ -128,6 +130,7 @@ struct rc_dev {
 	struct input_dev		*input_dev;
 	enum rc_driver_type		driver_type;
 	bool				idle;
+	bool				encode_wakeup;
 	u64				allowed_protocols[RC_FILTER_MAX];
 	u64				enabled_protocols[RC_FILTER_MAX];
 	u32				users;
-- 
1.8.3.2

[PATCH v2 8/9] rc: rc-loopback: Add loopback of filter scancodes

[James Hogan]

Add the s_filter callback to the rc-loopback driver, which instead of
setting the filter just feeds the scancode back through the input device
so that it can be verified.

Signed-off-by: James Hogan <james@albanarts.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
---
Changes in v2:
 - Move img-ir-raw test code to rc-loopback.
 - Handle new encode API, specifically -ENOBUFS when the buffer isn't
   long enough.
 - Set encode_wakeup so that the set of allowed wakeup protocols matches
   the set of raw IR encoders.
---
 drivers/media/rc/rc-loopback.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
index 0a88e0c..aefd335 100644
--- a/drivers/media/rc/rc-loopback.c
+++ b/drivers/media/rc/rc-loopback.c
@@ -26,6 +26,7 @@
 #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"
@@ -176,6 +177,42 @@ static int loop_set_carrier_report(struct rc_dev *dev, int enable)
 	return 0;
 }
 
+static int loop_set_filter(struct rc_dev *dev, enum rc_filter_type type,
+			   struct rc_scancode_filter *sc_filter)
+{
+	static const unsigned int max = 512;
+	struct ir_raw_event *raw;
+	int ret;
+	int i;
+
+	/* fine to disable filter */
+	if (!sc_filter->mask)
+		return 0;
+
+	/* encode the specified filter and loop it back */
+	raw = kmalloc(max * sizeof(*raw), GFP_KERNEL);
+	ret = ir_raw_encode_scancode(dev->enabled_protocols[type], sc_filter,
+				     raw, max);
+	/* still loop back the partial raw IR even if it's incomplete */
+	if (ret == -ENOBUFS)
+		ret = max;
+	if (ret >= 0) {
+		/* do the loopback */
+		for (i = 0; i < ret; ++i)
+			ir_raw_event_store(dev, &raw[i]);
+		ir_raw_event_handle(dev);
+
+		ret = 0;
+	} else if (type == RC_FILTER_NORMAL) {
+		/* accept any normal filter */
+		ret = 0;
+	}
+
+	kfree(raw);
+
+	return ret;
+}
+
 static int __init loop_init(void)
 {
 	struct rc_dev *rc;
@@ -195,6 +232,7 @@ static int __init loop_init(void)
 	rc->map_name		= RC_MAP_EMPTY;
 	rc->priv		= &loopdev;
 	rc->driver_type		= RC_DRIVER_IR_RAW;
+	rc->encode_wakeup	= true;
 	rc_set_allowed_protocols(rc, RC_BIT_ALL);
 	rc->timeout		= 100 * 1000 * 1000; /* 100 ms */
 	rc->min_timeout		= 1;
@@ -209,6 +247,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_filter		= loop_set_filter;
 
 	loopdev.txmask		= RXMASK_REGULAR;
 	loopdev.txcarrier	= 36000;
-- 
1.8.3.2

[PATCH v2 9/9] rc: nuvoton-cir: Add support for writing wakeup samples via sysfs filter callback

[James Hogan]

From: Antti Seppälä <a.seppala@gmail.com>

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>
Signed-off-by: James Hogan <james@albanarts.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Jarod Wilson <jarod@redhat.com>
Cc: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
---
Please note this patch is only build tested.

Changes in v2 (James Hogan):
 - Handle new -ENOBUFS when IR encoding buffer isn't long enough and
   reduce buffer size to the size of the wake fifo so that time isn't
   wasted processing encoded IR events that will be discarded. Also only
   discard the last event if the encoded data is complete.
 - Change reference to rc_dev::enabled_protocols to
   enabled_protocols[type] since it has been converted to an array.
 - Fix IR encoding buffer loop condition to be i < ret rather than i <=
   ret. The return value of ir_raw_encode_scancode is the number of
   events rather than the last event.
 - Set encode_wakeup so that the set of allowed wakeup protocols matches
   the set of raw IR encoders.
---
 drivers/media/rc/nuvoton-cir.c | 123 +++++++++++++++++++++++++++++++++++++++++
 drivers/media/rc/nuvoton-cir.h |   1 +
 include/media/rc-core.h        |   1 +
 3 files changed, 125 insertions(+)

diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index d244e1a..ec8f4fc 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -527,6 +527,127 @@ 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;
+	struct ir_raw_event *raw;
+	bool complete;
+
+	/* 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[type], sc_filter,
+				     raw, WAKE_FIFO_LEN);
+	complete = (ret != -ENOBUFS);
+	if (!complete)
+		ret = WAKE_FIFO_LEN;
+	else if (ret < 0)
+		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, count = 0; i < ret && count < WAKE_FIFO_LEN; ++i) {
+		val = NS_TO_US((raw[i]).duration) / SAMPLE_PERIOD;
+
+		/* Split too large values into several smaller ones */
+		while (val > 0 && count < WAKE_FIFO_LEN) {
+
+			/* Skip last value for better comparison tolerance */
+			if (complete && i == ret - 1 && 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
  *
@@ -1044,11 +1165,13 @@ static int nvt_probe(struct pnp_dev *pdev, const struct pnp_device_id *dev_id)
 	/* Set up the rc device */
 	rdev->priv = nvt;
 	rdev->driver_type = RC_DRIVER_IR_RAW;
+	rdev->encode_wakeup = true;
 	rc_set_allowed_protocols(rdev, RC_BIT_ALL);
 	rdev->open = nvt_open;
 	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 2d81d6c..80177da 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -271,6 +271,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: [PATCH v2 0/9] rc: Add IR encode based wakeup filtering

[Antti Seppälä]

On 15 March 2014 01:04, James Hogan <james@albanarts.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.
>
> I posted an RFC patchset showing how this could work, and Antti Seppälä
> posted additional patches to support rc5-sz and nuvoton-cir. This
> patchset improves the original RFC patches and combines & updates
> Antti's patches.
>
> I'm happy these patches are a good start at tackling the problem, as
> long as Antti is happy with them and they work for him of course.
>
> Future work could include:
>  - Encoders for more protocols.
>  - Carrier signal events (no use unless a driver makes use of it).
>
> Patch 1 adds the new encode API.
> Patches 2-3 adds some modulation helpers.
> Patches 4-6 adds some raw encode implementations.
> Patch 7 adds some rc-core support for encode based wakeup filtering.
> Patch 8 adds debug loopback of encoded scancode when filter set.
> Patch 9 (untested) adds encode based wakeup filtering to nuvoton-cir.
>

Hi James.

This is looking very good. I've reviewed the series and have only
minor comments to some of the patches which I'll post individually
shortly.

I've also tested the nuvoton with actual hardware with rc-5-sz and nec
encoders and both generate wakeup samples correctly and can wake the
system.

While doing my tests I also noticed that there is a small bug in the
wakeup_protocols handling where one can enable multiple protocols with
the + -notation. E.g. echo "+nec +rc-5" >
/sys/class/rc/rc0/wakeup_protocols shouldn't probably succeed.

-Antti

Re: [PATCH v2 6/9] rc: ir-rc5-sz-decoder: Add ir encoding support

[Antti Seppälä]

On 15 March 2014 01:04, James Hogan <james@albanarts.com> wrote:
> From: Antti Seppälä <a.seppala@gmail.com>
>
> 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>
> Signed-off-by: James Hogan <james@albanarts.com>
> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> Cc: David Härdeman <david@hardeman.nu>
> ---
> Changes in v2 (James Hogan):
>  - Turn ir_rc5_sz_encode() comment into kerneldoc and update to reflect
>    new API, with the -ENOBUFS return value for when the buffer is full
>    and only a partial message was encoded.
>  - Be more flexible around accepted scancode masks, as long as all the
>    important bits are set (0x2fff) and none of the unimportant bits are
>    set in the data. Also mask off the unimportant bits before passing to
>    ir_raw_gen_manchester().
>  - Explicitly enable leader bit in Manchester modulation timings.
> ---
>  drivers/media/rc/ir-rc5-sz-decoder.c | 45 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c
> index dc18b74..a342a4f 100644
> --- a/drivers/media/rc/ir-rc5-sz-decoder.c
> +++ b/drivers/media/rc/ir-rc5-sz-decoder.c
> @@ -127,9 +127,54 @@ out:
>         return -EINVAL;
>  }
>
> +static struct ir_raw_timings_manchester ir_rc5_sz_timings = {
> +       .leader                 = 1,
> +       .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
> + *
> + * Returns:    The number of events written.
> + *             -ENOBUFS if there isn't enough space in the array to fit the
> + *             encoding. In this case all @max events will have been written.
> + *             -EINVAL if the scancode is ambiguous or invalid.
> + */
> +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;
> +
> +       /* all important bits of scancode should be set in mask */
> +       if (~scancode->mask & 0x2fff)

Do we want to be so restrictive here? In my opinion it's quite nice to
be able to encode also the toggle bit if needed. Therefore a check
against 0x3fff would be a better choice.

I think the ability to encode toggle bit might also be nice to have
for rc-5(x) also.

> +               return -EINVAL;
> +       /* extra bits in mask should be zero in data */
> +       if (scancode->mask & scancode->data & ~0x2fff)
> +               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 & 0x2fff);

I'm not sure that the & 0x2fff is necessary. It has the ill effect of
eventually writing something else to hardware while still committing
the filter as unmodified original value. This will result in
difference between what sysfs states was written when read back and
what was actually written.

I think checks above are good enough to restrict the values and as
little modification as possible of the data should be done after that.

> +       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
>

Re: [PATCH v2 9/9] rc: nuvoton-cir: Add support for writing wakeup samples via sysfs filter callback

[Antti Seppälä]

On 15 March 2014 01:04, James Hogan <james@albanarts.com> wrote:
> From: Antti Seppälä <a.seppala@gmail.com>
>
> 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>
> Signed-off-by: James Hogan <james@albanarts.com>
> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> Cc: Jarod Wilson <jarod@redhat.com>
> Cc: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> Please note this patch is only build tested.
>
> Changes in v2 (James Hogan):
>  - Handle new -ENOBUFS when IR encoding buffer isn't long enough and
>    reduce buffer size to the size of the wake fifo so that time isn't
>    wasted processing encoded IR events that will be discarded. Also only
>    discard the last event if the encoded data is complete.
>  - Change reference to rc_dev::enabled_protocols to
>    enabled_protocols[type] since it has been converted to an array.
>  - Fix IR encoding buffer loop condition to be i < ret rather than i <=
>    ret. The return value of ir_raw_encode_scancode is the number of
>    events rather than the last event.
>  - Set encode_wakeup so that the set of allowed wakeup protocols matches
>    the set of raw IR encoders.
> ---
>  drivers/media/rc/nuvoton-cir.c | 123 +++++++++++++++++++++++++++++++++++++++++
>  drivers/media/rc/nuvoton-cir.h |   1 +
>  include/media/rc-core.h        |   1 +
>  3 files changed, 125 insertions(+)
>
> diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
> index d244e1a..ec8f4fc 100644
> --- a/drivers/media/rc/nuvoton-cir.c
> +++ b/drivers/media/rc/nuvoton-cir.c
> @@ -527,6 +527,127 @@ 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;
> +       }

Now that the encoders support partial encoding the above check against
WAKE_FIFO_LEN never triggers and can be removed.

> +
> +       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;
> +       struct ir_raw_event *raw;
> +       bool complete;
> +
> +       /* 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[type], sc_filter,
> +                                    raw, WAKE_FIFO_LEN);
> +       complete = (ret != -ENOBUFS);
> +       if (!complete)
> +               ret = WAKE_FIFO_LEN;
> +       else if (ret < 0)
> +               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, count = 0; i < ret && count < WAKE_FIFO_LEN; ++i) {
> +               val = NS_TO_US((raw[i]).duration) / SAMPLE_PERIOD;
> +
> +               /* Split too large values into several smaller ones */
> +               while (val > 0 && count < WAKE_FIFO_LEN) {
> +
> +                       /* Skip last value for better comparison tolerance */
> +                       if (complete && i == ret - 1 && 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
>   *
> @@ -1044,11 +1165,13 @@ static int nvt_probe(struct pnp_dev *pdev, const struct pnp_device_id *dev_id)
>         /* Set up the rc device */
>         rdev->priv = nvt;
>         rdev->driver_type = RC_DRIVER_IR_RAW;
> +       rdev->encode_wakeup = true;
>         rc_set_allowed_protocols(rdev, RC_BIT_ALL);
>         rdev->open = nvt_open;
>         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 2d81d6c..80177da 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -271,6 +271,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: [PATCH v2 6/9] rc: ir-rc5-sz-decoder: Add ir encoding support

[James Hogan]

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

Hi Antti,

On Sunday 16 March 2014 10:34:31 Antti Seppälä wrote:
> > +/**
> > + * 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
> > + *
> > + * Returns:    The number of events written.
> > + *             -ENOBUFS if there isn't enough space in the array to fit
> > the + *             encoding. In this case all @max events will have been
> > written. + *             -EINVAL if the scancode is ambiguous or invalid.
> > + */
> > +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;
> > +
> > +       /* all important bits of scancode should be set in mask */
> > +       if (~scancode->mask & 0x2fff)
> 
> Do we want to be so restrictive here? In my opinion it's quite nice to
> be able to encode also the toggle bit if needed. Therefore a check
> against 0x3fff would be a better choice.
> 
> I think the ability to encode toggle bit might also be nice to have
> for rc-5(x) also.
> 

I don't believe the toggle bit is encoded in the scancode though, so I'm not 
sure it makes sense to treat it like that. I'm not an expert on RC-5 like 
protocols or the use of the toggle bit though.

> > +               return -EINVAL;
> > +       /* extra bits in mask should be zero in data */
> > +       if (scancode->mask & scancode->data & ~0x2fff)
> > +               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 &
> > 0x2fff);
> 
> I'm not sure that the & 0x2fff is necessary. It has the ill effect of
> eventually writing something else to hardware while still committing
> the filter as unmodified original value. This will result in
> difference between what sysfs states was written when read back and
> what was actually written.
> 
> I think checks above are good enough to restrict the values and as
> little modification as possible of the data should be done after that.

I suspect it was the toggle bit I was thinking about, e.g.
echo 0x3fff > wakeup_filter
echo 0x2fff > wakeup_filter_mask

I had assumed shouldn't be able to affect the toggle bit (it's not set in 
mask).

Cheers
James

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

Re: [PATCH v2 9/9] rc: nuvoton-cir: Add support for writing wakeup samples via sysfs filter callback

[James Hogan]

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

On Sunday 16 March 2014 10:39:39 Antti Seppälä wrote:
> > +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;
> > +       }
> 
> Now that the encoders support partial encoding the above check against
> WAKE_FIFO_LEN never triggers and can be removed.

Yep, good point

Thanks
James

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

Re: [PATCH v2 6/9] rc: ir-rc5-sz-decoder: Add ir encoding support

[Antti Seppälä]

Hi James.

On 16 March 2014 13:50, James Hogan <james@albanarts.com> wrote:
> Hi Antti,
>
> On Sunday 16 March 2014 10:34:31 Antti Seppälä wrote:
>> > +
>> > +       /* all important bits of scancode should be set in mask */
>> > +       if (~scancode->mask & 0x2fff)
>>
>> Do we want to be so restrictive here? In my opinion it's quite nice to
>> be able to encode also the toggle bit if needed. Therefore a check
>> against 0x3fff would be a better choice.
>>
>> I think the ability to encode toggle bit might also be nice to have
>> for rc-5(x) also.
>>
>
> I don't believe the toggle bit is encoded in the scancode though, so I'm not
> sure it makes sense to treat it like that. I'm not an expert on RC-5 like
> protocols or the use of the toggle bit though.
>

Well I'm not an expert either but at least streamzap tends to have the
toggle bit enabled quite often when sending ir pulses.

When decoding the toggle is always removed from the scancode but when
encoding it would be useful to have the possibility to encode it in.
This is because setting the toggle bit into wakeup makes it easier to
wake the system with nuvoton hw as it is difficult to press the remote
key short time enough (less than around 112ms) to generate a pulse
without the toggle bit set.

-Antti

Re: [PATCH v2 6/9] rc: ir-rc5-sz-decoder: Add ir encoding support

[James Hogan]

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

On Sunday 16 March 2014 14:14:58 Antti Seppälä wrote:
> Hi James.
> 
> On 16 March 2014 13:50, James Hogan <james@albanarts.com> wrote:
> > Hi Antti,
> > 
> > On Sunday 16 March 2014 10:34:31 Antti Seppälä wrote:
> >> > +
> >> > +       /* all important bits of scancode should be set in mask */
> >> > +       if (~scancode->mask & 0x2fff)
> >> 
> >> Do we want to be so restrictive here? In my opinion it's quite nice to
> >> be able to encode also the toggle bit if needed. Therefore a check
> >> against 0x3fff would be a better choice.
> >> 
> >> I think the ability to encode toggle bit might also be nice to have
> >> for rc-5(x) also.
> > 
> > I don't believe the toggle bit is encoded in the scancode though, so I'm
> > not sure it makes sense to treat it like that. I'm not an expert on RC-5
> > like protocols or the use of the toggle bit though.
> 
> Well I'm not an expert either but at least streamzap tends to have the
> toggle bit enabled quite often when sending ir pulses.
> 
> When decoding the toggle is always removed from the scancode but when
> encoding it would be useful to have the possibility to encode it in.
> This is because setting the toggle bit into wakeup makes it easier to
> wake the system with nuvoton hw as it is difficult to press the remote
> key short time enough (less than around 112ms) to generate a pulse
> without the toggle bit set.

Fair enough. So changing the minimum rc5-sz masks to 0x3fff sounds reasonable 
to allow toggle to be controlled.

Just to clarify though, so you mean that the remote uses toggle=1 first (and 
in repeat codes) unless you press it a second time (new keypress) within a 
short amount of time?
I.e. like this?
Press	message toggle=1
		repeat toggle=1
		repeat toggle=1
unpress
Press	message toggle=!last_toggle only if within X ms, 1 otherwise

Sounds like for RC-5/RC-5X toggle should probably be taken from 0x00002000, 
0x00200000 of scancode respectively (just above "system" in both cases).

Cheers
James

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

Re: [PATCH v2 0/9] rc: Add IR encode based wakeup filtering

[James Hogan]

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

On Sunday 16 March 2014 10:22:02 Antti Seppälä wrote:
> On 15 March 2014 01:04, James Hogan <james@albanarts.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.
> > 
> > I posted an RFC patchset showing how this could work, and Antti Seppälä
> > posted additional patches to support rc5-sz and nuvoton-cir. This
> > patchset improves the original RFC patches and combines & updates
> > Antti's patches.
> > 
> > I'm happy these patches are a good start at tackling the problem, as
> > long as Antti is happy with them and they work for him of course.
> > 
> > Future work could include:
> >  - Encoders for more protocols.
> >  - Carrier signal events (no use unless a driver makes use of it).
> > 
> > Patch 1 adds the new encode API.
> > Patches 2-3 adds some modulation helpers.
> > Patches 4-6 adds some raw encode implementations.
> > Patch 7 adds some rc-core support for encode based wakeup filtering.
> > Patch 8 adds debug loopback of encoded scancode when filter set.
> > Patch 9 (untested) adds encode based wakeup filtering to nuvoton-cir.
> 
> Hi James.
> 
> This is looking very good. I've reviewed the series and have only
> minor comments to some of the patches which I'll post individually
> shortly.
> 
> I've also tested the nuvoton with actual hardware with rc-5-sz and nec
> encoders and both generate wakeup samples correctly and can wake the
> system.

Thanks for reviewing and testing!

> While doing my tests I also noticed that there is a small bug in the
> wakeup_protocols handling where one can enable multiple protocols with
> the + -notation. E.g. echo "+nec +rc-5" >
> /sys/class/rc/rc0/wakeup_protocols shouldn't probably succeed.

Yeh I'm in two minds about this now. It's actually a little awkward since some 
of the protocols have multiple variants (i.e. "rc-5" = RC5+RC5X), but an 
encoded message is only ever a single variant, so technically if you're going 
to draw the line for wakeup protocols it should probably be at one enabled 
variant, which isn't always convenient or necessary.

Note, ATM even disallowing "+proto" and "-proto" we would already have to 
guess which variant is desired from the scancode data, which in the case of 
NEC scancodes is a bit horrid since NEC scancodes are ambiguous. This actually 
means it's driver specific whether a filter mask of 0x0000ffff filters out 
NEC32/NEC-X messages (scancode/encode driver probably will since it needs to 
pick a variant, but software fallback won't).

Ideally there'd now be a way to specify the protocol variants exactly as well 
as whole protocols groups through this sysfs interface (and probably NEC 
should have protocol bits for each variant too, which is tricky ATM since 
keymaps can use scancodes of multiple variants and it's hard to guarantee 
which variant was intended for each key mapping by reading it).

Adding proto_names entries for each variant is easy enough, though I'm not 
sure what you'd call the one for RC_BIT_RC5 (since "rc-5" is taken to mean 
both RC5 and RC5X). We could then check that the enabled wakeup protocols fit 
entirely within one of the proto_names entry's proto mask if we think it's 
helpful (which would allow you to specify e.g. "sony12", or "sony" (sony 
12,15,20), or "sony -sony12" (sony 15,20), but not "+sony +nec").

Cheers
James

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

Re: [PATCH v2 6/9] rc: ir-rc5-sz-decoder: Add ir encoding support

[Antti Seppälä]

On 16 March 2014 23:18, James Hogan <james@albanarts.com> wrote:
> Fair enough. So changing the minimum rc5-sz masks to 0x3fff sounds reasonable
> to allow toggle to be controlled.
>
> Just to clarify though, so you mean that the remote uses toggle=1 first (and
> in repeat codes) unless you press it a second time (new keypress) within a
> short amount of time?
> I.e. like this?
> Press   message toggle=1
>                 repeat toggle=1
>                 repeat toggle=1
> unpress
> Press   message toggle=!last_toggle only if within X ms, 1 otherwise
>

Actually studying this a little closer it seems that it indeed behaves
like a "toggle":

Press   message toggle=1
                repeat toggle=1
                repeat toggle=1
unpress
Press   message toggle=!last_toggle, always

So the toggle is inverted between presses and its value is kept during
repeat. It however seems to behave a little bit sporadically here
tending to set the toggle bit on more often than off.

Anyway I think that allowing the toggle bit to be set in the scancode
does not really hurt. I guess most of the time people will use the
scancodes without the toggle bit.

Br,
-Antti

Re: [PATCH v2 0/9] rc: Add IR encode based wakeup filtering

[Antti Seppälä]

Hi James,

On 17 March 2014 00:41, James Hogan <james@albanarts.com> wrote:
>
> Yeh I'm in two minds about this now. It's actually a little awkward since some
> of the protocols have multiple variants (i.e. "rc-5" = RC5+RC5X), but an
> encoded message is only ever a single variant, so technically if you're going
> to draw the line for wakeup protocols it should probably be at one enabled
> variant, which isn't always convenient or necessary.

I'd very much prefer to have the selector as it currently is -
protocol groups instead of variants which would keep it consistent
with decoding protocol selection.

>
> Note, ATM even disallowing "+proto" and "-proto" we would already have to
> guess which variant is desired from the scancode data, which in the case of
> NEC scancodes is a bit horrid since NEC scancodes are ambiguous. This actually
> means it's driver specific whether a filter mask of 0x0000ffff filters out
> NEC32/NEC-X messages (scancode/encode driver probably will since it needs to
> pick a variant, but software fallback won't).
>

How common is it that NEC codes are really ambiguous? Or that a wrong
variant is selected for encoding? A quick look suggests that the
length of the scancode will be good enough way to determine which
variant is used for NEC, RC-5(X) and RC-6(A).

If the variant is really needed selecting it might be done in some
other sysfs file. But I'd not implement it yet if we can manage
without such logic.

-Antti

Re: [PATCH v2 0/9] rc: Add IR encode based wakeup filtering

[James Hogan]

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

On Monday 17 March 2014 19:01:51 Antti Seppälä wrote:
> On 17 March 2014 00:41, James Hogan <james@albanarts.com> wrote:
> > Yeh I'm in two minds about this now. It's actually a little awkward since
> > some of the protocols have multiple variants (i.e. "rc-5" = RC5+RC5X),
> > but an encoded message is only ever a single variant, so technically if
> > you're going to draw the line for wakeup protocols it should probably be
> > at one enabled variant, which isn't always convenient or necessary.
> 
> I'd very much prefer to have the selector as it currently is -
> protocol groups instead of variants which would keep it consistent
> with decoding protocol selection.

Yeh, I'll submit a patch to fix wakeup-protocols to disallow multiple groups 
of protocols from being enabled at the same time.

> > Note, ATM even disallowing "+proto" and "-proto" we would already have to
> > guess which variant is desired from the scancode data, which in the case
> > of
> > NEC scancodes is a bit horrid since NEC scancodes are ambiguous. This
> > actually means it's driver specific whether a filter mask of 0x0000ffff
> > filters out NEC32/NEC-X messages (scancode/encode driver probably will
> > since it needs to pick a variant, but software fallback won't).
> 
> How common is it that NEC codes are really ambiguous? Or that a wrong
> variant is selected for encoding? A quick look suggests that the
> length of the scancode will be good enough way to determine which
> variant is used for NEC, RC-5(X) and RC-6(A).

When I tried filtering for my TV remote it didn't work. It turned out to be 
because the extended nec scancode has the address bytes in the wrong order so 
that the bits are discontinuous compared to the raw data. The remote uses 
extended NEC but has zero in the lower byte of the address, which 
unfortunately goes in bits 23:16 of the scancode above the other byte of the 
address, so it looks as if it's using normal NEC (16bit scancodes). This is 
why I ended up making img-ir use the mask too in the decision.

It's ambiguous the other way too (which is probably a strong point against 
having actual protocol bits for each NEC variant, since they only differ in 
how the scancode is constructed). E.g. the Tivo keymap is 32-bit NEC, but has 
extended NEC scancodes where the bytes of the command are complements (i.e. 
the extended NEC command checksum passes). This makes it hard to filter on at 
the scancode level (the drivers will probably get it right for the hardware 
filters, but the software filter will likely get it wrong in those corner 
cases since it knows nothing of NEC).

There's multiple ways the NEC scancode formats could be improved 
(incompatibly!) to reduce the problems, but none are perfect.

E.g. one possibility is to scrap the NEC and extended NEC scancodes and just 
use 32-bit NEC scancodes format throughout:
0x[16-bit-address][16-bit-command]

which encodes scancodes for extended NEC like this:
0x[16-bit-address][~8-bit-command][8-bit-command]

and normal NEC like this:
0x[~8-bit-address][8-bit-address][~8-bit-command][8-bit-command]

Thanks
James

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

Re: [PATCH v2 0/9] rc: Add IR encode based wakeup filtering

[David Härdeman]

On Mon, Mar 17, 2014 at 10:34:25PM +0000, James Hogan wrote:
>It's ambiguous the other way too (which is probably a strong point against 
>having actual protocol bits for each NEC variant, since they only differ in 
>how the scancode is constructed). E.g. the Tivo keymap is 32-bit NEC, but has 
>extended NEC scancodes where the bytes of the command are complements (i.e. 
>the extended NEC command checksum passes). This makes it hard to filter on at 
>the scancode level (the drivers will probably get it right for the hardware 
>filters, but the software filter will likely get it wrong in those corner 
>cases since it knows nothing of NEC).
>
>There's multiple ways the NEC scancode formats could be improved 
>(incompatibly!) to reduce the problems, but none are perfect.
>
>E.g. one possibility is to scrap the NEC and extended NEC scancodes and just 
>use 32-bit NEC scancodes format throughout:

YES!

All the "knowledge" of "original" NEC (16 bit), "extended" NEC, etc that
have multiplied over both drivers and in various parts of rc-core is a
big mistake IMHO. The only sane way of handling NEC is to always treat
it as a 32 bit scancode (and only in cases where e.g. the hardware
returns or expects a 16 bit value should the scancode be converted
to/from the canonical 32 bit format). There is absolutely no advantages
in trying to parse or "understand" the NEC format unless it absolutely
cannot be avoided.

I haven't had the time to really review your patches in depth, but
whatever you do, please try to keep any knowledge of NEC 16/24/32 bit
distinction out of any functionality you add.

I had a suggested patch before which would also make the keymap handling
32-bit centric...essentially by redefining the set/get keymap ioctls a
bit (with backwards compatibility that guesses if the scancode is
16/24/32 bit based). It's been on my todo list for a long time to dust it
off...(yeah...I know)...
 
>0x[16-bit-address][16-bit-command]
>
>which encodes scancodes for extended NEC like this:
>0x[16-bit-address][~8-bit-command][8-bit-command]
>
>and normal NEC like this:
>0x[~8-bit-address][8-bit-address][~8-bit-command][8-bit-command]
>
>Thanks
>James

-- 
David Härdeman

Re: [PATCH v2 0/9] rc: Add IR encode based wakeup filtering

[Mauro Carvalho Chehab]

Em Fri, 14 Mar 2014 23:04:10 +0000
James Hogan <james@albanarts.com> escreveu:

> 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.
> 
> I posted an RFC patchset showing how this could work, and Antti Seppälä
> posted additional patches to support rc5-sz and nuvoton-cir. This
> patchset improves the original RFC patches and combines & updates
> Antti's patches.
> 
> I'm happy these patches are a good start at tackling the problem, as
> long as Antti is happy with them and they work for him of course.
> 
> Future work could include:
>  - Encoders for more protocols.
>  - Carrier signal events (no use unless a driver makes use of it).
> 
> Patch 1 adds the new encode API.
> Patches 2-3 adds some modulation helpers.
> Patches 4-6 adds some raw encode implementations.
> Patch 7 adds some rc-core support for encode based wakeup filtering.
> Patch 8 adds debug loopback of encoded scancode when filter set.
> Patch 9 (untested) adds encode based wakeup filtering to nuvoton-cir.
> 
> Changes in v2:

Any news about this patch series? There are some comments about them,
so I'll be tagging it as "changes requested" at patchwork, waiting
for a v3 (or is it already there in the middle of the 49 patches from
David?).

Regards,
Mauro

Re: [PATCH v2 0/9] rc: Add IR encode based wakeup filtering

[James Hogan]

Hi Mauro,

On Wednesday 23 July 2014 16:39:36 Mauro Carvalho Chehab wrote:
> Em Fri, 14 Mar 2014 23:04:10 +0000
> 
> James Hogan <james@albanarts.com> escreveu:
> > 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.
> > 
> > I posted an RFC patchset showing how this could work, and Antti Seppälä
> > posted additional patches to support rc5-sz and nuvoton-cir. This
> > patchset improves the original RFC patches and combines & updates
> > Antti's patches.
> > 
> > I'm happy these patches are a good start at tackling the problem, as
> > long as Antti is happy with them and they work for him of course.
> > 
> > Future work could include:
> >  - Encoders for more protocols.
> >  - Carrier signal events (no use unless a driver makes use of it).
> > 
> > Patch 1 adds the new encode API.
> > Patches 2-3 adds some modulation helpers.
> > Patches 4-6 adds some raw encode implementations.
> > Patch 7 adds some rc-core support for encode based wakeup filtering.
> > Patch 8 adds debug loopback of encoded scancode when filter set.
> > Patch 9 (untested) adds encode based wakeup filtering to nuvoton-cir.
> 
> > Changes in v2:
> Any news about this patch series? There are some comments about them,
> so I'll be tagging it as "changes requested" at patchwork, waiting
> for a v3 (or is it already there in the middle of the 49 patches from
> David?).

This patch series seems to have been forgotten. I do have a few changes on top 
of v2 to address the review comments, so as you say I should probably rebase 
and do a v3 at some point.

Cheers
James