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@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
> + *
> + * 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
