Hi,
On 28-03-19 09:10, Benjamin Tissoires wrote:
Hi Alex,
On Thu, Mar 28, 2019 at 4:34 AM Alex Henrie <alexhenrie24@xxxxxxxxx> wrote:
This enables the brightness-down and brightness-up keys on the Macally
QKEY keyboard. Similar workarounds are probably needed for quite a few
Macally keyboard models.
Based on the key translation code in the Apple keyboard driver.
Well, this driver is much more complex and we already have all the
facility in HID to do plain remapping (and we could even do that in
userspace with a hwdb entry).
So:
Signed-off-by: Alex Henrie <alexhenrie24@xxxxxxxxx>
---
drivers/hid/Kconfig | 1 +
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-macally.c | 57 +++++++++++++++++++++++++++++++++++++++
3 files changed, 61 insertions(+)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index aef4a2a690e1..082900477df5 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -239,6 +239,7 @@ config HID_MACALLY
supported devices:
- Macally ikey keyboard
+ - Macally QKEY keyboard
config HID_PRODIKEYS
tristate "Prodikeys PC-MIDI Keyboard support"
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index aacc7534b076..5afc3b7fe8ca 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -776,6 +776,9 @@
#define USB_DEVICE_ID_CRYSTALTOUCH 0x0006
#define USB_DEVICE_ID_CRYSTALTOUCH_DUAL 0x0007
+#define USB_VENDOR_ID_MACALLY 0x2222
+#define USB_DEVICE_ID_MACALLY_QKEY_KEYBOARD 0x0039
+
#define USB_VENDOR_ID_MADCATZ 0x0738
#define USB_DEVICE_ID_MADCATZ_BEATPAD 0x4540
#define USB_DEVICE_ID_MADCATZ_RAT5 0x1705
diff --git a/drivers/hid/hid-macally.c b/drivers/hid/hid-macally.c
index 6f62f059b795..2567babe8200 100644
--- a/drivers/hid/hid-macally.c
+++ b/drivers/hid/hid-macally.c
@@ -31,9 +31,64 @@ static __u8 *macally_report_fixup(struct hid_device *hdev, __u8 *rdesc,
return rdesc;
}
+struct macally_key_translation
+{
+ u16 from;
+ u16 to;
+};
+
+static const struct macally_key_translation qkey_brightness_keys[] =
+{
+ { KEY_SCROLLLOCK, KEY_BRIGHTNESSDOWN },
+ { KEY_PAUSE, KEY_BRIGHTNESSUP },
+ { }
+};
This declaration and its associated struct should be dropped and
inlined in input_mapping() (see below)
+
+static int macally_event(struct hid_device *hdev, struct hid_field *field,
+ struct hid_usage *usage, __s32 value)
+{
+ const struct macally_key_translation *trans;
+
+ switch (hdev->product) {
+ case USB_DEVICE_ID_MACALLY_QKEY_KEYBOARD:
+ trans = qkey_brightness_keys;
+ break;
+ default:
+ trans = NULL;
+ }
+
+ if (trans) {
+ while (trans->from) {
+ if (trans->from == usage->code) {
+ input_event(field->hidinput->input, usage->type,
+ trans->to, value);
+ return 1;
+ }
+ trans++;
+ }
+ }
+
+ return 0;
+}
Please drop the entire callback above
+
+static int macally_input_mapping(struct hid_device *hdev, struct hid_input *hi,
+ struct hid_field *field, struct hid_usage *usage,
+ unsigned long **bit, int *max)
+{
+ const struct macally_key_translation *trans;
+
+ /* Enable all needed keys */
+ for (trans = qkey_brightness_keys; trans->from; trans++)
+ set_bit(trans->to, hi->input->keybit);
See hid-accutouch:
if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
return 1;
}
So just add a switch instead of a plain 'if', check on the
KEY_SCROLLLOCK or KEY_PAUSE keycode and remap them to the correct
brightness up/down.
But again, you can get tot he same result by adding a hwdb entry.
However, given that you need to fix up the report descriptor for the
(other?) keyboard, I can be convinced to take this patch too.
Dmitry, Hans, do you think this belong to the kernel or should this be
taken into a udev rule?
Descriptor fixups obviously need to be done in the kernel, but I
believe that key-remapping which can be done through hwdb is best done
there as it is a lot easier to update 60-keyboard.hwdb then to change
to running a (lot) newer kernel and more importantly we should strive
to keep the kernel code as small as possible.
Now about this specific keyboards, a quick duckduckgo found me this
picture:
https://www.bhphotovideo.com/images/images1500x1500/macally_mkeyx_104_key_full_sized_1274821.jpg
And to me it seems scroll-lock and pause-break should be mapped
to zoom-put resp. zoom-in and Fn + scroll-lock / pause-break
should send the brightness down/up events. Alex have you checked if
using Fn + these keys sends a different hid usage-code event?
Hmm, I see all the top-tow keys have a special meaning and a
Fn + key meaning, where Fn seems to be required to get them to
behave as the normal key in that position, so I guess the
scroll-lock / pause-break is actually send when using the keys
with Fn ?
Regards,
Hans
