linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Alex Henrie <alexhenrie24@gmail.com>
Cc: jikos@kernel.org, jirislaby@gmail.com, stefan@glasen-hardt.de,
	ryan@finnie.org, morbidrsa@gmail.com, dan@pwienterprises.com,
	andreas.krist@gmail.com, rydberg@bitmath.org, lukas@wunner.de,
	linux-input@vger.kernel.org
Subject: Re: [PATCH v3] HID: apple: Use country code to detect ISO keyboards
Date: Tue, 6 Jun 2017 09:29:31 +0200	[thread overview]
Message-ID: <20170606072931.GT1293@mail.corp.redhat.com> (raw)
In-Reply-To: <20170602152839.7810-1-alexhenrie24@gmail.com>

On Jun 02 2017 or thereabouts, Alex Henrie wrote:
> At least on newer laptops, Apple uses the same USB ID for both ISO and
> ANSI keyboards. However, they have been good about filling in the
> bCountryCode field in the HID descriptor on all of their keyboards. A
> value of 13 indicates an ISO layout and other values indicate various
> country-specific ANSI layouts.
> 
> With this patch, users of Apple US keyboards will no longer have to run
> `echo 0 > /sys/module/hid_apple/parameters/iso_layout` to get a working
> tilde key.
> 
> Please test this patch and send feedback if you have a Macbook or an
> Apple keyboard.
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---

Looks good to me. Thanks for the v3:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

>  drivers/hid/hid-apple.c | 59 ++++++++++++++++++++++---------------------------
>  1 file changed, 27 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 2e046082210f..25b7bd56ae11 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -28,7 +28,7 @@
>  #define APPLE_IGNORE_MOUSE	0x0002
>  #define APPLE_HAS_FN		0x0004
>  #define APPLE_HIDDEV		0x0008
> -#define APPLE_ISO_KEYBOARD	0x0010
> +/* 0x0010 reserved, was: APPLE_ISO_KEYBOARD */
>  #define APPLE_MIGHTYMOUSE	0x0020
>  #define APPLE_INVERT_HWHEEL	0x0040
>  #define APPLE_IGNORE_HIDINPUT	0x0080
> @@ -36,6 +36,8 @@
>  
>  #define APPLE_FLAG_FKEY		0x01
>  
> +#define HID_COUNTRY_INTERNATIONAL_ISO	13
> +
>  static unsigned int fnmode = 1;
>  module_param(fnmode, uint, 0644);
>  MODULE_PARM_DESC(fnmode, "Mode of fn key on Apple keyboards (0 = disabled, "
> @@ -247,7 +249,7 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
>  	}
>  
>  	if (iso_layout) {
> -		if (asc->quirks & APPLE_ISO_KEYBOARD) {
> +		if (hid->country == HID_COUNTRY_INTERNATIONAL_ISO) {
>  			trans = apple_find_translation(apple_iso_keyboard, usage->code);
>  			if (trans) {
>  				input_event(input, usage->type, trans->to, value);
> @@ -412,60 +414,54 @@ static const struct hid_device_id apple_devices[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ANSI),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ISO),
> -		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
> -			APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_JIS),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ANSI),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ISO),
> -		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
> -			APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_JIS),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
>  			APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ANSI),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ISO),
> -		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
> -			APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_JIS),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
>  			APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_MINI_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_MINI_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_MINI_JIS),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_JIS),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO),
> -		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
> -			APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
>  			APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_REVB_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_REVB_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_REVB_JIS),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
> -		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
> -			APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ISO),
> -		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
> -			APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
>  				USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ANSI),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
> @@ -479,86 +475,85 @@ static const struct hid_device_id apple_devices[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING3_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING3_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING3_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING4_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING4_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING4A_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING4A_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6A_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6A_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6A_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5A_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5A_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5A_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7A_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7A_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7A_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING8_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING8_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING8_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING9_ANSI),
>  		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING9_ISO),
> -		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING9_JIS),
>  		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
>  	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_ANSI),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_ISO),
> -		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
> -			APPLE_ISO_KEYBOARD },
> +		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_JIS),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY),
> -- 
> 2.13.0
> 

  reply	other threads:[~2017-06-06  7:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-02 15:28 [PATCH v3] HID: apple: Use country code to detect ISO keyboards Alex Henrie
2017-06-06  7:29 ` Benjamin Tissoires [this message]
2017-06-08 12:03 ` Jiri Kosina
2017-06-08 15:15   ` Alex Henrie
2017-06-17 18:56   ` Lukas Wunner
2017-06-20 11:54     ` Bastien Nocera
2017-06-20 17:18       ` Alex Henrie
2017-06-20 18:00         ` Bastien Nocera
2017-08-05 17:09           ` Lukas Wunner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170606072931.GT1293@mail.corp.redhat.com \
    --to=benjamin.tissoires@redhat.com \
    --cc=alexhenrie24@gmail.com \
    --cc=andreas.krist@gmail.com \
    --cc=dan@pwienterprises.com \
    --cc=jikos@kernel.org \
    --cc=jirislaby@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=morbidrsa@gmail.com \
    --cc=ryan@finnie.org \
    --cc=rydberg@bitmath.org \
    --cc=stefan@glasen-hardt.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).