[PATCH] asus-radio: new driver for asus radio button for Windows 8

[PATCH] asus-radio: new driver for asus radio button for Windows 8

[Alex Hung]

ASUS introduced a new approach to handle wireless hotkey
since Windows 8.  When the hotkey is pressed, BIOS generates
a notification 0x88 to a new ACPI device, ATK4001.  This
new driver not only translates the notification to KEY_RFKILL
but also toggles its LED accordingly.

Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 drivers/platform/x86/Kconfig      |   11 ++
 drivers/platform/x86/Makefile     |    1 +
 drivers/platform/x86/asus-radio.c |  238 +++++++++++++++++++++++++++++++++++++
 3 files changed, 250 insertions(+)
 create mode 100644 drivers/platform/x86/asus-radio.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 638e7970..d2adc79 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -516,6 +516,17 @@ config EEEPC_LAPTOP
 	  If you have an Eee PC laptop, say Y or M here. If this driver
 	  doesn't work on your Eee PC, try eeepc-wmi instead.
 
+config ASUS_RADIO
+	tristate "ASUS radio button"
+	depends on ACPI
+	depends on INPUT
+	help
+	 This driver provides supports for new ASUS radio button for Windows 8.
+	 On such systems the driver should load automatically (via ACPI alias).
+
+	 To compile this driver as a module, choose M here: the module will
+	 be called hp-wireless.
+
 config ASUS_WMI
 	tristate "ASUS WMI Driver"
 	depends on ACPI_WMI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index f82232b..aecd403 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -3,6 +3,7 @@
 # x86 Platform-Specific Drivers
 #
 obj-$(CONFIG_ASUS_LAPTOP)	+= asus-laptop.o
+obj-$(CONFIG_ASUS_RADIO)	+= asus-radio.o
 obj-$(CONFIG_ASUS_WMI)		+= asus-wmi.o
 obj-$(CONFIG_ASUS_NB_WMI)	+= asus-nb-wmi.o
 obj-$(CONFIG_EEEPC_LAPTOP)	+= eeepc-laptop.o
diff --git a/drivers/platform/x86/asus-radio.c b/drivers/platform/x86/asus-radio.c
new file mode 100644
index 0000000..4d519ce
--- /dev/null
+++ b/drivers/platform/x86/asus-radio.c
@@ -0,0 +1,238 @@
+/*
+ *  asus-radio button for Windows 8
+ *
+ *  Copyright (C) 2015 Alex Hung <alex.hung@canonical.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/platform_device.h>
+#include <linux/acpi.h>
+#include <acpi/acpi_bus.h>
+#include <linux/rfkill.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Alex Hung");
+MODULE_ALIAS("acpi*:ATK4001:*");
+
+static struct platform_device *asuspl_dev;
+static struct input_dev *asusrb_input_dev;
+static struct rfkill *asus_rfkill;
+static struct acpi_device *asus_radio_device;
+static int radio_led_state;
+
+static const struct acpi_device_id asusrb_ids[] = {
+	{"ATK4001", 0},
+	{"", 0},
+};
+
+static int asus_led_set(bool blocked)
+{
+	acpi_status status;
+	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
+	struct acpi_object_list args = { 1, &arg0 };
+	unsigned long long output;
+
+	arg0.integer.value = blocked;
+	status = acpi_evaluate_integer(asus_radio_device->handle, "HSWC",
+				       &args, &output);
+	if (!ACPI_SUCCESS(status) || output == 0) {
+		pr_err("fail to change wireless LED.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int asus_rfk_set(void *data, bool blocked)
+{
+	radio_led_state = blocked ? 0 : 1;
+
+	return asus_led_set(radio_led_state);
+}
+
+static const struct rfkill_ops asus_rfk_ops = {
+	.set_block = asus_rfk_set,
+};
+
+static int asusrb_rfkill_setup(struct acpi_device *device)
+{
+	int err = 0;
+
+	asus_rfkill = rfkill_alloc("asus_radio",
+				   &device->dev,
+				   RFKILL_TYPE_WLAN,
+				   &asus_rfk_ops,
+				   device);
+	if (!asus_rfkill) {
+		pr_err("unable to allocate rfkill device\n");
+		return -ENOMEM;
+	}
+
+	err = rfkill_register(asus_rfkill);
+	if (err) {
+		pr_err("unable to register rfkill device\n");
+		rfkill_destroy(asus_rfkill);
+	}
+
+	return err;
+}
+
+static int asus_radio_input_setup(void)
+{
+	int err;
+
+	asusrb_input_dev = input_allocate_device();
+	if (!asusrb_input_dev)
+		return -ENOMEM;
+
+	asusrb_input_dev->name = "ASUS radio hotkeys";
+	asusrb_input_dev->phys = "atk4001/input0";
+	asusrb_input_dev->id.bustype = BUS_HOST;
+	asusrb_input_dev->evbit[0] = BIT(EV_KEY);
+	set_bit(KEY_RFKILL, asusrb_input_dev->keybit);
+
+	err = input_register_device(asusrb_input_dev);
+	if (err)
+		goto err_free_dev;
+
+	return 0;
+
+err_free_dev:
+	input_free_device(asusrb_input_dev);
+	return err;
+}
+
+static void asus_radio_input_destroy(void)
+{
+	input_unregister_device(asusrb_input_dev);
+}
+
+static void asusrb_notify(struct acpi_device *acpi_dev, u32 event)
+{
+	if (event != 0x88) {
+		pr_err("received unknown event (0x%x)\n", event);
+		return;
+	}
+
+	input_report_key(asusrb_input_dev, KEY_RFKILL, 1);
+	input_sync(asusrb_input_dev);
+	input_report_key(asusrb_input_dev, KEY_RFKILL, 0);
+	input_sync(asusrb_input_dev);
+}
+
+static int asusrb_add(struct acpi_device *device)
+{
+	int err;
+
+	asus_radio_device = device;
+
+	err = asus_radio_input_setup();
+	if (err) {
+		pr_err("failed to setup asus_radio hotkeys\n");
+		goto failed;
+	}
+
+	err = asusrb_rfkill_setup(device);
+	if (err)
+		pr_err("failed to setup asus_radio rfkill\n");
+ failed:
+	return err;
+}
+
+static int asusrb_remove(struct acpi_device *device)
+{
+	asus_radio_input_destroy();
+
+	if (asus_rfkill) {
+		rfkill_unregister(asus_rfkill);
+		rfkill_destroy(asus_rfkill);
+	}
+
+	return 0;
+}
+
+static struct acpi_driver asusrb_driver = {
+	.name	= "asus acpi radio",
+	.owner	= THIS_MODULE,
+	.ids	= asusrb_ids,
+	.ops	= {
+		.add	= asusrb_add,
+		.remove	= asusrb_remove,
+		.notify	= asusrb_notify,
+	},
+};
+
+static int asusrb_resume_handler(struct device *device)
+{
+	return asus_led_set(radio_led_state);
+}
+
+static const struct dev_pm_ops asuspl_pm_ops = {
+	.resume  = asusrb_resume_handler,
+};
+
+static struct platform_driver asuspl_driver = {
+	.driver = {
+		.name = "asus-radio",
+		.pm = &asuspl_pm_ops,
+	},
+};
+
+static int __init asusrb_init(void)
+{
+	int err;
+
+	pr_info("Initializing ATK4001 module\n");
+	err = acpi_bus_register_driver(&asusrb_driver);
+	if (err)
+		goto err_driver_reg;
+
+	err = platform_driver_register(&asuspl_driver);
+	if (err)
+		goto err_driver_reg;
+
+	asuspl_dev = platform_device_alloc("asus-radio", -1);
+	if (!asuspl_dev) {
+		err = -ENOMEM;
+		goto err_device_alloc;
+	}
+	err = platform_device_add(asuspl_dev);
+	if (err)
+		goto err_device_add;
+
+	return 0;
+
+err_device_add:
+	platform_device_put(asuspl_dev);
+err_device_alloc:
+	platform_driver_unregister(&asuspl_driver);
+err_driver_reg:
+	return err;
+}
+
+static void __exit asusrb_exit(void)
+{
+	pr_info("Exiting ATK4001 module\n");
+	acpi_bus_unregister_driver(&asusrb_driver);
+
+	if (asuspl_dev) {
+		platform_device_unregister(asuspl_dev);
+		platform_driver_unregister(&asuspl_driver);
+	}
+}
+
+module_init(asusrb_init);
+module_exit(asusrb_exit);
-- 
1.7.9.5

Re: [PATCH] asus-radio: new driver for asus radio button for Windows 8

[Darren Hart]

On Mon, Jun 22, 2015 at 05:20:05PM +0800, Alex Hung wrote:
> ASUS introduced a new approach to handle wireless hotkey
> since Windows 8.  When the hotkey is pressed, BIOS generates
> a notification 0x88 to a new ACPI device, ATK4001.  This
> new driver not only translates the notification to KEY_RFKILL
> but also toggles its LED accordingly.
> 
> Signed-off-by: Alex Hung <alex.hung@canonical.com>


Hi Alex,

Will you be maintaining this driver? If so, please add it to MAINTAINERS as part
of this patch. If not.... why not? ;-)

Would you object to renaming it to "asus-rbtn.c" and using CONFIG_ASUS_RBTN,
just to keep it similar to the recently added dell-rbtn - also the rbtn suffix
is a bit more descriptive of the driver function in my opinion.

Couple minor things below, otherwise appears to be in pretty good shape. How
many systems have you tested this on?


> ---
>  drivers/platform/x86/Kconfig      |   11 ++
>  drivers/platform/x86/Makefile     |    1 +
>  drivers/platform/x86/asus-radio.c |  238 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 250 insertions(+)
>  create mode 100644 drivers/platform/x86/asus-radio.c
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 638e7970..d2adc79 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -516,6 +516,17 @@ config EEEPC_LAPTOP
>  	  If you have an Eee PC laptop, say Y or M here. If this driver
>  	  doesn't work on your Eee PC, try eeepc-wmi instead.
>  
> +config ASUS_RADIO
> +	tristate "ASUS radio button"
> +	depends on ACPI
> +	depends on INPUT
> +	help
> +	 This driver provides supports for new ASUS radio button for Windows 8.
> +	 On such systems the driver should load automatically (via ACPI alias).
> +
> +	 To compile this driver as a module, choose M here: the module will
> +	 be called hp-wireless.
> +
>  config ASUS_WMI
>  	tristate "ASUS WMI Driver"
>  	depends on ACPI_WMI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index f82232b..aecd403 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -3,6 +3,7 @@
>  # x86 Platform-Specific Drivers
>  #
>  obj-$(CONFIG_ASUS_LAPTOP)	+= asus-laptop.o
> +obj-$(CONFIG_ASUS_RADIO)	+= asus-radio.o
>  obj-$(CONFIG_ASUS_WMI)		+= asus-wmi.o
>  obj-$(CONFIG_ASUS_NB_WMI)	+= asus-nb-wmi.o
>  obj-$(CONFIG_EEEPC_LAPTOP)	+= eeepc-laptop.o
> diff --git a/drivers/platform/x86/asus-radio.c b/drivers/platform/x86/asus-radio.c
> new file mode 100644
> index 0000000..4d519ce
> --- /dev/null
> +++ b/drivers/platform/x86/asus-radio.c
> @@ -0,0 +1,238 @@
> +/*
> + *  asus-radio button for Windows 8
> + *
> + *  Copyright (C) 2015 Alex Hung <alex.hung@canonical.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/platform_device.h>
> +#include <linux/acpi.h>
> +#include <acpi/acpi_bus.h>
> +#include <linux/rfkill.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Alex Hung");
> +MODULE_ALIAS("acpi*:ATK4001:*");
> +
> +static struct platform_device *asuspl_dev;
> +static struct input_dev *asusrb_input_dev;
> +static struct rfkill *asus_rfkill;
> +static struct acpi_device *asus_radio_device;
> +static int radio_led_state;
> +
> +static const struct acpi_device_id asusrb_ids[] = {
> +	{"ATK4001", 0},
> +	{"", 0},
> +};
> +
> +static int asus_led_set(bool blocked)
> +{
> +	acpi_status status;
> +	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> +	struct acpi_object_list args = { 1, &arg0 };
> +	unsigned long long output;
> +
> +	arg0.integer.value = blocked;
> +	status = acpi_evaluate_integer(asus_radio_device->handle, "HSWC",
> +				       &args, &output);
> +	if (!ACPI_SUCCESS(status) || output == 0) {
> +		pr_err("fail to change wireless LED.\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int asus_rfk_set(void *data, bool blocked)

*rfkill_set is more common and is more explicit

> +{
> +	radio_led_state = blocked ? 0 : 1;
> +
> +	return asus_led_set(radio_led_state);
> +}
> +
> +static const struct rfkill_ops asus_rfk_ops = {

asus_rfkill_ops is more common and more explicit (I'd prefer rfkill to rfk
wherever possible).

> +	.set_block = asus_rfk_set,
> +};
> +
> +static int asusrb_rfkill_setup(struct acpi_device *device)
> +{
> +	int err = 0;

err is never used before it is assigned, no need to initialize to 0 here.

> +
> +	asus_rfkill = rfkill_alloc("asus_radio",
> +				   &device->dev,
> +				   RFKILL_TYPE_WLAN,
> +				   &asus_rfk_ops,
> +				   device);
> +	if (!asus_rfkill) {
> +		pr_err("unable to allocate rfkill device\n");
> +		return -ENOMEM;
> +	}
> +
> +	err = rfkill_register(asus_rfkill);
> +	if (err) {
> +		pr_err("unable to register rfkill device\n");
> +		rfkill_destroy(asus_rfkill);
> +	}
> +
> +	return err;
> +}
> +
> +static int asus_radio_input_setup(void)
> +{
> +	int err;
> +
> +	asusrb_input_dev = input_allocate_device();
> +	if (!asusrb_input_dev)
> +		return -ENOMEM;
> +
> +	asusrb_input_dev->name = "ASUS radio hotkeys";
> +	asusrb_input_dev->phys = "atk4001/input0";
> +	asusrb_input_dev->id.bustype = BUS_HOST;
> +	asusrb_input_dev->evbit[0] = BIT(EV_KEY);
> +	set_bit(KEY_RFKILL, asusrb_input_dev->keybit);
> +
> +	err = input_register_device(asusrb_input_dev);
> +	if (err)
> +		goto err_free_dev;
> +
> +	return 0;
> +
> +err_free_dev:
> +	input_free_device(asusrb_input_dev);
> +	return err;
> +}
> +
> +static void asus_radio_input_destroy(void)
> +{
> +	input_unregister_device(asusrb_input_dev);
> +}
> +
> +static void asusrb_notify(struct acpi_device *acpi_dev, u32 event)
> +{
> +	if (event != 0x88) {

Prefer a define to a magic number, even if it is only used the once.

> +		pr_err("received unknown event (0x%x)\n", event);
> +		return;
> +	}
> +
> +	input_report_key(asusrb_input_dev, KEY_RFKILL, 1);
> +	input_sync(asusrb_input_dev);
> +	input_report_key(asusrb_input_dev, KEY_RFKILL, 0);
> +	input_sync(asusrb_input_dev);
> +}
> +
> +static int asusrb_add(struct acpi_device *device)
> +{
> +	int err;
> +
> +	asus_radio_device = device;
> +
> +	err = asus_radio_input_setup();
> +	if (err) {
> +		pr_err("failed to setup asus_radio hotkeys\n");
> +		goto failed;
> +	}
> +
> +	err = asusrb_rfkill_setup(device);
> +	if (err)
> +		pr_err("failed to setup asus_radio rfkill\n");
> + failed:

Please be consistent with your label indentation throughout the driver. I don't
see established precedent for with or without a space. Some argue using a space
improves debugability and patch legibility - but I don't enforce one or the
other - just be consistent in the file.

Also, this label isn't really necessary, just return err in the one location
used above and drop it.

> +	return err;
> +}
> +
> +static int asusrb_remove(struct acpi_device *device)
> +{
> +	asus_radio_input_destroy();
> +
> +	if (asus_rfkill) {
> +		rfkill_unregister(asus_rfkill);
> +		rfkill_destroy(asus_rfkill);
> +	}
> +
> +	return 0;
> +}
> +
> +static struct acpi_driver asusrb_driver = {
> +	.name	= "asus acpi radio",
> +	.owner	= THIS_MODULE,
> +	.ids	= asusrb_ids,
> +	.ops	= {
> +		.add	= asusrb_add,
> +		.remove	= asusrb_remove,
> +		.notify	= asusrb_notify,
> +	},
> +};
> +
> +static int asusrb_resume_handler(struct device *device)
> +{
> +	return asus_led_set(radio_led_state);
> +}
> +
> +static const struct dev_pm_ops asuspl_pm_ops = {
> +	.resume  = asusrb_resume_handler,
> +};
> +
> +static struct platform_driver asuspl_driver = {
> +	.driver = {
> +		.name = "asus-radio",
> +		.pm = &asuspl_pm_ops,
> +	},
> +};
> +
> +static int __init asusrb_init(void)
> +{
> +	int err;
> +
> +	pr_info("Initializing ATK4001 module\n");
> +	err = acpi_bus_register_driver(&asusrb_driver);
> +	if (err)
> +		goto err_driver_reg;
> +
> +	err = platform_driver_register(&asuspl_driver);
> +	if (err)
> +		goto err_driver_reg;
> +
> +	asuspl_dev = platform_device_alloc("asus-radio", -1);
> +	if (!asuspl_dev) {
> +		err = -ENOMEM;
> +		goto err_device_alloc;
> +	}
> +	err = platform_device_add(asuspl_dev);
> +	if (err)
> +		goto err_device_add;
> +
> +	return 0;
> +
> +err_device_add:
> +	platform_device_put(asuspl_dev);
> +err_device_alloc:
> +	platform_driver_unregister(&asuspl_driver);
> +err_driver_reg:
> +	return err;
> +}
> +
> +static void __exit asusrb_exit(void)
> +{
> +	pr_info("Exiting ATK4001 module\n");
> +	acpi_bus_unregister_driver(&asusrb_driver);
> +
> +	if (asuspl_dev) {
> +		platform_device_unregister(asuspl_dev);
> +		platform_driver_unregister(&asuspl_driver);
> +	}
> +}
> +
> +module_init(asusrb_init);
> +module_exit(asusrb_exit);
> -- 
> 1.7.9.5
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH] asus-radio: new driver for asus radio button for Windows 8

[Alex Hung]

Hi Darren,

Thanks for the comments. I sent v2 patch that addresses your feedbacks.

I tested ASUS systems and the driver is able to toggle ON/OFF of
wireless devices.  That's all ASUS systems I can get my hands on right
now.  If more are available I will test them too and fix bugs if there
is any.

On Tue, Jun 23, 2015 at 4:47 AM, Darren Hart <dvhart@infradead.org> wrote:
> On Mon, Jun 22, 2015 at 05:20:05PM +0800, Alex Hung wrote:
>> ASUS introduced a new approach to handle wireless hotkey
>> since Windows 8.  When the hotkey is pressed, BIOS generates
>> a notification 0x88 to a new ACPI device, ATK4001.  This
>> new driver not only translates the notification to KEY_RFKILL
>> but also toggles its LED accordingly.
>>
>> Signed-off-by: Alex Hung <alex.hung@canonical.com>
>
>
> Hi Alex,
>
> Will you be maintaining this driver? If so, please add it to MAINTAINERS as part
> of this patch. If not.... why not? ;-)
>
> Would you object to renaming it to "asus-rbtn.c" and using CONFIG_ASUS_RBTN,
> just to keep it similar to the recently added dell-rbtn - also the rbtn suffix
> is a bit more descriptive of the driver function in my opinion.
>
> Couple minor things below, otherwise appears to be in pretty good shape. How
> many systems have you tested this on?
>
>
>> ---
>>  drivers/platform/x86/Kconfig      |   11 ++
>>  drivers/platform/x86/Makefile     |    1 +
>>  drivers/platform/x86/asus-radio.c |  238 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 250 insertions(+)
>>  create mode 100644 drivers/platform/x86/asus-radio.c
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 638e7970..d2adc79 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -516,6 +516,17 @@ config EEEPC_LAPTOP
>>         If you have an Eee PC laptop, say Y or M here. If this driver
>>         doesn't work on your Eee PC, try eeepc-wmi instead.
>>
>> +config ASUS_RADIO
>> +     tristate "ASUS radio button"
>> +     depends on ACPI
>> +     depends on INPUT
>> +     help
>> +      This driver provides supports for new ASUS radio button for Windows 8.
>> +      On such systems the driver should load automatically (via ACPI alias).
>> +
>> +      To compile this driver as a module, choose M here: the module will
>> +      be called hp-wireless.
>> +
>>  config ASUS_WMI
>>       tristate "ASUS WMI Driver"
>>       depends on ACPI_WMI
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index f82232b..aecd403 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -3,6 +3,7 @@
>>  # x86 Platform-Specific Drivers
>>  #
>>  obj-$(CONFIG_ASUS_LAPTOP)    += asus-laptop.o
>> +obj-$(CONFIG_ASUS_RADIO)     += asus-radio.o
>>  obj-$(CONFIG_ASUS_WMI)               += asus-wmi.o
>>  obj-$(CONFIG_ASUS_NB_WMI)    += asus-nb-wmi.o
>>  obj-$(CONFIG_EEEPC_LAPTOP)   += eeepc-laptop.o
>> diff --git a/drivers/platform/x86/asus-radio.c b/drivers/platform/x86/asus-radio.c
>> new file mode 100644
>> index 0000000..4d519ce
>> --- /dev/null
>> +++ b/drivers/platform/x86/asus-radio.c
>> @@ -0,0 +1,238 @@
>> +/*
>> + *  asus-radio button for Windows 8
>> + *
>> + *  Copyright (C) 2015 Alex Hung <alex.hung@canonical.com>
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/input.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/acpi.h>
>> +#include <acpi/acpi_bus.h>
>> +#include <linux/rfkill.h>
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Alex Hung");
>> +MODULE_ALIAS("acpi*:ATK4001:*");
>> +
>> +static struct platform_device *asuspl_dev;
>> +static struct input_dev *asusrb_input_dev;
>> +static struct rfkill *asus_rfkill;
>> +static struct acpi_device *asus_radio_device;
>> +static int radio_led_state;
>> +
>> +static const struct acpi_device_id asusrb_ids[] = {
>> +     {"ATK4001", 0},
>> +     {"", 0},
>> +};
>> +
>> +static int asus_led_set(bool blocked)
>> +{
>> +     acpi_status status;
>> +     union acpi_object arg0 = { ACPI_TYPE_INTEGER };
>> +     struct acpi_object_list args = { 1, &arg0 };
>> +     unsigned long long output;
>> +
>> +     arg0.integer.value = blocked;
>> +     status = acpi_evaluate_integer(asus_radio_device->handle, "HSWC",
>> +                                    &args, &output);
>> +     if (!ACPI_SUCCESS(status) || output == 0) {
>> +             pr_err("fail to change wireless LED.\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int asus_rfk_set(void *data, bool blocked)
>
> *rfkill_set is more common and is more explicit
>
>> +{
>> +     radio_led_state = blocked ? 0 : 1;
>> +
>> +     return asus_led_set(radio_led_state);
>> +}
>> +
>> +static const struct rfkill_ops asus_rfk_ops = {
>
> asus_rfkill_ops is more common and more explicit (I'd prefer rfkill to rfk
> wherever possible).
>
>> +     .set_block = asus_rfk_set,
>> +};
>> +
>> +static int asusrb_rfkill_setup(struct acpi_device *device)
>> +{
>> +     int err = 0;
>
> err is never used before it is assigned, no need to initialize to 0 here.
>
>> +
>> +     asus_rfkill = rfkill_alloc("asus_radio",
>> +                                &device->dev,
>> +                                RFKILL_TYPE_WLAN,
>> +                                &asus_rfk_ops,
>> +                                device);
>> +     if (!asus_rfkill) {
>> +             pr_err("unable to allocate rfkill device\n");
>> +             return -ENOMEM;
>> +     }
>> +
>> +     err = rfkill_register(asus_rfkill);
>> +     if (err) {
>> +             pr_err("unable to register rfkill device\n");
>> +             rfkill_destroy(asus_rfkill);
>> +     }
>> +
>> +     return err;
>> +}
>> +
>> +static int asus_radio_input_setup(void)
>> +{
>> +     int err;
>> +
>> +     asusrb_input_dev = input_allocate_device();
>> +     if (!asusrb_input_dev)
>> +             return -ENOMEM;
>> +
>> +     asusrb_input_dev->name = "ASUS radio hotkeys";
>> +     asusrb_input_dev->phys = "atk4001/input0";
>> +     asusrb_input_dev->id.bustype = BUS_HOST;
>> +     asusrb_input_dev->evbit[0] = BIT(EV_KEY);
>> +     set_bit(KEY_RFKILL, asusrb_input_dev->keybit);
>> +
>> +     err = input_register_device(asusrb_input_dev);
>> +     if (err)
>> +             goto err_free_dev;
>> +
>> +     return 0;
>> +
>> +err_free_dev:
>> +     input_free_device(asusrb_input_dev);
>> +     return err;
>> +}
>> +
>> +static void asus_radio_input_destroy(void)
>> +{
>> +     input_unregister_device(asusrb_input_dev);
>> +}
>> +
>> +static void asusrb_notify(struct acpi_device *acpi_dev, u32 event)
>> +{
>> +     if (event != 0x88) {
>
> Prefer a define to a magic number, even if it is only used the once.
>
>> +             pr_err("received unknown event (0x%x)\n", event);
>> +             return;
>> +     }
>> +
>> +     input_report_key(asusrb_input_dev, KEY_RFKILL, 1);
>> +     input_sync(asusrb_input_dev);
>> +     input_report_key(asusrb_input_dev, KEY_RFKILL, 0);
>> +     input_sync(asusrb_input_dev);
>> +}
>> +
>> +static int asusrb_add(struct acpi_device *device)
>> +{
>> +     int err;
>> +
>> +     asus_radio_device = device;
>> +
>> +     err = asus_radio_input_setup();
>> +     if (err) {
>> +             pr_err("failed to setup asus_radio hotkeys\n");
>> +             goto failed;
>> +     }
>> +
>> +     err = asusrb_rfkill_setup(device);
>> +     if (err)
>> +             pr_err("failed to setup asus_radio rfkill\n");
>> + failed:
>
> Please be consistent with your label indentation throughout the driver. I don't
> see established precedent for with or without a space. Some argue using a space
> improves debugability and patch legibility - but I don't enforce one or the
> other - just be consistent in the file.
>
> Also, this label isn't really necessary, just return err in the one location
> used above and drop it.
>
>> +     return err;
>> +}
>> +
>> +static int asusrb_remove(struct acpi_device *device)
>> +{
>> +     asus_radio_input_destroy();
>> +
>> +     if (asus_rfkill) {
>> +             rfkill_unregister(asus_rfkill);
>> +             rfkill_destroy(asus_rfkill);
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static struct acpi_driver asusrb_driver = {
>> +     .name   = "asus acpi radio",
>> +     .owner  = THIS_MODULE,
>> +     .ids    = asusrb_ids,
>> +     .ops    = {
>> +             .add    = asusrb_add,
>> +             .remove = asusrb_remove,
>> +             .notify = asusrb_notify,
>> +     },
>> +};
>> +
>> +static int asusrb_resume_handler(struct device *device)
>> +{
>> +     return asus_led_set(radio_led_state);
>> +}
>> +
>> +static const struct dev_pm_ops asuspl_pm_ops = {
>> +     .resume  = asusrb_resume_handler,
>> +};
>> +
>> +static struct platform_driver asuspl_driver = {
>> +     .driver = {
>> +             .name = "asus-radio",
>> +             .pm = &asuspl_pm_ops,
>> +     },
>> +};
>> +
>> +static int __init asusrb_init(void)
>> +{
>> +     int err;
>> +
>> +     pr_info("Initializing ATK4001 module\n");
>> +     err = acpi_bus_register_driver(&asusrb_driver);
>> +     if (err)
>> +             goto err_driver_reg;
>> +
>> +     err = platform_driver_register(&asuspl_driver);
>> +     if (err)
>> +             goto err_driver_reg;
>> +
>> +     asuspl_dev = platform_device_alloc("asus-radio", -1);
>> +     if (!asuspl_dev) {
>> +             err = -ENOMEM;
>> +             goto err_device_alloc;
>> +     }
>> +     err = platform_device_add(asuspl_dev);
>> +     if (err)
>> +             goto err_device_add;
>> +
>> +     return 0;
>> +
>> +err_device_add:
>> +     platform_device_put(asuspl_dev);
>> +err_device_alloc:
>> +     platform_driver_unregister(&asuspl_driver);
>> +err_driver_reg:
>> +     return err;
>> +}
>> +
>> +static void __exit asusrb_exit(void)
>> +{
>> +     pr_info("Exiting ATK4001 module\n");
>> +     acpi_bus_unregister_driver(&asusrb_driver);
>> +
>> +     if (asuspl_dev) {
>> +             platform_device_unregister(asuspl_dev);
>> +             platform_driver_unregister(&asuspl_driver);
>> +     }
>> +}
>> +
>> +module_init(asusrb_init);
>> +module_exit(asusrb_exit);
>> --
>> 1.7.9.5
>>
>>
>
> --
> Darren Hart
> Intel Open Source Technology Center
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in



-- 
Cheers,
Alex Hung