Skip to contentSkip to conversation

eeepc-laptop: Fix user after free

PATCH Superseded Matthew Garrett 5 participants 18 messages Aug 4, 2008 – Oct 8, 2008

Subsystems (2)
ACPI Char and misc drivers
lore
Share & tools
lore

Matthew Garrett

author
#0 lore raw

PATCH 1/2 eeepc-laptop: Fix user after free

eeepc-laptop uses the hwmon struct after unregistering the device, causing an oops on module unload. Flip the ordering to fix.

Signed-off-by Matthew Garrett <[email protected]>

---

drivers/misc/eeepc-laptop.c +1 -1
index 9e8d79e..facdb98 100644
@@ -553,9 +553,9 @@ static void eeepc_hwmon_exit(void)
 	hwmon = eeepc_hwmon_device;
 	if (!hwmon)
 		return ;
-	hwmon_device_unregister(hwmon);
 	sysfs_remove_group(&hwmon->kobj,
 			   &hwmon_attribute_group);
+	hwmon_device_unregister(hwmon);
 	eeepc_hwmon_device = NULL;
 }
Signature · 1 line
Matthew Garrett | [email protected]
author
#1 lore raw

PATCH 2/2 eeepc-laptop: Use standard interfaces

eeepc-laptop currently only sends key events via ACPI and has non-standard rfkill control. Add an input device and use the rfkill infrastructure.

Signed-off-by Matthew Garrett <[email protected]>

---

This attempts to ensure that bluetoth and wlan rfkill devices are only created if the device is prseent, but I don't have a Bluetooth-enabled Eee to hand so I'm not certain it's correct. Testing with rfkill-input shows that the wifi interface works, though.

commit 0656cf909274db0e59bb570c2bddd242cf075e7f

Author: Matthew Garrett <[email protected]>
Date:   Mon Aug 4 18:00:57 2008 +0100

Rationalise interfacse

drivers/misc/eeepc-laptop.c +187 -19
index facdb98..ad55e60 100644
@@ -28,6 +28,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/acpi_bus.h>
 #include <linux/uaccess.h>
+#include <linux/input.h>
+#include <linux/rfkill.h>
 
 #define EEEPC_LAPTOP_VERSION	"0.1"
 

@@ -125,6 +127,10 @@ struct eeepc_hotk {
 					   by this BIOS */
 	uint init_flag;			/* Init flags */
 	u16 event_count[128];		/* count for each event */
+	struct input_dev *inputdev;
+	u16 *keycode_map;
+	struct rfkill *eeepc_wlan_rfkill;
+	struct rfkill *eeepc_bluetooth_rfkill;
 };
 
 /* The actual device the driver binds to */

@@ -140,6 +146,27 @@ static struct platform_driver platform_driver = {
 
 static struct platform_device *platform_device;
 
+struct key_entry {
+	char type;
+	u8 code;
+	u16 keycode;
+};
+
+enum { KE_KEY, KE_END };
+
+static struct key_entry eeepc_keymap[] = {
+	/* Sleep already handled via generic ACPI code */
+	{KE_KEY, 0x10, KEY_WLAN },
+	{KE_KEY, 0x12, KEY_PROG1 },
+	{KE_KEY, 0x13, KEY_MUTE },
+	{KE_KEY, 0x14, KEY_VOLUMEDOWN },
+	{KE_KEY, 0x15, KEY_VOLUMEUP },
+	{KE_KEY, 0x30, KEY_SWITCHVIDEOMODE },
+	{KE_KEY, 0x31, KEY_SWITCHVIDEOMODE },
+	{KE_KEY, 0x32, KEY_SWITCHVIDEOMODE },
+	{KE_END, 0},
+};
+
 /*
  * The hotkey driver declaration
  */

@@ -261,6 +288,32 @@ static int update_bl_status(struct backlight_device *bd)
 }
 
 /*
+ * Rfkill helpers
+ */
+
+static int eeepc_wlan_rfkill_set(void *data, enum rfkill_state state)
+{
+	return set_acpi(CM_ASL_WLAN, state);
+}
+
+static int eeepc_wlan_rfkill_state(void *data, enum rfkill_state *state)
+{
+	*state = get_acpi(CM_ASL_WLAN);
+	return 0;
+}
+
+static int eeepc_bluetooth_rfkill_set(void *data, enum rfkill_state state)
+{
+	return set_acpi(CM_ASL_BLUETOOTH, state);
+}
+
+static int eeepc_bluetooth_rfkill_state(void *data, enum rfkill_state *state)
+{
+	*state = get_acpi(CM_ASL_BLUETOOTH);
+	return 0;
+}
+
+/*
  * Sys helpers
  */
 static int parse_arg(const char *buf, unsigned long count, int *val)

@@ -311,13 +364,11 @@ static ssize_t show_sys_acpi(int cm, char *buf)
 EEEPC_CREATE_DEVICE_ATTR(camera, CM_ASL_CAMERA);
 EEEPC_CREATE_DEVICE_ATTR(cardr, CM_ASL_CARDREADER);
 EEEPC_CREATE_DEVICE_ATTR(disp, CM_ASL_DISPLAYSWITCH);
-EEEPC_CREATE_DEVICE_ATTR(wlan, CM_ASL_WLAN);
 
 static struct attribute *platform_attributes[] = {
 	&dev_attr_camera.attr,
 	&dev_attr_cardr.attr,
 	&dev_attr_disp.attr,
-	&dev_attr_wlan.attr,
 	NULL
 };
 

@@ -328,8 +379,64 @@ static struct attribute_group platform_attribute_group = {
 /*
  * Hotkey functions
  */
+static struct key_entry *eepc_get_entry_by_scancode(int code)
+{
+	struct key_entry *key;
+
+	for (key = eeepc_keymap; key->type != KE_END; key++)
+		if (code == key->code)
+			return key;
+
+	return NULL;
+}
+
+static struct key_entry *eepc_get_entry_by_keycode(int code)
+{
+	struct key_entry *key;
+
+	for (key = eeepc_keymap; key->type != KE_END; key++)
+		if (code == key->keycode && key->type == KE_KEY)
+			return key;
+
+	return NULL;
+}
+
+static int eeepc_getkeycode(struct input_dev *dev, int scancode, int *keycode)
+{
+	struct key_entry *key = eepc_get_entry_by_scancode(scancode);
+
+	if (key && key->type == KE_KEY) {
+		*keycode = key->keycode;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int eeepc_setkeycode(struct input_dev *dev, int scancode, int keycode)
+{
+	struct key_entry *key;
+	int old_keycode;
+
+	if (keycode < 0 || keycode > KEY_MAX)
+		return -EINVAL;
+
+	key = eepc_get_entry_by_scancode(scancode);
+	if (key && key->type == KE_KEY) {
+		old_keycode = key->keycode;
+		key->keycode = keycode;
+		set_bit(keycode, dev->keybit);
+		if (!eepc_get_entry_by_keycode(old_keycode))
+			clear_bit(old_keycode, dev->keybit);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static int eeepc_hotk_check(void)
 {
+	const struct key_entry *key;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	int result;
 

@@ -356,6 +463,31 @@ static int eeepc_hotk_check(void)
 			       "Get control methods supported: 0x%x\n",
 			       ehotk->cm_supported);
 		}
+		ehotk->inputdev = input_allocate_device();
+		if (!ehotk->inputdev) {
+			printk(EEEPC_INFO "Unable to allocate input device\n");
+			return 0;
+		}
+		ehotk->inputdev->name = "Asus EeePC extra buttons";
+		ehotk->inputdev->phys = EEEPC_HOTK_FILE "/input0";
+		ehotk->inputdev->id.bustype = BUS_HOST;
+		ehotk->inputdev->getkeycode = eeepc_getkeycode;
+		ehotk->inputdev->setkeycode = eeepc_setkeycode;
+
+		for (key = eeepc_keymap; key->type != KE_END; key++) {
+			switch (key->type) {
+			case KE_KEY:
+				set_bit(EV_KEY, ehotk->inputdev->evbit);
+				set_bit(key->keycode, ehotk->inputdev->keybit);
+				break;
+			}
+		}
+		result = input_register_device(ehotk->inputdev);
+		if (result) {
+			printk(EEEPC_INFO "Unable to register input device\n");
+			input_free_device(ehotk->inputdev);
+			return 0;
+		}
 	} else {
 		printk(EEEPC_ERR "Hotkey device not present, aborting\n");
 		return -EINVAL;

@@ -363,21 +495,6 @@ static int eeepc_hotk_check(void)
 	return 0;
 }
 
-static void notify_wlan(u32 *event)
-{
-	/* if DISABLE_ASL_WLAN is set, the notify code for fn+f2
-	   will always be 0x10 */
-	if (ehotk->cm_supported & (0x1 << CM_ASL_WLAN)) {
-		const char *method = cm_getv[CM_ASL_WLAN];
-		int value;
-		if (read_acpi_int(ehotk->handle, method, &value))
-			printk(EEEPC_WARNING "Error reading %s\n",
-			       method);
-		else if (value == 1)
-			*event = 0x11;
-	}
-}
-
 static void notify_brn(void)
 {
 	struct backlight_device *bd = eeepc_backlight_device;

@@ -386,14 +503,28 @@ static void notify_brn(void)
 
 static void eeepc_hotk_notify(acpi_handle handle, u32 event, void *data)
 {
+	static struct key_entry *key;
 	if (!ehotk)
 		return;
-	if (event == NOTIFY_WLAN_ON && (DISABLE_ASL_WLAN & ehotk->init_flag))
-		notify_wlan(&event);
 	if (event >= NOTIFY_BRN_MIN && event <= NOTIFY_BRN_MAX)
 		notify_brn();
 	acpi_bus_generate_proc_event(ehotk->device, event,
 				     ehotk->event_count[event % 128]++);
+	if (ehotk->inputdev) {
+		key = eepc_get_entry_by_scancode(event);
+		if (key) {
+			switch (key->type) {
+			case KE_KEY:
+				input_report_key(ehotk->inputdev, key->keycode,
+						 1);
+				input_sync(ehotk->inputdev);
+				input_report_key(ehotk->inputdev, key->keycode,
+						 0);
+				input_sync(ehotk->inputdev);
+				break;
+			}
+		}
+	}
 }
 
 static int eeepc_hotk_add(struct acpi_device *device)

@@ -420,6 +551,37 @@ static int eeepc_hotk_add(struct acpi_device *device)
 					     eeepc_hotk_notify, ehotk);
 	if (ACPI_FAILURE(status))
 		printk(EEEPC_ERR "Error installing notify handler\n");
+
+	if (get_acpi(CM_ASL_WLAN) != -1) {
+		ehotk->eeepc_wlan_rfkill = rfkill_allocate(&device->dev,
+							   RFKILL_TYPE_WLAN);
+
+		if (!ehotk->eeepc_wlan_rfkill)
+			goto end;
+
+		ehotk->eeepc_wlan_rfkill->name = "eeepc-wlan";
+		ehotk->eeepc_wlan_rfkill->toggle_radio = eeepc_wlan_rfkill_set;
+		ehotk->eeepc_wlan_rfkill->get_state = eeepc_wlan_rfkill_state;
+		ehotk->eeepc_wlan_rfkill->user_claim_unsupported = 0;
+		rfkill_register(ehotk->eeepc_wlan_rfkill);
+	}
+
+	if (get_acpi(CM_ASL_BLUETOOTH) != -1) {
+		ehotk->eeepc_bluetooth_rfkill =
+			rfkill_allocate(&device->dev, RFKILL_TYPE_BLUETOOTH);
+
+		if (!ehotk->eeepc_bluetooth_rfkill)
+			goto end;
+
+		ehotk->eeepc_bluetooth_rfkill->name = "eeepc-bluetooth";
+		ehotk->eeepc_bluetooth_rfkill->toggle_radio =
+			eeepc_bluetooth_rfkill_set;
+		ehotk->eeepc_bluetooth_rfkill->get_state =
+			eeepc_bluetooth_rfkill_state;
+		ehotk->eeepc_bluetooth_rfkill->user_claim_unsupported = 0;
+		rfkill_register(ehotk->eeepc_bluetooth_rfkill);
+	}
+
  end:
 	if (result) {
 		kfree(ehotk);

@@ -543,6 +705,12 @@ static void eeepc_backlight_exit(void)
 {
 	if (eeepc_backlight_device)
 		backlight_device_unregister(eeepc_backlight_device);
+	if (ehotk->inputdev)
+		input_unregister_device(ehotk->inputdev);
+	if (ehotk->eeepc_wlan_rfkill)
+		rfkill_unregister(ehotk->eeepc_wlan_rfkill);
+	if (ehotk->eeepc_bluetooth_rfkill)
+		rfkill_unregister(ehotk->eeepc_bluetooth_rfkill);
 	eeepc_backlight_device = NULL;
 }
Signature · 1 line
Matthew Garrett | [email protected]
#2 lore raw

Re: [PATCH 2/2] eeepc-laptop: Use standard interfaces

Quoted 5 lines · Matthew Garrett

eeepc-laptop currently only sends key events via ACPI and has non-standard rfkill control. Add an input device and use the rfkill infrastructure.

Signed-off-by Matthew Garrett <[email protected]>

Please use the rfkill_force_state() to report state changes, that will ensure that the events are immediately send to the rfkill layer.

Otherwise events will not be reported untill the next get_state() event.

Ivo

Quoted 315 lines

---

This attempts to ensure that bluetoth and wlan rfkill devices are only created if the device is prseent, but I don't have a Bluetooth-enabled Eee to hand so I'm not certain it's correct. Testing with rfkill-input shows that the wifi interface works, though.

commit 0656cf909274db0e59bb570c2bddd242cf075e7f

Author: Matthew Garrett <[email protected]>
Date:   Mon Aug 4 18:00:57 2008 +0100

Rationalise interfacse

drivers/misc/eeepc-laptop.c +187 -19
index facdb98..ad55e60 100644
@@ -28,6 +28,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/acpi_bus.h>
 #include <linux/uaccess.h>
+#include <linux/input.h>
+#include <linux/rfkill.h>
 
 #define EEEPC_LAPTOP_VERSION	"0.1"
 

@@ -125,6 +127,10 @@ struct eeepc_hotk {
 					   by this BIOS */
 	uint init_flag;			/* Init flags */
 	u16 event_count[128];		/* count for each event */
+	struct input_dev *inputdev;
+	u16 *keycode_map;
+	struct rfkill *eeepc_wlan_rfkill;
+	struct rfkill *eeepc_bluetooth_rfkill;
 };
 
 /* The actual device the driver binds to */

@@ -140,6 +146,27 @@ static struct platform_driver platform_driver = {
 
 static struct platform_device *platform_device;
 
+struct key_entry {
+	char type;
+	u8 code;
+	u16 keycode;
+};
+
+enum { KE_KEY, KE_END };
+
+static struct key_entry eeepc_keymap[] = {
+	/* Sleep already handled via generic ACPI code */
+	{KE_KEY, 0x10, KEY_WLAN },
+	{KE_KEY, 0x12, KEY_PROG1 },
+	{KE_KEY, 0x13, KEY_MUTE },
+	{KE_KEY, 0x14, KEY_VOLUMEDOWN },
+	{KE_KEY, 0x15, KEY_VOLUMEUP },
+	{KE_KEY, 0x30, KEY_SWITCHVIDEOMODE },
+	{KE_KEY, 0x31, KEY_SWITCHVIDEOMODE },
+	{KE_KEY, 0x32, KEY_SWITCHVIDEOMODE },
+	{KE_END, 0},
+};
+
 /*
  * The hotkey driver declaration
  */

@@ -261,6 +288,32 @@ static int update_bl_status(struct backlight_device *bd)
 }
 
 /*
+ * Rfkill helpers
+ */
+
+static int eeepc_wlan_rfkill_set(void *data, enum rfkill_state state)
+{
+	return set_acpi(CM_ASL_WLAN, state);
+}
+
+static int eeepc_wlan_rfkill_state(void *data, enum rfkill_state *state)
+{
+	*state = get_acpi(CM_ASL_WLAN);
+	return 0;
+}
+
+static int eeepc_bluetooth_rfkill_set(void *data, enum rfkill_state state)
+{
+	return set_acpi(CM_ASL_BLUETOOTH, state);
+}
+
+static int eeepc_bluetooth_rfkill_state(void *data, enum rfkill_state *state)
+{
+	*state = get_acpi(CM_ASL_BLUETOOTH);
+	return 0;
+}
+
+/*
  * Sys helpers
  */
 static int parse_arg(const char *buf, unsigned long count, int *val)

@@ -311,13 +364,11 @@ static ssize_t show_sys_acpi(int cm, char *buf)
 EEEPC_CREATE_DEVICE_ATTR(camera, CM_ASL_CAMERA);
 EEEPC_CREATE_DEVICE_ATTR(cardr, CM_ASL_CARDREADER);
 EEEPC_CREATE_DEVICE_ATTR(disp, CM_ASL_DISPLAYSWITCH);
-EEEPC_CREATE_DEVICE_ATTR(wlan, CM_ASL_WLAN);
 
 static struct attribute *platform_attributes[] = {
 	&dev_attr_camera.attr,
 	&dev_attr_cardr.attr,
 	&dev_attr_disp.attr,
-	&dev_attr_wlan.attr,
 	NULL
 };
 

@@ -328,8 +379,64 @@ static struct attribute_group platform_attribute_group = {
 /*
  * Hotkey functions
  */
+static struct key_entry *eepc_get_entry_by_scancode(int code)
+{
+	struct key_entry *key;
+
+	for (key = eeepc_keymap; key->type != KE_END; key++)
+		if (code == key->code)
+			return key;
+
+	return NULL;
+}
+
+static struct key_entry *eepc_get_entry_by_keycode(int code)
+{
+	struct key_entry *key;
+
+	for (key = eeepc_keymap; key->type != KE_END; key++)
+		if (code == key->keycode && key->type == KE_KEY)
+			return key;
+
+	return NULL;
+}
+
+static int eeepc_getkeycode(struct input_dev *dev, int scancode, int *keycode)
+{
+	struct key_entry *key = eepc_get_entry_by_scancode(scancode);
+
+	if (key && key->type == KE_KEY) {
+		*keycode = key->keycode;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int eeepc_setkeycode(struct input_dev *dev, int scancode, int keycode)
+{
+	struct key_entry *key;
+	int old_keycode;
+
+	if (keycode < 0 || keycode > KEY_MAX)
+		return -EINVAL;
+
+	key = eepc_get_entry_by_scancode(scancode);
+	if (key && key->type == KE_KEY) {
+		old_keycode = key->keycode;
+		key->keycode = keycode;
+		set_bit(keycode, dev->keybit);
+		if (!eepc_get_entry_by_keycode(old_keycode))
+			clear_bit(old_keycode, dev->keybit);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static int eeepc_hotk_check(void)
 {
+	const struct key_entry *key;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	int result;
 

@@ -356,6 +463,31 @@ static int eeepc_hotk_check(void)
 			       "Get control methods supported: 0x%x\n",
 			       ehotk->cm_supported);
 		}
+		ehotk->inputdev = input_allocate_device();
+		if (!ehotk->inputdev) {
+			printk(EEEPC_INFO "Unable to allocate input device\n");
+			return 0;
+		}
+		ehotk->inputdev->name = "Asus EeePC extra buttons";
+		ehotk->inputdev->phys = EEEPC_HOTK_FILE "/input0";
+		ehotk->inputdev->id.bustype = BUS_HOST;
+		ehotk->inputdev->getkeycode = eeepc_getkeycode;
+		ehotk->inputdev->setkeycode = eeepc_setkeycode;
+
+		for (key = eeepc_keymap; key->type != KE_END; key++) {
+			switch (key->type) {
+			case KE_KEY:
+				set_bit(EV_KEY, ehotk->inputdev->evbit);
+				set_bit(key->keycode, ehotk->inputdev->keybit);
+				break;
+			}
+		}
+		result = input_register_device(ehotk->inputdev);
+		if (result) {
+			printk(EEEPC_INFO "Unable to register input device\n");
+			input_free_device(ehotk->inputdev);
+			return 0;
+		}
 	} else {
 		printk(EEEPC_ERR "Hotkey device not present, aborting\n");
 		return -EINVAL;

@@ -363,21 +495,6 @@ static int eeepc_hotk_check(void)
 	return 0;
 }
 
-static void notify_wlan(u32 *event)
-{
-	/* if DISABLE_ASL_WLAN is set, the notify code for fn+f2
-	   will always be 0x10 */
-	if (ehotk->cm_supported & (0x1 << CM_ASL_WLAN)) {
-		const char *method = cm_getv[CM_ASL_WLAN];
-		int value;
-		if (read_acpi_int(ehotk->handle, method, &value))
-			printk(EEEPC_WARNING "Error reading %s\n",
-			       method);
-		else if (value == 1)
-			*event = 0x11;
-	}
-}
-
 static void notify_brn(void)
 {
 	struct backlight_device *bd = eeepc_backlight_device;

@@ -386,14 +503,28 @@ static void notify_brn(void)
 
 static void eeepc_hotk_notify(acpi_handle handle, u32 event, void *data)
 {
+	static struct key_entry *key;
 	if (!ehotk)
 		return;
-	if (event == NOTIFY_WLAN_ON && (DISABLE_ASL_WLAN & ehotk->init_flag))
-		notify_wlan(&event);
 	if (event >= NOTIFY_BRN_MIN && event <= NOTIFY_BRN_MAX)
 		notify_brn();
 	acpi_bus_generate_proc_event(ehotk->device, event,
 				     ehotk->event_count[event % 128]++);
+	if (ehotk->inputdev) {
+		key = eepc_get_entry_by_scancode(event);
+		if (key) {
+			switch (key->type) {
+			case KE_KEY:
+				input_report_key(ehotk->inputdev, key->keycode,
+						 1);
+				input_sync(ehotk->inputdev);
+				input_report_key(ehotk->inputdev, key->keycode,
+						 0);
+				input_sync(ehotk->inputdev);
+				break;
+			}
+		}
+	}
 }
 
 static int eeepc_hotk_add(struct acpi_device *device)

@@ -420,6 +551,37 @@ static int eeepc_hotk_add(struct acpi_device *device)
 					     eeepc_hotk_notify, ehotk);
 	if (ACPI_FAILURE(status))
 		printk(EEEPC_ERR "Error installing notify handler\n");
+
+	if (get_acpi(CM_ASL_WLAN) != -1) {
+		ehotk->eeepc_wlan_rfkill = rfkill_allocate(&device->dev,
+							   RFKILL_TYPE_WLAN);
+
+		if (!ehotk->eeepc_wlan_rfkill)
+			goto end;
+
+		ehotk->eeepc_wlan_rfkill->name = "eeepc-wlan";
+		ehotk->eeepc_wlan_rfkill->toggle_radio = eeepc_wlan_rfkill_set;
+		ehotk->eeepc_wlan_rfkill->get_state = eeepc_wlan_rfkill_state;
+		ehotk->eeepc_wlan_rfkill->user_claim_unsupported = 0;
+		rfkill_register(ehotk->eeepc_wlan_rfkill);
+	}
+
+	if (get_acpi(CM_ASL_BLUETOOTH) != -1) {
+		ehotk->eeepc_bluetooth_rfkill =
+			rfkill_allocate(&device->dev, RFKILL_TYPE_BLUETOOTH);
+
+		if (!ehotk->eeepc_bluetooth_rfkill)
+			goto end;
+
+		ehotk->eeepc_bluetooth_rfkill->name = "eeepc-bluetooth";
+		ehotk->eeepc_bluetooth_rfkill->toggle_radio =
+			eeepc_bluetooth_rfkill_set;
+		ehotk->eeepc_bluetooth_rfkill->get_state =
+			eeepc_bluetooth_rfkill_state;
+		ehotk->eeepc_bluetooth_rfkill->user_claim_unsupported = 0;
+		rfkill_register(ehotk->eeepc_bluetooth_rfkill);
+	}
+
  end:
 	if (result) {
 		kfree(ehotk);

@@ -543,6 +705,12 @@ static void eeepc_backlight_exit(void)
 {
 	if (eeepc_backlight_device)
 		backlight_device_unregister(eeepc_backlight_device);
+	if (ehotk->inputdev)
+		input_unregister_device(ehotk->inputdev);
+	if (ehotk->eeepc_wlan_rfkill)
+		rfkill_unregister(ehotk->eeepc_wlan_rfkill);
+	if (ehotk->eeepc_bluetooth_rfkill)
+		rfkill_unregister(ehotk->eeepc_bluetooth_rfkill);
 	eeepc_backlight_device = NULL;
 }
author
#3 lore raw

Re: [PATCH 2/2] eeepc-laptop: Use standard interfaces

Quoted 10 lines · Ivo van Doorn
Quoted 5 lines · Matthew Garrett

eeepc-laptop currently only sends key events via ACPI and has non-standard rfkill control. Add an input device and use the rfkill infrastructure.

Signed-off-by Matthew Garrett <[email protected]>

Please use the rfkill_force_state() to report state changes, that will ensure that the events are immediately send to the rfkill layer.

Why does writing to the sysfs file not generate an update implicitly? This is purely a software rfkill device, the input hotkey just generates KEY_WLAN and needs rfkill-input to handle it.

Signature · 1 line
Matthew Garrett <[email protected]>
#4 lore raw

Re: [PATCH 2/2] eeepc-laptop: Use standard interfaces

Quoted 6 lines · Matthew Garrett
Quoted 5 lines · Ivo van Doorn
Quoted 4 lines · Matthew Garrett

eeepc-laptop currently only sends key events via ACPI and has non-standard rfkill control. Add an input device and use the rfkill infrastructure.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Matthew Garrett <[email protected]>

Please use the rfkill_force_state() to report state changes, that will ensure that the events are immediately send to the rfkill layer.

Why does writing to the sysfs file not generate an update implicitly?

It does. rfkill_force_state() is used to propagate state changes done by outside sources back into rfkill. All internal changes caused by rfkill or known to rfkill are dealt with by rfkill itself.

If there are no such outside sources, you don't need rfkill_force_state() or get_state() at all, as long as you set rfkill->state properly before you call rfkill_register.

BTW: if it has a hardware rfkill switch that overrides the rfkill controller, that DOES count as an outside source of changes.

Signature · 4 lines
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
#5 lore raw

Re: [PATCH 2/2] eeepc-laptop: Use standard interfaces

In reply to Matthew Garrett

drivers/misc/eeepc-laptop.c static int update_bl_status(struct backlight_device *bd) · line 303

Quoted 10 lines · Matthew Garrett
drivers/misc/eeepc-laptop.c +10 -0
+static int eeepc_wlan_rfkill_set(void *data, enum rfkill_state state)
+{
+	return set_acpi(CM_ASL_WLAN, state);
+}
+
+static int eeepc_wlan_rfkill_state(void *data, enum rfkill_state *state)
+{
+	*state = get_acpi(CM_ASL_WLAN);
+	return 0;
+}

This might just be style, but I'd rather you didn't do this unless get_acpi and set_acpi do take enum rfkill_state... Doing state conversion would be a lot more future-proof, as well:

Something like:

	set_acpi(CM_ASL_WLAN, (state == RFKILL_STATE_UNBLOCKED) ? 0 : 1);

And:

	*state = get_acpi(CM_ASL_WLAN) ? RFKILL_STATE_SOFT_BLOCKED :
						RFKILL_STATE_UNBLOCKED;

Adjust as needed.

drivers/misc/eeepc-laptop.c static int eeepc_hotk_add(struct acpi_device *device) · line 565

Quoted 1 line
drivers/misc/eeepc-laptop.c +1 -0
+		ehotk->eeepc_wlan_rfkill->user_claim_unsupported = 0;

This is not needed, and if we get rid of user_claim_unsupported later, your code wouldn't need changes if you get rid of that line.

I have not paid much attention to it yet, but so far I have failed to understand what user_claim_unsupported is good for in the new style rfkill interface.

Signature · 4 lines
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
author
#6 lore raw

PATCH v2 1/2 eeepc-laptop: Fix user after free

In reply to Matthew Garrett

eeepc-laptop uses the hwmon struct after unregistering the device, causing an oops on module unload. Flip the ordering to fix.

Signed-off-by Matthew Garrett <[email protected]>

---

commit 95f4bc41ef256b8e3dfa94cabe1faf0776ac988c

Author: Matthew Garrett <[email protected]>
Date:   Mon Aug 4 17:57:40 2008 +0100

Fix use after free

drivers/misc/eeepc-laptop.c +1 -1
index 9e8d79e..facdb98 100644
@@ -553,9 +553,9 @@ static void eeepc_hwmon_exit(void)
 	hwmon = eeepc_hwmon_device;
 	if (!hwmon)
 		return ;
-	hwmon_device_unregister(hwmon);
 	sysfs_remove_group(&hwmon->kobj,
 			   &hwmon_attribute_group);
+	hwmon_device_unregister(hwmon);
 	eeepc_hwmon_device = NULL;
 }
Signature · 1 line
Matthew Garrett | [email protected]
author
#7 lore raw

PATCH v2 2/2 eeepc-laptop: Use standard interfaces

eeepc-laptop currently only sends key events via ACPI and has non-standard rfkill control. Add an input device and use the rfkill infrastructure.

Signed-off-by Matthew Garrett <[email protected]>

---

Updated to explicitly set state at registration time and use the enums rather than relying on their ordering (as suggested by Henrique)

drivers/misc/eeepc-laptop.c +209 -19
index facdb98..f7f1eae 100644
@@ -28,6 +28,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/acpi_bus.h>
 #include <linux/uaccess.h>
+#include <linux/input.h>
+#include <linux/rfkill.h>
 
 #define EEEPC_LAPTOP_VERSION	"0.1"
 

@@ -125,6 +127,10 @@ struct eeepc_hotk {
 					   by this BIOS */
 	uint init_flag;			/* Init flags */
 	u16 event_count[128];		/* count for each event */
+	struct input_dev *inputdev;
+	u16 *keycode_map;
+	struct rfkill *eeepc_wlan_rfkill;
+	struct rfkill *eeepc_bluetooth_rfkill;
 };
 
 /* The actual device the driver binds to */

@@ -140,6 +146,27 @@ static struct platform_driver platform_driver = {
 
 static struct platform_device *platform_device;
 
+struct key_entry {
+	char type;
+	u8 code;
+	u16 keycode;
+};
+
+enum { KE_KEY, KE_END };
+
+static struct key_entry eeepc_keymap[] = {
+	/* Sleep already handled via generic ACPI code */
+	{KE_KEY, 0x10, KEY_WLAN },
+	{KE_KEY, 0x12, KEY_PROG1 },
+	{KE_KEY, 0x13, KEY_MUTE },
+	{KE_KEY, 0x14, KEY_VOLUMEDOWN },
+	{KE_KEY, 0x15, KEY_VOLUMEUP },
+	{KE_KEY, 0x30, KEY_SWITCHVIDEOMODE },
+	{KE_KEY, 0x31, KEY_SWITCHVIDEOMODE },
+	{KE_KEY, 0x32, KEY_SWITCHVIDEOMODE },
+	{KE_END, 0},
+};
+
 /*
  * The hotkey driver declaration
  */

@@ -261,6 +288,44 @@ static int update_bl_status(struct backlight_device *bd)
 }
 
 /*
+ * Rfkill helpers
+ */
+
+static int eeepc_wlan_rfkill_set(void *data, enum rfkill_state state)
+{
+	if (state == RFKILL_STATE_SOFT_BLOCKED)
+		return set_acpi(CM_ASL_WLAN, 0);
+	else
+		return set_acpi(CM_ASL_WLAN, 1);
+}
+
+static int eeepc_wlan_rfkill_state(void *data, enum rfkill_state *state)
+{
+	if (get_acpi(CM_ASL_WLAN) == 1)
+		*state = RFKILL_STATE_UNBLOCKED;
+	else
+		*state = RFKILL_STATE_SOFT_BLOCKED;
+	return 0;
+}
+
+static int eeepc_wlan_bluetooth_set(void *data, enum rfkill_state state)
+{
+	if (state == RFKILL_STATE_SOFT_BLOCKED)
+		return set_acpi(CM_ASL_BLUETOOTH, 0);
+	else
+		return set_acpi(CM_ASL_BLUETOOTH, 1);
+}
+
+static int eeepc_wlan_bluetooth_state(void *data, enum rfkill_state *state)
+{
+	if (get_acpi(CM_ASL_BLUETOOTH) == 1)
+		*state = RFKILL_STATE_UNBLOCKED;
+	else
+		*state = RFKILL_STATE_SOFT_BLOCKED;
+	return 0;
+}
+
+/*
  * Sys helpers
  */
 static int parse_arg(const char *buf, unsigned long count, int *val)

@@ -311,13 +376,11 @@ static ssize_t show_sys_acpi(int cm, char *buf)
 EEEPC_CREATE_DEVICE_ATTR(camera, CM_ASL_CAMERA);
 EEEPC_CREATE_DEVICE_ATTR(cardr, CM_ASL_CARDREADER);
 EEEPC_CREATE_DEVICE_ATTR(disp, CM_ASL_DISPLAYSWITCH);
-EEEPC_CREATE_DEVICE_ATTR(wlan, CM_ASL_WLAN);
 
 static struct attribute *platform_attributes[] = {
 	&dev_attr_camera.attr,
 	&dev_attr_cardr.attr,
 	&dev_attr_disp.attr,
-	&dev_attr_wlan.attr,
 	NULL
 };
 

@@ -328,8 +391,64 @@ static struct attribute_group platform_attribute_group = {
 /*
  * Hotkey functions
  */
+static struct key_entry *eepc_get_entry_by_scancode(int code)
+{
+	struct key_entry *key;
+
+	for (key = eeepc_keymap; key->type != KE_END; key++)
+		if (code == key->code)
+			return key;
+
+	return NULL;
+}
+
+static struct key_entry *eepc_get_entry_by_keycode(int code)
+{
+	struct key_entry *key;
+
+	for (key = eeepc_keymap; key->type != KE_END; key++)
+		if (code == key->keycode && key->type == KE_KEY)
+			return key;
+
+	return NULL;
+}
+
+static int eeepc_getkeycode(struct input_dev *dev, int scancode, int *keycode)
+{
+	struct key_entry *key = eepc_get_entry_by_scancode(scancode);
+
+	if (key && key->type == KE_KEY) {
+		*keycode = key->keycode;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int eeepc_setkeycode(struct input_dev *dev, int scancode, int keycode)
+{
+	struct key_entry *key;
+	int old_keycode;
+
+	if (keycode < 0 || keycode > KEY_MAX)
+		return -EINVAL;
+
+	key = eepc_get_entry_by_scancode(scancode);
+	if (key && key->type == KE_KEY) {
+		old_keycode = key->keycode;
+		key->keycode = keycode;
+		set_bit(keycode, dev->keybit);
+		if (!eepc_get_entry_by_keycode(old_keycode))
+			clear_bit(old_keycode, dev->keybit);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static int eeepc_hotk_check(void)
 {
+	const struct key_entry *key;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	int result;
 

@@ -356,6 +475,31 @@ static int eeepc_hotk_check(void)
 			       "Get control methods supported: 0x%x\n",
 			       ehotk->cm_supported);
 		}
+		ehotk->inputdev = input_allocate_device();
+		if (!ehotk->inputdev) {
+			printk(EEEPC_INFO "Unable to allocate input device\n");
+			return 0;
+		}
+		ehotk->inputdev->name = "Asus EeePC extra buttons";
+		ehotk->inputdev->phys = EEEPC_HOTK_FILE "/input0";
+		ehotk->inputdev->id.bustype = BUS_HOST;
+		ehotk->inputdev->getkeycode = eeepc_getkeycode;
+		ehotk->inputdev->setkeycode = eeepc_setkeycode;
+
+		for (key = eeepc_keymap; key->type != KE_END; key++) {
+			switch (key->type) {
+			case KE_KEY:
+				set_bit(EV_KEY, ehotk->inputdev->evbit);
+				set_bit(key->keycode, ehotk->inputdev->keybit);
+				break;
+			}
+		}
+		result = input_register_device(ehotk->inputdev);
+		if (result) {
+			printk(EEEPC_INFO "Unable to register input device\n");
+			input_free_device(ehotk->inputdev);
+			return 0;
+		}
 	} else {
 		printk(EEEPC_ERR "Hotkey device not present, aborting\n");
 		return -EINVAL;

@@ -363,21 +507,6 @@ static int eeepc_hotk_check(void)
 	return 0;
 }
 
-static void notify_wlan(u32 *event)
-{
-	/* if DISABLE_ASL_WLAN is set, the notify code for fn+f2
-	   will always be 0x10 */
-	if (ehotk->cm_supported & (0x1 << CM_ASL_WLAN)) {
-		const char *method = cm_getv[CM_ASL_WLAN];
-		int value;
-		if (read_acpi_int(ehotk->handle, method, &value))
-			printk(EEEPC_WARNING "Error reading %s\n",
-			       method);
-		else if (value == 1)
-			*event = 0x11;
-	}
-}
-
 static void notify_brn(void)
 {
 	struct backlight_device *bd = eeepc_backlight_device;

@@ -386,14 +515,28 @@ static void notify_brn(void)
 
 static void eeepc_hotk_notify(acpi_handle handle, u32 event, void *data)
 {
+	static struct key_entry *key;
 	if (!ehotk)
 		return;
-	if (event == NOTIFY_WLAN_ON && (DISABLE_ASL_WLAN & ehotk->init_flag))
-		notify_wlan(&event);
 	if (event >= NOTIFY_BRN_MIN && event <= NOTIFY_BRN_MAX)
 		notify_brn();
 	acpi_bus_generate_proc_event(ehotk->device, event,
 				     ehotk->event_count[event % 128]++);
+	if (ehotk->inputdev) {
+		key = eepc_get_entry_by_scancode(event);
+		if (key) {
+			switch (key->type) {
+			case KE_KEY:
+				input_report_key(ehotk->inputdev, key->keycode,
+						 1);
+				input_sync(ehotk->inputdev);
+				input_report_key(ehotk->inputdev, key->keycode,
+						 0);
+				input_sync(ehotk->inputdev);
+				break;
+			}
+		}
+	}
 }
 
 static int eeepc_hotk_add(struct acpi_device *device)

@@ -420,6 +563,47 @@ static int eeepc_hotk_add(struct acpi_device *device)
 					     eeepc_hotk_notify, ehotk);
 	if (ACPI_FAILURE(status))
 		printk(EEEPC_ERR "Error installing notify handler\n");
+
+	if (get_acpi(CM_ASL_WLAN) != -1) {
+		ehotk->eeepc_wlan_rfkill = rfkill_allocate(&device->dev,
+							   RFKILL_TYPE_WLAN);
+
+		if (!ehotk->eeepc_wlan_rfkill)
+			goto end;
+
+		ehotk->eeepc_wlan_rfkill->name = "eeepc-wlan";
+		ehotk->eeepc_wlan_rfkill->toggle_radio = eeepc_wlan_rfkill_set;
+		ehotk->eeepc_wlan_rfkill->get_state = eeepc_wlan_rfkill_state;
+		if (get_acpi(CM_ASL_WIFI) == 1)
+			ehotk->eeepc_wlan_rfkill->state =
+				RFKILL_STATE_UNBLOCKED;
+		else
+			ehotk->eeepc_wlan_rfkill->state =
+				RFKILL_STATE_SOFT_BLOCKED;
+		rfkill_register(ehotk->eeepc_wlan_rfkill);
+	}
+
+	if (get_acpi(CM_ASL_BLUETOOTH) != -1) {
+		ehotk->eeepc_bluetooth_rfkill =
+			rfkill_allocate(&device->dev, RFKILL_TYPE_BLUETOOTH);
+
+		if (!ehotk->eeepc_bluetooth_rfkill)
+			goto end;
+
+		ehotk->eeepc_bluetooth_rfkill->name = "eeepc-bluetooth";
+		ehotk->eeepc_bluetooth_rfkill->toggle_radio =
+			eeepc_bluetooth_rfkill_set;
+		ehotk->eeepc_bluetooth_rfkill->get_state =
+			eeepc_bluetooth_rfkill_state;
+		if (get_acpi(CM_ASL_BLUETOOTH) == 1)
+			ehotk->eeepc_bluetooth_rfkill->state =
+				RFKILL_STATE_UNBLOCKED;
+		else
+			ehotk->eeepc_bluetooth_rfkill->state =
+				RFKILL_STATE_SOFT_BLOCKED;
+		rfkill_register(ehotk->eeepc_bluetooth_rfkill);
+	}
+
  end:
 	if (result) {
 		kfree(ehotk);

@@ -543,6 +727,12 @@ static void eeepc_backlight_exit(void)
 {
 	if (eeepc_backlight_device)
 		backlight_device_unregister(eeepc_backlight_device);
+	if (ehotk->inputdev)
+		input_unregister_device(ehotk->inputdev);
+	if (ehotk->eeepc_wlan_rfkill)
+		rfkill_unregister(ehotk->eeepc_wlan_rfkill);
+	if (ehotk->eeepc_bluetooth_rfkill)
+		rfkill_unregister(ehotk->eeepc_bluetooth_rfkill);
 	eeepc_backlight_device = NULL;
 }
Signature · 1 line
Matthew Garrett | [email protected]
#8 lore raw

Re: [PATCH v2 2/2] eeepc-laptop: Use standard interfaces

Quoted 4 lines · On Wed, 6 Aug 2008 10:25:23 +0100 Matthew Garrett <[email protected]> wrote:

eeepc-laptop currently only sends key events via ACPI and has non-standard rfkill control. Add an input device and use the rfkill infrastructure.

drivers/misc/eeepc-laptop.c: In function 'eeepc_hotk_add':
drivers/misc/eeepc-laptop.c:577: error: 'CM_ASL_WIFI' undeclared (first use in this function) drivers/misc/eeepc-laptop.c:577: error: (Each undeclared identifier is reported only once drivers/misc/eeepc-laptop.c:577: error: for each function it appears in.) drivers/misc/eeepc-laptop.c:595: error: 'eeepc_bluetooth_rfkill_set' undeclared (first use in this function) drivers/misc/eeepc-laptop.c:597: error: 'eeepc_bluetooth_rfkill_state' undeclared (first use in this function)

maybe this had some dependencies which I didn't know about.

author
#9 lore raw

Re: [PATCH v2 2/2] eeepc-laptop: Use standard interfaces

eeepc-laptop currently only sends key events via ACPI and has non-standard rfkill control. Add an input device and use the rfkill infrastructure.

Signed-off-by Matthew Garrett <[email protected]>

---

Sorry, managed to send the wrong version before. This one actually builds.

drivers/misc/eeepc-laptop.c +209 -19
index facdb98..017fa89 100644
@@ -28,6 +28,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/acpi_bus.h>
 #include <linux/uaccess.h>
+#include <linux/input.h>
+#include <linux/rfkill.h>
 
 #define EEEPC_LAPTOP_VERSION	"0.1"
 

@@ -125,6 +127,10 @@ struct eeepc_hotk {
 					   by this BIOS */
 	uint init_flag;			/* Init flags */
 	u16 event_count[128];		/* count for each event */
+	struct input_dev *inputdev;
+	u16 *keycode_map;
+	struct rfkill *eeepc_wlan_rfkill;
+	struct rfkill *eeepc_bluetooth_rfkill;
 };
 
 /* The actual device the driver binds to */

@@ -140,6 +146,27 @@ static struct platform_driver platform_driver = {
 
 static struct platform_device *platform_device;
 
+struct key_entry {
+	char type;
+	u8 code;
+	u16 keycode;
+};
+
+enum { KE_KEY, KE_END };
+
+static struct key_entry eeepc_keymap[] = {
+	/* Sleep already handled via generic ACPI code */
+	{KE_KEY, 0x10, KEY_WLAN },
+	{KE_KEY, 0x12, KEY_PROG1 },
+	{KE_KEY, 0x13, KEY_MUTE },
+	{KE_KEY, 0x14, KEY_VOLUMEDOWN },
+	{KE_KEY, 0x15, KEY_VOLUMEUP },
+	{KE_KEY, 0x30, KEY_SWITCHVIDEOMODE },
+	{KE_KEY, 0x31, KEY_SWITCHVIDEOMODE },
+	{KE_KEY, 0x32, KEY_SWITCHVIDEOMODE },
+	{KE_END, 0},
+};
+
 /*
  * The hotkey driver declaration
  */

@@ -261,6 +288,44 @@ static int update_bl_status(struct backlight_device *bd)
 }
 
 /*
+ * Rfkill helpers
+ */
+
+static int eeepc_wlan_rfkill_set(void *data, enum rfkill_state state)
+{
+	if (state == RFKILL_STATE_SOFT_BLOCKED)
+		return set_acpi(CM_ASL_WLAN, 0);
+	else
+		return set_acpi(CM_ASL_WLAN, 1);
+}
+
+static int eeepc_wlan_rfkill_state(void *data, enum rfkill_state *state)
+{
+	if (get_acpi(CM_ASL_WLAN) == 1)
+		*state = RFKILL_STATE_UNBLOCKED;
+	else
+		*state = RFKILL_STATE_SOFT_BLOCKED;
+	return 0;
+}
+
+static int eeepc_bluetooth_rfkill_set(void *data, enum rfkill_state state)
+{
+	if (state == RFKILL_STATE_SOFT_BLOCKED)
+		return set_acpi(CM_ASL_BLUETOOTH, 0);
+	else
+		return set_acpi(CM_ASL_BLUETOOTH, 1);
+}
+
+static int eeepc_bluetooth_rfkill_state(void *data, enum rfkill_state *state)
+{
+	if (get_acpi(CM_ASL_BLUETOOTH) == 1)
+		*state = RFKILL_STATE_UNBLOCKED;
+	else
+		*state = RFKILL_STATE_SOFT_BLOCKED;
+	return 0;
+}
+
+/*
  * Sys helpers
  */
 static int parse_arg(const char *buf, unsigned long count, int *val)

@@ -311,13 +376,11 @@ static ssize_t show_sys_acpi(int cm, char *buf)
 EEEPC_CREATE_DEVICE_ATTR(camera, CM_ASL_CAMERA);
 EEEPC_CREATE_DEVICE_ATTR(cardr, CM_ASL_CARDREADER);
 EEEPC_CREATE_DEVICE_ATTR(disp, CM_ASL_DISPLAYSWITCH);
-EEEPC_CREATE_DEVICE_ATTR(wlan, CM_ASL_WLAN);
 
 static struct attribute *platform_attributes[] = {
 	&dev_attr_camera.attr,
 	&dev_attr_cardr.attr,
 	&dev_attr_disp.attr,
-	&dev_attr_wlan.attr,
 	NULL
 };
 

@@ -328,8 +391,64 @@ static struct attribute_group platform_attribute_group = {
 /*
  * Hotkey functions
  */
+static struct key_entry *eepc_get_entry_by_scancode(int code)
+{
+	struct key_entry *key;
+
+	for (key = eeepc_keymap; key->type != KE_END; key++)
+		if (code == key->code)
+			return key;
+
+	return NULL;
+}
+
+static struct key_entry *eepc_get_entry_by_keycode(int code)
+{
+	struct key_entry *key;
+
+	for (key = eeepc_keymap; key->type != KE_END; key++)
+		if (code == key->keycode && key->type == KE_KEY)
+			return key;
+
+	return NULL;
+}
+
+static int eeepc_getkeycode(struct input_dev *dev, int scancode, int *keycode)
+{
+	struct key_entry *key = eepc_get_entry_by_scancode(scancode);
+
+	if (key && key->type == KE_KEY) {
+		*keycode = key->keycode;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int eeepc_setkeycode(struct input_dev *dev, int scancode, int keycode)
+{
+	struct key_entry *key;
+	int old_keycode;
+
+	if (keycode < 0 || keycode > KEY_MAX)
+		return -EINVAL;
+
+	key = eepc_get_entry_by_scancode(scancode);
+	if (key && key->type == KE_KEY) {
+		old_keycode = key->keycode;
+		key->keycode = keycode;
+		set_bit(keycode, dev->keybit);
+		if (!eepc_get_entry_by_keycode(old_keycode))
+			clear_bit(old_keycode, dev->keybit);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static int eeepc_hotk_check(void)
 {
+	const struct key_entry *key;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	int result;
 

@@ -356,6 +475,31 @@ static int eeepc_hotk_check(void)
 			       "Get control methods supported: 0x%x\n",
 			       ehotk->cm_supported);
 		}
+		ehotk->inputdev = input_allocate_device();
+		if (!ehotk->inputdev) {
+			printk(EEEPC_INFO "Unable to allocate input device\n");
+			return 0;
+		}
+		ehotk->inputdev->name = "Asus EeePC extra buttons";
+		ehotk->inputdev->phys = EEEPC_HOTK_FILE "/input0";
+		ehotk->inputdev->id.bustype = BUS_HOST;
+		ehotk->inputdev->getkeycode = eeepc_getkeycode;
+		ehotk->inputdev->setkeycode = eeepc_setkeycode;
+
+		for (key = eeepc_keymap; key->type != KE_END; key++) {
+			switch (key->type) {
+			case KE_KEY:
+				set_bit(EV_KEY, ehotk->inputdev->evbit);
+				set_bit(key->keycode, ehotk->inputdev->keybit);
+				break;
+			}
+		}
+		result = input_register_device(ehotk->inputdev);
+		if (result) {
+			printk(EEEPC_INFO "Unable to register input device\n");
+			input_free_device(ehotk->inputdev);
+			return 0;
+		}
 	} else {
 		printk(EEEPC_ERR "Hotkey device not present, aborting\n");
 		return -EINVAL;

@@ -363,21 +507,6 @@ static int eeepc_hotk_check(void)
 	return 0;
 }
 
-static void notify_wlan(u32 *event)
-{
-	/* if DISABLE_ASL_WLAN is set, the notify code for fn+f2
-	   will always be 0x10 */
-	if (ehotk->cm_supported & (0x1 << CM_ASL_WLAN)) {
-		const char *method = cm_getv[CM_ASL_WLAN];
-		int value;
-		if (read_acpi_int(ehotk->handle, method, &value))
-			printk(EEEPC_WARNING "Error reading %s\n",
-			       method);
-		else if (value == 1)
-			*event = 0x11;
-	}
-}
-
 static void notify_brn(void)
 {
 	struct backlight_device *bd = eeepc_backlight_device;

@@ -386,14 +515,28 @@ static void notify_brn(void)
 
 static void eeepc_hotk_notify(acpi_handle handle, u32 event, void *data)
 {
+	static struct key_entry *key;
 	if (!ehotk)
 		return;
-	if (event == NOTIFY_WLAN_ON && (DISABLE_ASL_WLAN & ehotk->init_flag))
-		notify_wlan(&event);
 	if (event >= NOTIFY_BRN_MIN && event <= NOTIFY_BRN_MAX)
 		notify_brn();
 	acpi_bus_generate_proc_event(ehotk->device, event,
 				     ehotk->event_count[event % 128]++);
+	if (ehotk->inputdev) {
+		key = eepc_get_entry_by_scancode(event);
+		if (key) {
+			switch (key->type) {
+			case KE_KEY:
+				input_report_key(ehotk->inputdev, key->keycode,
+						 1);
+				input_sync(ehotk->inputdev);
+				input_report_key(ehotk->inputdev, key->keycode,
+						 0);
+				input_sync(ehotk->inputdev);
+				break;
+			}
+		}
+	}
 }
 
 static int eeepc_hotk_add(struct acpi_device *device)

@@ -420,6 +563,47 @@ static int eeepc_hotk_add(struct acpi_device *device)
 					     eeepc_hotk_notify, ehotk);
 	if (ACPI_FAILURE(status))
 		printk(EEEPC_ERR "Error installing notify handler\n");
+
+	if (get_acpi(CM_ASL_WLAN) != -1) {
+		ehotk->eeepc_wlan_rfkill = rfkill_allocate(&device->dev,
+							   RFKILL_TYPE_WLAN);
+
+		if (!ehotk->eeepc_wlan_rfkill)
+			goto end;
+
+		ehotk->eeepc_wlan_rfkill->name = "eeepc-wlan";
+		ehotk->eeepc_wlan_rfkill->toggle_radio = eeepc_wlan_rfkill_set;
+		ehotk->eeepc_wlan_rfkill->get_state = eeepc_wlan_rfkill_state;
+		if (get_acpi(CM_ASL_WLAN) == 1)
+			ehotk->eeepc_wlan_rfkill->state =
+				RFKILL_STATE_UNBLOCKED;
+		else
+			ehotk->eeepc_wlan_rfkill->state =
+				RFKILL_STATE_SOFT_BLOCKED;
+		rfkill_register(ehotk->eeepc_wlan_rfkill);
+	}
+
+	if (get_acpi(CM_ASL_BLUETOOTH) != -1) {
+		ehotk->eeepc_bluetooth_rfkill =
+			rfkill_allocate(&device->dev, RFKILL_TYPE_BLUETOOTH);
+
+		if (!ehotk->eeepc_bluetooth_rfkill)
+			goto end;
+
+		ehotk->eeepc_bluetooth_rfkill->name = "eeepc-bluetooth";
+		ehotk->eeepc_bluetooth_rfkill->toggle_radio =
+			eeepc_bluetooth_rfkill_set;
+		ehotk->eeepc_bluetooth_rfkill->get_state =
+			eeepc_bluetooth_rfkill_state;
+		if (get_acpi(CM_ASL_BLUETOOTH) == 1)
+			ehotk->eeepc_bluetooth_rfkill->state =
+				RFKILL_STATE_UNBLOCKED;
+		else
+			ehotk->eeepc_bluetooth_rfkill->state =
+				RFKILL_STATE_SOFT_BLOCKED;
+		rfkill_register(ehotk->eeepc_bluetooth_rfkill);
+	}
+
  end:
 	if (result) {
 		kfree(ehotk);

@@ -543,6 +727,12 @@ static void eeepc_backlight_exit(void)
 {
 	if (eeepc_backlight_device)
 		backlight_device_unregister(eeepc_backlight_device);
+	if (ehotk->inputdev)
+		input_unregister_device(ehotk->inputdev);
+	if (ehotk->eeepc_wlan_rfkill)
+		rfkill_unregister(ehotk->eeepc_wlan_rfkill);
+	if (ehotk->eeepc_bluetooth_rfkill)
+		rfkill_unregister(ehotk->eeepc_bluetooth_rfkill);
 	eeepc_backlight_device = NULL;
 }
Signature · 1 line
Matthew Garrett | [email protected]
#10 lore raw

Re: [PATCH v2 2/2] eeepc-laptop: Use standard interfaces

Looks fine to me at first glance. I do wonder if there are no events at all you could hook to rfkill_force_state() instead of (or in addition to) using the get_state()?

Signature · 4 lines
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
author
#11 lore raw

Re: [PATCH v2 2/2] eeepc-laptop: Use standard interfaces

Quoted 3 lines · Henrique de Moraes Holschuh

Looks fine to me at first glance. I do wonder if there are no events at all you could hook to rfkill_force_state() instead of (or in addition to) using the get_state()?

The only change event is generated by hitting the wifi key, which ties into rfkill-input. Won't that already force a state update on the event?

Signature · 1 line
Matthew Garrett | [email protected]
#12 lore raw

Re: [PATCH v2 2/2] eeepc-laptop: Use standard interfaces

Quoted 7 lines · Matthew Garrett
Quoted 3 lines · Henrique de Moraes Holschuh

Looks fine to me at first glance. I do wonder if there are no events at all you could hook to rfkill_force_state() instead of (or in addition to) using the get_state()?

The only change event is generated by hitting the wifi key, which ties into rfkill-input. Won't that already force a state update on the event?

Yes, but one must keep this in mind:

1. If the event has no bearing on compulsory hardware state changes (i.e. the hardware won't change state by itself when the event happens, it is really just reporting a simple key press that will do nothing by itself), you just report the key as an event.

2. If the hardware/firmware *ALSO* compulsory changes the rfkill state (i.e. the event also means the real rfkill controller state probably changed), you take the opportunity to do a forced immediate state poll and rfkill_force_state() the new state.

So, it basically depends whether the EEEPC hardware/firmware does (1) or (2). If it is (1), the patch is correct. If it is (2), it should do a bit more stuff that ends up with a call to rfkill_force_state().

The reason is that there is absolutely *NO* reason why rfkill-input (or anything else for that matter) has to obey the input event. They can just discard it if they want: it is a local system policy matter. Therefore, if there is anything in *hardware* or *firmware* that is already acting on that event, you *MUST* arrange for a call to rfkill_force_state().

Signature · 4 lines
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
author
#13 lore raw

Re: [PATCH v2 2/2] eeepc-laptop: Use standard interfaces

Quoted 4 lines · Henrique de Moraes Holschuh

1. If the event has no bearing on compulsory hardware state changes (i.e. the hardware won't change state by itself when the event happens, it is really just reporting a simple key press that will do nothing by itself), you just report the key as an event.

Yes. That's all I do.

Quoted 4 lines

2. If the hardware/firmware *ALSO* compulsory changes the rfkill state (i.e. the event also means the real rfkill controller state probably changed), you take the opportunity to do a forced immediate state poll and rfkill_force_state() the new state.

The firmware does not perform any compulsory change.

Signature · 1 line
Matthew Garrett | [email protected]
#14 lore raw

Re: [PATCH v2 2/2] eeepc-laptop: Use standard interfaces

Hi Matthew!

Quoted 7 lines · Matthew Garrett
Quoted 4 lines · Henrique de Moraes Holschuh

1. If the event has no bearing on compulsory hardware state changes (i.e. the hardware won't change state by itself when the event happens, it is really just reporting a simple key press that will do nothing by itself), you just report the key as an event.

Quoted 8 lines

Yes. That's all I do.

Quoted 4 lines

2. If the hardware/firmware *ALSO* compulsory changes the rfkill state (i.e. the event also means the real rfkill controller state probably changed), you take the opportunity to do a forced immediate state poll and rfkill_force_state() the new state.

The firmware does not perform any compulsory change.

Then, I have no futher comments. Looks good to me.

Signature · 4 lines
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
author
#15 lore raw

Awkward rfkill corner cases

Quoted 1 line · Henrique de Moraes Holschuh

Then, I have no futher comments. Looks good to me.

Excellent, glad I've got that right.

One completely unrelated question. In the following situation (relevant to Dells, not the Eee)

* The system has a key (not a switch) that in firmware disables the hardware (HARD_BLOCKED)
* That key generates an event through the keyboard controller, but not through any other obviously detectable means
* The radio control is also controllable through software (SOFT_BLOCKED)

Should pressing the key generate a KEY_WLAN event?

I note that rfkill-input will, if the device is in HARD_BLOCKED state, attempt to set it to UNBLOCKED. This sounds like generating the keycode is the wrong thing to do, since it'll cause rfkill-input to try to undo the change that's just been made. However, if the key isn't mapped there's no obvious way for any of the stack to determine that a change has been made and propagate that to userspace. What should we be doing here?

Signature · 1 line
Matthew Garrett | [email protected]
#16 lore raw

Re: Awkward rfkill corner cases

Quoted 5 lines · Matthew Garrett

One completely unrelated question. In the following situation (relevant to Dells, not the Eee)

* The system has a key (not a switch) that in firmware disables the hardware (HARD_BLOCKED)

Ick. I sure hope you can query the firmware, so that you can look at it as if it were a switch instead of a key (and look at the key press event as a "switch changed state" event -- never mind it is difficult to hook to that event right now)?

I mean, trying to deal with firmware/hardware that changes states on its own in any other basis than "it is a switch", is error prone. You miss one event, you go out of sync. That's bad.

Quoted 2 lines

* That key generates an event through the keyboard controller, but not through any other obviously detectable means

Ok.

Quoted 3 lines

* The radio control is also controllable through software (SOFT_BLOCKED)

Should pressing the key generate a KEY_WLAN event?

Frankly? I think so. It would be nice if you could hook the key as a "hint that the rfkill controller may have changed state" on the driver, and ALSO as a normal input device (so that it can command more than just that one WLAN radio).

But I think it would be EVEN BETTER if you could somehow suppress that KEY_* event, and synthesize an EV_SW SW_WLAN in its place (you will have to ask Dmitry to add it, since it is a first use) instead. If you cannot, KEY_WLAN will have to make do.

Quoted 2 lines

I note that rfkill-input will, if the device is in HARD_BLOCKED state, attempt to set it to UNBLOCKED. This sounds like generating the keycode

I have some patches in flight that will make rfkill-input smarter about it. But that's just an enhancement. The current way it operates is annoying, but last time I checked, it doesn't blow up.

Just return -EPERM on your device driver's toggle_radio() callback if you are at HARD_BLOCKED and someone asks you to go to UNBLOCKED. That is what the API calls for (if it is not clear enough, we can improve the docs).

Quoted 5 lines

is the wrong thing to do, since it'll cause rfkill-input to try to undo the change that's just been made. However, if the key isn't mapped there's no obvious way for any of the stack to determine that a change has been made and propagate that to userspace. What should we be doing here?

Never worry about propagating rfkill state to userspace in a driver. rfkill will do it by itself using uevents, that code has already been accepted. The rfkill class will do it for all rfkill controllers, and rfkill input may be taught to do it later if userspace people ask for it (nobody did it yet because it is not very useful, since what you want is reports of what really IS happening to the radios, and those come from the rfkill class. All rfkill-input could tell userspace is what it is *trying* to change radio states, but not whether they did really happen).

The reason why you'd want to send a KEY_WLAN event (or, if you can, an EV_SW SW_WLAN event) is to change that key from something that controls an specific WLAN radio, to something that can *potentially* control every WLAN
radio attached to the box. It is *not* done to report status to userspace
[unless you meat report state to something in userspace that is doing what rfkill-input does in the kernel, but we haven't enhanced rfkill-input and rfkill to export all states needed for such an userspace implementation yet].

Signature · 4 lines
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
reviewer
#17 lore raw

Re: [PATCH v2 2/2] eeepc-laptop: Use standard interfaces

In reply to Matthew Garrett
Quoted 5 lines · Matthew Garrett

eeepc-laptop currently only sends key events via ACPI and has non-standard rfkill control. Add an input device and use the rfkill infrastructure.

Signed-off-by Matthew Garrett <[email protected]>

applied to acpi-test.
Corentin Chary, you're the driver maintainer, do you Ack?

thanks,
-Len