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@xxxxxxxxx>
The driver looks good to me:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
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@xxxxxxxxx>
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+MODULE_AUTHOR("Alex Henrie <alexhenrie24@xxxxxxxxx>");
+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);
