All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] rc: img-ir: rc5 and rc6 support added
@ 2014-12-04 15:38 Sifan Naeem
  2014-12-04 15:38 ` [PATCH 1/5] rc: img-ir: add scancode requests to a struct Sifan Naeem
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Sifan Naeem @ 2014-12-04 15:38 UTC (permalink / raw)
  To: james.hogan, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia, Sifan Naeem

This patch sets adds support for rc5 and rc6 decoder modules along with
workarounds for quirks in the hw which surfaces when decoding in 
biphase mode required by rc5 and rc6.

This patch set was based on head of linux-next commit:

commit 1ca7c606de868d172afb4eb65e04e290dbdb51ff
Author: Stephen Rothwell <sfr@canb.auug.org.au>
Date:   Thu Dec 4 19:49:10 2014 +1100


Sifan Naeem (5):
  rc: img-ir: add scancode requests to a struct
  rc: img-ir: pass toggle bit to the rc driver
  rc: img-ir: biphase enabled with workaround
  rc: img-ir: add philips rc5 decoder module
  rc: img-ir: add philips rc6 decoder module

 drivers/media/rc/img-ir/Kconfig        |   15 ++++
 drivers/media/rc/img-ir/Makefile       |    2 +
 drivers/media/rc/img-ir/img-ir-hw.c    |   80 +++++++++++++++++++---
 drivers/media/rc/img-ir/img-ir-hw.h    |   22 +++++-
 drivers/media/rc/img-ir/img-ir-jvc.c   |    8 +--
 drivers/media/rc/img-ir/img-ir-nec.c   |   24 +++----
 drivers/media/rc/img-ir/img-ir-rc5.c   |   88 ++++++++++++++++++++++++
 drivers/media/rc/img-ir/img-ir-rc6.c   |  117 ++++++++++++++++++++++++++++++++
 drivers/media/rc/img-ir/img-ir-sanyo.c |    8 +--
 drivers/media/rc/img-ir/img-ir-sharp.c |    8 +--
 drivers/media/rc/img-ir/img-ir-sony.c  |   12 ++--
 11 files changed, 342 insertions(+), 42 deletions(-)
 create mode 100644 drivers/media/rc/img-ir/img-ir-rc5.c
 create mode 100644 drivers/media/rc/img-ir/img-ir-rc6.c

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/5] rc: img-ir: add scancode requests to a struct
  2014-12-04 15:38 [PATCH 0/5] rc: img-ir: rc5 and rc6 support added Sifan Naeem
@ 2014-12-04 15:38 ` Sifan Naeem
  2014-12-08 16:47   ` James Hogan
  2014-12-04 15:38 ` [PATCH 2/5] rc: img-ir: pass toggle bit to the rc driver Sifan Naeem
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Sifan Naeem @ 2014-12-04 15:38 UTC (permalink / raw)
  To: james.hogan, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia, Sifan Naeem

The information being requested of hardware decode callbacks through
the img-ir-hw scancode API is mounting up, so combine it into a struct
which can be passed in with a single pointer rather than multiple
pointer arguments. This allows it to be extended more easily without
touching all the hardware decode callbacks.

Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
---
 drivers/media/rc/img-ir/img-ir-hw.c    |   16 +++++++++-------
 drivers/media/rc/img-ir/img-ir-hw.h    |   16 ++++++++++++++--
 drivers/media/rc/img-ir/img-ir-jvc.c   |    8 ++++----
 drivers/media/rc/img-ir/img-ir-nec.c   |   24 ++++++++++++------------
 drivers/media/rc/img-ir/img-ir-sanyo.c |    8 ++++----
 drivers/media/rc/img-ir/img-ir-sharp.c |    8 ++++----
 drivers/media/rc/img-ir/img-ir-sony.c  |   12 ++++++------
 7 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index ec49f94..61850a6 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -789,20 +789,22 @@ static void img_ir_handle_data(struct img_ir_priv *priv, u32 len, u64 raw)
 	struct img_ir_priv_hw *hw = &priv->hw;
 	const struct img_ir_decoder *dec = hw->decoder;
 	int ret = IMG_IR_SCANCODE;
-	u32 scancode;
-	enum rc_type protocol = RC_TYPE_UNKNOWN;
+	struct img_ir_scancode_req request;
+
+	request.protocol = RC_TYPE_UNKNOWN;
 
 	if (dec->scancode)
-		ret = dec->scancode(len, raw, &protocol, &scancode, hw->enabled_protocols);
+		ret = dec->scancode(len, raw, hw->enabled_protocols, &request);
 	else if (len >= 32)
-		scancode = (u32)raw;
+		request.scancode = (u32)raw;
 	else if (len < 32)
-		scancode = (u32)raw & ((1 << len)-1);
+		request.scancode = (u32)raw & ((1 << len)-1);
 	dev_dbg(priv->dev, "data (%u bits) = %#llx\n",
 		len, (unsigned long long)raw);
 	if (ret == IMG_IR_SCANCODE) {
-		dev_dbg(priv->dev, "decoded scan code %#x\n", scancode);
-		rc_keydown(hw->rdev, protocol, scancode, 0);
+		dev_dbg(priv->dev, "decoded scan code %#x\n",
+			request.scancode);
+		rc_keydown(hw->rdev, request.protocol, request.scancode, 0);
 		img_ir_end_repeat(priv);
 	} else if (ret == IMG_IR_REPEATCODE) {
 		if (hw->mode == IMG_IR_M_REPEATING) {
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
index 8fcc16c..1fc9583 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -133,6 +133,18 @@ struct img_ir_timing_regvals {
 #define IMG_IR_REPEATCODE	1	/* repeat the previous code */
 
 /**
+ * struct img_ir_scancode_req - Scancode request data.
+ * @protocol:	Protocol code of received message (defaults to
+ *		RC_TYPE_UNKNOWN).
+ * @scancode:	Scan code of received message (must be written by
+ *		handler if IMG_IR_SCANCODE is returned).
+ */
+struct img_ir_scancode_req {
+	enum rc_type protocol;
+	u32 scancode;
+};
+
+/**
  * struct img_ir_decoder - Decoder settings for an IR protocol.
  * @type:	Protocol types bitmap.
  * @tolerance:	Timing tolerance as a percentage (default 10%).
@@ -162,8 +174,8 @@ struct img_ir_decoder {
 	struct img_ir_control		control;
 
 	/* scancode logic */
-	int (*scancode)(int len, u64 raw, enum rc_type *protocol,
-			u32 *scancode, u64 enabled_protocols);
+	int (*scancode)(int len, u64 raw, u64 enabled_protocols,
+			struct img_ir_scancode_req *request);
 	int (*filter)(const struct rc_scancode_filter *in,
 		      struct img_ir_filter *out, u64 protocols);
 };
diff --git a/drivers/media/rc/img-ir/img-ir-jvc.c b/drivers/media/rc/img-ir/img-ir-jvc.c
index a60dda8..d3e2fc0 100644
--- a/drivers/media/rc/img-ir/img-ir-jvc.c
+++ b/drivers/media/rc/img-ir/img-ir-jvc.c
@@ -12,8 +12,8 @@
 #include "img-ir-hw.h"
 
 /* Convert JVC data to a scancode */
-static int img_ir_jvc_scancode(int len, u64 raw, enum rc_type *protocol,
-			       u32 *scancode, u64 enabled_protocols)
+static int img_ir_jvc_scancode(int len, u64 raw, u64 enabled_protocols,
+			       struct img_ir_scancode_req *request)
 {
 	unsigned int cust, data;
 
@@ -23,8 +23,8 @@ static int img_ir_jvc_scancode(int len, u64 raw, enum rc_type *protocol,
 	cust = (raw >> 0) & 0xff;
 	data = (raw >> 8) & 0xff;
 
-	*protocol = RC_TYPE_JVC;
-	*scancode = cust << 8 | data;
+	request->protocol = RC_TYPE_JVC;
+	request->scancode = cust << 8 | data;
 	return IMG_IR_SCANCODE;
 }
 
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c
index 7398975..27a7ea8 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -13,8 +13,8 @@
 #include <linux/bitrev.h>
 
 /* Convert NEC data to a scancode */
-static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
-			       u32 *scancode, u64 enabled_protocols)
+static int img_ir_nec_scancode(int len, u64 raw, u64 enabled_protocols,
+			       struct img_ir_scancode_req *request)
 {
 	unsigned int addr, addr_inv, data, data_inv;
 	/* a repeat code has no data */
@@ -30,23 +30,23 @@ static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
 	if ((data_inv ^ data) != 0xff) {
 		/* 32-bit NEC (used by Apple and TiVo remotes) */
 		/* scan encoding: as transmitted, MSBit = first received bit */
-		*scancode = bitrev8(addr)     << 24 |
-			    bitrev8(addr_inv) << 16 |
-			    bitrev8(data)     <<  8 |
-			    bitrev8(data_inv);
+		request->scancode = bitrev8(addr)     << 24 |
+				bitrev8(addr_inv) << 16 |
+				bitrev8(data)     <<  8 |
+				bitrev8(data_inv);
 	} else if ((addr_inv ^ addr) != 0xff) {
 		/* Extended NEC */
 		/* scan encoding: AAaaDD */
-		*scancode = addr     << 16 |
-			    addr_inv <<  8 |
-			    data;
+		request->scancode = addr     << 16 |
+				addr_inv <<  8 |
+				data;
 	} else {
 		/* Normal NEC */
 		/* scan encoding: AADD */
-		*scancode = addr << 8 |
-			    data;
+		request->scancode = addr << 8 |
+				data;
 	}
-	*protocol = RC_TYPE_NEC;
+	request->protocol = RC_TYPE_NEC;
 	return IMG_IR_SCANCODE;
 }
 
diff --git a/drivers/media/rc/img-ir/img-ir-sanyo.c b/drivers/media/rc/img-ir/img-ir-sanyo.c
index 6b0653e..f394994 100644
--- a/drivers/media/rc/img-ir/img-ir-sanyo.c
+++ b/drivers/media/rc/img-ir/img-ir-sanyo.c
@@ -23,8 +23,8 @@
 #include "img-ir-hw.h"
 
 /* Convert Sanyo data to a scancode */
-static int img_ir_sanyo_scancode(int len, u64 raw, enum rc_type *protocol,
-				 u32 *scancode, u64 enabled_protocols)
+static int img_ir_sanyo_scancode(int len, u64 raw, u64 enabled_protocols,
+				 struct img_ir_scancode_req *request)
 {
 	unsigned int addr, addr_inv, data, data_inv;
 	/* a repeat code has no data */
@@ -44,8 +44,8 @@ static int img_ir_sanyo_scancode(int len, u64 raw, enum rc_type *protocol,
 		return -EINVAL;
 
 	/* Normal Sanyo */
-	*protocol = RC_TYPE_SANYO;
-	*scancode = addr << 8 | data;
+	request->protocol = RC_TYPE_SANYO;
+	request->scancode = addr << 8 | data;
 	return IMG_IR_SCANCODE;
 }
 
diff --git a/drivers/media/rc/img-ir/img-ir-sharp.c b/drivers/media/rc/img-ir/img-ir-sharp.c
index 3300a38..fe5acc4 100644
--- a/drivers/media/rc/img-ir/img-ir-sharp.c
+++ b/drivers/media/rc/img-ir/img-ir-sharp.c
@@ -12,8 +12,8 @@
 #include "img-ir-hw.h"
 
 /* Convert Sharp data to a scancode */
-static int img_ir_sharp_scancode(int len, u64 raw, enum rc_type *protocol,
-				 u32 *scancode, u64 enabled_protocols)
+static int img_ir_sharp_scancode(int len, u64 raw, u64 enabled_protocols,
+				 struct img_ir_scancode_req *request)
 {
 	unsigned int addr, cmd, exp, chk;
 
@@ -32,8 +32,8 @@ static int img_ir_sharp_scancode(int len, u64 raw, enum rc_type *protocol,
 		/* probably the second half of the message */
 		return -EINVAL;
 
-	*protocol = RC_TYPE_SHARP;
-	*scancode = addr << 8 | cmd;
+	request->protocol = RC_TYPE_SHARP;
+	request->scancode = addr << 8 | cmd;
 	return IMG_IR_SCANCODE;
 }
 
diff --git a/drivers/media/rc/img-ir/img-ir-sony.c b/drivers/media/rc/img-ir/img-ir-sony.c
index 3a0f17b..7f7375f 100644
--- a/drivers/media/rc/img-ir/img-ir-sony.c
+++ b/drivers/media/rc/img-ir/img-ir-sony.c
@@ -12,8 +12,8 @@
 #include "img-ir-hw.h"
 
 /* Convert Sony data to a scancode */
-static int img_ir_sony_scancode(int len, u64 raw, enum rc_type *protocol,
-				u32 *scancode, u64 enabled_protocols)
+static int img_ir_sony_scancode(int len, u64 raw, u64 enabled_protocols,
+				struct img_ir_scancode_req *request)
 {
 	unsigned int dev, subdev, func;
 
@@ -25,7 +25,7 @@ static int img_ir_sony_scancode(int len, u64 raw, enum rc_type *protocol,
 		raw    >>= 7;
 		dev    = raw & 0x1f;	/* next 5 bits */
 		subdev = 0;
-		*protocol = RC_TYPE_SONY12;
+		request->protocol = RC_TYPE_SONY12;
 		break;
 	case 15:
 		if (!(enabled_protocols & RC_BIT_SONY15))
@@ -34,7 +34,7 @@ static int img_ir_sony_scancode(int len, u64 raw, enum rc_type *protocol,
 		raw    >>= 7;
 		dev    = raw & 0xff;	/* next 8 bits */
 		subdev = 0;
-		*protocol = RC_TYPE_SONY15;
+		request->protocol = RC_TYPE_SONY15;
 		break;
 	case 20:
 		if (!(enabled_protocols & RC_BIT_SONY20))
@@ -44,12 +44,12 @@ static int img_ir_sony_scancode(int len, u64 raw, enum rc_type *protocol,
 		dev    = raw & 0x1f;	/* next 5 bits */
 		raw    >>= 5;
 		subdev = raw & 0xff;	/* next 8 bits */
-		*protocol = RC_TYPE_SONY20;
+		request->protocol = RC_TYPE_SONY20;
 		break;
 	default:
 		return -EINVAL;
 	}
-	*scancode = dev << 16 | subdev << 8 | func;
+	request->scancode = dev << 16 | subdev << 8 | func;
 	return IMG_IR_SCANCODE;
 }
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] rc: img-ir: pass toggle bit to the rc driver
  2014-12-04 15:38 [PATCH 0/5] rc: img-ir: rc5 and rc6 support added Sifan Naeem
  2014-12-04 15:38 ` [PATCH 1/5] rc: img-ir: add scancode requests to a struct Sifan Naeem
@ 2014-12-04 15:38 ` Sifan Naeem
  2014-12-08 16:49   ` James Hogan
  2014-12-04 15:38 ` [PATCH 3/5] rc: img-ir: biphase enabled with workaround Sifan Naeem
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Sifan Naeem @ 2014-12-04 15:38 UTC (permalink / raw)
  To: james.hogan, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia, Sifan Naeem

Add toggle bit to struct img_ir_scancode_req so that protocols can
provide it to img_ir_handle_data(), and pass that toggle bit up to
rc_keydown instead of 0.

This is nedded for the upcoming rc-5 and rc-6 patches.

Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
---
 drivers/media/rc/img-ir/img-ir-hw.c |    8 +++++---
 drivers/media/rc/img-ir/img-ir-hw.h |    2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index 61850a6..4a1407b 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -792,6 +792,7 @@ static void img_ir_handle_data(struct img_ir_priv *priv, u32 len, u64 raw)
 	struct img_ir_scancode_req request;
 
 	request.protocol = RC_TYPE_UNKNOWN;
+	request.toggle   = 0;
 
 	if (dec->scancode)
 		ret = dec->scancode(len, raw, hw->enabled_protocols, &request);
@@ -802,9 +803,10 @@ static void img_ir_handle_data(struct img_ir_priv *priv, u32 len, u64 raw)
 	dev_dbg(priv->dev, "data (%u bits) = %#llx\n",
 		len, (unsigned long long)raw);
 	if (ret == IMG_IR_SCANCODE) {
-		dev_dbg(priv->dev, "decoded scan code %#x\n",
-			request.scancode);
-		rc_keydown(hw->rdev, request.protocol, request.scancode, 0);
+		dev_dbg(priv->dev, "decoded scan code %#x, toggle %u\n",
+			request.scancode, request.toggle);
+		rc_keydown(hw->rdev, request.protocol, request.scancode,
+			   request.toggle);
 		img_ir_end_repeat(priv);
 	} else if (ret == IMG_IR_REPEATCODE) {
 		if (hw->mode == IMG_IR_M_REPEATING) {
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
index 1fc9583..5e59e8e 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -138,10 +138,12 @@ struct img_ir_timing_regvals {
  *		RC_TYPE_UNKNOWN).
  * @scancode:	Scan code of received message (must be written by
  *		handler if IMG_IR_SCANCODE is returned).
+ * @toggle:	Toggle bit (defaults to 0).
  */
 struct img_ir_scancode_req {
 	enum rc_type protocol;
 	u32 scancode;
+	u8 toggle;
 };
 
 /**
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/5] rc: img-ir: biphase enabled with workaround
  2014-12-04 15:38 [PATCH 0/5] rc: img-ir: rc5 and rc6 support added Sifan Naeem
  2014-12-04 15:38 ` [PATCH 1/5] rc: img-ir: add scancode requests to a struct Sifan Naeem
  2014-12-04 15:38 ` [PATCH 2/5] rc: img-ir: pass toggle bit to the rc driver Sifan Naeem
@ 2014-12-04 15:38 ` Sifan Naeem
  2014-12-08 17:17   ` James Hogan
  2014-12-04 15:38 ` [PATCH 4/5] rc: img-ir: add philips rc5 decoder module Sifan Naeem
  2014-12-04 15:38 ` [PATCH 5/5] rc: img-ir: add philips rc6 " Sifan Naeem
  4 siblings, 1 reply; 14+ messages in thread
From: Sifan Naeem @ 2014-12-04 15:38 UTC (permalink / raw)
  To: james.hogan, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia, Sifan Naeem

Biphase decoding in the current img-ir has got a quirk, where multiple
Interrupts are generated when an incomplete IR code is received by the
decoder.

Patch adds a work around for the quirk and enables biphase decoding.

Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
---
 drivers/media/rc/img-ir/img-ir-hw.c |   56 +++++++++++++++++++++++++++++++++--
 drivers/media/rc/img-ir/img-ir-hw.h |    2 ++
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index 4a1407b..a977467 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -52,6 +52,11 @@ static struct img_ir_decoder *img_ir_decoders[] = {
 
 #define IMG_IR_QUIRK_CODE_BROKEN	0x1	/* Decode is broken */
 #define IMG_IR_QUIRK_CODE_LEN_INCR	0x2	/* Bit length needs increment */
+/*
+ * The decoder generates rapid interrupts without actually having
+ * received any new data after an incomplete IR code is decoded.
+ */
+#define IMG_IR_QUIRK_CODE_IRQ		0x4
 
 /* functions for preprocessing timings, ensuring max is set */
 
@@ -547,6 +552,7 @@ static void img_ir_set_decoder(struct img_ir_priv *priv,
 
 	/* stop the end timer and switch back to normal mode */
 	del_timer_sync(&hw->end_timer);
+	del_timer_sync(&hw->suspend_timer);
 	hw->mode = IMG_IR_M_NORMAL;
 
 	/* clear the wakeup scancode filter */
@@ -843,6 +849,26 @@ static void img_ir_end_timer(unsigned long arg)
 	spin_unlock_irq(&priv->lock);
 }
 
+/*
+ * Timer function to re-enable the current protocol after it had been
+ * cleared when invalid interrupts were generated due to a quirk in the
+ * img-ir decoder.
+ */
+static void img_ir_suspend_timer(unsigned long arg)
+{
+	struct img_ir_priv *priv = (struct img_ir_priv *)arg;
+
+	img_ir_write(priv, IMG_IR_IRQ_CLEAR,
+			IMG_IR_IRQ_ALL & ~IMG_IR_IRQ_EDGE);
+
+	/* Don't set IRQ if it has changed in a different context. */
+	if ((priv->hw.suspend_irqen & IMG_IR_IRQ_EDGE) ==
+				img_ir_read(priv, IMG_IR_IRQ_ENABLE))
+		img_ir_write(priv, IMG_IR_IRQ_ENABLE, priv->hw.suspend_irqen);
+	/* enable */
+	img_ir_write(priv, IMG_IR_CONTROL, priv->hw.reg_timings.ctrl);
+}
+
 #ifdef CONFIG_COMMON_CLK
 static void img_ir_change_frequency(struct img_ir_priv *priv,
 				    struct clk_notifier_data *change)
@@ -908,15 +934,37 @@ void img_ir_isr_hw(struct img_ir_priv *priv, u32 irq_status)
 	if (!hw->decoder)
 		return;
 
+	ct = hw->decoder->control.code_type;
+
 	ir_status = img_ir_read(priv, IMG_IR_STATUS);
-	if (!(ir_status & (IMG_IR_RXDVAL | IMG_IR_RXDVALD2)))
+	if (!(ir_status & (IMG_IR_RXDVAL | IMG_IR_RXDVALD2))) {
+		if (!(priv->hw.ct_quirks[ct] & IMG_IR_QUIRK_CODE_IRQ))
+			return;
+		/*
+		 * The below functionality is added as a work around to stop
+		 * multiple Interrupts generated when an incomplete IR code is
+		 * received by the decoder.
+		 * The decoder generates rapid interrupts without actually
+		 * having received any new data. After a single interrupt it's
+		 * expected to clear up, but instead multiple interrupts are
+		 * rapidly generated. only way to get out of this loop is to
+		 * reset the control register after a short delay.
+		 */
+		img_ir_write(priv, IMG_IR_CONTROL, 0);
+		hw->suspend_irqen = img_ir_read(priv, IMG_IR_IRQ_ENABLE);
+		img_ir_write(priv, IMG_IR_IRQ_ENABLE,
+			     hw->suspend_irqen & IMG_IR_IRQ_EDGE);
+
+		/* Timer activated to re-enable the protocol. */
+		mod_timer(&hw->suspend_timer,
+			  jiffies + msecs_to_jiffies(5));
 		return;
+	}
 	ir_status &= ~(IMG_IR_RXDVAL | IMG_IR_RXDVALD2);
 	img_ir_write(priv, IMG_IR_STATUS, ir_status);
 
 	len = (ir_status & IMG_IR_RXDLEN) >> IMG_IR_RXDLEN_SHIFT;
 	/* some versions report wrong length for certain code types */
-	ct = hw->decoder->control.code_type;
 	if (hw->ct_quirks[ct] & IMG_IR_QUIRK_CODE_LEN_INCR)
 		++len;
 
@@ -958,7 +1006,7 @@ static void img_ir_probe_hw_caps(struct img_ir_priv *priv)
 	hw->ct_quirks[IMG_IR_CODETYPE_PULSELEN]
 		|= IMG_IR_QUIRK_CODE_LEN_INCR;
 	hw->ct_quirks[IMG_IR_CODETYPE_BIPHASE]
-		|= IMG_IR_QUIRK_CODE_BROKEN;
+		|= IMG_IR_QUIRK_CODE_IRQ;
 	hw->ct_quirks[IMG_IR_CODETYPE_2BITPULSEPOS]
 		|= IMG_IR_QUIRK_CODE_BROKEN;
 }
@@ -977,6 +1025,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
 
 	/* Set up the end timer */
 	setup_timer(&hw->end_timer, img_ir_end_timer, (unsigned long)priv);
+	setup_timer(&hw->suspend_timer, img_ir_suspend_timer,
+				(unsigned long)priv);
 
 	/* Register a clock notifier */
 	if (!IS_ERR(priv->clk)) {
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
index 5e59e8e..8578aa7 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -221,6 +221,7 @@ enum img_ir_mode {
  * @rdev:		Remote control device
  * @clk_nb:		Notifier block for clock notify events.
  * @end_timer:		Timer until repeat timeout.
+ * @suspend_timer:	Timer to re-enable protocol.
  * @decoder:		Current decoder settings.
  * @enabled_protocols:	Currently enabled protocols.
  * @clk_hz:		Current core clock rate in Hz.
@@ -235,6 +236,7 @@ struct img_ir_priv_hw {
 	struct rc_dev			*rdev;
 	struct notifier_block		clk_nb;
 	struct timer_list		end_timer;
+	struct timer_list		suspend_timer;
 	const struct img_ir_decoder	*decoder;
 	u64				enabled_protocols;
 	unsigned long			clk_hz;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/5] rc: img-ir: add philips rc5 decoder module
  2014-12-04 15:38 [PATCH 0/5] rc: img-ir: rc5 and rc6 support added Sifan Naeem
                   ` (2 preceding siblings ...)
  2014-12-04 15:38 ` [PATCH 3/5] rc: img-ir: biphase enabled with workaround Sifan Naeem
@ 2014-12-04 15:38 ` Sifan Naeem
  2014-12-08 17:41   ` James Hogan
  2014-12-04 15:38 ` [PATCH 5/5] rc: img-ir: add philips rc6 " Sifan Naeem
  4 siblings, 1 reply; 14+ messages in thread
From: Sifan Naeem @ 2014-12-04 15:38 UTC (permalink / raw)
  To: james.hogan, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia, Sifan Naeem

Add img-ir module for decoding Philips rc5 protocol.

Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
---
 drivers/media/rc/img-ir/Kconfig      |    7 +++
 drivers/media/rc/img-ir/Makefile     |    1 +
 drivers/media/rc/img-ir/img-ir-hw.c  |    3 ++
 drivers/media/rc/img-ir/img-ir-hw.h  |    1 +
 drivers/media/rc/img-ir/img-ir-rc5.c |   88 ++++++++++++++++++++++++++++++++++
 5 files changed, 100 insertions(+)
 create mode 100644 drivers/media/rc/img-ir/img-ir-rc5.c

diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig
index 03ba9fc..b5b114f 100644
--- a/drivers/media/rc/img-ir/Kconfig
+++ b/drivers/media/rc/img-ir/Kconfig
@@ -59,3 +59,10 @@ config IR_IMG_SANYO
 	help
 	   Say Y here to enable support for the Sanyo protocol (used by Sanyo,
 	   Aiwa, Chinon remotes) in the ImgTec infrared decoder block.
+
+config IR_IMG_RC5
+	bool "Phillips RC5 protocol support"
+	depends on IR_IMG_HW
+	help
+	   Say Y here to enable support for the RC5 protocol in the ImgTec
+	   infrared decoder block.
diff --git a/drivers/media/rc/img-ir/Makefile b/drivers/media/rc/img-ir/Makefile
index 92a459d..898b1b8 100644
--- a/drivers/media/rc/img-ir/Makefile
+++ b/drivers/media/rc/img-ir/Makefile
@@ -6,6 +6,7 @@ img-ir-$(CONFIG_IR_IMG_JVC)	+= img-ir-jvc.o
 img-ir-$(CONFIG_IR_IMG_SONY)	+= img-ir-sony.o
 img-ir-$(CONFIG_IR_IMG_SHARP)	+= img-ir-sharp.o
 img-ir-$(CONFIG_IR_IMG_SANYO)	+= img-ir-sanyo.o
+img-ir-$(CONFIG_IR_IMG_RC5)	+= img-ir-rc5.o
 img-ir-objs			:= $(img-ir-y)
 
 obj-$(CONFIG_IR_IMG)		+= img-ir.o
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index a977467..322cdf8 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -42,6 +42,9 @@ static struct img_ir_decoder *img_ir_decoders[] = {
 #ifdef CONFIG_IR_IMG_SANYO
 	&img_ir_sanyo,
 #endif
+#ifdef CONFIG_IR_IMG_RC5
+	&img_ir_rc5,
+#endif
 	NULL
 };
 
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
index 8578aa7..f124ec5 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -187,6 +187,7 @@ extern struct img_ir_decoder img_ir_jvc;
 extern struct img_ir_decoder img_ir_sony;
 extern struct img_ir_decoder img_ir_sharp;
 extern struct img_ir_decoder img_ir_sanyo;
+extern struct img_ir_decoder img_ir_rc5;
 
 /**
  * struct img_ir_reg_timings - Reg values for decoder timings at clock rate.
diff --git a/drivers/media/rc/img-ir/img-ir-rc5.c b/drivers/media/rc/img-ir/img-ir-rc5.c
new file mode 100644
index 0000000..e1a0829
--- /dev/null
+++ b/drivers/media/rc/img-ir/img-ir-rc5.c
@@ -0,0 +1,88 @@
+/*
+ * ImgTec IR Decoder setup for Phillips RC-5 protocol.
+ *
+ * Copyright 2012-2014 Imagination Technologies Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include "img-ir-hw.h"
+
+/* Convert RC5 data to a scancode */
+static int img_ir_rc5_scancode(int len, u64 raw, u64 enabled_protocols,
+				struct img_ir_scancode_req *request)
+{
+	unsigned int addr, cmd, tgl, start;
+
+	/* Quirk in the decoder shifts everything by 2 to the left. */
+	raw   >>= 2;
+
+	start	=  (raw >> 13)	& 0x01;
+	tgl	=  (raw >> 11)	& 0x01;
+	addr	=  (raw >>  6)	& 0x1f;
+	cmd	=   raw		& 0x3f;
+	/*
+	 * 12th bit is used to extend the command in extended RC5 and has
+	 * no effect on standard RC5.
+	 */
+	cmd	+= ((raw >> 12) & 0x01) ? 0 : 0x40;
+
+	if (!start)
+		return -EINVAL;
+
+	request->protocol = RC_TYPE_RC5;
+	request->scancode = addr << 8 | cmd;
+	request->toggle   = tgl;
+	return IMG_IR_SCANCODE;
+}
+
+/* Convert RC5 scancode to RC5 data filter */
+static int img_ir_rc5_filter(const struct rc_scancode_filter *in,
+				 struct img_ir_filter *out, u64 protocols)
+{
+	/* Not supported by the hw. */
+	return -EINVAL;
+}
+
+/*
+ * RC-5 decoder
+ * see http://www.sbprojects.com/knowledge/ir/rc5.php
+ */
+struct img_ir_decoder img_ir_rc5 = {
+	.type      = RC_BIT_RC5,
+	.control   = {
+		.bitoriend2	= 1,
+		.code_type	= IMG_IR_CODETYPE_BIPHASE,
+		.decodend2	= 1,
+	},
+	/* main timings */
+	.tolerance	= 16,
+	.unit		= 888888, /* 1/36k*32=888.888microseconds */
+	.timings	= {
+		/* 10 symbol */
+		.s10 = {
+			.pulse	= { 1 },
+			.space	= { 1 },
+		},
+
+		/* 11 symbol */
+		.s11 = {
+			.pulse	= { 1 },
+			.space	= { 1 },
+		},
+
+		/* free time */
+		.ft  = {
+			.minlen = 14,
+			.maxlen = 14,
+			.ft_min = 5,
+		},
+	},
+
+	/* scancode logic */
+	.scancode	= img_ir_rc5_scancode,
+	.filter		= img_ir_rc5_filter,
+};
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/5] rc: img-ir: add philips rc6 decoder module
  2014-12-04 15:38 [PATCH 0/5] rc: img-ir: rc5 and rc6 support added Sifan Naeem
                   ` (3 preceding siblings ...)
  2014-12-04 15:38 ` [PATCH 4/5] rc: img-ir: add philips rc5 decoder module Sifan Naeem
@ 2014-12-04 15:38 ` Sifan Naeem
  2014-12-08 17:45   ` James Hogan
  4 siblings, 1 reply; 14+ messages in thread
From: Sifan Naeem @ 2014-12-04 15:38 UTC (permalink / raw)
  To: james.hogan, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia, Sifan Naeem

Add img-ir module for decoding Philips rc6 protocol.

Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
---
 drivers/media/rc/img-ir/Kconfig      |    8 +++
 drivers/media/rc/img-ir/Makefile     |    1 +
 drivers/media/rc/img-ir/img-ir-hw.c  |    3 +
 drivers/media/rc/img-ir/img-ir-hw.h  |    1 +
 drivers/media/rc/img-ir/img-ir-rc6.c |  117 ++++++++++++++++++++++++++++++++++
 5 files changed, 130 insertions(+)
 create mode 100644 drivers/media/rc/img-ir/img-ir-rc6.c

diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig
index b5b114f..4d3fca9 100644
--- a/drivers/media/rc/img-ir/Kconfig
+++ b/drivers/media/rc/img-ir/Kconfig
@@ -66,3 +66,11 @@ config IR_IMG_RC5
 	help
 	   Say Y here to enable support for the RC5 protocol in the ImgTec
 	   infrared decoder block.
+
+config IR_IMG_RC6
+	bool "Phillips RC6 protocol support"
+	depends on IR_IMG_HW
+	help
+	   Say Y here to enable support for the RC6 protocol in the ImgTec
+	   infrared decoder block.
+	   Note: This version only supports mode 0.
diff --git a/drivers/media/rc/img-ir/Makefile b/drivers/media/rc/img-ir/Makefile
index 898b1b8..8e6d458 100644
--- a/drivers/media/rc/img-ir/Makefile
+++ b/drivers/media/rc/img-ir/Makefile
@@ -7,6 +7,7 @@ img-ir-$(CONFIG_IR_IMG_SONY)	+= img-ir-sony.o
 img-ir-$(CONFIG_IR_IMG_SHARP)	+= img-ir-sharp.o
 img-ir-$(CONFIG_IR_IMG_SANYO)	+= img-ir-sanyo.o
 img-ir-$(CONFIG_IR_IMG_RC5)	+= img-ir-rc5.o
+img-ir-$(CONFIG_IR_IMG_RC6)	+= img-ir-rc6.o
 img-ir-objs			:= $(img-ir-y)
 
 obj-$(CONFIG_IR_IMG)		+= img-ir.o
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index 322cdf8..3b70dc2 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -45,6 +45,9 @@ static struct img_ir_decoder *img_ir_decoders[] = {
 #ifdef CONFIG_IR_IMG_RC5
 	&img_ir_rc5,
 #endif
+#ifdef CONFIG_IR_IMG_RC6
+	&img_ir_rc6,
+#endif
 	NULL
 };
 
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
index f124ec5..c7b6e1a 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -188,6 +188,7 @@ extern struct img_ir_decoder img_ir_sony;
 extern struct img_ir_decoder img_ir_sharp;
 extern struct img_ir_decoder img_ir_sanyo;
 extern struct img_ir_decoder img_ir_rc5;
+extern struct img_ir_decoder img_ir_rc6;
 
 /**
  * struct img_ir_reg_timings - Reg values for decoder timings at clock rate.
diff --git a/drivers/media/rc/img-ir/img-ir-rc6.c b/drivers/media/rc/img-ir/img-ir-rc6.c
new file mode 100644
index 0000000..bcd0822
--- /dev/null
+++ b/drivers/media/rc/img-ir/img-ir-rc6.c
@@ -0,0 +1,117 @@
+/*
+ * ImgTec IR Decoder setup for Phillips RC-6 protocol.
+ *
+ * Copyright 2012-2014 Imagination Technologies Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include "img-ir-hw.h"
+
+/* Convert RC6 data to a scancode */
+static int img_ir_rc6_scancode(int len, u64 raw, u64 enabled_protocols,
+				struct img_ir_scancode_req *request)
+{
+	unsigned int addr, cmd, mode, trl1, trl2;
+
+	/*
+	 * Due to a side effect of the decoder handling the double length
+	 * Trailer bit, the header information is a bit scrambled, and the
+	 * raw data is shifted incorrectly.
+	 * This workaround effectively recovers the header bits.
+	 *
+	 * The Header field should look like this:
+	 *
+	 * StartBit ModeBit2 ModeBit1 ModeBit0 TrailerBit
+	 *
+	 * But what we get is:
+	 *
+	 * ModeBit2 ModeBit1 ModeBit0 TrailerBit1 TrailerBit2
+	 *
+	 * The start bit is not important to recover the scancode.
+	 */
+
+	raw	>>= 27;
+
+	trl1	= (raw >>  17)	& 0x01;
+	trl2	= (raw >>  16)	& 0x01;
+
+	mode	= (raw >>  18)	& 0x07;
+	addr	= (raw >>   8)	& 0xff;
+	cmd	=  raw		& 0xff;
+
+	/*
+	 * Due to the above explained irregularity the trailer bits cannot
+	 * have the same value.
+	 */
+	if (trl1 == trl2)
+		return -EINVAL;
+
+	/* Only mode 0 supported for now */
+	if (mode)
+		return -EINVAL;
+
+	request->protocol = RC_TYPE_RC6_0;
+	request->scancode = addr << 8 | cmd;
+	request->toggle	  = trl2;
+	return IMG_IR_SCANCODE;
+}
+
+/* Convert RC6 scancode to RC6 data filter */
+static int img_ir_rc6_filter(const struct rc_scancode_filter *in,
+				 struct img_ir_filter *out, u64 protocols)
+{
+	/* Not supported by the hw. */
+	return -EINVAL;
+}
+
+/*
+ * RC-6 decoder
+ * see http://www.sbprojects.com/knowledge/ir/rc6.php
+ */
+struct img_ir_decoder img_ir_rc6 = {
+	.type		= RC_BIT_RC6_0,
+	.control	= {
+		.bitorien	= 1,
+		.code_type	= IMG_IR_CODETYPE_BIPHASE,
+		.decoden	= 1,
+		.decodinpol	= 1,
+	},
+	/* main timings */
+	.tolerance	= 20,
+	/*
+	 * Due to a quirk in the img-ir decoder, default header values do
+	 * not work, the values described below were extracted from
+	 * successful RTL test cases.
+	 */
+	.timings	= {
+		/* leader symbol */
+		.ldr = {
+			.pulse	= { 650 },
+			.space	= { 660 },
+		},
+		/* 0 symbol */
+		.s00 = {
+			.pulse	= { 370 },
+			.space	= { 370 },
+		},
+		/* 01 symbol */
+		.s01 = {
+			.pulse	= { 370 },
+			.space	= { 370 },
+		},
+		/* free time */
+		.ft  = {
+			.minlen = 21,
+			.maxlen = 21,
+			.ft_min = 2666,	/* 2.666 ms */
+		},
+	},
+
+	/* scancode logic */
+	.scancode	= img_ir_rc6_scancode,
+	.filter		= img_ir_rc6_filter,
+};
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] rc: img-ir: add scancode requests to a struct
  2014-12-04 15:38 ` [PATCH 1/5] rc: img-ir: add scancode requests to a struct Sifan Naeem
@ 2014-12-08 16:47   ` James Hogan
  0 siblings, 0 replies; 14+ messages in thread
From: James Hogan @ 2014-12-08 16:47 UTC (permalink / raw)
  To: Sifan Naeem, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia

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

On 04/12/14 15:38, Sifan Naeem wrote:
> The information being requested of hardware decode callbacks through
> the img-ir-hw scancode API is mounting up, so combine it into a struct
> which can be passed in with a single pointer rather than multiple
> pointer arguments. This allows it to be extended more easily without
> touching all the hardware decode callbacks.
> 
> Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>

Acked-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> ---
>  drivers/media/rc/img-ir/img-ir-hw.c    |   16 +++++++++-------
>  drivers/media/rc/img-ir/img-ir-hw.h    |   16 ++++++++++++++--
>  drivers/media/rc/img-ir/img-ir-jvc.c   |    8 ++++----
>  drivers/media/rc/img-ir/img-ir-nec.c   |   24 ++++++++++++------------
>  drivers/media/rc/img-ir/img-ir-sanyo.c |    8 ++++----
>  drivers/media/rc/img-ir/img-ir-sharp.c |    8 ++++----
>  drivers/media/rc/img-ir/img-ir-sony.c  |   12 ++++++------
>  7 files changed, 53 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
> index ec49f94..61850a6 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.c
> +++ b/drivers/media/rc/img-ir/img-ir-hw.c
> @@ -789,20 +789,22 @@ static void img_ir_handle_data(struct img_ir_priv *priv, u32 len, u64 raw)
>  	struct img_ir_priv_hw *hw = &priv->hw;
>  	const struct img_ir_decoder *dec = hw->decoder;
>  	int ret = IMG_IR_SCANCODE;
> -	u32 scancode;
> -	enum rc_type protocol = RC_TYPE_UNKNOWN;
> +	struct img_ir_scancode_req request;
> +
> +	request.protocol = RC_TYPE_UNKNOWN;
>  
>  	if (dec->scancode)
> -		ret = dec->scancode(len, raw, &protocol, &scancode, hw->enabled_protocols);
> +		ret = dec->scancode(len, raw, hw->enabled_protocols, &request);
>  	else if (len >= 32)
> -		scancode = (u32)raw;
> +		request.scancode = (u32)raw;
>  	else if (len < 32)
> -		scancode = (u32)raw & ((1 << len)-1);
> +		request.scancode = (u32)raw & ((1 << len)-1);
>  	dev_dbg(priv->dev, "data (%u bits) = %#llx\n",
>  		len, (unsigned long long)raw);
>  	if (ret == IMG_IR_SCANCODE) {
> -		dev_dbg(priv->dev, "decoded scan code %#x\n", scancode);
> -		rc_keydown(hw->rdev, protocol, scancode, 0);
> +		dev_dbg(priv->dev, "decoded scan code %#x\n",
> +			request.scancode);
> +		rc_keydown(hw->rdev, request.protocol, request.scancode, 0);
>  		img_ir_end_repeat(priv);
>  	} else if (ret == IMG_IR_REPEATCODE) {
>  		if (hw->mode == IMG_IR_M_REPEATING) {
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
> index 8fcc16c..1fc9583 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.h
> +++ b/drivers/media/rc/img-ir/img-ir-hw.h
> @@ -133,6 +133,18 @@ struct img_ir_timing_regvals {
>  #define IMG_IR_REPEATCODE	1	/* repeat the previous code */
>  
>  /**
> + * struct img_ir_scancode_req - Scancode request data.
> + * @protocol:	Protocol code of received message (defaults to
> + *		RC_TYPE_UNKNOWN).
> + * @scancode:	Scan code of received message (must be written by
> + *		handler if IMG_IR_SCANCODE is returned).
> + */
> +struct img_ir_scancode_req {
> +	enum rc_type protocol;
> +	u32 scancode;
> +};
> +
> +/**
>   * struct img_ir_decoder - Decoder settings for an IR protocol.
>   * @type:	Protocol types bitmap.
>   * @tolerance:	Timing tolerance as a percentage (default 10%).
> @@ -162,8 +174,8 @@ struct img_ir_decoder {
>  	struct img_ir_control		control;
>  
>  	/* scancode logic */
> -	int (*scancode)(int len, u64 raw, enum rc_type *protocol,
> -			u32 *scancode, u64 enabled_protocols);
> +	int (*scancode)(int len, u64 raw, u64 enabled_protocols,
> +			struct img_ir_scancode_req *request);
>  	int (*filter)(const struct rc_scancode_filter *in,
>  		      struct img_ir_filter *out, u64 protocols);
>  };
> diff --git a/drivers/media/rc/img-ir/img-ir-jvc.c b/drivers/media/rc/img-ir/img-ir-jvc.c
> index a60dda8..d3e2fc0 100644
> --- a/drivers/media/rc/img-ir/img-ir-jvc.c
> +++ b/drivers/media/rc/img-ir/img-ir-jvc.c
> @@ -12,8 +12,8 @@
>  #include "img-ir-hw.h"
>  
>  /* Convert JVC data to a scancode */
> -static int img_ir_jvc_scancode(int len, u64 raw, enum rc_type *protocol,
> -			       u32 *scancode, u64 enabled_protocols)
> +static int img_ir_jvc_scancode(int len, u64 raw, u64 enabled_protocols,
> +			       struct img_ir_scancode_req *request)
>  {
>  	unsigned int cust, data;
>  
> @@ -23,8 +23,8 @@ static int img_ir_jvc_scancode(int len, u64 raw, enum rc_type *protocol,
>  	cust = (raw >> 0) & 0xff;
>  	data = (raw >> 8) & 0xff;
>  
> -	*protocol = RC_TYPE_JVC;
> -	*scancode = cust << 8 | data;
> +	request->protocol = RC_TYPE_JVC;
> +	request->scancode = cust << 8 | data;
>  	return IMG_IR_SCANCODE;
>  }
>  
> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c
> index 7398975..27a7ea8 100644
> --- a/drivers/media/rc/img-ir/img-ir-nec.c
> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
> @@ -13,8 +13,8 @@
>  #include <linux/bitrev.h>
>  
>  /* Convert NEC data to a scancode */
> -static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
> -			       u32 *scancode, u64 enabled_protocols)
> +static int img_ir_nec_scancode(int len, u64 raw, u64 enabled_protocols,
> +			       struct img_ir_scancode_req *request)
>  {
>  	unsigned int addr, addr_inv, data, data_inv;
>  	/* a repeat code has no data */
> @@ -30,23 +30,23 @@ static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
>  	if ((data_inv ^ data) != 0xff) {
>  		/* 32-bit NEC (used by Apple and TiVo remotes) */
>  		/* scan encoding: as transmitted, MSBit = first received bit */
> -		*scancode = bitrev8(addr)     << 24 |
> -			    bitrev8(addr_inv) << 16 |
> -			    bitrev8(data)     <<  8 |
> -			    bitrev8(data_inv);
> +		request->scancode = bitrev8(addr)     << 24 |
> +				bitrev8(addr_inv) << 16 |
> +				bitrev8(data)     <<  8 |
> +				bitrev8(data_inv);
>  	} else if ((addr_inv ^ addr) != 0xff) {
>  		/* Extended NEC */
>  		/* scan encoding: AAaaDD */
> -		*scancode = addr     << 16 |
> -			    addr_inv <<  8 |
> -			    data;
> +		request->scancode = addr     << 16 |
> +				addr_inv <<  8 |
> +				data;
>  	} else {
>  		/* Normal NEC */
>  		/* scan encoding: AADD */
> -		*scancode = addr << 8 |
> -			    data;
> +		request->scancode = addr << 8 |
> +				data;
>  	}
> -	*protocol = RC_TYPE_NEC;
> +	request->protocol = RC_TYPE_NEC;
>  	return IMG_IR_SCANCODE;
>  }
>  
> diff --git a/drivers/media/rc/img-ir/img-ir-sanyo.c b/drivers/media/rc/img-ir/img-ir-sanyo.c
> index 6b0653e..f394994 100644
> --- a/drivers/media/rc/img-ir/img-ir-sanyo.c
> +++ b/drivers/media/rc/img-ir/img-ir-sanyo.c
> @@ -23,8 +23,8 @@
>  #include "img-ir-hw.h"
>  
>  /* Convert Sanyo data to a scancode */
> -static int img_ir_sanyo_scancode(int len, u64 raw, enum rc_type *protocol,
> -				 u32 *scancode, u64 enabled_protocols)
> +static int img_ir_sanyo_scancode(int len, u64 raw, u64 enabled_protocols,
> +				 struct img_ir_scancode_req *request)
>  {
>  	unsigned int addr, addr_inv, data, data_inv;
>  	/* a repeat code has no data */
> @@ -44,8 +44,8 @@ static int img_ir_sanyo_scancode(int len, u64 raw, enum rc_type *protocol,
>  		return -EINVAL;
>  
>  	/* Normal Sanyo */
> -	*protocol = RC_TYPE_SANYO;
> -	*scancode = addr << 8 | data;
> +	request->protocol = RC_TYPE_SANYO;
> +	request->scancode = addr << 8 | data;
>  	return IMG_IR_SCANCODE;
>  }
>  
> diff --git a/drivers/media/rc/img-ir/img-ir-sharp.c b/drivers/media/rc/img-ir/img-ir-sharp.c
> index 3300a38..fe5acc4 100644
> --- a/drivers/media/rc/img-ir/img-ir-sharp.c
> +++ b/drivers/media/rc/img-ir/img-ir-sharp.c
> @@ -12,8 +12,8 @@
>  #include "img-ir-hw.h"
>  
>  /* Convert Sharp data to a scancode */
> -static int img_ir_sharp_scancode(int len, u64 raw, enum rc_type *protocol,
> -				 u32 *scancode, u64 enabled_protocols)
> +static int img_ir_sharp_scancode(int len, u64 raw, u64 enabled_protocols,
> +				 struct img_ir_scancode_req *request)
>  {
>  	unsigned int addr, cmd, exp, chk;
>  
> @@ -32,8 +32,8 @@ static int img_ir_sharp_scancode(int len, u64 raw, enum rc_type *protocol,
>  		/* probably the second half of the message */
>  		return -EINVAL;
>  
> -	*protocol = RC_TYPE_SHARP;
> -	*scancode = addr << 8 | cmd;
> +	request->protocol = RC_TYPE_SHARP;
> +	request->scancode = addr << 8 | cmd;
>  	return IMG_IR_SCANCODE;
>  }
>  
> diff --git a/drivers/media/rc/img-ir/img-ir-sony.c b/drivers/media/rc/img-ir/img-ir-sony.c
> index 3a0f17b..7f7375f 100644
> --- a/drivers/media/rc/img-ir/img-ir-sony.c
> +++ b/drivers/media/rc/img-ir/img-ir-sony.c
> @@ -12,8 +12,8 @@
>  #include "img-ir-hw.h"
>  
>  /* Convert Sony data to a scancode */
> -static int img_ir_sony_scancode(int len, u64 raw, enum rc_type *protocol,
> -				u32 *scancode, u64 enabled_protocols)
> +static int img_ir_sony_scancode(int len, u64 raw, u64 enabled_protocols,
> +				struct img_ir_scancode_req *request)
>  {
>  	unsigned int dev, subdev, func;
>  
> @@ -25,7 +25,7 @@ static int img_ir_sony_scancode(int len, u64 raw, enum rc_type *protocol,
>  		raw    >>= 7;
>  		dev    = raw & 0x1f;	/* next 5 bits */
>  		subdev = 0;
> -		*protocol = RC_TYPE_SONY12;
> +		request->protocol = RC_TYPE_SONY12;
>  		break;
>  	case 15:
>  		if (!(enabled_protocols & RC_BIT_SONY15))
> @@ -34,7 +34,7 @@ static int img_ir_sony_scancode(int len, u64 raw, enum rc_type *protocol,
>  		raw    >>= 7;
>  		dev    = raw & 0xff;	/* next 8 bits */
>  		subdev = 0;
> -		*protocol = RC_TYPE_SONY15;
> +		request->protocol = RC_TYPE_SONY15;
>  		break;
>  	case 20:
>  		if (!(enabled_protocols & RC_BIT_SONY20))
> @@ -44,12 +44,12 @@ static int img_ir_sony_scancode(int len, u64 raw, enum rc_type *protocol,
>  		dev    = raw & 0x1f;	/* next 5 bits */
>  		raw    >>= 5;
>  		subdev = raw & 0xff;	/* next 8 bits */
> -		*protocol = RC_TYPE_SONY20;
> +		request->protocol = RC_TYPE_SONY20;
>  		break;
>  	default:
>  		return -EINVAL;
>  	}
> -	*scancode = dev << 16 | subdev << 8 | func;
> +	request->scancode = dev << 16 | subdev << 8 | func;
>  	return IMG_IR_SCANCODE;
>  }
>  
> 


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/5] rc: img-ir: pass toggle bit to the rc driver
  2014-12-04 15:38 ` [PATCH 2/5] rc: img-ir: pass toggle bit to the rc driver Sifan Naeem
@ 2014-12-08 16:49   ` James Hogan
  0 siblings, 0 replies; 14+ messages in thread
From: James Hogan @ 2014-12-08 16:49 UTC (permalink / raw)
  To: Sifan Naeem, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia

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

On 04/12/14 15:38, Sifan Naeem wrote:
> Add toggle bit to struct img_ir_scancode_req so that protocols can
> provide it to img_ir_handle_data(), and pass that toggle bit up to
> rc_keydown instead of 0.
> 
> This is nedded for the upcoming rc-5 and rc-6 patches.

Typo (nedded).

Otherwise:
Acked-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> 
> Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
> ---
>  drivers/media/rc/img-ir/img-ir-hw.c |    8 +++++---
>  drivers/media/rc/img-ir/img-ir-hw.h |    2 ++
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
> index 61850a6..4a1407b 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.c
> +++ b/drivers/media/rc/img-ir/img-ir-hw.c
> @@ -792,6 +792,7 @@ static void img_ir_handle_data(struct img_ir_priv *priv, u32 len, u64 raw)
>  	struct img_ir_scancode_req request;
>  
>  	request.protocol = RC_TYPE_UNKNOWN;
> +	request.toggle   = 0;
>  
>  	if (dec->scancode)
>  		ret = dec->scancode(len, raw, hw->enabled_protocols, &request);
> @@ -802,9 +803,10 @@ static void img_ir_handle_data(struct img_ir_priv *priv, u32 len, u64 raw)
>  	dev_dbg(priv->dev, "data (%u bits) = %#llx\n",
>  		len, (unsigned long long)raw);
>  	if (ret == IMG_IR_SCANCODE) {
> -		dev_dbg(priv->dev, "decoded scan code %#x\n",
> -			request.scancode);
> -		rc_keydown(hw->rdev, request.protocol, request.scancode, 0);
> +		dev_dbg(priv->dev, "decoded scan code %#x, toggle %u\n",
> +			request.scancode, request.toggle);
> +		rc_keydown(hw->rdev, request.protocol, request.scancode,
> +			   request.toggle);
>  		img_ir_end_repeat(priv);
>  	} else if (ret == IMG_IR_REPEATCODE) {
>  		if (hw->mode == IMG_IR_M_REPEATING) {
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
> index 1fc9583..5e59e8e 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.h
> +++ b/drivers/media/rc/img-ir/img-ir-hw.h
> @@ -138,10 +138,12 @@ struct img_ir_timing_regvals {
>   *		RC_TYPE_UNKNOWN).
>   * @scancode:	Scan code of received message (must be written by
>   *		handler if IMG_IR_SCANCODE is returned).
> + * @toggle:	Toggle bit (defaults to 0).
>   */
>  struct img_ir_scancode_req {
>  	enum rc_type protocol;
>  	u32 scancode;
> +	u8 toggle;
>  };
>  
>  /**
> 


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] rc: img-ir: biphase enabled with workaround
  2014-12-04 15:38 ` [PATCH 3/5] rc: img-ir: biphase enabled with workaround Sifan Naeem
@ 2014-12-08 17:17   ` James Hogan
  2014-12-11 18:54     ` Sifan Naeem
  0 siblings, 1 reply; 14+ messages in thread
From: James Hogan @ 2014-12-08 17:17 UTC (permalink / raw)
  To: Sifan Naeem, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia

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

On 04/12/14 15:38, Sifan Naeem wrote:
> Biphase decoding in the current img-ir has got a quirk, where multiple
> Interrupts are generated when an incomplete IR code is received by the
> decoder.
> 
> Patch adds a work around for the quirk and enables biphase decoding.
> 
> Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
> ---
>  drivers/media/rc/img-ir/img-ir-hw.c |   56 +++++++++++++++++++++++++++++++++--
>  drivers/media/rc/img-ir/img-ir-hw.h |    2 ++
>  2 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
> index 4a1407b..a977467 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.c
> +++ b/drivers/media/rc/img-ir/img-ir-hw.c
> @@ -52,6 +52,11 @@ static struct img_ir_decoder *img_ir_decoders[] = {
>  
>  #define IMG_IR_QUIRK_CODE_BROKEN	0x1	/* Decode is broken */
>  #define IMG_IR_QUIRK_CODE_LEN_INCR	0x2	/* Bit length needs increment */
> +/*
> + * The decoder generates rapid interrupts without actually having
> + * received any new data after an incomplete IR code is decoded.
> + */
> +#define IMG_IR_QUIRK_CODE_IRQ		0x4
>  
>  /* functions for preprocessing timings, ensuring max is set */
>  
> @@ -547,6 +552,7 @@ static void img_ir_set_decoder(struct img_ir_priv *priv,
>  
>  	/* stop the end timer and switch back to normal mode */
>  	del_timer_sync(&hw->end_timer);
> +	del_timer_sync(&hw->suspend_timer);

FYI, this'll need rebasing due to conflicting with "img-ir/hw: Fix
potential deadlock stopping timer". The new del_timer_sync will need to
be when spin lock isn't held, i.e. still next to the other one, and
don't forget to ensure that suspend_timer doesn't get started if
hw->stopping.

>  	hw->mode = IMG_IR_M_NORMAL;
>  
>  	/* clear the wakeup scancode filter */
> @@ -843,6 +849,26 @@ static void img_ir_end_timer(unsigned long arg)
>  	spin_unlock_irq(&priv->lock);
>  }
>  
> +/*
> + * Timer function to re-enable the current protocol after it had been
> + * cleared when invalid interrupts were generated due to a quirk in the
> + * img-ir decoder.
> + */
> +static void img_ir_suspend_timer(unsigned long arg)
> +{
> +	struct img_ir_priv *priv = (struct img_ir_priv *)arg;
> +

You should take the spin lock for most of this function now that
"img-ir/hw: Fix potential deadlock stopping timer" is applied and it is
safe to do so.

> +	img_ir_write(priv, IMG_IR_IRQ_CLEAR,
> +			IMG_IR_IRQ_ALL & ~IMG_IR_IRQ_EDGE);
> +
> +	/* Don't set IRQ if it has changed in a different context. */

Wouldn't hurt to clarify this while you're at it (it confused me for a
moment thinking it was concerned about the enabled raw event IRQs
(IMG_IR_IRQ_EDGE) changing).

Maybe "Don't overwrite enabled valid/match IRQs if they have already
been changed by e.g. a filter change".

Should you even be clearing IRQs in that case? Maybe safer to just treat
that case as a "return immediately without touching anything" sort of
situation.

> +	if ((priv->hw.suspend_irqen & IMG_IR_IRQ_EDGE) ==
> +				img_ir_read(priv, IMG_IR_IRQ_ENABLE))
> +		img_ir_write(priv, IMG_IR_IRQ_ENABLE, priv->hw.suspend_irqen);
> +	/* enable */
> +	img_ir_write(priv, IMG_IR_CONTROL, priv->hw.reg_timings.ctrl);
> +}
> +
>  #ifdef CONFIG_COMMON_CLK
>  static void img_ir_change_frequency(struct img_ir_priv *priv,
>  				    struct clk_notifier_data *change)
> @@ -908,15 +934,37 @@ void img_ir_isr_hw(struct img_ir_priv *priv, u32 irq_status)
>  	if (!hw->decoder)
>  		return;
>  
> +	ct = hw->decoder->control.code_type;
> +
>  	ir_status = img_ir_read(priv, IMG_IR_STATUS);
> -	if (!(ir_status & (IMG_IR_RXDVAL | IMG_IR_RXDVALD2)))
> +	if (!(ir_status & (IMG_IR_RXDVAL | IMG_IR_RXDVALD2))) {
> +		if (!(priv->hw.ct_quirks[ct] & IMG_IR_QUIRK_CODE_IRQ))

(I suggest adding "|| hw->stopping" to this case)

> +			return;
> +		/*
> +		 * The below functionality is added as a work around to stop
> +		 * multiple Interrupts generated when an incomplete IR code is
> +		 * received by the decoder.
> +		 * The decoder generates rapid interrupts without actually
> +		 * having received any new data. After a single interrupt it's
> +		 * expected to clear up, but instead multiple interrupts are
> +		 * rapidly generated. only way to get out of this loop is to
> +		 * reset the control register after a short delay.
> +		 */
> +		img_ir_write(priv, IMG_IR_CONTROL, 0);
> +		hw->suspend_irqen = img_ir_read(priv, IMG_IR_IRQ_ENABLE);

You're reusing hw->suspend_irqen. What if you get this workaround being
activated between img_ir_enable_wake() and img_ir_disable_wake()? I
suggest just using a new img_ir_priv_hw member.

The rest looks reasonable to me, even if unfortunate that it is
necessary in the first place.

Thanks for the hard work!

Cheers
James

> +		img_ir_write(priv, IMG_IR_IRQ_ENABLE,
> +			     hw->suspend_irqen & IMG_IR_IRQ_EDGE);
> +
> +		/* Timer activated to re-enable the protocol. */
> +		mod_timer(&hw->suspend_timer,
> +			  jiffies + msecs_to_jiffies(5));
>  		return;
> +	}
>  	ir_status &= ~(IMG_IR_RXDVAL | IMG_IR_RXDVALD2);
>  	img_ir_write(priv, IMG_IR_STATUS, ir_status);
>  
>  	len = (ir_status & IMG_IR_RXDLEN) >> IMG_IR_RXDLEN_SHIFT;
>  	/* some versions report wrong length for certain code types */
> -	ct = hw->decoder->control.code_type;
>  	if (hw->ct_quirks[ct] & IMG_IR_QUIRK_CODE_LEN_INCR)
>  		++len;
>  
> @@ -958,7 +1006,7 @@ static void img_ir_probe_hw_caps(struct img_ir_priv *priv)
>  	hw->ct_quirks[IMG_IR_CODETYPE_PULSELEN]
>  		|= IMG_IR_QUIRK_CODE_LEN_INCR;
>  	hw->ct_quirks[IMG_IR_CODETYPE_BIPHASE]
> -		|= IMG_IR_QUIRK_CODE_BROKEN;
> +		|= IMG_IR_QUIRK_CODE_IRQ;
>  	hw->ct_quirks[IMG_IR_CODETYPE_2BITPULSEPOS]
>  		|= IMG_IR_QUIRK_CODE_BROKEN;
>  }
> @@ -977,6 +1025,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
>  
>  	/* Set up the end timer */
>  	setup_timer(&hw->end_timer, img_ir_end_timer, (unsigned long)priv);
> +	setup_timer(&hw->suspend_timer, img_ir_suspend_timer,
> +				(unsigned long)priv);
>  
>  	/* Register a clock notifier */
>  	if (!IS_ERR(priv->clk)) {
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
> index 5e59e8e..8578aa7 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.h
> +++ b/drivers/media/rc/img-ir/img-ir-hw.h
> @@ -221,6 +221,7 @@ enum img_ir_mode {
>   * @rdev:		Remote control device
>   * @clk_nb:		Notifier block for clock notify events.
>   * @end_timer:		Timer until repeat timeout.
> + * @suspend_timer:	Timer to re-enable protocol.
>   * @decoder:		Current decoder settings.
>   * @enabled_protocols:	Currently enabled protocols.
>   * @clk_hz:		Current core clock rate in Hz.
> @@ -235,6 +236,7 @@ struct img_ir_priv_hw {
>  	struct rc_dev			*rdev;
>  	struct notifier_block		clk_nb;
>  	struct timer_list		end_timer;
> +	struct timer_list		suspend_timer;
>  	const struct img_ir_decoder	*decoder;
>  	u64				enabled_protocols;
>  	unsigned long			clk_hz;
> 


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/5] rc: img-ir: add philips rc5 decoder module
  2014-12-04 15:38 ` [PATCH 4/5] rc: img-ir: add philips rc5 decoder module Sifan Naeem
@ 2014-12-08 17:41   ` James Hogan
  0 siblings, 0 replies; 14+ messages in thread
From: James Hogan @ 2014-12-08 17:41 UTC (permalink / raw)
  To: Sifan Naeem, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia

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

On 04/12/14 15:38, Sifan Naeem wrote:
> Add img-ir module for decoding Philips rc5 protocol.
> 
> Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
> ---
>  drivers/media/rc/img-ir/Kconfig      |    7 +++
>  drivers/media/rc/img-ir/Makefile     |    1 +
>  drivers/media/rc/img-ir/img-ir-hw.c  |    3 ++
>  drivers/media/rc/img-ir/img-ir-hw.h  |    1 +
>  drivers/media/rc/img-ir/img-ir-rc5.c |   88 ++++++++++++++++++++++++++++++++++
>  5 files changed, 100 insertions(+)
>  create mode 100644 drivers/media/rc/img-ir/img-ir-rc5.c
> 
> diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig
> index 03ba9fc..b5b114f 100644
> --- a/drivers/media/rc/img-ir/Kconfig
> +++ b/drivers/media/rc/img-ir/Kconfig
> @@ -59,3 +59,10 @@ config IR_IMG_SANYO
>  	help
>  	   Say Y here to enable support for the Sanyo protocol (used by Sanyo,
>  	   Aiwa, Chinon remotes) in the ImgTec infrared decoder block.
> +
> +config IR_IMG_RC5
> +	bool "Phillips RC5 protocol support"

I think that should be "Philips" (if wikipedia is anything to go by).

Same elsewhere in this patch and patch 5.

Other than that,
Acked-by: James Hogan <james.hogan@imgtec.com>

(Note, I don't have RC-5/RC-6 capable hardware yet so can't test this
support)

Thanks
James

> +	depends on IR_IMG_HW
> +	help
> +	   Say Y here to enable support for the RC5 protocol in the ImgTec
> +	   infrared decoder block.
> diff --git a/drivers/media/rc/img-ir/Makefile b/drivers/media/rc/img-ir/Makefile
> index 92a459d..898b1b8 100644
> --- a/drivers/media/rc/img-ir/Makefile
> +++ b/drivers/media/rc/img-ir/Makefile
> @@ -6,6 +6,7 @@ img-ir-$(CONFIG_IR_IMG_JVC)	+= img-ir-jvc.o
>  img-ir-$(CONFIG_IR_IMG_SONY)	+= img-ir-sony.o
>  img-ir-$(CONFIG_IR_IMG_SHARP)	+= img-ir-sharp.o
>  img-ir-$(CONFIG_IR_IMG_SANYO)	+= img-ir-sanyo.o
> +img-ir-$(CONFIG_IR_IMG_RC5)	+= img-ir-rc5.o
>  img-ir-objs			:= $(img-ir-y)
>  
>  obj-$(CONFIG_IR_IMG)		+= img-ir.o
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
> index a977467..322cdf8 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.c
> +++ b/drivers/media/rc/img-ir/img-ir-hw.c
> @@ -42,6 +42,9 @@ static struct img_ir_decoder *img_ir_decoders[] = {
>  #ifdef CONFIG_IR_IMG_SANYO
>  	&img_ir_sanyo,
>  #endif
> +#ifdef CONFIG_IR_IMG_RC5
> +	&img_ir_rc5,
> +#endif
>  	NULL
>  };
>  
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
> index 8578aa7..f124ec5 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.h
> +++ b/drivers/media/rc/img-ir/img-ir-hw.h
> @@ -187,6 +187,7 @@ extern struct img_ir_decoder img_ir_jvc;
>  extern struct img_ir_decoder img_ir_sony;
>  extern struct img_ir_decoder img_ir_sharp;
>  extern struct img_ir_decoder img_ir_sanyo;
> +extern struct img_ir_decoder img_ir_rc5;
>  
>  /**
>   * struct img_ir_reg_timings - Reg values for decoder timings at clock rate.
> diff --git a/drivers/media/rc/img-ir/img-ir-rc5.c b/drivers/media/rc/img-ir/img-ir-rc5.c
> new file mode 100644
> index 0000000..e1a0829
> --- /dev/null
> +++ b/drivers/media/rc/img-ir/img-ir-rc5.c
> @@ -0,0 +1,88 @@
> +/*
> + * ImgTec IR Decoder setup for Phillips RC-5 protocol.
> + *
> + * Copyright 2012-2014 Imagination Technologies Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include "img-ir-hw.h"
> +
> +/* Convert RC5 data to a scancode */
> +static int img_ir_rc5_scancode(int len, u64 raw, u64 enabled_protocols,
> +				struct img_ir_scancode_req *request)
> +{
> +	unsigned int addr, cmd, tgl, start;
> +
> +	/* Quirk in the decoder shifts everything by 2 to the left. */
> +	raw   >>= 2;
> +
> +	start	=  (raw >> 13)	& 0x01;
> +	tgl	=  (raw >> 11)	& 0x01;
> +	addr	=  (raw >>  6)	& 0x1f;
> +	cmd	=   raw		& 0x3f;
> +	/*
> +	 * 12th bit is used to extend the command in extended RC5 and has
> +	 * no effect on standard RC5.
> +	 */
> +	cmd	+= ((raw >> 12) & 0x01) ? 0 : 0x40;
> +
> +	if (!start)
> +		return -EINVAL;
> +
> +	request->protocol = RC_TYPE_RC5;
> +	request->scancode = addr << 8 | cmd;
> +	request->toggle   = tgl;
> +	return IMG_IR_SCANCODE;
> +}
> +
> +/* Convert RC5 scancode to RC5 data filter */
> +static int img_ir_rc5_filter(const struct rc_scancode_filter *in,
> +				 struct img_ir_filter *out, u64 protocols)
> +{
> +	/* Not supported by the hw. */
> +	return -EINVAL;
> +}
> +
> +/*
> + * RC-5 decoder
> + * see http://www.sbprojects.com/knowledge/ir/rc5.php
> + */
> +struct img_ir_decoder img_ir_rc5 = {
> +	.type      = RC_BIT_RC5,
> +	.control   = {
> +		.bitoriend2	= 1,
> +		.code_type	= IMG_IR_CODETYPE_BIPHASE,
> +		.decodend2	= 1,
> +	},
> +	/* main timings */
> +	.tolerance	= 16,
> +	.unit		= 888888, /* 1/36k*32=888.888microseconds */
> +	.timings	= {
> +		/* 10 symbol */
> +		.s10 = {
> +			.pulse	= { 1 },
> +			.space	= { 1 },
> +		},
> +
> +		/* 11 symbol */
> +		.s11 = {
> +			.pulse	= { 1 },
> +			.space	= { 1 },
> +		},
> +
> +		/* free time */
> +		.ft  = {
> +			.minlen = 14,
> +			.maxlen = 14,
> +			.ft_min = 5,
> +		},
> +	},
> +
> +	/* scancode logic */
> +	.scancode	= img_ir_rc5_scancode,
> +	.filter		= img_ir_rc5_filter,
> +};
> 


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] rc: img-ir: add philips rc6 decoder module
  2014-12-04 15:38 ` [PATCH 5/5] rc: img-ir: add philips rc6 " Sifan Naeem
@ 2014-12-08 17:45   ` James Hogan
  0 siblings, 0 replies; 14+ messages in thread
From: James Hogan @ 2014-12-08 17:45 UTC (permalink / raw)
  To: Sifan Naeem, mchehab
  Cc: linux-kernel, linux-media, james.hartley, ezequiel.garcia

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

On 04/12/14 15:38, Sifan Naeem wrote:
> Add img-ir module for decoding Philips rc6 protocol.
> 
> Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>

Aside from the "Philips" thing:

Acked-by: James Hogan <james.hogan@imgtec.com>

(It's unpleasant having unexplained timings for RC-6, but it's better
than no RC-6 support, and hopefully in the future it can be improved).

Cheers
James

> ---
>  drivers/media/rc/img-ir/Kconfig      |    8 +++
>  drivers/media/rc/img-ir/Makefile     |    1 +
>  drivers/media/rc/img-ir/img-ir-hw.c  |    3 +
>  drivers/media/rc/img-ir/img-ir-hw.h  |    1 +
>  drivers/media/rc/img-ir/img-ir-rc6.c |  117 ++++++++++++++++++++++++++++++++++
>  5 files changed, 130 insertions(+)
>  create mode 100644 drivers/media/rc/img-ir/img-ir-rc6.c
> 
> diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig
> index b5b114f..4d3fca9 100644
> --- a/drivers/media/rc/img-ir/Kconfig
> +++ b/drivers/media/rc/img-ir/Kconfig
> @@ -66,3 +66,11 @@ config IR_IMG_RC5
>  	help
>  	   Say Y here to enable support for the RC5 protocol in the ImgTec
>  	   infrared decoder block.
> +
> +config IR_IMG_RC6
> +	bool "Phillips RC6 protocol support"
> +	depends on IR_IMG_HW
> +	help
> +	   Say Y here to enable support for the RC6 protocol in the ImgTec
> +	   infrared decoder block.
> +	   Note: This version only supports mode 0.
> diff --git a/drivers/media/rc/img-ir/Makefile b/drivers/media/rc/img-ir/Makefile
> index 898b1b8..8e6d458 100644
> --- a/drivers/media/rc/img-ir/Makefile
> +++ b/drivers/media/rc/img-ir/Makefile
> @@ -7,6 +7,7 @@ img-ir-$(CONFIG_IR_IMG_SONY)	+= img-ir-sony.o
>  img-ir-$(CONFIG_IR_IMG_SHARP)	+= img-ir-sharp.o
>  img-ir-$(CONFIG_IR_IMG_SANYO)	+= img-ir-sanyo.o
>  img-ir-$(CONFIG_IR_IMG_RC5)	+= img-ir-rc5.o
> +img-ir-$(CONFIG_IR_IMG_RC6)	+= img-ir-rc6.o
>  img-ir-objs			:= $(img-ir-y)
>  
>  obj-$(CONFIG_IR_IMG)		+= img-ir.o
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
> index 322cdf8..3b70dc2 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.c
> +++ b/drivers/media/rc/img-ir/img-ir-hw.c
> @@ -45,6 +45,9 @@ static struct img_ir_decoder *img_ir_decoders[] = {
>  #ifdef CONFIG_IR_IMG_RC5
>  	&img_ir_rc5,
>  #endif
> +#ifdef CONFIG_IR_IMG_RC6
> +	&img_ir_rc6,
> +#endif
>  	NULL
>  };
>  
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
> index f124ec5..c7b6e1a 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.h
> +++ b/drivers/media/rc/img-ir/img-ir-hw.h
> @@ -188,6 +188,7 @@ extern struct img_ir_decoder img_ir_sony;
>  extern struct img_ir_decoder img_ir_sharp;
>  extern struct img_ir_decoder img_ir_sanyo;
>  extern struct img_ir_decoder img_ir_rc5;
> +extern struct img_ir_decoder img_ir_rc6;
>  
>  /**
>   * struct img_ir_reg_timings - Reg values for decoder timings at clock rate.
> diff --git a/drivers/media/rc/img-ir/img-ir-rc6.c b/drivers/media/rc/img-ir/img-ir-rc6.c
> new file mode 100644
> index 0000000..bcd0822
> --- /dev/null
> +++ b/drivers/media/rc/img-ir/img-ir-rc6.c
> @@ -0,0 +1,117 @@
> +/*
> + * ImgTec IR Decoder setup for Phillips RC-6 protocol.
> + *
> + * Copyright 2012-2014 Imagination Technologies Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include "img-ir-hw.h"
> +
> +/* Convert RC6 data to a scancode */
> +static int img_ir_rc6_scancode(int len, u64 raw, u64 enabled_protocols,
> +				struct img_ir_scancode_req *request)
> +{
> +	unsigned int addr, cmd, mode, trl1, trl2;
> +
> +	/*
> +	 * Due to a side effect of the decoder handling the double length
> +	 * Trailer bit, the header information is a bit scrambled, and the
> +	 * raw data is shifted incorrectly.
> +	 * This workaround effectively recovers the header bits.
> +	 *
> +	 * The Header field should look like this:
> +	 *
> +	 * StartBit ModeBit2 ModeBit1 ModeBit0 TrailerBit
> +	 *
> +	 * But what we get is:
> +	 *
> +	 * ModeBit2 ModeBit1 ModeBit0 TrailerBit1 TrailerBit2
> +	 *
> +	 * The start bit is not important to recover the scancode.
> +	 */
> +
> +	raw	>>= 27;
> +
> +	trl1	= (raw >>  17)	& 0x01;
> +	trl2	= (raw >>  16)	& 0x01;
> +
> +	mode	= (raw >>  18)	& 0x07;
> +	addr	= (raw >>   8)	& 0xff;
> +	cmd	=  raw		& 0xff;
> +
> +	/*
> +	 * Due to the above explained irregularity the trailer bits cannot
> +	 * have the same value.
> +	 */
> +	if (trl1 == trl2)
> +		return -EINVAL;
> +
> +	/* Only mode 0 supported for now */
> +	if (mode)
> +		return -EINVAL;
> +
> +	request->protocol = RC_TYPE_RC6_0;
> +	request->scancode = addr << 8 | cmd;
> +	request->toggle	  = trl2;
> +	return IMG_IR_SCANCODE;
> +}
> +
> +/* Convert RC6 scancode to RC6 data filter */
> +static int img_ir_rc6_filter(const struct rc_scancode_filter *in,
> +				 struct img_ir_filter *out, u64 protocols)
> +{
> +	/* Not supported by the hw. */
> +	return -EINVAL;
> +}
> +
> +/*
> + * RC-6 decoder
> + * see http://www.sbprojects.com/knowledge/ir/rc6.php
> + */
> +struct img_ir_decoder img_ir_rc6 = {
> +	.type		= RC_BIT_RC6_0,
> +	.control	= {
> +		.bitorien	= 1,
> +		.code_type	= IMG_IR_CODETYPE_BIPHASE,
> +		.decoden	= 1,
> +		.decodinpol	= 1,
> +	},
> +	/* main timings */
> +	.tolerance	= 20,
> +	/*
> +	 * Due to a quirk in the img-ir decoder, default header values do
> +	 * not work, the values described below were extracted from
> +	 * successful RTL test cases.
> +	 */
> +	.timings	= {
> +		/* leader symbol */
> +		.ldr = {
> +			.pulse	= { 650 },
> +			.space	= { 660 },
> +		},
> +		/* 0 symbol */
> +		.s00 = {
> +			.pulse	= { 370 },
> +			.space	= { 370 },
> +		},
> +		/* 01 symbol */
> +		.s01 = {
> +			.pulse	= { 370 },
> +			.space	= { 370 },
> +		},
> +		/* free time */
> +		.ft  = {
> +			.minlen = 21,
> +			.maxlen = 21,
> +			.ft_min = 2666,	/* 2.666 ms */
> +		},
> +	},
> +
> +	/* scancode logic */
> +	.scancode	= img_ir_rc6_scancode,
> +	.filter		= img_ir_rc6_filter,
> +};
> 


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 3/5] rc: img-ir: biphase enabled with workaround
  2014-12-08 17:17   ` James Hogan
@ 2014-12-11 18:54     ` Sifan Naeem
  2014-12-12 10:55       ` James Hogan
  0 siblings, 1 reply; 14+ messages in thread
From: Sifan Naeem @ 2014-12-11 18:54 UTC (permalink / raw)
  To: James Hogan, mchehab
  Cc: linux-kernel, linux-media, James Hartley, Ezequiel Garcia



> -----Original Message-----
> From: James Hogan
> Sent: 08 December 2014 17:18
> To: Sifan Naeem; mchehab@osg.samsung.com
> Cc: linux-kernel@vger.kernel.org; linux-media@vger.kernel.org; James
> Hartley; Ezequiel Garcia
> Subject: Re: [PATCH 3/5] rc: img-ir: biphase enabled with workaround
> 
> On 04/12/14 15:38, Sifan Naeem wrote:
> > Biphase decoding in the current img-ir has got a quirk, where multiple
> > Interrupts are generated when an incomplete IR code is received by the
> > decoder.
> >
> > Patch adds a work around for the quirk and enables biphase decoding.
> >
> > Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
> > ---
> >  drivers/media/rc/img-ir/img-ir-hw.c |   56
> +++++++++++++++++++++++++++++++++--
> >  drivers/media/rc/img-ir/img-ir-hw.h |    2 ++
> >  2 files changed, 55 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
> > b/drivers/media/rc/img-ir/img-ir-hw.c
> > index 4a1407b..a977467 100644
> > --- a/drivers/media/rc/img-ir/img-ir-hw.c
> > +++ b/drivers/media/rc/img-ir/img-ir-hw.c
> > @@ -52,6 +52,11 @@ static struct img_ir_decoder *img_ir_decoders[] = {
> >
> >  #define IMG_IR_QUIRK_CODE_BROKEN	0x1	/* Decode is broken
> */
> >  #define IMG_IR_QUIRK_CODE_LEN_INCR	0x2	/* Bit length needs
> increment */
> > +/*
> > + * The decoder generates rapid interrupts without actually having
> > + * received any new data after an incomplete IR code is decoded.
> > + */
> > +#define IMG_IR_QUIRK_CODE_IRQ		0x4
> >
> >  /* functions for preprocessing timings, ensuring max is set */
> >
> > @@ -547,6 +552,7 @@ static void img_ir_set_decoder(struct img_ir_priv
> > *priv,
> >
> >  	/* stop the end timer and switch back to normal mode */
> >  	del_timer_sync(&hw->end_timer);
> > +	del_timer_sync(&hw->suspend_timer);
> 
> FYI, this'll need rebasing due to conflicting with "img-ir/hw: Fix potential
> deadlock stopping timer". The new del_timer_sync will need to be when spin
> lock isn't held, i.e. still next to the other one, and don't forget to ensure that
> suspend_timer doesn't get started if
> hw->stopping.
> 
Yes, I'll rebase and resend the patch.

> >  	hw->mode = IMG_IR_M_NORMAL;
> >
> >  	/* clear the wakeup scancode filter */ @@ -843,6 +849,26 @@ static
> > void img_ir_end_timer(unsigned long arg)
> >  	spin_unlock_irq(&priv->lock);
> >  }
> >
> > +/*
> > + * Timer function to re-enable the current protocol after it had been
> > + * cleared when invalid interrupts were generated due to a quirk in
> > +the
> > + * img-ir decoder.
> > + */
> > +static void img_ir_suspend_timer(unsigned long arg) {
> > +	struct img_ir_priv *priv = (struct img_ir_priv *)arg;
> > +
> 
> You should take the spin lock for most of this function now that
> "img-ir/hw: Fix potential deadlock stopping timer" is applied and it is safe to
> do so.
> 
done
> > +	img_ir_write(priv, IMG_IR_IRQ_CLEAR,
> > +			IMG_IR_IRQ_ALL & ~IMG_IR_IRQ_EDGE);
> > +
> > +	/* Don't set IRQ if it has changed in a different context. */
> 
> Wouldn't hurt to clarify this while you're at it (it confused me for a moment
> thinking it was concerned about the enabled raw event IRQs
> (IMG_IR_IRQ_EDGE) changing).
> 
Ok
> Maybe "Don't overwrite enabled valid/match IRQs if they have already been
> changed by e.g. a filter change".
> 
> Should you even be clearing IRQs in that case? Maybe safer to just treat that
> case as a "return immediately without touching anything" sort of situation.
> 
don't have to clear it for this work around to work, so will remove.

> > +	if ((priv->hw.suspend_irqen & IMG_IR_IRQ_EDGE) ==
> > +				img_ir_read(priv, IMG_IR_IRQ_ENABLE))
> > +		img_ir_write(priv, IMG_IR_IRQ_ENABLE, priv-
> >hw.suspend_irqen);
> > +	/* enable */
> > +	img_ir_write(priv, IMG_IR_CONTROL, priv->hw.reg_timings.ctrl); }
> > +
> >  #ifdef CONFIG_COMMON_CLK
> >  static void img_ir_change_frequency(struct img_ir_priv *priv,
> >  				    struct clk_notifier_data *change) @@ -
> 908,15 +934,37 @@ void
> > img_ir_isr_hw(struct img_ir_priv *priv, u32 irq_status)
> >  	if (!hw->decoder)
> >  		return;
> >
> > +	ct = hw->decoder->control.code_type;
> > +
> >  	ir_status = img_ir_read(priv, IMG_IR_STATUS);
> > -	if (!(ir_status & (IMG_IR_RXDVAL | IMG_IR_RXDVALD2)))
> > +	if (!(ir_status & (IMG_IR_RXDVAL | IMG_IR_RXDVALD2))) {
> > +		if (!(priv->hw.ct_quirks[ct] & IMG_IR_QUIRK_CODE_IRQ))
> 
> (I suggest adding "|| hw->stopping" to this case)
> 
> > +			return;
> > +		/*
> > +		 * The below functionality is added as a work around to stop
> > +		 * multiple Interrupts generated when an incomplete IR code
> is
> > +		 * received by the decoder.
> > +		 * The decoder generates rapid interrupts without actually
> > +		 * having received any new data. After a single interrupt it's
> > +		 * expected to clear up, but instead multiple interrupts are
> > +		 * rapidly generated. only way to get out of this loop is to
> > +		 * reset the control register after a short delay.
> > +		 */
> > +		img_ir_write(priv, IMG_IR_CONTROL, 0);
> > +		hw->suspend_irqen = img_ir_read(priv,
> IMG_IR_IRQ_ENABLE);
> 
> You're reusing hw->suspend_irqen. What if you get this workaround being
> activated between img_ir_enable_wake() and img_ir_disable_wake()? I
> suggest just using a new img_ir_priv_hw member.
> 
Sure.

Thanks,
Sifan

> The rest looks reasonable to me, even if unfortunate that it is necessary in
> the first place.
> 
> Thanks for the hard work!
> 
> Cheers
> James
> 
> > +		img_ir_write(priv, IMG_IR_IRQ_ENABLE,
> > +			     hw->suspend_irqen & IMG_IR_IRQ_EDGE);
> > +
> > +		/* Timer activated to re-enable the protocol. */
> > +		mod_timer(&hw->suspend_timer,
> > +			  jiffies + msecs_to_jiffies(5));
> >  		return;
> > +	}
> >  	ir_status &= ~(IMG_IR_RXDVAL | IMG_IR_RXDVALD2);
> >  	img_ir_write(priv, IMG_IR_STATUS, ir_status);
> >
> >  	len = (ir_status & IMG_IR_RXDLEN) >> IMG_IR_RXDLEN_SHIFT;
> >  	/* some versions report wrong length for certain code types */
> > -	ct = hw->decoder->control.code_type;
> >  	if (hw->ct_quirks[ct] & IMG_IR_QUIRK_CODE_LEN_INCR)
> >  		++len;
> >
> > @@ -958,7 +1006,7 @@ static void img_ir_probe_hw_caps(struct
> img_ir_priv *priv)
> >  	hw->ct_quirks[IMG_IR_CODETYPE_PULSELEN]
> >  		|= IMG_IR_QUIRK_CODE_LEN_INCR;
> >  	hw->ct_quirks[IMG_IR_CODETYPE_BIPHASE]
> > -		|= IMG_IR_QUIRK_CODE_BROKEN;
> > +		|= IMG_IR_QUIRK_CODE_IRQ;
> >  	hw->ct_quirks[IMG_IR_CODETYPE_2BITPULSEPOS]
> >  		|= IMG_IR_QUIRK_CODE_BROKEN;
> >  }
> > @@ -977,6 +1025,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
> >
> >  	/* Set up the end timer */
> >  	setup_timer(&hw->end_timer, img_ir_end_timer, (unsigned
> long)priv);
> > +	setup_timer(&hw->suspend_timer, img_ir_suspend_timer,
> > +				(unsigned long)priv);
> >
> >  	/* Register a clock notifier */
> >  	if (!IS_ERR(priv->clk)) {
> > diff --git a/drivers/media/rc/img-ir/img-ir-hw.h
> > b/drivers/media/rc/img-ir/img-ir-hw.h
> > index 5e59e8e..8578aa7 100644
> > --- a/drivers/media/rc/img-ir/img-ir-hw.h
> > +++ b/drivers/media/rc/img-ir/img-ir-hw.h
> > @@ -221,6 +221,7 @@ enum img_ir_mode {
> >   * @rdev:		Remote control device
> >   * @clk_nb:		Notifier block for clock notify events.
> >   * @end_timer:		Timer until repeat timeout.
> > + * @suspend_timer:	Timer to re-enable protocol.
> >   * @decoder:		Current decoder settings.
> >   * @enabled_protocols:	Currently enabled protocols.
> >   * @clk_hz:		Current core clock rate in Hz.
> > @@ -235,6 +236,7 @@ struct img_ir_priv_hw {
> >  	struct rc_dev			*rdev;
> >  	struct notifier_block		clk_nb;
> >  	struct timer_list		end_timer;
> > +	struct timer_list		suspend_timer;
> >  	const struct img_ir_decoder	*decoder;
> >  	u64				enabled_protocols;
> >  	unsigned long			clk_hz;
> >


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] rc: img-ir: biphase enabled with workaround
  2014-12-11 18:54     ` Sifan Naeem
@ 2014-12-12 10:55       ` James Hogan
  2014-12-12 12:35         ` Sifan Naeem
  0 siblings, 1 reply; 14+ messages in thread
From: James Hogan @ 2014-12-12 10:55 UTC (permalink / raw)
  To: Sifan Naeem, mchehab
  Cc: linux-kernel, linux-media, James Hartley, Ezequiel Garcia

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

Hi Sifan,

On 11/12/14 18:54, Sifan Naeem wrote:
>>> +/*
>>> + * Timer function to re-enable the current protocol after it had been
>>> + * cleared when invalid interrupts were generated due to a quirk in
>>> +the
>>> + * img-ir decoder.
>>> + */
>>> +static void img_ir_suspend_timer(unsigned long arg) {
>>> +	struct img_ir_priv *priv = (struct img_ir_priv *)arg;
>>> +
>>> +	img_ir_write(priv, IMG_IR_IRQ_CLEAR,
>>> +			IMG_IR_IRQ_ALL & ~IMG_IR_IRQ_EDGE);
>>> +
>>> +	/* Don't set IRQ if it has changed in a different context. */
>>
>> Should you even be clearing IRQs in that case? Maybe safer to just treat that
>> case as a "return immediately without touching anything" sort of situation.
>>
> don't have to clear it for this work around to work, so will remove.
> 
>>> +	if ((priv->hw.suspend_irqen & IMG_IR_IRQ_EDGE) ==
>>> +				img_ir_read(priv, IMG_IR_IRQ_ENABLE))
>>> +		img_ir_write(priv, IMG_IR_IRQ_ENABLE, priv-
>>> hw.suspend_irqen);
>>> +	/* enable */
>>> +	img_ir_write(priv, IMG_IR_CONTROL, priv->hw.reg_timings.ctrl); }

To clarify, I was only referring to the case where the irq mask has
changed unexpectedly. If it hasn't changed then it would seem to make
sense to clear pending interrupts (i.e. the ones we've been
intentionally ignoring) before re-enabling them.

When you say it works without, do you mean there never are pending
interrupts (if you don't press any other buttons on the remote)?

Cheers
James


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 3/5] rc: img-ir: biphase enabled with workaround
  2014-12-12 10:55       ` James Hogan
@ 2014-12-12 12:35         ` Sifan Naeem
  0 siblings, 0 replies; 14+ messages in thread
From: Sifan Naeem @ 2014-12-12 12:35 UTC (permalink / raw)
  To: James Hogan, mchehab
  Cc: linux-kernel, linux-media, James Hartley, Ezequiel Garcia



> -----Original Message-----
> From: James Hogan
> Sent: 12 December 2014 10:56
> To: Sifan Naeem; mchehab@osg.samsung.com
> Cc: linux-kernel@vger.kernel.org; linux-media@vger.kernel.org; James
> Hartley; Ezequiel Garcia
> Subject: Re: [PATCH 3/5] rc: img-ir: biphase enabled with workaround
> 
> Hi Sifan,
> 
> On 11/12/14 18:54, Sifan Naeem wrote:
> >>> +/*
> >>> + * Timer function to re-enable the current protocol after it had
> >>> +been
> >>> + * cleared when invalid interrupts were generated due to a quirk in
> >>> +the
> >>> + * img-ir decoder.
> >>> + */
> >>> +static void img_ir_suspend_timer(unsigned long arg) {
> >>> +	struct img_ir_priv *priv = (struct img_ir_priv *)arg;
> >>> +
> >>> +	img_ir_write(priv, IMG_IR_IRQ_CLEAR,
> >>> +			IMG_IR_IRQ_ALL & ~IMG_IR_IRQ_EDGE);
> >>> +
> >>> +	/* Don't set IRQ if it has changed in a different context. */
> >>
> >> Should you even be clearing IRQs in that case? Maybe safer to just
> >> treat that case as a "return immediately without touching anything" sort
> of situation.
> >>
> > don't have to clear it for this work around to work, so will remove.
> >
> >>> +	if ((priv->hw.suspend_irqen & IMG_IR_IRQ_EDGE) ==
> >>> +				img_ir_read(priv, IMG_IR_IRQ_ENABLE))
> >>> +		img_ir_write(priv, IMG_IR_IRQ_ENABLE, priv-
> >>> hw.suspend_irqen);
> >>> +	/* enable */
> >>> +	img_ir_write(priv, IMG_IR_CONTROL, priv->hw.reg_timings.ctrl); }
> 
> To clarify, I was only referring to the case where the irq mask has changed
> unexpectedly. If it hasn't changed then it would seem to make sense to clear
> pending interrupts (i.e. the ones we've been intentionally ignoring) before
> re-enabling them.
> 
> When you say it works without, do you mean there never are pending
> interrupts (if you don't press any other buttons on the remote)?
> 
Nope, with the change I submitted in v2 (removed the clearing IRQ) there are no pending interrupts at the end.
But as before it goes through the workaround couple of times for each interrupt before settling down.

Thanks,
Sifan

> Cheers
> James


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-12-12 12:35 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-04 15:38 [PATCH 0/5] rc: img-ir: rc5 and rc6 support added Sifan Naeem
2014-12-04 15:38 ` [PATCH 1/5] rc: img-ir: add scancode requests to a struct Sifan Naeem
2014-12-08 16:47   ` James Hogan
2014-12-04 15:38 ` [PATCH 2/5] rc: img-ir: pass toggle bit to the rc driver Sifan Naeem
2014-12-08 16:49   ` James Hogan
2014-12-04 15:38 ` [PATCH 3/5] rc: img-ir: biphase enabled with workaround Sifan Naeem
2014-12-08 17:17   ` James Hogan
2014-12-11 18:54     ` Sifan Naeem
2014-12-12 10:55       ` James Hogan
2014-12-12 12:35         ` Sifan Naeem
2014-12-04 15:38 ` [PATCH 4/5] rc: img-ir: add philips rc5 decoder module Sifan Naeem
2014-12-08 17:41   ` James Hogan
2014-12-04 15:38 ` [PATCH 5/5] rc: img-ir: add philips rc6 " Sifan Naeem
2014-12-08 17:45   ` James Hogan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.