[PATCH] Input: add appleir USB driver

[PATCH] Input: add appleir USB driver

[Greg KH]

From: Greg Kroah-Hartman <gregkh@suse.de>

This driver was originally written by James McKenzie but forward ported
and cleaned up by me to get it to work with modern kernel versions.

Tested on my mac mini and it actually works!

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---

Jiri, is it ok for this quirks addtion to go through Dmitry's triee?  Or
do you want me to split it out into two different patches?

Dmitry, is this ok to go through your tree?  Or I can take it as well if
you don't want it :)

 drivers/hid/usbhid/hid-quirks.c |    2 
 drivers/input/misc/Kconfig      |   12 +
 drivers/input/misc/Makefile     |    1 
 drivers/input/misc/appleir.c    |  361 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 376 insertions(+)

--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -77,6 +77,7 @@
 #define USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS   0x022e
 #define USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY	0x030a
 #define USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY	0x030b
+#define USB_DEVICE_ID_APPLE_IRCONTROL	0x8240
 #define USB_DEVICE_ID_APPLE_IRCONTROL4	0x8242
 
 #define USB_VENDOR_ID_ASUS		0x0b05
@@ -443,6 +444,7 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_FULLSPEED_INTERVAL },
 
 	{ USB_VENDOR_ID_BELKIN, USB_DEVICE_ID_FLIP_KVM, HID_QUIRK_HIDDEV },
+	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL, HID_QUIRK_IGNORE },
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4, HID_QUIRK_HIDDEV | HID_QUIRK_IGNORE_HIDINPUT },
 	{ USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_IR_REMOTE, HID_QUIRK_HIDDEV | HID_QUIRK_IGNORE_HIDINPUT },
 	{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_SIDEWINDER_GV, HID_QUIRK_HIDINPUT },
--- /dev/null
+++ b/drivers/input/misc/appleir.c
@@ -0,0 +1,361 @@
+/*
+ * appleir: USB driver for the apple ir device
+ *
+ * Original driver written by James McKenzie
+ * Ported to recent 2.6 kernel versions by Greg Kroah-Hartman <gregkh@suse.de>
+ *
+ * Copyright (C) 2006 James McKenzie
+ * Copyright (C) 2008 Greg Kroah-Hartman <greg@kroah.com>
+ * Copyright (C) 2008 Novell Inc.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/usb.h>
+#include <linux/usb/input.h>
+#include <asm/unaligned.h>
+#include <asm/byteorder.h>
+
+#define DRIVER_VERSION "v1.2"
+#define DRIVER_AUTHOR "James McKenzie"
+#define DRIVER_DESC "USB Apple MacMini IR Receiver driver"
+#define DRIVER_LICENSE "GPL"
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE(DRIVER_LICENSE);
+
+#define USB_VENDOR_ID_APPLE	0x05ac
+#define USB_DEVICE_ID_APPLE_IR	0x8240
+
+#define URB_SIZE	32
+
+#define MAX_KEYS	8
+#define MAX_KEYS_MASK	(MAX_KEYS - 1)
+
+struct appleir {
+	struct input_dev *input_dev;
+	u8 *data;
+	dma_addr_t dma_buf;
+	struct usb_device *usbdev;
+	struct urb *urb;
+	int timer_initted;
+	struct timer_list key_up_timer;
+	int current_key;
+	char phys[32];
+};
+
+static struct usb_device_id appleir_ids[] = {
+	{USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IR)},
+	{}
+};
+MODULE_DEVICE_TABLE(usb, appleir_ids);
+
+/* I have two devices both of which report the following */
+/* 25 87 ee 83 0a  	+  */
+/* 25 87 ee 83 0c  	-  */
+/* 25 87 ee 83 09	<< */
+/* 25 87 ee 83 06	>> */
+/* 25 87 ee 83 05	>" */
+/* 25 87 ee 83 03	menu */
+/* 26 00 00 00 00	for key repeat*/
+
+/* Thomas Glanzmann reports the following responses */
+/* 25 87 ee ca 0b	+  */
+/* 25 87 ee ca 0d	-  */
+/* 25 87 ee ca 08	<< */
+/* 25 87 ee ca 07	>> */
+/* 25 87 ee ca 04	>" */
+/* 25 87 ee ca 02 	menu */
+/* 26 00 00 00 00       for key repeat*/
+/* He also observes the following event sometimes */
+/* sent after a key is release, which I interpret */
+/* as a flat battery message */
+/* 25 87 e0 ca 06	flat battery */
+
+static int keymap[MAX_KEYS] = {
+	KEY_RESERVED,
+	KEY_MENU,
+	KEY_PLAYPAUSE,
+	KEY_NEXTSONG,
+	KEY_PREVIOUSSONG,
+	KEY_VOLUMEUP,
+	KEY_VOLUMEDOWN,
+	KEY_RESERVED,
+};
+
+static void dump_packet(struct appleir *appleir, char *msg, u8 *data, int len)
+{
+	int i;
+
+	printk(KERN_ERR "appleir: %s (%d bytes)", msg, len);
+
+	for (i = 0; i < len; ++i)
+		printk(" %02x", data[i]);
+	printk("\n");
+}
+
+static void key_up(struct appleir *appleir, int key)
+{
+	/* printk (KERN_ERR "key %d up\n", key); */
+	input_report_key(appleir->input_dev, key, 0);
+	input_sync(appleir->input_dev);
+}
+
+static void key_down(struct appleir *appleir, int key)
+{
+	/* printk (KERN_ERR "key %d down\n", key); */
+	input_report_key(appleir->input_dev, key, 1);
+	input_sync(appleir->input_dev);
+}
+
+static void battery_flat(struct appleir *appleir)
+{
+	dev_err(&appleir->input_dev->dev, "possible flat battery?\n");
+}
+
+static void key_up_tick(unsigned long data)
+{
+	struct appleir *appleir = (struct appleir *)data;
+
+	if (appleir->current_key) {
+		key_up(appleir, appleir->current_key);
+		appleir->current_key = 0;
+	}
+}
+
+static void new_data(struct appleir *appleir, u8 *data, int len)
+{
+	static const u8 keydown[] = { 0x25, 0x87, 0xee };
+	static const u8 keyrepeat[] = { 0x26, 0x00, 0x00, 0x00, 0x00 };
+	static const u8 flatbattery[] = { 0x25, 0x87, 0xe0 };
+
+#if 0
+	dump_packet(appleir, "received", data, len);
+#endif
+
+	if (len != 5)
+		return;
+
+	if (!memcmp(data, keydown, sizeof(keydown))) {
+		/*If we already have a key down, take it up before marking */
+		/*this one down */
+		if (appleir->current_key)
+			key_up(appleir, appleir->current_key);
+		appleir->current_key = keymap[(data[4] >> 1) & MAX_KEYS_MASK];
+
+		key_down(appleir, appleir->current_key);
+		/*remote doesn't do key up, either pull them up, in the test */
+		/*above, or here set a timer which pulls them up after 1/8 s */
+		mod_timer(&appleir->key_up_timer, jiffies + HZ / 8);
+
+		return;
+	}
+
+	if (!memcmp(data, keyrepeat, sizeof(keyrepeat))) {
+		key_down(appleir, appleir->current_key);
+		/*remote doesn't do key up, either pull them up, in the test */
+		/*above, or here set a timer which pulls them up after 1/8 s */
+		mod_timer(&appleir->key_up_timer, jiffies + HZ / 8);
+		return;
+	}
+
+	if (!memcmp(data, flatbattery, sizeof(flatbattery))) {
+		battery_flat(appleir);
+		/* Fall through */
+	}
+
+	dump_packet(appleir, "unknown packet", data, len);
+}
+
+static void appleir_urb(struct urb *urb)
+{
+	struct appleir *appleir = urb->context;
+	int status = urb->status;
+	int retval;
+
+	switch (status) {
+	case 0:
+		new_data(appleir, urb->transfer_buffer, urb->actual_length);
+		break;
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+		/* this urb is terminated, clean up */
+		dbg("%s - urb shutting down with status: %d", __func__,
+		    urb->status);
+		return;
+	default:
+		dbg("%s - nonzero urb status received: %d", __func__,
+		    urb->status);
+	}
+
+	retval = usb_submit_urb(urb, GFP_ATOMIC);
+	if (retval)
+		err("%s - usb_submit_urb failed with result %d", __func__,
+		    retval);
+}
+
+static int appleir_open(struct input_dev *dev)
+{
+	struct appleir *appleir = input_get_drvdata(dev);
+
+	if (usb_submit_urb(appleir->urb, GFP_KERNEL))
+		return -EIO;
+
+	return 0;
+}
+
+static void appleir_close(struct input_dev *dev)
+{
+	struct appleir *appleir = input_get_drvdata(dev);
+
+	usb_kill_urb(appleir->urb);
+	del_timer_sync(&appleir->key_up_timer);
+}
+
+static int appleir_probe(struct usb_interface *intf,
+			 const struct usb_device_id *id)
+{
+	struct usb_device *dev = interface_to_usbdev(intf);
+	struct usb_endpoint_descriptor *endpoint;
+	struct appleir *appleir = NULL;
+	struct input_dev *input_dev;
+	int retval = -ENOMEM;
+	int i;
+
+	appleir = kzalloc(sizeof(struct appleir), GFP_KERNEL);
+	if (!appleir)
+		goto fail;
+
+	appleir->data = usb_buffer_alloc(dev, URB_SIZE, GFP_KERNEL,
+					 &appleir->dma_buf);
+	if (!appleir->data)
+		goto fail;
+
+	appleir->urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!appleir->urb)
+		goto fail;
+
+	appleir->usbdev = dev;
+
+	input_dev = input_allocate_device();
+	if (!input_dev)
+		goto fail;
+
+	appleir->input_dev = input_dev;
+
+	usb_make_path(dev, appleir->phys, sizeof(appleir->phys));
+	strlcpy(appleir->phys, "/input0", sizeof(appleir->phys));
+
+	input_dev->name = "Apple Mac mini infrared remote control driver";
+	input_dev->phys = appleir->phys;
+	usb_to_input_id(dev, &input_dev->id);
+	input_dev->dev.parent = &intf->dev;
+	input_set_drvdata(input_dev, appleir);
+
+	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
+	input_dev->ledbit[0] = 0;
+
+	for (i = 0; i < MAX_KEYS; i++)
+		set_bit(keymap[i], input_dev->keybit);
+
+	clear_bit(0, input_dev->keybit);
+
+	input_dev->open = appleir_open;
+	input_dev->close = appleir_close;
+
+	endpoint = &intf->cur_altsetting->endpoint[0].desc;
+
+	usb_fill_int_urb(appleir->urb, dev,
+			 usb_rcvintpipe(dev, endpoint->bEndpointAddress),
+			 appleir->data, 8,
+			 appleir_urb, appleir, endpoint->bInterval);
+
+	appleir->urb->transfer_dma = appleir->dma_buf;
+	appleir->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+	usb_set_intfdata(intf, appleir);
+
+	init_timer(&appleir->key_up_timer);
+
+	appleir->key_up_timer.function = key_up_tick;
+	appleir->key_up_timer.data = (unsigned long)appleir;
+
+	appleir->timer_initted++;
+
+	retval = input_register_device(appleir->input_dev);
+	if (retval)
+		goto fail;
+
+	return 0;
+
+fail:
+	if (appleir) {
+		if (appleir->data)
+			usb_buffer_free(dev, URB_SIZE, appleir->data,
+					appleir->dma_buf);
+
+		if (appleir->timer_initted)
+			del_timer_sync(&appleir->key_up_timer);
+
+		if (appleir->input_dev)
+			input_free_device(appleir->input_dev);
+
+		kfree(appleir);
+	}
+
+	return retval;
+}
+
+static void appleir_disconnect(struct usb_interface *intf)
+{
+	struct appleir *appleir = usb_get_intfdata(intf);
+
+	usb_set_intfdata(intf, NULL);
+	if (appleir) {
+		input_unregister_device(appleir->input_dev);
+		if (appleir->timer_initted)
+			del_timer_sync(&appleir->key_up_timer);
+		usb_kill_urb(appleir->urb);
+		usb_free_urb(appleir->urb);
+		usb_buffer_free(interface_to_usbdev(intf), URB_SIZE,
+				appleir->data, appleir->dma_buf);
+		kfree(appleir);
+	}
+}
+
+static struct usb_driver appleir_driver = {
+	.name = "appleir",
+	.probe = appleir_probe,
+	.disconnect = appleir_disconnect,
+	.id_table = appleir_ids,
+};
+
+static int __init appleir_init(void)
+{
+	int retval;
+
+	retval = usb_register(&appleir_driver);
+	if (retval)
+		goto out;
+	info(DRIVER_VERSION ":" DRIVER_DESC);
+out:
+	return retval;
+}
+
+static void __exit appleir_exit(void)
+{
+	usb_deregister(&appleir_driver);
+}
+
+module_init(appleir_init);
+module_exit(appleir_exit);
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -149,6 +149,18 @@ config INPUT_KEYSPAN_REMOTE
 	  To compile this driver as a module, choose M here: the module will
 	  be called keyspan_remote.
 
+config INPUT_APPLEIR
+	tristate "Apple Mac Mini USB IR receiver (built in)"
+	depends on USB_ARCH_HAS_HCD
+	select USB
+	help
+	  Say Y here if you want to use a Apple USB remote control.  This
+	  device is traditionally inside an Intel Apple Mac Mini, but might
+	  show up in other places.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called appleir.
+
 config INPUT_POWERMATE
 	tristate "Griffin PowerMate and Contour Jog support"
 	depends on USB_ARCH_HAS_HCD
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_INPUT_YEALINK)		+= yealink.
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
 obj-$(CONFIG_INPUT_UINPUT)		+= uinput.o
 obj-$(CONFIG_INPUT_APANEL)		+= apanel.o
+obj-$(CONFIG_INPUT_APPLEIR)		+= appleir.o

Re: [PATCH] Input: add appleir USB driver

[Matthew Garrett]

On Wed, May 14, 2008 at 03:15:19PM -0700, Greg KH wrote:

> +	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL, HID_QUIRK_IGNORE },
>  	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4, HID_QUIRK_HIDDEV | HID_QUIRK_IGNORE_HIDINPUT },

Hm. How is the IRCONTROL4 handled? Is the protocol completely different?

> +	  Say Y here if you want to use a Apple USB remote control.  This
> +	  device is traditionally inside an Intel Apple Mac Mini, but might
> +	  show up in other places.

Minor nit, but it's supported on all the desktop Intel Macs and not just 
the Mini.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Input: add appleir USB driver

[Greg KH]

On Thu, May 15, 2008 at 12:27:22AM +0100, Matthew Garrett wrote:
> On Wed, May 14, 2008 at 03:15:19PM -0700, Greg KH wrote:
> 
> > +	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL, HID_QUIRK_IGNORE },
> >  	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4, HID_QUIRK_HIDDEV | HID_QUIRK_IGNORE_HIDINPUT },
> 
> Hm. How is the IRCONTROL4 handled? Is the protocol completely different?

I know nothing about that device, sorry.  I think it is a totally
different protocol from what I can tell looking at the code.

> > +	  Say Y here if you want to use a Apple USB remote control.  This
> > +	  device is traditionally inside an Intel Apple Mac Mini, but might
> > +	  show up in other places.
> 
> Minor nit, but it's supported on all the desktop Intel Macs and not just 
> the Mini.

Ah, didn't realize that, nice to know, we should have a wider userbase
then :)

thanks,

greg k-h

Re: [PATCH] Input: add appleir USB driver

[Dmitry Torokhov]

Hi Greg,

On Wed, May 14, 2008 at 03:15:19PM -0700, Greg KH wrote:
> From: Greg Kroah-Hartman <gregkh@suse.de>
> 
> This driver was originally written by James McKenzie but forward ported
> and cleaned up by me to get it to work with modern kernel versions.
> 
> Tested on my mac mini and it actually works!
> 
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> ---
> 
> Jiri, is it ok for this quirks addtion to go through Dmitry's triee?  Or
> do you want me to split it out into two different patches?
> 
> Dmitry, is this ok to go through your tree?  Or I can take it as well if
> you don't want it :)
>

I'll take it, although I have a couple of comments.

> +
> +struct appleir {
> +	struct input_dev *input_dev;
> +	u8 *data;
> +	dma_addr_t dma_buf;
> +	struct usb_device *usbdev;
> +	struct urb *urb;
> +	int timer_initted;
> +	struct timer_list key_up_timer;
> +	int current_key;
> +	char phys[32];
> +};
> +
> +static struct usb_device_id appleir_ids[] = {
> +	{USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IR)},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(usb, appleir_ids);
> +
> +/* I have two devices both of which report the following */
> +/* 25 87 ee 83 0a  	+  */
> +/* 25 87 ee 83 0c  	-  */
> +/* 25 87 ee 83 09	<< */
> +/* 25 87 ee 83 06	>> */
> +/* 25 87 ee 83 05	>" */
> +/* 25 87 ee 83 03	menu */
> +/* 26 00 00 00 00	for key repeat*/
> +
> +/* Thomas Glanzmann reports the following responses */
> +/* 25 87 ee ca 0b	+  */
> +/* 25 87 ee ca 0d	-  */
> +/* 25 87 ee ca 08	<< */
> +/* 25 87 ee ca 07	>> */
> +/* 25 87 ee ca 04	>" */
> +/* 25 87 ee ca 02 	menu */
> +/* 26 00 00 00 00       for key repeat*/
> +/* He also observes the following event sometimes */
> +/* sent after a key is release, which I interpret */
> +/* as a flat battery message */
> +/* 25 87 e0 ca 06	flat battery */
> +
> +static int keymap[MAX_KEYS] = {
> +	KEY_RESERVED,
> +	KEY_MENU,
> +	KEY_PLAYPAUSE,
> +	KEY_NEXTSONG,
> +	KEY_PREVIOUSSONG,
> +	KEY_VOLUMEUP,
> +	KEY_VOLUMEDOWN,
> +	KEY_RESERVED,
> +};
> +
> +static void dump_packet(struct appleir *appleir, char *msg, u8 *data, int len)
> +{
> +	int i;
> +
> +	printk(KERN_ERR "appleir: %s (%d bytes)", msg, len);
> +
> +	for (i = 0; i < len; ++i)
> +		printk(" %02x", data[i]);
> +	printk("\n");
> +}
> +
> +static void key_up(struct appleir *appleir, int key)
> +{
> +	/* printk (KERN_ERR "key %d up\n", key); */
> +	input_report_key(appleir->input_dev, key, 0);
> +	input_sync(appleir->input_dev);
> +}
> +
> +static void key_down(struct appleir *appleir, int key)
> +{
> +	/* printk (KERN_ERR "key %d down\n", key); */
> +	input_report_key(appleir->input_dev, key, 1);
> +	input_sync(appleir->input_dev);
> +}
> +
> +static void battery_flat(struct appleir *appleir)
> +{
> +	dev_err(&appleir->input_dev->dev, "possible flat battery?\n");
> +}
> +
> +static void key_up_tick(unsigned long data)
> +{
> +	struct appleir *appleir = (struct appleir *)data;
> +
> +	if (appleir->current_key) {
> +		key_up(appleir, appleir->current_key);
> +		appleir->current_key = 0;
> +	}
> +}
> +
> +static void new_data(struct appleir *appleir, u8 *data, int len)
> +{
> +	static const u8 keydown[] = { 0x25, 0x87, 0xee };
> +	static const u8 keyrepeat[] = { 0x26, 0x00, 0x00, 0x00, 0x00 };
> +	static const u8 flatbattery[] = { 0x25, 0x87, 0xe0 };
> +
> +#if 0
> +	dump_packet(appleir, "received", data, len);
> +#endif
> +
> +	if (len != 5)
> +		return;
> +
> +	if (!memcmp(data, keydown, sizeof(keydown))) {
> +		/*If we already have a key down, take it up before marking */
> +		/*this one down */
> +		if (appleir->current_key)
> +			key_up(appleir, appleir->current_key);
> +		appleir->current_key = keymap[(data[4] >> 1) & MAX_KEYS_MASK];
> +
> +		key_down(appleir, appleir->current_key);
> +		/*remote doesn't do key up, either pull them up, in the test */
> +		/*above, or here set a timer which pulls them up after 1/8 s */
> +		mod_timer(&appleir->key_up_timer, jiffies + HZ / 8);
> +
> +		return;
> +	}
> +
> +	if (!memcmp(data, keyrepeat, sizeof(keyrepeat))) {
> +		key_down(appleir, appleir->current_key);

Repeats are usually transmitted as an event different from normal
key down (event values for repeat is 2 vs 1 for key down).

> +		/*remote doesn't do key up, either pull them up, in the test */
> +		/*above, or here set a timer which pulls them up after 1/8 s */
> +		mod_timer(&appleir->key_up_timer, jiffies + HZ / 8);
> +		return;
> +	}
> +
> +	if (!memcmp(data, flatbattery, sizeof(flatbattery))) {
> +		battery_flat(appleir);
> +		/* Fall through */
> +	}
> +
> +	dump_packet(appleir, "unknown packet", data, len);
> +}
> +
> +static void appleir_urb(struct urb *urb)
> +{
> +	struct appleir *appleir = urb->context;
> +	int status = urb->status;
> +	int retval;
> +
> +	switch (status) {
> +	case 0:
> +		new_data(appleir, urb->transfer_buffer, urb->actual_length);
> +		break;
> +	case -ECONNRESET:
> +	case -ENOENT:
> +	case -ESHUTDOWN:
> +		/* this urb is terminated, clean up */
> +		dbg("%s - urb shutting down with status: %d", __func__,
> +		    urb->status);
> +		return;
> +	default:
> +		dbg("%s - nonzero urb status received: %d", __func__,
> +		    urb->status);
> +	}
> +
> +	retval = usb_submit_urb(urb, GFP_ATOMIC);
> +	if (retval)
> +		err("%s - usb_submit_urb failed with result %d", __func__,
> +		    retval);
> +}
> +
> +static int appleir_open(struct input_dev *dev)
> +{
> +	struct appleir *appleir = input_get_drvdata(dev);
> +
> +	if (usb_submit_urb(appleir->urb, GFP_KERNEL))
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static void appleir_close(struct input_dev *dev)
> +{
> +	struct appleir *appleir = input_get_drvdata(dev);
> +
> +	usb_kill_urb(appleir->urb);
> +	del_timer_sync(&appleir->key_up_timer);
> +}
> +
> +static int appleir_probe(struct usb_interface *intf,
> +			 const struct usb_device_id *id)
> +{
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	struct usb_endpoint_descriptor *endpoint;
> +	struct appleir *appleir = NULL;

The assigment is not needed.

> +	struct input_dev *input_dev;
> +	int retval = -ENOMEM;
> +	int i;
> +
> +	appleir = kzalloc(sizeof(struct appleir), GFP_KERNEL);
> +	if (!appleir)
> +		goto fail;
> +
> +	appleir->data = usb_buffer_alloc(dev, URB_SIZE, GFP_KERNEL,
> +					 &appleir->dma_buf);
> +	if (!appleir->data)
> +		goto fail;
> +
> +	appleir->urb = usb_alloc_urb(0, GFP_KERNEL);
> +	if (!appleir->urb)
> +		goto fail;
> +
> +	appleir->usbdev = dev;
> +
> +	input_dev = input_allocate_device();
> +	if (!input_dev)
> +		goto fail;
> +
> +	appleir->input_dev = input_dev;
> +
> +	usb_make_path(dev, appleir->phys, sizeof(appleir->phys));
> +	strlcpy(appleir->phys, "/input0", sizeof(appleir->phys));
> +
> +	input_dev->name = "Apple Mac mini infrared remote control driver";
> +	input_dev->phys = appleir->phys;
> +	usb_to_input_id(dev, &input_dev->id);
> +	input_dev->dev.parent = &intf->dev;
> +	input_set_drvdata(input_dev, appleir);
> +
> +	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
> +	input_dev->ledbit[0] = 0;
> +
> +	for (i = 0; i < MAX_KEYS; i++)
> +		set_bit(keymap[i], input_dev->keybit);
> +
> +	clear_bit(0, input_dev->keybit);
> +
> +	input_dev->open = appleir_open;
> +	input_dev->close = appleir_close;
> +
> +	endpoint = &intf->cur_altsetting->endpoint[0].desc;
> +
> +	usb_fill_int_urb(appleir->urb, dev,
> +			 usb_rcvintpipe(dev, endpoint->bEndpointAddress),
> +			 appleir->data, 8,
> +			 appleir_urb, appleir, endpoint->bInterval);
> +
> +	appleir->urb->transfer_dma = appleir->dma_buf;
> +	appleir->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> +	usb_set_intfdata(intf, appleir);
> +
> +	init_timer(&appleir->key_up_timer);
> +
> +	appleir->key_up_timer.function = key_up_tick;
> +	appleir->key_up_timer.data = (unsigned long)appleir;
> +
> +	appleir->timer_initted++;
> +
> +	retval = input_register_device(appleir->input_dev);
> +	if (retval)
> +		goto fail;
> +
> +	return 0;
> +
> +fail:
> +	if (appleir) {
> +		if (appleir->data)
> +			usb_buffer_free(dev, URB_SIZE, appleir->data,
> +					appleir->dma_buf);
> +
> +		if (appleir->timer_initted)
> +			del_timer_sync(&appleir->key_up_timer);

You dont need to do it here. The timer is guranteed to be not running
since it is starte din ->open().

> +
> +		if (appleir->input_dev)
> +			input_free_device(appleir->input_dev);
> +
> +		kfree(appleir);
> +	}
> +
> +	return retval;
> +}
> +
> +static void appleir_disconnect(struct usb_interface *intf)
> +{
> +	struct appleir *appleir = usb_get_intfdata(intf);
> +
> +	usb_set_intfdata(intf, NULL);
> +	if (appleir) {
> +		input_unregister_device(appleir->input_dev);
> +		if (appleir->timer_initted)
> +			del_timer_sync(&appleir->key_up_timer);
> +		usb_kill_urb(appleir->urb);

Already done in ->close()

> +		usb_free_urb(appleir->urb);
> +		usb_buffer_free(interface_to_usbdev(intf), URB_SIZE,
> +				appleir->data, appleir->dma_buf);
> +		kfree(appleir);
> +	}
> +}
> +
> +static struct usb_driver appleir_driver = {
> +	.name = "appleir",
> +	.probe = appleir_probe,
> +	.disconnect = appleir_disconnect,
> +	.id_table = appleir_ids,
> +};
> +
> +static int __init appleir_init(void)
> +{
> +	int retval;
> +
> +	retval = usb_register(&appleir_driver);
> +	if (retval)
> +		goto out;
> +	info(DRIVER_VERSION ":" DRIVER_DESC);

Do we need to print the driver identification? I personally like drivers
to be silent unless they find a device but if you prefer to have it
that's fine.

-- 
Dmitry

Re: [PATCH] Input: add appleir USB driver

[Sitsofe Wheeler]

On Wed, 14 May 2008 16:49:09 -0700, Greg KH wrote:

> On Thu, May 15, 2008 at 12:27:22AM +0100, Matthew Garrett wrote:
>> On Wed, May 14, 2008 at 03:15:19PM -0700, Greg KH wrote:
>> 
>> > +	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL,
>> > HID_QUIRK_IGNORE },
>> >  	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4,
>> >  	HID_QUIRK_HIDDEV | HID_QUIRK_IGNORE_HIDINPUT },
>> 
>> Hm. How is the IRCONTROL4 handled? Is the protocol completely
>> different?
> 
> I know nothing about that device, sorry.  I think it is a totally
> different protocol from what I can tell looking at the code.
> 
>> > +	  Say Y here if you want to use a Apple USB remote control.  This 
+
>> >   device is traditionally inside an Intel Apple Mac Mini, but might +
>> >   show up in other places.
>> 
>> Minor nit, but it's supported on all the desktop Intel Macs and not
>> just the Mini.
> 
> Ah, didn't realize that, nice to know, we should have a wider userbase
> then :)

I'm not 100% sure whether Mac Pro desktops support the remote controls. 
Additionally saying USB remote makes it sound like a tethered device - 
perhaps it should be "USB IR remote" (although maybe that's acronym 
overload).

Do modern Mac Laptops (MacBook / MacBook Pro) support their IR remotes 
some other way?

-- 
Sitsofe | http://sucs.org/~sits/

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Wed, May 14, 2008 at 15:15:19 -0700, Greg KH wrote:
> From: Greg Kroah-Hartman <gregkh@suse.de>
> 
> This driver was originally written by James McKenzie but forward ported
> and cleaned up by me to get it to work with modern kernel versions.
> 
> Tested on my mac mini and it actually works!
> 
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> ---
> 
> Jiri, is it ok for this quirks addtion to go through Dmitry's triee?  Or
> do you want me to split it out into two different patches?
> 
> Dmitry, is this ok to go through your tree?  Or I can take it as well if
> you don't want it :)
> 
>  drivers/hid/usbhid/hid-quirks.c |    2 
>  drivers/input/misc/Kconfig      |   12 +
>  drivers/input/misc/Makefile     |    1 
>  drivers/input/misc/appleir.c    |  361 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 376 insertions(+)
> 
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -77,6 +77,7 @@
>  #define USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS   0x022e
>  #define USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY	0x030a
>  #define USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY	0x030b
> +#define USB_DEVICE_ID_APPLE_IRCONTROL	0x8240
>  #define USB_DEVICE_ID_APPLE_IRCONTROL4	0x8242
>  
>  #define USB_VENDOR_ID_ASUS		0x0b05
> @@ -443,6 +444,7 @@ static const struct hid_blacklist {
>  	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_FULLSPEED_INTERVAL },
>  
>  	{ USB_VENDOR_ID_BELKIN, USB_DEVICE_ID_FLIP_KVM, HID_QUIRK_HIDDEV },
> +	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL, HID_QUIRK_IGNORE },
>  	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4, HID_QUIRK_HIDDEV | HID_QUIRK_IGNORE_HIDINPUT },
>  	{ USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_IR_REMOTE, HID_QUIRK_HIDDEV | HID_QUIRK_IGNORE_HIDINPUT },
>  	{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_SIDEWINDER_GV, HID_QUIRK_HIDINPUT },

Hi,                                                                             
                                                                                
I'm pretty sure that this breaks the macmini LIRC driver again, see             
commit 3e1928e8793208802589aae851b6685671187242.                                
                                                                                
Regards,                                                                        
Tino                                                                            

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Wed, May 14, 2008 at 15:15:19 -0700, Greg KH wrote:
> From: Greg Kroah-Hartman <gregkh@suse.de>
> 
> This driver was originally written by James McKenzie but forward ported
> and cleaned up by me to get it to work with modern kernel versions.
> 
> Tested on my mac mini and it actually works!

Did you test suspend to RAM? Last time I used the original driver, I
had to reload it after resume, otherwise LIRC didn't receive events
anymore.

Regards,
Tino

Re: [PATCH] Input: add appleir USB driver

[Dmitry Torokhov]

Hi Tino,

On Thu, May 15, 2008 at 03:21:08PM +0200, Tino Keitel wrote:
> Hi,
>
> I'm pretty sure that this breaks the macmini LIRC driver again, see
> commit 3e1928e8793208802589aae851b6685671187242.
>

But with in-kernel driver can't LIRC feed of event device? Ane people
that dont use LIRC can also have access to the remote?

-- 
Dmitry

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Thu, May 15, 2008 at 09:45:52 -0400, Dmitry Torokhov wrote:
> Hi Tino,
> 
> On Thu, May 15, 2008 at 03:21:08PM +0200, Tino Keitel wrote:
> > Hi,
> >
> > I'm pretty sure that this breaks the macmini LIRC driver again, see
> > commit 3e1928e8793208802589aae851b6685671187242.
> >
> 
> But with in-kernel driver can't LIRC feed of event device?

Sure, it can. But in its current state, it breaks existing scenarios
and seems to have restrictions. See the comment below regarding
learning remotes, and my other mail in this thread regarding broken
suspend with the appleir driver.

> Ane people that dont use LIRC can also have access to the remote?

I know. I just wanted to point out that this is a regression for all
people who use the macmini LIRC driver. This driver was present at
least in the last 2 LIRC releases. The quirks stuff in the above patch
looks like there is no way to make the macmini LIRC driver work with
the patch applied. It relies on a USB HID device, which wouldn't be
created anymore, and the user has no way to bring it back. Please
correct me with a pointer to the appropriate documentation if I'm
wrong.

>From the user's point of view: There are no official kernel release
notes about what devices are added/removed to/from the various ignore
lists and blacklists. The kernel doesn't produce any output about
devices that are ignored or blacklisted in may cases (and also this
one). The user has no indication why his LIRC setup stops working with
the new kernel.

Even if all LIRC users switch to the appleir driver, what about people
who use a learning remote to have more than 6 keys that the Apple
remote has? Does this work at all? After a quick look at the key
handling it seems to me that the codes of the 6 keys are hardcoded in
the driver. So a learning remote with more keys wouldn't work anymore.

Regards,
Tino

Re: [PATCH] Input: add appleir USB driver

[Dmitry Torokhov]

On Thu, May 15, 2008 at 07:49:39PM +0200, Tino Keitel wrote:
> On Thu, May 15, 2008 at 09:45:52 -0400, Dmitry Torokhov wrote:
> > Hi Tino,
> > 
> > On Thu, May 15, 2008 at 03:21:08PM +0200, Tino Keitel wrote:
> > > Hi,
> > >
> > > I'm pretty sure that this breaks the macmini LIRC driver again, see
> > > commit 3e1928e8793208802589aae851b6685671187242.
> > >
> > 
> > But with in-kernel driver can't LIRC feed of event device?
> 
> Sure, it can. But in its current state, it breaks existing scenarios
> and seems to have restrictions. See the comment below regarding
> learning remotes, and my other mail in this thread regarding broken
> suspend with the appleir driver.
> 

These needs to be fixed before it is in mainline, no argument here.

> > Ane people that dont use LIRC can also have access to the remote?
> 
> I know. I just wanted to point out that this is a regression for all
> people who use the macmini LIRC driver. This driver was present at
> least in the last 2 LIRC releases. The quirks stuff in the above patch
> looks like there is no way to make the macmini LIRC driver work with
> the patch applied. It relies on a USB HID device, which wouldn't be
> created anymore, and the user has no way to bring it back. Please
> correct me with a pointer to the appropriate documentation if I'm
> wrong.
>

There is a way to dynamically manipulate quirks for a VID/PID pair via
sysfs. I think if you set the quirk to 0 it will efefctively disable
the ignore quirk restoring the old behaviour.

> From the user's point of view: There are no official kernel release
> notes about what devices are added/removed to/from the various ignore
> lists and blacklists. The kernel doesn't produce any output about
> devices that are ignored or blacklisted in may cases (and also this
> one). The user has no indication why his LIRC setup stops working with
> the new kernel.
>

Not sure what we can do here... The only thing I guess is better
commit message mentioning LIRC setup concerns.

> Even if all LIRC users switch to the appleir driver, what about people
> who use a learning remote to have more than 6 keys that the Apple
> remote has? Does this work at all? After a quick look at the key
> handling it seems to me that the codes of the 6 keys are hardcoded in
> the driver. So a learning remote with more keys wouldn't work anymore.
>

We'll have to adjust the driver to allow changing keymap on a
per-device base from userspace. That's pretty easy actually.

-- 
Dmitry

Re: [PATCH] Input: add appleir USB driver

[Greg KH]

On Thu, May 15, 2008 at 09:45:52AM -0400, Dmitry Torokhov wrote:
> Hi Tino,
> 
> On Thu, May 15, 2008 at 03:21:08PM +0200, Tino Keitel wrote:
> > Hi,
> >
> > I'm pretty sure that this breaks the macmini LIRC driver again, see
> > commit 3e1928e8793208802589aae851b6685671187242.
> >
> 
> But with in-kernel driver can't LIRC feed of event device? Ane people
> that dont use LIRC can also have access to the remote?

And, to be blunt, why would an in-kernel driver be concerned about an
out-of-tree-project-that-refuses-to-be-included chunk of code?

thanks,

greg k-h

Re: [PATCH] Input: add appleir USB driver

[Greg KH]

On Thu, May 15, 2008 at 03:40:46PM +0200, Tino Keitel wrote:
> On Wed, May 14, 2008 at 15:15:19 -0700, Greg KH wrote:
> > From: Greg Kroah-Hartman <gregkh@suse.de>
> > 
> > This driver was originally written by James McKenzie but forward ported
> > and cleaned up by me to get it to work with modern kernel versions.
> > 
> > Tested on my mac mini and it actually works!
> 
> Did you test suspend to RAM? Last time I used the original driver, I
> had to reload it after resume, otherwise LIRC didn't receive events
> anymore.

Yes, suspend to ram did seem to work for me with this driver.  But I
haven't tested that in a while, will do so later today.

I don't use LIRC though, so I really can not speak for that chunk of
code at all.

thanks,

greg k-h

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Thu, May 15, 2008 at 14:35:54 -0400, Dmitry Torokhov wrote:
> On Thu, May 15, 2008 at 07:49:39PM +0200, Tino Keitel wrote:

[...]

> > I know. I just wanted to point out that this is a regression for all
> > people who use the macmini LIRC driver. This driver was present at
> > least in the last 2 LIRC releases. The quirks stuff in the above patch
> > looks like there is no way to make the macmini LIRC driver work with
> > the patch applied. It relies on a USB HID device, which wouldn't be
> > created anymore, and the user has no way to bring it back. Please
> > correct me with a pointer to the appropriate documentation if I'm
> > wrong.
> >
> 
> There is a way to dynamically manipulate quirks for a VID/PID pair
> via sysfs. I think if you set the quirk to 0 it will efefctively
> disable the ignore quirk restoring the old behaviour.

Where in sysfs? I found module/usbhid/parameters/quirks and quirks
files in several devices/pci0000:00/* directories. I guess you are
talking about the latter.

How can I tell what device I need to use, or what is the recommended
way? I can only think of something ugly like

$ find /sys -name "idProduct" | xargs grep 8240

and similar with idVendor.

Does setting the quirks to 0 lead to the creation of a HID device
immediately? Where is this documented? Could it be implemented in a way
such that the user has a HID device, as long as he doesn't enable the
appleir driver?

> 
> > From the user's point of view: There are no official kernel release
> > notes about what devices are added/removed to/from the various
> > ignore lists and blacklists. The kernel doesn't produce any output
> > about devices that are ignored or blacklisted in may cases (and
> > also this one). The user has no indication why his LIRC setup stops
> > working with the new kernel.
> >
> 
> Not sure what we can do here... The only thing I guess is better
> commit message mentioning LIRC setup concerns.

Who reads commit messages? I think it should be easy to add some
printk()s saying something like "skipping device foo, because it is on
the ignore list".

> > Even if all LIRC users switch to the appleir driver, what about
> > people who use a learning remote to have more than 6 keys that the
> > Apple remote has? Does this work at all? After a quick look at the
> > key handling it seems to me that the codes of the 6 keys are
> > hardcoded in the driver. So a learning remote with more keys
> > wouldn't work anymore.
> >
> 
> We'll have to adjust the driver to allow changing keymap on a
> per-device base from userspace. That's pretty easy actually.

I'm not talking about the keymap that is visible in userspace, but
about the keys on the remote that are detected by the kernel. The Apple
remote has only 6 keys, and they are mapped in a static array:

#define MAX_KEYS       8
static int keymap[MAX_KEYS] = ...

With the LIRC driver, I can use a learning remote with much more keys,
and then just use irrecord to create a LIRC config file.

Regards,
Tino

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Thu, May 15, 2008 at 11:40:34 -0700, Greg KH wrote:
> On Thu, May 15, 2008 at 09:45:52AM -0400, Dmitry Torokhov wrote:
> > Hi Tino,
> > 
> > On Thu, May 15, 2008 at 03:21:08PM +0200, Tino Keitel wrote:
> > > Hi,
> > >
> > > I'm pretty sure that this breaks the macmini LIRC driver again, see
> > > commit 3e1928e8793208802589aae851b6685671187242.
> > >
> > 
> > But with in-kernel driver can't LIRC feed of event device? Ane people
> > that dont use LIRC can also have access to the remote?
> 
> And, to be blunt, why would an in-kernel driver be concerned about an
> out-of-tree-project-that-refuses-to-be-included chunk of code?

The macmini LIRC driver is a pure userspace driver, so nothing needs to
be ever included in the kernel.

Regards,
Tino

Re: [PATCH] Input: add appleir USB driver

[Greg KH]

On Thu, May 15, 2008 at 10:59:59PM +0200, Tino Keitel wrote:
> On Thu, May 15, 2008 at 11:40:34 -0700, Greg KH wrote:
> > On Thu, May 15, 2008 at 09:45:52AM -0400, Dmitry Torokhov wrote:
> > > Hi Tino,
> > > 
> > > On Thu, May 15, 2008 at 03:21:08PM +0200, Tino Keitel wrote:
> > > > Hi,
> > > >
> > > > I'm pretty sure that this breaks the macmini LIRC driver again, see
> > > > commit 3e1928e8793208802589aae851b6685671187242.
> > > >
> > > 
> > > But with in-kernel driver can't LIRC feed of event device? Ane people
> > > that dont use LIRC can also have access to the remote?
> > 
> > And, to be blunt, why would an in-kernel driver be concerned about an
> > out-of-tree-project-that-refuses-to-be-included chunk of code?
> 
> The macmini LIRC driver is a pure userspace driver, so nothing needs to
> be ever included in the kernel.

So does this driver work with the macmini USB ir device today somehow?

>From what I can tell, this driver is still necessary for things to work
properly for this hardware, right?

I'm totally confused now...

greg k-h

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Thu, May 15, 2008 at 14:11:17 -0700, Greg KH wrote:
> On Thu, May 15, 2008 at 10:59:59PM +0200, Tino Keitel wrote:

[...]

> > The macmini LIRC driver is a pure userspace driver, so nothing
> > needs to
> > be ever included in the kernel.
>
> So does this driver work with the macmini USB ir device today
> somehow?

Yes, I'm using it for months.

> From what I can tell, this driver is still necessary for things to
> work
> properly for this hardware, right?

When the appleir kernel driver is used, the IR device acts as in input
device, not as a HID device. LIRC has to be configured to use its
dev/input driver in this case. This is a generic driver to use any
input device as lirc device. The dev/input driver relies on the key
events that are generated by the kernel.

Regards,
Tino

Re: [PATCH] Input: add appleir USB driver

[Greg KH]

On Fri, May 16, 2008 at 01:27:33AM +0200, Tino Keitel wrote:
> On Thu, May 15, 2008 at 14:11:17 -0700, Greg KH wrote:
> > On Thu, May 15, 2008 at 10:59:59PM +0200, Tino Keitel wrote:
> 
> [...]
> 
> > > The macmini LIRC driver is a pure userspace driver, so nothing
> > > needs to
> > > be ever included in the kernel.
> >
> > So does this driver work with the macmini USB ir device today
> > somehow?
> 
> Yes, I'm using it for months.
> 
> > From what I can tell, this driver is still necessary for things to
> > work
> > properly for this hardware, right?
> 
> When the appleir kernel driver is used, the IR device acts as in input
> device, not as a HID device. LIRC has to be configured to use its
> dev/input driver in this case. This is a generic driver to use any
> input device as lirc device. The dev/input driver relies on the key
> events that are generated by the kernel.

Hm, I just got a private email (for some odd reason) saying this driver
isn't needed at all, they just add a hid quirk line.

Now why that hid quirk line hadn't been added for the year or so that
the person knew about it, sure is odd to me...

I'll test it out and probably reduce this whole thing to a one-line
hid-quirk patch...

ugh, why do people not send stuff upstream, it just wastes everyone's
time in the end :(

greg k-h

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Thu, May 15, 2008 at 19:32:36 -0700, Greg KH wrote:
> On Fri, May 16, 2008 at 01:27:33AM +0200, Tino Keitel wrote:

[...]

> > When the appleir kernel driver is used, the IR device acts as in input
> > device, not as a HID device. LIRC has to be configured to use its
> > dev/input driver in this case. This is a generic driver to use any
> > input device as lirc device. The dev/input driver relies on the key
> > events that are generated by the kernel.
> 
> Hm, I just got a private email (for some odd reason) saying this driver
> isn't needed at all, they just add a hid quirk line.

Maybe this email came from a person who uses a distro kernel that
already included the appleir driver? IIRC at least Ubuntu did that
once. If not, I'd be interested in the detailed setup.

> Now why that hid quirk line hadn't been added for the year or so that
> the person knew about it, sure is odd to me...

There was a quirk line in the past. It was introduced by accident with
2.6.22 with commit a417a21e10831bca695b4ba9c74f4ddf5a95ac06. The commit
message only mentions keyboard and mouse devices, so I think the quirk
for the IR device was unintended.

Then I wasted several hours when I tried to use the macmini LIRC driver
with 2.6.22. The HID device that I saw came from my LCD, not from the
IR device, and I wasn't aware of the HID ignore quirk as the kernel
didn't mention it when the device was detected. That's why I also
suggested that the kernel should be more verbose about such ignore
quirks and blacklists.

In the end I found that quirk an requested to remove it. This was done
in 2.6.23-rc2.

Regards,
Tino

Re: [PATCH] Input: add appleir USB driver

[Jiri Kosina]

On Thu, 15 May 2008, Tino Keitel wrote:

> Does setting the quirks to 0 lead to the creation of a HID device 
> immediately? 

The easiest way is either to handle this in the early stage, i.e. specify 
'quirks' module parameter during the module insertion.

Other possibility is to detach the driver from the device using 'unbind' 
functionality of sysfs. libusb has a call for this too.

> Where is this documented? 

modinfo usbhid.ko

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Fri, May 16, 2008 at 09:19:39 +0200, Jiri Kosina wrote:
> On Thu, 15 May 2008, Tino Keitel wrote:
> 
> > Does setting the quirks to 0 lead to the creation of a HID device 
> > immediately? 
> 
> The easiest way is either to handle this in the early stage, i.e. specify 
> 'quirks' module parameter during the module insertion.
> 
> Other possibility is to detach the driver from the device using 'unbind' 
> functionality of sysfs. libusb has a call for this too.

Thanks for the hint.

So I guess that the LIRC driver should be extended to watch out for a
USB device with the prober vendor/device ID and unbind it before usage,
right?

This wouldn't help users who use the current LIRC with a kernel that
has the HID ignore quirk enabled by default, though (i.e., it would be
a regression).

Regards,
Tino

Re: [PATCH] Input: add appleir USB driver

[Dmitry Torokhov]

On Thu, May 15, 2008 at 10:59:50PM +0200, Tino Keitel wrote:
> On Thu, May 15, 2008 at 14:35:54 -0400, Dmitry Torokhov wrote:
> > On Thu, May 15, 2008 at 07:49:39PM +0200, Tino Keitel wrote:
> > 
> > > From the user's point of view: There are no official kernel release
> > > notes about what devices are added/removed to/from the various
> > > ignore lists and blacklists. The kernel doesn't produce any output
> > > about devices that are ignored or blacklisted in may cases (and
> > > also this one). The user has no indication why his LIRC setup stops
> > > working with the new kernel.
> > >
> > 
> > Not sure what we can do here... The only thing I guess is better
> > commit message mentioning LIRC setup concerns.
> 
> Who reads commit messages? I think it should be easy to add some
> printk()s saying something like "skipping device foo, because it is on
> the ignore list".

Then every user of Wacom tablet will be alarmed and ask "why my tablet
is ignored". The only thing that I can think of is making that quirk
compiled in depending on whether CONFIG_APPLEIR is selected.

> 
> > > Even if all LIRC users switch to the appleir driver, what about
> > > people who use a learning remote to have more than 6 keys that the
> > > Apple remote has? Does this work at all? After a quick look at the
> > > key handling it seems to me that the codes of the 6 keys are
> > > hardcoded in the driver. So a learning remote with more keys
> > > wouldn't work anymore.
> > >
> > 
> > We'll have to adjust the driver to allow changing keymap on a
> > per-device base from userspace. That's pretty easy actually.
> 
> I'm not talking about the keymap that is visible in userspace, but
> about the keys on the remote that are detected by the kernel. The Apple
> remote has only 6 keys, and they are mapped in a static array:
> 
> #define MAX_KEYS       8
> static int keymap[MAX_KEYS] = ...
> 
> With the LIRC driver, I can use a learning remote with much more keys,
> and then just use irrecord to create a LIRC config file.
> 

Does the learning remote have the same VID/PID as AppleIr? If not I am
not sure why you are btringing it here.

-- 
Dmitry

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Fri, May 16, 2008 at 09:13:41 -0400, Dmitry Torokhov wrote:

[...]

> Does the learning remote have the same VID/PID as AppleIr? If not I am
> not sure why you are btringing it here.

It has no VID/PID at all, as it is not connected to the computer. It
just sends IR codes to the IR sensor in the Mac mini (the builtin USB
device that the appleir driver is for). And as the learning remote has
more than 6 keys, it can send more than 6 different codes. The Apple
remote has only 6 keys, and only these keys are handled by the driver.

Regards,
Tino

Re: [PATCH] Input: add appleir USB driver

[Dmitry Torokhov]

On Fri, May 16, 2008 at 03:32:34PM +0200, Tino Keitel wrote:
> On Fri, May 16, 2008 at 09:13:41 -0400, Dmitry Torokhov wrote:
> 
> [...]
> 
> > Does the learning remote have the same VID/PID as AppleIr? If not I am
> > not sure why you are btringing it here.
> 
> It has no VID/PID at all, as it is not connected to the computer. It
> just sends IR codes to the IR sensor in the Mac mini (the builtin USB
> device that the appleir driver is for). And as the learning remote has
> more than 6 keys, it can send more than 6 different codes. The Apple
> remote has only 6 keys, and only these keys are handled by the driver.
> 

OK, I see.. How many codes does the sensor support? We should simply
allow for the key table big enough and allow userpsace to
assign/modify keycodes.

-- 
Dmitry

Re: [PATCH] Input: add appleir USB driver

[Tino Keitel]

On Fri, May 16, 2008 at 09:53:47 -0400, Dmitry Torokhov wrote:
> On Fri, May 16, 2008 at 03:32:34PM +0200, Tino Keitel wrote:
> > On Fri, May 16, 2008 at 09:13:41 -0400, Dmitry Torokhov wrote:
> > 
> > [...]
> > 
> > > Does the learning remote have the same VID/PID as AppleIr? If not I am
> > > not sure why you are btringing it here.
> > 
> > It has no VID/PID at all, as it is not connected to the computer. It
> > just sends IR codes to the IR sensor in the Mac mini (the builtin USB
> > device that the appleir driver is for). And as the learning remote has
> > more than 6 keys, it can send more than 6 different codes. The Apple
> > remote has only 6 keys, and only these keys are handled by the driver.
> > 
> 
> OK, I see.. How many codes does the sensor support? We should simply
> allow for the key table big enough and allow userpsace to
> assign/modify keycodes.

I have no idea. I currently don't have access to my lircd.conf, but
IIRC the keycodes differ in 2 changing bytes.

Regards,
Tino

Re: [PATCH] Input: add appleir USB driver

[Scott D. Davilla]

That would be me sending to Greg. my bad. I should be posting here 
and I've disciplined myself accordingly (a smack upside the head).

The history of the appleir patch is that James McKenzie created it as 
a method to get IR controller support working for the MacMini. This 
was way back in time before Apple EFI even had a PC BIOS 
compatibility layer.

The patch was migrated to the MacTel mactel-linux svn by huceke and 
stayed for 2.6.17, 2.6.18, 2.6.20 and 2.6.21 kernel usage. There were 
numerous other patches there that did not migrate upstream for 
various reasons mainly I think because it was known that they would 
never be accepted. These were niche patches for Linux support on 
Apple hardware, some using EFI boots, others using Apples EFI PC BIOS 
compatibility layer. Some patches were pushed upstream (imacfb), 
others stayed.

In 2.6.22, the appleir patch was dropped at mactel-linux because 
usbhid support was added in the kernel by including the proper 
VID/PID so that the Apple IR devices (now more than one) was 
recognized by usbhid and can be seen as "/dev/usb/usbhid0" typically. 
I'm not sure when support for the MacMini using usbhid was added to 
LIRC.

Just about every distro that has any form of IR support uses LIRC and 
LIRC uses usbhid for Apple IR support. This is a userland device as 
it should be.

The Apple IR remote can increment an IR user device id such that you 
can pair specific IR remotes to computers and not have them control 
each other. This user device id is the last byte in the pre_data 
(LIRC speak) so there are 256 possible values. There are also 256 
possible key values. I believe there might be forbidden values as the 
actual IR protocol has a checksum. You can use a JP1 programmable 
remote or a learning remote to "extend" beyond the six button limit 
of the Apple IR remote controller. Numerous users do this as it 
eliminates having to purchase a replacement IR remote and IR receiver 
if they find a six button IR remote limiting.

So while you could continue with including this patch, you would need 
to include key tables for 256 x 256 keys and provide a method to 
disable this device in favor of the usbhid method. A method to 
disable the usbhid usage could be using the modprobe option usbhid 
quirk and passing the ignore flag (the usbhid modprobe quirk passing 
was enabled in 2.6.23.x

I do not understand why this patch is not dropped. The Apple IR is a 
USB HID device and LIRC handles this very nicely. If you do not want 
to use LIRC, then talk to the usbhid device directly.

I use it for the AppleTV (which has an even different device id). I 
have two patches pending that add IR support and audio support for 
the AppleTV. They are pending even though the patches have existed 
since the 2.6.22 kernel because the only way then to boot the AppleTV 
was a nasty EFI patch which would never be accepted.

The AppleTV now has a stable bootloader (atv-bootloader) that uses a 
standard non-EFI linux kerenel and kexec to launch the real standard 
kernel in a similar method as Linux on the PS3. The atv-bootloader 
translates EFI structures into E820 structures and moves the ACPI and 
SMBIOS pointers to where Linux can find them and sets up for a vesafb 
device. The AppleTV is a pure EFI box with no PC BIOS compatibility 
layer.

With atv-bootloader in place and stable (several months and numerous 
users), now I can work on pushing these two patches upstream.

I'm off list so please "CC" to respond.

Scott