Re: [PATCH] Input: add appleir USB driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bastien,

(adding the input and HID maintainers to the recipient list).

On Thu, Nov 15, 2012 at 7:13 PM, Bastien Nocera <hadess@xxxxxxxxxx> wrote:
>
> This driver was originally written by James McKenzie, updated by
> Greg Kroah-Hartman, further updated by myself, with suspend support
> added.
>
> More recent versions of the IR receiver are also supported through
> a patch by Alex Karpenko. The patch also adds support for the 2nd
> and 5th generation of the controller, and the menu key on newer
> brushed metal remotes.
>
> Tested on a MacbookAir1,1
>
> Signed-off-by: Bastien Nocera <hadess@xxxxxxxxxx>
> ---
>
> Resend, as the original patch never made it. I cleaned up the patch a
> bit further, and test compiled it, but didn't have a chance to test it
> as I don't have a machine with that hardware available anymore.

Fabien, in CC, gracefully accepted to test and to try to adapt this
patch depending on the reviews. So we can ask for tests and changes!

>
>  Documentation/input/appleir.txt |  46 ++++
>  drivers/hid/hid-apple.c         |   4 -
>  drivers/hid/hid-core.c          |   7 +-
>  drivers/hid/hid-ids.h           |   5 +-
>  drivers/input/misc/Kconfig      |  13 +
>  drivers/input/misc/Makefile     |   1 +
>  drivers/input/misc/appleir.c    | 527 ++++++++++++++++++++++++++++++++++++++++

If this device presents itself as a hid device, there are much chances
that we can use the hid .raw_event interface. This will help us for
the usb part and remove potential bugs and support and greatly
simplify this driver.

>  7 files changed, 596 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/input/appleir.txt
>  create mode 100644 drivers/input/misc/appleir.c
>
> diff --git a/Documentation/input/appleir.txt b/Documentation/input/appleir.txt
> new file mode 100644
> index 0000000..db637fb
> --- /dev/null
> +++ b/Documentation/input/appleir.txt
> @@ -0,0 +1,46 @@
> +Apple IR receiver Driver (appleir)
> +----------------------------------
> +       Copyright (C) 2009 Bastien Nocera <hadess@xxxxxxxxxx>
> +
> +The appleir driver is a kernel input driver to handle Apple's IR
> +receivers (and associated remotes) in the kernel.
> +
> +The driver is an input driver which only handles "official" remotes
> +as built and sold by Apple.
> +
> +Authors
> +-------
> +
> +James McKenzie (original driver)
> +Alex Karpenko (05ac:8242 support)
> +Greg Kroah-Hartman (cleanups and original submission)
> +Bastien Nocera (further cleanups, brushed metal "enter"
> +button support and suspend support)
> +
> +Supported hardware
> +------------------
> +
> +- All Apple laptops and desktops from 2005 onwards, except:
> +  - the unibody Macbook (2009)
> +  - Mac Pro (all versions)
> +- Apple TV (all revisions prior to September 2010)
> +
> +The remote will only support the 6 (old white) or 7 (brushed metal) buttons
> +of the remotes as sold by Apple. See the next section if you want to use
> +other remotes or want to use lirc with the device instead of the kernel driver.
> +
> +Using lirc (native) instead of the kernel driver
> +------------------------------------------------
> +
> +First, you will need to disable the kernel driver for the receiver.
> +
> +This can be achieved by passing quirks to the usbhid driver.
> +The quirk line would be:
> +usbhid.quirks=0x05ac:0x8242:0x40000010
> +
> +With 0x05ac being the vendor ID (Apple, you shouldn't need to change this)
> +With 0x8242 being the product ID (check the output of lsusb for your hardware)
> +And 0x10 being "HID_QUIRK_HIDDEV_FORCE" and 0x40000000 being "HID_QUIRK_NO_IGNORE"
> +
> +This should force the creation of a hiddev device for the receiver, and
> +make it usable under lirc.
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index fd7722a..30a4824 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -390,10 +390,6 @@ static void apple_remove(struct hid_device *hdev)
>  }
>
>  static const struct hid_device_id apple_devices[] = {
> -       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ATV_IRCONTROL),
> -               .driver_data = APPLE_HIDDEV | APPLE_IGNORE_HIDINPUT },
> -       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4),
> -               .driver_data = APPLE_HIDDEV | APPLE_IGNORE_HIDINPUT },
>         { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MIGHTYMOUSE),
>                 .driver_data = APPLE_MIGHTYMOUSE | APPLE_INVERT_HWHEEL },
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index f4109fd..3fd4c10 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1471,8 +1471,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
>         { HID_USB_DEVICE(USB_VENDOR_ID_A4TECH, USB_DEVICE_ID_A4TECH_X5_005D) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_A4TECH, USB_DEVICE_ID_A4TECH_RP_649) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_ACRUX, 0x0802) },
> -       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ATV_IRCONTROL) },
> -       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MIGHTYMOUSE) },
>         { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGICMOUSE) },
>         { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGICTRACKPAD) },
> @@ -1925,6 +1923,11 @@ static const struct hid_device_id hid_ignore_list[] = {
>         { HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_24) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_AIRCABLE, USB_DEVICE_ID_AIRCABLE1) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_ALCOR, USB_DEVICE_ID_ALCOR_USBRS232) },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL) },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL2) },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL3) },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4) },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL5) },

Using a hid kernel driver will move these additions to hid_have_special_driver.

>         { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_LCM)},
>         { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_LCM2)},
>         { HID_USB_DEVICE(USB_VENDOR_ID_AVERMEDIA, USB_DEVICE_ID_AVER_FM_MR800) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 9d7a428..a4af9a9 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -137,8 +137,11 @@
>  #define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ISO   0x0256
>  #define USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY   0x030a
>  #define USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY    0x030b
> -#define USB_DEVICE_ID_APPLE_ATV_IRCONTROL      0x8241

not sure we should change this define to an undocumented one.

> +#define USB_DEVICE_ID_APPLE_IRCONTROL  0x8240
> +#define USB_DEVICE_ID_APPLE_IRCONTROL2 0x1440
> +#define USB_DEVICE_ID_APPLE_IRCONTROL3 0x8241
>  #define USB_DEVICE_ID_APPLE_IRCONTROL4 0x8242
> +#define USB_DEVICE_ID_APPLE_IRCONTROL5 0x8243
>
>  #define USB_VENDOR_ID_ASUS             0x0486
>  #define USB_DEVICE_ID_ASUS_T91MT       0x0185
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 2a1647e..c1890c3 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -321,6 +321,19 @@ config INPUT_KXTJ9_POLLED_MODE
>         help
>           Say Y here if you need accelerometer to work in polling mode.
>
> +config INPUT_APPLEIR
> +       tristate "Apple infrared receiver (built in)"
> +       depends on USB_ARCH_HAS_HCD
> +       select USB
> +       help
> +         Say Y here if you want to use a Apple infrared remote control. All
> +         the Apple computers from 2005 onwards include such a port, except
> +         the unibody Macbook (2009), and Mac Pros. This receiver is also
> +         used in the Apple TV set-top box prior to the 2010 model.
> +
> +         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
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 1f874af..c00d562 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_INPUT_ADXL34X)           += adxl34x.o
>  obj-$(CONFIG_INPUT_ADXL34X_I2C)                += adxl34x-i2c.o
>  obj-$(CONFIG_INPUT_ADXL34X_SPI)                += adxl34x-spi.o
>  obj-$(CONFIG_INPUT_APANEL)             += apanel.o
> +obj-$(CONFIG_INPUT_APPLEIR)            += appleir.o
>  obj-$(CONFIG_INPUT_ATI_REMOTE2)                += ati_remote2.o
>  obj-$(CONFIG_INPUT_ATLAS_BTNS)         += atlas_btns.o
>  obj-$(CONFIG_INPUT_BFIN_ROTARY)                += bfin_rotary.o
> diff --git a/drivers/input/misc/appleir.c b/drivers/input/misc/appleir.c
> new file mode 100644
> index 0000000..c6ca58c
> --- /dev/null
> +++ b/drivers/input/misc/appleir.c
> @@ -0,0 +1,527 @@
> +/*
> + * 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@xxxxxxx>
> + *
> + * Copyright (C) 2006 James McKenzie
> + * Copyright (C) 2008 Greg Kroah-Hartman <greg@xxxxxxxxx>
> + * Copyright (C) 2008 Novell Inc.
> + * Copyright (C) 2010, 2012 Bastien Nocera <hadess@xxxxxxxxxx>
> + *
> + * 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/usb/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 "Apple infrared receiver driver"
> +#define DRIVER_LICENSE "GPL"

No need to create these macros.

> +
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_LICENSE(DRIVER_LICENSE);
> +
> +#define USB_VENDOR_ID_APPLE                    0x05ac
> +#define USB_DEVICE_ID_APPLE_IRCONTROL          0x8240
> +#define USB_DEVICE_ID_APPLE_IRCONTROL2         0x1440
> +#define USB_DEVICE_ID_APPLE_IRCONTROL3         0x8241
> +#define USB_DEVICE_ID_APPLE_IRCONTROL4         0x8242
> +#define USB_DEVICE_ID_APPLE_IRCONTROL5         0x8243

these definitions are already in drivers/hid/hid-ids.h, so they can be
skipped in the hid version.

> +
> +#define URB_SIZE       32
> +
> +#define MAX_KEYS       9
> +#define MAX_KEYS_MASK  (MAX_KEYS - 1)

can be skipped too.

> +
> +#define dbginfo(dev, format, arg...) do { if (debug) dev_info(dev , format , ## arg); } while (0)
> +
> +static int debug;
> +module_param(debug, int, 0644);
> +MODULE_PARM_DESC(debug, "Enable extra debug messages and information");
> +
> +/* 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 */
> +
> +/* Alexandre Karpenko reports the following responses for Device ID 0x8242 */
> +/* 25 87 ee 47 0b      +  */
> +/* 25 87 ee 47 0d      -  */
> +/* 25 87 ee 47 08      << */
> +/* 25 87 ee 47 07      >> */
> +/* 25 87 ee 47 04      >" */
> +/* 25 87 ee 47 02      menu */
> +/* 26 87 ee 47 **      for key repeat (** is the code of the key being held) */
> +
> +/* Bastien Nocera's "new" remote */
> +/* 25 87 ee 91 5f      followed by
> + * 25 87 ee 91 05      gives you >"
> + *
> + * 25 87 ee 91 5c      followed by
> + * 25 87 ee 91 05      gives you the middle button */
> +
> +static const unsigned short appleir_key_table[] = {
> +       KEY_RESERVED,
> +       KEY_MENU,
> +       KEY_PLAYPAUSE,
> +       KEY_FORWARD,
> +       KEY_BACK,
> +       KEY_VOLUMEUP,
> +       KEY_VOLUMEDOWN,
> +       KEY_ENTER,
> +       KEY_RESERVED,
> +};
> +
> +struct appleir {
> +       struct input_dev *input_dev;
> +       unsigned short keymap[ARRAY_SIZE(appleir_key_table)];

why this keymap is embedded in the struct? It's basically just a copy
of appleir_key_table and it's not modified anytime.

> +       u8 *data;
> +       dma_addr_t dma_buf;
> +       struct usb_device *usbdev;
> +       unsigned int flags;
> +       struct urb *urb;
> +       struct timer_list key_up_timer;

all these usb stuff can be skipped with hid.

> +       int current_key;
> +       int prev_key_idx;
> +       char phys[32];

same for phys

> +};
> +
> +static DEFINE_MUTEX(appleir_mutex);

no need to maintain a mutex with hid

> +
> +enum {
> +       APPLEIR_OPENED = 0x1,
> +       APPLEIR_SUSPENDED = 0x2,
> +};
> +
> +static struct usb_device_id appleir_ids[] = {
> +       { USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL) },
> +       { USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL2) },
> +       { USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL3) },
> +       { USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4) },
> +       { USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL5) },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(usb, appleir_ids);
> +
> +static void dump_packet(struct appleir *appleir, char *msg, u8 *data, int len)
> +{
> +       int i;
> +
> +       printk(KERN_ERR "appleir: %s (%d bytes)", msg, len);

Should be KERN_INFO when used with if (debug)

> +
> +       for (i = 0; i < len; ++i)
> +               printk(" %02x", data[i]);
> +       printk(" (should be command %d)\n", (data[4] >> 1) & MAX_KEYS_MASK);

Since 3.6, you can use syntax like dev_XXX(&dev, "%*ph\n", n, buf);

So this function can be skipped entirely as you can use %*ph with dbginfo.

> +}
> +
> +static int get_key(int data)
> +{
> +       switch (data) {
> +       case 0x02:
> +       case 0x03:
> +               /* menu */
> +               return 1;
> +       case 0x04:
> +       case 0x05:
> +               /* >" */
> +               return 2;
> +       case 0x06:
> +       case 0x07:
> +               /* >> */
> +               return 3;
> +       case 0x08:
> +       case 0x09:
> +               /* << */
> +               return 4;
> +       case 0x0a:
> +       case 0x0b:
> +               /* + */
> +               return 5;
> +       case 0x0c:
> +       case 0x0d:
> +               /* - */
> +               return 6;
> +       case 0x5c:
> +               /* Middle button, on newer remotes,
> +                * part of a 2 packet-command */
> +               return -7;
> +       default:
> +               return -1;
> +       }
> +}
> +
> +static void key_up(struct appleir *appleir, int key)
> +{
> +       dbginfo(&appleir->input_dev->dev, "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)
> +{
> +       dbginfo(&appleir->input_dev->dev, "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, };
> +       static const u8 flatbattery[] = { 0x25, 0x87, 0xe0 };
> +
> +       if (debug)
> +               dump_packet(appleir, "received", data, len);

use dginfo directly

> +
> +       if (len != 5)
> +               return;
> +
> +       if (!memcmp(data, keydown, sizeof(keydown))) {
> +               int index;
> +
> +               /* 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);
> +
> +               /* Handle dual packet commands */
> +               if (appleir->prev_key_idx > 0)
> +                       index = appleir->prev_key_idx;
> +               else
> +                       index = get_key(data[4]);
> +
> +               if (index > 0) {
> +                       appleir->current_key = appleir->keymap[index];
> +
> +                       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);
> +                       appleir->prev_key_idx = 0;
> +                       return;
> +               } else if (index == -7) {
> +                       /* Remember key for next packet */
> +                       appleir->prev_key_idx = 0 - index;
> +                       return;
> +               }
> +       }
> +
> +       appleir->prev_key_idx = 0;
> +
> +       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);
> +}

Then, the usb part of the driver can be skipped.

> +
> +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 */
> +               dbginfo(&appleir->input_dev->dev,
> +                       "%s - urb shutting down with status: %d",
> +                       __func__, urb->status);
> +               return;
> +       default:
> +               dbginfo(&appleir->input_dev->dev,
> +                       "%s - nonzero urb status received: %d",
> +                       __func__, urb->status);
> +       }
> +
> +       retval = usb_submit_urb(urb, GFP_ATOMIC);
> +       if (retval)
> +               dev_err(&appleir->input_dev->dev,
> +                       "%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);
> +       struct usb_interface *intf = usb_ifnum_to_if(appleir->usbdev, 0);
> +       int r;
> +
> +       r = usb_autopm_get_interface(intf);
> +       if (r) {
> +               dev_err(&intf->dev,
> +                       "%s(): usb_autopm_get_interface() = %d\n", __func__, r);
> +               return r;
> +       }
> +
> +       mutex_lock(&appleir_mutex);
> +
> +       if (usb_submit_urb(appleir->urb, GFP_ATOMIC)) {
> +               r = -EIO;
> +               goto fail;
> +       }
> +
> +       appleir->flags |= APPLEIR_OPENED;
> +
> +       mutex_unlock(&appleir_mutex);
> +
> +       usb_autopm_put_interface(intf);
> +
> +       return 0;
> +fail:
> +       mutex_unlock(&appleir_mutex);
> +       usb_autopm_put_interface(intf);
> +       return r;
> +}
> +
> +static void appleir_close(struct input_dev *dev)
> +{
> +       struct appleir *appleir = input_get_drvdata(dev);
> +
> +       mutex_lock(&appleir_mutex);
> +
> +       if (!(appleir->flags & APPLEIR_SUSPENDED)) {
> +               usb_kill_urb(appleir->urb);
> +               del_timer_sync(&appleir->key_up_timer);
> +       }
> +
> +       appleir->flags &= ~APPLEIR_OPENED;
> +
> +       mutex_unlock(&appleir_mutex);
> +}
> +
> +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 allocfail;
> +
> +       appleir->data = usb_alloc_coherent(dev, URB_SIZE, GFP_KERNEL,
> +                                        &appleir->dma_buf);
> +       if (!appleir->data)
> +               goto usbfail;
> +
> +       appleir->urb = usb_alloc_urb(0, GFP_KERNEL);
> +       if (!appleir->urb)
> +               goto urbfail;
> +
> +       appleir->usbdev = dev;
> +
> +       input_dev = input_allocate_device();
> +       if (!input_dev)
> +               goto inputfail;
> +
> +       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 Infrared Remote Controller";
> +       input_dev->phys = appleir->phys;
> +       usb_to_input_id(dev, &input_dev->id);
> +       input_dev->dev.parent = &intf->dev;
> +       input_dev->keycode = appleir->keymap;
> +       input_dev->keycodesize = sizeof(unsigned short);
> +       input_dev->keycodemax = ARRAY_SIZE(appleir->keymap);
> +
> +       input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
> +
> +       memcpy(appleir->keymap, appleir_key_table, sizeof(appleir->keymap));
> +       for (i = 0; i < ARRAY_SIZE(appleir_key_table); i++)
> +               set_bit(appleir->keymap[i], input_dev->keybit);
> +       clear_bit(KEY_RESERVED, input_dev->keybit);
> +
> +       input_set_drvdata(input_dev, appleir);
> +       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;
> +
> +       setup_timer(&appleir->key_up_timer,
> +                   key_up_tick, (unsigned long) appleir);
> +
> +       retval = input_register_device(appleir->input_dev);
> +       if (retval)
> +               goto inputfail;
> +
> +       usb_set_intfdata(intf, appleir);
> +
> +       return 0;
> +
> +inputfail:

it seems that inputfail should not call input_free_device.

> +       input_free_device(appleir->input_dev);
> +
> +urbfail:

urbfail should not free the urb, it has failed to allocate

> +       usb_free_urb(appleir->urb);
> +
> +usbfail:

ditto

> +       usb_free_coherent(dev, URB_SIZE, appleir->data,
> +                       appleir->dma_buf);
> +
> +allocfail:

this is called when appleir == NULL, so no need to free it.
Apparently, all these references should be move from 1 line :)

> +       kfree(appleir);
> +
> +       return retval;
> +}
> +
> +static void appleir_disconnect(struct usb_interface *intf)
> +{
> +       struct appleir *appleir = usb_get_intfdata(intf);
> +
> +       usb_set_intfdata(intf, NULL);
> +       input_unregister_device(appleir->input_dev);
> +       usb_free_urb(appleir->urb);
> +       usb_free_coherent(interface_to_usbdev(intf), URB_SIZE,
> +                       appleir->data, appleir->dma_buf);
> +       kfree(appleir);
> +}
> +
> +static int appleir_suspend(struct usb_interface *interface,
> +                          pm_message_t message)

I don't think suspend and resume should be set with the hid driver.

> +{
> +       struct appleir *appleir = usb_get_intfdata(interface);
> +
> +       mutex_lock(&appleir_mutex);
> +       if (appleir->flags & APPLEIR_OPENED)
> +               usb_kill_urb(appleir->urb);
> +
> +       appleir->flags |= APPLEIR_SUSPENDED;
> +
> +       mutex_unlock(&appleir_mutex);
> +
> +       return 0;
> +}
> +
> +static int appleir_resume(struct usb_interface *interface)
> +{
> +       struct appleir *appleir;
> +       int r = 0;
> +
> +       appleir = usb_get_intfdata(interface);
> +
> +       mutex_lock(&appleir_mutex);
> +       if (appleir->flags & APPLEIR_OPENED) {
> +               struct usb_endpoint_descriptor *endpoint;
> +               unsigned int pipe;
> +
> +               endpoint = &interface->cur_altsetting->endpoint[0].desc;
> +               pipe = usb_rcvintpipe(appleir->usbdev,
> +                                     endpoint->bEndpointAddress);
> +               usb_fill_int_urb(appleir->urb, appleir->usbdev,
> +                                pipe,
> +                                appleir->data, 8,
> +                                appleir_urb, appleir, endpoint->bInterval);
> +               appleir->urb->transfer_dma = appleir->dma_buf;
> +               appleir->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> +               /* And reset the USB device */
> +               if (usb_submit_urb(appleir->urb, GFP_ATOMIC))
> +                       r = -EIO;
> +       }
> +
> +       appleir->flags &= ~APPLEIR_SUSPENDED;
> +
> +       mutex_unlock(&appleir_mutex);
> +
> +       return r;
> +}
> +
> +static struct usb_driver appleir_driver = {
> +       .name                 = "appleir",
> +       .probe                = appleir_probe,
> +       .disconnect           = appleir_disconnect,
> +       .suspend              = appleir_suspend,
> +       .resume               = appleir_resume,
> +       .reset_resume         = appleir_resume,
> +       .id_table             = appleir_ids,
> +};
> +
> +static int __init appleir_init(void)
> +{
> +       return usb_register(&appleir_driver);
> +}
> +
> +static void __exit appleir_exit(void)
> +{
> +       usb_deregister(&appleir_driver);
> +}
> +
> +module_init(appleir_init);
> +module_exit(appleir_exit);
> --
> 1.8.0
>
>

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


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux