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

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

[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.

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().

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 258c639..1730b5b 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -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);
 
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 86cb2d2..d2bb5b5 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -551,6 +551,11 @@ static void input_disconnect_device(struct input_dev *dev)
 	spin_unlock_irq(&dev->event_lock);
 }
 
+/*
+ * Those routines handle the default case where no [gs]etkeycode() is
+ * defined. In this case, an array indexed by the scancode is used.
+ */
+
 static int input_fetch_keycode(struct input_dev *dev, int scancode)
 {
 	switch (dev->keycodesize) {
@@ -565,25 +570,74 @@ static int input_fetch_keycode(struct input_dev *dev, int scancode)
 	}
 }
 
-static int input_default_getkeycode(struct input_dev *dev,
-				    int scancode, int *keycode)
+/*
+ * Supports only 8, 16 and 32 bit scancodes. It wouldn't be that
+ * hard to write some machine-endian logic to support 24 bit scancodes,
+ * but it seemed overkill. It should also be noticed that, since there
+ * are, in general, less than 256 scancodes sparsed into the scancode
+ * space, even with 16 bits, the codespace is sparsed, with leads into
+ * memory and code ineficiency, when retrieving the entire scancode
+ * space.
+ * So, it is highly recommended to implement getkeycodebig/setkeycodebig
+ * instead of using a normal table approach, when more than 8 bits is
+ * needed for the scancode.
+ */
+static int input_fetch_scancode(struct keycode_table_entry *kt_entry,
+				u32 *scancode)
 {
+	switch (kt_entry->len) {
+	case 1:
+		*scancode = *((u8 *)kt_entry->scancode);
+		break;
+	case 2:
+		*scancode = *((u16 *)kt_entry->scancode);
+		break;
+	case 4:
+		*scancode = *((u32 *)kt_entry->scancode);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+
+static int input_default_getkeycode_from_index(struct input_dev *dev,
+				    struct keycode_table_entry *kt_entry)
+{
+	u32 scancode = kt_entry->index;
+
 	if (!dev->keycodesize)
 		return -EINVAL;
 
 	if (scancode >= dev->keycodemax)
 		return -EINVAL;
 
-	*keycode = input_fetch_keycode(dev, scancode);
+	kt_entry->keycode = input_fetch_keycode(dev, scancode);
+	memcpy(kt_entry->scancode, &scancode, 4);
 
 	return 0;
 }
 
+static int input_default_getkeycode_from_scancode(struct input_dev *dev,
+				    struct keycode_table_entry *kt_entry)
+{
+	if (input_fetch_scancode(kt_entry, &kt_entry->index))
+		return -EINVAL;
+
+	return input_default_getkeycode_from_index(dev, kt_entry);
+}
+
+
 static int input_default_setkeycode(struct input_dev *dev,
-				    int scancode, int keycode)
+				    struct keycode_table_entry *kt_entry)
 {
 	int old_keycode;
 	int i;
+	u32 scancode;
+
+	if (input_fetch_scancode(kt_entry, &scancode))
+		return -EINVAL;
 
 	if (scancode >= dev->keycodemax)
 		return -EINVAL;
@@ -591,32 +645,33 @@ static int input_default_setkeycode(struct input_dev *dev,
 	if (!dev->keycodesize)
 		return -EINVAL;
 
-	if (dev->keycodesize < sizeof(keycode) && (keycode >> (dev->keycodesize * 8)))
+	if (dev->keycodesize < sizeof(dev->keycode) &&
+	    (kt_entry->keycode >> (dev->keycodesize * 8)))
 		return -EINVAL;
 
 	switch (dev->keycodesize) {
 		case 1: {
 			u8 *k = (u8 *)dev->keycode;
 			old_keycode = k[scancode];
-			k[scancode] = keycode;
+			k[scancode] = kt_entry->keycode;
 			break;
 		}
 		case 2: {
 			u16 *k = (u16 *)dev->keycode;
 			old_keycode = k[scancode];
-			k[scancode] = keycode;
+			k[scancode] = kt_entry->keycode;
 			break;
 		}
 		default: {
 			u32 *k = (u32 *)dev->keycode;
 			old_keycode = k[scancode];
-			k[scancode] = keycode;
+			k[scancode] = kt_entry->keycode;
 			break;
 		}
 	}
 
 	clear_bit(old_keycode, dev->keybit);
-	set_bit(keycode, dev->keybit);
+	set_bit(kt_entry->keycode, dev->keybit);
 
 	for (i = 0; i < dev->keycodemax; i++) {
 		if (input_fetch_keycode(dev, i) == old_keycode) {
@@ -629,6 +684,109 @@ static int input_default_setkeycode(struct input_dev *dev,
 }
 
 /**
+ * input_get_keycode_big - retrieve keycode currently mapped to a given scancode
+ * @dev: input device which keymap is being queried
+ * @kt_entry: keytable entry
+ *
+ * This function should be called by anyone interested in retrieving current
+ * keymap. Presently evdev handlers use it.
+ */
+int input_get_keycode_big(struct input_dev *dev,
+			  struct keycode_table_entry *kt_entry)
+{
+	if (dev->getkeycode) {
+		u32 scancode = kt_entry->index;
+
+		/*
+		 * Support for legacy drivers, that don't implement the new
+		 * ioctls
+		 */
+		memcpy(kt_entry->scancode, &scancode, 4);
+		return dev->getkeycode(dev, scancode,
+				       &kt_entry->keycode);
+	} else
+		return dev->getkeycodebig_from_index(dev, kt_entry);
+}
+EXPORT_SYMBOL(input_get_keycode_big);
+
+/**
+ * input_set_keycode_big - attribute a keycode to a given scancode
+ * @dev: input device which keymap is being queried
+ * @kt_entry: keytable entry
+ *
+ * This function should be called by anyone needing to update current
+ * keymap. Presently keyboard and evdev handlers use it.
+ */
+int input_set_keycode_big(struct input_dev *dev,
+			  struct keycode_table_entry *kt_entry)
+{
+	unsigned long flags;
+	int old_keycode;
+	int retval = -EINVAL;
+	u32 uninitialized_var(scancode);
+
+	if (kt_entry->keycode < 0 || kt_entry->keycode > KEY_MAX)
+		return -EINVAL;
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+
+	/*
+	 * We need to know the old scancode, in order to generate a
+	 * keyup effect, if the set operation happens successfully
+	 */
+	if (dev->getkeycode) {
+		/*
+		 * Support for legacy drivers, that don't implement the new
+		 * ioctls
+		 */
+		if (!dev->setkeycode)
+			goto out;
+
+		if (input_fetch_scancode(kt_entry, &scancode))
+			return -EINVAL;
+
+		retval = dev->getkeycode(dev, scancode,
+					 &old_keycode);
+	} else {
+		int new_keycode = kt_entry->keycode;
+
+		retval = dev->getkeycodebig_from_scancode(dev, kt_entry);
+		old_keycode = kt_entry->keycode;
+		kt_entry->keycode = new_keycode;
+	}
+
+	if (retval)
+		goto out;
+
+	if (dev->getkeycode)
+		retval = dev->setkeycode(dev, scancode,
+					 kt_entry->keycode);
+	else
+		retval = dev->setkeycodebig(dev, kt_entry);
+	if (retval)
+		goto out;
+
+	/*
+	 * Simulate keyup event if keycode is not present
+	 * in the keymap anymore
+	 */
+	if (test_bit(EV_KEY, dev->evbit) &&
+	    !is_event_supported(old_keycode, dev->keybit, KEY_MAX) &&
+	    __test_and_clear_bit(old_keycode, dev->key)) {
+
+		input_pass_event(dev, EV_KEY, old_keycode, 0);
+		if (dev->sync)
+			input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
+	}
+
+ out:
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	return retval;
+}
+EXPORT_SYMBOL(input_set_keycode_big);
+
+/**
  * input_get_keycode - retrieve keycode currently mapped to a given scancode
  * @dev: input device which keymap is being queried
  * @scancode: scancode (or its equivalent for device in question) for which
@@ -640,10 +798,31 @@ static int input_default_setkeycode(struct input_dev *dev,
  */
 int input_get_keycode(struct input_dev *dev, int scancode, int *keycode)
 {
-	if (scancode < 0)
-		return -EINVAL;
+	if (dev->getkeycode) {
+		/*
+		 * Use the legacy calls
+		 */
+		return dev->getkeycode(dev, scancode, keycode);
+	} else {
+		int retval;
+		struct keycode_table_entry kt_entry;
 
-	return dev->getkeycode(dev, scancode, keycode);
+		/*
+		 * Userspace is using a legacy call with a driver ported
+		 * to the new way. This is a bad idea with long sparsed
+		 * tables, since lots of the retrieved values will be in
+		 * blank. Also, it makes sense only if the table size is
+		 * lower than 2^32.
+		 */
+		memset(&kt_entry, 0, sizeof(kt_entry));
+		kt_entry.len = 4;
+		kt_entry.index = scancode;
+
+		retval = dev->getkeycodebig_from_index(dev, &kt_entry);
+
+		*keycode = kt_entry.keycode;
+		return retval;
+	}
 }
 EXPORT_SYMBOL(input_get_keycode);
 
@@ -662,21 +841,46 @@ int input_set_keycode(struct input_dev *dev, int scancode, int keycode)
 	int old_keycode;
 	int retval;
 
-	if (scancode < 0)
-		return -EINVAL;
-
 	if (keycode < 0 || keycode > KEY_MAX)
 		return -EINVAL;
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
-	retval = dev->getkeycode(dev, scancode, &old_keycode);
-	if (retval)
-		goto out;
+	if (dev->getkeycode) {
+		/*
+		 * Use the legacy calls
+		 */
+		retval = dev->getkeycode(dev, scancode, &old_keycode);
+		if (retval)
+			goto out;
 
-	retval = dev->setkeycode(dev, scancode, keycode);
-	if (retval)
-		goto out;
+		retval = dev->setkeycode(dev, scancode, keycode);
+		if (retval)
+			goto out;
+	} else {
+		struct keycode_table_entry kt_entry;
+
+		/*
+		 * Userspace is using a legacy call with a driver ported
+		 * to the new way. This is a bad idea with long sparsed
+		 * tables, since lots of the retrieved values will be in
+		 * blank. Also, it makes sense only if the table size is
+		 * lower than 2^32.
+		 */
+		memset(&kt_entry, 0, sizeof(kt_entry));
+		kt_entry.len = 4;
+		kt_entry.scancode = (char *)&scancode;
+
+		retval = dev->getkeycodebig_from_scancode(dev, &kt_entry);
+		if (retval)
+			goto out;
+
+		kt_entry.keycode = keycode;
+
+		retval = dev->setkeycodebig(dev, &kt_entry);
+		if (retval)
+			goto out;
+	}
 
 	/*
 	 * Simulate keyup event if keycode is not present
@@ -1585,11 +1789,17 @@ int input_register_device(struct input_dev *dev)
 		dev->rep[REP_PERIOD] = 33;
 	}
 
-	if (!dev->getkeycode)
-		dev->getkeycode = input_default_getkeycode;
+	if (!dev->getkeycode) {
+		if (!dev->getkeycodebig_from_index)
+			dev->getkeycodebig_from_index = input_default_getkeycode_from_index;
+		if (!dev->getkeycodebig_from_scancode)
+			dev->getkeycodebig_from_scancode = input_default_getkeycode_from_scancode;
+	}
 
-	if (!dev->setkeycode)
-		dev->setkeycode = input_default_setkeycode;
+	if (dev->setkeycode) {
+		if (!dev->setkeycodebig)
+			dev->setkeycodebig = input_default_setkeycode;
+	}
 
 	dev_set_name(&dev->dev, "input%ld",
 		     (unsigned long) atomic_inc_return(&input_no) - 1);
diff --git a/include/linux/input.h b/include/linux/input.h
index 663208a..6445fc9 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -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;
 
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

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’

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
index 8dd74ae..a42c953 100644
--- a/drivers/media/video/saa7134/saa7134-input.c
+++ b/drivers/media/video/saa7134/saa7134-input.c
@@ -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);
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/cx88/cx88-input.c b/drivers/media/video/cx88/cx88-input.c
index 8f1b846..a7214d0 100644
--- a/drivers/media/video/cx88/cx88-input.c
+++ b/drivers/media/video/cx88/cx88-input.c
@@ -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);
 
diff --git a/drivers/media/video/cx88/cx88-video.c b/drivers/media/video/cx88/cx88-video.c
index 48c450f..8e414f7 100644
--- a/drivers/media/video/cx88/cx88-video.c
+++ b/drivers/media/video/cx88/cx88-video.c
@@ -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);
 
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index b5f054d..bdb03d3 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -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                                                 */
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

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.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index c9a986d..cb57cc2 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -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;
 }
 
diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
index 3eae128..11f23f4 100644
--- a/drivers/media/IR/ir-raw-event.c
+++ b/drivers/media/IR/ir-raw-event.c
@@ -14,13 +14,41 @@
 
 #include <media/ir-core.h>
 #include <linux/workqueue.h>
+#include <linux/spinlock.h>
 
 /* Define the max number of bit transitions per IR keycode */
 #define MAX_IR_EVENT_SIZE	256
 
 /* Used to handle IR raw handler extensions */
 static LIST_HEAD(ir_raw_handler_list);
-static DEFINE_MUTEX(ir_raw_handler_lock);
+static spinlock_t ir_raw_handler_lock;
+
+/**
+ * RUN_DECODER()	- runs an operation on all IR decoders
+ * @ops:	IR raw handler operation to be called
+ * @arg:	arguments to be passed to the callback
+ *
+ * Calls ir_raw_handler::ops for all registered IR handlers. It prevents
+ * new decode addition/removal while running, by locking ir_raw_handler_lock
+ * mutex. If an error occurs, it stops the ops. Otherwise, it returns a sum
+ * of the return codes.
+ */
+#define RUN_DECODER(ops, ...) ({					    \
+	struct ir_raw_handler		*_ir_raw_handler;		    \
+	int _sumrc = 0, _rc;						    \
+	spin_lock(&ir_raw_handler_lock);				    \
+	list_for_each_entry(_ir_raw_handler, &ir_raw_handler_list, list) {  \
+		if (_ir_raw_handler->ops) {				    \
+			_rc = _ir_raw_handler->ops(__VA_ARGS__);	    \
+			if (_rc < 0)					    \
+				break;					    \
+			_sumrc += _rc;					    \
+		}							    \
+	}								    \
+	spin_unlock(&ir_raw_handler_lock);				    \
+	_sumrc;								    \
+})
+
 
 /* Used to load the decoders */
 static struct work_struct wq_load;
@@ -38,6 +66,8 @@ int ir_raw_event_register(struct input_dev *input_dev)
 	int rc, size;
 
 	ir->raw = kzalloc(sizeof(*ir->raw), GFP_KERNEL);
+	if (!ir->raw)
+		return -ENOMEM;
 
 	size = sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE * 2;
 	size = roundup_pow_of_two(size);
@@ -48,6 +78,19 @@ int ir_raw_event_register(struct input_dev *input_dev)
 	set_bit(EV_REP, input_dev->evbit);
 
 	rc = kfifo_alloc(&ir->raw->kfifo, size, GFP_KERNEL);
+	if (rc < 0) {
+		kfree(ir->raw);
+		ir->raw = NULL;
+		return rc;
+	}
+
+	rc = RUN_DECODER(raw_register, input_dev);
+	if (rc < 0) {
+		kfifo_free(&ir->raw->kfifo);
+		kfree(ir->raw);
+		ir->raw = NULL;
+		return rc;
+	}
 
 	return rc;
 }
@@ -62,6 +105,8 @@ void ir_raw_event_unregister(struct input_dev *input_dev)
 
 	del_timer_sync(&ir->raw->timer_keyup);
 
+	RUN_DECODER(raw_unregister, input_dev);
+
 	kfifo_free(&ir->raw->kfifo);
 	kfree(ir->raw);
 	ir->raw = NULL;
@@ -109,7 +154,6 @@ int ir_raw_event_handle(struct input_dev *input_dev)
 	int				rc;
 	struct ir_raw_event		*evs;
 	int 				len, i;
-	struct ir_raw_handler		*ir_raw_handler;
 
 	/*
 	 * Store the events into a temporary buffer. This allows calling more than
@@ -133,13 +177,10 @@ int ir_raw_event_handle(struct input_dev *input_dev)
 
 	/*
 	 * Call all ir decoders. This allows decoding the same event with
-	 * more than one protocol handler.
-	 * FIXME: better handle the returned code: does it make sense to use
-	 * other decoders, if the first one already handled the IR?
+	 * more than one protocol handler. It returns the number of keystrokes
+	 * sent to the event interface
 	 */
-	list_for_each_entry(ir_raw_handler, &ir_raw_handler_list, list) {
-		rc = ir_raw_handler->decode(input_dev, evs, len);
-	}
+	rc = RUN_DECODER(decode, input_dev, evs, len);
 
 	kfree(evs);
 
@@ -153,18 +194,18 @@ EXPORT_SYMBOL_GPL(ir_raw_event_handle);
 
 int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler)
 {
-	mutex_lock(&ir_raw_handler_lock);
+	spin_lock(&ir_raw_handler_lock);
 	list_add_tail(&ir_raw_handler->list, &ir_raw_handler_list);
-	mutex_unlock(&ir_raw_handler_lock);
+	spin_unlock(&ir_raw_handler_lock);
 	return 0;
 }
 EXPORT_SYMBOL(ir_raw_handler_register);
 
 void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler)
 {
-	mutex_lock(&ir_raw_handler_lock);
+	spin_lock(&ir_raw_handler_lock);
 	list_del(&ir_raw_handler->list);
-	mutex_unlock(&ir_raw_handler_lock);
+	spin_unlock(&ir_raw_handler_lock);
 }
 EXPORT_SYMBOL(ir_raw_handler_unregister);
 
@@ -181,6 +222,10 @@ 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);
-}
\ No newline at end of file
+#endif
+}
diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
index 38f60e4..8dd74ae 100644
--- a/drivers/media/video/saa7134/saa7134-input.c
+++ b/drivers/media/video/saa7134/saa7134-input.c
@@ -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);
 
 
 	/*
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index c377bf4..c704fa7 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -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
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

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.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index 2dbce59..435d83e 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -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;
 	}
 
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

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.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index cb57cc2..83a9912 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -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)
diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
index 11f23f4..371d88e 100644
--- a/drivers/media/IR/ir-raw-event.c
+++ b/drivers/media/IR/ir-raw-event.c
@@ -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);
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

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.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-keytable.c b/drivers/media/IR/ir-keytable.c
index f25193c..db54562 100644
--- a/drivers/media/IR/ir-keytable.c
+++ b/drivers/media/IR/ir-keytable.c
@@ -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);
diff --git a/drivers/media/video/saa7134/saa7134-core.c b/drivers/media/video/saa7134/saa7134-core.c
index a7ad781..68cda10 100644
--- a/drivers/media/video/saa7134/saa7134-core.c
+++ b/drivers/media/video/saa7134/saa7134-core.c
@@ -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);
diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
index a42c953..9e2b32c 100644
--- a/drivers/media/video/saa7134/saa7134-input.c
+++ b/drivers/media/video/saa7134/saa7134-input.c
@@ -399,7 +399,14 @@ static int get_key_pinnacle_color(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
 
 void saa7134_input_irq(struct saa7134_dev *dev)
 {
-	struct card_ir *ir = dev->remote;
+	struct card_ir *ir;
+
+	if (!dev || !dev->remote)
+		return;
+
+	ir = dev->remote;
+	if (!ir->running)
+		return;
 
 	if (ir->nec_gpio) {
 		saa7134_nec_irq(dev);
@@ -431,10 +438,20 @@ void ir_raw_decode_timer_end(unsigned long data)
 	ir->active = 0;
 }
 
-void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir)
+static int __saa7134_ir_start(void *priv)
 {
+	struct saa7134_dev *dev = priv;
+	struct card_ir *ir;
+
+	if (!dev)
+		return -EINVAL;
+
+	ir  = dev->remote;
+	if (!ir)
+		return -EINVAL;
+
 	if (ir->running)
-		return;
+		return 0;
 
 	ir->running = 1;
 	if (ir->polling) {
@@ -466,11 +483,21 @@ void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir)
 		ir->timer_end.data = (unsigned long)dev;
 		ir->active = 0;
 	}
+
+	return 0;
 }
 
-void saa7134_ir_stop(struct saa7134_dev *dev)
+static void __saa7134_ir_stop(void *priv)
 {
-	struct card_ir *ir = dev->remote;
+	struct saa7134_dev *dev = priv;
+	struct card_ir *ir;
+
+	if (!dev)
+		return;
+
+	ir  = dev->remote;
+	if (!ir)
+		return;
 
 	if (!ir->running)
 		return;
@@ -486,8 +513,42 @@ void saa7134_ir_stop(struct saa7134_dev *dev)
 	}
 
 	ir->running = 0;
+
+	return;
 }
 
+int saa7134_ir_start(struct saa7134_dev *dev)
+{
+	if (dev->remote->users)
+		return __saa7134_ir_start(dev);
+
+	return 0;
+}
+
+void saa7134_ir_stop(struct saa7134_dev *dev)
+{
+	if (dev->remote->users)
+		__saa7134_ir_stop(dev);
+}
+
+static int saa7134_ir_open(void *priv)
+{
+	struct saa7134_dev *dev = priv;
+
+	dev->remote->users++;
+	return __saa7134_ir_start(dev);
+}
+
+static void saa7134_ir_close(void *priv)
+{
+	struct saa7134_dev *dev = priv;
+
+	dev->remote->users--;
+	if (!dev->remote->users)
+		__saa7134_ir_stop(dev);
+}
+
+
 int saa7134_ir_change_protocol(void *priv, u64 ir_type)
 {
 	struct saa7134_dev *dev = priv;
@@ -512,7 +573,7 @@ int saa7134_ir_change_protocol(void *priv, u64 ir_type)
 		saa7134_ir_stop(dev);
 		ir->nec_gpio = nec_gpio;
 		ir->rc5_gpio = rc5_gpio;
-		saa7134_ir_start(dev, ir);
+		saa7134_ir_start(dev);
 	} else {
 		ir->nec_gpio = nec_gpio;
 		ir->rc5_gpio = rc5_gpio;
@@ -787,9 +848,13 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	snprintf(ir->phys, sizeof(ir->phys), "pci-%s/ir0",
 		 pci_name(dev->pci));
 
+
+	ir->props.priv = dev;
+	ir->props.open = saa7134_ir_open;
+	ir->props.close = saa7134_ir_close;
+
 	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;
 
 		/* Set IR protocol */
@@ -814,25 +879,21 @@ 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;
+		goto err_out_free;
 	if (ir_codes->ir_type != IR_TYPE_OTHER) {
 		err = ir_raw_event_register(ir->dev);
 		if (err)
-			goto err_out_stop;
+			goto err_out_free;
 	}
 
-	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;
 
 	return 0;
 
- err_out_stop:
-	saa7134_ir_stop(dev);
+err_out_free:
 	dev->remote = NULL;
- err_out_free:
 	kfree(ir);
 	return err;
 }
diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h
index c3a1ae0..cad8aee 100644
--- a/drivers/media/video/saa7134/saa7134.h
+++ b/drivers/media/video/saa7134/saa7134.h
@@ -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);
 
 
diff --git a/include/media/ir-common.h b/include/media/ir-common.h
index 87f2ec7..e403a9a 100644
--- a/include/media/ir-common.h
+++ b/include/media/ir-common.h
@@ -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;
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index c704fa7..9a2f308 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -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 {
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

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’

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
index 8dd74ae..a42c953 100644
--- a/drivers/media/video/saa7134/saa7134-input.c
+++ b/drivers/media/video/saa7134/saa7134-input.c
@@ -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);
-- 
1.6.6.1


--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Mauro Carvalho Chehab]

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.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-keytable.c b/drivers/media/IR/ir-keytable.c
index ab60730..f25193c 100644
--- a/drivers/media/IR/ir-keytable.c
+++ b/drivers/media/IR/ir-keytable.c
@@ -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;
diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index a58c717..104482a 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -14,21 +14,51 @@
 
 #include <media/ir-core.h>
 
-/* Start time: 4.5 ms  */
-#define MIN_START_TIME	3900000
-#define MAX_START_TIME	5100000
+/* Start time: 4.5 ms + 560 us of the next pulse */
+#define MIN_START_TIME	(3900000 + 560000)
+#define MAX_START_TIME	(5100000 + 560000)
 
-/* Pulse time: 560 us  */
-#define MIN_PULSE_TIME	460000
-#define MAX_PULSE_TIME	660000
+/* Bit 1 time: 2.25ms us */
+#define MIN_BIT1_TIME	2050000
+#define MAX_BIT1_TIME	2450000
 
-/* Bit 1 space time: 2.25ms-560 us */
-#define MIN_BIT1_TIME	1490000
-#define MAX_BIT1_TIME	1890000
+/* Bit 0 time: 1.12ms us */
+#define MIN_BIT0_TIME	920000
+#define MAX_BIT0_TIME	1320000
 
-/* Bit 0 space time: 1.12ms-560 us */
-#define MIN_BIT0_TIME	360000
-#define MAX_BIT0_TIME	760000
+/* Total IR code is 110 ms, including the 9 ms for the start pulse */
+#define MAX_NEC_TIME	4000000
+
+/* Total IR code is 110 ms, including the 9 ms for the start pulse */
+#define MIN_REPEAT_TIME	99000000
+#define MAX_REPEAT_TIME	112000000
+
+/* Repeat time: 2.25ms us */
+#define MIN_REPEAT_START_TIME	2050000
+#define MAX_REPEAT_START_TIME	3000000
+
+#define REPEAT_TIME	240 /* ms */
+
+/** 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
+ * @len:	the length of the buffer
+ */
+static int is_repeat(struct ir_raw_event *evs, int len, int pos)
+{
+	if ((evs[pos].delta.tv_nsec < MIN_REPEAT_START_TIME) ||
+	    (evs[pos].delta.tv_nsec > MAX_REPEAT_START_TIME))
+		return 0;
+
+	if (++pos >= len)
+		return 0;
+
+	if ((evs[pos].delta.tv_nsec < MIN_REPEAT_TIME) ||
+	    (evs[pos].delta.tv_nsec > MAX_REPEAT_TIME))
+		return 0;
+
+	return 1;
+}
 
 /**
  * __ir_nec_decode() - Decode one NEC pulsecode
@@ -36,49 +66,59 @@
  * @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
+ * This function returns -EINVAL if no pulse got decoded,
+ * 0 if buffer is empty and 1 if one keycode were handled.
  */
 static int __ir_nec_decode(struct input_dev *input_dev,
 			   struct ir_raw_event *evs,
 			   int len, int *pos)
 {
+	struct ir_input_dev *ir = input_get_drvdata(input_dev);
 	int count = -1;
 	int ircode = 0, not_code = 0;
 
 	/* Be sure that the first event is an start one and is a pulse */
 	for (; *pos < len; (*pos)++) {
-		if (evs[*pos].type & (IR_START_EVENT | IR_PULSE))
+		/* Very long delays are considered as start events */
+		if (evs[*pos].delta.tv_nsec > MAX_NEC_TIME)
 			break;
-	}
-	(*pos)++;	/* First event doesn't contain data */
+		if (evs[*pos].type & IR_START_EVENT)
+			break;
+		IR_dprintk(1, "%luus: Spurious NEC %s\n",
+			   (evs[*pos].delta.tv_nsec + 500) / 1000,
+			   (evs[*pos].type & IR_SPACE) ? "space" : "pulse");
 
+	}
 	if (*pos >= len)
 		return 0;
 
+	(*pos)++;	/* First event doesn't contain data */
+
+	if (evs[*pos].type != IR_PULSE)
+		goto err;
+
+	/* Check if it is a NEC repeat event */
+	if (is_repeat(evs, len, *pos)) {
+		*pos += 2;
+		if (ir->keypressed) {
+			mod_timer(&ir->raw->timer_keyup,
+				jiffies + msecs_to_jiffies(REPEAT_TIME));
+			IR_dprintk(1, "NEC repeat event\n");
+			return 1;
+		} else {
+			IR_dprintk(1, "missing NEC repeat event\n");
+			return 0;
+		}
+	}
+
 	/* First space should have 4.5 ms otherwise is not NEC protocol */
-	if ((evs[*pos].delta.tv_nsec < MIN_START_TIME) |
-	    (evs[*pos].delta.tv_nsec > MAX_START_TIME) |
-	    (evs[*pos].type != IR_SPACE))
+	if ((evs[*pos].delta.tv_nsec < MIN_START_TIME) ||
+	    (evs[*pos].delta.tv_nsec > MAX_START_TIME))
 		goto err;
 
-	/*
-	 * FIXME: need to implement the repeat sequence
-	 */
-
 	count = 0;
 	for ((*pos)++; *pos < len; (*pos)++) {
 		int bit;
-
-		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 (++*pos >= len)
-			goto err;
-		if (evs[*pos].type != IR_SPACE)
-			goto err;
-
 		if ((evs[*pos].delta.tv_nsec > MIN_BIT1_TIME) &&
 		    (evs[*pos].delta.tv_nsec < MAX_BIT1_TIME))
 			bit = 1;
@@ -107,6 +147,7 @@ static int __ir_nec_decode(struct input_dev *input_dev,
 		if (++count == 32)
 			break;
 	}
+	*pos++;
 
 	/*
 	 * Fixme: may need to accept Extended NEC protocol?
@@ -119,12 +160,15 @@ static 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);
+	mod_timer(&ir->raw->timer_keyup,
+		  jiffies + msecs_to_jiffies(REPEAT_TIME));
 
-	return ircode;
+	return 1;
 err:
-	IR_dprintk(1, "NEC decoded failed at bit %d while decoding %luus time\n",
-		   count, (evs[*pos].delta.tv_nsec + 500) / 1000);
+	IR_dprintk(1, "NEC decoded failed at bit %d (%s) while decoding %luus time\n",
+		   count,
+		   (evs[*pos].type & IR_SPACE) ? "space" : "pulse",
+		   (evs[*pos].delta.tv_nsec + 500) / 1000);
 
 	return -EINVAL;
 }
@@ -145,7 +189,7 @@ int ir_nec_decode(struct input_dev *input_dev,
 	int rc = 0;
 
 	while (pos < len) {
-		if (__ir_nec_decode(input_dev, evs, len, &pos) >= 0)
+		if (__ir_nec_decode(input_dev, evs, len, &pos) > 0)
 			rc++;
 	}
 
diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
index 9c71ac8..0ae5543 100644
--- a/drivers/media/IR/ir-raw-event.c
+++ b/drivers/media/IR/ir-raw-event.c
@@ -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;
diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
index 740adb3..38f60e4 100644
--- a/drivers/media/video/saa7134/saa7134-input.c
+++ b/drivers/media/video/saa7134/saa7134-input.c
@@ -424,8 +424,11 @@ 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;
 
 	ir_raw_event_handle(dev->remote->dev);
+
+	ir->active = 0;
 }
 
 void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir)
@@ -461,6 +464,7 @@ void saa7134_ir_start(struct saa7134_dev *dev, struct card_ir *ir)
 		init_timer(&ir->timer_end);
 		ir->timer_end.function = ir_raw_decode_timer_end;
 		ir->timer_end.data = (unsigned long)dev;
+		ir->active = 0;
 	}
 }
 
@@ -476,8 +480,10 @@ 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)
+	else if (ir->raw_decode) {
 		del_timer_sync(&ir->timer_end);
+		ir->active = 0;
+	}
 
 	ir->running = 0;
 }
@@ -950,39 +956,24 @@ static int saa7134_raw_decode_irq(struct saa7134_dev *dev)
 	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;
+
+	/*
+	 * Wait 15 ms from the start of the first IR event before processing
+	 * the event. This time is enough for NEC protocol. May need adjustments
+	 * to work with other protocols.
+	 */
+	if (!ir->active) {
+		timeout = jiffies + jiffies_to_msecs(15);
+		mod_timer(&ir->timer_end, timeout);
+		ir->active = 1;
 	}
 
-	/* 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;
 }
 
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index 9e03528..8d8ed7e 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -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 {
-- 
1.6.6.1

[PATCH 03/15] V4L/DVB: saa7134: add code to allow changing IR protocol

[Mauro Carvalho Chehab]

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
index d7e73de..8187928 100644
--- a/drivers/media/video/saa7134/saa7134-input.c
+++ b/drivers/media/video/saa7134/saa7134-input.c
@@ -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;
diff --git a/include/media/ir-common.h b/include/media/ir-common.h
index c30b283..41469b7 100644
--- a/include/media/ir-common.h
+++ b/include/media/ir-common.h
@@ -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;
-- 
1.6.6.1

[PATCH 02/15] V4L/DVB: saa7134: use a full scancode table for M135A

[Mauro Carvalho Chehab]

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-keymaps.c b/drivers/media/IR/ir-keymaps.c
index dfc777b..55e7acd 100644
--- a/drivers/media/IR/ir-keymaps.c
+++ b/drivers/media/IR/ir-keymaps.c
@@ -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 <mchehab@infradead.org> */
-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 <mchehab@infradead.org>
+ */
+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 <oldium.pro@seznam.cz> */
 static struct ir_scancode ir_codes_avermedia_cardbus[] = {
diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
index 8c67904..d7e73de 100644
--- a/drivers/media/video/saa7134/saa7134-input.c
+++ b/drivers/media/video/saa7134/saa7134-input.c
@@ -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:
diff --git a/include/media/ir-common.h b/include/media/ir-common.h
index c662980..c30b283 100644
--- a/include/media/ir-common.h
+++ b/include/media/ir-common.h
@@ -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;
-- 
1.6.6.1

[PATCH 08/15] V4L/DVB: ir-core: dynamically load the compiled IR protocols

[Mauro Carvalho Chehab]

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.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/Kconfig b/drivers/media/IR/Kconfig
index 4dde7d1..de410d4 100644
--- a/drivers/media/IR/Kconfig
+++ b/drivers/media/IR/Kconfig
@@ -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
diff --git a/drivers/media/IR/Makefile b/drivers/media/IR/Makefile
index 18794c7..6140b27 100644
--- a/drivers/media/IR/Makefile
+++ b/drivers/media/IR/Makefile
@@ -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
diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index 104482a..c9a986d 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -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 <mchehab@redhat.com>
  *
@@ -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 <mchehab@redhat.com>");
+MODULE_AUTHOR("Red Hat Inc. (http://www.redhat.com)");
+MODULE_DESCRIPTION("NEC IR protocol decoder");
diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
index 0ae5543..3eae128 100644
--- a/drivers/media/IR/ir-raw-event.c
+++ b/drivers/media/IR/ir-raw-event.c
@@ -13,10 +13,18 @@
  */
 
 #include <media/ir-core.h>
+#include <linux/workqueue.h>
 
 /* Define the max number of bit transitions per IR keycode */
 #define MAX_IR_EVENT_SIZE	256
 
+/* Used to handle IR raw handler extensions */
+static LIST_HEAD(ir_raw_handler_list);
+static DEFINE_MUTEX(ir_raw_handler_lock);
+
+/* Used to load the decoders */
+static struct work_struct wq_load;
+
 static void ir_keyup_timer(unsigned long data)
 {
 	struct input_dev *input_dev = (struct input_dev *)data;
@@ -101,6 +109,7 @@ int ir_raw_event_handle(struct input_dev *input_dev)
 	int				rc;
 	struct ir_raw_event		*evs;
 	int 				len, i;
+	struct ir_raw_handler		*ir_raw_handler;
 
 	/*
 	 * Store the events into a temporary buffer. This allows calling more than
@@ -122,10 +131,56 @@ int ir_raw_event_handle(struct input_dev *input_dev)
 			evs[i].type, (evs[i].delta.tv_nsec + 500) / 1000);
 	}
 
-	rc = ir_nec_decode(input_dev, evs, len);
+	/*
+	 * Call all ir decoders. This allows decoding the same event with
+	 * more than one protocol handler.
+	 * FIXME: better handle the returned code: does it make sense to use
+	 * other decoders, if the first one already handled the IR?
+	 */
+	list_for_each_entry(ir_raw_handler, &ir_raw_handler_list, list) {
+		rc = ir_raw_handler->decode(input_dev, evs, len);
+	}
 
 	kfree(evs);
 
 	return rc;
 }
 EXPORT_SYMBOL_GPL(ir_raw_event_handle);
+
+/*
+ * Extension interface - used to register the IR decoders
+ */
+
+int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler)
+{
+	mutex_lock(&ir_raw_handler_lock);
+	list_add_tail(&ir_raw_handler->list, &ir_raw_handler_list);
+	mutex_unlock(&ir_raw_handler_lock);
+	return 0;
+}
+EXPORT_SYMBOL(ir_raw_handler_register);
+
+void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler)
+{
+	mutex_lock(&ir_raw_handler_lock);
+	list_del(&ir_raw_handler->list);
+	mutex_unlock(&ir_raw_handler_lock);
+}
+EXPORT_SYMBOL(ir_raw_handler_unregister);
+
+static void init_decoders(struct work_struct *work)
+{
+	/* Load the decoder modules */
+
+	load_nec_decode();
+
+	/* If needed, we may later add some init code. In this case,
+	   it is needed to change the CONFIG_MODULE test at ir-core.h
+	 */
+}
+
+void ir_raw_init(void)
+{
+	INIT_WORK(&wq_load, init_decoders);
+	schedule_work(&wq_load);
+}
\ No newline at end of file
diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index ee6b36d..2dbce59 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -1,6 +1,6 @@
 /* ir-register.c - handle IR scancode->keycode tables
  *
- * Copyright (C) 2009 by Mauro Carvalho Chehab <mchehab@redhat.com>
+ * Copyright (C) 2009-2010 by Mauro Carvalho Chehab <mchehab@redhat.com>
  *
  * 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;
 }
 
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index 8d8ed7e..c377bf4 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -1,6 +1,8 @@
 /*
  * Remote Controller core header
  *
+ * Copyright (C) 2009-2010 by Mauro Carvalho Chehab <mchehab@redhat.com>
+ *
  * 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 */
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index bbddf2f..ee6b36d 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -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;
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index 16360eb..a58c717 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -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);
diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
index 7382995..740adb3 100644
--- a/drivers/media/video/saa7134/saa7134-input.c
+++ b/drivers/media/video/saa7134/saa7134-input.c
@@ -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)
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index 198fd61..9e03528 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -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,
-- 
1.6.6.1

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

[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.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

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

diff --git a/drivers/media/IR/Makefile b/drivers/media/IR/Makefile
index 171890e..18794c7 100644
--- a/drivers/media/IR/Makefile
+++ b/drivers/media/IR/Makefile
@@ -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
diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
new file mode 100644
index 0000000..16360eb
--- /dev/null
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -0,0 +1,131 @@
+/* ir-raw-event.c - handle IR Pulse/Space event
+ *
+ * Copyright (C) 2010 by Mauro Carvalho Chehab <mchehab@redhat.com>
+ *
+ * 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>
+
+/* Start time: 4.5 ms  */
+#define MIN_START_TIME	3900000
+#define MAX_START_TIME	5100000
+
+/* 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
+
+
+/** 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;
+
+	/* 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
+	 */
+
+	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)
+			break;
+	}
+
+	/*
+	 * Fixme: may need to accept Extended NEC protocol?
+	 */
+	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);
diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
new file mode 100644
index 0000000..9c71ac8
--- /dev/null
+++ b/drivers/media/IR/ir-raw-event.c
@@ -0,0 +1,117 @@
+/* ir-raw-event.c - handle IR Pulse/Space event
+ *
+ * Copyright (C) 2010 by Mauro Carvalho Chehab <mchehab@redhat.com>
+ *
+ * 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);
diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
index 8187928..7382995 100644
--- a/drivers/media/video/saa7134/saa7134-input.c
+++ b/drivers/media/video/saa7134/saa7134-input.c
@@ -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
  */
diff --git a/include/media/ir-common.h b/include/media/ir-common.h
index 41469b7..87f2ec7 100644
--- a/include/media/ir-common.h
+++ b/include/media/ir-common.h
@@ -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 */
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index 1eae72d..369969d 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -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
-- 
1.6.6.1

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

[Mauro Carvalho Chehab]

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-keytable.c b/drivers/media/IR/ir-keytable.c
index 2d9ba84..ab60730 100644
--- a/drivers/media/IR/ir-keytable.c
+++ b/drivers/media/IR/ir-keytable.c
@@ -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
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index 369969d..198fd61 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -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)
-- 
1.6.6.1

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

[Andy Walls]

On Thu, 2010-04-01 at 14:56 -0300, Mauro Carvalho Chehab wrote:
> 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.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> 
>  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....


> diff --git a/drivers/media/IR/Makefile b/drivers/media/IR/Makefile
> index 171890e..18794c7 100644
> --- a/drivers/media/IR/Makefile
> +++ b/drivers/media/IR/Makefile
> @@ -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
> diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
> new file mode 100644
> index 0000000..16360eb
> --- /dev/null
> +++ b/drivers/media/IR/ir-nec-decoder.c
> @@ -0,0 +1,131 @@
> +/* ir-raw-event.c - handle IR Pulse/Space event
> + *
> + * Copyright (C) 2010 by Mauro Carvalho Chehab <mchehab@redhat.com>
> + *
> + * 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>
> +
> +/* 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.


> +/* 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.


> +/** 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;
> +
> +	/* 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




> +	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)
> +			break;
> +	}
> +
> +	/*
> +	 * Fixme: may need to accept Extended NEC protocol?
> +	 */

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



> +	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);
> diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
> new file mode 100644
> index 0000000..9c71ac8
> --- /dev/null
> +++ b/drivers/media/IR/ir-raw-event.c
> @@ -0,0 +1,117 @@
> +/* ir-raw-event.c - handle IR Pulse/Space event
> + *
> + * Copyright (C) 2010 by Mauro Carvalho Chehab <mchehab@redhat.com>
> + *
> + * 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);
> diff --git a/drivers/media/video/saa7134/saa7134-input.c b/drivers/media/video/saa7134/saa7134-input.c
> index 8187928..7382995 100644
> --- a/drivers/media/video/saa7134/saa7134-input.c
> +++ b/drivers/media/video/saa7134/saa7134-input.c
> @@ -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
>   */
> diff --git a/include/media/ir-common.h b/include/media/ir-common.h
> index 41469b7..87f2ec7 100644
> --- a/include/media/ir-common.h
> +++ b/include/media/ir-common.h
> @@ -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 */
> diff --git a/include/media/ir-core.h b/include/media/ir-core.h
> index 1eae72d..369969d 100644
> --- a/include/media/ir-core.h
> +++ b/include/media/ir-core.h
> @@ -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),
> +};
> +

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

Regards,
Andy

>  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

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

[Andy Walls]

On Fri, 2010-04-02 at 19:39 -0400, Andy Walls wrote:
> On Thu, 2010-04-01 at 14:56 -0300, Mauro Carvalho Chehab wrote:

> > +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

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

[Mauro Carvalho Chehab]

Andy Walls wrote:
> On Fri, 2010-04-02 at 19:39 -0400, Andy Walls wrote:
>> On Thu, 2010-04-01 at 14:56 -0300, Mauro Carvalho Chehab wrote:
> 
>>> +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.

Why is it wrong? It seems appropriate to me.
> 
> 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.

-- 

Cheers,
Mauro

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

[Mauro Carvalho Chehab]

Andy Walls wrote:
> On Thu, 2010-04-01 at 14:56 -0300, Mauro Carvalho Chehab wrote:
>> 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.
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>>
>>  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 ;)

> I do have some comments....
> 
> 
>> diff --git a/drivers/media/IR/Makefile b/drivers/media/IR/Makefile
>> index 171890e..18794c7 100644
>> --- a/drivers/media/IR/Makefile
>> +++ b/drivers/media/IR/Makefile
>> @@ -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
>> diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
>> new file mode 100644
>> index 0000000..16360eb
>> --- /dev/null
>> +++ b/drivers/media/IR/ir-nec-decoder.c
>> @@ -0,0 +1,131 @@
>> +/* ir-raw-event.c - handle IR Pulse/Space event
>> + *
>> + * Copyright (C) 2010 by Mauro Carvalho Chehab <mchehab@redhat.com>
>> + *
>> + * 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>
>> +
>> +/* 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.

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.

> 
> 
>> +/* 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.

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.

> 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.

>> +/** 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;
>> +
>> +	/* 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.)

Ok, I will take a look on it.
> 
> 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).

> 
>> +	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)
>> +			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.
 
-- 

Cheers,
Mauro

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

[Andy Walls]

On Fri, 2010-04-02 at 22:32 -0300, Mauro Carvalho Chehab wrote:
> Andy Walls wrote:
 
> > 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.

Understood.


> >> +/* 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.
> 
> 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.  


> 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)?




> >> +/* 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.
> 
> 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.


> > 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

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

[Mauro Carvalho Chehab]

Andy Walls wrote:
> On Fri, 2010-04-02 at 22:32 -0300, Mauro Carvalho Chehab wrote:
>> Andy Walls wrote:
>  
>>> 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.
> 
> 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).

>> 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.

>> 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.
 
 
>>> 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.

> 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;
> 	}

Seems ok to me. I've used the same concept on my code.
 
> 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).

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

-- 

Cheers,
Mauro

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

[Andy Walls]

On Sat, 2010-04-03 at 19:56 -0300, Mauro Carvalho Chehab wrote:
> Andy Walls wrote:
> > On Fri, 2010-04-02 at 22:32 -0300, Mauro Carvalho Chehab wrote:
> >> Andy Walls wrote:
> >  
> >>> 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.
> > 
> > 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.

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.


> > 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.



> >> 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?

 
> > 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

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

[Mauro Carvalho Chehab]

Andy Walls wrote:
> On Sat, 2010-04-03 at 19:56 -0300, Mauro Carvalho Chehab wrote:
>> Andy Walls wrote:
>>> On Fri, 2010-04-02 at 22:32 -0300, Mauro Carvalho Chehab wrote:
>>>> Andy Walls wrote:
>>>  
>>>>> 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.
>>> 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.
> 
> 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.
> 
> 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.

> 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.

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.
> 
> 
>>> 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.

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.
> 
> 
> 
>>>> 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?
> 
>  
>>> 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
> 


-- 

Cheers,
Mauro

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

[Andy Walls]

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

On Sun, 2010-04-04 at 15:00 -0300, Mauro Carvalho Chehab wrote:
> Andy Walls wrote:

> > 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.

:)


 
> > 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.
> 
> 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.



> 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).


> 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


> 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


[-- Attachment #2: hpg-grey-ir-pulses.txt.gz --]
[-- Type: application/x-gzip, Size: 2520 bytes --]

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

[Mauro Carvalho Chehab]

Andy Walls wrote:
>>> 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!
 
>> 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.

>> 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. 

>> 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.

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.

> 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.
> 
> Regards,
> Andy
> 

-- 

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.

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

[Mauro Carvalho Chehab]

Mauro Carvalho Chehab wrote:
> Andy Walls wrote:
>> 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.

-- 

Cheers,
Mauro

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

[David Härdeman]

On Thu, Apr 01, 2010 at 02:56:31PM -0300, Mauro Carvalho Chehab wrote:
> 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.
...
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 663208a..6445fc9 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -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 */
> +};

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[];
};

-- 
David Härdeman

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

[David Härdeman]

On Thu, Apr 01, 2010 at 02:56:31PM -0300, Mauro Carvalho Chehab wrote:
> 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.
...
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 663208a..6445fc9 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -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 */
> +};

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[];
};

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html