acpi,rfkill,backlight: comapl-laptop update - use rfkill switch subsystem

From: Cezary Jackiewicz <[email protected]>

Remove unnecessary attributes, use rfkill switch subsystem.

Signed-off-by Cezary Jackiewicz <[email protected]>

---
diff -Nuar a/drivers/misc/compal-laptop.c b/drivers/misc/compal-laptop.c

drivers/misc/compal-laptop.c +120 -131

@@ -43,6 +43,7 @@
 #include <linux/backlight.h>
 #include <linux/platform_device.h>
 #include <linux/autoconf.h>
+#include <linux/rfkill.h>
 
 #define COMPAL_DRIVER_VERSION "0.3.0"
 #define COMPAL_DRIVER_NAME "compal-laptop"

@@ -58,6 +59,10 @@
 #define WLAN_MASK	0x01
 #define BT_MASK		0x02
 
+/* rfkill switches */
+static struct rfkill *bluetooth_rfkill;
+static struct rfkill *wlan_rfkill;
+
 static int force;
 module_param(force, bool, 0);
 MODULE_PARM_DESC(force, "Force driver load, ignore DMI data");

@@ -83,67 +88,6 @@
 	return (int) result;
 }
 
-static int set_wlan_state(int state)
-{
-	u8 result, value;
-
-	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
-
-	if ((result & KILLSWITCH_MASK) == 0)
-		return -EINVAL;
-	else {
-		if (state)
-			value = (u8) (result | WLAN_MASK);
-		else
-			value = (u8) (result & ~WLAN_MASK);
-		ec_write(COMPAL_EC_COMMAND_WIRELESS, value);
-	}
-
-	return 0;
-}
-
-static int set_bluetooth_state(int state)
-{
-	u8 result, value;
-
-	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
-
-	if ((result & KILLSWITCH_MASK) == 0)
-		return -EINVAL;
-	else {
-		if (state)
-			value = (u8) (result | BT_MASK);
-		else
-			value = (u8) (result & ~BT_MASK);
-		ec_write(COMPAL_EC_COMMAND_WIRELESS, value);
-	}
-
-	return 0;
-}
-
-static int get_wireless_state(int *wlan, int *bluetooth)
-{
-	u8 result;
-
-	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
-
-	if (wlan) {
-		if ((result & KILLSWITCH_MASK) == 0)
-			*wlan = 0;
-		else
-			*wlan = result & WLAN_MASK;
-	}
-
-	if (bluetooth) {
-		if ((result & KILLSWITCH_MASK) == 0)
-			*bluetooth = 0;
-		else
-			*bluetooth = (result & BT_MASK) >> 1;
-	}
-
-	return 0;
-}
-
 /* Backlight device stuff */
 
 static int bl_get_brightness(struct backlight_device *b)

@@ -166,93 +110,125 @@
 
 /* Platform device */
 
-static ssize_t show_wlan(struct device *dev,
-	struct device_attribute *attr, char *buf)
-{
-	int ret, enabled;
+static struct platform_driver compal_driver = {
+	.driver = {
+		.name = COMPAL_DRIVER_NAME,
+		.owner = THIS_MODULE,
+	}
+};
 
-	ret = get_wireless_state(&enabled, NULL);
-	if (ret < 0)
-		return ret;
+static struct platform_device *compal_device;
 
-	return sprintf(buf, "%i\n", enabled);
-}
+/* rfkill stuff */
 
-static ssize_t show_raw(struct device *dev,
-	struct device_attribute *attr, char *buf)
+static int wlan_rfk_set(void *data, enum rfkill_state state)
 {
-	u8 result;
+	u8 result, value;
 
 	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
 
-	return sprintf(buf, "%i\n", result);
+	switch (state) {
+	case RFKILL_STATE_UNBLOCKED:
+		value = (u8) (result | WLAN_MASK);
+		break;
+	case RFKILL_STATE_SOFT_BLOCKED:
+		value = (u8) (result & ~WLAN_MASK);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ec_write(COMPAL_EC_COMMAND_WIRELESS, value);
+
+	return 0;
 }
 
-static ssize_t show_bluetooth(struct device *dev,
-	struct device_attribute *attr, char *buf)
+static int wlan_rfk_get(void *data, enum rfkill_state *state)
 {
-	int ret, enabled;
+	u8 result;
+
+	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
 
-	ret = get_wireless_state(NULL, &enabled);
-	if (ret < 0)
-		return ret;
+	if ((result & KILLSWITCH_MASK) == 0)
+		*state = RFKILL_STATE_HARD_BLOCKED;
+	else
+		*state = ((result & WLAN_MASK) != 0) ?
+			RFKILL_STATE_UNBLOCKED : RFKILL_STATE_SOFT_BLOCKED;
 
-	return sprintf(buf, "%i\n", enabled);
+	return 0;
 }
 
-static ssize_t store_wlan_state(struct device *dev,
-	struct device_attribute *attr, const char *buf, size_t count)
+static int bluetooth_rfk_set(void *data, enum rfkill_state state)
 {
-	int state, ret;
+	u8 result, value;
+
+	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
 
-	if (sscanf(buf, "%i", &state) != 1 || (state < 0 || state > 1))
+	switch (state) {
+	case RFKILL_STATE_UNBLOCKED:
+		value = (u8) (result | BT_MASK);
+		break;
+	case RFKILL_STATE_SOFT_BLOCKED:
+		value = (u8) (result & ~BT_MASK);
+		break;
+	default:
 		return -EINVAL;
+	}
 
-	ret = set_wlan_state(state);
-	if (ret < 0)
-		return ret;
+	ec_write(COMPAL_EC_COMMAND_WIRELESS, value);
 
-	return count;
+	return 0;
 }
 
-static ssize_t store_bluetooth_state(struct device *dev,
-	struct device_attribute *attr, const char *buf, size_t count)
+static int bluetooth_rfk_get(void *data, enum rfkill_state *state)
 {
-	int state, ret;
+	u8 result;
 
-	if (sscanf(buf, "%i", &state) != 1 || (state < 0 || state > 1))
-		return -EINVAL;
+	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
 
-	ret = set_bluetooth_state(state);
-	if (ret < 0)
-		return ret;
-
-	return count;
-}
-
-static DEVICE_ATTR(bluetooth, 0644, show_bluetooth, store_bluetooth_state);
-static DEVICE_ATTR(wlan, 0644, show_wlan, store_wlan_state);
-static DEVICE_ATTR(raw, 0444, show_raw, NULL);
-
-static struct attribute *compal_attributes[] = {
-	&dev_attr_bluetooth.attr,
-	&dev_attr_wlan.attr,
-	&dev_attr_raw.attr,
-	NULL
-};
+	if ((result & KILLSWITCH_MASK) == 0)
+		*state = RFKILL_STATE_HARD_BLOCKED;
+	else
+		*state = (((result & BT_MASK) >> 1) != 0) ?
+			RFKILL_STATE_UNBLOCKED : RFKILL_STATE_SOFT_BLOCKED;
 
-static struct attribute_group compal_attribute_group = {
-	.attrs = compal_attributes
-};
+	return 0;
+}
 
-static struct platform_driver compal_driver = {
-	.driver = {
-		.name = COMPAL_DRIVER_NAME,
-		.owner = THIS_MODULE,
+static int __init compal_rfkill(struct rfkill **rfk,
+				const enum rfkill_type rfktype,
+				const char *name,
+				int (*toggle_radio)(void *, enum rfkill_state),
+				int (*get_state)(void *, enum rfkill_state *))
+{
+	int res;
+	enum rfkill_state initial_state;
+
+	(*rfk) = rfkill_allocate(&compal_device->dev, rfktype);
+	if (!*rfk) {
+		printk(COMPAL_ERR
+			"failed to allocate memory for rfkill class\n");
+		return -ENOMEM;
 	}
-};
 
-static struct platform_device *compal_device;
+	(*rfk)->name = name;
+	(*rfk)->get_state = get_state;
+	(*rfk)->toggle_radio = toggle_radio;
+	if (!get_state(NULL, &initial_state))
+		(*rfk)->state = initial_state;
+
+	res = rfkill_register(*rfk);
+	if (res < 0) {
+		printk(COMPAL_ERR
+			"failed to register %s rfkill switch: %d\n",
+			name, res);
+		rfkill_free(*rfk);
+		*rfk = NULL;
+		return res;
+	}
+
+	return 0;
+}
 
 /* Initialization */
 

@@ -342,23 +318,28 @@
 
 	ret = platform_device_add(compal_device);
 	if (ret)
-		goto fail_platform_device1;
+		goto fail_platform_device;
 
-	ret = sysfs_create_group(&compal_device->dev.kobj,
-		&compal_attribute_group);
-	if (ret)
-		goto fail_platform_device2;
+	/* Register rfkill stuff */
+
+	compal_rfkill(&wlan_rfkill,
+			RFKILL_TYPE_WLAN,
+			"compal_laptop_wlan_sw",
+			wlan_rfk_set,
+			wlan_rfk_get);
+
+	compal_rfkill(&bluetooth_rfkill,
+			RFKILL_TYPE_BLUETOOTH,
+			"compal_laptop_bluetooth_sw",
+			bluetooth_rfk_set,
+			bluetooth_rfk_get);
 
 	printk(COMPAL_INFO "driver "COMPAL_DRIVER_VERSION
 			" successfully loaded.\n");
 
 	return 0;
 
-fail_platform_device2:
-
-	platform_device_del(compal_device);
-
-fail_platform_device1:
+fail_platform_device:
 
 	platform_device_put(compal_device);
 

@@ -375,8 +356,16 @@
 
 static void __exit compal_cleanup(void)
 {
+	if (bluetooth_rfkill) {
+		rfkill_unregister(bluetooth_rfkill);
+		bluetooth_rfkill = NULL;
+	}
+
+	if (wlan_rfkill) {
+		rfkill_unregister(wlan_rfkill);
+		wlan_rfkill = NULL;
+	}
 
-	sysfs_remove_group(&compal_device->dev.kobj, &compal_attribute_group);
 	platform_device_unregister(compal_device);
 	platform_driver_unregister(&compal_driver);
 	backlight_device_unregister(compalbl_device);

Quoted 3 lines

From: Cezary Jackiewicz <[email protected]>

Remove unnecessary attributes, use rfkill switch subsystem.

I'm missing a call to rfkill_force_state() to inform the rfkill subsystem that the key has been toggled. This function should be called when the hardware has raised the interrupt about the pressed event, or when the function which polls the register notices the state change.

As the patch works now, it means that the driver will only listen to events coming from rfkill and it doen't provide any updates itself.

Ivo

Dnia 2008-07-09, o godz. 23:33:01
Ivo van Doorn <[email protected]> napisał(a):

Quoted 10 lines · Cezary Jackiewicz

Quoted 3 lines

From: Cezary Jackiewicz <[email protected]>

Remove unnecessary attributes, use rfkill switch subsystem.

I'm missing a call to rfkill_force_state() to inform the rfkill subsystem that the key has been toggled. This function should be called when the hardware has raised the interrupt about the pressed event, or when the function which polls the register notices the state change.

Quoted 4 lines

As the patch works now, it means that the driver will only listen to events coming from rfkill and it doen't provide any updates itself.

Ivo

Does calling rfkill_force_state () is mandatory? This driver implement get_state() hook, @state is always up-to-date.

It is not mandatory if you are writing rfkill support for a driver that does not come with a rfkill switch. Such drivers can make use of the rfkill events produced by the hardware which does have such a switch.

When the hardware does have the rfkill switch, then yes rfkill_force_state() is mandatory. The get_state() callback function is optional, and allows rfkill to differentiate between soft and hardblock.

Quoted 4 lines · Ivo van Doorn

It is not mandatory if you are writing rfkill support for a driver that does not come with a rfkill switch. Such drivers can make use of the rfkill events produced by the hardware which does have such a switch.

Quoted 3 lines

When the hardware does have the rfkill switch, then yes rfkill_force_state() is mandatory. The get_state() callback function is optional, and allows rfkill to differentiate between soft and hardblock.

Do you want me to mark rfkill_force_state() as mandatory in the docs? It *IS* the preferred way to deal with firmware/hardware-initiated state changes, after all.

The rfkill subsystem will limp along without it, even when there are hardware rfkill lines... but no OSD function will work, as the system will pick up the change only when someone reads or writes to the state attribute...

Quoted 3 lines

Do you want me to mark rfkill_force_state() as mandatory in the docs? It *IS* the preferred way to deal with firmware/hardware-initiated state changes, after all.

Please do. Thanks.

Quoted 4 lines

The rfkill subsystem will limp along without it, even when there are hardware rfkill lines... but no OSD function will work, as the system will pick up the change only when someone reads or writes to the state attribute...

That reason alone is good enough for me to mark it mandatory for any drivers which features the rfkill key. :)

Does calling rfkill_force_state () is mandatory? This driver implement get_state() hook, @state is always up-to-date.