Re: [PATCH] Input: add appleir USB driver

[Bastien Nocera <hadess@hadess.net>]

On Mon, 2012-11-19 at 16:32 +0100, Benjamin Tissoires wrote:
> Hi Bastien,
> 
> (adding the input and HID maintainers to the recipient list).
> 
> On Thu, Nov 15, 2012 at 7:13 PM, Bastien Nocera <hadess@hadess.net> 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@hadess.net>
> > ---
> >
> > 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!

\o/

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

I don't understand the comment here. The name is an artifact of where
the receiver was first seen and the Apple TV receiver is actually just
another model of this same receiver. So it makes sense to consolidate
below.

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


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

It would be modified if you change the keymap.

Cheers