Skip to contentSkip to conversation

ir-core keytable patches

PATCH In review [email protected] 2 participants 7 messages Apr 2, 2010 – Apr 8, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover ir-core keytable patches

The following patchset provides a number of bug fixes and additional features to ir-core in drivers/media/IR.

The first two patches are mostly cleanups to drivers/media/IR/ir-keytable.c while the third patch is an example of a conversion of drivers/media/dvb/ttpci/budget-ci.c to use the new functionality (rather than the drivers/media/IR/ir-functions.c functionality).

Signature · 5 lines
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#1 lore raw

PATCH 1/3 drivers/media/IR - improve keytable code

The attached patch rewrites much of the keytable code in drivers/media/IR/ir-keytable.c.

The scancodes are now inserted into the array in sorted order which allows for a binary search on lookup.

The code has also been shrunk by about 150 lines.

In addition it fixes the following bugs:

Any use of ir_seek_table() was racy.

ir_dev->driver_name is leaked between ir_input_register() and ir_input_unregister().

ir_setkeycode() unconditionally does clear_bit() on dev->keybit when removing a mapping, but there might be another mapping with a different scancode and the same keycode.

This version has been updated to incorporate patch feedback from Mauro Carvalho Chehab.

drivers/media/IR/ir-keymaps.c +1 -0
Index: ir/drivers/media/IR/ir-keymaps.c
===================================================================
@@ -39,6 +39,7 @@
 struct ir_scancode_table tabname ## _table = {			\
 	.scan = tabname,					\
 	.size = ARRAY_SIZE(tabname),				\
+	.len = ARRAY_SIZE(tabname),				\
 	.ir_type = type,					\
 	.name = #irname,					\
 };								\
drivers/media/IR/ir-keytable.c +194 -336

631 lines not on this page · raw message

include/media/ir-core.h +3 -1
Index: ir/include/media/ir-core.h
===================================================================
@@ -46,7 +46,9 @@
 
 struct ir_scancode_table {
 	struct ir_scancode	*scan;
-	int			size;
+	unsigned int		size;	/* Max number of entries */
+	unsigned int		len;	/* Used number of entries */
+	unsigned int		alloc;	/* Size of *scan in bytes */
 	u64			ir_type;
 	char			*name;
 	spinlock_t		lock;

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

#2 lore raw

PATCH 2/3 drivers/media/IR - improve keyup/keydown logic

In reply to [email protected]

The attached patch rewrites the keyup/keydown logic in drivers/media/IR/ir-keytable.c.

(applies to http://git.linuxtv.org/mchehab/ir.git)

All knowledge of keystates etc is now internal to ir-keytable.c and not scattered around ir-raw-event.c and ir-nec-decoder.c (where it doesn't belong).

In addition, I've changed the API slightly so that ir_input_dev is passed as the first argument rather than input_dev. If we're ever going to support multiple keytables we need to move towards making ir_input_dev the main interface from a driver POV and obscure away the input_dev as an implementational detail in ir-core.

Mauro, once this patch is merged I'll start working on patches to migrate drivers which use ir_input_* in ir-functions.c over to the ir-keytable.c code instead.

Changes since last version:
Clarify a comment
Refreshed to apply on top of previous patch
Let the new functions take input_dev as their main argument (as there's no agreement on this point yet)

drivers/media/IR/ir-keytable.c +102 -25
Index: ir/drivers/media/IR/ir-keytable.c
===================================================================
@@ -20,6 +20,9 @@
 #define IR_TAB_MIN_SIZE	256
 #define IR_TAB_MAX_SIZE	8192
 
+/* FIXME: IR_KEYPRESS_TIMEOUT should be protocol specific */
+#define IR_KEYPRESS_TIMEOUT 250
+
 /**
  * ir_resize_table() - resizes a scancode table if necessary
  * @rc_tab:	the ir_scancode_table to resize
@@ -250,56 +253,124 @@
 
 /**
  * ir_keyup() - generates input event to cleanup a key press
- * @input_dev:	the struct input_dev descriptor of the device
+ * @ir:		the struct ir_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().
+ * This routine is used to signal that a key has been released on the
+ * remote control. It reports a keyup input event via input_report_key().
  */
-void ir_keyup(struct input_dev *dev)
+static void ir_keyup(struct ir_input_dev *ir)
 {
-	struct ir_input_dev *ir = input_get_drvdata(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;
+	IR_dprintk(1, "keyup key 0x%04x\n", ir->last_keycode);
+	input_report_key(ir->input_dev, ir->last_keycode, 0);
+	input_sync(ir->input_dev);
+	ir->keypressed = false;
+}
+
+/**
+ * ir_timer_keyup() - generates a keyup event after a timeout
+ * @cookie:	a pointer to struct ir_input_dev passed to setup_timer()
+ *
+ * This routine will generate a keyup event some time after a keydown event
+ * is generated when no further activity has been detected.
+ */
+static void ir_timer_keyup(unsigned long cookie)
+{
+	struct ir_input_dev *ir = (struct ir_input_dev *)cookie;
+	unsigned long flags;
+
+	/*
+	 * ir->keyup_jiffies is used to prevent a race condition if a
+	 * hardware interrupt occurs at this point and the keyup timer
+	 * event is moved further into the future as a result.
+	 *
+	 * The timer will then be reactivated and this function called
+	 * again in the future. We need to exit gracefully in that case
+	 * to allow the input subsystem to do its auto-repeat magic or
+	 * a keyup event might follow immediately after the keydown.
+	 */
+	spin_lock_irqsave(&ir->keylock, flags);
+	if (time_is_after_eq_jiffies(ir->keyup_jiffies))
+		ir_keyup(ir);
+	spin_unlock_irqrestore(&ir->keylock, flags);
+}
+
+/**
+ * ir_repeat() - notifies the IR core that a key is still pressed
+ * @dev:	the struct input_dev descriptor of the device
+ *
+ * This routine is used by IR decoders when a repeat message which does
+ * not include the necessary bits to reproduce the scancode has been
+ * received.
+ */
+void ir_repeat(struct input_dev *dev)
+{
+	unsigned long flags;
+	struct ir_input_dev *ir = input_get_drvdata(dev);
+
+	spin_lock_irqsave(&ir->keylock, flags);
+
+	if (!ir->keypressed)
+		goto out;
+
+	ir->keyup_jiffies = jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT);
+	mod_timer(&ir->timer_keyup, ir->keyup_jiffies);
+
+out:
+	spin_unlock_irqrestore(&ir->keylock, flags);
 }
-EXPORT_SYMBOL_GPL(ir_keyup);
+EXPORT_SYMBOL_GPL(ir_repeat);
 
 /**
  * ir_keydown() - generates input event for a key press
- * @input_dev:	the struct input_dev descriptor of the device
+ * @dev:	the struct input_dev descriptor of the device
  * @scancode:	the scancode that we're seeking
+ * @toggle:	the toggle value (protocol dependent, if the protocol doesn't
+ *		support toggle values, this should be set to zero)
  *
  * 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)
+void ir_keydown(struct input_dev *dev, int scancode, u8 toggle)
 {
+	unsigned long flags;
 	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);
+	spin_lock_irqsave(&ir->keylock, flags);
 
-	if (KEY_RESERVED == keycode)
-		return;
+	/* Repeat event? */
+	if (ir->keypressed &&
+	    ir->last_scancode == scancode &&
+	    ir->last_toggle == toggle)
+		goto set_timer;
+
+	/* Release old keypress */
+	ir_keyup(ir);
+
+	ir->last_scancode = scancode;
+	ir->last_toggle = toggle;
+	ir->last_keycode = keycode;
 
-	ir->keycode = keycode;
-	ir->keypressed = 1;
+	if (keycode == KEY_RESERVED)
+		goto out;
 
+	/* Register a keypress */
+	ir->keypressed = true;
 	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);
+		   dev->name, keycode, scancode);
+	input_report_key(dev, ir->last_keycode, 1);
 	input_sync(dev);
-	
+
+set_timer:
+	ir->keyup_jiffies = jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT);
+	mod_timer(&ir->timer_keyup, ir->keyup_jiffies);
+out:
+	spin_unlock_irqrestore(&ir->keylock, flags);
 }
 EXPORT_SYMBOL_GPL(ir_keydown);
 
@@ -352,8 +423,12 @@
 	input_dev->getkeycode = ir_getkeycode;
 	input_dev->setkeycode = ir_setkeycode;
 	input_set_drvdata(input_dev, ir_dev);
+	ir_dev->input_dev = input_dev;
 
 	spin_lock_init(&ir_dev->rc_tab.lock);
+	spin_lock_init(&ir_dev->keylock);
+	setup_timer(&ir_dev->timer_keyup, ir_timer_keyup, (unsigned long)ir_dev);
+
 	ir_dev->rc_tab.name = rc_tab->name;
 	ir_dev->rc_tab.ir_type = rc_tab->ir_type;
 	ir_dev->rc_tab.alloc = roundup_pow_of_two(rc_tab->len *
@@ -370,6 +445,8 @@
 		   ir_dev->rc_tab.size, ir_dev->rc_tab.alloc);
 
 	set_bit(EV_KEY, input_dev->evbit);
+	set_bit(EV_REP, input_dev->evbit);
+
 	if (ir_setkeytable(input_dev, &ir_dev->rc_tab, rc_tab)) {
 		rc = -ENOMEM;
 		goto out_table;
@@ -412,7 +489,7 @@
 		return;
 
 	IR_dprintk(1, "Freed keycode table\n");
-
+	del_timer_sync(&ir_dev->timer_keyup);
 	rc_tab = &ir_dev->rc_tab;
 	rc_tab->size = 0;
 	kfree(rc_tab->scan);
drivers/media/IR/ir-nec-decoder.c +2 -5
Index: ir/drivers/media/IR/ir-nec-decoder.c
===================================================================
@@ -179,8 +179,7 @@
 	if (is_repeat(evs, len, *pos)) {
 		*pos += 2;
 		if (ir->keypressed) {
-			mod_timer(&ir->raw->timer_keyup,
-				jiffies + msecs_to_jiffies(REPEAT_TIME));
+			ir_repeat(input_dev);
 			IR_dprintk(1, "NEC repeat event\n");
 			return 1;
 		} else {
@@ -237,9 +236,7 @@
 	}
 
 	IR_dprintk(1, "NEC scancode 0x%04x\n", ircode);
-	ir_keydown(input_dev, ircode);
-	mod_timer(&ir->raw->timer_keyup,
-		  jiffies + msecs_to_jiffies(REPEAT_TIME));
+	ir_keydown(input_dev, ircode, 0);
 
 	return 1;
 err:
drivers/media/IR/ir-raw-event.c +0 -14
Index: ir/drivers/media/IR/ir-raw-event.c
===================================================================
@@ -53,13 +53,6 @@
 /* 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;
-
-	ir_keyup(input_dev);
-}
-
 int ir_raw_event_register(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir = input_get_drvdata(input_dev);
@@ -72,11 +65,6 @@
 	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);
 	if (rc < 0) {
 		kfree (ir->raw);
@@ -103,8 +91,6 @@
 	if (!ir->raw)
 		return;
 
-	del_timer_sync(&ir->raw->timer_keyup);
-
 	RUN_DECODER(raw_unregister, input_dev);
 
 	kfifo_free(&ir->raw->kfifo);
include/media/ir-core.h +10 -5
Index: ir/include/media/ir-core.h
===================================================================
@@ -70,7 +70,6 @@
 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 {
@@ -80,10 +79,16 @@
 	unsigned long			devno;		/* device number */
 	const struct ir_dev_props	*props;		/* Device properties */
 	struct ir_raw_event_ctrl	*raw;		/* for raw pulse/space events */
+	struct input_dev		*input_dev;	/* the input device associated with this device */
 
 	/* key info - needed by IR keycode handlers */
-	u32				keycode;	/* linux key code */
-	int				keypressed;	/* current state */
+	spinlock_t			keylock;	/* protects the below members */
+	bool				keypressed;	/* current state */
+	unsigned long			keyup_jiffies;	/* when should the current keypress be released? */
+	struct timer_list		timer_keyup;	/* timer for releasing a keypress */
+	u32				last_keycode;	/* keycode of last command */
+	u32				last_scancode;	/* scancode of last command */
+	u8				last_toggle;	/* toggle of last command */
 };
 
 struct ir_raw_handler {
@@ -102,8 +107,8 @@
 
 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);
+void ir_repeat(struct input_dev *dev);
+void ir_keydown(struct input_dev *dev, int scancode, u8 toggle);
 int ir_input_register(struct input_dev *dev,
 		      const struct ir_scancode_table *ir_codes,
 		      const struct ir_dev_props *props,

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

#3 lore raw

PATCH 3/3 Convert drivers/media/dvb/ttpci/budget-ci.c to use ir-core

In reply to [email protected]

This patch converts drivers/media/dvb/ttpci/budget-ci.c to use ir-core rather than rolling its own keydown timeout handler and reporting keys via drivers/media/IR/ir-functions.c.

drivers/media/dvb/ttpci/budget-ci.c +3 -35
Index: ir/drivers/media/dvb/ttpci/budget-ci.c
===================================================================
@@ -35,7 +35,7 @@
 #include <linux/interrupt.h>
 #include <linux/input.h>
 #include <linux/spinlock.h>
-#include <media/ir-common.h>
+#include <media/ir-core.h>
 
 #include "budget.h"
 
@@ -82,12 +82,6 @@
 #define SLOTSTATUS_READY	8
 #define SLOTSTATUS_OCCUPIED	(SLOTSTATUS_PRESENT|SLOTSTATUS_RESET|SLOTSTATUS_READY)
 
-/*
- * Milliseconds during which a key is regarded as pressed.
- * If an identical command arrives within this time, the timer will start over.
- */
-#define IR_KEYPRESS_TIMEOUT	250
-
 /* RC5 device wildcard */
 #define IR_DEVICE_ANY		255
 
@@ -104,12 +98,9 @@
 struct budget_ci_ir {
 	struct input_dev *dev;
 	struct tasklet_struct msp430_irq_tasklet;
-	struct timer_list timer_keyup;
 	char name[72]; /* 40 + 32 for (struct saa7146_dev).name */
 	char phys[32];
-	struct ir_input_state state;
 	int rc5_device;
-	u32 last_raw;
 	u32 ir_key;
 	bool have_command;
 };
@@ -124,18 +115,11 @@
 	u8 tuner_pll_address; /* used for philips_tdm1316l configs */
 };
 
-static void msp430_ir_keyup(unsigned long data)
-{
-	struct budget_ci_ir *ir = (struct budget_ci_ir *) data;
-	ir_input_nokey(ir->dev, &ir->state);
-}
-
 static void msp430_ir_interrupt(unsigned long data)
 {
 	struct budget_ci *budget_ci = (struct budget_ci *) data;
 	struct input_dev *dev = budget_ci->ir.dev;
 	u32 command = ttpci_budget_debiread(&budget_ci->budget, DEBINOSWAP, DEBIADDR_IR, 2, 1, 0) >> 8;
-	u32 raw;
 
 	/*
 	 * The msp430 chip can generate two different bytes, command and device
@@ -171,20 +155,12 @@
 		return;
 	budget_ci->ir.have_command = false;
 
+	/* FIXME: We should generate complete scancodes with device info */
 	if (budget_ci->ir.rc5_device != IR_DEVICE_ANY &&
 	    budget_ci->ir.rc5_device != (command & 0x1f))
 		return;
 
-	/* Is this a repeated key sequence? (same device, command, toggle) */
-	raw = budget_ci->ir.ir_key | (command << 8);
-	if (budget_ci->ir.last_raw != raw || !timer_pending(&budget_ci->ir.timer_keyup)) {
-		ir_input_nokey(dev, &budget_ci->ir.state);
-		ir_input_keydown(dev, &budget_ci->ir.state,
-				 budget_ci->ir.ir_key);
-		budget_ci->ir.last_raw = raw;
-	}
-
-	mod_timer(&budget_ci->ir.timer_keyup, jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT));
+	ir_keydown(dev, budget_ci->ir.ir_key, (command & 0x20) ? 1 : 0);
 }
 
 static int msp430_ir_init(struct budget_ci *budget_ci)
@@ -251,11 +227,6 @@
 
 	ir_input_init(input_dev, &budget_ci->ir.state, IR_TYPE_RC5);
 
-	/* initialise the key-up timeout handler */
-	init_timer(&budget_ci->ir.timer_keyup);
-	budget_ci->ir.timer_keyup.function = msp430_ir_keyup;
-	budget_ci->ir.timer_keyup.data = (unsigned long) &budget_ci->ir;
-	budget_ci->ir.last_raw = 0xffff; /* An impossible value */
 	error = ir_input_register(input_dev, ir_codes, NULL, MODULE_NAME);
 	if (error) {
 		printk(KERN_ERR "budget_ci: could not init driver for IR device (code %d)\n", error);
@@ -284,9 +255,6 @@
 	saa7146_setgpio(saa, 3, SAA7146_GPIO_INPUT);
 	tasklet_kill(&budget_ci->ir.msp430_irq_tasklet);
 
-	del_timer_sync(&dev->timer);
-	ir_input_nokey(dev, &budget_ci->ir.state);
-
 	ir_input_unregister(dev);
 }
 

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

#4 lore raw

Re: [patch 3/3] Convert drivers/media/dvb/ttpci/budget-ci.c to use ir-core

Quoted 3 lines · [email protected] wrote:

This patch converts drivers/media/dvb/ttpci/budget-ci.c to use ir-core rather than rolling its own keydown timeout handler and reporting keys via drivers/media/IR/ir-functions.c.

Hmm... had you test this patch? It got me an error here:

drivers/media/dvb/ttpci/budget-ci.c: In function ‘msp430_ir_init’:
drivers/media/dvb/ttpci/budget-ci.c:228: error: implicit declaration of function ‘ir_input_init’ drivers/media/dvb/ttpci/budget-ci.c:228: error: ‘struct budget_ci_ir’ has no member named ‘state’

The fix is trivial. Just drop this line:

        ir_input_init(input_dev, &budget_ci->ir.state, IR_TYPE_RC5);

It shouldn't cause any troubles, since the only things this function currently do are:

        ir->ir_type = ir_type;

        if (repeat)
                set_bit(EV_REP, dev->evbit);

As the repeat is inside ir-core, and the ir struct is not used anymore, this removal should cause no harm.

So, I am dropping the line at the code I'm committing at v4l-dvb.git, to avoid bisect breakages.

Quoted 112 lines

Index: ir/drivers/media/dvb/ttpci/budget-ci.c
===================================================================

drivers/media/dvb/ttpci/budget-ci.c +3 -36
@@ -35,7 +35,7 @@
 #include <linux/interrupt.h>
 #include <linux/input.h>
 #include <linux/spinlock.h>
-#include <media/ir-common.h>
+#include <media/ir-core.h>
 
 #include "budget.h"
 
@@ -82,12 +82,6 @@
 #define SLOTSTATUS_READY	8
 #define SLOTSTATUS_OCCUPIED	(SLOTSTATUS_PRESENT|SLOTSTATUS_RESET|SLOTSTATUS_READY)
 
-/*
- * Milliseconds during which a key is regarded as pressed.
- * If an identical command arrives within this time, the timer will start over.
- */
-#define IR_KEYPRESS_TIMEOUT	250
-
 /* RC5 device wildcard */
 #define IR_DEVICE_ANY		255
 
@@ -104,12 +98,9 @@
 struct budget_ci_ir {
 	struct input_dev *dev;
 	struct tasklet_struct msp430_irq_tasklet;
-	struct timer_list timer_keyup;
 	char name[72]; /* 40 + 32 for (struct saa7146_dev).name */
 	char phys[32];
-	struct ir_input_state state;
 	int rc5_device;
-	u32 last_raw;
 	u32 ir_key;
 	bool have_command;
 };
@@ -124,18 +115,11 @@
 	u8 tuner_pll_address; /* used for philips_tdm1316l configs */
 };
 
-static void msp430_ir_keyup(unsigned long data)
-{
-	struct budget_ci_ir *ir = (struct budget_ci_ir *) data;
-	ir_input_nokey(ir->dev, &ir->state);
-}
-
 static void msp430_ir_interrupt(unsigned long data)
 {
 	struct budget_ci *budget_ci = (struct budget_ci *) data;
 	struct input_dev *dev = budget_ci->ir.dev;
 	u32 command = ttpci_budget_debiread(&budget_ci->budget, DEBINOSWAP, DEBIADDR_IR, 2, 1, 0) >> 8;
-	u32 raw;
 
 	/*
 	 * The msp430 chip can generate two different bytes, command and device
@@ -171,20 +155,12 @@
 		return;
 	budget_ci->ir.have_command = false;
 
+	/* FIXME: We should generate complete scancodes with device info */
 	if (budget_ci->ir.rc5_device != IR_DEVICE_ANY &&
 	    budget_ci->ir.rc5_device != (command & 0x1f))
 		return;
 
-	/* Is this a repeated key sequence? (same device, command, toggle) */
-	raw = budget_ci->ir.ir_key | (command << 8);
-	if (budget_ci->ir.last_raw != raw || !timer_pending(&budget_ci->ir.timer_keyup)) {
-		ir_input_nokey(dev, &budget_ci->ir.state);
-		ir_input_keydown(dev, &budget_ci->ir.state,
-				 budget_ci->ir.ir_key);
-		budget_ci->ir.last_raw = raw;
-	}
-
-	mod_timer(&budget_ci->ir.timer_keyup, jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT));
+	ir_keydown(dev, budget_ci->ir.ir_key, (command & 0x20) ? 1 : 0);
 }
 
 static int msp430_ir_init(struct budget_ci *budget_ci)
@@ -251,11 +227,6 @@
 
 	ir_input_init(input_dev, &budget_ci->ir.state, IR_TYPE_RC5);
 
-	/* initialise the key-up timeout handler */
-	init_timer(&budget_ci->ir.timer_keyup);
-	budget_ci->ir.timer_keyup.function = msp430_ir_keyup;
-	budget_ci->ir.timer_keyup.data = (unsigned long) &budget_ci->ir;
-	budget_ci->ir.last_raw = 0xffff; /* An impossible value */
 	error = ir_input_register(input_dev, ir_codes, NULL, MODULE_NAME);
 	if (error) {
 		printk(KERN_ERR "budget_ci: could not init driver for IR device (code %d)\n", error);
@@ -284,9 +255,6 @@
 	saa7146_setgpio(saa, 3, SAA7146_GPIO_INPUT);
 	tasklet_kill(&budget_ci->ir.msp430_irq_tasklet);
 
-	del_timer_sync(&dev->timer);
-	ir_input_nokey(dev, &budget_ci->ir.state);
-
 	ir_input_unregister(dev);
 }
 
 
--

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

Signature · 2 lines
Cheers,
Mauro
#5 lore raw

Re: [patch 3/3] Convert drivers/media/dvb/ttpci/budget-ci.c to use ir-core

Quoted 6 lines · Mauro Carvalho Chehab
Quoted 3 lines · [email protected] wrote:

This patch converts drivers/media/dvb/ttpci/budget-ci.c to use ir-core rather than rolling its own keydown timeout handler and reporting keys via drivers/media/IR/ir-functions.c.

Hmm... had you test this patch? It got me an error here:

Sorry, I must have sent you the wrong one :)

Quoted 14 lines

drivers/media/dvb/ttpci/budget-ci.c: In function ‘msp430_ir_init’:
drivers/media/dvb/ttpci/budget-ci.c:228: error: implicit declaration of function ‘ir_input_init’ drivers/media/dvb/ttpci/budget-ci.c:228: error: ‘struct budget_ci_ir’ has no member named ‘state’

The fix is trivial. Just drop this line:

       ir_input_init(input_dev, &budget_ci->ir.state, IR_TYPE_RC5);

It shouldn't cause any troubles, since the only things this function currently do are:

       ir->ir_type = ir_type;

       if (repeat)
               set_bit(EV_REP, dev->evbit);
Quoted 5 lines

As the repeat is inside ir-core, and the ir struct is not used anymore, this removal should cause no harm.

So, I am dropping the line at the code I'm committing at v4l-dvb.git, to avoid bisect breakages.

You're entirely correct, that line should have been dropped (I even sent the same thing as part of my latest patch series before I read this mail, but if you can fixup the original patch that'd be even better).

Signature · 5 lines
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#6 lore raw

Re: [patch 3/3] Convert drivers/media/dvb/ttpci/budget-ci.c to use ir-core

Quoted 31 lines · David Härdeman
Quoted 6 lines · Mauro Carvalho Chehab
Quoted 3 lines · [email protected] wrote:

This patch converts drivers/media/dvb/ttpci/budget-ci.c to use ir-core rather than rolling its own keydown timeout handler and reporting keys via drivers/media/IR/ir-functions.c.

Hmm... had you test this patch? It got me an error here:

Sorry, I must have sent you the wrong one :)

Quoted 21 lines

drivers/media/dvb/ttpci/budget-ci.c: In function ‘msp430_ir_init’:
drivers/media/dvb/ttpci/budget-ci.c:228: error: implicit declaration of function ‘ir_input_init’ drivers/media/dvb/ttpci/budget-ci.c:228: error: ‘struct budget_ci_ir’ has no member named ‘state’

The fix is trivial. Just drop this line:

       ir_input_init(input_dev, &budget_ci->ir.state, IR_TYPE_RC5);

It shouldn't cause any troubles, since the only things this function currently do are:

       ir->ir_type = ir_type;

       if (repeat)
               set_bit(EV_REP, dev->evbit);

As the repeat is inside ir-core, and the ir struct is not used anymore, this removal
should cause no harm.

Quoted 7 lines
Quoted 3 lines

So, I am dropping the line at the code I'm committing at v4l-dvb.git, to avoid bisect
breakages.

You're entirely correct, that line should have been dropped (I even sent the same thing as part of my latest patch series before I read this mail, but if you can fixup the original patch that'd be even better).

While I don't care much on experimental trees, I always do a make allyesconfig and try to compile all drivers before pushing on my master tree. This helps to avoid some silly mistakes to go upstream ;)

Signature · 2 lines
Cheers,
Mauro