Re: [PATCH 3/5] rc: img-ir: biphase enabled with workaround

[James Hogan]

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: signature.asc
Description: OpenPGP digital signature