Re: [PATCH V5] HID: asus: add support for ASUS N-Key keyboard

On Wed, 19 Aug 2020, Luke Jones wrote:

On Tue, Aug 18, 2020 at 20:37, Jiri Kosina [off-list ref] wrote:

quoted

On Tue, 18 Aug 2020, Luke Jones wrote:

quoted

@@ -751,14 +841,14 @@ static int asus_input_mapping(struct hid_device

*hdev,
        usage->hid == (HID_UP_GENDEVCTRLS | 0x0026)))
    return -1;

 -	/* ASUS-specific keyboard hotkeys */
 -	if ((usage->hid & HID_USAGE_PAGE) == 0xff310000) {
 +	/* ASUS-specific keyboard hotkeys and led backlight */
 +	if ((usage->hid & HID_USAGE_PAGE) == HID_UP_ASUSVENDOR) {
    switch (usage->hid & HID_USAGE) {
    case 0x10: asus_map_key_clear(KEY_BRIGHTNESSDOWN);	break;
    case 0x20: asus_map_key_clear(KEY_BRIGHTNESSUP);		break;
    case 0x35: asus_map_key_clear(KEY_DISPLAY_OFF);		break;
    case 0x6c: asus_map_key_clear(KEY_SLEEP);		break;
 -		case 0x7c: asus_map_key_clear(KEY_MICMUTE);		break;
 +		case 0x7c: asus_map_key_clear(KEY_F20);		break;

This change doesn't seem to be mentioned in the changelog; why is it OK in
general case for other devices sharing this codepath?

Do you mean the HID_UP_ASUSVENDOR? This evaluates to 0xff310000. I was unsure
how to address that - should I mention it in changes?

Nah, that one is clear :)

The MICMUTE issue: "change "Mic Toggle" to use a keycode that works" maybe I
should elaborate on this - the keycode appears to be unused or undetected by
KDE, Gnome, XFCE, and so the mic would never toggle. F20 gives the desired
effect.

Yes, this one I was wondering about. Please document that in the 
changelog.

Thank you for the feedback. I will submit a revised patch. Should this 
be in a new thread as I've done so far, or as a reply here?

Either is fine. But let's wait a bit for Dmitry's ACK first for the new 
keycode constants.

Thanks!

-- 
Jiri Kosina
SUSE Labs