[PATCH] hid/apple: add module parameter to swap the Command and Option keys

[PATCH] hid/apple: add module parameter to swap the Command and Option keys

[Paul Collins]

Apple keyboards have the Command keys (a.k.a. the Apple or "waffle" key)
in the positions normally occupied on PC keyboards by the Alt/AltGr keys,
and the Option or Alt keys in the position occupied by the so-called
Windows keys.  Folks who have been using PC-type computers for too long
generally have insurmountable muscle memory in this regard.

This patch adds a module parameter, defaulting to off, that swaps these keys.

The same effect can also be achieved by changing the console and X
keymaps, but this approach does not scale.  For example, I don't want to
have to learn how to reconfigure Wayland's keymaps when the future
arrives and we all start using it, and there may be applications I don't
know about that also read keyboard events directly.

Signed-off-by: Paul Collins <paul@ondioline.org>
---
 drivers/hid/hid-apple.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 7ed94cf..63e9ee3 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -40,6 +40,11 @@ module_param(fnmode, uint, 0644);
 MODULE_PARM_DESC(fnmode, "Mode of fn key on Apple keyboards (0 = disabled, "
 		"[1] = fkeyslast, 2 = fkeysfirst)");
 
+static unsigned int swapmodifiers = 0;
+module_param(swapmodifiers, uint, 0644);
+MODULE_PARM_DESC(fnmode, "Modifiers match labels or positions "
+		 "([0] = labels, 1 = positions)");
+
 struct apple_sc {
 	unsigned long quirks;
 	unsigned int fn_on;
@@ -123,6 +128,14 @@ static struct apple_key_translation apple_iso_keyboard[] = {
 	{ }
 };
 
+static struct apple_key_translation apple_swap_modifiers[] = {
+	{ KEY_LEFTALT,	KEY_LEFTMETA },
+	{ KEY_RIGHTALT,	KEY_RIGHTMETA },
+	{ KEY_LEFTMETA,	KEY_LEFTALT },
+	{ KEY_RIGHTMETA, KEY_RIGHTALT },
+	{ }
+};
+
 static struct apple_key_translation *apple_find_translation(
 		struct apple_key_translation *table, u16 from)
 {
@@ -199,6 +212,14 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 		}
 	}
 
+	if (swapmodifiers) {
+		trans = apple_find_translation(apple_swap_modifiers, usage->code);
+		if (trans) {
+			input_event(input, usage->type, trans->to, value);
+			return 1;
+		}
+	}
+
 	if (asc->quirks & APPLE_ISO_KEYBOARD) {
 		trans = apple_find_translation(apple_iso_keyboard, usage->code);
 		if (trans) {

-- 
Paul Collins
Wellington, New Zealand

Dag vijandelijk luchtschip de huismeester is dood

Re: [PATCH] hid/apple: add module parameter to swap the Command and Option keys

[Randy Dunlap]

Paul Collins wrote:
> Apple keyboards have the Command keys (a.k.a. the Apple or "waffle" key)
> in the positions normally occupied on PC keyboards by the Alt/AltGr keys,
> and the Option or Alt keys in the position occupied by the so-called
> Windows keys.  Folks who have been using PC-type computers for too long
> generally have insurmountable muscle memory in this regard.
> 
> This patch adds a module parameter, defaulting to off, that swaps these keys.
> 
> The same effect can also be achieved by changing the console and X
> keymaps, but this approach does not scale.  For example, I don't want to
> have to learn how to reconfigure Wayland's keymaps when the future
> arrives and we all start using it, and there may be applications I don't
> know about that also read keyboard events directly.

Hi,
Where do you suggest that this option be documented?

> Signed-off-by: Paul Collins <paul@ondioline.org>
> ---
>  drivers/hid/hid-apple.c |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 7ed94cf..63e9ee3 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -40,6 +40,11 @@ module_param(fnmode, uint, 0644);
>  MODULE_PARM_DESC(fnmode, "Mode of fn key on Apple keyboards (0 = disabled, "
>  		"[1] = fkeyslast, 2 = fkeysfirst)");
>  
> +static unsigned int swapmodifiers = 0;
> +module_param(swapmodifiers, uint, 0644);
> +MODULE_PARM_DESC(fnmode, "Modifiers match labels or positions "
> +		 "([0] = labels, 1 = positions)");
> +
>  struct apple_sc {
>  	unsigned long quirks;
>  	unsigned int fn_on;
> @@ -123,6 +128,14 @@ static struct apple_key_translation apple_iso_keyboard[] = {
>  	{ }
>  };
>  
> +static struct apple_key_translation apple_swap_modifiers[] = {
> +	{ KEY_LEFTALT,	KEY_LEFTMETA },
> +	{ KEY_RIGHTALT,	KEY_RIGHTMETA },
> +	{ KEY_LEFTMETA,	KEY_LEFTALT },
> +	{ KEY_RIGHTMETA, KEY_RIGHTALT },
> +	{ }
> +};
> +
>  static struct apple_key_translation *apple_find_translation(
>  		struct apple_key_translation *table, u16 from)
>  {
> @@ -199,6 +212,14 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
>  		}
>  	}
>  
> +	if (swapmodifiers) {
> +		trans = apple_find_translation(apple_swap_modifiers, usage->code);
> +		if (trans) {
> +			input_event(input, usage->type, trans->to, value);
> +			return 1;
> +		}
> +	}
> +
>  	if (asc->quirks & APPLE_ISO_KEYBOARD) {
>  		trans = apple_find_translation(apple_iso_keyboard, usage->code);
>  		if (trans) {
> 

Thanks,
-- 
~Randy

Re: [PATCH] hid/apple: add module parameter to swap the Command and Option keys

[Paul Collins]

Randy Dunlap <randy.dunlap@oracle.com> writes:

> Paul Collins wrote:
>> Apple keyboards have the Command keys (a.k.a. the Apple or "waffle" key)
>> in the positions normally occupied on PC keyboards by the Alt/AltGr keys,
>> and the Option or Alt keys in the position occupied by the so-called
>> Windows keys.  Folks who have been using PC-type computers for too long
>> generally have insurmountable muscle memory in this regard.
>> 
>> This patch adds a module parameter, defaulting to off, that swaps these keys.
>
> Where do you suggest that this option be documented?

Existing practice is to not document hid-apple options

        [burly(linux-2.6)] grep -r fnmode Documentation/ | wc -l
        0

so I'm open to suggestions.

-- 
Paul Collins
Wellington, New Zealand

Dag vijandelijk luchtschip de huismeester is dood

Re: [PATCH] hid/apple: add module parameter to swap the Command and Option keys

[Jiri Slaby]

On 6.3.2009 06:33, Paul Collins wrote:
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -40,6 +40,11 @@ module_param(fnmode, uint, 0644);
>   MODULE_PARM_DESC(fnmode, "Mode of fn key on Apple keyboards (0 = disabled, "
>   		"[1] = fkeyslast, 2 = fkeysfirst)");
>
> +static unsigned int swapmodifiers = 0;

Nowadays compilers optimize this away already, but no need to initialize 
.bss stuff to zero anyway.

> +module_param(swapmodifiers, uint, 0644);
> +MODULE_PARM_DESC(fnmode, "Modifiers match labels or positions "

Weird first parameter => swapmodifiers.

Maybe this should rather be a bool instead of uint.

> +		 "([0] = labels, 1 = positions)");
> +
>   struct apple_sc {
>   	unsigned long quirks;
>   	unsigned int fn_on;
> @@ -123,6 +128,14 @@ static struct apple_key_translation apple_iso_keyboard[] = {
>   	{ }
>   };
>
> +static struct apple_key_translation apple_swap_modifiers[] = {

This might go into .rodata, right? => const

Anyway this is not feasible to do in this patch. Would you like to post 
a followup patch which will also modify the functions which use the 
translation tables?

[PATCH 1/2] hid/apple: add module parameter to swap Command and Option keys

[Paul Collins]

Apple keyboards have the Command keys (a.k.a. the Apple or "waffle" key)
in the positions normally occupied on PC keyboards by the Alt/AltGr keys,
and the Option or Alt keys in the position occupied by the so-called
Windows keys.  Folks who have been using PC-type computers for too long
generally have insurmountable muscle memory in this regard.

This patch adds a module parameter, defaulting to off, that swaps these keys.

The same effect can also be achieved by changing the console and X
keymaps, but this approach does not scale.  For example, I don't want to
have to learn how to reconfigure Wayland's keymaps when the future
arrives and we all start using it, and there may be applications I don't
know about that also read keyboard events directly.

Signed-off-by: Paul Collins <paul@ondioline.org>
---
 drivers/hid/hid-apple.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index aa28aed..74429cc 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -40,6 +40,11 @@ module_param(fnmode, uint, 0644);
 MODULE_PARM_DESC(fnmode, "Mode of fn key on Apple keyboards (0 = disabled, "
 		"[1] = fkeyslast, 2 = fkeysfirst)");
 
+static int swapmodifiers;
+module_param(swapmodifiers, bool, 0644);
+MODULE_PARM_DESC(swapmodifiers, "Modifiers match labels or positions "
+		 "([0] = labels, 1 = positions)");
+
 struct apple_sc {
 	unsigned long quirks;
 	unsigned int fn_on;
@@ -123,6 +128,14 @@ static struct apple_key_translation apple_iso_keyboard[] = {
 	{ }
 };
 
+static struct apple_key_translation apple_swap_modifiers[] = {
+	{ KEY_LEFTALT,	KEY_LEFTMETA },
+	{ KEY_RIGHTALT,	KEY_RIGHTMETA },
+	{ KEY_LEFTMETA,	KEY_LEFTALT },
+	{ KEY_RIGHTMETA, KEY_RIGHTALT },
+	{ }
+};
+
 static struct apple_key_translation *apple_find_translation(
 		struct apple_key_translation *table, u16 from)
 {
@@ -199,6 +212,14 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 		}
 	}
 
+	if (swapmodifiers) {
+		trans = apple_find_translation(apple_swap_modifiers, usage->code);
+		if (trans) {
+			input_event(input, usage->type, trans->to, value);
+			return 1;
+		}
+	}
+
 	if (asc->quirks & APPLE_ISO_KEYBOARD) {
 		trans = apple_find_translation(apple_iso_keyboard, usage->code);
 		if (trans) {
-- 
1.6.2

[PATCH 2/2] hid/apple: constify arrays of struct apple_key_translation and adjust users

[Paul Collins]

Mark arrays of struct apple_key_translation const so that they may be placed in
.rodata, and adjust users to suit.

Signed-off-by: Paul Collins <paul@ondioline.org>
---
 drivers/hid/hid-apple.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 74429cc..04604df 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -58,7 +58,7 @@ struct apple_key_translation {
 	u8 flags;
 };
 
-static struct apple_key_translation apple_fn_keys[] = {
+static const struct apple_key_translation apple_fn_keys[] = {
 	{ KEY_BACKSPACE, KEY_DELETE },
 	{ KEY_ENTER,	KEY_INSERT },
 	{ KEY_F1,	KEY_BRIGHTNESSDOWN, APPLE_FLAG_FKEY },
@@ -80,7 +80,7 @@ static struct apple_key_translation apple_fn_keys[] = {
 	{ }
 };
 
-static struct apple_key_translation powerbook_fn_keys[] = {
+static const struct apple_key_translation powerbook_fn_keys[] = {
 	{ KEY_BACKSPACE, KEY_DELETE },
 	{ KEY_F1,	KEY_BRIGHTNESSDOWN,     APPLE_FLAG_FKEY },
 	{ KEY_F2,	KEY_BRIGHTNESSUP,       APPLE_FLAG_FKEY },
@@ -99,7 +99,7 @@ static struct apple_key_translation powerbook_fn_keys[] = {
 	{ }
 };
 
-static struct apple_key_translation powerbook_numlock_keys[] = {
+static const struct apple_key_translation powerbook_numlock_keys[] = {
 	{ KEY_J,	KEY_KP1 },
 	{ KEY_K,	KEY_KP2 },
 	{ KEY_L,	KEY_KP3 },
@@ -122,13 +122,13 @@ static struct apple_key_translation powerbook_numlock_keys[] = {
 	{ }
 };
 
-static struct apple_key_translation apple_iso_keyboard[] = {
+static const struct apple_key_translation apple_iso_keyboard[] = {
 	{ KEY_GRAVE,	KEY_102ND },
 	{ KEY_102ND,	KEY_GRAVE },
 	{ }
 };
 
-static struct apple_key_translation apple_swap_modifiers[] = {
+static const struct apple_key_translation apple_swap_modifiers[] = {
 	{ KEY_LEFTALT,	KEY_LEFTMETA },
 	{ KEY_RIGHTALT,	KEY_RIGHTMETA },
 	{ KEY_LEFTMETA,	KEY_LEFTALT },
@@ -136,10 +136,10 @@ static struct apple_key_translation apple_swap_modifiers[] = {
 	{ }
 };
 
-static struct apple_key_translation *apple_find_translation(
-		struct apple_key_translation *table, u16 from)
+static const struct apple_key_translation *apple_find_translation(
+		const struct apple_key_translation *table, u16 from)
 {
-	struct apple_key_translation *trans;
+	const struct apple_key_translation *trans;
 
 	/* Look for the translation */
 	for (trans = table; trans->from; trans++)
@@ -153,7 +153,7 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 		struct hid_usage *usage, __s32 value)
 {
 	struct apple_sc *asc = hid_get_drvdata(hid);
-	struct apple_key_translation *trans;
+	const struct apple_key_translation *trans;
 
 	if (usage->code == KEY_FN) {
 		asc->fn_on = !!value;
@@ -274,7 +274,7 @@ static void apple_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 
 static void apple_setup_input(struct input_dev *input)
 {
-	struct apple_key_translation *trans;
+	const struct apple_key_translation *trans;
 
 	set_bit(KEY_NUMLOCK, input->keybit);
 
-- 
1.6.2

Re: [PATCH 2/2] hid/apple: constify arrays of struct apple_key_translation and adjust users

[Jiri Kosina]

On Sat, 7 Mar 2009, Paul Collins wrote:

> Mark arrays of struct apple_key_translation const so that they may be placed in
> .rodata, and adjust users to suit.
> 
> Signed-off-by: Paul Collins <paul@ondioline.org>

Applied, thanks Paul.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 1/2] hid/apple: add module parameter to swap Command and Option keys

[Dmitry Torokhov]

Hi Paul,

On Sat, Mar 07, 2009 at 05:07:14PM +1300, Paul Collins wrote:
> Apple keyboards have the Command keys (a.k.a. the Apple or "waffle" key)
> in the positions normally occupied on PC keyboards by the Alt/AltGr keys,
> and the Option or Alt keys in the position occupied by the so-called
> Windows keys.  Folks who have been using PC-type computers for too long
> generally have insurmountable muscle memory in this regard.
> 
> This patch adds a module parameter, defaulting to off, that swaps these keys.
> 
> The same effect can also be achieved by changing the console and X
> keymaps, but this approach does not scale.  For example, I don't want to
> have to learn how to reconfigure Wayland's keymaps when the future
> arrives and we all start using it, and there may be applications I don't
> know about that also read keyboard events directly.
> 

We have a mechanism to alter in-kernel "scancode" to mapping from
userspace by issuing EVIOCSKEYCODE ioctl and there are a few utilities
written, in addition to HAL using it. So the best way I think is to
simly add an optional HAL policy.

-- 
Dmitry

Re: [PATCH 1/2] hid/apple: add module parameter to swap Command and Option keys

[Jiri Kosina]

On Sat, 7 Mar 2009, Paul Collins wrote:

> Apple keyboards have the Command keys (a.k.a. the Apple or "waffle" key) 
> in the positions normally occupied on PC keyboards by the Alt/AltGr 
> keys, and the Option or Alt keys in the position occupied by the 
> so-called Windows keys.  Folks who have been using PC-type computers for 
> too long generally have insurmountable muscle memory in this regard.
> This patch adds a module parameter, defaulting to off, that swaps these keys.

Hi Paul,

as this looks like "usability" thing (i.e. nothing that would be implied 
by the actual hardware design), wouldn't it make more sense to do the 
remapping from userspace using EVIOCSKEYCODE ioctl()? HID devices support 
it for quite some time already.

There are various tools to do this in a user-friendly way. Keytouch, to 
pick one -- http://keytouch.sourceforge.net/

-- 
Jiri Kosina
SUSE Labs