diff mbox series

[v2] HID: macally: Add support for Macally ikey keyboard

Message ID 20190403031817.14722-1-alexhenrie24@gmail.com (mailing list archive)
State Mainlined
Commit 161f62cd07fde123fd52bf6d5b6fd6513cca968e
Delegated to: Jiri Kosina
Headers show
Series [v2] HID: macally: Add support for Macally ikey keyboard | expand

Commit Message

Alex Henrie April 3, 2019, 3:18 a.m. UTC
This enables the power and equals keys on the Macally ikey keyboard.

Based on the Cougar gaming keyboard HID driver, which uses the same
vendor ID.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/hid/Kconfig       | 10 +++++++++
 drivers/hid/Makefile      |  1 +
 drivers/hid/hid-ids.h     |  1 +
 drivers/hid/hid-macally.c | 45 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 57 insertions(+)
 create mode 100644 drivers/hid/hid-macally.c

Comments

Hans de Goede April 3, 2019, 7:06 a.m. UTC | #1
Hi,
On 03-04-19 05:18, Alex Henrie wrote:
> This enables the power and equals keys on the Macally ikey keyboard.
> 
> Based on the Cougar gaming keyboard HID driver, which uses the same
> vendor ID.
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

The driver looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

I do have something to discuss about this with the HID subsys maintainers,
see comment inline.

> ---
>   drivers/hid/Kconfig       | 10 +++++++++
>   drivers/hid/Makefile      |  1 +
>   drivers/hid/hid-ids.h     |  1 +
>   drivers/hid/hid-macally.c | 45 +++++++++++++++++++++++++++++++++++++++
>   4 files changed, 57 insertions(+)
>   create mode 100644 drivers/hid/hid-macally.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 6ca8d322b487..aef4a2a690e1 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -230,6 +230,16 @@ config HID_COUGAR
>   	Supported devices:
>   	- Cougar 500k Gaming Keyboard
>   
> +config HID_MACALLY
> +	tristate "Macally devices"
> +	depends on HID
> +	help
> +	Support for Macally devices that are not fully compliant with the
> +	HID standard.
> +
> +	supported devices:
> +	- Macally ikey keyboard
> +
>   config HID_PRODIKEYS
>   	tristate "Prodikeys PC-MIDI Keyboard support"
>   	depends on HID && SND
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 170163b41303..44b9caea46a7 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_HID_LENOVO)	+= hid-lenovo.o
>   obj-$(CONFIG_HID_LOGITECH)	+= hid-logitech.o
>   obj-$(CONFIG_HID_LOGITECH_DJ)	+= hid-logitech-dj.o
>   obj-$(CONFIG_HID_LOGITECH_HIDPP)	+= hid-logitech-hidpp.o
> +obj-$(CONFIG_HID_MACALLY)	+= hid-macally.o
>   obj-$(CONFIG_HID_MAGICMOUSE)	+= hid-magicmouse.o
>   obj-$(CONFIG_HID_MALTRON)	+= hid-maltron.o
>   obj-$(CONFIG_HID_MAYFLASH)	+= hid-mf.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b6d93f4ad037..aacc7534b076 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -1034,6 +1034,7 @@
>   #define USB_DEVICE_ID_SINO_LITE_CONTROLLER	0x3008
>   
>   #define USB_VENDOR_ID_SOLID_YEAR			0x060b
> +#define USB_DEVICE_ID_MACALLY_IKEY_KEYBOARD		0x0001
>   #define USB_DEVICE_ID_COUGAR_500K_GAMING_KEYBOARD	0x500a
>   #define USB_DEVICE_ID_COUGAR_700K_GAMING_KEYBOARD	0x700a
>   
> diff --git a/drivers/hid/hid-macally.c b/drivers/hid/hid-macally.c
> new file mode 100644
> index 000000000000..9a4fc7dffb14
> --- /dev/null
> +++ b/drivers/hid/hid-macally.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  HID driver for quirky Macally devices
> + *
> + *  Copyright (c) 2019 Alex Henrie <alexhenrie24@gmail.com>
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +MODULE_AUTHOR("Alex Henrie <alexhenrie24@gmail.com>");
> +MODULE_DESCRIPTION("Macally devices");
> +MODULE_LICENSE("GPL");
> +
> +/*
> + * The Macally ikey keyboard says that its logical and usage maximums are both
> + * 101, but the power key is 102 and the equals key is 103
> + */

I was wondering if this is something which more keyboards suffer from, a quick
grep finds a couple which do exactly the same thing (but with a different new
maximum value:

hid-apple.c:
static __u8 *apple_report_fixup(struct hid_device *hdev, __u8 *rdesc,
                 unsigned int *rsize)
{
         struct apple_sc *asc = hid_get_drvdata(hdev);

         if ((asc->quirks & APPLE_RDESC_JIS) && *rsize >= 60 &&
                         rdesc[53] == 0x65 && rdesc[59] == 0x65) {
                 hid_info(hdev,
                          "fixing up MacBook JIS keyboard report descriptor\n");
                 rdesc[53] = rdesc[59] = 0xe7;
         }
         return rdesc;
}

hid-nti.c:
static __u8 *nti_usbsun_report_fixup(struct hid_device *hdev, __u8 *rdesc,
                 unsigned int *rsize)
{
         if (*rsize >= 60 && rdesc[53] == 0x65 && rdesc[59] == 0x65) {
                 hid_info(hdev, "fixing up NTI USB-SUN keyboard adapter report de
                 rdesc[53] = rdesc[59] = 0xe7;
         }
         return rdesc;
}

And a few wich are close, but slightly different:

hid-asus.c:
static __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
                 unsigned int *rsize)
{
         struct asus_drvdata *drvdata = hid_get_drvdata(hdev);

         if (drvdata->quirks & QUIRK_FIX_NOTEBOOK_REPORT &&
                         *rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x65)
                 hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
                 rdesc[55] = 0xdd;
         }
	... (other quirks for other devices)
}

hid-aureal.c:
static __u8 *aureal_report_fixup(struct hid_device *hdev, __u8 *rdesc,
                 unsigned int *rsize)
{
         if (*rsize >= 54 && rdesc[52] == 0x25 && rdesc[53] == 0x01) {
                 dev_info(&hdev->dev, "fixing Aureal Cy se W-01RN USB_V3.1 report
                 rdesc[53] = 0x65;
         }
         return rdesc;
}

hid-ortek.c:
static __u8 *ortek_report_fixup(struct hid_device *hdev, __u8 *rdesc,
                 unsigned int *rsize)
{
         if (*rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x01) {
                 hid_info(hdev, "Fixing up logical maximum in report descriptor (
                 rdesc[55] = 0x92;
         } else if (*rsize >= 54 && rdesc[52] == 0x25 && rdesc[53] == 0x01) {
                 hid_info(hdev, "Fixing up logical maximum in report descriptor (
                 rdesc[53] = 0x65;
         }
         return rdesc;
}

So I'm wondering if we cannot come up with some generic helper for this,
which drivers could then call from their report-fixup; or, even better
maybe have a HID_QUIRK_FIX_KEYBOARD_MAXIMUM which causes the generic code
to bump the maximum to 0xe7 (*), then the hid-nti, hid-macally and hid-ortek
drivers could be dropped and replaced with a single line entry in hid-quirks.c

Regards,

Hans

*) Or probably 0xdf, 0xe0 - 0xe7 are the ctrl/shift/alt/etc. modifiers.





> +static __u8 *macally_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> +				 unsigned int *rsize)
> +{
> +	if (*rsize >= 60 && rdesc[53] == 0x65 && rdesc[59] == 0x65) {
> +		hid_info(hdev,
> +			"fixing up Macally ikey keyboard report descriptor\n");
> +		rdesc[53] = rdesc[59] = 0x67;
> +	}
> +	return rdesc;
> +}
> +
> +static struct hid_device_id macally_id_table[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_SOLID_YEAR,
> +			 USB_DEVICE_ID_MACALLY_IKEY_KEYBOARD) },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(hid, macally_id_table);
> +
> +static struct hid_driver macally_driver = {
> +	.name			= "macally",
> +	.id_table		= macally_id_table,
> +	.report_fixup		= macally_report_fixup,
> +};
> +
> +module_hid_driver(macally_driver);
>
Benjamin Tissoires April 3, 2019, 3:12 p.m. UTC | #2
Hi,

On Wed, Apr 3, 2019 at 9:06 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
> On 03-04-19 05:18, Alex Henrie wrote:
> > This enables the power and equals keys on the Macally ikey keyboard.
> >
> > Based on the Cougar gaming keyboard HID driver, which uses the same
> > vendor ID.
> >
> > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
>
> The driver looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Thanks, I should be all set to take this for 5.2 then. I'll let you
know when it is pushed upstream.

>
> I do have something to discuss about this with the HID subsys maintainers,
> see comment inline.
>

/me ducks away....

> > ---
> >   drivers/hid/Kconfig       | 10 +++++++++
> >   drivers/hid/Makefile      |  1 +
> >   drivers/hid/hid-ids.h     |  1 +
> >   drivers/hid/hid-macally.c | 45 +++++++++++++++++++++++++++++++++++++++
> >   4 files changed, 57 insertions(+)
> >   create mode 100644 drivers/hid/hid-macally.c
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 6ca8d322b487..aef4a2a690e1 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -230,6 +230,16 @@ config HID_COUGAR
> >       Supported devices:
> >       - Cougar 500k Gaming Keyboard
> >
> > +config HID_MACALLY
> > +     tristate "Macally devices"
> > +     depends on HID
> > +     help
> > +     Support for Macally devices that are not fully compliant with the
> > +     HID standard.
> > +
> > +     supported devices:
> > +     - Macally ikey keyboard
> > +
> >   config HID_PRODIKEYS
> >       tristate "Prodikeys PC-MIDI Keyboard support"
> >       depends on HID && SND
> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > index 170163b41303..44b9caea46a7 100644
> > --- a/drivers/hid/Makefile
> > +++ b/drivers/hid/Makefile
> > @@ -65,6 +65,7 @@ obj-$(CONFIG_HID_LENOVO)    += hid-lenovo.o
> >   obj-$(CONFIG_HID_LOGITECH)  += hid-logitech.o
> >   obj-$(CONFIG_HID_LOGITECH_DJ)       += hid-logitech-dj.o
> >   obj-$(CONFIG_HID_LOGITECH_HIDPP)    += hid-logitech-hidpp.o
> > +obj-$(CONFIG_HID_MACALLY)    += hid-macally.o
> >   obj-$(CONFIG_HID_MAGICMOUSE)        += hid-magicmouse.o
> >   obj-$(CONFIG_HID_MALTRON)   += hid-maltron.o
> >   obj-$(CONFIG_HID_MAYFLASH)  += hid-mf.o
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index b6d93f4ad037..aacc7534b076 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -1034,6 +1034,7 @@
> >   #define USB_DEVICE_ID_SINO_LITE_CONTROLLER  0x3008
> >
> >   #define USB_VENDOR_ID_SOLID_YEAR                    0x060b
> > +#define USB_DEVICE_ID_MACALLY_IKEY_KEYBOARD          0x0001
> >   #define USB_DEVICE_ID_COUGAR_500K_GAMING_KEYBOARD   0x500a
> >   #define USB_DEVICE_ID_COUGAR_700K_GAMING_KEYBOARD   0x700a
> >
> > diff --git a/drivers/hid/hid-macally.c b/drivers/hid/hid-macally.c
> > new file mode 100644
> > index 000000000000..9a4fc7dffb14
> > --- /dev/null
> > +++ b/drivers/hid/hid-macally.c
> > @@ -0,0 +1,45 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + *  HID driver for quirky Macally devices
> > + *
> > + *  Copyright (c) 2019 Alex Henrie <alexhenrie24@gmail.com>
> > + */
> > +
> > +#include <linux/hid.h>
> > +#include <linux/module.h>
> > +
> > +#include "hid-ids.h"
> > +
> > +MODULE_AUTHOR("Alex Henrie <alexhenrie24@gmail.com>");
> > +MODULE_DESCRIPTION("Macally devices");
> > +MODULE_LICENSE("GPL");
> > +
> > +/*
> > + * The Macally ikey keyboard says that its logical and usage maximums are both
> > + * 101, but the power key is 102 and the equals key is 103
> > + */
>
> I was wondering if this is something which more keyboards suffer from, a quick
> grep finds a couple which do exactly the same thing (but with a different new
> maximum value:
>
> hid-apple.c:
> static __u8 *apple_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>                  unsigned int *rsize)
> {
>          struct apple_sc *asc = hid_get_drvdata(hdev);
>
>          if ((asc->quirks & APPLE_RDESC_JIS) && *rsize >= 60 &&
>                          rdesc[53] == 0x65 && rdesc[59] == 0x65) {
>                  hid_info(hdev,
>                           "fixing up MacBook JIS keyboard report descriptor\n");
>                  rdesc[53] = rdesc[59] = 0xe7;
>          }
>          return rdesc;
> }
>
> hid-nti.c:
> static __u8 *nti_usbsun_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>                  unsigned int *rsize)
> {
>          if (*rsize >= 60 && rdesc[53] == 0x65 && rdesc[59] == 0x65) {
>                  hid_info(hdev, "fixing up NTI USB-SUN keyboard adapter report de
>                  rdesc[53] = rdesc[59] = 0xe7;
>          }
>          return rdesc;
> }
>
> And a few wich are close, but slightly different:
>
> hid-asus.c:
> static __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>                  unsigned int *rsize)
> {
>          struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>
>          if (drvdata->quirks & QUIRK_FIX_NOTEBOOK_REPORT &&
>                          *rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x65)
>                  hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
>                  rdesc[55] = 0xdd;
>          }
>         ... (other quirks for other devices)
> }
>
> hid-aureal.c:
> static __u8 *aureal_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>                  unsigned int *rsize)
> {
>          if (*rsize >= 54 && rdesc[52] == 0x25 && rdesc[53] == 0x01) {
>                  dev_info(&hdev->dev, "fixing Aureal Cy se W-01RN USB_V3.1 report
>                  rdesc[53] = 0x65;
>          }
>          return rdesc;
> }
>
> hid-ortek.c:
> static __u8 *ortek_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>                  unsigned int *rsize)
> {
>          if (*rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x01) {
>                  hid_info(hdev, "Fixing up logical maximum in report descriptor (
>                  rdesc[55] = 0x92;
>          } else if (*rsize >= 54 && rdesc[52] == 0x25 && rdesc[53] == 0x01) {
>                  hid_info(hdev, "Fixing up logical maximum in report descriptor (
>                  rdesc[53] = 0x65;
>          }
>          return rdesc;
> }
>
> So I'm wondering if we cannot come up with some generic helper for this,
> which drivers could then call from their report-fixup; or, even better
> maybe have a HID_QUIRK_FIX_KEYBOARD_MAXIMUM which causes the generic code
> to bump the maximum to 0xe7 (*), then the hid-nti, hid-macally and hid-ortek
> drivers could be dropped and replaced with a single line entry in hid-quirks.c

I am not a big fan of the idea. Having a specific quirk is not going
to help much IMO because it will be quite hard to grasp what it does.
Right now adding a new module doesn't cost much, especially since
hid-generic is nice enough to unbind itself.

With Jiri, we talked (a lot) about having those report descriptors
overrides provided by userspace. I know I already mentioned this to
you a while ago (and more recently IIRC), but one idea could be to
have the override as a firmware file that would get loaded by udev
through request_firmware() (if I remember correctly the API).

But I am also wondering if we could not use eBPF to actually fix those
HID devices programmatically, which mean we could externalize those
fixes without actually needing the full report descriptor.

Anyway, until we have a generic and good enough solution, I'd rather
we keep accepting those report fixups in the kernel.

Cheers,
Benjamin

>
> Regards,
>
> Hans
>
> *) Or probably 0xdf, 0xe0 - 0xe7 are the ctrl/shift/alt/etc. modifiers.
>
>
>
>
>
> > +static __u8 *macally_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> > +                              unsigned int *rsize)
> > +{
> > +     if (*rsize >= 60 && rdesc[53] == 0x65 && rdesc[59] == 0x65) {
> > +             hid_info(hdev,
> > +                     "fixing up Macally ikey keyboard report descriptor\n");
> > +             rdesc[53] = rdesc[59] = 0x67;
> > +     }
> > +     return rdesc;
> > +}
> > +
> > +static struct hid_device_id macally_id_table[] = {
> > +     { HID_USB_DEVICE(USB_VENDOR_ID_SOLID_YEAR,
> > +                      USB_DEVICE_ID_MACALLY_IKEY_KEYBOARD) },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(hid, macally_id_table);
> > +
> > +static struct hid_driver macally_driver = {
> > +     .name                   = "macally",
> > +     .id_table               = macally_id_table,
> > +     .report_fixup           = macally_report_fixup,
> > +};
> > +
> > +module_hid_driver(macally_driver);
> >
Hans de Goede April 3, 2019, 3:14 p.m. UTC | #3
Hi,

On 03-04-19 17:12, Benjamin Tissoires wrote:
> Hi,
> 
> On Wed, Apr 3, 2019 at 9:06 AM Hans de Goede <hdegoede@redhat.com> wrote:

<snip>

>> I was wondering if this is something which more keyboards suffer from, a quick
>> grep finds a couple which do exactly the same thing (but with a different new
>> maximum value:
>>
>> hid-apple.c:
>> static __u8 *apple_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>>                   unsigned int *rsize)
>> {
>>           struct apple_sc *asc = hid_get_drvdata(hdev);
>>
>>           if ((asc->quirks & APPLE_RDESC_JIS) && *rsize >= 60 &&
>>                           rdesc[53] == 0x65 && rdesc[59] == 0x65) {
>>                   hid_info(hdev,
>>                            "fixing up MacBook JIS keyboard report descriptor\n");
>>                   rdesc[53] = rdesc[59] = 0xe7;
>>           }
>>           return rdesc;
>> }
>>
>> hid-nti.c:
>> static __u8 *nti_usbsun_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>>                   unsigned int *rsize)
>> {
>>           if (*rsize >= 60 && rdesc[53] == 0x65 && rdesc[59] == 0x65) {
>>                   hid_info(hdev, "fixing up NTI USB-SUN keyboard adapter report de
>>                   rdesc[53] = rdesc[59] = 0xe7;
>>           }
>>           return rdesc;
>> }
>>
>> And a few wich are close, but slightly different:
>>
>> hid-asus.c:
>> static __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>>                   unsigned int *rsize)
>> {
>>           struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>>
>>           if (drvdata->quirks & QUIRK_FIX_NOTEBOOK_REPORT &&
>>                           *rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x65)
>>                   hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
>>                   rdesc[55] = 0xdd;
>>           }
>>          ... (other quirks for other devices)
>> }
>>
>> hid-aureal.c:
>> static __u8 *aureal_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>>                   unsigned int *rsize)
>> {
>>           if (*rsize >= 54 && rdesc[52] == 0x25 && rdesc[53] == 0x01) {
>>                   dev_info(&hdev->dev, "fixing Aureal Cy se W-01RN USB_V3.1 report
>>                   rdesc[53] = 0x65;
>>           }
>>           return rdesc;
>> }
>>
>> hid-ortek.c:
>> static __u8 *ortek_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>>                   unsigned int *rsize)
>> {
>>           if (*rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x01) {
>>                   hid_info(hdev, "Fixing up logical maximum in report descriptor (
>>                   rdesc[55] = 0x92;
>>           } else if (*rsize >= 54 && rdesc[52] == 0x25 && rdesc[53] == 0x01) {
>>                   hid_info(hdev, "Fixing up logical maximum in report descriptor (
>>                   rdesc[53] = 0x65;
>>           }
>>           return rdesc;
>> }
>>
>> So I'm wondering if we cannot come up with some generic helper for this,
>> which drivers could then call from their report-fixup; or, even better
>> maybe have a HID_QUIRK_FIX_KEYBOARD_MAXIMUM which causes the generic code
>> to bump the maximum to 0xe7 (*), then the hid-nti, hid-macally and hid-ortek
>> drivers could be dropped and replaced with a single line entry in hid-quirks.c
> 
> I am not a big fan of the idea. Having a specific quirk is not going
> to help much IMO because it will be quite hard to grasp what it does.
> Right now adding a new module doesn't cost much, especially since
> hid-generic is nice enough to unbind itself.
> 
> With Jiri, we talked (a lot) about having those report descriptors
> overrides provided by userspace. I know I already mentioned this to
> you a while ago (and more recently IIRC), but one idea could be to
> have the override as a firmware file that would get loaded by udev
> through request_firmware() (if I remember correctly the API).
> 
> But I am also wondering if we could not use eBPF to actually fix those
> HID devices programmatically, which mean we could externalize those
> fixes without actually needing the full report descriptor.
> 
> Anyway, until we have a generic and good enough solution, I'd rather
> we keep accepting those report fixups in the kernel.

Ok, fair enough.

Regards,

Hans
Benjamin Tissoires April 3, 2019, 4:19 p.m. UTC | #4
On Wed, Apr 3, 2019 at 5:18 AM Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> This enables the power and equals keys on the Macally ikey keyboard.
>
> Based on the Cougar gaming keyboard HID driver, which uses the same
> vendor ID.
>
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---

Applied to for-5.2/macally

Thanks!

Cheers,
Benjamin

>  drivers/hid/Kconfig       | 10 +++++++++
>  drivers/hid/Makefile      |  1 +
>  drivers/hid/hid-ids.h     |  1 +
>  drivers/hid/hid-macally.c | 45 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 57 insertions(+)
>  create mode 100644 drivers/hid/hid-macally.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 6ca8d322b487..aef4a2a690e1 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -230,6 +230,16 @@ config HID_COUGAR
>         Supported devices:
>         - Cougar 500k Gaming Keyboard
>
> +config HID_MACALLY
> +       tristate "Macally devices"
> +       depends on HID
> +       help
> +       Support for Macally devices that are not fully compliant with the
> +       HID standard.
> +
> +       supported devices:
> +       - Macally ikey keyboard
> +
>  config HID_PRODIKEYS
>         tristate "Prodikeys PC-MIDI Keyboard support"
>         depends on HID && SND
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 170163b41303..44b9caea46a7 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_HID_LENOVO)      += hid-lenovo.o
>  obj-$(CONFIG_HID_LOGITECH)     += hid-logitech.o
>  obj-$(CONFIG_HID_LOGITECH_DJ)  += hid-logitech-dj.o
>  obj-$(CONFIG_HID_LOGITECH_HIDPP)       += hid-logitech-hidpp.o
> +obj-$(CONFIG_HID_MACALLY)      += hid-macally.o
>  obj-$(CONFIG_HID_MAGICMOUSE)   += hid-magicmouse.o
>  obj-$(CONFIG_HID_MALTRON)      += hid-maltron.o
>  obj-$(CONFIG_HID_MAYFLASH)     += hid-mf.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b6d93f4ad037..aacc7534b076 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -1034,6 +1034,7 @@
>  #define USB_DEVICE_ID_SINO_LITE_CONTROLLER     0x3008
>
>  #define USB_VENDOR_ID_SOLID_YEAR                       0x060b
> +#define USB_DEVICE_ID_MACALLY_IKEY_KEYBOARD            0x0001
>  #define USB_DEVICE_ID_COUGAR_500K_GAMING_KEYBOARD      0x500a
>  #define USB_DEVICE_ID_COUGAR_700K_GAMING_KEYBOARD      0x700a
>
> diff --git a/drivers/hid/hid-macally.c b/drivers/hid/hid-macally.c
> new file mode 100644
> index 000000000000..9a4fc7dffb14
> --- /dev/null
> +++ b/drivers/hid/hid-macally.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  HID driver for quirky Macally devices
> + *
> + *  Copyright (c) 2019 Alex Henrie <alexhenrie24@gmail.com>
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +MODULE_AUTHOR("Alex Henrie <alexhenrie24@gmail.com>");
> +MODULE_DESCRIPTION("Macally devices");
> +MODULE_LICENSE("GPL");
> +
> +/*
> + * The Macally ikey keyboard says that its logical and usage maximums are both
> + * 101, but the power key is 102 and the equals key is 103
> + */
> +static __u8 *macally_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> +                                unsigned int *rsize)
> +{
> +       if (*rsize >= 60 && rdesc[53] == 0x65 && rdesc[59] == 0x65) {
> +               hid_info(hdev,
> +                       "fixing up Macally ikey keyboard report descriptor\n");
> +               rdesc[53] = rdesc[59] = 0x67;
> +       }
> +       return rdesc;
> +}
> +
> +static struct hid_device_id macally_id_table[] = {
> +       { HID_USB_DEVICE(USB_VENDOR_ID_SOLID_YEAR,
> +                        USB_DEVICE_ID_MACALLY_IKEY_KEYBOARD) },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(hid, macally_id_table);
> +
> +static struct hid_driver macally_driver = {
> +       .name                   = "macally",
> +       .id_table               = macally_id_table,
> +       .report_fixup           = macally_report_fixup,
> +};
> +
> +module_hid_driver(macally_driver);
> --
> 2.21.0
>
Alex Henrie April 3, 2019, 4:24 p.m. UTC | #5
On Wed, Apr 3, 2019 at 10:19 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Wed, Apr 3, 2019 at 5:18 AM Alex Henrie <alexhenrie24@gmail.com> wrote:
> >
> > This enables the power and equals keys on the Macally ikey keyboard.
> >
> > Based on the Cougar gaming keyboard HID driver, which uses the same
> > vendor ID.
> >
> > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> > ---
>
> Applied to for-5.2/macally
>
> Thanks!

Thank you!

-Alex
diff mbox series

Patch

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 6ca8d322b487..aef4a2a690e1 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -230,6 +230,16 @@  config HID_COUGAR
 	Supported devices:
 	- Cougar 500k Gaming Keyboard
 
+config HID_MACALLY
+	tristate "Macally devices"
+	depends on HID
+	help
+	Support for Macally devices that are not fully compliant with the
+	HID standard.
+
+	supported devices:
+	- Macally ikey keyboard
+
 config HID_PRODIKEYS
 	tristate "Prodikeys PC-MIDI Keyboard support"
 	depends on HID && SND
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 170163b41303..44b9caea46a7 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -65,6 +65,7 @@  obj-$(CONFIG_HID_LENOVO)	+= hid-lenovo.o
 obj-$(CONFIG_HID_LOGITECH)	+= hid-logitech.o
 obj-$(CONFIG_HID_LOGITECH_DJ)	+= hid-logitech-dj.o
 obj-$(CONFIG_HID_LOGITECH_HIDPP)	+= hid-logitech-hidpp.o
+obj-$(CONFIG_HID_MACALLY)	+= hid-macally.o
 obj-$(CONFIG_HID_MAGICMOUSE)	+= hid-magicmouse.o
 obj-$(CONFIG_HID_MALTRON)	+= hid-maltron.o
 obj-$(CONFIG_HID_MAYFLASH)	+= hid-mf.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b6d93f4ad037..aacc7534b076 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1034,6 +1034,7 @@ 
 #define USB_DEVICE_ID_SINO_LITE_CONTROLLER	0x3008
 
 #define USB_VENDOR_ID_SOLID_YEAR			0x060b
+#define USB_DEVICE_ID_MACALLY_IKEY_KEYBOARD		0x0001
 #define USB_DEVICE_ID_COUGAR_500K_GAMING_KEYBOARD	0x500a
 #define USB_DEVICE_ID_COUGAR_700K_GAMING_KEYBOARD	0x700a
 
diff --git a/drivers/hid/hid-macally.c b/drivers/hid/hid-macally.c
new file mode 100644
index 000000000000..9a4fc7dffb14
--- /dev/null
+++ b/drivers/hid/hid-macally.c
@@ -0,0 +1,45 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  HID driver for quirky Macally devices
+ *
+ *  Copyright (c) 2019 Alex Henrie <alexhenrie24@gmail.com>
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+MODULE_AUTHOR("Alex Henrie <alexhenrie24@gmail.com>");
+MODULE_DESCRIPTION("Macally devices");
+MODULE_LICENSE("GPL");
+
+/*
+ * The Macally ikey keyboard says that its logical and usage maximums are both
+ * 101, but the power key is 102 and the equals key is 103
+ */
+static __u8 *macally_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+				 unsigned int *rsize)
+{
+	if (*rsize >= 60 && rdesc[53] == 0x65 && rdesc[59] == 0x65) {
+		hid_info(hdev,
+			"fixing up Macally ikey keyboard report descriptor\n");
+		rdesc[53] = rdesc[59] = 0x67;
+	}
+	return rdesc;
+}
+
+static struct hid_device_id macally_id_table[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SOLID_YEAR,
+			 USB_DEVICE_ID_MACALLY_IKEY_KEYBOARD) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, macally_id_table);
+
+static struct hid_driver macally_driver = {
+	.name			= "macally",
+	.id_table		= macally_id_table,
+	.report_fixup		= macally_report_fixup,
+};
+
+module_hid_driver(macally_driver);