Skip to contentSkip to conversation

V4L/DVB: ir-core: Add callbacks for input/evdev open/close on IR core

PATCH In review Mauro Carvalho Chehab 4 participants 27 messages Apr 1, 2010 – Apr 24, 2010 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore
#24 lore raw

PATCH 13/15 V4L/DVB: ir-core: Add callbacks for input/evdev open/close on IR core

In reply to a message not in the archive

Especially when IR needs to do polling, it generates lots of wakeups per second. This makes no sense, if the input event device is closed.

Adds a callback handler to the IR hardware driver, to allow registering an open/close ops.

drivers/media/IR/ir-keytable.c +17 -0
index f25193c..db54562 100644
@@ -445,6 +445,19 @@ void ir_keydown(struct input_dev *dev, int scancode)
 }
 EXPORT_SYMBOL_GPL(ir_keydown);
 
+static int ir_open(struct input_dev *input_dev)
+{
+	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+
+	return ir_dev->props->open(ir_dev->props->priv);
+}
+
+static void ir_close(struct input_dev *input_dev)
+{
+	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+
+	ir_dev->props->close(ir_dev->props->priv);
+}
 
 /**
  * ir_input_register() - sets the IR keycode table and add the handlers
@@ -494,6 +507,10 @@ int ir_input_register(struct input_dev *input_dev,
 
 	ir_copy_table(&ir_dev->rc_tab, rc_tab);
 	ir_dev->props = props;
+	if (props && props->open)
+		input_dev->open = ir_open;
+	if (props && props->close)
+		input_dev->close = ir_close;
 
 	/* set the bits for the keys */
 	IR_dprintk(1, "key map size: %d\n", rc_tab->size);
drivers/media/video/saa7134/saa7134-core.c +1 -1
index a7ad781..68cda10 100644
@@ -1227,7 +1227,7 @@ static int saa7134_resume(struct pci_dev *pci_dev)
 	if (card_has_mpeg(dev))
 		saa7134_ts_init_hw(dev);
 	if (dev->remote)
-		saa7134_ir_start(dev, dev->remote);
+		saa7134_ir_start(dev);
 	saa7134_hw_enable1(dev);
 
 	msleep(100);
drivers/media/video/saa7134/saa7134-input.c +75 -14

152 lines not on this page · raw message

drivers/media/video/saa7134/saa7134.h +2 -2
index c3a1ae0..cad8aee 100644
@@ -20,7 +20,7 @@
  */
 
 #include <linux/version.h>
-#define SAA7134_VERSION_CODE KERNEL_VERSION(0,2,15)
+#define SAA7134_VERSION_CODE KERNEL_VERSION(0, 2, 16)
 
 #include <linux/pci.h>
 #include <linux/i2c.h>
@@ -810,7 +810,7 @@ int  saa7134_input_init1(struct saa7134_dev *dev);
 void saa7134_input_fini(struct saa7134_dev *dev);
 void saa7134_input_irq(struct saa7134_dev *dev);
 void saa7134_probe_i2c_ir(struct saa7134_dev *dev);
-void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir);
+int saa7134_ir_start(struct saa7134_dev *dev);
 void saa7134_ir_stop(struct saa7134_dev *dev);
 
 
include/media/ir-common.h +1 -0
index 87f2ec7..e403a9a 100644
@@ -50,6 +50,7 @@ struct card_ir {
 	struct ir_input_state   ir;
 	char                    name[32];
 	char                    phys[32];
+	int			users;
 
 	u32			running:1;
 	struct ir_dev_props	props;
include/media/ir-core.h +2 -0
index c704fa7..9a2f308 100644
@@ -56,6 +56,8 @@ struct ir_dev_props {
 	unsigned long allowed_protos;
 	void 		*priv;
 	int (*change_protocol)(void *priv, u64 ir_type);
+	int (*open)(void *priv);
+	void (*close)(void *priv);
 };
 
 struct ir_raw_event {
Signature · 1 line
1.6.6.1
#25 lore raw

PATCH 14/15 V4L/DVB: cx88: Only start IR if the input device is opened

In reply to a message not in the archive
drivers/media/video/cx88/cx88-input.c +60 -9
index 8f1b846..a7214d0 100644
@@ -39,6 +39,10 @@ struct cx88_IR {
 	struct cx88_core *core;
 	struct input_dev *input;
 	struct ir_input_state ir;
+	struct ir_dev_props props;
+
+	int users;
+
 	char name[32];
 	char phys[32];
 
@@ -160,8 +164,16 @@ static enum hrtimer_restart cx88_ir_work(struct hrtimer *timer)
 	return HRTIMER_RESTART;
 }
 
-void cx88_ir_start(struct cx88_core *core, struct cx88_IR *ir)
+static int __cx88_ir_start(void *priv)
 {
+	struct cx88_core *core = priv;
+	struct cx88_IR *ir;
+
+	if (!core || !core->ir)
+		return -EINVAL;
+
+	ir = core->ir;
+
 	if (ir->polling) {
 		hrtimer_init(&ir->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 		ir->timer.function = cx88_ir_work;
@@ -174,10 +186,18 @@ void cx88_ir_start(struct cx88_core *core, struct cx88_IR *ir)
 		cx_write(MO_DDS_IO, 0xa80a80);	/* 4 kHz sample rate */
 		cx_write(MO_DDSCFG_IO, 0x5);	/* enable */
 	}
+	return 0;
 }
 
-void cx88_ir_stop(struct cx88_core *core, struct cx88_IR *ir)
+static void __cx88_ir_stop(void *priv)
 {
+	struct cx88_core *core = priv;
+	struct cx88_IR *ir;
+
+	if (!core || !core->ir)
+		return;
+
+	ir = core->ir;
 	if (ir->sampling) {
 		cx_write(MO_DDSCFG_IO, 0x0);
 		core->pci_irqmask &= ~PCI_INT_IR_SMPINT;
@@ -187,6 +207,37 @@ void cx88_ir_stop(struct cx88_core *core, struct cx88_IR *ir)
 		hrtimer_cancel(&ir->timer);
 }
 
+int cx88_ir_start(struct cx88_core *core)
+{
+	if (core->ir->users)
+		return __cx88_ir_start(core);
+
+	return 0;
+}
+
+void cx88_ir_stop(struct cx88_core *core)
+{
+	if (core->ir->users)
+		__cx88_ir_stop(core);
+}
+
+static int cx88_ir_open(void *priv)
+{
+	struct cx88_core *core = priv;
+
+	core->ir->users++;
+	return __cx88_ir_start(core);
+}
+
+static void cx88_ir_close(void *priv)
+{
+	struct cx88_core *core = priv;
+
+	core->ir->users--;
+	if (!core->ir->users)
+		__cx88_ir_stop(core);
+}
+
 /* ---------------------------------------------------------------------- */
 
 int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
@@ -382,19 +433,19 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
 	ir->core = core;
 	core->ir = ir;
 
-	cx88_ir_start(core, ir);
+	ir->props.priv = core;
+	ir->props.open = cx88_ir_open;
+	ir->props.close = cx88_ir_close;
 
 	/* all done */
-	err = ir_input_register(ir->input, ir_codes, NULL, MODULE_NAME);
+	err = ir_input_register(ir->input, ir_codes, &ir->props, MODULE_NAME);
 	if (err)
-		goto err_out_stop;
+		goto err_out_free;
 
 	return 0;
 
- err_out_stop:
-	cx88_ir_stop(core, ir);
-	core->ir = NULL;
  err_out_free:
+	core->ir = NULL;
 	kfree(ir);
 	return err;
 }
@@ -407,7 +458,7 @@ int cx88_ir_fini(struct cx88_core *core)
 	if (NULL == ir)
 		return 0;
 
-	cx88_ir_stop(core, ir);
+	cx88_ir_stop(core);
 	ir_input_unregister(ir->input);
 	kfree(ir);
 
drivers/media/video/cx88/cx88-video.c +3 -3
index 48c450f..8e414f7 100644
@@ -1977,7 +1977,7 @@ static void __devexit cx8800_finidev(struct pci_dev *pci_dev)
 	}
 
 	if (core->ir)
-		cx88_ir_stop(core, core->ir);
+		cx88_ir_stop(core);
 
 	cx88_shutdown(core); /* FIXME */
 	pci_disable_device(pci_dev);
@@ -2015,7 +2015,7 @@ static int cx8800_suspend(struct pci_dev *pci_dev, pm_message_t state)
 	spin_unlock(&dev->slock);
 
 	if (core->ir)
-		cx88_ir_stop(core, core->ir);
+		cx88_ir_stop(core);
 	/* FIXME -- shutdown device */
 	cx88_shutdown(core);
 
@@ -2056,7 +2056,7 @@ static int cx8800_resume(struct pci_dev *pci_dev)
 	/* FIXME: re-initialize hardware */
 	cx88_reset(core);
 	if (core->ir)
-		cx88_ir_start(core, core->ir);
+		cx88_ir_start(core);
 
 	cx_set(MO_PCI_INTMSK, core->pci_irqmask);
 
drivers/media/video/cx88/cx88.h +3 -3
index b5f054d..bdb03d3 100644
@@ -41,7 +41,7 @@
 
 #include <linux/version.h>
 #include <linux/mutex.h>
-#define CX88_VERSION_CODE KERNEL_VERSION(0,0,7)
+#define CX88_VERSION_CODE KERNEL_VERSION(0, 0, 8)
 
 #define UNSET (-1U)
 
@@ -683,8 +683,8 @@ s32 cx88_dsp_detect_stereo_sap(struct cx88_core *core);
 int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci);
 int cx88_ir_fini(struct cx88_core *core);
 void cx88_ir_irq(struct cx88_core *core);
-void cx88_ir_start(struct cx88_core *core, struct cx88_IR *ir);
-void cx88_ir_stop(struct cx88_core *core, struct cx88_IR *ir);
+int cx88_ir_start(struct cx88_core *core);
+void cx88_ir_stop(struct cx88_core *core);
 
 /* ----------------------------------------------------------- */
 /* cx88-mpeg.c                                                 */
Signature · 1 line
1.6.6.1
#2 lore raw

PATCH 2/15 V4L/DVB: saa7134: use a full scancode table for M135A

In reply to a message not in the archive
drivers/media/IR/ir-keymaps.c +50 -48
index dfc777b..55e7acd 100644
@@ -122,55 +122,57 @@ static struct ir_scancode ir_codes_avermedia_dvbt[] = {
 };
 IR_TABLE(avermedia_dvbt, IR_TYPE_UNKNOWN, ir_codes_avermedia_dvbt);
 
-/* Mauro Carvalho Chehab <[email protected]> */
-static struct ir_scancode ir_codes_avermedia_m135a[] = {
-	{ 0x00, KEY_POWER2 },
-	{ 0x2e, KEY_DOT },		/* '.' */
-	{ 0x01, KEY_MODE },		/* TV/FM */
-
-	{ 0x05, KEY_1 },
-	{ 0x06, KEY_2 },
-	{ 0x07, KEY_3 },
-	{ 0x09, KEY_4 },
-	{ 0x0a, KEY_5 },
-	{ 0x0b, KEY_6 },
-	{ 0x0d, KEY_7 },
-	{ 0x0e, KEY_8 },
-	{ 0x0f, KEY_9 },
-	{ 0x11, KEY_0 },
-
-	{ 0x13, KEY_RIGHT },		/* -> */
-	{ 0x12, KEY_LEFT },		/* <- */
-
-	{ 0x17, KEY_SLEEP },		/* Capturar Imagem */
-	{ 0x10, KEY_SHUFFLE },		/* Amostra */
-
-	/* FIXME: The keys bellow aren't ok */
-
-	{ 0x43, KEY_CHANNELUP },
-	{ 0x42, KEY_CHANNELDOWN },
-	{ 0x1f, KEY_VOLUMEUP },
-	{ 0x1e, KEY_VOLUMEDOWN },
-	{ 0x0c, KEY_ENTER },
-
-	{ 0x14, KEY_MUTE },
-	{ 0x08, KEY_AUDIO },
-
-	{ 0x03, KEY_TEXT },
-	{ 0x04, KEY_EPG },
-	{ 0x2b, KEY_TV2 },		/* TV2 */
-
-	{ 0x1d, KEY_RED },
-	{ 0x1c, KEY_YELLOW },
-	{ 0x41, KEY_GREEN },
-	{ 0x40, KEY_BLUE },
-
-	{ 0x1a, KEY_PLAYPAUSE },
-	{ 0x19, KEY_RECORD },
-	{ 0x18, KEY_PLAY },
-	{ 0x1b, KEY_STOP },
+/*
+ * Avermedia M135A with IR model RM-JX
+ * The same codes exist on both Positivo (BR) and original IR
+ * Mauro Carvalho Chehab <[email protected]>
+ */
+static struct ir_scancode ir_codes_avermedia_m135a_rm_jx[] = {
+	{ 0x0200, KEY_POWER2 },
+	{ 0x022e, KEY_DOT },		/* '.' */
+	{ 0x0201, KEY_MODE },		/* TV/FM or SOURCE */
+
+	{ 0x0205, KEY_1 },
+	{ 0x0206, KEY_2 },
+	{ 0x0207, KEY_3 },
+	{ 0x0209, KEY_4 },
+	{ 0x020a, KEY_5 },
+	{ 0x020b, KEY_6 },
+	{ 0x020d, KEY_7 },
+	{ 0x020e, KEY_8 },
+	{ 0x020f, KEY_9 },
+	{ 0x0211, KEY_0 },
+
+	{ 0x0213, KEY_RIGHT },		/* -> or L */
+	{ 0x0212, KEY_LEFT },		/* <- or R */
+
+	{ 0x0217, KEY_SLEEP },		/* Capturar Imagem or Snapshot */
+	{ 0x0210, KEY_SHUFFLE },	/* Amostra or 16 chan prev */
+
+	{ 0x0303, KEY_CHANNELUP },
+	{ 0x0302, KEY_CHANNELDOWN },
+	{ 0x021f, KEY_VOLUMEUP },
+	{ 0x021e, KEY_VOLUMEDOWN },
+	{ 0x020c, KEY_ENTER },		/* Full Screen */
+
+	{ 0x0214, KEY_MUTE },
+	{ 0x0208, KEY_AUDIO },
+
+	{ 0x0203, KEY_TEXT },		/* Teletext */
+	{ 0x0204, KEY_EPG },
+	{ 0x022b, KEY_TV2 },		/* TV2 or PIP */
+
+	{ 0x021d, KEY_RED },
+	{ 0x021c, KEY_YELLOW },
+	{ 0x0301, KEY_GREEN },
+	{ 0x0300, KEY_BLUE },
+
+	{ 0x021a, KEY_PLAYPAUSE },
+	{ 0x0219, KEY_RECORD },
+	{ 0x0218, KEY_PLAY },
+	{ 0x021b, KEY_STOP },
 };
-IR_TABLE(avermedia_m135a, IR_TYPE_UNKNOWN, ir_codes_avermedia_m135a);
+IR_TABLE(aver-m135a-RM-JX, IR_TYPE_NEC, ir_codes_avermedia_m135a_rm_jx);
 
 /* Oldrich Jedlicka <[email protected]> */
 static struct ir_scancode ir_codes_avermedia_cardbus[] = {
drivers/media/video/saa7134/saa7134-input.c +2 -2
index 8c67904..d7e73de 100644
@@ -524,9 +524,9 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 		saa_setb(SAA7134_GPIO_GPSTATUS0, 0x4);
 		break;
 	case SAA7134_BOARD_AVERMEDIA_M135A:
-		ir_codes     = &ir_codes_avermedia_m135a_table;
+		ir_codes     = &ir_codes_avermedia_m135a_rm_jx_table;
 		mask_keydown = 0x0040000;
-		mask_keycode = 0x00013f;
+		mask_keycode = 0xffff;
 		nec_gpio     = 1;
 		break;
 	case SAA7134_BOARD_AVERMEDIA_777:
include/media/ir-common.h +1 -1
index c662980..c30b283 100644
@@ -102,7 +102,7 @@ void ir_rc5_timer_keyup(unsigned long data);
 extern struct ir_scancode_table ir_codes_empty_table;
 extern struct ir_scancode_table ir_codes_avermedia_table;
 extern struct ir_scancode_table ir_codes_avermedia_dvbt_table;
-extern struct ir_scancode_table ir_codes_avermedia_m135a_table;
+extern struct ir_scancode_table ir_codes_avermedia_m135a_rm_jx_table;
 extern struct ir_scancode_table ir_codes_avermedia_cardbus_table;
 extern struct ir_scancode_table ir_codes_apac_viewcomp_table;
 extern struct ir_scancode_table ir_codes_pixelview_table;
Signature · 1 line
1.6.6.1
#3 lore raw

PATCH 3/15 V4L/DVB: saa7134: add code to allow changing IR protocol

In reply to a message not in the archive
drivers/media/video/saa7134/saa7134-input.c +60 -4
index d7e73de..8187928 100644
@@ -420,6 +420,10 @@ static void saa7134_input_timer(unsigned long data)
 
 void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir)
 {
+	if (ir->running)
+		return;
+
+	ir->running = 1;
 	if (ir->polling) {
 		setup_timer(&ir->timer, saa7134_input_timer,
 			    (unsigned long)dev);
@@ -447,8 +451,50 @@ void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir)
 
 void saa7134_ir_stop(struct saa7134_dev *dev)
 {
+	struct card_ir *ir = dev->remote;
+
+	if (!ir->running)
+		return;
 	if (dev->remote->polling)
 		del_timer_sync(&dev->remote->timer);
+	else if (ir->rc5_gpio)
+		del_timer_sync(&ir->timer_end);
+	else if (ir->nec_gpio)
+		tasklet_kill(&ir->tlet);
+	ir->running = 0;
+}
+
+int saa7134_ir_change_protocol(void *priv, u64 ir_type)
+{
+	struct saa7134_dev *dev = priv;
+	struct card_ir *ir = dev->remote;
+	u32 nec_gpio, rc5_gpio;
+
+	if (ir_type == IR_TYPE_RC5) {
+		dprintk("Changing protocol to RC5\n");
+		nec_gpio = 0;
+		rc5_gpio = 1;
+	} else if (ir_type == IR_TYPE_NEC) {
+		dprintk("Changing protocol to NEC\n");
+		nec_gpio = 1;
+		rc5_gpio = 0;
+	} else {
+		dprintk("IR protocol type %ud is not supported\n",
+			(unsigned)ir_type);
+		return -EINVAL;
+	}
+
+	if (ir->running) {
+		saa7134_ir_stop(dev);
+		ir->nec_gpio = nec_gpio;
+		ir->rc5_gpio = rc5_gpio;
+		saa7134_ir_start(dev, ir);
+	} else {
+		ir->nec_gpio = nec_gpio;
+		ir->rc5_gpio = rc5_gpio;
+	}
+
+	return 0;
 }
 
 int saa7134_input_init1(struct saa7134_dev *dev)
@@ -697,6 +743,9 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	}
 
 	ir->dev = input_dev;
+	dev->remote = ir;
+
+	ir->running = 0;
 
 	/* init hardware-specific stuff */
 	ir->mask_keycode = mask_keycode;
@@ -712,6 +761,14 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	snprintf(ir->phys, sizeof(ir->phys), "pci-%s/ir0",
 		 pci_name(dev->pci));
 
+	if (ir_codes->ir_type != IR_TYPE_OTHER) {
+		ir->props.allowed_protos = IR_TYPE_RC5 | IR_TYPE_NEC;
+		ir->props.priv = dev;
+		ir->props.change_protocol = saa7134_ir_change_protocol;
+
+		/* Set IR protocol */
+		saa7134_ir_change_protocol(ir->props.priv, ir_codes->ir_type);
+	}
 	err = ir_input_init(input_dev, &ir->ir, ir_type);
 	if (err < 0)
 		goto err_out_free;
@@ -729,13 +786,12 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	}
 	input_dev->dev.parent = &dev->pci->dev;
 
-	dev->remote = ir;
-	saa7134_ir_start(dev, ir);
-
-	err = ir_input_register(ir->dev, ir_codes, NULL, MODULE_NAME);
+	err = ir_input_register(ir->dev, ir_codes, &ir->props, MODULE_NAME);
 	if (err)
 		goto err_out_stop;
 
+	saa7134_ir_start(dev, ir);
+
 	/* the remote isn't as bouncy as a keyboard */
 	ir->dev->rep[REP_DELAY] = repeat_delay;
 	ir->dev->rep[REP_PERIOD] = repeat_period;
include/media/ir-common.h +3 -0
index c30b283..41469b7 100644
@@ -51,6 +51,9 @@ struct card_ir {
 	char                    name[32];
 	char                    phys[32];
 
+	u32			running:1;
+	struct ir_dev_props	props;
+
 	/* Usual gpio signalling */
 
 	u32                     mask_keycode;
Signature · 1 line
1.6.6.1
#16 lore raw

PATCH 5/15 V4L/DVB: ir-core: add two functions to report keyup/keydown events

In reply to a message not in the archive
drivers/media/IR/ir-keytable.c +56 -1
index 2d9ba84..ab60730 100644
@@ -364,7 +364,7 @@ static int ir_setkeycode(struct input_dev *dev,
  *
  * This routine is used by the input routines when a key is pressed at the
  * IR. The scancode is received and needs to be converted into a keycode.
- * If the key is not found, it returns KEY_UNKNOWN. Otherwise, returns the
+ * If the key is not found, it returns KEY_RESERVED. Otherwise, returns the
  * corresponding keycode from the table.
  */
 u32 ir_g_keycode_from_table(struct input_dev *dev, u32 scancode)
@@ -391,6 +391,61 @@ u32 ir_g_keycode_from_table(struct input_dev *dev, u32 scancode)
 EXPORT_SYMBOL_GPL(ir_g_keycode_from_table);
 
 /**
+ * ir_keyup() - generates input event to cleanup a key press
+ * @input_dev:	the struct input_dev descriptor of the device
+ *
+ * This routine is used by the input routines when a key is pressed at the
+ * IR. It reports a keyup input event via input_report_key().
+ */
+void ir_keyup(struct input_dev *dev)
+{
+	struct ir_input_dev *ir = input_get_drvdata(dev);
+
+	if (!ir->keypressed)
+		return;
+
+	input_report_key(dev, ir->keycode, 0);
+	input_sync(dev);
+	ir->keypressed = 0;
+}
+EXPORT_SYMBOL_GPL(ir_keyup);
+
+/**
+ * ir_keydown() - generates input event for a key press
+ * @input_dev:	the struct input_dev descriptor of the device
+ * @scancode:	the scancode that we're seeking
+ *
+ * This routine is used by the input routines when a key is pressed at the
+ * IR. It gets the keycode for a scancode and reports an input event via
+ * input_report_key().
+ */
+void ir_keydown(struct input_dev *dev, int scancode)
+{
+	struct ir_input_dev *ir = input_get_drvdata(dev);
+
+	u32 keycode = ir_g_keycode_from_table(dev, scancode);
+
+	/* If already sent a keydown, do a keyup */
+	if (ir->keypressed)
+		ir_keyup(dev);
+
+	if (KEY_RESERVED == keycode)
+		return;
+
+	ir->keycode = keycode;
+	ir->keypressed = 1;
+
+	IR_dprintk(1, "%s: key down event, key 0x%04x, scancode 0x%04x\n",
+		dev->name, keycode, scancode);
+
+	input_report_key(dev, ir->keycode, 1);
+	input_sync(dev);
+
+}
+EXPORT_SYMBOL_GPL(ir_keydown);
+
+
+/**
  * ir_input_register() - sets the IR keycode table and add the handlers
  *			    for keymap table get/set
  * @input_dev:	the struct input_dev descriptor of the device
include/media/ir-core.h +4 -0
index 369969d..198fd61 100644
@@ -72,6 +72,10 @@ struct ir_input_dev {
 	unsigned long			devno;		/* device number */
 	const struct ir_dev_props	*props;		/* Device properties */
 	struct ir_raw_event_ctrl	*raw;		/* for raw pulse/space events */
+
+	/* key info - needed by IR keycode handlers */
+	u32				keycode;	/* linux key code */
+	int				keypressed;	/* current state */
 };
 
 #define to_ir_input_dev(_attr) container_of(_attr, struct ir_input_dev, attr)
Signature · 1 line
1.6.6.1
#18 lore raw

PATCH 7/15 V4L/DVB: saa7134: don't wait too much to generate an IR event on raw_decode

In reply to a message not in the archive

At raw_decode mode, the key is processed after the end of a timer. The previous code resets the timer every time something is received at the IR port. While this works fine with IR's that don't implement repeat, like Avermedia RM-JX IR, it keeps waiting until keydown, on IR's that implement NEC repeat command, like the Terratec yellow.

The solution is to change the behaviour to do the timeout after the first received data.

The timeout is currently set to 15 ms, as it works fine with NEC protcocol. It may need some adjustments to support other protocols and to better handle spurious detections that may happen with some IR sensors.

drivers/media/IR/ir-keytable.c +1 -0
index ab60730..f25193c 100644
@@ -404,6 +404,7 @@ void ir_keyup(struct input_dev *dev)
 	if (!ir->keypressed)
 		return;
 
+	IR_dprintk(1, "keyup key 0x%04x\n", ir->keycode);
 	input_report_key(dev, ir->keycode, 0);
 	input_sync(dev);
 	ir->keypressed = 0;
drivers/media/IR/ir-nec-decoder.c +83 -39

178 lines not on this page · raw message

drivers/media/IR/ir-raw-event.c +14 -0
index 9c71ac8..0ae5543 100644
@@ -17,6 +17,13 @@
 /* Define the max number of bit transitions per IR keycode */
 #define MAX_IR_EVENT_SIZE	256
 
+static void ir_keyup_timer(unsigned long data)
+{
+	struct input_dev *input_dev = (struct input_dev *)data;
+
+	ir_keyup(input_dev);
+}
+
 int ir_raw_event_register(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir = input_get_drvdata(input_dev);
@@ -27,6 +34,11 @@ int ir_raw_event_register(struct input_dev *input_dev)
 	size = sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE * 2;
 	size = roundup_pow_of_two(size);
 
+	init_timer(&ir->raw->timer_keyup);
+	ir->raw->timer_keyup.function = ir_keyup_timer;
+	ir->raw->timer_keyup.data = (unsigned long)input_dev;
+	set_bit(EV_REP, input_dev->evbit);
+
 	rc = kfifo_alloc(&ir->raw->kfifo, size, GFP_KERNEL);
 
 	return rc;
@@ -40,6 +52,8 @@ void ir_raw_event_unregister(struct input_dev *input_dev)
 	if (!ir->raw)
 		return;
 
+	del_timer_sync(&ir->raw->timer_keyup);
+
 	kfifo_free(&ir->raw->kfifo);
 	kfree(ir->raw);
 	ir->raw = NULL;
drivers/media/video/saa7134/saa7134-input.c +17 -26

78 lines not on this page · raw message

include/media/ir-core.h +2 -0
index 9e03528..8d8ed7e 100644
@@ -18,6 +18,7 @@
 #include <linux/spinlock.h>
 #include <linux/kfifo.h>
 #include <linux/time.h>
+#include <linux/timer.h>
 
 extern int ir_core_debug;
 #define IR_dprintk(level, fmt, arg...)	if (ir_core_debug >= level) \
@@ -63,6 +64,7 @@ struct ir_raw_event {
 struct ir_raw_event_ctrl {
 	struct kfifo			kfifo;		/* fifo for the pulse/space events */
 	struct timespec			last_event;	/* when last event occurred */
+	struct timer_list		timer_keyup;	/* timer for key release */
 };
 
 struct ir_input_dev {
Signature · 1 line
1.6.6.1
#22 lore raw

PATCH 11/15 V4L/DVB: saa7134: clear warning noise

In reply to a message not in the archive

drivers/media/video/saa7134/saa7134-input.c: In function ‘saa7134_raw_decode_irq’:
drivers/media/video/saa7134/saa7134-input.c:957: warning: unused variable ‘oldpulse’ drivers/media/video/saa7134/saa7134-input.c:957: warning: unused variable ‘count’

drivers/media/video/saa7134/saa7134-input.c +1 -1
index 8dd74ae..a42c953 100644
@@ -954,7 +954,7 @@ static int saa7134_raw_decode_irq(struct saa7134_dev *dev)
 {
 	struct card_ir	*ir = dev->remote;
 	unsigned long 	timeout;
-	int count, pulse, oldpulse;
+	int pulse;
 
 	/* Generate initial event */
 	saa_clearb(SAA7134_GPIO_GPMODE3, SAA7134_GPIO_GPRESCAN);
Signature · 7 lines
1.6.6.1


--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#21 lore raw

PATCH 10/15 V4L/DVB: ir-nec-decoder: Add sysfs node to enable/disable per irrcv

In reply to a message not in the archive

With the help of raw_register/raw_unregister, adds a sysfs group associated with the decoder, inside the corresponding irrcv node.

Writing 1 to nec_decoder/enabled enables the decoder, while writing 0 disables it.

drivers/media/IR/ir-nec-decoder.c +132 -1
index cb57cc2..83a9912 100644
@@ -39,6 +39,85 @@
 
 #define REPEAT_TIME	240 /* ms */
 
+/* Used to register nec_decoder clients */
+static LIST_HEAD(decoder_list);
+static spinlock_t decoder_lock;
+
+struct decoder_data {
+	struct list_head	list;
+	struct ir_input_dev	*ir_dev;
+	int			enabled:1;
+};
+
+
+/**
+ * get_decoder_data()	- gets decoder data
+ * @input_dev:	input device
+ *
+ * Returns the struct decoder_data that corresponds to a device
+ */
+
+static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
+{
+	struct decoder_data *data = NULL;
+
+	spin_lock(&decoder_lock);
+	list_for_each_entry(data, &decoder_list, list) {
+		if (data->ir_dev == ir_dev)
+			break;
+	}
+	spin_unlock(&decoder_lock);
+	return data;
+}
+
+static ssize_t store_enabled(struct device *d,
+			     struct device_attribute *mattr,
+			     const char *buf,
+			     size_t len)
+{
+	unsigned long value;
+	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
+	struct decoder_data *data = get_decoder_data(ir_dev);
+
+	if (!data)
+		return -EINVAL;
+
+	if (strict_strtoul(buf, 10, &value) || value > 1)
+		return -EINVAL;
+
+	data->enabled = value;
+
+	return len;
+}
+
+static ssize_t show_enabled(struct device *d,
+			     struct device_attribute *mattr, char *buf)
+{
+	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
+	struct decoder_data *data = get_decoder_data(ir_dev);
+
+	if (!data)
+		return -EINVAL;
+
+	if (data->enabled)
+		return sprintf(buf, "1\n");
+	else
+	return sprintf(buf, "0\n");
+}
+
+static DEVICE_ATTR(enabled, S_IRUGO | S_IWUSR, show_enabled, store_enabled);
+
+static struct attribute *decoder_attributes[] = {
+	&dev_attr_enabled.attr,
+	NULL
+};
+
+static struct attribute_group decoder_attribute_group = {
+	.name	= "nec_decoder",
+	.attrs	= decoder_attributes,
+};
+
+
 /** is_repeat - Check if it is a NEC repeat event
  * @input_dev:	the struct input_dev descriptor of the device
  * @pos:	the position of the first event
@@ -184,9 +263,15 @@ static int ir_nec_decode(struct input_dev *input_dev,
 			 struct ir_raw_event *evs,
 			 int len)
 {
+	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct decoder_data *data;
 	int pos = 0;
 	int rc = 0;
 
+	data = get_decoder_data(ir_dev);
+	if (!data || !data->enabled)
+		return 0;
+
 	while (pos < len) {
 		if (__ir_nec_decode(input_dev, evs, len, &pos) > 0)
 			rc++;
@@ -194,8 +279,54 @@ static int ir_nec_decode(struct input_dev *input_dev,
 	return rc;
 }
 
+static int ir_nec_register(struct input_dev *input_dev)
+{
+	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct decoder_data *data;
+	int rc;
+
+	rc = sysfs_create_group(&ir_dev->dev.kobj, &decoder_attribute_group);
+	if (rc < 0)
+		return rc;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data) {
+		sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
+		return -ENOMEM;
+	}
+
+	data->ir_dev = ir_dev;
+	data->enabled = 1;
+
+	spin_lock(&decoder_lock);
+	list_add_tail(&data->list, &decoder_list);
+	spin_unlock(&decoder_lock);
+
+	return 0;
+}
+
+static int ir_nec_unregister(struct input_dev *input_dev)
+{
+	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	static struct decoder_data *data;
+
+	data = get_decoder_data(ir_dev);
+	if (!data)
+		return 0;
+
+	sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
+
+	spin_lock(&decoder_lock);
+	list_del(&data->list);
+	spin_unlock(&decoder_lock);
+
+	return 0;
+}
+
 static struct ir_raw_handler nec_handler = {
-	.decode = ir_nec_decode,
+	.decode		= ir_nec_decode,
+	.raw_register	= ir_nec_register,
+	.raw_unregister	= ir_nec_unregister,
 };
 
 static int __init ir_nec_decode_init(void)
drivers/media/IR/ir-raw-event.c +1 -1
index 11f23f4..371d88e 100644
@@ -223,7 +223,7 @@ static void init_decoders(struct work_struct *work)
 void ir_raw_init(void)
 {
 	spin_lock_init(&ir_raw_handler_lock);
-	
+
 #ifdef MODULE
 	INIT_WORK(&wq_load, init_decoders);
 	schedule_work(&wq_load);
Signature · 1 line
1.6.6.1
#20 lore raw

PATCH 9/15 V4L/DVB: ir-core: prepare to add more operations for ir decoders

In reply to a message not in the archive

Some decoders and a lirc_dev interface may need some other operations to work. For example: IR device register/unregister and ir_keydown events may need to be tracked.

As some operations can occur in interrupt time, and a lock is needed to prevent un-registering a decode while decoding a key, the lock needed to be convert into a spin lock.

drivers/media/IR/ir-nec-decoder.c +1 -5
index c9a986d..cb57cc2 100644
@@ -178,8 +178,7 @@ err:
  * @input_dev:	the struct input_dev descriptor of the device
  * @evs:	event array with type/duration of pulse/space
  * @len:	length of the array
- * This function returns the number of decoded pulses or -EINVAL if no
- * pulse got decoded
+ * This function returns the number of decoded pulses
  */
 static int ir_nec_decode(struct input_dev *input_dev,
 			 struct ir_raw_event *evs,
@@ -192,9 +191,6 @@ static int ir_nec_decode(struct input_dev *input_dev,
 		if (__ir_nec_decode(input_dev, evs, len, &pos) > 0)
 			rc++;
 	}
-
-	if (!rc)
-		return -EINVAL;
 	return rc;
 }
 
drivers/media/IR/ir-raw-event.c +58 -13

134 lines not on this page · raw message

drivers/media/video/saa7134/saa7134-input.c +1 -1
index 38f60e4..8dd74ae 100644
@@ -960,7 +960,7 @@ static int saa7134_raw_decode_irq(struct saa7134_dev *dev)
 	saa_clearb(SAA7134_GPIO_GPMODE3, SAA7134_GPIO_GPRESCAN);
 	saa_setb(SAA7134_GPIO_GPMODE3, SAA7134_GPIO_GPRESCAN);
 	pulse = saa_readl(SAA7134_GPIO_GPSTATUS0 >> 2) & ir->mask_keydown;
-	ir_raw_event_store(dev->remote->dev, pulse? IR_PULSE : IR_SPACE);
+	ir_raw_event_store(dev->remote->dev, pulse ? IR_PULSE : IR_SPACE);
 
 
 	/*
include/media/ir-core.h +2 -5
index c377bf4..c704fa7 100644
@@ -88,6 +88,8 @@ struct ir_raw_handler {
 	int (*decode)(struct input_dev *input_dev,
 		      struct ir_raw_event *evs,
 		      int len);
+	int (*raw_register)(struct input_dev *input_dev);
+	int (*raw_unregister)(struct input_dev *input_dev);
 };
 
 #define to_ir_input_dev(_attr) container_of(_attr, struct ir_input_dev, attr)
@@ -116,12 +118,7 @@ int ir_raw_event_store(struct input_dev *input_dev, enum raw_event_type type);
 int ir_raw_event_handle(struct input_dev *input_dev);
 int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler);
 void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler);
-
-#ifdef MODULE
 void ir_raw_init(void);
-#else
-#define ir_raw_init() 0
-#endif
 
 /* from ir-nec-decoder.c */
 #ifdef CONFIG_IR_NEC_DECODER_MODULE
Signature · 1 line
1.6.6.1
#19 lore raw

PATCH 8/15 V4L/DVB: ir-core: dynamically load the compiled IR protocols

In reply to a message not in the archive

Instead of hardcoding the protocols into ir-core, add a register interface for the IR protocol decoders, and convert ir-nec-decoder into a client of ir-core.

With this approach, it is possible to dynamically load the needed IR protocols, and to add a RAW IR interface module, registered as one IR raw protocol decoder.

This patch opens a way to register a lirc_dev interface to work as an userspace IR protocol decoder.

drivers/media/IR/Kconfig +9 -0
index 4dde7d1..de410d4 100644
@@ -7,3 +7,12 @@ config VIDEO_IR
 	tristate
 	depends on IR_CORE
 	default IR_CORE
+
+config IR_NEC_DECODER
+	tristate "Enable IR raw decoder for NEC protocol"
+	depends on IR_CORE
+	default y
+
+	---help---
+	   Enable this option if you have IR with NEC protocol, and
+	   if the IR is decoded in software
drivers/media/IR/Makefile +2 -1
index 18794c7..6140b27 100644
@@ -1,5 +1,6 @@
 ir-common-objs  := ir-functions.o ir-keymaps.o
-ir-core-objs	:= ir-keytable.o ir-sysfs.o ir-raw-event.o ir-nec-decoder.o
+ir-core-objs	:= ir-keytable.o ir-sysfs.o ir-raw-event.o
 
 obj-$(CONFIG_IR_CORE) += ir-core.o
 obj-$(CONFIG_VIDEO_IR) += ir-common.o
+obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
drivers/media/IR/ir-nec-decoder.c +29 -6
index 104482a..c9a986d 100644
@@ -1,4 +1,4 @@
-/* ir-raw-event.c - handle IR Pulse/Space event
+/* ir-nec-decoder.c - handle NEC IR Pulse/Space protocol
  *
  * Copyright (C) 2010 by Mauro Carvalho Chehab <[email protected]>
  *
@@ -147,7 +147,7 @@ static int __ir_nec_decode(struct input_dev *input_dev,
 		if (++count == 32)
 			break;
 	}
-	*pos++;
+	(*pos)++;
 
 	/*
 	 * Fixme: may need to accept Extended NEC protocol?
@@ -181,9 +181,9 @@ err:
  * This function returns the number of decoded pulses or -EINVAL if no
  * pulse got decoded
  */
-int ir_nec_decode(struct input_dev *input_dev,
-			   struct ir_raw_event *evs,
-			   int len)
+static int ir_nec_decode(struct input_dev *input_dev,
+			 struct ir_raw_event *evs,
+			 int len)
 {
 	int pos = 0;
 	int rc = 0;
@@ -198,4 +198,27 @@ int ir_nec_decode(struct input_dev *input_dev,
 	return rc;
 }
 
-EXPORT_SYMBOL_GPL(ir_nec_decode);
+static struct ir_raw_handler nec_handler = {
+	.decode = ir_nec_decode,
+};
+
+static int __init ir_nec_decode_init(void)
+{
+	ir_raw_handler_register(&nec_handler);
+
+	printk(KERN_INFO "IR NEC protocol handler initialized\n");
+	return 0;
+}
+
+static void __exit ir_nec_decode_exit(void)
+{
+	ir_raw_handler_unregister(&nec_handler);
+}
+
+module_init(ir_nec_decode_init);
+module_exit(ir_nec_decode_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Mauro Carvalho Chehab <[email protected]>");
+MODULE_AUTHOR("Red Hat Inc. (http://www.redhat.com)");
+MODULE_DESCRIPTION("NEC IR protocol decoder");
drivers/media/IR/ir-raw-event.c +56 -1

83 lines not on this page · raw message

drivers/media/IR/ir-sysfs.c +5 -2
index ee6b36d..2dbce59 100644
@@ -1,6 +1,6 @@
 /* ir-register.c - handle IR scancode->keycode tables
  *
- * Copyright (C) 2009 by Mauro Carvalho Chehab <[email protected]>
+ * Copyright (C) 2009-2010 by Mauro Carvalho Chehab <[email protected]>
  *
  * 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
@@ -27,7 +27,7 @@ static char *ir_devnode(struct device *dev, mode_t *mode)
 	return kasprintf(GFP_KERNEL, "irrcv/%s", dev_name(dev));
 }
 
-struct class ir_input_class = {
+static struct class ir_input_class = {
 	.name		= "irrcv",
 	.devnode	= ir_devnode,
 };
@@ -250,6 +250,9 @@ static int __init ir_core_init(void)
 		return rc;
 	}
 
+	/* Initialize/load the decoders that will be used */
+	ir_raw_init();
+
 	return 0;
 }
 
include/media/ir-core.h +24 -5
index 8d8ed7e..c377bf4 100644
@@ -1,6 +1,8 @@
 /*
  * Remote Controller core header
  *
+ * Copyright (C) 2009-2010 by Mauro Carvalho Chehab <[email protected]>
+ *
  * 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 version 2 of the License.
@@ -80,6 +82,14 @@ struct ir_input_dev {
 	int				keypressed;	/* current state */
 };
 
+struct ir_raw_handler {
+	struct list_head list;
+
+	int (*decode)(struct input_dev *input_dev,
+		      struct ir_raw_event *evs,
+		      int len);
+};
+
 #define to_ir_input_dev(_attr) container_of(_attr, struct ir_input_dev, attr)
 
 /* Routines from ir-keytable.c */
@@ -104,11 +114,20 @@ int ir_raw_event_register(struct input_dev *input_dev);
 void ir_raw_event_unregister(struct input_dev *input_dev);
 int ir_raw_event_store(struct input_dev *input_dev, enum raw_event_type type);
 int ir_raw_event_handle(struct input_dev *input_dev);
+int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler);
+void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler);
+
+#ifdef MODULE
+void ir_raw_init(void);
+#else
+#define ir_raw_init() 0
+#endif
 
 /* from ir-nec-decoder.c */
-int ir_nec_decode(struct input_dev *input_dev,
-		  struct ir_raw_event *evs,
-		  int len);
-
-
+#ifdef CONFIG_IR_NEC_DECODER_MODULE
+#define load_nec_decode()	request_module("ir-nec-decoder")
+#else
+#define load_nec_decode()	0
 #endif
+
+#endif /* _IR_CORE */
Signature · 1 line
1.6.6.1
#17 lore raw

PATCH 6/15 V4L/DVB: ir-core/saa7134: Move ir keyup/keydown code to the ir-core

In reply to a message not in the archive
drivers/media/IR/ir-nec-decoder.c +58 -32
index 16360eb..a58c717 100644
@@ -30,37 +30,35 @@
 #define MIN_BIT0_TIME	360000
 #define MAX_BIT0_TIME	760000
 
-
-/** Decode NEC pulsecode. This code can take up to 76.5 ms to run.
-	Unfortunately, using IRQ to decode pulse didn't work, since it uses
-	a pulse train of 38KHz. This means one pulse on each 52 us
-*/
-
-int ir_nec_decode(struct input_dev *input_dev,
-		  struct ir_raw_event *evs,
-		  int len)
+/**
+ * __ir_nec_decode() - Decode one NEC pulsecode
+ * @input_dev:	the struct input_dev descriptor of the device
+ * @evs:	event array with type/duration of pulse/space
+ * @len:	length of the array
+ * @pos:	position to start seeking for a code
+ * This function returns the decoded ircode or -EINVAL if no pulse got decoded
+ */
+static int __ir_nec_decode(struct input_dev *input_dev,
+			   struct ir_raw_event *evs,
+			   int len, int *pos)
 {
-	int i, count = -1;
+	int count = -1;
 	int ircode = 0, not_code = 0;
-#if 0
-	/* Needed only after porting the event code to the decoder */
-	struct ir_input_dev *ir = input_get_drvdata(input_dev);
-#endif
 
 	/* Be sure that the first event is an start one and is a pulse */
-	for (i = 0; i < len; i++) {
-		if (evs[i].type & (IR_START_EVENT | IR_PULSE))
+	for (; *pos < len; (*pos)++) {
+		if (evs[*pos].type & (IR_START_EVENT | IR_PULSE))
 			break;
 	}
-	i++;	/* First event doesn't contain data */
+	(*pos)++;	/* First event doesn't contain data */
 
-	if (i >= len)
+	if (*pos >= len)
 		return 0;
 
 	/* First space should have 4.5 ms otherwise is not NEC protocol */
-	if ((evs[i].delta.tv_nsec < MIN_START_TIME) |
-	    (evs[i].delta.tv_nsec > MAX_START_TIME) |
-	    (evs[i].type != IR_SPACE))
+	if ((evs[*pos].delta.tv_nsec < MIN_START_TIME) |
+	    (evs[*pos].delta.tv_nsec > MAX_START_TIME) |
+	    (evs[*pos].type != IR_SPACE))
 		goto err;
 
 	/*
@@ -68,24 +66,24 @@ int ir_nec_decode(struct input_dev *input_dev,
 	 */
 
 	count = 0;
-	for (i++; i < len; i++) {
+	for ((*pos)++; *pos < len; (*pos)++) {
 		int bit;
 
-		if ((evs[i].delta.tv_nsec < MIN_PULSE_TIME) |
-		    (evs[i].delta.tv_nsec > MAX_PULSE_TIME) |
-		    (evs[i].type != IR_PULSE))
+		if ((evs[*pos].delta.tv_nsec < MIN_PULSE_TIME) |
+		    (evs[*pos].delta.tv_nsec > MAX_PULSE_TIME) |
+		    (evs[*pos].type != IR_PULSE))
 			goto err;
 
-		if (++i >= len)
+		if (++*pos >= len)
 			goto err;
-		if (evs[i].type != IR_SPACE)
+		if (evs[*pos].type != IR_SPACE)
 			goto err;
 
-		if ((evs[i].delta.tv_nsec > MIN_BIT1_TIME) &&
-		    (evs[i].delta.tv_nsec < MAX_BIT1_TIME))
+		if ((evs[*pos].delta.tv_nsec > MIN_BIT1_TIME) &&
+		    (evs[*pos].delta.tv_nsec < MAX_BIT1_TIME))
 			bit = 1;
-		else if ((evs[i].delta.tv_nsec > MIN_BIT0_TIME) &&
-			 (evs[i].delta.tv_nsec < MAX_BIT0_TIME))
+		else if ((evs[*pos].delta.tv_nsec > MIN_BIT0_TIME) &&
+			 (evs[*pos].delta.tv_nsec < MAX_BIT0_TIME))
 			bit = 0;
 		else
 			goto err;
@@ -120,12 +118,40 @@ int ir_nec_decode(struct input_dev *input_dev,
 	}
 
 	IR_dprintk(1, "NEC scancode 0x%04x\n", ircode);
+	ir_keydown(input_dev, ircode);
+	ir_keyup(input_dev);
 
 	return ircode;
 err:
 	IR_dprintk(1, "NEC decoded failed at bit %d while decoding %luus time\n",
-		   count, (evs[i].delta.tv_nsec + 500) / 1000);
+		   count, (evs[*pos].delta.tv_nsec + 500) / 1000);
 
 	return -EINVAL;
 }
+
+/**
+ * __ir_nec_decode() - Decodes all NEC pulsecodes on a given array
+ * @input_dev:	the struct input_dev descriptor of the device
+ * @evs:	event array with type/duration of pulse/space
+ * @len:	length of the array
+ * This function returns the number of decoded pulses or -EINVAL if no
+ * pulse got decoded
+ */
+int ir_nec_decode(struct input_dev *input_dev,
+			   struct ir_raw_event *evs,
+			   int len)
+{
+	int pos = 0;
+	int rc = 0;
+
+	while (pos < len) {
+		if (__ir_nec_decode(input_dev, evs, len, &pos) >= 0)
+			rc++;
+	}
+
+	if (!rc)
+		return -EINVAL;
+	return rc;
+}
+
 EXPORT_SYMBOL_GPL(ir_nec_decode);
drivers/media/video/saa7134/saa7134-input.c +1 -11
index 7382995..740adb3 100644
@@ -424,18 +424,8 @@ static void saa7134_input_timer(unsigned long data)
 void ir_raw_decode_timer_end(unsigned long data)
 {
 	struct saa7134_dev *dev = (struct saa7134_dev *)data;
-	struct card_ir *ir = dev->remote;
-	int rc;
 
-	/*
-	 * FIXME: the IR key handling code should be called by the decoder,
-	 * after implementing the repeat mode
-	 */
-	rc = ir_raw_event_handle(dev->remote->dev);
-	if (rc >= 0) {
-		ir_input_keydown(ir->dev, &ir->ir, rc);
-		ir_input_nokey(ir->dev, &ir->ir);
-	}
+	ir_raw_event_handle(dev->remote->dev);
 }
 
 void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir)
include/media/ir-core.h +2 -1
index 198fd61..9e03528 100644
@@ -84,7 +84,8 @@ struct ir_input_dev {
 
 u32 ir_g_keycode_from_table(struct input_dev *input_dev,
 			    u32 scancode);
-
+void ir_keyup(struct input_dev *dev);
+void ir_keydown(struct input_dev *dev, int scancode);
 int ir_input_register(struct input_dev *dev,
 		      const struct ir_scancode_table *ir_codes,
 		      const struct ir_dev_props *props,
Signature · 1 line
1.6.6.1
#4 lore raw

PATCH 4/15 V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

In reply to a message not in the archive

Adds a method to pass IR raw pulse/code events into ir-core. This is needed in order to support LIRC. It also helps to move common code from the drivers into the core.

In order to allow testing, it implements a simple NEC protocol decoder at ir-nec-decoder.c file. The logic is about the same used at saa7134 driver that handles Avermedia M135A and Encore FM53 boards.

 create mode 100644 drivers/media/IR/ir-nec-decoder.c
 create mode 100644 drivers/media/IR/ir-raw-event.c
drivers/media/IR/Makefile +1 -1
index 171890e..18794c7 100644
@@ -1,5 +1,5 @@
 ir-common-objs  := ir-functions.o ir-keymaps.o
-ir-core-objs	:= ir-keytable.o ir-sysfs.o
+ir-core-objs	:= ir-keytable.o ir-sysfs.o ir-raw-event.o ir-nec-decoder.o
 
 obj-$(CONFIG_IR_CORE) += ir-core.o
 obj-$(CONFIG_VIDEO_IR) += ir-common.o
drivers/media/IR/ir-nec-decoder.c new file +131 -0

131 lines not on this page · raw message

drivers/media/IR/ir-raw-event.c new file +117 -0

117 lines not on this page · raw message

drivers/media/video/saa7134/saa7134-input.c +81 -4

166 lines not on this page · raw message

include/media/ir-common.h +3 -0
index 41469b7..87f2ec7 100644
@@ -82,6 +82,9 @@ struct card_ir {
 	/* NEC decoding */
 	u32			nec_gpio;
 	struct tasklet_struct   tlet;
+
+	/* IR core raw decoding */
+	u32			raw_decode;
 };
 
 /* Routines from ir-functions.c */
include/media/ir-core.h +32 -0
index 1eae72d..369969d 100644
@@ -16,6 +16,8 @@
 
 #include <linux/input.h>
 #include <linux/spinlock.h>
+#include <linux/kfifo.h>
+#include <linux/time.h>
 
 extern int ir_core_debug;
 #define IR_dprintk(level, fmt, arg...)	if (ir_core_debug >= level) \
@@ -27,6 +29,13 @@ extern int ir_core_debug;
 #define IR_TYPE_NEC	(1  << 2)
 #define IR_TYPE_OTHER	(((u64)1) << 63l)
 
+enum raw_event_type {
+	IR_SPACE	= (1 << 0),
+	IR_PULSE	= (1 << 1),
+	IR_START_EVENT	= (1 << 2),
+	IR_STOP_EVENT	= (1 << 3),
+};
+
 struct ir_scancode {
 	u16	scancode;
 	u32	keycode;
@@ -46,6 +55,15 @@ struct ir_dev_props {
 	int (*change_protocol)(void *priv, u64 ir_type);
 };
 
+struct ir_raw_event {
+	struct timespec		delta;	/* Time spent before event */
+	enum raw_event_type	type;	/* event type */
+};
+
+struct ir_raw_event_ctrl {
+	struct kfifo			kfifo;		/* fifo for the pulse/space events */
+	struct timespec			last_event;	/* when last event occurred */
+};
 
 struct ir_input_dev {
 	struct device			dev;		/* device */
@@ -53,7 +71,9 @@ struct ir_input_dev {
 	struct ir_scancode_table	rc_tab;		/* scan/key table */
 	unsigned long			devno;		/* device number */
 	const struct ir_dev_props	*props;		/* Device properties */
+	struct ir_raw_event_ctrl	*raw;		/* for raw pulse/space events */
 };
+
 #define to_ir_input_dev(_attr) container_of(_attr, struct ir_input_dev, attr)
 
 /* Routines from ir-keytable.c */
@@ -72,4 +92,16 @@ void ir_input_unregister(struct input_dev *input_dev);
 int ir_register_class(struct input_dev *input_dev);
 void ir_unregister_class(struct input_dev *input_dev);
 
+/* Routines from ir-raw-event.c */
+int ir_raw_event_register(struct input_dev *input_dev);
+void ir_raw_event_unregister(struct input_dev *input_dev);
+int ir_raw_event_store(struct input_dev *input_dev, enum raw_event_type type);
+int ir_raw_event_handle(struct input_dev *input_dev);
+
+/* from ir-nec-decoder.c */
+int ir_nec_decode(struct input_dev *input_dev,
+		  struct ir_raw_event *evs,
+		  int len);
+
+
 #endif
Signature · 1 line
1.6.6.1
#26 lore raw

PATCH 15/15 V4L/DVB: input: Add support for EVIO[CS]GKEYCODEBIG

In reply to a message not in the archive

Several devices use a high number of bits for scancodes. One important group is the Remote Controllers. Some new protocols like RC-6 define a scancode space of 64 bits.

The current EVIO[CS]GKEYCODE ioctls allow replace the scancode/keycode translation tables, but it is limited to up to 32 bits for scancode.

Also, if userspace wants to clean the existing table, replacing it by a new one, it needs to run a loop calling the old ioctls, over the entire sparsed scancode userspace.

To solve those problems, this patch introduces two new ioctls:

	EVIOCGKEYCODEBIG - reads a scancode from the translation table;
	EVIOSGKEYCODEBIG - writes a scancode into the translation table.

The EVIOSGKEYCODEBIG can also be used to cleanup the translation entries by associating KEY_RESERVED to a scancode.

EVIOCGKEYCODEBIG uses kt_entry::index field in order to retrieve a keycode from the table. This field is unused on EVIOSGKEYCODEBIG.

By default, kernel will implement a default handler that will work with both EVIO[CS]GKEYCODEBIG and the legacy EVIO[CS]GKEYCODE ioctls.

Compatibility code were also added to allow drivers that implement only the ops handler for EVIO[CS]GKEYCODE to keep working.

Userspace compatibility for EVIO[CS]GKEYCODE is also granted: the evdev/input ioctl handler will automatically map those ioctls with the new getkeycodebig()/setkeycodebig() operations to handle a request using the legacy API.

So, new drivers should only implement the EVIO[CS]GKEYCODEBIG operation
handlers: getkeycodebig()/setkeycodebig().

drivers/input/evdev.c +39 -0
index 258c639..1730b5b 100644
@@ -513,6 +513,8 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 	struct input_absinfo abs;
 	struct ff_effect effect;
 	int __user *ip = (int __user *)p;
+	struct keycode_table_entry kt, *kt_p = p;
+	char scancode[16];
 	int i, t, u, v;
 	int error;
 
@@ -567,6 +569,43 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 
 		return input_set_keycode(dev, t, v);
 
+	case EVIOCGKEYCODEBIG:
+		if (copy_from_user(&kt, kt_p, sizeof(kt)))
+			return -EFAULT;
+
+		if (kt.len > sizeof(scancode))
+			return -EINVAL;
+
+		kt.scancode = scancode;
+
+		error = input_get_keycode_big(dev, &kt);
+		if (error)
+			return error;
+
+		if (copy_to_user(kt_p, &kt, sizeof(kt)))
+			return -EFAULT;
+
+		/* FIXME: probably need some compat32 code */
+		if (copy_to_user(kt_p->scancode, kt.scancode, kt.len))
+			return -EFAULT;
+
+		return 0;
+
+	case EVIOCSKEYCODEBIG:
+		if (copy_from_user(&kt, kt_p, sizeof(kt)))
+			return -EFAULT;
+
+		if (kt.len > sizeof(scancode))
+			return -EINVAL;
+
+		kt.scancode = scancode;
+
+		/* FIXME: probably need some compat32 code */
+		if (copy_from_user(kt.scancode, kt_p->scancode, kt.len))
+			return -EFAULT;
+
+		return input_set_keycode_big(dev, &kt);
+
 	case EVIOCRMFF:
 		return input_ff_erase(dev, (int)(unsigned long) p, file);
 
drivers/input/input.c +235 -25

346 lines not on this page · raw message

include/linux/input.h +35 -5
index 663208a..6445fc9 100644
@@ -34,7 +34,7 @@ struct input_event {
  * Protocol version.
  */
 
-#define EV_VERSION		0x010000
+#define EV_VERSION		0x010001
 
 /*
  * IOCTLs (0x00 - 0x7f)
@@ -56,12 +56,22 @@ struct input_absinfo {
 	__s32 resolution;
 };
 
+struct keycode_table_entry {
+	__u32 keycode;		/* e.g. KEY_A */
+	__u32 index;            /* Index for the given scan/key table, on EVIOCGKEYCODEBIG */
+	__u32 len;		/* Length of the scancode */
+	__u32 reserved[2];	/* Reserved for future usage */
+	char *scancode;		/* scancode, in machine-endian */
+};
+
 #define EVIOCGVERSION		_IOR('E', 0x01, int)			/* get driver version */
 #define EVIOCGID		_IOR('E', 0x02, struct input_id)	/* get device ID */
 #define EVIOCGREP		_IOR('E', 0x03, int[2])			/* get repeat settings */
 #define EVIOCSREP		_IOW('E', 0x03, int[2])			/* set repeat settings */
 #define EVIOCGKEYCODE		_IOR('E', 0x04, int[2])			/* get keycode */
 #define EVIOCSKEYCODE		_IOW('E', 0x04, int[2])			/* set keycode */
+#define EVIOCGKEYCODEBIG	_IOR('E', 0x04, struct keycode_table_entry) /* get keycode */
+#define EVIOCSKEYCODEBIG	_IOW('E', 0x04, struct keycode_table_entry) /* set keycode */
 
 #define EVIOCGNAME(len)		_IOC(_IOC_READ, 'E', 0x06, len)		/* get device name */
 #define EVIOCGPHYS(len)		_IOC(_IOC_READ, 'E', 0x07, len)		/* get physical location */
@@ -1022,13 +1032,22 @@ struct ff_effect {
  * @keycodemax: size of keycode table
  * @keycodesize: size of elements in keycode table
  * @keycode: map of scancodes to keycodes for this device
- * @setkeycode: optional method to alter current keymap, used to implement
+ * @setkeycode: optional legacy method to alter current keymap, used to
+ *	implement sparse keymaps. Shouldn't be used on new drivers
+ * @getkeycode: optional legacy method to retrieve current keymap.
+ *	Shouldn't be used on new drivers.
+ * @setkeycodebig: optional method to alter current keymap, used to implement
  *	sparse keymaps. If not supplied default mechanism will be used.
  *	The method is being called while holding event_lock and thus must
  *	not sleep
- * @getkeycode: optional method to retrieve current keymap. If not supplied
- *	default mechanism will be used. The method is being called while
- *	holding event_lock and thus must not sleep
+ * @getkeycodebig_from_index: optional method to retrieve current keymap from
+ *      an array index. If not supplied default mechanism will be used.
+ *	The method is being called while holding event_lock and thus must
+ *      not sleep
+ * @getkeycodebig_from_scancode: optional method to retrieve current keymap
+ *	from an scancode. If not supplied default mechanism will be used.
+ *	The method is being called while holding event_lock and thus must
+ *      not sleep
  * @ff: force feedback structure associated with the device if device
  *	supports force feedback effects
  * @repeat_key: stores key code of the last key pressed; used to implement
@@ -1101,6 +1120,12 @@ struct input_dev {
 	void *keycode;
 	int (*setkeycode)(struct input_dev *dev, int scancode, int keycode);
 	int (*getkeycode)(struct input_dev *dev, int scancode, int *keycode);
+	int (*setkeycodebig)(struct input_dev *dev,
+			     struct keycode_table_entry *kt_entry);
+	int (*getkeycodebig_from_index)(struct input_dev *dev,
+			     struct keycode_table_entry *kt_entry);
+	int (*getkeycodebig_from_scancode)(struct input_dev *dev,
+			     struct keycode_table_entry *kt_entry);
 
 	struct ff_device *ff;
 
@@ -1366,6 +1391,11 @@ static inline void input_set_abs_params(struct input_dev *dev, int axis, int min
 
 int input_get_keycode(struct input_dev *dev, int scancode, int *keycode);
 int input_set_keycode(struct input_dev *dev, int scancode, int keycode);
+int input_get_keycode_big(struct input_dev *dev,
+			  struct keycode_table_entry *kt_entry);
+int input_set_keycode_big(struct input_dev *dev,
+			  struct keycode_table_entry *kt_entry);
+
 
 extern struct class input_class;
 
Signature · 1 line
1.6.6.1
#23 lore raw

PATCH 12/15 V4L/DVB: ir-core: rename sysfs remote controller class from ir to rc

In reply to a message not in the archive

IR is an alias for Infrared Remote, while RC is an alias for Remote Controller.

While currently all implementations are with Infrared Remote Controller, this subsystem is not meant to be used only by IR type of RC's. So, as discussed on both linux-media and linux-input, the better is to rename the subsystem as Remote Controller.

While, currently, the only application that uses the /sys/class/irrcv is ir-keytable application, and its sysfs support works only with the current linux-next code, it is still possible to change the userspace API without the risk of breaking applications. So, better to rename this sooner than later.

Later patches will be needed to rename the files and to move them away from drivers/media, but this is not a critical issue. So, for now, let's just change the name of the sysfs class/nodes.

drivers/media/IR/ir-sysfs.c +10 -10
index 2dbce59..435d83e 100644
@@ -21,14 +21,14 @@
 /* bit array to represent IR sysfs device number */
 static unsigned long ir_core_dev_number;
 
-/* class for /sys/class/irrcv */
+/* class for /sys/class/rc */
 static char *ir_devnode(struct device *dev, mode_t *mode)
 {
-	return kasprintf(GFP_KERNEL, "irrcv/%s", dev_name(dev));
+	return kasprintf(GFP_KERNEL, "rc/%s", dev_name(dev));
 }
 
 static struct class ir_input_class = {
-	.name		= "irrcv",
+	.name		= "rc",
 	.devnode	= ir_devnode,
 };
 
@@ -39,7 +39,7 @@ static struct class ir_input_class = {
  * @buf:	a pointer to the output buffer
  *
  * This routine is a callback routine for input read the IR protocol type.
- * it is trigged by reading /sys/class/irrcv/irrcv?/current_protocol.
+ * it is trigged by reading /sys/class/rc/rcrcv?/current_protocol.
  * It returns the protocol name, as understood by the driver.
  */
 static ssize_t show_protocol(struct device *d,
@@ -74,7 +74,7 @@ static ssize_t show_protocol(struct device *d,
  * @len:	length of the input buffer
  *
  * This routine is a callback routine for changing the IR protocol type.
- * it is trigged by reading /sys/class/irrcv/irrcv?/current_protocol.
+ * it is trigged by reading /sys/class/rc/rcrcv?/current_protocol.
  * It changes the IR the protocol name, if the IR type is recognized
  * by the driver.
  * If an unknown protocol name is used, returns -EINVAL.
@@ -171,7 +171,7 @@ static struct device_type ir_dev_type = {
 };
 
 /**
- * ir_register_class() - creates the sysfs for /sys/class/irrcv/irrcv?
+ * ir_register_class() - creates the sysfs for /sys/class/rc/rcrcv?
  * @input_dev:	the struct input_dev descriptor of the device
  *
  * This routine is used to register the syfs code for IR class
@@ -191,7 +191,7 @@ int ir_register_class(struct input_dev *input_dev)
 	ir_dev->dev.type = &ir_dev_type;
 	ir_dev->dev.class = &ir_input_class;
 	ir_dev->dev.parent = input_dev->dev.parent;
-	dev_set_name(&ir_dev->dev, "irrcv%d", devno);
+	dev_set_name(&ir_dev->dev, "rcrcv%d", devno);
 	dev_set_drvdata(&ir_dev->dev, ir_dev);
 	rc = device_register(&ir_dev->dev);
 	if (rc)
@@ -222,7 +222,7 @@ int ir_register_class(struct input_dev *input_dev)
 
 /**
  * ir_unregister_class() - removes the sysfs for sysfs for
- *			   /sys/class/irrcv/irrcv?
+ *			   /sys/class/rc/rcrcv?
  * @input_dev:	the struct input_dev descriptor of the device
  *
  * This routine is used to unregister the syfs code for IR class
@@ -239,14 +239,14 @@ void ir_unregister_class(struct input_dev *input_dev)
 }
 
 /*
- * Init/exit code for the module. Basically, creates/removes /sys/class/irrcv
+ * Init/exit code for the module. Basically, creates/removes /sys/class/rc
  */
 
 static int __init ir_core_init(void)
 {
 	int rc = class_register(&ir_input_class);
 	if (rc) {
-		printk(KERN_ERR "ir_core: unable to register irrcv class\n");
+		printk(KERN_ERR "ir_core: unable to register rc class\n");
 		return rc;
 	}
 
Signature · 1 line
1.6.6.1
#1 lore raw

PATCH 1/15 V4L/DVB: ir-core: be less pedantic with RC protocol name

In reply to a message not in the archive
drivers/media/IR/ir-sysfs.c +1 -1
index bbddf2f..ee6b36d 100644
@@ -92,7 +92,7 @@ static ssize_t store_protocol(struct device *d,
 
 	buf = strsep((char **) &data, "\n");
 
-	if (!strcasecmp(buf, "rc-5"))
+	if (!strcasecmp(buf, "rc-5") || !strcasecmp(buf, "rc5"))
 		ir_type = IR_TYPE_RC5;
 	else if (!strcasecmp(buf, "pd"))
 		ir_type = IR_TYPE_PD;
Signature · 1 line
1.6.6.1
#5 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

In reply to Mauro Carvalho Chehab
Quoted 4 lines · Mauro Carvalho Chehab

Adds a method to pass IR raw pulse/code events into ir-core. This is needed in order to support LIRC. It also helps to move common code from the drivers into the core.

Quoted 8 lines

In order to allow testing, it implements a simple NEC protocol decoder at ir-nec-decoder.c file. The logic is about the same used at saa7134 driver that handles Avermedia M135A and Encore FM53 boards.

 create mode 100644 drivers/media/IR/ir-nec-decoder.c
 create mode 100644 drivers/media/IR/ir-raw-event.c

Hi Mauro,

I haven't taken a very hard look at this since I'm very busy this month.

It looks OK so far. I do have some comments....

Quoted 26 lines
drivers/media/IR/Makefile +1 -1
index 171890e..18794c7 100644
@@ -1,5 +1,5 @@
 ir-common-objs  := ir-functions.o ir-keymaps.o
-ir-core-objs	:= ir-keytable.o ir-sysfs.o
+ir-core-objs	:= ir-keytable.o ir-sysfs.o ir-raw-event.o ir-nec-decoder.o
 
 obj-$(CONFIG_IR_CORE) += ir-core.o
 obj-$(CONFIG_VIDEO_IR) += ir-common.o
drivers/media/IR/ir-nec-decoder.c new file +9 -0
new file mode 100644
index 0000000..16360eb
@@ -0,0 +1,131 @@
+/* ir-raw-event.c - handle IR Pulse/Space event
+ *
+ * Copyright (C) 2010 by Mauro Carvalho Chehab <[email protected]>
+ *
+ * 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 version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,

drivers/media/IR/ir-nec-decoder.c line 19

Quoted 10 lines
drivers/media/IR/ir-nec-decoder.c new file +10 -0
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include <media/ir-core.h>
+
+/* Start time: 4.5 ms  */
+#define MIN_START_TIME	3900000
+#define MAX_START_TIME	5100000

Hmmm.

An NEC header pulse is nominally 16 * 560 us = 8.96 ms
An NEC header space is nominally 8 * 560 us = 4.48 ms
An NEC repeat header space is nominally 4 * 560 us = 2.24 ms

I think you need a more explicit name than {MIN,MAX}_START_TIME.

drivers/media/IR/ir-nec-decoder.c line 32

Quoted 12 lines
drivers/media/IR/ir-nec-decoder.c new file +12 -0
+/* Pulse time: 560 us  */
+#define MIN_PULSE_TIME	460000
+#define MAX_PULSE_TIME	660000
+
+/* Bit 1 space time: 2.25ms-560 us */
+#define MIN_BIT1_TIME	1490000
+#define MAX_BIT1_TIME	1890000
+
+/* Bit 0 space time: 1.12ms-560 us */
+#define MIN_BIT0_TIME	360000
+#define MAX_BIT0_TIME	760000
+

The fundamental unit of time in the NEC protocol is ideally:

	4192/197 cycles / 38 kHz = 559978.6 ns ~= 560 ns

All other time durations in the NEC protocol are multiples of this unit. See:

	http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/2cfef53b95a2#l1.96

If you define the your above constants in terms of that time unit, it makes the tolerances you added in explicitly visible when reading the source.

Quoted 25 lines
drivers/media/IR/ir-nec-decoder.c new file +25 -0
+/** Decode NEC pulsecode. This code can take up to 76.5 ms to run.
+	Unfortunately, using IRQ to decode pulse didn't work, since it uses
+	a pulse train of 38KHz. This means one pulse on each 52 us
+*/
+
+int ir_nec_decode(struct input_dev *input_dev,
+		  struct ir_raw_event *evs,
+		  int len)
+{
+	int i, count = -1;
+	int ircode = 0, not_code = 0;
+#if 0
+	/* Needed only after porting the event code to the decoder */
+	struct ir_input_dev *ir = input_get_drvdata(input_dev);
+#endif
+
+	/* Be sure that the first event is an start one and is a pulse */
+	for (i = 0; i < len; i++) {
+		if (evs[i].type & (IR_START_EVENT | IR_PULSE))
+			break;
+	}
+	i++;	/* First event doesn't contain data */
+
+	if (i >= len)
+		return 0;

drivers/media/IR/ir-nec-decoder.c line 68

Quoted 10 lines
drivers/media/IR/ir-nec-decoder.c new file +10 -0
+
+	/* First space should have 4.5 ms otherwise is not NEC protocol */
+	if ((evs[i].delta.tv_nsec < MIN_START_TIME) |
+	    (evs[i].delta.tv_nsec > MAX_START_TIME) |
+	    (evs[i].type != IR_SPACE))
+		goto err;
+
+	/*
+	 * FIXME: need to implement the repeat sequence
+	 */

I have an NEC protocol decoder here:

	http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/2cfef53b95a2

If you would find it useful, please feel free to borrow ideas or parts of the code to implement any features you are missing. (That code works by converting a mark-space pair to an "nec_symbol", and then taking action based on the symbol.)

I suspect you will want to implement the repeat sequence. It is hard not to get a repeat sequence from a remote.

NEC ideally sends a repeat at intervals of:

	4192 cycles * 38 kHz = 110.316 ms
Quoted 36 lines
drivers/media/IR/ir-nec-decoder.c new file +36 -0
+	count = 0;
+	for (i++; i < len; i++) {
+		int bit;
+
+		if ((evs[i].delta.tv_nsec < MIN_PULSE_TIME) |
+		    (evs[i].delta.tv_nsec > MAX_PULSE_TIME) |
+		    (evs[i].type != IR_PULSE))
+			goto err;
+
+		if (++i >= len)
+			goto err;
+		if (evs[i].type != IR_SPACE)
+			goto err;
+
+		if ((evs[i].delta.tv_nsec > MIN_BIT1_TIME) &&
+		    (evs[i].delta.tv_nsec < MAX_BIT1_TIME))
+			bit = 1;
+		else if ((evs[i].delta.tv_nsec > MIN_BIT0_TIME) &&
+			 (evs[i].delta.tv_nsec < MAX_BIT0_TIME))
+			bit = 0;
+		else
+			goto err;
+
+		if (bit) {
+			int shift = count;
+			/* Address first, then command */
+			if (shift < 8) {
+				shift += 8;
+				ircode |= 1 << shift;
+			} else if (shift < 16) {
+				not_code |= 1 << shift;
+			} else if (shift < 24) {
+				shift -= 16;
+				ircode |= 1 << shift;
+			} else {
+				shift -= 24;

drivers/media/IR/ir-nec-decoder.c line 115

Quoted 10 lines
drivers/media/IR/ir-nec-decoder.c new file +10 -0
+				not_code |= 1 << shift;
+			}
+		}
+		if (++count == 32)
+			break;
+	}
+
+	/*
+	 * Fixme: may need to accept Extended NEC protocol?
+	 */

Both of the NEC remotes that I own use the extended protocol, IIRC.

Quoted 350 lines
drivers/media/IR/ir-nec-decoder.c new file +16 -0
+	if ((ircode & ~not_code) != ircode) {
+		IR_dprintk(1, "NEC checksum error: code 0x%04x, not-code 0x%04x\n",
+			   ircode, not_code);
+		return -EINVAL;
+	}
+
+	IR_dprintk(1, "NEC scancode 0x%04x\n", ircode);
+
+	return ircode;
+err:
+	IR_dprintk(1, "NEC decoded failed at bit %d while decoding %luus time\n",
+		   count, (evs[i].delta.tv_nsec + 500) / 1000);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(ir_nec_decode);
drivers/media/IR/ir-raw-event.c new file +117 -0
new file mode 100644
index 0000000..9c71ac8
@@ -0,0 +1,117 @@
+/* ir-raw-event.c - handle IR Pulse/Space event
+ *
+ * Copyright (C) 2010 by Mauro Carvalho Chehab <[email protected]>
+ *
+ * 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 version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include <media/ir-core.h>
+
+/* Define the max number of bit transitions per IR keycode */
+#define MAX_IR_EVENT_SIZE	256
+
+int ir_raw_event_register(struct input_dev *input_dev)
+{
+	struct ir_input_dev *ir = input_get_drvdata(input_dev);
+	int rc, size;
+
+	ir->raw = kzalloc(sizeof(*ir->raw), GFP_KERNEL);
+
+	size = sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE * 2;
+	size = roundup_pow_of_two(size);
+
+	rc = kfifo_alloc(&ir->raw->kfifo, size, GFP_KERNEL);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(ir_raw_event_register);
+
+void ir_raw_event_unregister(struct input_dev *input_dev)
+{
+	struct ir_input_dev *ir = input_get_drvdata(input_dev);
+
+	if (!ir->raw)
+		return;
+
+	kfifo_free(&ir->raw->kfifo);
+	kfree(ir->raw);
+	ir->raw = NULL;
+}
+EXPORT_SYMBOL_GPL(ir_raw_event_unregister);
+
+int ir_raw_event_store(struct input_dev *input_dev, enum raw_event_type type)
+{
+	struct ir_input_dev	*ir = input_get_drvdata(input_dev);
+	struct timespec		ts;
+	struct ir_raw_event	event;
+	int			rc;
+
+	if (!ir->raw)
+		return -EINVAL;
+
+	event.type = type;
+	event.delta.tv_sec = 0;
+	event.delta.tv_nsec = 0;
+
+	ktime_get_ts(&ts);
+
+	if (timespec_equal(&ir->raw->last_event, &event.delta))
+		event.type |= IR_START_EVENT;
+	else
+		event.delta = timespec_sub(ts, ir->raw->last_event);
+
+	memcpy(&ir->raw->last_event, &ts, sizeof(ts));
+
+	if (event.delta.tv_sec) {
+		event.type |= IR_START_EVENT;
+		event.delta.tv_sec = 0;
+		event.delta.tv_nsec = 0;
+	}
+
+	kfifo_in(&ir->raw->kfifo, &event, sizeof(event));
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(ir_raw_event_store);
+
+int ir_raw_event_handle(struct input_dev *input_dev)
+{
+	struct ir_input_dev		*ir = input_get_drvdata(input_dev);
+	int				rc;
+	struct ir_raw_event		*evs;
+	int 				len, i;
+
+	/*
+	 * Store the events into a temporary buffer. This allows calling more than
+	 * one decoder to deal with the received data
+	 */
+	len = kfifo_len(&ir->raw->kfifo) / sizeof(*evs);
+	if (!len)
+		return 0;
+	evs = kmalloc(len * sizeof(*evs), GFP_ATOMIC);
+
+	for (i = 0; i < len; i++) {
+		rc = kfifo_out(&ir->raw->kfifo, &evs[i], sizeof(*evs));
+		if (rc != sizeof(*evs)) {
+			IR_dprintk(1, "overflow error: received %d instead of %zd\n",
+				   rc, sizeof(*evs));
+			return -EINVAL;
+		}
+		IR_dprintk(2, "event type %d, time before event: %07luus\n",
+			evs[i].type, (evs[i].delta.tv_nsec + 500) / 1000);
+	}
+
+	rc = ir_nec_decode(input_dev, evs, len);
+
+	kfree(evs);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(ir_raw_event_handle);
drivers/media/video/saa7134/saa7134-input.c +81 -4
index 8187928..7382995 100644
@@ -67,6 +67,7 @@ MODULE_PARM_DESC(disable_other_ir, "disable full codes of "
 /* Helper functions for RC5 and NEC decoding at GPIO16 or GPIO18 */
 static int saa7134_rc5_irq(struct saa7134_dev *dev);
 static int saa7134_nec_irq(struct saa7134_dev *dev);
+static int saa7134_raw_decode_irq(struct saa7134_dev *dev);
 static void nec_task(unsigned long data);
 static void saa7134_nec_timer(unsigned long data);
 
@@ -402,10 +403,12 @@ void saa7134_input_irq(struct saa7134_dev *dev)
 
 	if (ir->nec_gpio) {
 		saa7134_nec_irq(dev);
-	} else if (!ir->polling && !ir->rc5_gpio) {
+	} else if (!ir->polling && !ir->rc5_gpio && !ir->raw_decode) {
 		build_key(dev);
 	} else if (ir->rc5_gpio) {
 		saa7134_rc5_irq(dev);
+	} else if (ir->raw_decode) {
+		saa7134_raw_decode_irq(dev);
 	}
 }
 
@@ -418,6 +421,23 @@ static void saa7134_input_timer(unsigned long data)
 	mod_timer(&ir->timer, jiffies + msecs_to_jiffies(ir->polling));
 }
 
+void ir_raw_decode_timer_end(unsigned long data)
+{
+	struct saa7134_dev *dev = (struct saa7134_dev *)data;
+	struct card_ir *ir = dev->remote;
+	int rc;
+
+	/*
+	 * FIXME: the IR key handling code should be called by the decoder,
+	 * after implementing the repeat mode
+	 */
+	rc = ir_raw_event_handle(dev->remote->dev);
+	if (rc >= 0) {
+		ir_input_keydown(ir->dev, &ir->ir, rc);
+		ir_input_nokey(ir->dev, &ir->ir);
+	}
+}
+
 void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir)
 {
 	if (ir->running)
@@ -446,6 +466,11 @@ void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir)
 		setup_timer(&ir->timer_keyup, saa7134_nec_timer,
 			    (unsigned long)dev);
 		tasklet_init(&ir->tlet, nec_task, (unsigned long)dev);
+	} else if (ir->raw_decode) {
+		/* set timer_end for code completion */
+		init_timer(&ir->timer_end);
+		ir->timer_end.function = ir_raw_decode_timer_end;
+		ir->timer_end.data = (unsigned long)dev;
 	}
 }
 
@@ -461,6 +486,9 @@ void saa7134_ir_stop(struct saa7134_dev *dev)
 		del_timer_sync(&ir->timer_end);
 	else if (ir->nec_gpio)
 		tasklet_kill(&ir->tlet);
+	else if (ir->raw_decode)
+		del_timer_sync(&ir->timer_end);
+
 	ir->running = 0;
 }
 
@@ -508,6 +536,7 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	int polling      = 0;
 	int rc5_gpio	 = 0;
 	int nec_gpio	 = 0;
+	int raw_decode   = 0;
 	u64 ir_type = IR_TYPE_OTHER;
 	int err;
 
@@ -573,7 +602,7 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 		ir_codes     = &ir_codes_avermedia_m135a_rm_jx_table;
 		mask_keydown = 0x0040000;
 		mask_keycode = 0xffff;
-		nec_gpio     = 1;
+		raw_decode   = 1;
 		break;
 	case SAA7134_BOARD_AVERMEDIA_777:
 	case SAA7134_BOARD_AVERMEDIA_A16AR:
@@ -754,6 +783,7 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	ir->polling      = polling;
 	ir->rc5_gpio	 = rc5_gpio;
 	ir->nec_gpio	 = nec_gpio;
+	ir->raw_decode	 = raw_decode;
 
 	/* init input device */
 	snprintf(ir->name, sizeof(ir->name), "saa7134 IR (%s)",
@@ -761,7 +791,7 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	snprintf(ir->phys, sizeof(ir->phys), "pci-%s/ir0",
 		 pci_name(dev->pci));
 
-	if (ir_codes->ir_type != IR_TYPE_OTHER) {
+	if (ir_codes->ir_type != IR_TYPE_OTHER && !raw_decode) {
 		ir->props.allowed_protos = IR_TYPE_RC5 | IR_TYPE_NEC;
 		ir->props.priv = dev;
 		ir->props.change_protocol = saa7134_ir_change_protocol;
@@ -789,6 +819,11 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	err = ir_input_register(ir->dev, ir_codes, &ir->props, MODULE_NAME);
 	if (err)
 		goto err_out_stop;
+	if (ir_codes->ir_type != IR_TYPE_OTHER) {
+		err = ir_raw_event_register(ir->dev);
+		if (err)
+			goto err_out_stop;
+	}
 
 	saa7134_ir_start(dev, ir);
 
@@ -812,6 +847,7 @@ void saa7134_input_fini(struct saa7134_dev *dev)
 		return;
 
 	saa7134_ir_stop(dev);
+	ir_raw_event_unregister(dev->remote->dev);
 	ir_input_unregister(dev->remote->dev);
 	kfree(dev->remote);
 	dev->remote = NULL;
@@ -918,6 +954,48 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev)
 	i2c_new_device(&dev->i2c_adap, &info);
 }
 
+static int saa7134_raw_decode_irq(struct saa7134_dev *dev)
+{
+	struct card_ir	*ir = dev->remote;
+	unsigned long 	timeout;
+	int count, pulse, oldpulse;
+
+	/* Disable IR IRQ line */
+	saa_clearl(SAA7134_IRQ2, SAA7134_IRQ2_INTE_GPIO18);
+
+	/* Generate initial event */
+	saa_clearb(SAA7134_GPIO_GPMODE3, SAA7134_GPIO_GPRESCAN);
+	saa_setb(SAA7134_GPIO_GPMODE3, SAA7134_GPIO_GPRESCAN);
+	pulse = saa_readl(SAA7134_GPIO_GPSTATUS0 >> 2) & ir->mask_keydown;
+	ir_raw_event_store(dev->remote->dev, pulse? IR_PULSE : IR_SPACE);
+
+#if 1
+	/* Wait up to 10 ms for event change */
+	oldpulse = pulse;
+	for (count = 0; count < 1000; count++)  {
+		udelay(10);
+		/* rising SAA7134_GPIO_GPRESCAN reads the status */
+		saa_clearb(SAA7134_GPIO_GPMODE3, SAA7134_GPIO_GPRESCAN);
+		saa_setb(SAA7134_GPIO_GPMODE3, SAA7134_GPIO_GPRESCAN);
+		pulse = saa_readl(SAA7134_GPIO_GPSTATUS0 >> 2)
+			& ir->mask_keydown;
+		if (pulse != oldpulse)
+			break;
+	}
+
+	/* Store final event */
+	ir_raw_event_store(dev->remote->dev, pulse? IR_PULSE : IR_SPACE);
+#endif
+	/* Wait 15 ms before deciding to do something else */
+	timeout = jiffies + jiffies_to_msecs(15);
+	mod_timer(&ir->timer_end, timeout);
+
+	/* Enable IR IRQ line */
+	saa_setl(SAA7134_IRQ2, SAA7134_IRQ2_INTE_GPIO18);
+
+	return 1;
+}
+
 static int saa7134_rc5_irq(struct saa7134_dev *dev)
 {
 	struct card_ir *ir = dev->remote;
@@ -960,7 +1038,6 @@ static int saa7134_rc5_irq(struct saa7134_dev *dev)
 	return 1;
 }
 
-
 /* On NEC protocol, One has 2.25 ms, and zero has 1.125 ms
    The first pulse (start) has 9 + 4.5 ms
  */
include/media/ir-common.h +3 -0
index 41469b7..87f2ec7 100644
@@ -82,6 +82,9 @@ struct card_ir {
 	/* NEC decoding */
 	u32			nec_gpio;
 	struct tasklet_struct   tlet;
+
+	/* IR core raw decoding */
+	u32			raw_decode;
 };
 
 /* Routines from ir-functions.c */
include/media/ir-core.h +2 -0
index 1eae72d..369969d 100644
@@ -16,6 +16,8 @@
 
 #include <linux/input.h>
 #include <linux/spinlock.h>
+#include <linux/kfifo.h>
+#include <linux/time.h>
 
 extern int ir_core_debug;
 #define IR_dprintk(level, fmt, arg...)	if (ir_core_debug >= level) \

include/media/ir-core.h extern int ir_core_debug; · line 38

Quoted 10 lines
include/media/ir-core.h +7 -0
@@ -27,6 +29,13 @@ extern int ir_core_debug;
 #define IR_TYPE_NEC	(1  << 2)
 #define IR_TYPE_OTHER	(((u64)1) << 63l)
 
+enum raw_event_type {
+	IR_SPACE	= (1 << 0),
+	IR_PULSE	= (1 << 1),
+	IR_START_EVENT	= (1 << 2),
+	IR_STOP_EVENT	= (1 << 3),
+};
+

Why are these events encoded as bit flags? Shouldn't they all be orthogonal?

Regards,
Andy

Quoted 46 lines
include/media/ir-core.h +23 -0
 struct ir_scancode {
 	u16	scancode;
 	u32	keycode;
@@ -46,6 +55,15 @@ struct ir_dev_props {
 	int (*change_protocol)(void *priv, u64 ir_type);
 };
 
+struct ir_raw_event {
+	struct timespec		delta;	/* Time spent before event */
+	enum raw_event_type	type;	/* event type */
+};
+
+struct ir_raw_event_ctrl {
+	struct kfifo			kfifo;		/* fifo for the pulse/space events */
+	struct timespec			last_event;	/* when last event occurred */
+};
 
 struct ir_input_dev {
 	struct device			dev;		/* device */
@@ -53,7 +71,9 @@ struct ir_input_dev {
 	struct ir_scancode_table	rc_tab;		/* scan/key table */
 	unsigned long			devno;		/* device number */
 	const struct ir_dev_props	*props;		/* Device properties */
+	struct ir_raw_event_ctrl	*raw;		/* for raw pulse/space events */
 };
+
 #define to_ir_input_dev(_attr) container_of(_attr, struct ir_input_dev, attr)
 
 /* Routines from ir-keytable.c */
@@ -72,4 +92,16 @@ void ir_input_unregister(struct input_dev *input_dev);
 int ir_register_class(struct input_dev *input_dev);
 void ir_unregister_class(struct input_dev *input_dev);
 
+/* Routines from ir-raw-event.c */
+int ir_raw_event_register(struct input_dev *input_dev);
+void ir_raw_event_unregister(struct input_dev *input_dev);
+int ir_raw_event_store(struct input_dev *input_dev, enum raw_event_type type);
+int ir_raw_event_handle(struct input_dev *input_dev);
+
+/* from ir-nec-decoder.c */
+int ir_nec_decode(struct input_dev *input_dev,
+		  struct ir_raw_event *evs,
+		  int len);
+
+
 #endif
#6 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

Quoted 12 lines · Andy Walls

include/media/ir-core.h extern int ir_core_debug; · line 38

Quoted 7 lines · Mauro Carvalho Chehab
include/media/ir-core.h +7 -0
+enum raw_event_type {
+	IR_SPACE	= (1 << 0),
+	IR_PULSE	= (1 << 1),
+	IR_START_EVENT	= (1 << 2),
+	IR_STOP_EVENT	= (1 << 3),
+};
+

Why are these events encoded as bit flags? Shouldn't they all be orthogonal?

^^^^^^^^^^
Argh, wrong word.

Shouldn't they all be mutually exclusive?

Regards,
Andy

#7 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

Quoted 12 lines · Andy Walls
Quoted 11 lines · Andy Walls

include/media/ir-core.h extern int ir_core_debug; · line 38

Quoted 7 lines · Mauro Carvalho Chehab
include/media/ir-core.h +7 -0
+enum raw_event_type {
+	IR_SPACE	= (1 << 0),
+	IR_PULSE	= (1 << 1),
+	IR_START_EVENT	= (1 << 2),
+	IR_STOP_EVENT	= (1 << 3),
+};
+

Why are these events encoded as bit flags? Shouldn't they all be orthogonal?

Quoted 2 lines

^^^^^^^^^^
Argh, wrong word.

Why is it wrong? It seems appropriate to me.

Quoted 2 lines

Shouldn't they all be mutually exclusive?

space x pulse are mutually exclusive, and start x stop are also mutually exclusive, but you may have several possible combinations for an event. The hole set of possibilities are:

IR_SPACE
IR_PULSE

IR_SPACE | IR_START_EVENT
IR_SPACE | IR_STOP_EVENT
IR_PULSE | IR_START_EVENT
IR_PULSE | IR_STOP_EVENT

With bit flags, it is possible to cover all the above combinations.

In a matter of fact, the driver is currently not using the stop events.

Signature · 2 lines
Cheers,
Mauro
#8 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

In reply to Andy Walls
Quoted 11 lines · Andy Walls
Quoted 10 lines · Mauro Carvalho Chehab

Adds a method to pass IR raw pulse/code events into ir-core. This is needed in order to support LIRC. It also helps to move common code from the drivers into the core.

In order to allow testing, it implements a simple NEC protocol decoder at ir-nec-decoder.c file. The logic is about the same used at saa7134 driver that handles Avermedia M135A and Encore FM53 boards.

Quoted 8 lines
Quoted 2 lines
 create mode 100644 drivers/media/IR/ir-nec-decoder.c
 create mode 100644 drivers/media/IR/ir-raw-event.c

Hi Mauro,

I haven't taken a very hard look at this since I'm very busy this month.

It looks OK so far.

Thank you for your review.

One general comment: my main target of writing the NEC decoder is to have one decoder for testing. I know that there are several other implementations, so I didn't try to write a perfect decoder, but, instead, I wrote a code that works, and that allows me to continue the Remote Controller subsystem design. In other words, for sure there are lots of space for improvements there ;)

Quoted 42 lines

I do have some comments....

Quoted 26 lines
drivers/media/IR/Makefile +1 -1
index 171890e..18794c7 100644
@@ -1,5 +1,5 @@
 ir-common-objs  := ir-functions.o ir-keymaps.o
-ir-core-objs	:= ir-keytable.o ir-sysfs.o
+ir-core-objs	:= ir-keytable.o ir-sysfs.o ir-raw-event.o ir-nec-decoder.o
 
 obj-$(CONFIG_IR_CORE) += ir-core.o
 obj-$(CONFIG_VIDEO_IR) += ir-common.o
drivers/media/IR/ir-nec-decoder.c new file +9 -0
new file mode 100644
index 0000000..16360eb
@@ -0,0 +1,131 @@
+/* ir-raw-event.c - handle IR Pulse/Space event
+ *
+ * Copyright (C) 2010 by Mauro Carvalho Chehab <[email protected]>
+ *
+ * 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 version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,

drivers/media/IR/ir-nec-decoder.c line 19

Quoted 10 lines
drivers/media/IR/ir-nec-decoder.c new file +10 -0
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include <media/ir-core.h>
+
+/* Start time: 4.5 ms  */
+#define MIN_START_TIME	3900000
+#define MAX_START_TIME	5100000

Hmmm.

Quoted 5 lines

An NEC header pulse is nominally 16 * 560 us = 8.96 ms
An NEC header space is nominally 8 * 560 us = 4.48 ms
An NEC repeat header space is nominally 4 * 560 us = 2.24 ms

I think you need a more explicit name than {MIN,MAX}_START_TIME.

Part of the problem with this decoder is that it was conceived to work with the saa7134 driver. The driver is currently programmed to trigger IRQ on just one of the edge (positive or negative, I need to double check). Due to that, this time is just half of the time it should be.

I've changed on a latter patch my decoder to work with just the duration of the bits.

After reviewing the datasheet, now I think I know how to program IRQ to trigger on both edges. So, my idea is to enable it and rewrite the decoder.

Quoted 17 lines
Quoted 4 lines
drivers/media/IR/ir-nec-decoder.c new file +2 -0
+/* Pulse time: 560 us  */
+#define MIN_PULSE_TIME	460000

drivers/media/IR/ir-nec-decoder.c line 32

Quoted 10 lines
drivers/media/IR/ir-nec-decoder.c new file +10 -0
+#define MAX_PULSE_TIME	660000
+
+/* Bit 1 space time: 2.25ms-560 us */
+#define MIN_BIT1_TIME	1490000
+#define MAX_BIT1_TIME	1890000
+
+/* Bit 0 space time: 1.12ms-560 us */
+#define MIN_BIT0_TIME	360000
+#define MAX_BIT0_TIME	760000
+

The fundamental unit of time in the NEC protocol is ideally:

Quoted 3 lines
	4192/197 cycles / 38 kHz = 559978.6 ns ~= 560 ns

All other time durations in the NEC protocol are multiples of this unit.

Yes, I know. By max/min, I've meant to handle delta variations around the main time, since the driver may miss the exact moment where it were supposed to collect the timestamp.

Quoted 7 lines

See:

	http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/2cfef53b95a2#l1.96

If you define the your above constants in terms of that time unit, it makes the tolerances you added in explicitly visible when reading the source.

I'll take a look on your code and work to improve this decoder. The way you've declared is for sure cleaner than mine.

Quoted 38 lines
Quoted 25 lines
drivers/media/IR/ir-nec-decoder.c new file +25 -0
+/** Decode NEC pulsecode. This code can take up to 76.5 ms to run.
+	Unfortunately, using IRQ to decode pulse didn't work, since it uses
+	a pulse train of 38KHz. This means one pulse on each 52 us
+*/
+
+int ir_nec_decode(struct input_dev *input_dev,
+		  struct ir_raw_event *evs,
+		  int len)
+{
+	int i, count = -1;
+	int ircode = 0, not_code = 0;
+#if 0
+	/* Needed only after porting the event code to the decoder */
+	struct ir_input_dev *ir = input_get_drvdata(input_dev);
+#endif
+
+	/* Be sure that the first event is an start one and is a pulse */
+	for (i = 0; i < len; i++) {
+		if (evs[i].type & (IR_START_EVENT | IR_PULSE))
+			break;
+	}
+	i++;	/* First event doesn't contain data */
+
+	if (i >= len)
+		return 0;

drivers/media/IR/ir-nec-decoder.c line 68

Quoted 10 lines
drivers/media/IR/ir-nec-decoder.c new file +10 -0
+
+	/* First space should have 4.5 ms otherwise is not NEC protocol */
+	if ((evs[i].delta.tv_nsec < MIN_START_TIME) |
+	    (evs[i].delta.tv_nsec > MAX_START_TIME) |
+	    (evs[i].type != IR_SPACE))
+		goto err;
+
+	/*
+	 * FIXME: need to implement the repeat sequence
+	 */

I have an NEC protocol decoder here:

Quoted 6 lines
	http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/2cfef53b95a2

If you would find it useful, please feel free to borrow ideas or parts of the code to implement any features you are missing. (That code works by converting a mark-space pair to an "nec_symbol", and then taking action based on the symbol.)

Ok, I will take a look on it.

Quoted 7 lines

I suspect you will want to implement the repeat sequence. It is hard not to get a repeat sequence from a remote.

NEC ideally sends a repeat at intervals of:

	4192 cycles * 38 kHz = 110.316 ms

I suspect that the original RC shipped with this board doesn't produce a repeat event. I have another IR here that produces. I'll double test and see if I can improve the repeat code (a latter patch implements a repeat code).

Quoted 41 lines
Quoted 41 lines
drivers/media/IR/ir-nec-decoder.c new file +40 -0
+	count = 0;
+	for (i++; i < len; i++) {
+		int bit;
+
+		if ((evs[i].delta.tv_nsec < MIN_PULSE_TIME) |
+		    (evs[i].delta.tv_nsec > MAX_PULSE_TIME) |
+		    (evs[i].type != IR_PULSE))
+			goto err;
+
+		if (++i >= len)
+			goto err;
+		if (evs[i].type != IR_SPACE)
+			goto err;
+
+		if ((evs[i].delta.tv_nsec > MIN_BIT1_TIME) &&
+		    (evs[i].delta.tv_nsec < MAX_BIT1_TIME))
+			bit = 1;
+		else if ((evs[i].delta.tv_nsec > MIN_BIT0_TIME) &&
+			 (evs[i].delta.tv_nsec < MAX_BIT0_TIME))
+			bit = 0;
+		else
+			goto err;
+
+		if (bit) {
+			int shift = count;
+			/* Address first, then command */
+			if (shift < 8) {
+				shift += 8;
+				ircode |= 1 << shift;
+			} else if (shift < 16) {
+				not_code |= 1 << shift;
+			} else if (shift < 24) {
+				shift -= 16;
+				ircode |= 1 << shift;
+			} else {
+				shift -= 24;
+				not_code |= 1 << shift;
+			}
+		}
+		if (++count == 32)
Quoted 8 lines

drivers/media/IR/ir-nec-decoder.c line 115

Quoted 6 lines
drivers/media/IR/ir-nec-decoder.c new file +6 -0
+			break;
+	}
+
+	/*
+	 * Fixme: may need to accept Extended NEC protocol?
+	 */

Both of the NEC remotes that I own use the extended protocol, IIRC.

I found one NEC IR here that uses the extended protocol. The issue I have here is that maybe it could be interesting to allow enable or disable a more pedantic check. At least on the room I'm working, I have two strong fluorescent lamps that interfere on the IR sensor of the saa7134 board. I'll probably add a sysfs node to allow enable/ disable the strict check for non-extended protocol.

Signature · 2 lines
Cheers,
Mauro
#9 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

Quoted 4 lines · Mauro Carvalho Chehab
Quoted 2 lines · Andy Walls

I haven't taken a very hard look at this since I'm very busy this month.

Quoted 8 lines
Quoted 1 line

It looks OK so far.

Thank you for your review.

One general comment: my main target of writing the NEC decoder is to have one decoder for testing. I know that there are several other implementations, so I didn't try to write a perfect decoder, but, instead, I wrote a code that works, and that allows me to continue the Remote Controller subsystem design.

Understood.

Quoted 10 lines
Quoted 10 lines

drivers/media/IR/ir-nec-decoder.c line 19

Quoted 3 lines
drivers/media/IR/ir-nec-decoder.c new file +3 -0
+/* Start time: 4.5 ms  */
+#define MIN_START_TIME	3900000
+#define MAX_START_TIME	5100000

Hmmm.

An NEC header pulse is nominally 16 * 560 us = 8.96 ms
An NEC header space is nominally 8 * 560 us = 4.48 ms
An NEC repeat header space is nominally 4 * 560 us = 2.24 ms

Quoted 6 lines
Quoted 1 line

I think you need a more explicit name than {MIN,MAX}_START_TIME.

Part of the problem with this decoder is that it was conceived to work with the saa7134 driver. The driver is currently programmed to trigger IRQ on just one of the edge (positive or negative, I need to double check). Due to that, this time is just half of the time it should be.

Hmm. It is the right time duration for the space in a normal, non-repeat, header.

Quoted 5 lines

I've changed on a latter patch my decoder to work with just the duration of the bits.

After reviewing the datasheet, now I think I know how to program IRQ to trigger on both edges. So, my idea is to enable it and rewrite the decoder.

So that brings up and interesting decoder design requirement.

Is it the case that some drivers will only be able to perform leading edge detection (measuring time between marks) or trailing edge detection (measuring time between spaces)?

Quoted 15 lines
Quoted 15 lines

drivers/media/IR/ir-nec-decoder.c line 32

Quoted 12 lines
drivers/media/IR/ir-nec-decoder.c new file +12 -0
+/* Pulse time: 560 us  */
+#define MIN_PULSE_TIME	460000
+#define MAX_PULSE_TIME	660000
+
+/* Bit 1 space time: 2.25ms-560 us */
+#define MIN_BIT1_TIME	1490000
+#define MAX_BIT1_TIME	1890000
+
+/* Bit 0 space time: 1.12ms-560 us */
+#define MIN_BIT0_TIME	360000
+#define MAX_BIT0_TIME	760000
+

The fundamental unit of time in the NEC protocol is ideally:

Quoted 7 lines
Quoted 3 lines
	4192/197 cycles / 38 kHz = 559978.6 ns ~= 560 ns

All other time durations in the NEC protocol are multiples of this unit.

Yes, I know. By max/min, I've meant to handle delta variations around the main time, since the driver may miss the exact moment where it were supposed to collect the timestamp.

Yes, and the remote's oscillator can be pretty far off for the ideal timing too.

Quoted 7 lines
Quoted 1 line

Both of the NEC remotes that I own use the extended protocol, IIRC.

I found one NEC IR here that uses the extended protocol. The issue I have here is that maybe it could be interesting to allow enable or disable a more pedantic check. At least on the room I'm working, I have two strong fluorescent lamps that interfere on the IR sensor of the saa7134 board. I'll probably add a sysfs node to allow enable/ disable the strict check for non-extended protocol.

Would that make the setting global or would it be on a per remote control basis?

The way I handled extended NEC addressing or stardard NEC addressing was implicit given the specified expected remote control address.

For setting the adress for which to watch I did something like

	if ((specified_address & 0xff00) == 0) {
		/* Store the 8 bit NEC address in 16 bits as A'A */
		ir_input->addr = (specified_address ^ 0xff) << 8 |
				  specified_address;
	} else {
		/* Store a 16 bit Extended NEC address directly */
		ir_input->addr = specified_address;
	}

And then when checking the incoming remote code:

	if (ir_input->addr != decoded_addr)
		return;

The requirement for proper bit complement of an 8-bit address was encoded in the expected 16-bit address. So if what it decoded as an address didn't match the expected 16 bits, it discarded the code.

I don't know if a driver or end user can set the expectaion of a remote control's NEC address in your recent design (or if the intent was to not require it and use discovery).

Regards,
Andy

#10 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

Quoted 6 lines · Andy Walls
Quoted 5 lines · Mauro Carvalho Chehab
Quoted 3 lines · Andy Walls

I haven't taken a very hard look at this since I'm very busy this month.

It looks OK so far.

Quoted 8 lines
Quoted 6 lines

Thank you for your review.

One general comment: my main target of writing the NEC decoder is to have one decoder for testing. I know that there are several other implementations, so I didn't try to write a perfect decoder, but, instead, I wrote a code that works, and that allows me to continue the Remote Controller subsystem design.

Understood.

Btw, I just finish rewriting the nec decoder:

http://git.linuxtv.org/mchehab/ir.git?a=blob;f=drivers/media/IR/ir-nec-decoder.c;h=33b260f517f509efd9c55067eb89c8cd748ed12c;hb=09b1808271b3d705b839ee3239fd1c85b7289f41

I've got your constants and a few of your ideas, but the code is different:
instead of getting a pulse/mark pair, the code handles event by event. Also, it is controlled by a state machine.

The end result is that the code seems very reliable. It also handles both NEC and NEC extended.

(btw, I just noticed that I named the bit0/bit1 symbols wrong - the timings there are just for space - I'll write a patch fixing it).

Quoted 11 lines
Quoted 5 lines

I've changed on a latter patch my decoder to work with just the duration of the bits.

After reviewing the datasheet, now I think I know how to program IRQ to trigger on both edges. So, my idea is to enable it and rewrite the decoder.

So that brings up and interesting decoder design requirement.

Is it the case that some drivers will only be able to perform leading edge detection (measuring time between marks) or trailing edge detection (measuring time between spaces)?

In the specific case of saa7134, the IRQ can be enabled for a positive and/or for a negative edge.

I'm not sure about the other IRQ driven hardware. On cx88, there's no IRQ code for IR - maybe it is not supported. I haven't check yet how IRQ's work on bttv. I've no idea about the other devices that support raw IR decoding.

Quoted 6 lines
Quoted 3 lines

Yes, I know. By max/min, I've meant to handle delta variations around the main time, since the driver may miss the exact moment where it were supposed to collect the timestamp.

Yes, and the remote's oscillator can be pretty far off for the ideal timing too.

I noticed.

Quoted 9 lines
Quoted 6 lines
Quoted 1 line

Both of the NEC remotes that I own use the extended protocol, IIRC.

I found one NEC IR here that uses the extended protocol. The issue I have here is that maybe it could be interesting to allow enable or disable a more pedantic check. At least on the room I'm working, I have two strong fluorescent lamps that interfere on the IR sensor of the saa7134 board. I'll probably add a sysfs node to allow enable/ disable the strict check for non-extended protocol.

Would that make the setting global or would it be on a per remote control basis?

The protocol sysfs nodes are per device. Yet, for now, I haven't created such node.

Quoted 5 lines

The way I handled extended NEC addressing or stardard NEC addressing was implicit given the specified expected remote control address.

For setting the adress for which to watch I did something like

Quoted 8 lines
	if ((specified_address & 0xff00) == 0) {
		/* Store the 8 bit NEC address in 16 bits as A'A */
		ir_input->addr = (specified_address ^ 0xff) << 8 |
				  specified_address;
	} else {
		/* Store a 16 bit Extended NEC address directly */
		ir_input->addr = specified_address;
	}

Seems ok to me. I've used the same concept on my code.

Quoted 5 lines

And then when checking the incoming remote code:

	if (ir_input->addr != decoded_addr)
		return;
Quoted 8 lines

The requirement for proper bit complement of an 8-bit address was encoded in the expected 16-bit address. So if what it decoded as an address didn't match the expected 16 bits, it discarded the code.

I don't know if a driver or end user can set the expectaion of a remote control's NEC address in your recent design (or if the intent was to not require it and use discovery).

Well, bet to let it as-is. If later needed, it would be easy to add a sysfs node parameter to control it.

Signature · 2 lines
Cheers,
Mauro
#11 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

Quoted 18 lines · Mauro Carvalho Chehab
Quoted 6 lines · Andy Walls
Quoted 5 lines · Mauro Carvalho Chehab
Quoted 3 lines · Andy Walls

I haven't taken a very hard look at this since I'm very busy this month.

It looks OK so far.

Quoted 8 lines
Quoted 6 lines

Thank you for your review.

One general comment: my main target of writing the NEC decoder is to have one decoder for testing. I know that there are several other implementations, so I didn't try to write a perfect decoder, but, instead, I wrote a code that works, and that allows me to continue the Remote Controller subsystem design.

Understood.

Btw, I just finish rewriting the nec decoder:

Quoted 8 lines

http://git.linuxtv.org/mchehab/ir.git?a=blob;f=drivers/media/IR/ir-nec-decoder.c;h=33b260f517f509efd9c55067eb89c8cd748ed12c;hb=09b1808271b3d705b839ee3239fd1c85b7289f41

I've got your constants and a few of your ideas, but the code is different:
instead of getting a pulse/mark pair, the code handles event by event. Also, it is controlled by a state machine.

The end result is that the code seems very reliable. It also handles both NEC and NEC extended.

Looks good.

On intervals that are supposed to be longer than 1 NEC_UNIT, the tolerance of NEC_UNIT / 2 is a little unforgiving. However, if this decoder is called not knowing apriori that the protocol is NEC, it is probably the right thing to do.

And when you have time:

I think all that is missing is a glitch (low pass) filter to discard pulses much shorter than 1 NEC_UNIT. A way to generate random IR glitches is with bright sunlight reflecting off of a basin of water that's surface is being disturbed to make waves.

LIRC has a software glitch filter implementation in

	lirc-0.8.5/drivers/lirc_serial/lirc_serial.c:frbwrite()

but it's not the simplest code to understand and it keeps its state in static variables in the function.

(My kernel NEC decoder implementation didn't have a software glitch filter, because there was a filter provided by the hardware. For NEC, I decided to discard any pulse less than 5/8 * NEC_UNIT. For RC-5, I set it to discard pulses less than 3/4 of a pulse time.)

Since a glitch filter is probably going to be needed by a number of drivers and since the minimum acceptable pulse depends slightly on the protocol, it probably makes sense for

1. A driver to indicate if its raw events need glitch filtering

2. A common glitch filtering library function that can be used by all decoders, and that also can accept a decoder specified minimum acceptable pulse width.

Quoted 10 lines
Quoted 3 lines

Is it the case that some drivers will only be able to perform leading edge detection (measuring time between marks) or trailing edge detection (measuring time between spaces)?

In the specific case of saa7134, the IRQ can be enabled for a positive and/or for a negative edge.

I'm not sure about the other IRQ driven hardware. On cx88, there's no IRQ code for IR - maybe it is not supported. I haven't check yet how IRQ's work on bttv. I've no idea about the other devices that support raw IR decoding.

I can look into what the cx88 and bttv chips can do. How the boards are wired up is a different issue.

Quoted 11 lines
Quoted 8 lines
Quoted 5 lines

I found one NEC IR here that uses the extended protocol. The issue I have here is that maybe it could be interesting to allow enable or disable a more pedantic check. At least on the room I'm working, I have two strong fluorescent lamps that interfere on the IR sensor of the saa7134 board. I'll probably add a sysfs node to allow enable/ disable the strict check for non-extended protocol.

Would that make the setting global or would it be on a per remote control basis?

The protocol sysfs nodes are per device. Yet, for now, I haven't created such node.

Per device means per IR receiver device?

Quoted 6 lines
Quoted 3 lines

I don't know if a driver or end user can set the expectaion of a remote control's NEC address in your recent design (or if the intent was to not require it and use discovery).

Well, bet to let it as-is. If later needed, it would be easy to add a sysfs node parameter to control it.

OK.

Regards,
Andy

#12 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

Quoted 26 lines · Andy Walls
Quoted 16 lines · Mauro Carvalho Chehab
Quoted 5 lines · Andy Walls
Quoted 4 lines · Mauro Carvalho Chehab
Quoted 2 lines · Andy Walls

I haven't taken a very hard look at this since I'm very busy this month.

Quoted 8 lines
Quoted 7 lines
Quoted 1 line

It looks OK so far.

Thank you for your review.

One general comment: my main target of writing the NEC decoder is to have one decoder for testing. I know that there are several other implementations, so I didn't try to write a perfect decoder, but, instead, I wrote a code that works, and that allows me to continue the Remote Controller subsystem design.

Understood.

Btw, I just finish rewriting the nec decoder:

Quoted 8 lines

http://git.linuxtv.org/mchehab/ir.git?a=blob;f=drivers/media/IR/ir-nec-decoder.c;h=33b260f517f509efd9c55067eb89c8cd748ed12c;hb=09b1808271b3d705b839ee3239fd1c85b7289f41

I've got your constants and a few of your ideas, but the code is different:
instead of getting a pulse/mark pair, the code handles event by event. Also, it is controlled by a state machine.

The end result is that the code seems very reliable. It also handles both NEC and NEC extended.

Quoted 6 lines

Looks good.

On intervals that are supposed to be longer than 1 NEC_UNIT, the tolerance of NEC_UNIT / 2 is a little unforgiving. However, if this decoder is called not knowing apriori that the protocol is NEC, it is probably the right thing to do.

Yes. That's why the code is a little rigid. We want to reduce the risk of badly decoded codes.

Quoted 7 lines

And when you have time:

I think all that is missing is a glitch (low pass) filter to discard pulses much shorter than 1 NEC_UNIT. A way to generate random IR glitches is with bright sunlight reflecting off of a basin of water that's surface is being disturbed to make waves.

I have a better way: just let my IR sensor to be pointed to the fluorescent lamp I have on my room... It produces _lots_ of glitches.

Quoted 17 lines

LIRC has a software glitch filter implementation in

	lirc-0.8.5/drivers/lirc_serial/lirc_serial.c:frbwrite()

but it's not the simplest code to understand and it keeps its state in static variables in the function.

(My kernel NEC decoder implementation didn't have a software glitch filter, because there was a filter provided by the hardware. For NEC, I decided to discard any pulse less than 5/8 * NEC_UNIT. For RC-5, I set it to discard pulses less than 3/4 of a pulse time.)

Since a glitch filter is probably going to be needed by a number of drivers and since the minimum acceptable pulse depends slightly on the protocol, it probably makes sense for

Quoted 5 lines

1. A driver to indicate if its raw events need glitch filtering

2. A common glitch filtering library function that can be used by all decoders, and that also can accept a decoder specified minimum acceptable pulse width.

Seems a nice improvement. I doubt I'll have time for handling it right now, since there are still many things to do, but I'll put it on my todo list. Of course, patches adding it are wellcome ;)

Btw, I added a RC-5 decoder there, at my IR experimental tree:
http://git.linuxtv.org/mchehab/ir.git

Unfortunately, there's some problem with either my Remote Controller or with the saa7134 driver. After 11 bits received, after the 2 start bits, it receives a pause (see the enclosed sequence).

I'm starting to suspect that the Hauppauge Grey IR produces a sequence with shorter bits, but, as the hardware decoders are capable or receiving IR codes, it may also be a hardware problem.

Quoted 8 lines
Quoted 8 lines
Quoted 5 lines

Is it the case that some drivers will only be able to perform leading edge detection (measuring time between marks) or trailing edge detection (measuring time between spaces)?

In the specific case of saa7134, the IRQ can be enabled for a positive and/or for a negative edge.

Quoted 6 lines
Quoted 3 lines

I'm not sure about the other IRQ driven hardware. On cx88, there's no IRQ code for IR - maybe it is not supported. I haven't check yet how IRQ's work on bttv. I've no idea about the other devices that support raw IR decoding.

I can look into what the cx88 and bttv chips can do. How the boards are wired up is a different issue.

Thanks. Unfortunately, the cx88 devices I have here in hand have hardware IR decoders, or no IR support. I haven't check yet the bttv devices.

Btw, by playing with one of them I got one that has a broken^Hincomplete decoding support: It validates the address internally, and returns only the command, if the address is the expected one.

Quoted 27 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

I found one NEC IR here that uses the extended protocol. The issue I have here is that maybe it could be interesting to allow enable or disable a more pedantic check. At least on the room I'm working, I have two strong fluorescent lamps that interfere on the IR sensor of the saa7134 board. I'll probably add a sysfs node to allow enable/ disable the strict check for non-extended protocol.

Quoted 4 lines
Quoted 2 lines

Would that make the setting global or would it be on a per remote control basis?

The protocol sysfs nodes are per device. Yet, for now, I haven't created such node.

Per device means per IR receiver device?

Quoted 5 lines
Quoted 3 lines

I don't know if a driver or end user can set the expectaion of a remote control's NEC address in your recent design (or if the intent was to not require it and use discovery).

Well, bet to let it as-is. If later needed, it would be easy to add a sysfs node parameter to control it.

OK.

Regards,
Andy

Signature · 2 lines
Cheers,
Mauro
#13 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

Quoted 10 lines · Mauro Carvalho Chehab
Quoted 5 lines · Andy Walls

And when you have time:

A way to generate random IR
glitches is with bright sunlight reflecting off of a basin of water that's surface is being disturbed to make waves.

I have a better way: just let my IR sensor to be pointed to the fluorescent lamp I have on my room... It produces _lots_ of glitches.

:)

Quoted 6 lines
Quoted 6 lines

Since a glitch filter is probably going to be needed by a number of drivers and since the minimum acceptable pulse depends slightly on the protocol, it probably makes sense for

1. A driver to indicate if its raw events need glitch filtering

Quoted 7 lines
Quoted 3 lines

2. A common glitch filtering library function that can be used by all decoders, and that also can accept a decoder specified minimum acceptable pulse width.

Seems a nice improvement. I doubt I'll have time for handling it right now, since there are still many things to do, but I'll put it on my todo list. Of course, patches adding it are wellcome ;)

:)

OK. When I find time I'll hack something up as a prototype.

Quoted 2 lines

Btw, I added a RC-5 decoder there, at my IR experimental tree:
http://git.linuxtv.org/mchehab/ir.git

I'll try to review it some time this week. Streaming state machine decoders do seem to be best way to go with these decoders.

I have an RC-5 decoder in cx23885-input.c that isn't as clean as the NEC protocol decoder I developed. The cx23885-input.c RC-5 decoder is not a very explicit state machine however (it is a bit hack-ish).

Quoted 3 lines

Unfortunately, there's some problem with either my Remote Controller or with the saa7134 driver. After 11 bits received, after the 2 start bits, it receives a pause (see the enclosed sequence).

-ENOATTACHMENT

Quoted 3 lines

I'm starting to suspect that the Hauppauge Grey IR produces a sequence with shorter bits, but, as the hardware decoders are capable or receiving IR codes, it may also be a hardware problem.

The fundamental unit in RC-5 is 32 cycles / 36 kHz = 888889 ns ~= 889 us.

I turned on the cx23888-ir.c debugging on the HVR-1850 and using a Hauppague grey remote (address 0x1e IIRC) and got this as just one example:

cx23885[1]/888-ir: rx read:     802037 ns  mark
cx23885[1]/888-ir: rx read:     852704 ns  space
cx23885[1]/888-ir: rx read:     775370 ns  mark
cx23885[1]/888-ir: rx read:     852407 ns  space
cx23885[1]/888-ir: rx read:     802037 ns  mark
cx23885[1]/888-ir: rx read:     852852 ns  space
cx23885[1]/888-ir: rx read:     775667 ns  mark
cx23885[1]/888-ir: rx read:     852407 ns  space
cx23885[1]/888-ir: rx read:     801741 ns  mark
cx23885[1]/888-ir: rx read:     852852 ns  space
cx23885[1]/888-ir: rx read:     775667 ns  mark
cx23885[1]/888-ir: rx read:     852407 ns  space
cx23885[1]/888-ir: rx read:    1602926 ns  mark
cx23885[1]/888-ir: rx read:     852407 ns  space
cx23885[1]/888-ir: rx read:     801741 ns  mark
cx23885[1]/888-ir: rx read:     852852 ns  space
cx23885[1]/888-ir: rx read:     775074 ns  mark
cx23885[1]/888-ir: rx read:     853148 ns  space
cx23885[1]/888-ir: rx read:     801593 ns  mark
cx23885[1]/888-ir: rx read:     852704 ns  space
cx23885[1]/888-ir: rx read:     775667 ns  mark
cx23885[1]/888-ir: rx read:     852556 ns  space
cx23885[1]/888-ir: rx read:     801741 ns  mark
cx23885[1]/888-ir: rx read:     852259 ns  space
cx23885[1]/888-ir: rx read:     775963 ns  mark

cx23885[1]/888-ir: rx read: end of rx

That should be a press of '0' on the remote.

'end of rx' means the hardware measured a really long space.

I also had the hardware low pass filter on. I think that would effect
the space measurements by making them shorter, if IR noise caused a glitch.

Note that many of the marks are a bit shorter than the ideal 889 us. In fact the single marks from the grey remote seem to alternate between 775 us and 802 us.

I have attached a larger capture of (attempted) single presses of the digits '0' through '9' and then an intentionally held down press of '7'.

With a quick glance, I don't see pauses from the grey remote.

Regards,
Andy

#14 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

Quoted 10 lines · Andy Walls
Quoted 6 lines
Quoted 3 lines

2. A common glitch filtering library function that can be used by all decoders, and that also can accept a decoder specified minimum acceptable pulse width.

Seems a nice improvement. I doubt I'll have time for handling it right now, since there are still many things to do, but I'll put it on my todo list. Of course, patches adding it are wellcome ;)

:)

OK. When I find time I'll hack something up as a prototype.

Thanks!

Quoted 9 lines
Quoted 2 lines

Btw, I added a RC-5 decoder there, at my IR experimental tree:
http://git.linuxtv.org/mchehab/ir.git

I'll try to review it some time this week. Streaming state machine decoders do seem to be best way to go with these decoders.

I have an RC-5 decoder in cx23885-input.c that isn't as clean as the NEC protocol decoder I developed. The cx23885-input.c RC-5 decoder is not a very explicit state machine however (it is a bit hack-ish).

The state machine seems to be working fine with the code, but I think I found the issue: it was expecting 14 bits after the start+toggle bits, instead of a total of 14 bits. I'll fix it. I'll probably end by simplifying it to have only 3 states: inactive, mark-space and trailer.

Quoted 5 lines
Quoted 3 lines

Unfortunately, there's some problem with either my Remote Controller or with the saa7134 driver. After 11 bits received, after the 2 start bits, it receives a pause (see the enclosed sequence).

-ENOATTACHMENT

Sorry! It is basically the same output as yours. See enclosed.

Quoted 41 lines
Quoted 3 lines

I'm starting to suspect that the Hauppauge Grey IR produces a sequence with shorter bits, but, as the hardware decoders are capable or receiving IR codes, it may also be a hardware problem.

The fundamental unit in RC-5 is 32 cycles / 36 kHz = 888889 ns ~= 889 us.

I turned on the cx23888-ir.c debugging on the HVR-1850 and using a Hauppague grey remote (address 0x1e IIRC) and got this as just one example:

cx23885[1]/888-ir: rx read:     802037 ns  mark
cx23885[1]/888-ir: rx read:     852704 ns  space
cx23885[1]/888-ir: rx read:     775370 ns  mark
cx23885[1]/888-ir: rx read:     852407 ns  space
cx23885[1]/888-ir: rx read:     802037 ns  mark
cx23885[1]/888-ir: rx read:     852852 ns  space
cx23885[1]/888-ir: rx read:     775667 ns  mark
cx23885[1]/888-ir: rx read:     852407 ns  space
cx23885[1]/888-ir: rx read:     801741 ns  mark
cx23885[1]/888-ir: rx read:     852852 ns  space
cx23885[1]/888-ir: rx read:     775667 ns  mark
cx23885[1]/888-ir: rx read:     852407 ns  space
cx23885[1]/888-ir: rx read:    1602926 ns  mark
cx23885[1]/888-ir: rx read:     852407 ns  space
cx23885[1]/888-ir: rx read:     801741 ns  mark
cx23885[1]/888-ir: rx read:     852852 ns  space
cx23885[1]/888-ir: rx read:     775074 ns  mark
cx23885[1]/888-ir: rx read:     853148 ns  space
cx23885[1]/888-ir: rx read:     801593 ns  mark
cx23885[1]/888-ir: rx read:     852704 ns  space
cx23885[1]/888-ir: rx read:     775667 ns  mark
cx23885[1]/888-ir: rx read:     852556 ns  space
cx23885[1]/888-ir: rx read:     801741 ns  mark
cx23885[1]/888-ir: rx read:     852259 ns  space
cx23885[1]/888-ir: rx read:     775963 ns  mark

cx23885[1]/888-ir: rx read: end of rx

That should be a press of '0' on the remote.

'end of rx' means the hardware measured a really long space.

Quoted 7 lines

I also had the hardware low pass filter on. I think that would effect
the space measurements by making them shorter, if IR noise caused a glitch.

Note that many of the marks are a bit shorter than the ideal 889 us. In fact the single marks from the grey remote seem to alternate between 775 us and 802 us.

The same happened here. The carrier doesn't seem to be precisely 36 kHz. The code on saa7134 has a way to adjust the time, plus a logic a timer to adjust the end of a RC5 code reception. It seems a good idea to allow adjusting those timers via sysfs.

Quoted 4 lines

I have attached a larger capture of (attempted) single presses of the digits '0' through '9' and then an intentionally held down press of '7'.

With a quick glance, I don't see pauses from the grey remote.

Thanks for your dumps! It is not clear that the saa7134 is doing the right thing, and that the IR uses less bits than the standard.

Quoted 4 lines

Regards,
Andy

Signature · 34 lines
Cheers,
Mauro

The ir-raw-event output for digit '0' is:

[ 2803.106396] ir_raw_event_handle: event type 6, time before event: 0000000us
[ 2803.106404] ir_raw_event_handle: event type 1, time before event: 0000919us
[ 2803.106409] ir_raw_event_handle: event type 2, time before event: 0000868us
[ 2803.106412] ir_raw_event_handle: event type 1, time before event: 0000893us
[ 2803.106415] ir_raw_event_handle: event type 2, time before event: 0000869us
[ 2803.106418] ir_raw_event_handle: event type 1, time before event: 0000921us
[ 2803.106424] ir_raw_event_handle: event type 2, time before event: 0000869us
[ 2803.106427] ir_raw_event_handle: event type 1, time before event: 0000893us
[ 2803.106433] ir_raw_event_handle: event type 2, time before event: 0000869us
[ 2803.106436] ir_raw_event_handle: event type 1, time before event: 0000922us
[ 2803.106442] ir_raw_event_handle: event type 2, time before event: 0000868us
[ 2803.106444] ir_raw_event_handle: event type 1, time before event: 0000893us
[ 2803.106451] ir_raw_event_handle: event type 2, time before event: 0000869us
[ 2803.106453] ir_raw_event_handle: event type 1, time before event: 0001788us
[ 2803.106460] ir_raw_event_handle: event type 2, time before event: 0000869us
[ 2803.106462] ir_raw_event_handle: event type 1, time before event: 0000921us
[ 2803.106469] ir_raw_event_handle: event type 2, time before event: 0000868us
[ 2803.106471] ir_raw_event_handle: event type 1, time before event: 0000893us
[ 2803.106478] ir_raw_event_handle: event type 2, time before event: 0000869us
[ 2803.106480] ir_raw_event_handle: event type 1, time before event: 0000921us
[ 2803.106486] ir_raw_event_handle: event type 2, time before event: 0000869us
[ 2803.106489] ir_raw_event_handle: event type 1, time before event: 0000893us
[ 2803.106495] ir_raw_event_handle: event type 2, time before event: 0000869us
[ 2803.106498] ir_raw_event_handle: event type 1, time before event: 0000921us
[ 2803.106504] ir_raw_event_handle: event type 2, time before event: 0000869us
[ 2803.106507] ir_raw_event_handle: event type 1, time before event: 0000893us
[ 2803.106513] ir_raw_event_handle: event type 2, time before event: 0090992us

The type 1 is space, type 2 is mark, type 6 is mark+start.
#15 lore raw

Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core

Quoted 9 lines · Mauro Carvalho Chehab
Quoted 3 lines · Andy Walls

I have an RC-5 decoder in cx23885-input.c that isn't as clean as the NEC protocol decoder I developed. The cx23885-input.c RC-5 decoder is not a very explicit state machine however (it is a bit hack-ish).

The state machine seems to be working fine with the code, but I think I found the issue: it was expecting 14 bits after the start+toggle bits, instead of a total of 14 bits. I'll fix it. I'll probably end by simplifying it to have only 3 states: inactive, mark-space and trailer.

Done. I've re-written the state machine logic. The code is now simpler to understand, require less processing and works properly with RC-5.

Instead of generating an intermediate code, like the code in ir-functions, it measures directly the length of each pulse or space event and generate the corresponding bit directly, putting it into a shift register. At the end of the 14 bits reception, the shift register will contain the scancode.

When compared with saa7134 original RC5 decoder, this code is much more reliable, since it doesn't propagate the errors, if the frequency is not precisely 36 kHz.

I tested here with my device and it is properly recognizing the Hauppauge Grey IR keys.

Both NEC and RC-5 decoders can run in parallel.

The patch here:

http://git.linuxtv.org/mchehab/ir.git?a=commitdiff;h=37b215ea1280a621d652469cd35328a208f8ef77

And the complete code:

http://git.linuxtv.org/mchehab/ir.git?a=blob;f=drivers/media/IR/ir-rc5-decoder.c;h=a62277b625a8ed78028e7060a677598eeae03ffe;hb=37b215ea1280a621d652469cd35328a208f8ef77

I'll likely send an email to the ML with the RC patches that are on my experimental tree, to properly document, and merge it at the -git, together with the other pending requests.

Signature · 2 lines
Cheers,
Mauro
#27 lore raw

Re: [PATCH 15/15] V4L/DVB: input: Add support for EVIO[CS]GKEYCODEBIG

In reply to Mauro Carvalho Chehab
Quoted 14 lines · Mauro Carvalho Chehab

Several devices use a high number of bits for scancodes. One important group is the Remote Controllers. Some new protocols like RC-6 define a scancode space of 64 bits.

The current EVIO[CS]GKEYCODE ioctls allow replace the scancode/keycode translation tables, but it is limited to up to 32 bits for scancode.

Also, if userspace wants to clean the existing table, replacing it by a new one, it needs to run a loop calling the old ioctls, over the entire sparsed scancode userspace.

To solve those problems, this patch introduces two new ioctls:

	EVIOCGKEYCODEBIG - reads a scancode from the translation table;
	EVIOSGKEYCODEBIG - writes a scancode into the translation table.

...

Quoted 14 lines
include/linux/input.h +1 -1
index 663208a..6445fc9 100644
@@ -34,7 +34,7 @@ struct input_event {
  * Protocol version.
  */
 
-#define EV_VERSION		0x010000
+#define EV_VERSION		0x010001
 
 /*
  * IOCTLs (0x00 - 0x7f)

include/linux/input.h struct input_absinfo { · line 65

Quoted 10 lines
include/linux/input.h +7 -0
@@ -56,12 +56,22 @@ struct input_absinfo {
 	__s32 resolution;
 };
 
+struct keycode_table_entry {
+	__u32 keycode;		/* e.g. KEY_A */
+	__u32 index;            /* Index for the given scan/key table, on EVIOCGKEYCODEBIG */
+	__u32 len;		/* Length of the scancode */
+	__u32 reserved[2];	/* Reserved for future usage */
+	char *scancode;		/* scancode, in machine-endian */
+};

Wouldn't changing the scancode member from a pointer to a flexible array member (C99 feature, which I assume is ok since other C99 features are already in use in the kernel code) remove the need for any compat32 code?

struct keycode_table_entry {
	__u32 keycode;
	__u32 index;
	__u32 len;
	__u32 reserved[2];
	char scancode[];
};
Signature · 5 lines
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html