Fujitsu Amilo PA 1510 key-release events quirk

PATCH In review Daniel Mierswa 2 participants 8 messages Nov 29, 2008 – Dec 3, 2008

Subsystems (2)

HID Input drivers

lore

Share & tools

lore

Daniel Mierswa

author

2008-11-29 22:08 UTC #0 lore raw

PATCH 1/1 Fujitsu Amilo PA 1510 key-release events quirk

heya,
the fujitsu amilo pa 1510 laptop needs a little lovin' regarding the release event of the volume up and down keys. I attached a format-patch (against torvalds/linux-2.6.git ed313489badef16d7) which separates the generation of release events for certain scancodes into a macro since 3 dmi matches are already using it and generation of the missing release events for that particular laptop. Please post back with any advice and thoughts for that patch.

Thanks in advance.

Signature · 4 lines

Mierswa, Daniel

If you still don't like it, that's ok: that's why I'm boss. I simply know better than you do.
              --- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22

Quoted 1 line

From a197dfb0eed23b87418c0a7fe6a562910046576e Mon Sep 17 00:00:00 2001

From: Daniel Mierswa <[email protected]>
Date: Sat, 29 Nov 2008 22:39:34 +0100
Subject: [PATCH] Separate macro for generation of keyrelease events

---
 drivers/input/keyboard/atkbd.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

drivers/input/keyboard/atkbd.c +13 ~~-10~~

index 22016ca..99ef522 100644
@@ -834,6 +834,17 @@ static void atkbd_disconnect(struct serio *serio)
 }
 
 /*
+ * generate release events for the keycodes given in forced_release_keys[]
+ */
+#define GEN_RELEASE_EVENT \
+	int i; \
+\
+	if (atkbd->set == 2) \
+		for (i = 0; i < ARRAY_SIZE(forced_release_keys); i++) \
+			__set_bit(forced_release_keys[i], \
+				atkbd->force_release_mask);
+
+/*
  * Most special keys (Fn+F?) on Dell laptops do not generate release
  * events so we have to do it ourselves.
  */

@@ -842,12 +853,8 @@ static void atkbd_dell_laptop_keymap_fixup(struct atkbd *atkbd)
 	const unsigned int forced_release_keys[] = {
 		0x85, 0x86, 0x87, 0x88, 0x89, 0x8a, 0x8b, 0x8f, 0x93,
 	};
-	int i;
 
-	if (atkbd->set == 2)
-		for (i = 0; i < ARRAY_SIZE(forced_release_keys); i++)
-			__set_bit(forced_release_keys[i],
-				  atkbd->force_release_mask);
+	GEN_RELEASE_EVENT
 }
 
 /*

@@ -859,12 +866,8 @@ static void atkbd_hp_keymap_fixup(struct atkbd *atkbd)
 	const unsigned int forced_release_keys[] = {
 		0x94,
 	};
-	int i;
 
-	if (atkbd->set == 2)
-		for (i = 0; i < ARRAY_SIZE(forced_release_keys); i++)
-			__set_bit(forced_release_keys[i],
-					atkbd->force_release_mask);
+	GEN_RELEASE_EVENT
 }
 
 /*

Signature · 1 line

1.6.0.4

Quoted 1 line

From add1de59a8337442154fb3d7427ae8fd500de47a Mon Sep 17 00:00:00 2001

From: Daniel Mierswa <[email protected]>
Date: Sat, 29 Nov 2008 22:40:20 +0100
Subject: [PATCH] Fujitsu Siemens Amilo PA 1510 quirks

The volume up and down keys on the Fujitsu Siemens Amilo PA 1510 laptop won't generate release events, so we have to do that. Use the same way that is already used with other models.

---
 drivers/input/keyboard/atkbd.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

drivers/input/keyboard/atkbd.c +22 -0

index 99ef522..1250242 100644
@@ -845,6 +845,19 @@ static void atkbd_disconnect(struct serio *serio)
 				atkbd->force_release_mask);
 
 /*
+ * The volume up and volume down special keys on a Fujitsu Amilo PA 1510 laptop
+ * do not generate release events so we have to do it ourselves.
+ */
+static void atkbd_amilopa1510_keymap_fixup(struct atkbd *atkbd)
+{
+	const unsigned int forced_release_keys[] = {
+		0xb0, 0xae,
+	};
+
+	GEN_RELEASE_EVENT
+}
+
+/*
  * Most special keys (Fn+F?) on Dell laptops do not generate release
  * events so we have to do it ourselves.
  */

@@ -1463,6 +1476,15 @@ static struct dmi_system_id atkbd_dmi_quirk_table[] __initdata = {
 		.driver_data = atkbd_dell_laptop_keymap_fixup,
 	},
 	{
+		.ident = "Fujitsu Amilo PA 1510",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"), 
+			DMI_MATCH(DMI_PRODUCT_NAME, "AMILO Pa 1510"),
+		},
+		.callback = atkbd_setup_fixup,
+		.driver_data = atkbd_amilopa1510_keymap_fixup,
+	},
+	{
 		.ident = "HP 2133",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),

Signature · 1 line

1.6.0.4

Andrew Morton

2008-11-30 07:38 UTC #1 lore raw

Quoted 9 lines · On Sat, 29 Nov 2008 22:59:36 +0100 Daniel Mierswa <[email protected]> wrote:

heya,
the fujitsu amilo pa 1510 laptop needs a little lovin' regarding the release event of the volume up and down keys. I attached a format-patch (against torvalds/linux-2.6.git ed313489badef16d7) which separates the generation of release events for certain scancodes into a macro since 3 dmi matches are already using it and generation of the missing release events for that particular laptop. Please post back with any advice and thoughts for that patch.

Please don't send more than one patch per email. I shall comment on the two patches in two separate replies.

Quoted 8 lines

From a197dfb0eed23b87418c0a7fe6a562910046576e Mon Sep 17 00:00:00 2001
From: Daniel Mierswa <[email protected]>
Date: Sat, 29 Nov 2008 22:39:34 +0100
Subject: [PATCH] Separate macro for generation of keyrelease events
---
 drivers/input/keyboard/atkbd.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

Please also cc [email protected] on input-related patches.

Quoted 38 lines

drivers/input/keyboard/atkbd.c +12 -5

index 22016ca..99ef522 100644
@@ -834,6 +834,17 @@ static void atkbd_disconnect(struct serio *serio)
 }
 
 /*
+ * generate release events for the keycodes given in forced_release_keys[]
+ */
+#define GEN_RELEASE_EVENT \
+	int i; \
+\
+	if (atkbd->set == 2) \
+		for (i = 0; i < ARRAY_SIZE(forced_release_keys); i++) \
+			__set_bit(forced_release_keys[i], \
+				atkbd->force_release_mask);
+
+/*
  * Most special keys (Fn+F?) on Dell laptops do not generate release
  * events so we have to do it ourselves.
  */

@@ -842,12 +853,8 @@ static void atkbd_dell_laptop_keymap_fixup(struct atkbd *atkbd)
 	const unsigned int forced_release_keys[] = {
 		0x85, 0x86, 0x87, 0x88, 0x89, 0x8a, 0x8b, 0x8f, 0x93,
 	};
-	int i;
 
-	if (atkbd->set == 2)
-		for (i = 0; i < ARRAY_SIZE(forced_release_keys); i++)
-			__set_bit(forced_release_keys[i],
-				  atkbd->force_release_mask);
+	GEN_RELEASE_EVENT
 }
 
 /*

@@ -859,12 +866,8 @@ static void atkbd_hp_keymap_fixup(struct atkbd *atkbd)
 	const unsigned int forced_release_keys[] = {

drivers/input/keyboard/atkbd.c static void atkbd_hp_keymap_fixup(struct atkbd *atkbd) · line 871

Quoted 10 lines

drivers/input/keyboard/atkbd.c +1 -5

 		0x94,
 	};
-	int i;
 
-	if (atkbd->set == 2)
-		for (i = 0; i < ARRAY_SIZE(forced_release_keys); i++)
-			__set_bit(forced_release_keys[i],
-					atkbd->force_release_mask);
+	GEN_RELEASE_EVENT
 }

oh my. Please, no. Just write a C function:

void atkbd_gen_release_event(unsigned int *keys, unsigned nr_keys)

{
	...
}

Andrew Morton

2008-11-30 07:42 UTC #2 lore raw

Quoted 3 lines · On Sat, 29 Nov 2008 23:38:26 -0800 Andrew Morton <[email protected]>…

Just write a C function:
void atkbd_gen_release_event(unsigned int *keys, unsigned nr_keys)

^ const

Quoted 5 lines

{
	...
}

Daniel Mierswa

author

2008-12-01 20:59 UTC #3 lore raw

Re: [PATCH] Fujitsu Amilo PA 1510 key-release events quirk (PATCH 1/3)

In reply to Andrew Morton

Split patches as requested in "[PATCH] Fujitsu Amilo PA 1510 key-release events quirk"
Here's the first out of 3.

Quoted 3 lines · Andrew Morton

Please don't send more than one patch per email. I shall comment on the two patches in two separate replies.

done, except that they are three now :)

Quoted 3 lines · Andrew Morton

Please also cc [email protected] on input-related patches.

done

Quoted 8 lines · Andrew Morton

oh my. Please, no. Just write a C function:
void atkbd_gen_release_event(unsigned int *keys, unsigned nr_keys)
{
	...
}

done, I prefer that myself, although I first thought it might be a bad idea to pass around pointers there, since
other developers might expect ARRAY_SIZE to work in gen_release_events.

Signature · 4 lines

Mierswa, Daniel

If you still don't like it, that's ok: that's why I'm boss. I simply know better than you do.
               --- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22

Quoted 1 line

From f4889ca0b6895abdd1888cb1b56dd411045780be Mon Sep 17 00:00:00 2001

From: Daniel Mierswa <[email protected]>
Date: Sun, 30 Nov 2008 13:14:00 +0100
Subject: [PATCH] use function for generation of keyrelease events

---
 drivers/input/keyboard/atkbd.c |   27 +++++++++++++++++----------
 1 files changed, 17 insertions(+), 10 deletions(-)

drivers/input/keyboard/atkbd.c +17 ~~-10~~

index 22016ca..c239745 100644
@@ -834,6 +834,19 @@ static void atkbd_disconnect(struct serio *serio)
 }
 
 /*
+ * generate release events for the keycodes given in keys[]
+ */
+static void atkbd_gen_release_event(struct atkbd* atkbd, const unsigned int *keys,
+			unsigned int nr_keys)
+{
+	unsigned int i;
+
+	if (atkbd->set == 2)
+		for (i = 0; i < nr_keys; i++)
+			__set_bit(keys[i], atkbd->force_release_mask);
+}
+
+/*
  * Most special keys (Fn+F?) on Dell laptops do not generate release
  * events so we have to do it ourselves.
  */

@@ -842,12 +855,9 @@ static void atkbd_dell_laptop_keymap_fixup(struct atkbd *atkbd)
 	const unsigned int forced_release_keys[] = {
 		0x85, 0x86, 0x87, 0x88, 0x89, 0x8a, 0x8b, 0x8f, 0x93,
 	};
-	int i;
 
-	if (atkbd->set == 2)
-		for (i = 0; i < ARRAY_SIZE(forced_release_keys); i++)
-			__set_bit(forced_release_keys[i],
-				  atkbd->force_release_mask);
+	atkbd_gen_release_event(atkbd, forced_release_keys,
+		ARRAY_SIZE(forced_release_keys));
 }
 
 /*

@@ -859,12 +869,9 @@ static void atkbd_hp_keymap_fixup(struct atkbd *atkbd)
 	const unsigned int forced_release_keys[] = {
 		0x94,
 	};
-	int i;
 
-	if (atkbd->set == 2)
-		for (i = 0; i < ARRAY_SIZE(forced_release_keys); i++)
-			__set_bit(forced_release_keys[i],
-					atkbd->force_release_mask);
+	atkbd_gen_release_event(atkbd, forced_release_keys,
+		ARRAY_SIZE(forced_release_keys));
 }
 
 /*

Signature · 1 line

1.6.0.4

Daniel Mierswa

author

2008-12-01 21:00 UTC #4 lore raw

Re: [PATCH] Fujitsu Amilo PA 1510 key-release events quirk (PATCH 2/3)

In reply to Andrew Morton

Split patches as requested in "[PATCH] Fujitsu Amilo PA 1510 key-release events quirk"

Quoted 2 lines · Andrew Morton

It's best to make forced_release_keys static as well - there is no need to build this array on the stack at runtime

done.

Signature · 4 lines

Mierswa, Daniel

If you still don't like it, that's ok: that's why I'm boss. I simply know better than you do.
               --- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22

Quoted 1 line

From 3b4eeb11c3dc09005fdd10d3fb22fb2267fd0d39 Mon Sep 17 00:00:00 2001

From: Daniel Mierswa <[email protected]>
Date: Sun, 30 Nov 2008 13:14:45 +0100
Subject: [PATCH] make forced_release_keys static

---
 drivers/input/keyboard/atkbd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/input/keyboard/atkbd.c +2 -2

index c239745..11ded7e 100644
@@ -852,7 +852,7 @@ static void atkbd_gen_release_event(struct atkbd* atkbd, const unsigned int *key
  */
 static void atkbd_dell_laptop_keymap_fixup(struct atkbd *atkbd)
 {
-	const unsigned int forced_release_keys[] = {
+	static const unsigned int forced_release_keys[] = {
 		0x85, 0x86, 0x87, 0x88, 0x89, 0x8a, 0x8b, 0x8f, 0x93,
 	};

@@ -866,7 +866,7 @@ static void atkbd_dell_laptop_keymap_fixup(struct atkbd *atkbd)
  */
 static void atkbd_hp_keymap_fixup(struct atkbd *atkbd)
 {
-	const unsigned int forced_release_keys[] = {
+	static const unsigned int forced_release_keys[] = {
 		0x94,
 	};

Signature · 1 line

1.6.0.4

Daniel Mierswa

author

2008-12-01 21:01 UTC #5 lore raw

Re: [PATCH] Fujitsu Amilo PA 1510 key-release events quirk (PATCH 3/3)

In reply to Andrew Morton

Split patches as requested in "[PATCH] Fujitsu Amilo PA 1510 key-release events quirk"

Signature · 4 lines

Mierswa, Daniel

If you still don't like it, that's ok: that's why I'm boss. I simply know better than you do.
               --- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22

Quoted 1 line

From 1a786f4d720ad0dbaf4b8a03a5237b8433753a4d Mon Sep 17 00:00:00 2001

From: Daniel Mierswa <[email protected]>
Date: Sun, 30 Nov 2008 13:17:01 +0100
Subject: [PATCH] Fujitsu Siemens Amilo PA 1510 quirks

The volume up and down keys on the Fujitsu Siemens Amilo PA 1510 laptop won't generate release events, so we have to do that. Use the same way that is already used with other models.

---
 drivers/input/keyboard/atkbd.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

drivers/input/keyboard/atkbd.c +23 -0

index 11ded7e..7347cd4 100644
@@ -875,6 +875,20 @@ static void atkbd_hp_keymap_fixup(struct atkbd *atkbd)
 }
 
 /*
+ * The volume up and volume down special keys on a Fujitsu Amilo PA 1510 laptop
+ * do not generate release events so we have to do it ourselves.
+ */
+static void atkbd_amilopa1510_keymap_fixup(struct atkbd *atkbd)
+{
+	static const unsigned int forced_release_keys[] = {
+		0xb0, 0xae,
+	};
+
+	atkbd_gen_release_event(atkbd, forced_release_keys,
+		ARRAY_SIZE(forced_release_keys));
+}
+
+/*
  * atkbd_set_keycode_table() initializes keyboard's keycode table
  * according to the selected scancode set
  */

@@ -1475,6 +1489,15 @@ static struct dmi_system_id atkbd_dmi_quirk_table[] __initdata = {
 		.callback = atkbd_setup_fixup,
 		.driver_data = atkbd_hp_keymap_fixup,
 	},
+	{
+		.ident = "Fujitsu Amilo PA 1510",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "AMILO Pa 1510"),
+		},
+		.callback = atkbd_setup_fixup,
+		.driver_data = atkbd_amilopa1510_keymap_fixup,
+	},
 	{ }
 };

Signature · 1 line

1.6.0.4

Andrew Morton

2008-12-03 06:11 UTC #6 lore raw

Re: [PATCH] Fujitsu Amilo PA 1510 key-release events quirk (PATCH 3/3)

Quoted 2 lines · On Mon, 01 Dec 2008 22:01:26 +0100 Daniel Mierswa <[email protected]> wrote:

Split patches as requested in "[PATCH] Fujitsu Amilo PA 1510 key-release events quirk"

Ho hum. Please see Documentation/SubmittingPatches - I think you managed to get everything in there wrong ;)

I unpickled the patches. Please send a Signed-off-by: for this work.

Andrew Morton

2008-11-30 07:42 UTC #7 lore raw

In reply to Daniel Mierswa

Quoted 20 lines · On Sat, 29 Nov 2008 22:59:36 +0100 Daniel Mierswa <[email protected]> wrote:

From: Daniel Mierswa <[email protected]>
Date: Sat, 29 Nov 2008 22:40:20 +0100
Subject: [PATCH] Fujitsu Siemens Amilo PA 1510 quirks
The volume up and down keys on the Fujitsu Siemens Amilo PA 1510 laptop won't generate release events, so we have to do that. Use the same way that is already used with other models.
---
 drivers/input/keyboard/atkbd.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)
drivers/input/keyboard/atkbd.c +1 -0
index 99ef522..1250242 100644
@@ -845,6 +845,19 @@ static void atkbd_disconnect(struct serio *serio)
 				atkbd->force_release_mask);
 
 /*
+ * The volume up and volume down special keys on a Fujitsu Amilo PA 1510 laptop

drivers/input/keyboard/atkbd.c static void atkbd_disconnect(struct serio *serio) · line 858

Quoted 10 lines

drivers/input/keyboard/atkbd.c +10 -0

+ * do not generate release events so we have to do it ourselves.
+ */
+static void atkbd_amilopa1510_keymap_fixup(struct atkbd *atkbd)
+{
+	const unsigned int forced_release_keys[] = {
+		0xb0, 0xae,
+	};
+
+	GEN_RELEASE_EVENT
+}

It's best to make forced_release_keys static as well - there is no need to build this array on the stack at runtime. The compiler _could_ perform this optimisation itself andallegedly it will sometimes do so. Last time I checked it did not.

Quoted 20 lines

drivers/input/keyboard/atkbd.c +10 -0

+/*
  * Most special keys (Fn+F?) on Dell laptops do not generate release
  * events so we have to do it ourselves.
  */

@@ -1463,6 +1476,15 @@ static struct dmi_system_id atkbd_dmi_quirk_table[] __initdata = {
 		.driver_data = atkbd_dell_laptop_keymap_fixup,
 	},
 	{
+		.ident = "Fujitsu Amilo PA 1510",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"), 
+			DMI_MATCH(DMI_PRODUCT_NAME, "AMILO Pa 1510"),
+		},
+		.callback = atkbd_setup_fixup,
+		.driver_data = atkbd_amilopa1510_keymap_fixup,
+	},
+	{
 		.ident = "HP 2133",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),