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

drivers/hid/usbhid/hid-quirks.c +2 -0

@@ -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 },

drivers/input/misc/appleir.c new file +361 -0

361 lines not on this page · raw message

drivers/input/misc/Kconfig +12 -0

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

drivers/input/misc/Makefile +1 -0

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

Matthew Garrett

2008-05-14 23:27 UTC #1 lore raw

drivers/hid/usbhid/hid-quirks.c static const struct hid_blacklist { · line 448

Quoted 2 lines · Greg KH

drivers/hid/usbhid/hid-quirks.c +1 -0

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

drivers/input/misc/Kconfig config INPUT_KEYSPAN_REMOTE · line 159

Quoted 3 lines

drivers/input/misc/Kconfig +3 -0

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

Signature · 1 line

Matthew Garrett | [email protected]

Greg KH

author

2008-05-14 23:55 UTC #2 lore raw

Quoted 6 lines · Matthew Garrett

drivers/hid/usbhid/hid-quirks.c static const struct hid_blacklist { · line 448
Quoted 2 lines · Greg KH
drivers/hid/usbhid/hid-quirks.c +1 -0
+	{ 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.

Quoted 6 lines

drivers/input/misc/Kconfig config INPUT_KEYSPAN_REMOTE · line 159
Quoted 3 lines
drivers/input/misc/Kconfig +3 -0
+	  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

Sitsofe Wheeler

2008-05-15 06:20 UTC #3 lore raw

Quoted 11 lines · Greg KH

Quoted 9 lines · Matthew Garrett
Review
Quoted 4 lines · Greg KH
(quoted diff) +1 -0
+	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL,
HID_QUIRK_IGNORE },
(quoted diff) +0 -0
 	{ 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?

Quoted 4 lines

I know nothing about that device, sorry. I think it is a totally different protocol from what I can tell looking at the code.
Quoted 1 line
drivers/input/misc/Kconfig config INPUT_KEYSPAN_REMOTE · line 157
Quoted 1 line
drivers/input/misc/Kconfig +1 -0
+	  Say Y here if you want to use a Apple USB remote control.  This 

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
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?

Signature · 1 line

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

Dmitry Torokhov

maintainer

2008-05-15 03:50 UTC #4 lore raw

In reply to Greg KH

Hi Greg,

Quoted 9 lines · Greg KH

From: Greg Kroah-Hartman <[email protected]>
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 <[email protected]>

Quoted 8 lines

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

Quoted 112 lines

drivers/input/misc/appleir.c new file +112 -0

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

drivers/input/misc/appleir.c line 164

Quoted 10 lines

drivers/input/misc/appleir.c new file +10 -0

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

Quoted 56 lines

drivers/input/misc/appleir.c new file +56 -0

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

drivers/input/misc/appleir.c line 230

Quoted 10 lines

drivers/input/misc/appleir.c new file +10 -0

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

Quoted 68 lines

drivers/input/misc/appleir.c new file +68 -0

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

drivers/input/misc/appleir.c line 308

Quoted 10 lines

drivers/input/misc/appleir.c new file +10 -0

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

Quoted 10 lines

drivers/input/misc/appleir.c new file +10 -0

+
+		if (appleir->input_dev)
+			input_free_device(appleir->input_dev);
+
+		kfree(appleir);
+	}
+
+	return retval;
+}
+

drivers/input/misc/appleir.c line 328

Quoted 10 lines

drivers/input/misc/appleir.c new file +10 -0

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

Quoted 12 lines

drivers/input/misc/appleir.c new file +12 -0

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

drivers/input/misc/appleir.c line 350

Quoted 10 lines

drivers/input/misc/appleir.c new file +10 -0

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

Signature · 1 line

Dmitry

Tino Keitel

2008-05-15 13:21 UTC #5 lore raw

In reply to Greg KH

Quoted 34 lines · Greg KH

From: Greg Kroah-Hartman <[email protected]>
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 <[email protected]>
---
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(+)
drivers/hid/usbhid/hid-quirks.c +1 -0
@@ -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

drivers/hid/usbhid/hid-quirks.c static const struct hid_blacklist { · line 450

Quoted 7 lines

drivers/hid/usbhid/hid-quirks.c +1 -0

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

Dmitry Torokhov

maintainer

2008-05-15 13:45 UTC #6 lore raw

Hi Tino,

Quoted 5 lines · Tino Keitel

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?

Signature · 5 lines

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

Tino Keitel

2008-05-15 17:49 UTC #7 lore raw

Quoted 10 lines · Dmitry Torokhov

Hi Tino,
Quoted 5 lines · Tino Keitel
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.

Quoted 1 line

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.

Quoted 1 line

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

Dmitry Torokhov

maintainer

2008-05-15 18:35 UTC #8 lore raw

Quoted 10 lines · Tino Keitel

Quoted 9 lines · Dmitry Torokhov
Hi Tino,
Quoted 5 lines · Tino Keitel
Hi,
I'm pretty sure that this breaks the macmini LIRC driver again, see commit 3e1928e8793208802589aae851b6685671187242.

Quoted 7 lines

Quoted 1 line
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.

Quoted 11 lines

Quoted 1 line
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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Signature · 5 lines

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

Tino Keitel

2008-05-15 20:59 UTC #9 lore raw

Quoted 1 line · Dmitry Torokhov

On Thu, May 15, 2008 at 07:49:39PM +0200, Tino Keitel wrote:

[...]

Quoted 10 lines

Quoted 9 lines
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.

Quoted 3 lines

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?

Quoted 11 lines

Quoted 8 lines
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".

Quoted 10 lines

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

Jiri Kosina

2008-05-16 07:19 UTC #10 lore raw

Quoted 2 lines · Tino Keitel

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.

Quoted 1 line

Where is this documented?

modinfo usbhid.ko

Signature · 2 lines

Jiri Kosina
SUSE Labs

Tino Keitel

2008-05-16 07:26 UTC #11 lore raw

Quoted 10 lines · Jiri Kosina

Quoted 2 lines · Tino Keitel
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

Dmitry Torokhov

maintainer

2008-05-16 13:13 UTC #12 lore raw

In reply to Tino Keitel

Quoted 11 lines · Tino Keitel

Quoted 10 lines · Dmitry Torokhov
Quoted 7 lines · Tino Keitel
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.

Quoted 6 lines

Quoted 2 lines
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.

Quoted 16 lines

Quoted 11 lines
Quoted 8 lines
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:

Quoted 6 lines

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

Signature · 1 line

Dmitry

Tino Keitel

2008-05-16 13:35 UTC #13 lore raw

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

[...]

Quoted 2 lines

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

Dmitry Torokhov

maintainer

2008-05-16 13:53 UTC #14 lore raw

Quoted 4 lines · Tino Keitel

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

Quoted 9 lines

Quoted 2 lines
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.

Signature · 5 lines

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

Tino Keitel

2008-05-16 14:07 UTC #15 lore raw

Quoted 15 lines · Dmitry Torokhov

Quoted 4 lines · Tino Keitel
On Fri, May 16, 2008 at 09:13:41 -0400, Dmitry Torokhov wrote:
[...]
Quoted 9 lines
Quoted 2 lines
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.

Quoted 3 lines

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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Greg KH

author

2008-05-15 19:50 UTC #16 lore raw

In reply to Dmitry Torokhov

Quoted 11 lines · Dmitry Torokhov

Hi Tino,
Quoted 5 lines · Tino Keitel
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

Tino Keitel

2008-05-15 20:59 UTC #17 lore raw

Quoted 10 lines · Greg KH

Quoted 9 lines · Dmitry Torokhov
Hi Tino,
Quoted 5 lines · Tino Keitel
Hi,
I'm pretty sure that this breaks the macmini LIRC driver again, see commit 3e1928e8793208802589aae851b6685671187242.

Quoted 5 lines

Quoted 2 lines
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

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

Greg KH

author

2008-05-15 21:11 UTC #18 lore raw

Quoted 11 lines · Tino Keitel

Quoted 10 lines · Greg KH
Quoted 9 lines · Dmitry Torokhov
Hi Tino,
Quoted 5 lines · Tino Keitel
Hi,
I'm pretty sure that this breaks the macmini LIRC driver again, see commit 3e1928e8793208802589aae851b6685671187242.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
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?

Quoted 1 line

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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Tino Keitel

2008-05-15 23:26 UTC #19 lore raw

Quoted 1 line · Greg KH

On Thu, May 15, 2008 at 10:59:59PM +0200, Tino Keitel wrote:

[...]

Quoted 6 lines

Quoted 3 lines
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.

Quoted 3 lines

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

Greg KH

author

2008-05-16 02:33 UTC #20 lore raw

Quoted 18 lines · Tino Keitel

Quoted 1 line · Greg KH
On Thu, May 15, 2008 at 10:59:59PM +0200, Tino Keitel wrote:
[...]
Quoted 6 lines
Quoted 3 lines
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.
Quoted 3 lines
From what I can tell, this driver is still necessary for things to work
properly for this hardware, right?

Quoted 5 lines

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

Tino Keitel

2008-05-16 05:43 UTC #21 lore raw

Quoted 1 line · Greg KH

On Fri, May 16, 2008 at 01:27:33AM +0200, Tino Keitel wrote:

[...]

Quoted 8 lines

Quoted 5 lines
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.

Quoted 2 lines

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

Tino Keitel

2008-05-15 13:40 UTC #22 lore raw

In reply to Greg KH

Quoted 6 lines · Greg KH

From: Greg Kroah-Hartman <[email protected]>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Greg KH

author

2008-05-15 19:50 UTC #23 lore raw

Quoted 11 lines · Tino Keitel

Quoted 6 lines · Greg KH
From: Greg Kroah-Hartman <[email protected]>
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