Re: [PATCH] HID: apple: Disable Fn-key key-re-mapping on clone keyboards

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 4/25/20 11:30 AM, João Moreno wrote:
> Hi Hans,
> 
> On Fri, 24 Apr 2020 at 13:22, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> The Maxxter KB-BT-001 Bluetooth keyboard, which looks somewhat like the
>> Apple Wireless Keyboard, is using the vendor and product IDs (05AC:0239)
>> of the Apple Wireless Keyboard (2009 ANSI version) <sigh>.
>>
>> But its F1 - F10 keys are marked as sending F1 - F10, not the special
>> functions hid-apple.c maps them too; and since its descriptors do not
>> contain the HID_UP_CUSTOM | 0x0003 usage apple-hid looks for for the
>> Fn-key, apple_setup_input() never gets called, so F1 - F6 are mapped
>> to key-codes which have not been set in the keybit array causing them
>> to not send any events at all.
> 
> Oh no.
> 
>>
>> The lack of a usage code matching the Fn key in the clone is actually
>> useful as this allows solving this problem in a generic way.
>>
>> This commits adds a fn_found flag and it adds a input_configured
>> callback which checks if this flag is set once all usages have been
>> mapped. If it is not set, then assume this is a clone and clear the
>> quirks bitmap so that the hid-apple code does not add any special
>> handling to this keyboard.
>>
>> This fixes F1 - F6 not sending anything at all and F7 - F12 sending
>> the wrong codes on the Maxxter KB-BT-001 Bluetooth keyboard and on
>> similar clones.
>>
>> Cc: Joao Moreno <mail@joaomoreno.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Note hid-apple also enables APPLE_NUMLOCK_EMULATION for
>> USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_ANSI. At least on the Maxxter clone
>> this causes numlock to get stuck in the pressed state after pressing it
>> twice (its get stuck when pressed when the numlock led is on). This might
>> be specific to the clone, but it would be good to also check if this is
>> not an issue on an actual Apple Wireless Keyboard.
>>
>> Joao, can you check the numlock-emulation stuff on your 05AC:0256
>> (APPLE_ALU_WIRELESS_2011_ISO) keyboard?
> 
> Unfortunately I don't have that keyboard any more around. Maybe
> someone else around could give this a try?

Ok, thank you for letting us know. This was sorta orthogonal
to this patch, its just that I noticed that the numlock emulation
behaved weird with the maxxter clone. Not sure if this is due to
the clone behaving differently then the original Mac keyboards,
or if its an issue where the hid-apply code has regressed over time.

Regards,

Hans


> 
>> ---
>>   drivers/hid/hid-apple.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
>> index d732d1d10caf..6909c045fece 100644
>> --- a/drivers/hid/hid-apple.c
>> +++ b/drivers/hid/hid-apple.c
>> @@ -54,6 +54,7 @@ MODULE_PARM_DESC(swap_opt_cmd, "Swap the Option (\"Alt\") and Command (\"Flag\")
>>   struct apple_sc {
>>          unsigned long quirks;
>>          unsigned int fn_on;
>> +       unsigned int fn_found;
>>          DECLARE_BITMAP(pressed_numlock, KEY_CNT);
>>   };
>>
>> @@ -339,12 +340,15 @@ static int apple_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>>                  struct hid_field *field, struct hid_usage *usage,
>>                  unsigned long **bit, int *max)
>>   {
>> +       struct apple_sc *asc = hid_get_drvdata(hdev);
>> +
>>          if (usage->hid == (HID_UP_CUSTOM | 0x0003) ||
>>                          usage->hid == (HID_UP_MSVENDOR | 0x0003) ||
>>                          usage->hid == (HID_UP_HPVENDOR2 | 0x0003)) {
>>                  /* The fn key on Apple USB keyboards */
>>                  set_bit(EV_REP, hi->input->evbit);
>>                  hid_map_usage_clear(hi, usage, bit, max, EV_KEY, KEY_FN);
>> +               asc->fn_found = true;
>>                  apple_setup_input(hi->input);
>>                  return 1;
>>          }
>> @@ -371,6 +375,19 @@ static int apple_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>>          return 0;
>>   }
>>
>> +static int apple_input_configured(struct hid_device *hdev,
>> +               struct hid_input *hidinput)
>> +{
>> +       struct apple_sc *asc = hid_get_drvdata(hdev);
>> +
>> +       if ((asc->quirks & APPLE_HAS_FN) && !asc->fn_found) {
>> +               hid_info(hdev, "Fn key not found (Apple Wireless Keyboard clone?), disabling Fn key handling\n");
>> +               asc->quirks = 0;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>   static int apple_probe(struct hid_device *hdev,
>>                  const struct hid_device_id *id)
>>   {
>> @@ -585,6 +602,7 @@ static struct hid_driver apple_driver = {
>>          .event = apple_event,
>>          .input_mapping = apple_input_mapping,
>>          .input_mapped = apple_input_mapped,
>> +       .input_configured = apple_input_configured,
>>   };
>>   module_hid_driver(apple_driver);
>>
>> --
>> 2.26.0
>>
>