Re: [PATCH v2 3/5] rc: img-ir: biphase enabled with workaround
Hi Sifan,
On 11/12/14 20:06, 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.
Changes from v1:
* rebased due to conflict with img-ir/hw: Fix potential deadlock stopping
timer
* spinlock taken in img_ir_suspend_timer
* check for hw-stopping before handling quirks in img_ir_isr_hw
* new memeber added to img_ir_priv_hw to save irq status over suspend
For future reference, the list of changes between patchset versions is
usually put after a --- so that it doesn't get included in the final
git commit message. You can also add any Acked-by/Reviewed-by tags
you've been given to new versions of patchset, assuming nothing
significant has changed in that patch (maintainers generally add
relevant tags for you, that are sent in response to the patches being
applied).
Anyway, the whole patchset looks okay to me, aside from the one question
I just asked on patch 3 of v1, which I'm not so sure about. I'll let you
decide whether that needs changing since you have the hardware to verify it.
So for the whole patchset feel free to add my:
Acked-by: James Hogan james.ho...@imgtec.com
Thanks
James
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
---
drivers/media/rc/img-ir/img-ir-hw.c | 60
+--
drivers/media/rc/img-ir/img-ir-hw.h |4 +++
2 files changed, 61 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 9cecda7..5c32f05 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_IRQ0x4
/* functions for preprocessing timings, ensuring max is set */
@@ -542,6 +547,7 @@ static void img_ir_set_decoder(struct img_ir_priv *priv,
*/
spin_unlock_irq(priv-lock);
del_timer_sync(hw-end_timer);
+ del_timer_sync(hw-suspend_timer);
spin_lock_irq(priv-lock);
hw-stopping = false;
@@ -861,6 +867,29 @@ 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;
+
+ spin_lock_irq(priv-lock);
+ /*
+ * Don't overwrite enabled valid/match IRQs if they have already been
+ * changed by e.g. a filter change.
+ */
+ if ((priv-hw.quirk_suspend_irq IMG_IR_IRQ_EDGE) ==
+ img_ir_read(priv, IMG_IR_IRQ_ENABLE))
+ img_ir_write(priv, IMG_IR_IRQ_ENABLE,
+ priv-hw.quirk_suspend_irq);
+ /* enable */
+ img_ir_write(priv, IMG_IR_CONTROL, priv-hw.reg_timings.ctrl);
+ spin_unlock_irq(priv-lock);
+}
+
#ifdef CONFIG_COMMON_CLK
static void img_ir_change_frequency(struct img_ir_priv *priv,
struct clk_notifier_data *change)
@@ -926,15 +955,38 @@ 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) ||
+ hw-stopping)
+ 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-quirk_suspend_irq = img_ir_read(priv, IMG_IR_IRQ_ENABLE);
+ img_ir_write(priv, IMG_IR_IRQ_ENABLE,
+ hw-quirk_suspend_irq
Re: [PATCH v2 3/5] rc: img-ir: biphase enabled with workaround
On 12/12/14 12:07, James Hogan wrote: Hi Sifan, On 11/12/14 20:06, 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. Changes from v1: * rebased due to conflict with img-ir/hw: Fix potential deadlock stopping timer * spinlock taken in img_ir_suspend_timer * check for hw-stopping before handling quirks in img_ir_isr_hw * new memeber added to img_ir_priv_hw to save irq status over suspend For future reference, the list of changes between patchset versions is usually put after a --- so that it doesn't get included in the final git commit message. You can also add any Acked-by/Reviewed-by tags you've been given to new versions of patchset, assuming nothing significant has changed in that patch (maintainers generally add relevant tags for you, that are sent in response to the patches being applied). Anyway, the whole patchset looks okay to me, aside from the one question I just asked on patch 3 of v1, which I'm not so sure about. I'll let you decide whether that needs changing since you have the hardware to verify it. So for the whole patchset feel free to add my: Acked-by: James Hogan james.ho...@imgtec.com Mauro: Assuming no other changes are requested in this patchset, do you want these resent with the moving of changelogs out of the main commit messages? Cheers James signature.asc Description: OpenPGP digital signature
[PATCH v2 3/5] rc: img-ir: biphase enabled with workaround
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.
Changes from v1:
* rebased due to conflict with img-ir/hw: Fix potential deadlock stopping
timer
* spinlock taken in img_ir_suspend_timer
* check for hw-stopping before handling quirks in img_ir_isr_hw
* new memeber added to img_ir_priv_hw to save irq status over suspend
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
---
drivers/media/rc/img-ir/img-ir-hw.c | 60 +--
drivers/media/rc/img-ir/img-ir-hw.h |4 +++
2 files changed, 61 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 9cecda7..5c32f05 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 */
@@ -542,6 +547,7 @@ static void img_ir_set_decoder(struct img_ir_priv *priv,
*/
spin_unlock_irq(priv-lock);
del_timer_sync(hw-end_timer);
+ del_timer_sync(hw-suspend_timer);
spin_lock_irq(priv-lock);
hw-stopping = false;
@@ -861,6 +867,29 @@ 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;
+
+ spin_lock_irq(priv-lock);
+ /*
+* Don't overwrite enabled valid/match IRQs if they have already been
+* changed by e.g. a filter change.
+*/
+ if ((priv-hw.quirk_suspend_irq IMG_IR_IRQ_EDGE) ==
+ img_ir_read(priv, IMG_IR_IRQ_ENABLE))
+ img_ir_write(priv, IMG_IR_IRQ_ENABLE,
+ priv-hw.quirk_suspend_irq);
+ /* enable */
+ img_ir_write(priv, IMG_IR_CONTROL, priv-hw.reg_timings.ctrl);
+ spin_unlock_irq(priv-lock);
+}
+
#ifdef CONFIG_COMMON_CLK
static void img_ir_change_frequency(struct img_ir_priv *priv,
struct clk_notifier_data *change)
@@ -926,15 +955,38 @@ 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) ||
+ hw-stopping)
+ 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-quirk_suspend_irq = img_ir_read(priv, IMG_IR_IRQ_ENABLE);
+ img_ir_write(priv, IMG_IR_IRQ_ENABLE,
+hw-quirk_suspend_irq 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;
@@ -976,7 +1028,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]
- |=
