2008-08-30 11:21:14

by Carlos Corbacho

[permalink] [raw]
Subject: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth

This patch implements rfkill support for the wireless and bluetooth devices
commonly found on Acer laptops.

For now, we will always poll these devices once a second to guarantee we can
catch state changes. On newer Acer laptops, it may be possible to rely on WMI
events to do this instead, and experimental support for this will be added in
a later patch.

3G has been deliberately left off for now, as we still have no way to detect
it, (nor, AFAIK, has any Linux user tried the code) and on laptops that don't
support 3G, trying to poll for the status will leave the logs full of ACPI
tracebacks.

The old sysfs interface for wireless and 3G will be removed in a later patch.

Signed-off-by: Carlos Corbacho <[email protected]>
---

drivers/misc/Kconfig | 1 +
drivers/misc/acer-wmi.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 0 deletions(-)


diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a726f3b..6abb959 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -145,6 +145,7 @@ config ACER_WMI
depends on NEW_LEDS
depends on BACKLIGHT_CLASS_DEVICE
depends on SERIO_I8042
+ depends on RFKILL
select ACPI_WMI
---help---
This is a driver for newer Acer (and Wistron) laptops. It adds
diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index b2d9878..e30d349 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -33,6 +33,8 @@
#include <linux/platform_device.h>
#include <linux/acpi.h>
#include <linux/i8042.h>
+#include <linux/rfkill.h>
+#include <linux/workqueue.h>
#include <linux/debugfs.h>

#include <acpi/acpi_drivers.h>
@@ -157,6 +159,9 @@ struct acer_debug {
u32 wmid_devices;
};

+static struct rfkill *wireless_rfkill;
+static struct rfkill *bluetooth_rfkill;
+
/* Each low-level interface must define at least some of the following */
struct wmi_interface {
/* The WMI device type */
@@ -930,6 +935,82 @@ static void acer_backlight_exit(void)
}

/*
+ * Rfkill devices
+ */
+static struct workqueue_struct *rfkill_workqueue;
+
+#define ACER_RFKILL(device, cap) \
+static void device##_rfkill_update(struct work_struct *ignored); \
+static int device##_rfkill_set(void *data, enum rfkill_state state) \
+{ \
+ acpi_status status; \
+ status = set_u32((u32) (state == RFKILL_STATE_UNBLOCKED), cap); \
+ if (ACPI_FAILURE(status)) \
+ return -ENODEV; \
+ return 0; \
+} \
+static DECLARE_DELAYED_WORK(device##_rfkill_work, device##_rfkill_update); \
+static void device##_rfkill_update(struct work_struct *ignored) \
+{ \
+ u32 state; \
+ acpi_status status; \
+ status = get_u32(&state, cap); \
+ \
+ if (ACPI_SUCCESS(status)) \
+ rfkill_force_state(device##_rfkill, state); \
+ queue_delayed_work(rfkill_workqueue, &device##_rfkill_work, \
+ round_jiffies_relative(HZ)); \
+}
+
+ACER_RFKILL(wireless, ACER_CAP_WIRELESS);
+ACER_RFKILL(bluetooth, ACER_CAP_BLUETOOTH);
+
+static int acer_rfkill_init(struct device *dev)
+{
+ u32 state;
+
+ wireless_rfkill = rfkill_allocate(dev, RFKILL_TYPE_WLAN);
+ wireless_rfkill->name = "acer-wireless";
+ get_u32(&state, ACER_CAP_WIRELESS);
+ wireless_rfkill->state = state;
+ wireless_rfkill->toggle_radio = wireless_rfkill_set;
+ wireless_rfkill->user_claim_unsupported = 1;
+
+ bluetooth_rfkill = rfkill_allocate(dev, RFKILL_TYPE_BLUETOOTH);
+ bluetooth_rfkill->name = "acer-bluetooth";
+ get_u32(&state, ACER_CAP_BLUETOOTH);
+ bluetooth_rfkill->state = state;
+ bluetooth_rfkill->toggle_radio = bluetooth_rfkill_set;
+ bluetooth_rfkill->user_claim_unsupported = 1;
+
+ rfkill_register(wireless_rfkill);
+ if (has_cap(ACER_CAP_BLUETOOTH))
+ rfkill_register(bluetooth_rfkill);
+
+ rfkill_workqueue = create_singlethread_workqueue("rfkill_workqueue");
+ if (!rfkill_workqueue)
+ goto error_rfkill_workqueue;
+ queue_delayed_work(rfkill_workqueue, &wireless_rfkill_work, HZ);
+ queue_delayed_work(rfkill_workqueue, &bluetooth_rfkill_work, HZ);
+
+ return 0;
+
+error_rfkill_workqueue:
+ return -ENODEV;
+}
+
+static void acer_rfkill_exit(void)
+{
+ cancel_delayed_work_sync(&wireless_rfkill_work);
+ cancel_delayed_work_sync(&bluetooth_rfkill_work);
+ destroy_workqueue(rfkill_workqueue);
+ rfkill_unregister(wireless_rfkill);
+ if (has_cap(ACER_CAP_BLUETOOTH))
+ rfkill_unregister(bluetooth_rfkill);
+ return;
+}
+
+/*
* Read/ write bool sysfs macro
*/
#define show_set_bool(value, cap) \
@@ -1023,8 +1104,14 @@ static int __devinit acer_platform_probe(struct platform_device *device)
goto error_brightness;
}

+ err = acer_rfkill_init(&device->dev);
+ if (err)
+ goto error_rfkill;
+
return 0;

+error_rfkill:
+ acer_rfkill_exit();
error_brightness:
acer_led_exit();
error_mailled:
@@ -1037,6 +1124,8 @@ static int acer_platform_remove(struct platform_device *device)
acer_led_exit();
if (has_cap(ACER_CAP_BRIGHTNESS))
acer_backlight_exit();
+
+ acer_rfkill_exit();
return 0;
}



2008-08-30 15:54:10

by Carlos Corbacho

[permalink] [raw]
Subject: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth (v2)

Thanks for the comments - version 2 below.

Also, is there any particular reason why rfkill explicitly doesn't save &
restore states on hibernation/ suspend-to-disk?

-Carlos
---
This patch implements rfkill support for the wireless and bluetooth devices
commonly found on Acer laptops.

For now, we will always poll these devices once a second to guarantee we can
catch state changes. On newer Acer laptops, it may be possible to rely on WMI
events to do this instead, and experimental support for this will be added in
a later patch.

3G has been deliberately left off for now, as we still have no way to detect
it, (nor, AFAIK, has any Linux user tried the code) and on laptops that don't
support 3G, trying to poll for the status will leave the logs full of ACPI
tracebacks.

The old sysfs interface for wireless and 3G will be removed in a later patch.

Signed-off-by: Carlos Corbacho <[email protected]>
---

drivers/misc/Kconfig | 1
drivers/misc/acer-wmi.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+), 0 deletions(-)


diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a726f3b..6abb959 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -145,6 +145,7 @@ config ACER_WMI
depends on NEW_LEDS
depends on BACKLIGHT_CLASS_DEVICE
depends on SERIO_I8042
+ depends on RFKILL
select ACPI_WMI
---help---
This is a driver for newer Acer (and Wistron) laptops. It adds
diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index b2d9878..2b7ee77 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -33,6 +33,8 @@
#include <linux/platform_device.h>
#include <linux/acpi.h>
#include <linux/i8042.h>
+#include <linux/rfkill.h>
+#include <linux/workqueue.h>
#include <linux/debugfs.h>

#include <acpi/acpi_drivers.h>
@@ -157,6 +159,9 @@ struct acer_debug {
u32 wmid_devices;
};

+static struct rfkill *wireless_rfkill;
+static struct rfkill *bluetooth_rfkill;
+
/* Each low-level interface must define at least some of the following */
struct wmi_interface {
/* The WMI device type */
@@ -930,6 +935,93 @@ static void acer_backlight_exit(void)
}

/*
+ * Rfkill devices
+ */
+static struct workqueue_struct *rfkill_workqueue;
+
+#define ACER_RFKILL(device, cap) \
+static void device##_rfkill_update(struct work_struct *ignored); \
+static int device##_rfkill_set(void *data, enum rfkill_state state) \
+{ \
+ acpi_status status; \
+ status = set_u32((u32) (state == RFKILL_STATE_UNBLOCKED), cap); \
+ if (ACPI_FAILURE(status)) \
+ return -ENODEV; \
+ return 0; \
+} \
+static DECLARE_DELAYED_WORK(device##_rfkill_work, device##_rfkill_update); \
+static void device##_rfkill_update(struct work_struct *ignored) \
+{ \
+ u32 state; \
+ acpi_status status; \
+ status = get_u32(&state, cap); \
+ \
+ if (ACPI_SUCCESS(status)) \
+ rfkill_force_state(device##_rfkill, state); \
+ queue_delayed_work(rfkill_workqueue, &device##_rfkill_work, \
+ round_jiffies_relative(HZ)); \
+}
+
+ACER_RFKILL(wireless, ACER_CAP_WIRELESS);
+ACER_RFKILL(bluetooth, ACER_CAP_BLUETOOTH);
+
+static int acer_rfkill_init(struct device *dev)
+{
+ int err;
+ u32 state;
+
+ wireless_rfkill = rfkill_allocate(dev, RFKILL_TYPE_WLAN);
+ if (!wireless_rfkill)
+ return -ENOMEM;
+ wireless_rfkill->name = "acer-wireless";
+ get_u32(&state, ACER_CAP_WIRELESS);
+ wireless_rfkill->state = state ? RFKILL_STATE_UNBLOCKED :
+ RFKILL_STATE_SOFT_BLOCKED;
+ wireless_rfkill->toggle_radio = wireless_rfkill_set;
+ wireless_rfkill->user_claim_unsupported = 1;
+
+ err = rfkill_register(wireless_rfkill);
+ if (err)
+ return err;
+
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ bluetooth_rfkill = rfkill_allocate(dev, RFKILL_TYPE_BLUETOOTH);
+ if (!bluetooth_rfkill)
+ return -ENOMEM;
+ bluetooth_rfkill->name = "acer-bluetooth";
+ get_u32(&state, ACER_CAP_BLUETOOTH);
+ bluetooth_rfkill->state = state ? RFKILL_STATE_UNBLOCKED :
+ RFKILL_STATE_SOFT_BLOCKED;
+ bluetooth_rfkill->toggle_radio = bluetooth_rfkill_set;
+ bluetooth_rfkill->user_claim_unsupported = 1;
+ err = rfkill_register(bluetooth_rfkill);
+ if (err)
+ return err;
+ }
+
+ rfkill_workqueue = create_singlethread_workqueue("rfkill_workqueue");
+ if (!rfkill_workqueue)
+ return -ENOMEM;
+ queue_delayed_work(rfkill_workqueue, &wireless_rfkill_work, HZ);
+ queue_delayed_work(rfkill_workqueue, &bluetooth_rfkill_work, HZ);
+
+ return 0;
+}
+
+static void acer_rfkill_exit(void)
+{
+ cancel_delayed_work_sync(&wireless_rfkill_work);
+ cancel_delayed_work_sync(&bluetooth_rfkill_work);
+ destroy_workqueue(rfkill_workqueue);
+ rfkill_unregister(wireless_rfkill);
+ rfkill_free(wireless_rfkill);
+ if (has_cap(ACER_CAP_BLUETOOTH))
+ rfkill_unregister(bluetooth_rfkill);
+ rfkill_free(bluetooth_rfkill);
+ return;
+}
+
+/*
* Read/ write bool sysfs macro
*/
#define show_set_bool(value, cap) \
@@ -1023,8 +1115,14 @@ static int __devinit acer_platform_probe(struct platform_device *device)
goto error_brightness;
}

+ err = acer_rfkill_init(&device->dev);
+ if (err)
+ goto error_rfkill;
+
return 0;

+error_rfkill:
+ acer_rfkill_exit();
error_brightness:
acer_led_exit();
error_mailled:
@@ -1037,6 +1135,8 @@ static int acer_platform_remove(struct platform_device *device)
acer_led_exit();
if (has_cap(ACER_CAP_BRIGHTNESS))
acer_backlight_exit();
+
+ acer_rfkill_exit();
return 0;
}


Subject: Re: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth

On Sat, 30 Aug 2008, Carlos Corbacho wrote:
> +static void device##_rfkill_update(struct work_struct *ignored) \
> +{ \
> + u32 state; \
> + acpi_status status; \
> + status = get_u32(&state, cap); \
> + \
> + if (ACPI_SUCCESS(status)) \
> + rfkill_force_state(device##_rfkill, state); \
> + queue_delayed_work(rfkill_workqueue, &device##_rfkill_work, \
> + round_jiffies_relative(HZ)); \
> +}

Please explicitly map from state (int) to the parameter for
rfkill_force_state (enum rfkill_state). The compiler will optimize it if it
is numerically equal.

> +static int acer_rfkill_init(struct device *dev)
> +{
> + u32 state;
> +
> + wireless_rfkill = rfkill_allocate(dev, RFKILL_TYPE_WLAN);

Where's the error handling?

> + wireless_rfkill->name = "acer-wireless";
> + get_u32(&state, ACER_CAP_WIRELESS);
> + wireless_rfkill->state = state;

Needs proper map between types to be future-proof.

> + wireless_rfkill->toggle_radio = wireless_rfkill_set;
> + wireless_rfkill->user_claim_unsupported = 1;
> +
> + bluetooth_rfkill = rfkill_allocate(dev, RFKILL_TYPE_BLUETOOTH);

Where's the error handling?

> + bluetooth_rfkill->name = "acer-bluetooth";
> + get_u32(&state, ACER_CAP_BLUETOOTH);
> + bluetooth_rfkill->state = state;

Needs proper map between types to be future-proof.

BTW: you also need error handling on rfkill_register calls.

Since rfkill is a safety measure that the user will expect to be there
(after you add support and he starts using it), if you cannot register the
rfkill interfaces, you should fail the driver load.

--
"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

Subject: Re: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth (v2)

On Sat, 30 Aug 2008, Carlos Corbacho wrote:
> +static void device##_rfkill_update(struct work_struct *ignored) \
> +{ \
> + u32 state; \
> + acpi_status status; \
> + status = get_u32(&state, cap); \
> + \
> + if (ACPI_SUCCESS(status)) \
> + rfkill_force_state(device##_rfkill, state); \
> + queue_delayed_work(rfkill_workqueue, &device##_rfkill_work, \
> + round_jiffies_relative(HZ)); \
> +}

You are still doing state = (int); rfkill_force_state(foo, state);

Please do a rfkill_force_state(foo, state ? RFKILL_STATE_UNBLOCKED :
RFKILL_STATE_SOFT_BLOCKED);

or whatever is correct for what the values of (state) mean.

> + err = rfkill_register(wireless_rfkill);
> + if (err)
> + return err;

You have to manually dealocate wireless_rfkill on the error path here using
rfkill_free (and maybe set it to NULL to avoid double-free or somesuch), or
you will leak memory.

I heavily suggest re-reading the kernel doc info for every rfkill function
you're using. They're well described and documented, including caveats such
as the one above.

> + if (!rfkill_workqueue)
> + return -ENOMEM;

Needs to dealocate the stuff you alocated, unless the caller will do so.

> + destroy_workqueue(rfkill_workqueue);
> + rfkill_unregister(wireless_rfkill);
> + rfkill_free(wireless_rfkill);

I know it is somewhat surprising, but you must never rfkill_free something
you have called rfkill_unregister on. You use rfkill_free only to get rid
of memory when rfkill_register fails.

--
"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

2008-09-12 19:44:34

by Carlos Corbacho

[permalink] [raw]
Subject: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth (v4)

This patch implements rfkill support for the wireless and bluetooth devices
commonly found on Acer laptops.

For now, we will always poll these devices once a second to guarantee we can
catch state changes. On newer Acer laptops, it may be possible to rely on WMI
events to do this instead, and experimental support for this will be added in
a later patch.

3G has been deliberately left off for now, as we still have no way to detect
it, (nor, AFAIK, has any Linux user tried the code) and on laptops that don't
support 3G, trying to poll for the status will leave the logs full of ACPI
tracebacks.

The old sysfs interface for wireless and 3G will be removed in a later patch.

Signed-off-by: Carlos Corbacho <[email protected]>
---

drivers/misc/Kconfig | 1
drivers/misc/acer-wmi.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 131 insertions(+), 1 deletions(-)


diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a726f3b..6abb959 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -145,6 +145,7 @@ config ACER_WMI
depends on NEW_LEDS
depends on BACKLIGHT_CLASS_DEVICE
depends on SERIO_I8042
+ depends on RFKILL
select ACPI_WMI
---help---
This is a driver for newer Acer (and Wistron) laptops. It adds
diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index b2d9878..3da0a69 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -33,6 +33,8 @@
#include <linux/platform_device.h>
#include <linux/acpi.h>
#include <linux/i8042.h>
+#include <linux/rfkill.h>
+#include <linux/workqueue.h>
#include <linux/debugfs.h>

#include <acpi/acpi_drivers.h>
@@ -157,6 +159,9 @@ struct acer_debug {
u32 wmid_devices;
};

+static struct rfkill *wireless_rfkill;
+static struct rfkill *bluetooth_rfkill;
+
/* Each low-level interface must define at least some of the following */
struct wmi_interface {
/* The WMI device type */
@@ -930,6 +935,126 @@ static void acer_backlight_exit(void)
}

/*
+ * Rfkill devices
+ */
+static struct workqueue_struct *rfkill_workqueue;
+
+static void acer_rfkill_update(struct work_struct *ignored);
+static DECLARE_DELAYED_WORK(acer_rfkill_work, acer_rfkill_update);
+static void acer_rfkill_update(struct work_struct *ignored)
+{
+ u32 state;
+ acpi_status status;
+
+ status = get_u32(&state, ACER_CAP_WIRELESS);
+ if (ACPI_SUCCESS(status))
+ rfkill_force_state(wireless_rfkill, state ?
+ RFKILL_STATE_UNBLOCKED : RFKILL_STATE_SOFT_BLOCKED);
+
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ status = get_u32(&state, ACER_CAP_BLUETOOTH);
+ if (ACPI_SUCCESS(status))
+ rfkill_force_state(bluetooth_rfkill, state ?
+ RFKILL_STATE_UNBLOCKED :
+ RFKILL_STATE_SOFT_BLOCKED);
+ }
+
+ queue_delayed_work(rfkill_workqueue, &acer_rfkill_work,
+ round_jiffies_relative(HZ));
+}
+
+static int acer_rfkill_set(void *data, enum rfkill_state state)
+{
+ acpi_status status;
+ u32 *cap = data;
+ status = set_u32((u32) (state == RFKILL_STATE_UNBLOCKED), *cap);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+ return 0;
+}
+
+static int acer_rfkill_register(struct device *dev, struct rfkill **rfkill_dev,
+enum rfkill_type type, char *name, u32 cap)
+{
+ int err;
+ u32 state;
+ u32 *data;
+
+ *rfkill_dev = rfkill_allocate(dev, type);
+ if (!*rfkill_dev)
+ return -ENOMEM;
+ (*rfkill_dev)->name = name;
+ get_u32(&state, cap);
+ (*rfkill_dev)->state = state ? RFKILL_STATE_UNBLOCKED :
+ RFKILL_STATE_SOFT_BLOCKED;
+ data = kzalloc(sizeof(u32), GFP_KERNEL);
+ if (!data) {
+ rfkill_free(*rfkill_dev);
+ return -ENOMEM;
+ }
+ *data = cap;
+ (*rfkill_dev)->data = data;
+ (*rfkill_dev)->toggle_radio = acer_rfkill_set;
+ (*rfkill_dev)->user_claim_unsupported = 1;
+
+ err = rfkill_register(*rfkill_dev);
+ if (err) {
+ kfree((*rfkill_dev)->data);
+ rfkill_free(*rfkill_dev);
+ return err;
+ }
+ return 0;
+}
+
+static int acer_rfkill_init(struct device *dev)
+{
+ int err;
+
+ err = acer_rfkill_register(dev, &wireless_rfkill, RFKILL_TYPE_WLAN,
+ "acer-wireless", ACER_CAP_WIRELESS);
+ if (err)
+ return err;
+
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ err = acer_rfkill_register(dev, &bluetooth_rfkill,
+ RFKILL_TYPE_BLUETOOTH, "acer-bluetooth",
+ ACER_CAP_BLUETOOTH);
+ if (err) {
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(wireless_rfkill);
+ return err;
+ }
+ }
+
+ rfkill_workqueue = create_singlethread_workqueue("rfkill_workqueue");
+ if (!rfkill_workqueue) {
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ kfree(bluetooth_rfkill->data);
+ rfkill_unregister(bluetooth_rfkill);
+ }
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(wireless_rfkill);
+ return -ENOMEM;
+ }
+ queue_delayed_work(rfkill_workqueue, &acer_rfkill_work, HZ);
+
+ return 0;
+}
+
+static void acer_rfkill_exit(void)
+{
+ cancel_delayed_work_sync(&acer_rfkill_work);
+ destroy_workqueue(rfkill_workqueue);
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(wireless_rfkill);
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(bluetooth_rfkill);
+ }
+ return;
+}
+
+/*
* Read/ write bool sysfs macro
*/
#define show_set_bool(value, cap) \
@@ -1023,7 +1148,9 @@ static int __devinit acer_platform_probe(struct platform_device *device)
goto error_brightness;
}

- return 0;
+ err = acer_rfkill_init(&device->dev);
+
+ return err;

error_brightness:
acer_led_exit();
@@ -1037,6 +1164,8 @@ static int acer_platform_remove(struct platform_device *device)
acer_led_exit();
if (has_cap(ACER_CAP_BRIGHTNESS))
acer_backlight_exit();
+
+ acer_rfkill_exit();
return 0;
}


2008-09-14 12:18:13

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth (v5)

On Fri, Sep 12, 2008 at 11:04:19PM +0100, Carlos Corbacho wrote:
> This patch implements rfkill support for the wireless and bluetooth devices
> commonly found on Acer laptops.

Yep, looks good, thanks.

--
Dmitry

Subject: Re: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth (v3)

On Sun, 07 Sep 2008, Carlos Corbacho wrote:
> + status = get_u32(&state, ACER_CAP_WIRELESS);
> + if (ACPI_SUCCESS(status))
> + rfkill_force_state(wireless_rfkill, state);

Please fix all calls to rfkill_force_state(). Refer to my previous post on
this thread.

--
"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

2008-09-12 22:04:28

by Carlos Corbacho

[permalink] [raw]
Subject: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth (v5)

This patch implements rfkill support for the wireless and bluetooth devices
commonly found on Acer laptops.

For now, we will always poll these devices once a second to guarantee we can
catch state changes. On newer Acer laptops, it may be possible to rely on WMI
events to do this instead, and experimental support for this will be added in
a later patch.

3G has been deliberately left off for now, as we still have no way to detect
it, (nor, AFAIK, has any Linux user tried the code) and on laptops that don't
support 3G, trying to poll for the status will leave the logs full of ACPI
tracebacks.

The old sysfs interface for wireless and 3G will be removed in a later patch.

Signed-off-by: Carlos Corbacho <[email protected]>
---

drivers/misc/Kconfig | 1
drivers/misc/acer-wmi.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 130 insertions(+), 1 deletions(-)


diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a726f3b..6abb959 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -145,6 +145,7 @@ config ACER_WMI
depends on NEW_LEDS
depends on BACKLIGHT_CLASS_DEVICE
depends on SERIO_I8042
+ depends on RFKILL
select ACPI_WMI
---help---
This is a driver for newer Acer (and Wistron) laptops. It adds
diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index b2d9878..11263f0 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -33,6 +33,8 @@
#include <linux/platform_device.h>
#include <linux/acpi.h>
#include <linux/i8042.h>
+#include <linux/rfkill.h>
+#include <linux/workqueue.h>
#include <linux/debugfs.h>

#include <acpi/acpi_drivers.h>
@@ -157,6 +159,9 @@ struct acer_debug {
u32 wmid_devices;
};

+static struct rfkill *wireless_rfkill;
+static struct rfkill *bluetooth_rfkill;
+
/* Each low-level interface must define at least some of the following */
struct wmi_interface {
/* The WMI device type */
@@ -930,6 +935,125 @@ static void acer_backlight_exit(void)
}

/*
+ * Rfkill devices
+ */
+static struct workqueue_struct *rfkill_workqueue;
+
+static void acer_rfkill_update(struct work_struct *ignored);
+static DECLARE_DELAYED_WORK(acer_rfkill_work, acer_rfkill_update);
+static void acer_rfkill_update(struct work_struct *ignored)
+{
+ u32 state;
+ acpi_status status;
+
+ status = get_u32(&state, ACER_CAP_WIRELESS);
+ if (ACPI_SUCCESS(status))
+ rfkill_force_state(wireless_rfkill, state ?
+ RFKILL_STATE_UNBLOCKED : RFKILL_STATE_SOFT_BLOCKED);
+
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ status = get_u32(&state, ACER_CAP_BLUETOOTH);
+ if (ACPI_SUCCESS(status))
+ rfkill_force_state(bluetooth_rfkill, state ?
+ RFKILL_STATE_UNBLOCKED :
+ RFKILL_STATE_SOFT_BLOCKED);
+ }
+
+ queue_delayed_work(rfkill_workqueue, &acer_rfkill_work,
+ round_jiffies_relative(HZ));
+}
+
+static int acer_rfkill_set(void *data, enum rfkill_state state)
+{
+ acpi_status status;
+ u32 *cap = data;
+ status = set_u32((u32) (state == RFKILL_STATE_UNBLOCKED), *cap);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+ return 0;
+}
+
+static struct rfkill * acer_rfkill_register(struct device *dev,
+enum rfkill_type type, char *name, u32 cap)
+{
+ int err;
+ u32 state;
+ u32 *data;
+ struct rfkill *rfkill_dev;
+
+ rfkill_dev = rfkill_allocate(dev, type);
+ if (!rfkill_dev)
+ return ERR_PTR(-ENOMEM);
+ rfkill_dev->name = name;
+ get_u32(&state, cap);
+ rfkill_dev->state = state ? RFKILL_STATE_UNBLOCKED :
+ RFKILL_STATE_SOFT_BLOCKED;
+ data = kzalloc(sizeof(u32), GFP_KERNEL);
+ if (!data) {
+ rfkill_free(rfkill_dev);
+ return ERR_PTR(-ENOMEM);
+ }
+ *data = cap;
+ rfkill_dev->data = data;
+ rfkill_dev->toggle_radio = acer_rfkill_set;
+ rfkill_dev->user_claim_unsupported = 1;
+
+ err = rfkill_register(rfkill_dev);
+ if (err) {
+ kfree(rfkill_dev->data);
+ rfkill_free(rfkill_dev);
+ return ERR_PTR(err);
+ }
+ return rfkill_dev;
+}
+
+static int acer_rfkill_init(struct device *dev)
+{
+ wireless_rfkill = acer_rfkill_register(dev, RFKILL_TYPE_WLAN,
+ "acer-wireless", ACER_CAP_WIRELESS);
+ if (IS_ERR(wireless_rfkill))
+ return PTR_ERR(wireless_rfkill);
+
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ bluetooth_rfkill = acer_rfkill_register(dev,
+ RFKILL_TYPE_BLUETOOTH, "acer-bluetooth",
+ ACER_CAP_BLUETOOTH);
+ if (IS_ERR(bluetooth_rfkill)) {
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(wireless_rfkill);
+ return PTR_ERR(bluetooth_rfkill);
+ }
+ }
+
+ rfkill_workqueue = create_singlethread_workqueue("rfkill_workqueue");
+ if (!rfkill_workqueue) {
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ kfree(bluetooth_rfkill->data);
+ rfkill_unregister(bluetooth_rfkill);
+ }
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(wireless_rfkill);
+ return -ENOMEM;
+ }
+ queue_delayed_work(rfkill_workqueue, &acer_rfkill_work, HZ);
+
+ return 0;
+}
+
+static void acer_rfkill_exit(void)
+{
+ cancel_delayed_work_sync(&acer_rfkill_work);
+ destroy_workqueue(rfkill_workqueue);
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(wireless_rfkill);
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(bluetooth_rfkill);
+ }
+ return;
+}
+
+/*
* Read/ write bool sysfs macro
*/
#define show_set_bool(value, cap) \
@@ -1023,7 +1147,9 @@ static int __devinit acer_platform_probe(struct platform_device *device)
goto error_brightness;
}

- return 0;
+ err = acer_rfkill_init(&device->dev);
+
+ return err;

error_brightness:
acer_led_exit();
@@ -1037,6 +1163,8 @@ static int acer_platform_remove(struct platform_device *device)
acer_led_exit();
if (has_cap(ACER_CAP_BRIGHTNESS))
acer_backlight_exit();
+
+ acer_rfkill_exit();
return 0;
}


Subject: Re: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth (v4)

Seems good at first glance, I have no further reservations about this patch.

Acked-by: Henrique de Moraes Holschuh <[email protected]>

--
"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

2008-09-12 20:43:15

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth (v4)

Hi Carlos,

On Fri, Sep 12, 2008 at 08:44:26PM +0100, Carlos Corbacho wrote:
> +
> +static int acer_rfkill_register(struct device *dev, struct rfkill **rfkill_dev,
> +enum rfkill_type type, char *name, u32 cap)
> +{
> + int err;
> + u32 state;
> + u32 *data;
> +
> + *rfkill_dev = rfkill_allocate(dev, type);
> + if (!*rfkill_dev)
> + return -ENOMEM;
> + (*rfkill_dev)->name = name;
> + get_u32(&state, cap);
> + (*rfkill_dev)->state = state ? RFKILL_STATE_UNBLOCKED :
> + RFKILL_STATE_SOFT_BLOCKED;
> + data = kzalloc(sizeof(u32), GFP_KERNEL);
> + if (!data) {
> + rfkill_free(*rfkill_dev);
> + return -ENOMEM;
> + }
> + *data = cap;
> + (*rfkill_dev)->data = data;
> + (*rfkill_dev)->toggle_radio = acer_rfkill_set;
> + (*rfkill_dev)->user_claim_unsupported = 1;
> +
> + err = rfkill_register(*rfkill_dev);
> + if (err) {
> + kfree((*rfkill_dev)->data);
> + rfkill_free(*rfkill_dev);
> + return err;
> + }
> + return 0;
> +}
> +

Sorry if I am late to the party but this function is best to have
'struct rfkill *' return value and use ERR_PTR() to signal errors.

--
Dmitry

2008-09-07 17:13:30

by Carlos Corbacho

[permalink] [raw]
Subject: [RFC] acer-wmi: Add rfkill support for wireless and bluetooth (v3)

acer-wmi: Add rfkill support for wireless and bluetooth

From: Carlos Corbacho <[email protected]>

This patch implements rfkill support for the wireless and bluetooth devices
commonly found on Acer laptops.

For now, we will always poll these devices once a second to guarantee we can
catch state changes. On newer Acer laptops, it may be possible to rely on WMI
events to do this instead, and experimental support for this will be added in
a later patch.

3G has been deliberately left off for now, as we still have no way to detect
it, (nor, AFAIK, has any Linux user tried the code) and on laptops that don't
support 3G, trying to poll for the status will leave the logs full of ACPI
tracebacks.

The old sysfs interface for wireless and 3G will be removed in a later patch.

Signed-off-by: Carlos Corbacho <[email protected]>
---

drivers/misc/Kconfig | 1
drivers/misc/acer-wmi.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 128 insertions(+), 1 deletions(-)


diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a726f3b..6abb959 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -145,6 +145,7 @@ config ACER_WMI
depends on NEW_LEDS
depends on BACKLIGHT_CLASS_DEVICE
depends on SERIO_I8042
+ depends on RFKILL
select ACPI_WMI
---help---
This is a driver for newer Acer (and Wistron) laptops. It adds
diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index b2d9878..860c34d 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -33,6 +33,8 @@
#include <linux/platform_device.h>
#include <linux/acpi.h>
#include <linux/i8042.h>
+#include <linux/rfkill.h>
+#include <linux/workqueue.h>
#include <linux/debugfs.h>

#include <acpi/acpi_drivers.h>
@@ -157,6 +159,9 @@ struct acer_debug {
u32 wmid_devices;
};

+static struct rfkill *wireless_rfkill;
+static struct rfkill *bluetooth_rfkill;
+
/* Each low-level interface must define at least some of the following */
struct wmi_interface {
/* The WMI device type */
@@ -930,6 +935,123 @@ static void acer_backlight_exit(void)
}

/*
+ * Rfkill devices
+ */
+static struct workqueue_struct *rfkill_workqueue;
+
+static void acer_rfkill_update(struct work_struct *ignored);
+static DECLARE_DELAYED_WORK(acer_rfkill_work, acer_rfkill_update);
+static void acer_rfkill_update(struct work_struct *ignored)
+{
+ u32 state;
+ acpi_status status;
+
+ status = get_u32(&state, ACER_CAP_WIRELESS);
+ if (ACPI_SUCCESS(status))
+ rfkill_force_state(wireless_rfkill, state);
+
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ status = get_u32(&state, ACER_CAP_BLUETOOTH);
+ if (ACPI_SUCCESS(status))
+ rfkill_force_state(bluetooth_rfkill, state);
+ }
+
+ queue_delayed_work(rfkill_workqueue, &acer_rfkill_work,
+ round_jiffies_relative(HZ));
+}
+
+static int acer_rfkill_set(void *data, enum rfkill_state state)
+{
+ acpi_status status;
+ u32 *cap = data;
+ status = set_u32((u32) (state == RFKILL_STATE_UNBLOCKED), *cap);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+ return 0;
+}
+
+static int acer_rfkill_register(struct device *dev, struct rfkill **rfkill_dev,
+enum rfkill_type type, char *name, u32 cap)
+{
+ int err;
+ u32 state;
+ u32 *data;
+
+ *rfkill_dev = rfkill_allocate(dev, type);
+ if (!*rfkill_dev)
+ return -ENOMEM;
+ (*rfkill_dev)->name = name;
+ get_u32(&state, cap);
+ (*rfkill_dev)->state = state ? RFKILL_STATE_UNBLOCKED :
+ RFKILL_STATE_SOFT_BLOCKED;
+ data = kzalloc(sizeof(u32), GFP_KERNEL);
+ if (!data) {
+ rfkill_free(*rfkill_dev);
+ return -ENOMEM;
+ }
+ *data = cap;
+ (*rfkill_dev)->data = data;
+ (*rfkill_dev)->toggle_radio = acer_rfkill_set;
+ (*rfkill_dev)->user_claim_unsupported = 1;
+
+ err = rfkill_register(*rfkill_dev);
+ if (err) {
+ kfree((*rfkill_dev)->data);
+ rfkill_free(*rfkill_dev);
+ return err;
+ }
+ return 0;
+}
+
+static int acer_rfkill_init(struct device *dev)
+{
+ int err;
+
+ err = acer_rfkill_register(dev, &wireless_rfkill, RFKILL_TYPE_WLAN,
+ "acer-wireless", ACER_CAP_WIRELESS);
+ if (err)
+ return err;
+
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ err = acer_rfkill_register(dev, &bluetooth_rfkill,
+ RFKILL_TYPE_BLUETOOTH, "acer-bluetooth",
+ ACER_CAP_BLUETOOTH);
+ if (err) {
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(wireless_rfkill);
+ return err;
+ }
+ }
+
+ rfkill_workqueue = create_singlethread_workqueue("rfkill_workqueue");
+ if (!rfkill_workqueue) {
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ kfree(bluetooth_rfkill->data);
+ rfkill_unregister(bluetooth_rfkill);
+ }
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(wireless_rfkill);
+ return -ENOMEM;
+ }
+ queue_delayed_work(rfkill_workqueue, &acer_rfkill_work, HZ);
+
+ return 0;
+}
+
+static void acer_rfkill_exit(void)
+{
+ cancel_delayed_work_sync(&acer_rfkill_work);
+ destroy_workqueue(rfkill_workqueue);
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(wireless_rfkill);
+ if (has_cap(ACER_CAP_BLUETOOTH)) {
+ kfree(wireless_rfkill->data);
+ rfkill_unregister(bluetooth_rfkill);
+ }
+ return;
+}
+
+/*
* Read/ write bool sysfs macro
*/
#define show_set_bool(value, cap) \
@@ -1023,7 +1145,9 @@ static int __devinit acer_platform_probe(struct platform_device *device)
goto error_brightness;
}

- return 0;
+ err = acer_rfkill_init(&device->dev);
+
+ return err;

error_brightness:
acer_led_exit();
@@ -1037,6 +1161,8 @@ static int acer_platform_remove(struct platform_device *device)
acer_led_exit();
if (has_cap(ACER_CAP_BRIGHTNESS))
acer_backlight_exit();
+
+ acer_rfkill_exit();
return 0;
}