[PATCH 0/1] dell-laptop: Only install the i8042 filter when rfkill is active

[PATCH 0/1] dell-laptop: Only install the i8042 filter when rfkill is active

[Hans de Goede]

Hi Matthew,

https://bugzilla.redhat.com/show_bug.cgi?id=1045807 has made me take a closer
look at the dell-laptop.c code-paths on devices which are not on the rfkill
whitelist. Compared to 3.12 there is one change on these devices, the
i8042 filter gets installed independently of the whitelist, and if a
wireless keycode is detected by the filter a dell_send_request(buf, 17, 11)
will be done to query the rfkill settings.

Although unlikely to be the cause of rhbz#1045807, or to cause any problems
(the old rfkill code had the same issue on blacklisted devices), it seems
better to change the code so that there are no functional differences at all
on non white-listed models. This patch does this, can you send it to Linus
for the 3.13 ?

Thanks & Regards,

Hans

[PATCH] dell-laptop: Only install the i8042 filter when rfkill is active

[Hans de Goede]

Installing the i8042 filter is not useful on machines where rfkill is not
whitelisted, so move the filter installation into dell_setup_rfkill,
after the whitelist check.

This avoids doing a needless and potentially troublesome rfkill query
(dell_send_request(buf, 17, 11)) when the wireless Fn key gets pressed on
non whitelisted laptops.

This patch was written as a result of:
https://bugzilla.redhat.com/show_bug.cgi?id=1045807
It is not yet clear if this is related, but it is a good idea to not register
the i8042 filter in general.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/dell-laptop.c | 63 +++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index c608b1d..49a4f37 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -559,6 +559,29 @@ static void dell_update_rfkill(struct work_struct *ignored)
 }
 static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
 
+static bool dell_laptop_i8042_filter(unsigned char data, unsigned char str,
+			      struct serio *port)
+{
+	static bool extended;
+
+	if (str & 0x20)
+		return false;
+
+	if (unlikely(data == 0xe0)) {
+		extended = true;
+		return false;
+	} else if (unlikely(extended)) {
+		switch (data) {
+		case 0x8:
+			schedule_delayed_work(&dell_rfkill_work,
+					      round_jiffies_relative(HZ / 4));
+			break;
+		}
+		extended = false;
+	}
+
+	return false;
+}
 
 static int __init dell_setup_rfkill(void)
 {
@@ -633,7 +656,16 @@ static int __init dell_setup_rfkill(void)
 			goto err_wwan;
 	}
 
+	ret = i8042_install_filter(dell_laptop_i8042_filter);
+	if (ret) {
+		pr_warn("Unable to install key filter\n");
+		goto err_filter;
+	}
+
 	return 0;
+err_filter:
+	if (wwan_rfkill)
+		rfkill_unregister(wwan_rfkill);
 err_wwan:
 	rfkill_destroy(wwan_rfkill);
 	if (bluetooth_rfkill)
@@ -755,30 +787,6 @@ static void touchpad_led_exit(void)
 	led_classdev_unregister(&touchpad_led);
 }
 
-static bool dell_laptop_i8042_filter(unsigned char data, unsigned char str,
-			      struct serio *port)
-{
-	static bool extended;
-
-	if (str & 0x20)
-		return false;
-
-	if (unlikely(data == 0xe0)) {
-		extended = true;
-		return false;
-	} else if (unlikely(extended)) {
-		switch (data) {
-		case 0x8:
-			schedule_delayed_work(&dell_rfkill_work,
-					      round_jiffies_relative(HZ / 4));
-			break;
-		}
-		extended = false;
-	}
-
-	return false;
-}
-
 static int __init dell_init(void)
 {
 	int max_intensity = 0;
@@ -828,12 +836,6 @@ static int __init dell_init(void)
 		goto fail_rfkill;
 	}
 
-	ret = i8042_install_filter(dell_laptop_i8042_filter);
-	if (ret) {
-		pr_warn("Unable to install key filter\n");
-		goto fail_filter;
-	}
-
 	if (quirks && quirks->touchpad_led)
 		touchpad_led_init(&platform_device->dev);
 
@@ -885,7 +887,6 @@ static int __init dell_init(void)
 fail_backlight:
 	i8042_remove_filter(dell_laptop_i8042_filter);
 	cancel_delayed_work_sync(&dell_rfkill_work);
-fail_filter:
 	dell_cleanup_rfkill();
 fail_rfkill:
 	free_page((unsigned long)bufferpage);
-- 
1.8.4.2

Re: [PATCH] dell-laptop: Only install the i8042 filter when rfkill is active

[Hans de Goede]

Hi,

On 12/24/2013 08:34 PM, Hans de Goede wrote:
> Installing the i8042 filter is not useful on machines where rfkill is not
> whitelisted, so move the filter installation into dell_setup_rfkill,
> after the whitelist check.

p.s.

Note the cleanup paths are not changed, I did not bother with tracking
if we have installed the filter and only calling i8042_remove_filter if
we have, because i8042_remove_filter is a nop of the passed in filter
is not installed.