| Message ID | 20180411094953.12053-1-rombert@apache.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 11 Apr 2018, Robert Munteanu wrote: > Changelog: > > - v2: modifier keys work, some combinations are still troublesome > - v3: style cleanup, rebase on top of 4.14 > - v4: remove most debugging calls, make init info useful for user, > rebased on top of 4.15 > - v5: fix the HID descriptor as suggested by Benjamin Tissoires, > use existing USB vendor id, update comment style, add SPDX > license identifier, rename to hid-redragon, stop registering > two input devices, rebased on top of 4.16 This should contain the text that goes directly into the git commit changelog, therefore should instead contain something like "This adds a news driver for HW XX, that needs a specialized driver bacuse .... and things to specifically point out in implementation are ....". Could you please provide that, so that I can commit it? Thanks.
On Mon, Apr 16, 2018 at 11:49 AM, Jiri Kosina <jikos@kernel.org> wrote: > On Wed, 11 Apr 2018, Robert Munteanu wrote: > >> Changelog: >> >> - v2: modifier keys work, some combinations are still troublesome >> - v3: style cleanup, rebase on top of 4.14 >> - v4: remove most debugging calls, make init info useful for user, >> rebased on top of 4.15 >> - v5: fix the HID descriptor as suggested by Benjamin Tissoires, >> use existing USB vendor id, update comment style, add SPDX >> license identifier, rename to hid-redragon, stop registering >> two input devices, rebased on top of 4.16 > > This should contain the text that goes directly into the git commit > changelog, therefore should instead contain something like "This adds a > news driver for HW XX, that needs a specialized driver bacuse .... and > things to specifically point out in implementation are ....". > > Could you please provide that, so that I can commit it? Jiri, please hold off, I have a few comments while quickly reviewing the patch. Cheers, Benjamin > > Thanks. > > -- > Jiri Kosina > SUSE Labs > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Robert, On Wed, Apr 11, 2018 at 11:49 AM, Robert Munteanu <rombert@apache.org> wrote: > Changelog: > > - v2: modifier keys work, some combinations are still troublesome > - v3: style cleanup, rebase on top of 4.14 > - v4: remove most debugging calls, make init info useful for user, > rebased on top of 4.15 > - v5: fix the HID descriptor as suggested by Benjamin Tissoires, > use existing USB vendor id, update comment style, add SPDX > license identifier, rename to hid-redragon, stop registering > two input devices, rebased on top of 4.16 As Jiri said, please provide a correct commit message. I have a few nitpicks in the driver, v6 should be fine: > > Signed-off-by: Robert Munteanu <rombert@apache.org> > --- > drivers/hid/Kconfig | 7 ++++ > drivers/hid/Makefile | 1 + > drivers/hid/hid-ids.h | 1 + > drivers/hid/hid-quirks.c | 3 ++ > drivers/hid/hid-redragon.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 101 insertions(+) > create mode 100644 drivers/hid/hid-redragon.c > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index 19c499f5623d..1125e4813716 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -560,6 +560,13 @@ config HID_MAYFLASH > Say Y here if you have HJZ Mayflash PS3 game controller adapters > and want to enable force feedback support. > > +config HID_REDRAGON > + tristate "Redragon keyboards" > + depends on HID > + default !EXPERT > + ---help--- > + Support for Redragon keyboards that need fix-ups to work properly. > + > config HID_MICROSOFT > tristate "Microsoft non-fully HID-compliant devices" > depends on HID > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index eb13b9e92d85..a36f3f40ba63 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS) += hid-picolcd_debugfs.o > > obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o > +obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ > hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \ > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index 9454ac134ce2..41a64d0e91f9 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -599,6 +599,7 @@ > #define USB_VENDOR_ID_JESS 0x0c45 > #define USB_DEVICE_ID_JESS_YUREX 0x1010 > #define USB_DEVICE_ID_ASUS_MD_5112 0x5112 > +#define USB_DEVICE_ID_REDRAGON_ASURA 0x760b > > #define USB_VENDOR_ID_JESS2 0x0f30 > #define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111 > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c > index e92b77fa574a..5f1253f1739a 100644 > --- a/drivers/hid/hid-quirks.c > +++ b/drivers/hid/hid-quirks.c > @@ -557,6 +557,9 @@ static const struct hid_device_id hid_have_special_driver[] = { > #if IS_ENABLED(CONFIG_HID_PRODIKEYS) > { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) }, > #endif > +#if IS_ENABLED(CONFIG_HID_REDRAGON) > + { HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA) }, > +#endif Please drop this hunk. v4.16 should work without changing hid_have_special_driver. This way, you will be sure that an initramfs that doesn't include hid-redragon.ko will allo people to type their LUKS password. > #if IS_ENABLED(CONFIG_HID_RETRODE) > { HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY, USB_DEVICE_ID_RETRODE2) }, > #endif > diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid-redragon.c > new file mode 100644 > index 000000000000..ff98a5dbb8e2 > --- /dev/null > +++ b/drivers/hid/hid-redragon.c > @@ -0,0 +1,89 @@ > +/* > + * HID driver for Redragon keyboards > + * > + * Copyright (c) 2017 Robert Munteanu > + * SPDX-License-Identifier: GPL-2.0 > + */ > + > +/* > + * 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; either version 2 of the License, or (at your option) > + * any later version. > + */ > + > +#include <linux/device.h> > +#include <linux/input.h> you probably don't need input.h > +#include <linux/hid.h> > +#include <linux/module.h> > +#include <linux/log2.h> log2.h? I am not sure you need it either > +#include <linux/input-event-codes.h> probably drop this one too. > + > +#include "hid-ids.h" > + > + > +/* > + * The Redragon Asura keyboard sends an incorrect HID descriptor. > + * At byte 100 it contains > + * > + * 0x81, 0x00 > + * > + * which is Input (Data, Arr, Abs), but it should be > + * > + * 0x81, 0x02 > + * > + * which is Input (Data, Var, Abs), which is consistent with the way > + * key codes are generated. > + */ > + > +static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8 *rdesc, > + unsigned int *rsize) > +{ > + if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] == 0x00) { > + dev_info(&hdev->dev, "Fixing Redragon ASURA report descriptor.\n"); > + rdesc[101] = 0x02; > + } > + > + return rdesc; > +} > + > +static int redragon_probe(struct hid_device *dev, > + const struct hid_device_id *id) > +{ > + int ret; > + > + ret = hid_parse(dev); > + if (ret) { > + hid_err(dev, "parse failed\n"); > + return ret; > + } > + > + /* do not register unused input device */ > + if (dev->maxapplication == 1) > + return 0; > + > + ret = hid_hw_start(dev, HID_CONNECT_DEFAULT); > + if (ret) { > + hid_err(dev, "hw start failed\n"); > + return ret; > + } > + > + return 0; > +} > +static const struct hid_device_id redragon_devices[] = { > + {HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA)}, > + {} > +}; > + > +MODULE_DEVICE_TABLE(hid, redragon_devices); > + > +static struct hid_driver redragon_driver = { > + .name = "redragon", > + .id_table = redragon_devices, > + .report_fixup = redragon_report_fixup, > + .probe = redragon_probe > +}; > + > +module_hid_driver(redragon_driver); > + > +MODULE_LICENSE("GPL"); The SPDX header says GPL-v2. And IIRC if there is the SPDX header you can drop the MODULE_LICENSE (not entirely sure though). Cheers, Benjamin > -- > 2.16.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Benjamin and Jiri, On Mon, 2018-04-16 at 12:02 +0200, Benjamin Tissoires wrote: > Hi Robert, > > On Wed, Apr 11, 2018 at 11:49 AM, Robert Munteanu <rombert@apache.org > > wrote: > > Changelog: > > > > - v2: modifier keys work, some combinations are still troublesome > > - v3: style cleanup, rebase on top of 4.14 > > - v4: remove most debugging calls, make init info useful for user, > > rebased on top of 4.15 > > - v5: fix the HID descriptor as suggested by Benjamin Tissoires, > > use existing USB vendor id, update comment style, add SPDX > > license identifier, rename to hid-redragon, stop registering > > two input devices, rebased on top of 4.16 > > As Jiri said, please provide a correct commit message. Will do. > > I have a few nitpicks in the driver, v6 should be fine: > > > > > Signed-off-by: Robert Munteanu <rombert@apache.org> > > --- > > drivers/hid/Kconfig | 7 ++++ > > drivers/hid/Makefile | 1 + > > drivers/hid/hid-ids.h | 1 + > > drivers/hid/hid-quirks.c | 3 ++ > > drivers/hid/hid-redragon.c | 89 > > ++++++++++++++++++++++++++++++++++++++++++++++ > > 5 files changed, 101 insertions(+) > > create mode 100644 drivers/hid/hid-redragon.c > > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > index 19c499f5623d..1125e4813716 100644 > > --- a/drivers/hid/Kconfig > > +++ b/drivers/hid/Kconfig > > @@ -560,6 +560,13 @@ config HID_MAYFLASH > > Say Y here if you have HJZ Mayflash PS3 game controller > > adapters > > and want to enable force feedback support. > > > > +config HID_REDRAGON > > + tristate "Redragon keyboards" > > + depends on HID > > + default !EXPERT > > + ---help--- > > + Support for Redragon keyboards that need fix-ups to work > > properly. > > + > > config HID_MICROSOFT > > tristate "Microsoft non-fully HID-compliant devices" > > depends on HID > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > > index eb13b9e92d85..a36f3f40ba63 100644 > > --- a/drivers/hid/Makefile > > +++ b/drivers/hid/Makefile > > @@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS) += > > hid-picolcd_debugfs.o > > > > obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o > > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o > > +obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o > > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o > > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o > > \ > > hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \ > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > index 9454ac134ce2..41a64d0e91f9 100644 > > --- a/drivers/hid/hid-ids.h > > +++ b/drivers/hid/hid-ids.h > > @@ -599,6 +599,7 @@ > > #define USB_VENDOR_ID_JESS 0x0c45 > > #define USB_DEVICE_ID_JESS_YUREX 0x1010 > > #define USB_DEVICE_ID_ASUS_MD_5112 0x5112 > > +#define USB_DEVICE_ID_REDRAGON_ASURA 0x760b > > > > #define USB_VENDOR_ID_JESS2 0x0f30 > > #define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111 > > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c > > index e92b77fa574a..5f1253f1739a 100644 > > --- a/drivers/hid/hid-quirks.c > > +++ b/drivers/hid/hid-quirks.c > > @@ -557,6 +557,9 @@ static const struct hid_device_id > > hid_have_special_driver[] = { > > #if IS_ENABLED(CONFIG_HID_PRODIKEYS) > > { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, > > USB_DEVICE_ID_PRODIKEYS_PCMIDI) }, > > #endif > > +#if IS_ENABLED(CONFIG_HID_REDRAGON) > > + { > > HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA) > > }, > > +#endif > > Please drop this hunk. v4.16 should work without changing > hid_have_special_driver. This way, you will be sure that an initramfs > that doesn't include hid-redragon.ko will allo people to type their > LUKS password. I recall testing without this change resulted in the driver not being picked up. I will retest ( running 4.16.0 here, can update to 4.16.1 soon ). In case the driver is not picked up, where should I start looking? > > > #if IS_ENABLED(CONFIG_HID_RETRODE) > > { HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY, > > USB_DEVICE_ID_RETRODE2) }, > > #endif > > diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid- > > redragon.c > > new file mode 100644 > > index 000000000000..ff98a5dbb8e2 > > --- /dev/null > > +++ b/drivers/hid/hid-redragon.c > > @@ -0,0 +1,89 @@ > > +/* > > + * HID driver for Redragon keyboards > > + * > > + * Copyright (c) 2017 Robert Munteanu > > + * SPDX-License-Identifier: GPL-2.0 > > + */ > > + > > +/* > > + * 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; either version 2 of the License, or (at > > your option) > > + * any later version. > > + */ > > + > > +#include <linux/device.h> > > +#include <linux/input.h> > > you probably don't need input.h > > > +#include <linux/hid.h> > > +#include <linux/module.h> > > +#include <linux/log2.h> > > log2.h? I am not sure you need it either > > > +#include <linux/input-event-codes.h> > > probably drop this one too. I'll drop the missing imports, those are leftovers from my initial work. > > > + > > +#include "hid-ids.h" > > + > > + > > +/* > > + * The Redragon Asura keyboard sends an incorrect HID descriptor. > > + * At byte 100 it contains > > + * > > + * 0x81, 0x00 > > + * > > + * which is Input (Data, Arr, Abs), but it should be > > + * > > + * 0x81, 0x02 > > + * > > + * which is Input (Data, Var, Abs), which is consistent with the > > way > > + * key codes are generated. > > + */ > > + > > +static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8 > > *rdesc, > > + unsigned int *rsize) > > +{ > > + if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] == > > 0x00) { > > + dev_info(&hdev->dev, "Fixing Redragon ASURA report > > descriptor.\n"); > > + rdesc[101] = 0x02; > > + } > > + > > + return rdesc; > > +} > > + > > +static int redragon_probe(struct hid_device *dev, > > + const struct hid_device_id *id) > > +{ > > + int ret; > > + > > + ret = hid_parse(dev); > > + if (ret) { > > + hid_err(dev, "parse failed\n"); > > + return ret; > > + } > > + > > + /* do not register unused input device */ > > + if (dev->maxapplication == 1) > > + return 0; > > + > > + ret = hid_hw_start(dev, HID_CONNECT_DEFAULT); > > + if (ret) { > > + hid_err(dev, "hw start failed\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > +static const struct hid_device_id redragon_devices[] = { > > + {HID_USB_DEVICE(USB_VENDOR_ID_JESS, > > USB_DEVICE_ID_REDRAGON_ASURA)}, > > + {} > > +}; > > + > > +MODULE_DEVICE_TABLE(hid, redragon_devices); > > + > > +static struct hid_driver redragon_driver = { > > + .name = "redragon", > > + .id_table = redragon_devices, > > + .report_fixup = redragon_report_fixup, > > + .probe = redragon_probe > > +}; > > + > > +module_hid_driver(redragon_driver); > > + > > +MODULE_LICENSE("GPL"); > > The SPDX header says GPL-v2. And IIRC if there is the SPDX header you > can drop the MODULE_LICENSE (not entirely sure though). Ack, will adjust and re-test. Thanks for the review, Robert -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 16, 2018 at 12:14 PM, Robert Munteanu <rombert@apache.org> wrote: > Hi Benjamin and Jiri, > > On Mon, 2018-04-16 at 12:02 +0200, Benjamin Tissoires wrote: >> Hi Robert, >> >> On Wed, Apr 11, 2018 at 11:49 AM, Robert Munteanu <rombert@apache.org >> > wrote: >> > Changelog: >> > >> > - v2: modifier keys work, some combinations are still troublesome >> > - v3: style cleanup, rebase on top of 4.14 >> > - v4: remove most debugging calls, make init info useful for user, >> > rebased on top of 4.15 >> > - v5: fix the HID descriptor as suggested by Benjamin Tissoires, >> > use existing USB vendor id, update comment style, add SPDX >> > license identifier, rename to hid-redragon, stop registering >> > two input devices, rebased on top of 4.16 >> >> As Jiri said, please provide a correct commit message. > > Will do. > >> >> I have a few nitpicks in the driver, v6 should be fine: >> >> > >> > Signed-off-by: Robert Munteanu <rombert@apache.org> >> > --- >> > drivers/hid/Kconfig | 7 ++++ >> > drivers/hid/Makefile | 1 + >> > drivers/hid/hid-ids.h | 1 + >> > drivers/hid/hid-quirks.c | 3 ++ >> > drivers/hid/hid-redragon.c | 89 >> > ++++++++++++++++++++++++++++++++++++++++++++++ >> > 5 files changed, 101 insertions(+) >> > create mode 100644 drivers/hid/hid-redragon.c >> > >> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig >> > index 19c499f5623d..1125e4813716 100644 >> > --- a/drivers/hid/Kconfig >> > +++ b/drivers/hid/Kconfig >> > @@ -560,6 +560,13 @@ config HID_MAYFLASH >> > Say Y here if you have HJZ Mayflash PS3 game controller >> > adapters >> > and want to enable force feedback support. >> > >> > +config HID_REDRAGON >> > + tristate "Redragon keyboards" >> > + depends on HID >> > + default !EXPERT >> > + ---help--- >> > + Support for Redragon keyboards that need fix-ups to work >> > properly. >> > + >> > config HID_MICROSOFT >> > tristate "Microsoft non-fully HID-compliant devices" >> > depends on HID >> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile >> > index eb13b9e92d85..a36f3f40ba63 100644 >> > --- a/drivers/hid/Makefile >> > +++ b/drivers/hid/Makefile >> > @@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS) += >> > hid-picolcd_debugfs.o >> > >> > obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o >> > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o >> > +obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o >> > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o >> > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o >> > \ >> > hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \ >> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h >> > index 9454ac134ce2..41a64d0e91f9 100644 >> > --- a/drivers/hid/hid-ids.h >> > +++ b/drivers/hid/hid-ids.h >> > @@ -599,6 +599,7 @@ >> > #define USB_VENDOR_ID_JESS 0x0c45 >> > #define USB_DEVICE_ID_JESS_YUREX 0x1010 >> > #define USB_DEVICE_ID_ASUS_MD_5112 0x5112 >> > +#define USB_DEVICE_ID_REDRAGON_ASURA 0x760b >> > >> > #define USB_VENDOR_ID_JESS2 0x0f30 >> > #define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111 >> > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c >> > index e92b77fa574a..5f1253f1739a 100644 >> > --- a/drivers/hid/hid-quirks.c >> > +++ b/drivers/hid/hid-quirks.c >> > @@ -557,6 +557,9 @@ static const struct hid_device_id >> > hid_have_special_driver[] = { >> > #if IS_ENABLED(CONFIG_HID_PRODIKEYS) >> > { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, >> > USB_DEVICE_ID_PRODIKEYS_PCMIDI) }, >> > #endif >> > +#if IS_ENABLED(CONFIG_HID_REDRAGON) >> > + { >> > HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA) >> > }, >> > +#endif >> >> Please drop this hunk. v4.16 should work without changing >> hid_have_special_driver. This way, you will be sure that an initramfs >> that doesn't include hid-redragon.ko will allo people to type their >> LUKS password. > > I recall testing without this change resulted in the driver not being > picked up. I will retest ( running 4.16.0 here, can update to 4.16.1 > soon ). In case the driver is not picked up, where should I start > looking? > >> >> > #if IS_ENABLED(CONFIG_HID_RETRODE) >> > { HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY, >> > USB_DEVICE_ID_RETRODE2) }, >> > #endif >> > diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid- >> > redragon.c >> > new file mode 100644 >> > index 000000000000..ff98a5dbb8e2 >> > --- /dev/null >> > +++ b/drivers/hid/hid-redragon.c >> > @@ -0,0 +1,89 @@ >> > +/* >> > + * HID driver for Redragon keyboards >> > + * >> > + * Copyright (c) 2017 Robert Munteanu >> > + * SPDX-License-Identifier: GPL-2.0 >> > + */ >> > + >> > +/* >> > + * 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; either version 2 of the License, or (at >> > your option) >> > + * any later version. >> > + */ >> > + >> > +#include <linux/device.h> >> > +#include <linux/input.h> >> >> you probably don't need input.h >> >> > +#include <linux/hid.h> >> > +#include <linux/module.h> >> > +#include <linux/log2.h> >> >> log2.h? I am not sure you need it either >> >> > +#include <linux/input-event-codes.h> >> >> probably drop this one too. > > I'll drop the missing imports, those are leftovers from my initial > work. > >> >> > + >> > +#include "hid-ids.h" >> > + >> > + >> > +/* >> > + * The Redragon Asura keyboard sends an incorrect HID descriptor. >> > + * At byte 100 it contains >> > + * >> > + * 0x81, 0x00 >> > + * >> > + * which is Input (Data, Arr, Abs), but it should be >> > + * >> > + * 0x81, 0x02 >> > + * >> > + * which is Input (Data, Var, Abs), which is consistent with the >> > way >> > + * key codes are generated. >> > + */ >> > + >> > +static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8 >> > *rdesc, >> > + unsigned int *rsize) >> > +{ >> > + if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] == >> > 0x00) { >> > + dev_info(&hdev->dev, "Fixing Redragon ASURA report >> > descriptor.\n"); >> > + rdesc[101] = 0x02; >> > + } >> > + >> > + return rdesc; >> > +} >> > + >> > +static int redragon_probe(struct hid_device *dev, >> > + const struct hid_device_id *id) >> > +{ >> > + int ret; >> > + >> > + ret = hid_parse(dev); >> > + if (ret) { >> > + hid_err(dev, "parse failed\n"); >> > + return ret; >> > + } >> > + >> > + /* do not register unused input device */ >> > + if (dev->maxapplication == 1) >> > + return 0; >> > + >> > + ret = hid_hw_start(dev, HID_CONNECT_DEFAULT); >> > + if (ret) { >> > + hid_err(dev, "hw start failed\n"); >> > + return ret; >> > + } >> > + >> > + return 0; >> > +} >> > +static const struct hid_device_id redragon_devices[] = { >> > + {HID_USB_DEVICE(USB_VENDOR_ID_JESS, >> > USB_DEVICE_ID_REDRAGON_ASURA)}, >> > + {} >> > +}; >> > + >> > +MODULE_DEVICE_TABLE(hid, redragon_devices); >> > + >> > +static struct hid_driver redragon_driver = { >> > + .name = "redragon", >> > + .id_table = redragon_devices, >> > + .report_fixup = redragon_report_fixup, >> > + .probe = redragon_probe >> > +}; >> > + >> > +module_hid_driver(redragon_driver); >> > + >> > +MODULE_LICENSE("GPL"); >> >> The SPDX header says GPL-v2. And IIRC if there is the SPDX header you >> can drop the MODULE_LICENSE (not entirely sure though). > > Ack, will adjust and re-test. Actually, you might be correct. I just read Mauro's blog: https://blogs.s-osg.org/linux-kernel-license-practices-revisited-spdx/ and it says "Types of licenses for MODULE_LICENSE() macro -> 'GPL' -> GNU Public License v2 or later" (in the second table). Cheers, Benjamin > > Thanks for the review, > > Robert -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2018-04-16 at 15:36 +0200, Benjamin Tissoires wrote: > > > > +MODULE_LICENSE("GPL"); > > > > > > The SPDX header says GPL-v2. And IIRC if there is the SPDX header > > > you > > > can drop the MODULE_LICENSE (not entirely sure though). > > > > Ack, will adjust and re-test. > > Actually, you might be correct. I just read Mauro's blog: > https://blogs.s-osg.org/linux-kernel-license-practices-revisited-spdx > / > and it says "Types of licenses for MODULE_LICENSE() macro -> 'GPL' -> > GNU Public License v2 or later" (in the second table). Currently the header says 'SPDX-License-Identifier: GPL-2.0', which is 'GNU General Public License v2.0 only' according to https://spdx.org/licenses/ So we can't use that with MODULE_LICENSE("GPL") which means 'GNU General Public License v2.0 or later' . I don't have a strong preference for either, but I'll go with the 'or later' part since that seems to be more in favour ( shorter argument for the MODULE_LICENSE macro ). Robert -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 19c499f5623d..1125e4813716 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -560,6 +560,13 @@ config HID_MAYFLASH Say Y here if you have HJZ Mayflash PS3 game controller adapters and want to enable force feedback support. +config HID_REDRAGON + tristate "Redragon keyboards" + depends on HID + default !EXPERT + ---help--- + Support for Redragon keyboards that need fix-ups to work properly. + config HID_MICROSOFT tristate "Microsoft non-fully HID-compliant devices" depends on HID diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index eb13b9e92d85..a36f3f40ba63 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS) += hid-picolcd_debugfs.o obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o obj-$(CONFIG_HID_PRIMAX) += hid-primax.o +obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o obj-$(CONFIG_HID_RETRODE) += hid-retrode.o obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \ diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 9454ac134ce2..41a64d0e91f9 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -599,6 +599,7 @@ #define USB_VENDOR_ID_JESS 0x0c45 #define USB_DEVICE_ID_JESS_YUREX 0x1010 #define USB_DEVICE_ID_ASUS_MD_5112 0x5112 +#define USB_DEVICE_ID_REDRAGON_ASURA 0x760b #define USB_VENDOR_ID_JESS2 0x0f30 #define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111 diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index e92b77fa574a..5f1253f1739a 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -557,6 +557,9 @@ static const struct hid_device_id hid_have_special_driver[] = { #if IS_ENABLED(CONFIG_HID_PRODIKEYS) { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) }, #endif +#if IS_ENABLED(CONFIG_HID_REDRAGON) + { HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA) }, +#endif #if IS_ENABLED(CONFIG_HID_RETRODE) { HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY, USB_DEVICE_ID_RETRODE2) }, #endif diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid-redragon.c new file mode 100644 index 000000000000..ff98a5dbb8e2 --- /dev/null +++ b/drivers/hid/hid-redragon.c @@ -0,0 +1,89 @@ +/* + * HID driver for Redragon keyboards + * + * Copyright (c) 2017 Robert Munteanu + * SPDX-License-Identifier: GPL-2.0 + */ + +/* + * 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; either version 2 of the License, or (at your option) + * any later version. + */ + +#include <linux/device.h> +#include <linux/input.h> +#include <linux/hid.h> +#include <linux/module.h> +#include <linux/log2.h> +#include <linux/input-event-codes.h> + +#include "hid-ids.h" + + +/* + * The Redragon Asura keyboard sends an incorrect HID descriptor. + * At byte 100 it contains + * + * 0x81, 0x00 + * + * which is Input (Data, Arr, Abs), but it should be + * + * 0x81, 0x02 + * + * which is Input (Data, Var, Abs), which is consistent with the way + * key codes are generated. + */ + +static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8 *rdesc, + unsigned int *rsize) +{ + if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] == 0x00) { + dev_info(&hdev->dev, "Fixing Redragon ASURA report descriptor.\n"); + rdesc[101] = 0x02; + } + + return rdesc; +} + +static int redragon_probe(struct hid_device *dev, + const struct hid_device_id *id) +{ + int ret; + + ret = hid_parse(dev); + if (ret) { + hid_err(dev, "parse failed\n"); + return ret; + } + + /* do not register unused input device */ + if (dev->maxapplication == 1) + return 0; + + ret = hid_hw_start(dev, HID_CONNECT_DEFAULT); + if (ret) { + hid_err(dev, "hw start failed\n"); + return ret; + } + + return 0; +} +static const struct hid_device_id redragon_devices[] = { + {HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA)}, + {} +}; + +MODULE_DEVICE_TABLE(hid, redragon_devices); + +static struct hid_driver redragon_driver = { + .name = "redragon", + .id_table = redragon_devices, + .report_fixup = redragon_report_fixup, + .probe = redragon_probe +}; + +module_hid_driver(redragon_driver); + +MODULE_LICENSE("GPL");
Changelog: - v2: modifier keys work, some combinations are still troublesome - v3: style cleanup, rebase on top of 4.14 - v4: remove most debugging calls, make init info useful for user, rebased on top of 4.15 - v5: fix the HID descriptor as suggested by Benjamin Tissoires, use existing USB vendor id, update comment style, add SPDX license identifier, rename to hid-redragon, stop registering two input devices, rebased on top of 4.16 Signed-off-by: Robert Munteanu <rombert@apache.org> --- drivers/hid/Kconfig | 7 ++++ drivers/hid/Makefile | 1 + drivers/hid/hid-ids.h | 1 + drivers/hid/hid-quirks.c | 3 ++ drivers/hid/hid-redragon.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 101 insertions(+) create mode 100644 drivers/hid/hid-redragon.c