[3/3,media] winbond-cir: Adjust sample frequency to improve reliability

Commit Message

Anton Blanchard July 9, 2012, 2:02 a.m. UTC

Hi David,

> Just to make sure something like that isn't happening, could you
> correct the line in wbcir_irq_rx() which currently reads:
> 
> rawir.duration = US_TO_NS((irdata & 0x7F) * 10);
> 
> so that it reads
> 
> rawir.duration = US_TO_NS(((irdata & 0x7F) + 1) * 10);

Sure, I have added the change. This is what my diff to mainline looks
like right now:


Here is the debug output:

http://ozlabs.org/~anton/winbond.log1.gz

> Another possibility is that the printk in the interrupt handler causes
> overhead...could you do a debug run without the printk in the
> interrupt handler?

Here is the output without the printk in the interrupt handler:

http://ozlabs.org/~anton/winbond.log2.gz

Anton
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Härdeman Aug. 20, 2012, 9:40 p.m. UTC | #1

On Mon, Jul 09, 2012 at 12:02:08PM +1000, Anton Blanchard wrote:
>
>Hi David,
>
>> Just to make sure something like that isn't happening, could you
>> correct the line in wbcir_irq_rx() which currently reads:
>> 
>> rawir.duration = US_TO_NS((irdata & 0x7F) * 10);
>> 
>> so that it reads
>> 
>> rawir.duration = US_TO_NS(((irdata & 0x7F) + 1) * 10);
>
>Sure, I have added the change. This is what my diff to mainline looks
>like right now:

Hmm, still no good theory here.

Could you try the suggestions Konstantin Dimitrov proposed (i.e. the
Harmony fixups) and see if that makes any difference? Perhaps the
harmony actually does manage to send some bogus data (of sufficiently
short duration not to show up in the captured RX data)...

Patch

diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
index 342c2c8..6381c11 100644
--- a/drivers/media/rc/winbond-cir.c
+++ b/drivers/media/rc/winbond-cir.c
@@ -232,7 +232,7 @@  MODULE_PARM_DESC(invert, "Invert the signal from the IR receiver");
 
 static bool txandrx; /* default = 0 */
 module_param(txandrx, bool, 0444);
-MODULE_PARM_DESC(invert, "Allow simultaneous TX and RX");
+MODULE_PARM_DESC(txandrx, "Allow simultaneous TX and RX");
 
 static unsigned int wake_sc = 0x800F040C;
 module_param(wake_sc, uint, 0644);
@@ -358,7 +358,8 @@  wbcir_irq_rx(struct wbcir_data *data, struct pnp_dev *device)
 		if (data->rxstate == WBCIR_RXSTATE_ERROR)
 			continue;
 		rawir.pulse = irdata & 0x80 ? false : true;
-		rawir.duration = US_TO_NS((irdata & 0x7F) * 10);
+		rawir.duration = US_TO_NS(((irdata & 0x7F) + 1) * 10);
+		printk(KERN_DEBUG "%x %d %d\n", irdata, rawir.pulse, rawir.duration);
 		ir_raw_event_store_with_filter(data->dev, &rawir);
 	}
 
@@ -1026,6 +1027,7 @@  wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
 	data->dev->input_id.product = WBCIR_ID_FAMILY;
 	data->dev->input_id.version = WBCIR_ID_CHIP;
 	data->dev->map_name = RC_MAP_RC6_MCE;
+	data->dev->timeout = MS_TO_NS(100);
 	data->dev->s_idle = wbcir_idle_rx;
 	data->dev->s_tx_mask = wbcir_txmask;
 	data->dev->s_tx_carrier = wbcir_txcarrier;