[PATCH 0/4] IR/imon: split out mouse events and fix bugs

[PATCH 0/4] IR/imon: split out mouse events and fix bugs

[Jarod Wilson]

These patches make the imon driver slightly less distasteful. The meat of
the changes are from David's patch to split the IR mouse mode and
panel/knob events out onto their own input device, leaving just IR keys to
come through the IR device. This facilitates further abstraction of the
ir/rc-core interface, but allows us to get these patches in ahead of
David's major reshuffle that is targeted for post-2.6.37-rc1 (basically,
after Dmitry's large keycode patches are merged in mainline).

Additionally, while David's patch unknowingly addressed many of the issues
in https://bugzilla.kernel.org/show_bug.cgi?id=16351, there are a few more
issues addressed by the spinlock patch (at least, in theory, since in
practice, it doesn't really seem to matter much to me, but Anssi suggested
that some locking may be a good idea in the bug :).

Finally, there's a bit of reshuffling of auto-config bits for the 0xffdc
imon devices so the mce-only ones get set up w/the mce key table by
default instead of the imon pad one (based on input from Anders Eriksson
over on the lirc list).

David Härdeman (1):
      imon: split mouse events to a separate input dev

Jarod Wilson (3):
      IR: export ir_keyup so imon driver can use it directly
      IR/imon: protect ictx's kc and last_keycode w/spinlock
      IR/imon: set up mce-only devices w/mce keytable

 drivers/media/IR/imon.c        |  583 +++++++++++++++++++++++-----------------
 drivers/media/IR/ir-keytable.c |    3 +-
 include/media/ir-core.h        |    1 +
 3 files changed, 344 insertions(+), 243 deletions(-)

-- 
Jarod Wilson
jarod@redhat.com

[PATCH 1/4] IR: export ir_keyup so imon driver can use it directly

[Jarod Wilson]

>From d31919ac08ba9a203bd673bbed18e78293ceaa68 Mon Sep 17 00:00:00 2001
From: Jarod Wilson <jarod@redhat.com>
Date: Wed, 15 Sep 2010 14:31:12 -0400
Subject: [PATCH 1/4] IR: export ir_keyup so imon driver can use it directly
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The imon driver currently reimplements its own version of ir_keyup
(along with key release timer functionality also already present in the
core IR code). A follow-up imon patch will make use of ir_keyup and the
IR stack's key release code.

Trivial extraction from David Härdeman's pending rc-core merge and
device interface abstraction patchset to facilitate merging a patch
based on his imon input dev split patch ahead of the larger churn, which
is slated for post-2.6.37-rc1 (after Dmitry's large keycode patches are
merged in mainline).

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/ir-keytable.c |    3 ++-
 include/media/ir-core.h        |    1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/media/IR/ir-keytable.c b/drivers/media/IR/ir-keytable.c
index 7961d59..59510cd 100644
--- a/drivers/media/IR/ir-keytable.c
+++ b/drivers/media/IR/ir-keytable.c
@@ -285,7 +285,7 @@ EXPORT_SYMBOL_GPL(ir_g_keycode_from_table);
  * 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().
  */
-static void ir_keyup(struct ir_input_dev *ir)
+void ir_keyup(struct ir_input_dev *ir)
 {
 	if (!ir->keypressed)
 		return;
@@ -295,6 +295,7 @@ static void ir_keyup(struct ir_input_dev *ir)
 	input_sync(ir->input_dev);
 	ir->keypressed = false;
 }
+EXPORT_SYMBOL_GPL(ir_keyup);
 
 /**
  * ir_timer_keyup() - generates a keyup event after a timeout
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index eb7fddf..4dd43d4 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -157,6 +157,7 @@ void ir_input_unregister(struct input_dev *input_dev);
 
 void ir_repeat(struct input_dev *dev);
 void ir_keydown(struct input_dev *dev, int scancode, u8 toggle);
+void ir_keyup(struct ir_input_dev *ir);
 u32 ir_g_keycode_from_table(struct input_dev *input_dev, u32 scancode);
 
 /* From ir-raw-event.c */
-- 
1.7.2.2


-- 
Jarod Wilson
jarod@redhat.com

[PATCH 2/4] imon: split mouse events to a separate input dev

[Jarod Wilson]

>From 4ceb1642b756e7a11753c6fae645806d2514c54a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20H=C3=A4rdeman?= <david@hardeman.nu>
Date: Wed, 15 Sep 2010 14:42:07 -0400
Subject: [PATCH 2/4] imon: split mouse events to a separate input dev
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is a stab at separating the mouse (and front panel/knob) events
out to a separate input device. This is necessary in preparation for
the next patch which makes the rc-core input dev opaque to rc
drivers.

I can't verify the correctness of the patch beyond the fact that it
compiles without warnings. The driver has resisted most of my
attempts at understanding it properly...for example, the double calls
to le64_to_cpu() and be64_to_cpu() which are applied in
imon_incoming_packet() and imon_panel_key_lookup() would amount
to a bswab64() call, irregardless of the cpu endianness, and I think
the code wouldn't have worked on a big-endian machine...

Signed-off-by: David Härdeman <david@hardeman.nu>

- Minor alterations to apply with minimal core IR changes
- Use timer for imon keys too, since its entirely possible for the
  receiver to miss release codes (either by way of another key being
  pressed while the first is held or by the remote pointing away from
  the recevier when the key is release. yes, I know, its ugly).
- Bump driver version number, since this is a fairly significant change
  (for the much much better).

Tested successfully w/an imon knob receiver.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/imon.c |  273 +++++++++++++++++++++++++++-------------------
 1 files changed, 160 insertions(+), 113 deletions(-)

diff --git a/drivers/media/IR/imon.c b/drivers/media/IR/imon.c
index c185422..d36fe72 100644
--- a/drivers/media/IR/imon.c
+++ b/drivers/media/IR/imon.c
@@ -44,7 +44,7 @@
 #define MOD_AUTHOR	"Jarod Wilson <jarod@wilsonet.com>"
 #define MOD_DESC	"Driver for SoundGraph iMON MultiMedia IR/Display"
 #define MOD_NAME	"imon"
-#define MOD_VERSION	"0.9.1"
+#define MOD_VERSION	"0.9.2"
 
 #define DISPLAY_MINOR_BASE	144
 #define DEVICE_NAME	"lcd%d"
@@ -121,21 +121,25 @@ struct imon_context {
 	u16 vendor;			/* usb vendor ID */
 	u16 product;			/* usb product ID */
 
-	struct input_dev *idev;		/* input device for remote */
+	struct input_dev *rdev;		/* input device for remote */
+	struct input_dev *idev;		/* input device for panel & IR mouse */
 	struct input_dev *touch;	/* input device for touchscreen */
 
 	u32 kc;				/* current input keycode */
 	u32 last_keycode;		/* last reported input keycode */
+	u32 rc_scancode;		/* the computed remote scancode */
+	u8 rc_toggle;			/* the computed remote toggle bit */
 	u64 ir_type;			/* iMON or MCE (RC6) IR protocol? */
-	u8 mce_toggle_bit;		/* last mce toggle bit */
 	bool release_code;		/* some keys send a release code */
 
 	u8 display_type;		/* store the display type */
 	bool pad_mouse;			/* toggle kbd(0)/mouse(1) mode */
 
+	char name_rdev[128];		/* rc input device name */
+	char phys_rdev[64];		/* rc input device phys path */
+
 	char name_idev[128];		/* input device name */
 	char phys_idev[64];		/* input device phys path */
-	struct timer_list itimer;	/* input device timer, need for rc6 */
 
 	char name_touch[128];		/* touch screen name */
 	char phys_touch[64];		/* touch screen phys path */
@@ -956,17 +960,6 @@ static void usb_tx_callback(struct urb *urb)
 }
 
 /**
- * mce/rc6 keypresses have no distinct release code, use timer
- */
-static void imon_mce_timeout(unsigned long data)
-{
-	struct imon_context *ictx = (struct imon_context *)data;
-
-	input_report_key(ictx->idev, ictx->last_keycode, 0);
-	input_sync(ictx->idev);
-}
-
-/**
  * report touchscreen input
  */
 static void imon_touch_display_timeout(unsigned long data)
@@ -1006,9 +999,6 @@ int imon_ir_change_protocol(void *priv, u64 ir_type)
 		dev_dbg(dev, "Configuring IR receiver for MCE protocol\n");
 		ir_proto_packet[0] = 0x01;
 		pad_mouse = false;
-		init_timer(&ictx->itimer);
-		ictx->itimer.data = (unsigned long)ictx;
-		ictx->itimer.function = imon_mce_timeout;
 		break;
 	case IR_TYPE_UNKNOWN:
 	case IR_TYPE_OTHER:
@@ -1147,20 +1137,21 @@ static int stabilize(int a, int b, u16 timeout, u16 threshold)
 	return result;
 }
 
-static u32 imon_remote_key_lookup(struct imon_context *ictx, u32 hw_code)
+static u32 imon_remote_key_lookup(struct imon_context *ictx, u32 scancode)
 {
-	u32 scancode = be32_to_cpu(hw_code);
 	u32 keycode;
 	u32 release;
 	bool is_release_code = false;
 
 	/* Look for the initial press of a button */
-	keycode = ir_g_keycode_from_table(ictx->idev, scancode);
+	keycode = ir_g_keycode_from_table(ictx->rdev, scancode);
+	ictx->rc_toggle = 0x0;
+	ictx->rc_scancode = scancode;
 
 	/* Look for the release of a button */
 	if (keycode == KEY_RESERVED) {
 		release = scancode & ~0x4000;
-		keycode = ir_g_keycode_from_table(ictx->idev, release);
+		keycode = ir_g_keycode_from_table(ictx->rdev, release);
 		if (keycode != KEY_RESERVED)
 			is_release_code = true;
 	}
@@ -1170,9 +1161,8 @@ static u32 imon_remote_key_lookup(struct imon_context *ictx, u32 hw_code)
 	return keycode;
 }
 
-static u32 imon_mce_key_lookup(struct imon_context *ictx, u32 hw_code)
+static u32 imon_mce_key_lookup(struct imon_context *ictx, u32 scancode)
 {
-	u32 scancode = be32_to_cpu(hw_code);
 	u32 keycode;
 
 #define MCE_KEY_MASK 0x7000
@@ -1186,18 +1176,21 @@ static u32 imon_mce_key_lookup(struct imon_context *ictx, u32 hw_code)
 	 * but we can't or them into all codes, as some keys are decoded in
 	 * a different way w/o the same use of the toggle bit...
 	 */
-	if ((scancode >> 24) & 0x80)
+	if (scancode & 0x80000000)
 		scancode = scancode | MCE_KEY_MASK | MCE_TOGGLE_BIT;
 
-	keycode = ir_g_keycode_from_table(ictx->idev, scancode);
+	ictx->rc_scancode = scancode;
+	keycode = ir_g_keycode_from_table(ictx->rdev, scancode);
+
+	/* not used in mce mode, but make sure we know its false */
+	ictx->release_code = false;
 
 	return keycode;
 }
 
-static u32 imon_panel_key_lookup(u64 hw_code)
+static u32 imon_panel_key_lookup(u64 code)
 {
 	int i;
-	u64 code = be64_to_cpu(hw_code);
 	u32 keycode = KEY_RESERVED;
 
 	for (i = 0; i < ARRAY_SIZE(imon_panel_key_table); i++) {
@@ -1284,8 +1277,7 @@ static void imon_pad_to_keys(struct imon_context *ictx, unsigned char *buf)
 	int dir = 0;
 	char rel_x = 0x00, rel_y = 0x00;
 	u16 timeout, threshold;
-	u64 temp_key;
-	u32 remote_key;
+	u32 scancode = KEY_RESERVED;
 
 	/*
 	 * The imon directional pad functions more like a touchpad. Bytes 3 & 4
@@ -1314,21 +1306,27 @@ static void imon_pad_to_keys(struct imon_context *ictx, unsigned char *buf)
 				}
 				buf[2] = dir & 0xFF;
 				buf[3] = (dir >> 8) & 0xFF;
-				memcpy(&temp_key, buf, sizeof(temp_key));
-				remote_key = (u32) (le64_to_cpu(temp_key)
-						    & 0xffffffff);
-				ictx->kc = imon_remote_key_lookup(ictx,
-								  remote_key);
+				scancode = be32_to_cpu(*((u32 *)buf));
 			}
 		} else {
+			/*
+			 * Hack alert: instead of using keycodes, we have
+			 * to use hard-coded scancodes here...
+			 */
 			if (abs(rel_y) > abs(rel_x)) {
 				buf[2] = (rel_y > 0) ? 0x7F : 0x80;
 				buf[3] = 0;
-				ictx->kc = (rel_y > 0) ? KEY_DOWN : KEY_UP;
+				if (rel_y > 0)
+					scancode = 0x01007f00; /* KEY_DOWN */
+				else
+					scancode = 0x01008000; /* KEY_UP */
 			} else {
 				buf[2] = 0;
 				buf[3] = (rel_x > 0) ? 0x7F : 0x80;
-				ictx->kc = (rel_x > 0) ? KEY_RIGHT : KEY_LEFT;
+				if (rel_x > 0)
+					scancode = 0x0100007f; /* KEY_RIGHT */
+				else
+					scancode = 0x01000080; /* KEY_LEFT */
 			}
 		}
 
@@ -1370,29 +1368,43 @@ static void imon_pad_to_keys(struct imon_context *ictx, unsigned char *buf)
 			}
 			buf[2] = dir & 0xFF;
 			buf[3] = (dir >> 8) & 0xFF;
-			memcpy(&temp_key, buf, sizeof(temp_key));
-			remote_key = (u32) (le64_to_cpu(temp_key) & 0xffffffff);
-			ictx->kc = imon_remote_key_lookup(ictx, remote_key);
+			scancode = be32_to_cpu(*((u32 *)buf));
 		} else {
+			/*
+			 * Hack alert: instead of using keycodes, we have
+			 * to use hard-coded scancodes here...
+			 */
 			if (abs(rel_y) > abs(rel_x)) {
 				buf[2] = (rel_y > 0) ? 0x7F : 0x80;
 				buf[3] = 0;
-				ictx->kc = (rel_y > 0) ? KEY_DOWN : KEY_UP;
+				if (rel_y > 0)
+					scancode = 0x01007f00; /* KEY_DOWN */
+				else
+					scancode = 0x01008000; /* KEY_UP */
 			} else {
 				buf[2] = 0;
 				buf[3] = (rel_x > 0) ? 0x7F : 0x80;
-				ictx->kc = (rel_x > 0) ? KEY_RIGHT : KEY_LEFT;
+				if (rel_x > 0)
+					scancode = 0x0100007f; /* KEY_RIGHT */
+				else
+					scancode = 0x01000080; /* KEY_LEFT */
 			}
 		}
 	}
+
+	if (scancode)
+		ictx->kc = imon_remote_key_lookup(ictx, scancode);
 }
 
+/**
+ * figure out if these is a press or a release. We don't actually
+ * care about repeats, as those will be auto-generated within the IR
+ * subsystem for repeating scancodes.
+ */
 static int imon_parse_press_type(struct imon_context *ictx,
 				 unsigned char *buf, u8 ktype)
 {
 	int press_type = 0;
-	int rep_delay = ictx->idev->rep[REP_DELAY];
-	int rep_period = ictx->idev->rep[REP_PERIOD];
 
 	/* key release of 0x02XXXXXX key */
 	if (ictx->kc == KEY_RESERVED && buf[0] == 0x02 && buf[3] == 0x00)
@@ -1408,22 +1420,10 @@ static int imon_parse_press_type(struct imon_context *ictx,
 		 buf[2] == 0x81 && buf[3] == 0xb7)
 		ictx->kc = ictx->last_keycode;
 
-	/* mce-specific button handling */
+	/* mce-specific button handling, no keyup events */
 	else if (ktype == IMON_KEY_MCE) {
-		/* initial press */
-		if (ictx->kc != ictx->last_keycode
-		    || buf[2] != ictx->mce_toggle_bit) {
-			ictx->last_keycode = ictx->kc;
-			ictx->mce_toggle_bit = buf[2];
-			press_type = 1;
-			mod_timer(&ictx->itimer,
-				  jiffies + msecs_to_jiffies(rep_delay));
-		/* repeat */
-		} else {
-			press_type = 2;
-			mod_timer(&ictx->itimer,
-				  jiffies + msecs_to_jiffies(rep_period));
-		}
+		ictx->rc_toggle = buf[2];
+		press_type = 1;
 
 	/* incoherent or irrelevant data */
 	} else if (ictx->kc == KEY_RESERVED)
@@ -1452,36 +1452,38 @@ static void imon_incoming_packet(struct imon_context *ictx,
 	u32 kc;
 	bool norelease = false;
 	int i;
-	u64 temp_key;
-	u64 panel_key = 0;
-	u32 remote_key = 0;
+	u64 scancode;
 	struct input_dev *idev = NULL;
+	struct ir_input_dev *irdev = NULL;
 	int press_type = 0;
 	int msec;
 	struct timeval t;
 	static struct timeval prev_time = { 0, 0 };
-	u8 ktype = IMON_KEY_IMON;
+	u8 ktype;
 
 	idev = ictx->idev;
+	irdev = input_get_drvdata(idev);
 
 	/* filter out junk data on the older 0xffdc imon devices */
 	if ((buf[0] == 0xff) && (buf[1] == 0xff) && (buf[2] == 0xff))
 		return;
 
 	/* Figure out what key was pressed */
-	memcpy(&temp_key, buf, sizeof(temp_key));
 	if (len == 8 && buf[7] == 0xee) {
+		scancode = be64_to_cpu(*((u64 *)buf));
 		ktype = IMON_KEY_PANEL;
-		panel_key = le64_to_cpu(temp_key);
-		kc = imon_panel_key_lookup(panel_key);
+		kc = imon_panel_key_lookup(scancode);
 	} else {
-		remote_key = (u32) (le64_to_cpu(temp_key) & 0xffffffff);
+		scancode = be32_to_cpu(*((u32 *)buf));
 		if (ictx->ir_type == IR_TYPE_RC6) {
+			ktype = IMON_KEY_IMON;
 			if (buf[0] == 0x80)
 				ktype = IMON_KEY_MCE;
-			kc = imon_mce_key_lookup(ictx, remote_key);
-		} else
-			kc = imon_remote_key_lookup(ictx, remote_key);
+			kc = imon_mce_key_lookup(ictx, scancode);
+		} else {
+			ktype = IMON_KEY_IMON;
+			kc = imon_remote_key_lookup(ictx, scancode);
+		}
 	}
 
 	/* keyboard/mouse mode toggle button */
@@ -1504,6 +1506,7 @@ static void imon_incoming_packet(struct imon_context *ictx,
 	if (ictx->display_type == IMON_DISPLAY_TYPE_VGA && len == 8 &&
 	    buf[7] == 0x86) {
 		imon_touch_event(ictx, buf);
+		return;
 
 	/* look for mouse events with pad in mouse mode */
 	} else if (ictx->pad_mouse) {
@@ -1534,9 +1537,20 @@ static void imon_incoming_packet(struct imon_context *ictx,
 	if (ictx->kc == KEY_UNKNOWN)
 		goto unknown_key;
 
-	/* KEY_MUTE repeats from MCE and knob need to be suppressed */
-	if ((ictx->kc == KEY_MUTE && ictx->kc == ictx->last_keycode)
-	    && (buf[7] == 0xee || ktype == IMON_KEY_MCE)) {
+	if (ktype != IMON_KEY_PANEL) {
+		if (press_type == 0)
+			ir_keyup(irdev);
+		else {
+			ir_keydown(ictx->rdev, ictx->rc_scancode,
+				   ictx->rc_toggle);
+			ictx->last_keycode = ictx->kc;
+		}
+		return;
+	}
+
+	/* Only panel type events left to process now */
+	/* KEY_MUTE repeats from knob need to be suppressed */
+	if (ictx->kc == KEY_MUTE && ictx->kc == ictx->last_keycode) {
 		do_gettimeofday(&t);
 		msec = tv2int(&t, &prev_time);
 		prev_time = t;
@@ -1547,11 +1561,9 @@ static void imon_incoming_packet(struct imon_context *ictx,
 	input_report_key(idev, ictx->kc, press_type);
 	input_sync(idev);
 
-	/* panel keys and some remote keys don't generate a release */
-	if (panel_key || norelease) {
-		input_report_key(idev, ictx->kc, 0);
-		input_sync(idev);
-	}
+	/* panel keys don't generate a release */
+	input_report_key(idev, ictx->kc, 0);
+	input_sync(idev);
 
 	ictx->last_keycode = ictx->kc;
 
@@ -1559,8 +1571,7 @@ static void imon_incoming_packet(struct imon_context *ictx,
 
 unknown_key:
 	dev_info(dev, "%s: unknown keypress, code 0x%llx\n", __func__,
-		 (panel_key ? be64_to_cpu(panel_key) :
-			      be32_to_cpu(remote_key)));
+		 (long long)scancode);
 	return;
 
 not_input_data:
@@ -1651,31 +1662,71 @@ static void usb_rx_callback_intf1(struct urb *urb)
 	usb_submit_urb(ictx->rx_urb_intf1, GFP_ATOMIC);
 }
 
+static struct input_dev *imon_init_rdev(struct imon_context *ictx)
+{
+	struct input_dev *rdev;
+	struct ir_dev_props *props;
+	int ret;
+
+	rdev = input_allocate_device();
+	props = kzalloc(sizeof(*props), GFP_KERNEL);
+	if (!rdev || !props) {
+		dev_err(ictx->dev, "remote control dev allocation failed\n");
+		goto out;
+	}
+
+	snprintf(ictx->name_rdev, sizeof(ictx->name_rdev),
+		 "iMON Remote (%04x:%04x)", ictx->vendor, ictx->product);
+	usb_make_path(ictx->usbdev_intf0, ictx->phys_rdev,
+		      sizeof(ictx->phys_rdev));
+	strlcat(ictx->phys_rdev, "/input0", sizeof(ictx->phys_rdev));
+
+	rdev->name = ictx->name_rdev;
+	rdev->phys = ictx->phys_rdev;
+	usb_to_input_id(ictx->usbdev_intf0, &rdev->id);
+	rdev->dev.parent = ictx->dev;
+	rdev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+	input_set_drvdata(rdev, ictx);
+
+	props->priv = ictx;
+	props->driver_type = RC_DRIVER_SCANCODE;
+	props->allowed_protos = IR_TYPE_OTHER | IR_TYPE_RC6; /* iMON PAD or MCE */
+	props->change_protocol = imon_ir_change_protocol;
+	ictx->props = props;
+
+	ret = ir_input_register(rdev, RC_MAP_IMON_PAD, props, MOD_NAME);
+	if (ret < 0) {
+		dev_err(ictx->dev, "remote input dev register failed\n");
+		goto out;
+	}
+
+	return rdev;
+
+out:
+	kfree(props);
+	input_free_device(rdev);
+	return NULL;
+}
+
 static struct input_dev *imon_init_idev(struct imon_context *ictx)
 {
 	struct input_dev *idev;
-	struct ir_dev_props *props;
 	int ret, i;
 
 	idev = input_allocate_device();
 	if (!idev) {
-		dev_err(ictx->dev, "remote input dev allocation failed\n");
-		goto idev_alloc_failed;
-	}
-
-	props = kzalloc(sizeof(struct ir_dev_props), GFP_KERNEL);
-	if (!props) {
-		dev_err(ictx->dev, "remote ir dev props allocation failed\n");
-		goto props_alloc_failed;
+		dev_err(ictx->dev, "input dev allocation failed\n");
+		goto out;
 	}
 
 	snprintf(ictx->name_idev, sizeof(ictx->name_idev),
-		 "iMON Remote (%04x:%04x)", ictx->vendor, ictx->product);
+		 "iMON Panel, Knob and Mouse(%04x:%04x)",
+		 ictx->vendor, ictx->product);
 	idev->name = ictx->name_idev;
 
 	usb_make_path(ictx->usbdev_intf0, ictx->phys_idev,
 		      sizeof(ictx->phys_idev));
-	strlcat(ictx->phys_idev, "/input0", sizeof(ictx->phys_idev));
+	strlcat(ictx->phys_idev, "/input1", sizeof(ictx->phys_idev));
 	idev->phys = ictx->phys_idev;
 
 	idev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) | BIT_MASK(EV_REL);
@@ -1691,30 +1742,20 @@ static struct input_dev *imon_init_idev(struct imon_context *ictx)
 		__set_bit(kc, idev->keybit);
 	}
 
-	props->priv = ictx;
-	props->driver_type = RC_DRIVER_SCANCODE;
-	/* IR_TYPE_OTHER maps to iMON PAD remote, IR_TYPE_RC6 to MCE remote */
-	props->allowed_protos = IR_TYPE_OTHER | IR_TYPE_RC6;
-	props->change_protocol = imon_ir_change_protocol;
-	ictx->props = props;
-
 	usb_to_input_id(ictx->usbdev_intf0, &idev->id);
 	idev->dev.parent = ictx->dev;
+	input_set_drvdata(idev, ictx);
 
-	ret = ir_input_register(idev, RC_MAP_IMON_PAD, props, MOD_NAME);
+	ret = input_register_device(idev);
 	if (ret < 0) {
-		dev_err(ictx->dev, "remote input dev register failed\n");
-		goto idev_register_failed;
+		dev_err(ictx->dev, "input dev register failed\n");
+		goto out;
 	}
 
 	return idev;
 
-idev_register_failed:
-	kfree(props);
-props_alloc_failed:
+out:
 	input_free_device(idev);
-idev_alloc_failed:
-
 	return NULL;
 }
 
@@ -1736,7 +1777,7 @@ static struct input_dev *imon_init_touch(struct imon_context *ictx)
 
 	usb_make_path(ictx->usbdev_intf1, ictx->phys_touch,
 		      sizeof(ictx->phys_touch));
-	strlcat(ictx->phys_touch, "/input1", sizeof(ictx->phys_touch));
+	strlcat(ictx->phys_touch, "/input2", sizeof(ictx->phys_touch));
 	touch->phys = ictx->phys_touch;
 
 	touch->evbit[0] =
@@ -1911,6 +1952,12 @@ static struct imon_context *imon_init_intf0(struct usb_interface *intf)
 		goto idev_setup_failed;
 	}
 
+	ictx->rdev = imon_init_rdev(ictx);
+	if (!ictx->rdev) {
+		dev_err(dev, "%s: rc device setup failed\n", __func__);
+		goto rdev_setup_failed;
+	}
+
 	usb_fill_int_urb(ictx->rx_urb_intf0, ictx->usbdev_intf0,
 		usb_rcvintpipe(ictx->usbdev_intf0,
 			ictx->rx_endpoint_intf0->bEndpointAddress),
@@ -1928,7 +1975,9 @@ static struct imon_context *imon_init_intf0(struct usb_interface *intf)
 	return ictx;
 
 urb_submit_failed:
-	ir_input_unregister(ictx->idev);
+	ir_input_unregister(ictx->rdev);
+rdev_setup_failed:
+	input_unregister_device(ictx->idev);
 idev_setup_failed:
 find_endpoint_failed:
 	mutex_unlock(&ictx->lock);
@@ -2289,7 +2338,8 @@ static void __devexit imon_disconnect(struct usb_interface *interface)
 	if (ifnum == 0) {
 		ictx->dev_present_intf0 = false;
 		usb_kill_urb(ictx->rx_urb_intf0);
-		ir_input_unregister(ictx->idev);
+		input_unregister_device(ictx->idev);
+		ir_input_unregister(ictx->rdev);
 		if (ictx->display_supported) {
 			if (ictx->display_type == IMON_DISPLAY_TYPE_LCD)
 				usb_deregister_dev(interface, &imon_lcd_class);
@@ -2309,11 +2359,8 @@ static void __devexit imon_disconnect(struct usb_interface *interface)
 		mutex_unlock(&ictx->lock);
 		if (!ictx->display_isopen)
 			free_imon_context(ictx);
-	} else {
-		if (ictx->ir_type == IR_TYPE_RC6)
-			del_timer_sync(&ictx->itimer);
+	} else
 		mutex_unlock(&ictx->lock);
-	}
 
 	mutex_unlock(&driver_lock);
 
-- 
1.7.2.2

-- 
Jarod Wilson
jarod@redhat.com

[PATCH 3/4] IR/imon: protect ictx's kc and last_keycode w/spinlock

[Jarod Wilson]

>From 1fd62121d93ca507ec6f2f692121f853a2c46889 Mon Sep 17 00:00:00 2001
From: Jarod Wilson <jarod@redhat.com>
Date: Wed, 15 Sep 2010 14:56:03 -0400
Subject: [PATCH 3/4] IR/imon: protect ictx's kc and last_keycode w/spinlock

Lest we get our keycodes wrong... Thus far, in practice, I've not found
it to actually matter, but its one of the issues raised in
https://bugzilla.kernel.org/show_bug.cgi?id=16351 that wasn't addressed
by converting to using native IR keydown/up functions.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/imon.c |   52 +++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/media/IR/imon.c b/drivers/media/IR/imon.c
index d36fe72..4b73b8e 100644
--- a/drivers/media/IR/imon.c
+++ b/drivers/media/IR/imon.c
@@ -1,7 +1,7 @@
 /*
  *   imon.c:	input and display driver for SoundGraph iMON IR/VFD/LCD
  *
- *   Copyright(C) 2009  Jarod Wilson <jarod@wilsonet.com>
+ *   Copyright(C) 2010  Jarod Wilson <jarod@wilsonet.com>
  *   Portions based on the original lirc_imon driver,
  *	Copyright(C) 2004  Venky Raju(dev@venky.ws)
  *
@@ -125,6 +125,7 @@ struct imon_context {
 	struct input_dev *idev;		/* input device for panel & IR mouse */
 	struct input_dev *touch;	/* input device for touchscreen */
 
+	spinlock_t kc_lock;		/* make sure we get keycodes right */
 	u32 kc;				/* current input keycode */
 	u32 last_keycode;		/* last reported input keycode */
 	u32 rc_scancode;		/* the computed remote scancode */
@@ -1210,6 +1211,9 @@ static bool imon_mouse_event(struct imon_context *ictx,
 	u8 right_shift = 1;
 	bool mouse_input = true;
 	int dir = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ictx->kc_lock, flags);
 
 	/* newer iMON device PAD or mouse button */
 	if (ictx->product != 0xffdc && (buf[0] & 0x01) && len == 5) {
@@ -1241,6 +1245,8 @@ static bool imon_mouse_event(struct imon_context *ictx,
 	} else
 		mouse_input = false;
 
+	spin_unlock_irqrestore(&ictx->kc_lock, flags);
+
 	if (mouse_input) {
 		dev_dbg(ictx->dev, "sending mouse data via input subsystem\n");
 
@@ -1255,7 +1261,9 @@ static bool imon_mouse_event(struct imon_context *ictx,
 					 buf[1] >> right_shift & 0x1);
 		}
 		input_sync(ictx->idev);
+		spin_lock_irqsave(&ictx->kc_lock, flags);
 		ictx->last_keycode = ictx->kc;
+		spin_unlock_irqrestore(&ictx->kc_lock, flags);
 	}
 
 	return mouse_input;
@@ -1278,6 +1286,7 @@ static void imon_pad_to_keys(struct imon_context *ictx, unsigned char *buf)
 	char rel_x = 0x00, rel_y = 0x00;
 	u16 timeout, threshold;
 	u32 scancode = KEY_RESERVED;
+	unsigned long flags;
 
 	/*
 	 * The imon directional pad functions more like a touchpad. Bytes 3 & 4
@@ -1301,7 +1310,11 @@ static void imon_pad_to_keys(struct imon_context *ictx, unsigned char *buf)
 				dir = stabilize((int)rel_x, (int)rel_y,
 						timeout, threshold);
 				if (!dir) {
+					spin_lock_irqsave(&ictx->kc_lock,
+							  flags);
 					ictx->kc = KEY_UNKNOWN;
+					spin_unlock_irqrestore(&ictx->kc_lock,
+							       flags);
 					return;
 				}
 				buf[2] = dir & 0xFF;
@@ -1363,7 +1376,9 @@ static void imon_pad_to_keys(struct imon_context *ictx, unsigned char *buf)
 			dir = stabilize((int)rel_x, (int)rel_y,
 					timeout, threshold);
 			if (!dir) {
+				spin_lock_irqsave(&ictx->kc_lock, flags);
 				ictx->kc = KEY_UNKNOWN;
+				spin_unlock_irqrestore(&ictx->kc_lock, flags);
 				return;
 			}
 			buf[2] = dir & 0xFF;
@@ -1392,8 +1407,11 @@ static void imon_pad_to_keys(struct imon_context *ictx, unsigned char *buf)
 		}
 	}
 
-	if (scancode)
+	if (scancode) {
+		spin_lock_irqsave(&ictx->kc_lock, flags);
 		ictx->kc = imon_remote_key_lookup(ictx, scancode);
+		spin_unlock_irqrestore(&ictx->kc_lock, flags);
+	}
 }
 
 /**
@@ -1405,6 +1423,9 @@ static int imon_parse_press_type(struct imon_context *ictx,
 				 unsigned char *buf, u8 ktype)
 {
 	int press_type = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ictx->kc_lock, flags);
 
 	/* key release of 0x02XXXXXX key */
 	if (ictx->kc == KEY_RESERVED && buf[0] == 0x02 && buf[3] == 0x00)
@@ -1437,6 +1458,8 @@ static int imon_parse_press_type(struct imon_context *ictx,
 	else
 		press_type = 1;
 
+	spin_unlock_irqrestore(&ictx->kc_lock, flags);
+
 	return press_type;
 }
 
@@ -1449,6 +1472,7 @@ static void imon_incoming_packet(struct imon_context *ictx,
 	int len = urb->actual_length;
 	unsigned char *buf = urb->transfer_buffer;
 	struct device *dev = ictx->dev;
+	unsigned long flags;
 	u32 kc;
 	bool norelease = false;
 	int i;
@@ -1486,6 +1510,7 @@ static void imon_incoming_packet(struct imon_context *ictx,
 		}
 	}
 
+	spin_lock_irqsave(&ictx->kc_lock, flags);
 	/* keyboard/mouse mode toggle button */
 	if (kc == KEY_KEYBOARD && !ictx->release_code) {
 		ictx->last_keycode = kc;
@@ -1493,6 +1518,7 @@ static void imon_incoming_packet(struct imon_context *ictx,
 			ictx->pad_mouse = ~(ictx->pad_mouse) & 0x1;
 			dev_dbg(dev, "toggling to %s mode\n",
 				ictx->pad_mouse ? "mouse" : "keyboard");
+			spin_unlock_irqrestore(&ictx->kc_lock, flags);
 			return;
 		} else {
 			ictx->pad_mouse = 0;
@@ -1501,6 +1527,7 @@ static void imon_incoming_packet(struct imon_context *ictx,
 	}
 
 	ictx->kc = kc;
+	spin_unlock_irqrestore(&ictx->kc_lock, flags);
 
 	/* send touchscreen events through input subsystem if touchpad data */
 	if (ictx->display_type == IMON_DISPLAY_TYPE_VGA && len == 8 &&
@@ -1534,8 +1561,10 @@ static void imon_incoming_packet(struct imon_context *ictx,
 	if (press_type < 0)
 		goto not_input_data;
 
+	spin_lock_irqsave(&ictx->kc_lock, flags);
 	if (ictx->kc == KEY_UNKNOWN)
 		goto unknown_key;
+	spin_unlock_irqrestore(&ictx->kc_lock, flags);
 
 	if (ktype != IMON_KEY_PANEL) {
 		if (press_type == 0)
@@ -1543,33 +1572,43 @@ static void imon_incoming_packet(struct imon_context *ictx,
 		else {
 			ir_keydown(ictx->rdev, ictx->rc_scancode,
 				   ictx->rc_toggle);
+			spin_lock_irqsave(&ictx->kc_lock, flags);
 			ictx->last_keycode = ictx->kc;
+			spin_unlock_irqrestore(&ictx->kc_lock, flags);
 		}
 		return;
 	}
 
 	/* Only panel type events left to process now */
+	spin_lock_irqsave(&ictx->kc_lock, flags);
+
 	/* KEY_MUTE repeats from knob need to be suppressed */
 	if (ictx->kc == KEY_MUTE && ictx->kc == ictx->last_keycode) {
 		do_gettimeofday(&t);
 		msec = tv2int(&t, &prev_time);
 		prev_time = t;
-		if (msec < idev->rep[REP_DELAY])
+		if (msec < idev->rep[REP_DELAY]) {
+			spin_unlock_irqrestore(&ictx->kc_lock, flags);
 			return;
+		}
 	}
+	kc = ictx->kc;
+
+	spin_unlock_irqrestore(&ictx->kc_lock, flags);
 
-	input_report_key(idev, ictx->kc, press_type);
+	input_report_key(idev, kc, press_type);
 	input_sync(idev);
 
 	/* panel keys don't generate a release */
-	input_report_key(idev, ictx->kc, 0);
+	input_report_key(idev, kc, 0);
 	input_sync(idev);
 
-	ictx->last_keycode = ictx->kc;
+	ictx->last_keycode = kc;
 
 	return;
 
 unknown_key:
+	spin_unlock_irqrestore(&ictx->kc_lock, flags);
 	dev_info(dev, "%s: unknown keypress, code 0x%llx\n", __func__,
 		 (long long)scancode);
 	return;
@@ -1927,6 +1966,7 @@ static struct imon_context *imon_init_intf0(struct usb_interface *intf)
 	}
 
 	mutex_init(&ictx->lock);
+	spin_lock_init(&ictx->kc_lock);
 
 	mutex_lock(&ictx->lock);
 
-- 
1.7.2.2


-- 
Jarod Wilson
jarod@redhat.com

[PATCH 4/4] IR/imon: set up mce-only devices w/mce keytable

[Jarod Wilson]

>From 321eb136b4c933a6a2e6583ccf110c22874c02fe Mon Sep 17 00:00:00 2001
From: Jarod Wilson <jarod@redhat.com>
Date: Tue, 14 Sep 2010 23:28:41 -0400
Subject: [PATCH 4/4] IR/imon: set up mce-only devices w/mce keytable

Currently, they get set up with the pad keytable, which they can't
actually use at all. Also add another variant of volume scancodes from
another 0xffdc device, and properly set up the 0x9e 0xffdc device as an
iMON VFD w/MCE proto IR.

Based on data and a prior patch from Anders Eriksson on the lirc list.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/imon.c |  264 ++++++++++++++++++++++++----------------------
 1 files changed, 138 insertions(+), 126 deletions(-)

diff --git a/drivers/media/IR/imon.c b/drivers/media/IR/imon.c
index 4b73b8e..ef81d38 100644
--- a/drivers/media/IR/imon.c
+++ b/drivers/media/IR/imon.c
@@ -292,6 +292,9 @@ static const struct {
 	{ 0x000100000000ffeell, KEY_VOLUMEUP },
 	{ 0x010000000000ffeell, KEY_VOLUMEDOWN },
 	{ 0x000000000100ffeell, KEY_MUTE },
+	/* 0xffdc iMON MCE VFD */
+	{ 0x00010000ffffffeell, KEY_VOLUMEUP },
+	{ 0x01000000ffffffeell, KEY_VOLUMEDOWN },
 	/* iMON Knob values */
 	{ 0x000100ffffffffeell, KEY_VOLUMEUP },
 	{ 0x010000ffffffffeell, KEY_VOLUMEDOWN },
@@ -1701,11 +1704,128 @@ static void usb_rx_callback_intf1(struct urb *urb)
 	usb_submit_urb(ictx->rx_urb_intf1, GFP_ATOMIC);
 }
 
+/*
+ * The 0x15c2:0xffdc device ID was used for umpteen different imon
+ * devices, and all of them constantly spew interrupts, even when there
+ * is no actual data to report. However, byte 6 of this buffer looks like
+ * its unique across device variants, so we're trying to key off that to
+ * figure out which display type (if any) and what IR protocol the device
+ * actually supports. These devices have their IR protocol hard-coded into
+ * their firmware, they can't be changed on the fly like the newer hardware.
+ */
+static void imon_get_ffdc_type(struct imon_context *ictx)
+{
+	u8 ffdc_cfg_byte = ictx->usb_rx_buf[6];
+	u8 detected_display_type = IMON_DISPLAY_TYPE_NONE;
+	u64 allowed_protos = IR_TYPE_OTHER;
+
+	switch (ffdc_cfg_byte) {
+	/* iMON Knob, no display, iMON IR + vol knob */
+	case 0x21:
+		dev_info(ictx->dev, "0xffdc iMON Knob, iMON IR");
+		ictx->display_supported = false;
+		break;
+	/* iMON 2.4G LT (usb stick), no display, iMON RF */
+	case 0x4e:
+		dev_info(ictx->dev, "0xffdc iMON 2.4G LT, iMON RF");
+		ictx->display_supported = false;
+		ictx->rf_device = true;
+		break;
+	/* iMON VFD, no IR (does have vol knob tho) */
+	case 0x35:
+		dev_info(ictx->dev, "0xffdc iMON VFD + knob, no IR");
+		detected_display_type = IMON_DISPLAY_TYPE_VFD;
+		break;
+	/* iMON VFD, iMON IR */
+	case 0x24:
+	case 0x85:
+		dev_info(ictx->dev, "0xffdc iMON VFD, iMON IR");
+		detected_display_type = IMON_DISPLAY_TYPE_VFD;
+		break;
+	/* iMON LCD, MCE IR */
+	case 0x9e:
+		dev_info(ictx->dev, "0xffdc iMON VFD, MCE IR");
+		detected_display_type = IMON_DISPLAY_TYPE_VFD;
+		allowed_protos = IR_TYPE_RC6;
+		break;
+	/* iMON LCD, MCE IR */
+	case 0x9f:
+		dev_info(ictx->dev, "0xffdc iMON LCD, MCE IR");
+		detected_display_type = IMON_DISPLAY_TYPE_LCD;
+		allowed_protos = IR_TYPE_RC6;
+		break;
+	default:
+		dev_info(ictx->dev, "Unknown 0xffdc device, "
+			 "defaulting to VFD and iMON IR");
+		detected_display_type = IMON_DISPLAY_TYPE_VFD;
+		break;
+	}
+
+	printk(KERN_CONT " (id 0x%02x)\n", ffdc_cfg_byte);
+
+	ictx->display_type = detected_display_type;
+	ictx->props->allowed_protos = allowed_protos;
+	ictx->ir_type = allowed_protos;
+}
+
+static void imon_set_display_type(struct imon_context *ictx)
+{
+	u8 configured_display_type = IMON_DISPLAY_TYPE_VFD;
+
+	/*
+	 * Try to auto-detect the type of display if the user hasn't set
+	 * it by hand via the display_type modparam. Default is VFD.
+	 */
+
+	if (display_type == IMON_DISPLAY_TYPE_AUTO) {
+		switch (ictx->product) {
+		case 0xffdc:
+			/* set in imon_get_ffdc_type() */
+			configured_display_type = ictx->display_type;
+			break;
+		case 0x0034:
+		case 0x0035:
+			configured_display_type = IMON_DISPLAY_TYPE_VGA;
+			break;
+		case 0x0038:
+		case 0x0039:
+		case 0x0045:
+			configured_display_type = IMON_DISPLAY_TYPE_LCD;
+			break;
+		case 0x003c:
+		case 0x0041:
+		case 0x0042:
+		case 0x0043:
+			configured_display_type = IMON_DISPLAY_TYPE_NONE;
+			ictx->display_supported = false;
+			break;
+		case 0x0036:
+		case 0x0044:
+		default:
+			configured_display_type = IMON_DISPLAY_TYPE_VFD;
+			break;
+		}
+	} else {
+		configured_display_type = display_type;
+		if (display_type == IMON_DISPLAY_TYPE_NONE)
+			ictx->display_supported = false;
+		else
+			ictx->display_supported = true;
+		dev_info(ictx->dev, "%s: overriding display type to %d via "
+			 "modparam\n", __func__, display_type);
+	}
+
+	ictx->display_type = configured_display_type;
+}
+
 static struct input_dev *imon_init_rdev(struct imon_context *ictx)
 {
 	struct input_dev *rdev;
 	struct ir_dev_props *props;
 	int ret;
+	char *ir_codes = NULL;
+	const unsigned char fp_packet[] = { 0x40, 0x00, 0x00, 0x00,
+					    0x00, 0x00, 0x00, 0x88 };
 
 	rdev = input_allocate_device();
 	props = kzalloc(sizeof(*props), GFP_KERNEL);
@@ -1733,7 +1853,24 @@ static struct input_dev *imon_init_rdev(struct imon_context *ictx)
 	props->change_protocol = imon_ir_change_protocol;
 	ictx->props = props;
 
-	ret = ir_input_register(rdev, RC_MAP_IMON_PAD, props, MOD_NAME);
+	/* Enable front-panel buttons and/or knobs */
+	memcpy(ictx->usb_tx_buf, &fp_packet, sizeof(fp_packet));
+	ret = send_packet(ictx);
+	/* Not fatal, but warn about it */
+	if (ret)
+		dev_info(ictx->dev, "panel buttons/knobs setup failed\n");
+
+	if (ictx->product == 0xffdc)
+		imon_get_ffdc_type(ictx);
+
+	imon_set_display_type(ictx);
+
+	if (ictx->ir_type == IR_TYPE_RC6)
+		ir_codes = RC_MAP_IMON_MCE;
+	else
+		ir_codes = RC_MAP_IMON_PAD;
+
+	ret = ir_input_register(rdev, ir_codes, props, MOD_NAME);
 	if (ret < 0) {
 		dev_err(ictx->dev, "remote input dev register failed\n");
 		goto out;
@@ -2099,116 +2236,6 @@ rx_urb_alloc_failed:
 	return NULL;
 }
 
-/*
- * The 0x15c2:0xffdc device ID was used for umpteen different imon
- * devices, and all of them constantly spew interrupts, even when there
- * is no actual data to report. However, byte 6 of this buffer looks like
- * its unique across device variants, so we're trying to key off that to
- * figure out which display type (if any) and what IR protocol the device
- * actually supports. These devices have their IR protocol hard-coded into
- * their firmware, they can't be changed on the fly like the newer hardware.
- */
-static void imon_get_ffdc_type(struct imon_context *ictx)
-{
-	u8 ffdc_cfg_byte = ictx->usb_rx_buf[6];
-	u8 detected_display_type = IMON_DISPLAY_TYPE_NONE;
-	u64 allowed_protos = IR_TYPE_OTHER;
-
-	switch (ffdc_cfg_byte) {
-	/* iMON Knob, no display, iMON IR + vol knob */
-	case 0x21:
-		dev_info(ictx->dev, "0xffdc iMON Knob, iMON IR");
-		ictx->display_supported = false;
-		break;
-	/* iMON 2.4G LT (usb stick), no display, iMON RF */
-	case 0x4e:
-		dev_info(ictx->dev, "0xffdc iMON 2.4G LT, iMON RF");
-		ictx->display_supported = false;
-		ictx->rf_device = true;
-		break;
-	/* iMON VFD, no IR (does have vol knob tho) */
-	case 0x35:
-		dev_info(ictx->dev, "0xffdc iMON VFD + knob, no IR");
-		detected_display_type = IMON_DISPLAY_TYPE_VFD;
-		break;
-	/* iMON VFD, iMON IR */
-	case 0x24:
-	case 0x85:
-		dev_info(ictx->dev, "0xffdc iMON VFD, iMON IR");
-		detected_display_type = IMON_DISPLAY_TYPE_VFD;
-		break;
-	/* iMON LCD, MCE IR */
-	case 0x9e:
-	case 0x9f:
-		dev_info(ictx->dev, "0xffdc iMON LCD, MCE IR");
-		detected_display_type = IMON_DISPLAY_TYPE_LCD;
-		allowed_protos = IR_TYPE_RC6;
-		break;
-	default:
-		dev_info(ictx->dev, "Unknown 0xffdc device, "
-			 "defaulting to VFD and iMON IR");
-		detected_display_type = IMON_DISPLAY_TYPE_VFD;
-		break;
-	}
-
-	printk(KERN_CONT " (id 0x%02x)\n", ffdc_cfg_byte);
-
-	ictx->display_type = detected_display_type;
-	ictx->props->allowed_protos = allowed_protos;
-	ictx->ir_type = allowed_protos;
-}
-
-static void imon_set_display_type(struct imon_context *ictx,
-				  struct usb_interface *intf)
-{
-	u8 configured_display_type = IMON_DISPLAY_TYPE_VFD;
-
-	/*
-	 * Try to auto-detect the type of display if the user hasn't set
-	 * it by hand via the display_type modparam. Default is VFD.
-	 */
-
-	if (display_type == IMON_DISPLAY_TYPE_AUTO) {
-		switch (ictx->product) {
-		case 0xffdc:
-			/* set in imon_get_ffdc_type() */
-			configured_display_type = ictx->display_type;
-			break;
-		case 0x0034:
-		case 0x0035:
-			configured_display_type = IMON_DISPLAY_TYPE_VGA;
-			break;
-		case 0x0038:
-		case 0x0039:
-		case 0x0045:
-			configured_display_type = IMON_DISPLAY_TYPE_LCD;
-			break;
-		case 0x003c:
-		case 0x0041:
-		case 0x0042:
-		case 0x0043:
-			configured_display_type = IMON_DISPLAY_TYPE_NONE;
-			ictx->display_supported = false;
-			break;
-		case 0x0036:
-		case 0x0044:
-		default:
-			configured_display_type = IMON_DISPLAY_TYPE_VFD;
-			break;
-		}
-	} else {
-		configured_display_type = display_type;
-		if (display_type == IMON_DISPLAY_TYPE_NONE)
-			ictx->display_supported = false;
-		else
-			ictx->display_supported = true;
-		dev_info(ictx->dev, "%s: overriding display type to %d via "
-			 "modparam\n", __func__, display_type);
-	}
-
-	ictx->display_type = configured_display_type;
-}
-
 static void imon_init_display(struct imon_context *ictx,
 			      struct usb_interface *intf)
 {
@@ -2249,8 +2276,6 @@ static int __devinit imon_probe(struct usb_interface *interface,
 	struct imon_context *ictx = NULL;
 	struct imon_context *first_if_ctx = NULL;
 	u16 vendor, product;
-	const unsigned char fp_packet[] = { 0x40, 0x00, 0x00, 0x00,
-					    0x00, 0x00, 0x00, 0x88 };
 
 	code_length = BUF_CHUNK_SIZE * 8;
 
@@ -2291,19 +2316,6 @@ static int __devinit imon_probe(struct usb_interface *interface,
 	usb_set_intfdata(interface, ictx);
 
 	if (ifnum == 0) {
-		/* Enable front-panel buttons and/or knobs */
-		memcpy(ictx->usb_tx_buf, &fp_packet, sizeof(fp_packet));
-		ret = send_packet(ictx);
-		/* Not fatal, but warn about it */
-		if (ret)
-			dev_info(dev, "failed to enable panel buttons "
-				 "and/or knobs\n");
-
-		if (product == 0xffdc)
-			imon_get_ffdc_type(ictx);
-
-		imon_set_display_type(ictx, interface);
-
 		if (product == 0xffdc && ictx->rf_device) {
 			sysfs_err = sysfs_create_group(&interface->dev.kobj,
 						       &imon_rf_attribute_group);
-- 
1.7.2.2


-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH 4/4] IR/imon: set up mce-only devices w/mce keytable

[Anders Eriksson]

jarod@redhat.com said:

> +	/* iMON LCD, MCE IR */ 
> +	case 0x9e: 
> +		dev_info(ictx->dev, "0xffdc iMON VFD, MCE IR"); 
> +		detected_display_type = IMON_DISPLAY_TYPE_VFD;
> +		allowed_protos = IR_TYPE_RC6; +		break; 
> +	/* iMON LCD, MCE IR */ +	case 0x9f:
> 

That "LCD" in the comment should be VFD.

/Anders

Re: [PATCH 2/4] imon: split mouse events to a separate input dev

[David Härdeman]

On Thu, September 16, 2010 07:22, Jarod Wilson wrote:
> This is a stab at separating the mouse (and front panel/knob) events
> out to a separate input device. This is necessary in preparation for
> the next patch which makes the rc-core input dev opaque to rc
> drivers.
>
> I can't verify the correctness of the patch beyond the fact that it
> compiles without warnings. The driver has resisted most of my
> attempts at understanding it properly...for example, the double calls
> to le64_to_cpu() and be64_to_cpu() which are applied in
> imon_incoming_packet() and imon_panel_key_lookup() would amount
> to a bswab64() call, irregardless of the cpu endianness, and I think
> the code wouldn't have worked on a big-endian machine...
>
> Signed-off-by: David Härdeman <david@hardeman.nu>
>
> - Minor alterations to apply with minimal core IR changes
> - Use timer for imon keys too, since its entirely possible for the
>   receiver to miss release codes (either by way of another key being
>   pressed while the first is held or by the remote pointing away from
>   the recevier when the key is release. yes, I know, its ugly).

Where's the additional timer usage exactly? I can't see any in the patch...

-- 
David Härdeman

Re: [PATCH 4/4] IR/imon: set up mce-only devices w/mce keytable

[Jarod Wilson]

On Thu, Sep 16, 2010 at 10:11:16AM +0200, Anders Eriksson wrote:
> 
> 
> 
> jarod@redhat.com said:
> 
> > +	/* iMON LCD, MCE IR */ 
> > +	case 0x9e: 
> > +		dev_info(ictx->dev, "0xffdc iMON VFD, MCE IR"); 
> > +		detected_display_type = IMON_DISPLAY_TYPE_VFD;
> > +		allowed_protos = IR_TYPE_RC6; +		break; 
> > +	/* iMON LCD, MCE IR */ +	case 0x9f:
> > 
> 
> That "LCD" in the comment should be VFD.

Ah, dammit, copy-paste fail. Will fix, thanks!

-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH 2/4] imon: split mouse events to a separate input dev

[Jarod Wilson]

On Thu, Sep 16, 2010 at 01:32:07PM +0200, David Härdeman wrote:
> On Thu, September 16, 2010 07:22, Jarod Wilson wrote:
> > This is a stab at separating the mouse (and front panel/knob) events
> > out to a separate input device. This is necessary in preparation for
> > the next patch which makes the rc-core input dev opaque to rc
> > drivers.
> >
> > I can't verify the correctness of the patch beyond the fact that it
> > compiles without warnings. The driver has resisted most of my
> > attempts at understanding it properly...for example, the double calls
> > to le64_to_cpu() and be64_to_cpu() which are applied in
> > imon_incoming_packet() and imon_panel_key_lookup() would amount
> > to a bswab64() call, irregardless of the cpu endianness, and I think
> > the code wouldn't have worked on a big-endian machine...
> >
> > Signed-off-by: David Härdeman <david@hardeman.nu>
> >
> > - Minor alterations to apply with minimal core IR changes
> > - Use timer for imon keys too, since its entirely possible for the
> >   receiver to miss release codes (either by way of another key being
> >   pressed while the first is held or by the remote pointing away from
> >   the recevier when the key is release. yes, I know, its ugly).
> 
> Where's the additional timer usage exactly? I can't see any in the patch...

For ktype == IMON_KEY_IMON in your original patch, keys were submitted
with ir_keydown_notimeout, and in this version, they're submitted with
plain old ir_keydown, which has a built-in timeout.

-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH 2/4] imon: split mouse events to a separate input dev

[David Härdeman]

On Thu, September 16, 2010 15:34, Jarod Wilson wrote:
> On Thu, Sep 16, 2010 at 01:32:07PM +0200, David Härdeman wrote:
>> On Thu, September 16, 2010 07:22, Jarod Wilson wrote:
>> > This is a stab at separating the mouse (and front panel/knob) events
>> > out to a separate input device. This is necessary in preparation for
>> > the next patch which makes the rc-core input dev opaque to rc
>> > drivers.
>> >
>> > I can't verify the correctness of the patch beyond the fact that it
>> > compiles without warnings. The driver has resisted most of my
>> > attempts at understanding it properly...for example, the double calls
>> > to le64_to_cpu() and be64_to_cpu() which are applied in
>> > imon_incoming_packet() and imon_panel_key_lookup() would amount
>> > to a bswab64() call, irregardless of the cpu endianness, and I think
>> > the code wouldn't have worked on a big-endian machine...
>> >
>> > Signed-off-by: David Härdeman <david@hardeman.nu>
>> >
>> > - Minor alterations to apply with minimal core IR changes
>> > - Use timer for imon keys too, since its entirely possible for the
>> >   receiver to miss release codes (either by way of another key being
>> >   pressed while the first is held or by the remote pointing away from
>> >   the recevier when the key is release. yes, I know, its ugly).
>>
>> Where's the additional timer usage exactly? I can't see any in the
>> patch...
>
> For ktype == IMON_KEY_IMON in your original patch, keys were submitted
> with ir_keydown_notimeout, and in this version, they're submitted with
> plain old ir_keydown, which has a built-in timeout.

Oh, I see. But since you're not adding any timer to the driver code itself
I do not consider the use of plain ir_keydown to be ugly at all (contrary
to what your comment indicated).

Maybe a keyup hardware event is generated (handled via ir_keyup, good),
maybe it isnt't (handled via ir-core timeout which calls ir_keyup
eventually, good).

-- 
David Härdeman

Re: [PATCH 2/4] imon: split mouse events to a separate input dev

[Jarod Wilson]

On Thu, Sep 16, 2010 at 03:43:42PM +0200, David Härdeman wrote:
> On Thu, September 16, 2010 15:34, Jarod Wilson wrote:
> > On Thu, Sep 16, 2010 at 01:32:07PM +0200, David Härdeman wrote:
> >> On Thu, September 16, 2010 07:22, Jarod Wilson wrote:
> >> > This is a stab at separating the mouse (and front panel/knob) events
> >> > out to a separate input device. This is necessary in preparation for
> >> > the next patch which makes the rc-core input dev opaque to rc
> >> > drivers.
> >> >
> >> > I can't verify the correctness of the patch beyond the fact that it
> >> > compiles without warnings. The driver has resisted most of my
> >> > attempts at understanding it properly...for example, the double calls
> >> > to le64_to_cpu() and be64_to_cpu() which are applied in
> >> > imon_incoming_packet() and imon_panel_key_lookup() would amount
> >> > to a bswab64() call, irregardless of the cpu endianness, and I think
> >> > the code wouldn't have worked on a big-endian machine...
> >> >
> >> > Signed-off-by: David Härdeman <david@hardeman.nu>
> >> >
> >> > - Minor alterations to apply with minimal core IR changes
> >> > - Use timer for imon keys too, since its entirely possible for the
> >> >   receiver to miss release codes (either by way of another key being
> >> >   pressed while the first is held or by the remote pointing away from
> >> >   the recevier when the key is release. yes, I know, its ugly).
> >>
> >> Where's the additional timer usage exactly? I can't see any in the
> >> patch...
> >
> > For ktype == IMON_KEY_IMON in your original patch, keys were submitted
> > with ir_keydown_notimeout, and in this version, they're submitted with
> > plain old ir_keydown, which has a built-in timeout.
> 
> Oh, I see. But since you're not adding any timer to the driver code itself
> I do not consider the use of plain ir_keydown to be ugly at all (contrary
> to what your comment indicated).

Oh, sorry, that rant was about the receiver hardware, not the actual code
-- yes, the code gets much much cleaner here. :)

> Maybe a keyup hardware event is generated (handled via ir_keyup, good),
> maybe it isnt't (handled via ir-core timeout which calls ir_keyup
> eventually, good).

Yeah, its behaving much better for the specific cases Anssi mentioned in
the bug now. We also get the automatic release of a key that wasn't
released before pressing a second key, by way of the ir_keyup call in
ir_keydown.

-- 
Jarod Wilson
jarod@redhat.com