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 <[email protected]>
---
MAINTAINERS | 6 +
drivers/platform/x86/Kconfig | 11 ++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/asus-rbtn.c | 240 +++++++++++++++++++++++++++++++++++++++
4 files changed, 258 insertions(+)
create mode 100644 drivers/platform/x86/asus-rbtn.c
diff --git a/MAINTAINERS b/MAINTAINERS
index d8afd29..03711ce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1673,6 +1673,12 @@ S: Maintained
F: drivers/platform/x86/asus*.c
F: drivers/platform/x86/eeepc*.c
+ASUS RADIO BUTTON DRIVER
+M: Alex Hung <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/platform/x86/asus-rbtn.c
+
ASYNCHRONOUS TRANSFERS/TRANSFORMS (IOAT) API
R: Dan Williams <[email protected]>
W: http://sourceforge.net/projects/xscaleiop
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f9f205c..a8ac885 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_RBTN
+ 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 asus-rbtn.
+
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..6710bb3 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_RBTN) += asus-rbtn.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-rbtn.c b/drivers/platform/x86/asus-rbtn.c
new file mode 100644
index 0000000..a469881
--- /dev/null
+++ b/drivers/platform/x86/asus-rbtn.c
@@ -0,0 +1,240 @@
+/*
+ * asus-rbtn radio button for Windows 8
+ *
+ * Copyright (C) 2015 Alex Hung <[email protected]>
+ *
+ * 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:*");
+
+#define ASUS_RBTN_NOTIFY 0x88
+
+static struct platform_device *asuspl_dev;
+static struct input_dev *asusrb_input_dev;
+static struct rfkill *asus_rfkill;
+static struct acpi_device *asus_rbtn_device;
+static int radio_led_state;
+
+static const struct acpi_device_id asusrb_ids[] = {
+ {"ATK4001", 0},
+ {"", 0},
+};
+
+static int asus_radio_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_rbtn_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_rfkill_set(void *data, bool blocked)
+{
+ radio_led_state = blocked ? 0 : 1;
+
+ return asus_radio_led_set(radio_led_state);
+}
+
+static const struct rfkill_ops asus_rfkill_ops = {
+ .set_block = asus_rfkill_set,
+};
+
+static int asusrb_rfkill_setup(struct acpi_device *device)
+{
+ int err;
+
+ asus_rfkill = rfkill_alloc("asus_rbtn",
+ &device->dev,
+ RFKILL_TYPE_WLAN,
+ &asus_rfkill_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_rbtn_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_rbtn_input_destroy(void)
+{
+ input_unregister_device(asusrb_input_dev);
+}
+
+static void asusrb_notify(struct acpi_device *acpi_dev, u32 event)
+{
+ if (event != ASUS_RBTN_NOTIFY) {
+ 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_rbtn_device = device;
+
+ err = asus_rbtn_input_setup();
+ if (err) {
+ pr_err("failed to setup asus_rbtn hotkeys\n");
+ return err;
+ }
+
+ err = asusrb_rfkill_setup(device);
+ if (err)
+ pr_err("failed to setup asus_rbtn rfkill\n");
+
+ return err;
+}
+
+static int asusrb_remove(struct acpi_device *device)
+{
+ asus_rbtn_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 button",
+ .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_radio_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-rbtn",
+ .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-rbtn", -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.9.1
On Wed, Jun 24, 2015 at 10:57:51AM +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 <[email protected]>
> ---
> MAINTAINERS | 6 +
> drivers/platform/x86/Kconfig | 11 ++
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/asus-rbtn.c | 240 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 258 insertions(+)
> create mode 100644 drivers/platform/x86/asus-rbtn.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d8afd29..03711ce 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1673,6 +1673,12 @@ S: Maintained
> F: drivers/platform/x86/asus*.c
> F: drivers/platform/x86/eeepc*.c
>
> +ASUS RADIO BUTTON DRIVER
> +M: Alex Hung <[email protected]>
> +L: [email protected]
> +S: Maintained
> +F: drivers/platform/x86/asus-rbtn.c
> +
> ASYNCHRONOUS TRANSFERS/TRANSFORMS (IOAT) API
> R: Dan Williams <[email protected]>
> W: http://sourceforge.net/projects/xscaleiop
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index f9f205c..a8ac885 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_RBTN
> + tristate "ASUS radio button"
> + depends on ACPI
> + depends on INPUT
> + help
> + This driver provides supports for new ASUS radio button for Windows 8.
s/supports/support/
Also, avoid using "new" in the Kconfig as this lives forever, in 10 years, it
won't be so new :-)
Consider:
"This driver supports the ASUS radio button for Windows 8."
(And maybe fix the entry for HP_WIRELESS while you're at it in a separate patch)
...
> +static int asus_rbtn_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;
I missed this on the first round. There is no need for a goto here at all:
int ret;
...
ret = input_register_Device(asusrb_input_dev);
if (ret)
input_free_device(asusrb_input_dev);
return ret;
Much nicer IMHO.
Do you have a strong preference for err over ret? In most cases in this driver,
ret would be the more typical choice in my experience.
I suppose this is modeled after hp-wireless which has the same error path in
hp_wireless_input_setup I mentioned above and uses err throughout - consistency
is a good thing. I won't argue over the ret/err thing as there is precedent in
this subsystem for similar drivers.
--
Darren Hart
Intel Open Source Technology Center
On Wed, 2015-06-24 at 10:57 +0800, Alex Hung wrote:
> --- /dev/null
> +++ b/drivers/platform/x86/asus-rbtn.c
> +MODULE_ALIAS("acpi*:ATK4001:*");
This looked odd. It turned out this is the pattern that
scripts/mod/file2alias.c::do_acpi_entry() creates.
> +static const struct acpi_device_id asusrb_ids[] = {
> + {"ATK4001", 0},
> + {"", 0},
> +};
I think you should just put
MODULE_DEVICE_TABLE(acpi, asusrb_ids);
here, like all other drivers do, and drop the odd looking alias.
All others drivers except drivers/platform/x86/hp-wireless.c, that is.
(I noticed that you also wrote that driver.) It should just use
MODULE_DEVICE_TABLE() too
> +static int __init asusrb_init(void)
> +{
> + int err;
> +
> + [...]
> +
> + asuspl_dev = platform_device_alloc("asus-rbtn", -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) {
If asusrb_exit() will be run asusrb_init() must have completed
successfully before, right? And is there a way for asuspl_dev to be
NULL after asusrb_init() succeeded?
> + platform_device_unregister(asuspl_dev);
> + platform_driver_unregister(&asuspl_driver);
> + }
> +}
> +
> +module_init(asusrb_init);
> +module_exit(asusrb_exit);
Thanks,
Paul Bolle
On Thu, Jun 25, 2015 at 08:58:04AM +0200, Paul Bolle wrote:
> On Wed, 2015-06-24 at 10:57 +0800, Alex Hung wrote:
> > --- /dev/null
> > +++ b/drivers/platform/x86/asus-rbtn.c
>
> > +MODULE_ALIAS("acpi*:ATK4001:*");
>
> This looked odd. It turned out this is the pattern that
> scripts/mod/file2alias.c::do_acpi_entry() creates.
>
> > +static const struct acpi_device_id asusrb_ids[] = {
> > + {"ATK4001", 0},
> > + {"", 0},
> > +};
>
> I think you should just put
> MODULE_DEVICE_TABLE(acpi, asusrb_ids);
>
> here, like all other drivers do, and drop the odd looking alias.
>
> All others drivers except drivers/platform/x86/hp-wireless.c, that is.
> (I noticed that you also wrote that driver.) It should just use
> MODULE_DEVICE_TABLE() too
Thanks for digging in to that, it raised an eyebrow for me as well, but I didn't
dig into it after finding at least one other instance of it.... :-)
--
Darren Hart
Intel Open Source Technology Center
Hi!
On Wednesday 24 June 2015 10:57:51 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 <[email protected]>
...
> +static int asus_radio_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_rbtn_device->handle, "HSWC",
> + &args, &output);
What is this ACPI call doing? Just set LED control? Or something more?
> + if (!ACPI_SUCCESS(status) || output == 0) {
> + pr_err("fail to change wireless LED.\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int asus_rfkill_set(void *data, bool blocked)
> +{
> + radio_led_state = blocked ? 0 : 1;
> +
> + return asus_radio_led_set(radio_led_state);
> +}
In my opinion this is not good idea that "rfkill block" call from
userspace just change LED on/off state and nothing more...
If above ACPI call just change LED, then should not be this in LED
subsystem instead rfkill one? Or why do you prefer to use rfkill
interface instead led?
--
Pali Rohár
[email protected]
On Fri, Jun 26, 2015 at 10:56 PM, Pali Rohár <[email protected]> wrote:
> Hi!
>
> On Wednesday 24 June 2015 10:57:51 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 <[email protected]>
>
> ...
>
>> +static int asus_radio_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_rbtn_device->handle, "HSWC",
>> + &args, &output);
>
> What is this ACPI call doing? Just set LED control? Or something more?
>
>> + if (!ACPI_SUCCESS(status) || output == 0) {
>> + pr_err("fail to change wireless LED.\n");
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int asus_rfkill_set(void *data, bool blocked)
>> +{
>> + radio_led_state = blocked ? 0 : 1;
>> +
>> + return asus_radio_led_set(radio_led_state);
>> +}
>
> In my opinion this is not good idea that "rfkill block" call from
> userspace just change LED on/off state and nothing more...
>
> If above ACPI call just change LED, then should not be this in LED
> subsystem instead rfkill one? Or why do you prefer to use rfkill
> interface instead led?
It indeed controls LED only at the moment. My intention was to have
have everything work without the need to modify any userspace
applications. Current it is 1) aus-rbtn issues KEY_RFKILL 2) an
userspace application changes rfkill states, and 3) both radio and LED
work. It will also work when a user enable/disable wireless devices
on a user application which uses rfkill interface.
Come to think about it now, I may have to handle LED with WLAN and BT
but I will have to find a system with both devices later.
I am not too familiar with userspace applications v.s. LED. Is it
possible to do the same (i.e. without touching userspace)? I think
rfkill is good interface to handle whatever needs doing when changing
wireless states, such as LED controls. However, if other approach can
meet the need I am happy to investigate.
>
> --
> Pali Rohár
> [email protected]
--
Cheers,
Alex Hung
On Friday 26 June 2015 23:24:10 Alex Hung wrote:
> On Fri, Jun 26, 2015 at 10:56 PM, Pali Rohár <[email protected]> wrote:
> > Hi!
> >
> > On Wednesday 24 June 2015 10:57:51 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 <[email protected]>
> >
> > ...
> >
> >> +static int asus_radio_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_rbtn_device->handle, "HSWC",
> >> + &args, &output);
> >
> > What is this ACPI call doing? Just set LED control? Or something more?
> >
> >> + if (!ACPI_SUCCESS(status) || output == 0) {
> >> + pr_err("fail to change wireless LED.\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int asus_rfkill_set(void *data, bool blocked)
> >> +{
> >> + radio_led_state = blocked ? 0 : 1;
> >> +
> >> + return asus_radio_led_set(radio_led_state);
> >> +}
> >
> > In my opinion this is not good idea that "rfkill block" call from
> > userspace just change LED on/off state and nothing more...
> >
> > If above ACPI call just change LED, then should not be this in LED
> > subsystem instead rfkill one? Or why do you prefer to use rfkill
> > interface instead led?
>
> It indeed controls LED only at the moment. My intention was to have
> have everything work without the need to modify any userspace
> applications. Current it is 1) aus-rbtn issues KEY_RFKILL 2) an
> userspace application changes rfkill states, and 3) both radio and LED
> work. It will also work when a user enable/disable wireless devices
> on a user application which uses rfkill interface.
>
> Come to think about it now, I may have to handle LED with WLAN and BT
> but I will have to find a system with both devices later.
>
> I am not too familiar with userspace applications v.s. LED. Is it
> possible to do the same (i.e. without touching userspace)? I think
> rfkill is good interface to handle whatever needs doing when changing
> wireless states, such as LED controls. However, if other approach can
> meet the need I am happy to investigate.
>
There are triggers for led which automatically enable/disable led. I
think that configuring default wifi/bluetooth trigger for that new led
could work...
--
Pali Rohár
[email protected]
Pali,
Thanks for comments, but will you be able to provide more details so
it is more clear how this works?
On Mon, Jun 29, 2015 at 8:29 PM, Pali Rohár <[email protected]> wrote:
> On Friday 26 June 2015 23:24:10 Alex Hung wrote:
>> On Fri, Jun 26, 2015 at 10:56 PM, Pali Rohár <[email protected]> wrote:
>> > Hi!
>> >
>> > On Wednesday 24 June 2015 10:57:51 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 <[email protected]>
>> >
>> > ...
>> >
>> >> +static int asus_radio_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_rbtn_device->handle, "HSWC",
>> >> + &args, &output);
>> >
>> > What is this ACPI call doing? Just set LED control? Or something more?
>> >
>> >> + if (!ACPI_SUCCESS(status) || output == 0) {
>> >> + pr_err("fail to change wireless LED.\n");
>> >> + return -EINVAL;
>> >> + }
>> >> +
>> >> + return 0;
>> >> +}
>> >> +
>> >> +static int asus_rfkill_set(void *data, bool blocked)
>> >> +{
>> >> + radio_led_state = blocked ? 0 : 1;
>> >> +
>> >> + return asus_radio_led_set(radio_led_state);
>> >> +}
>> >
>> > In my opinion this is not good idea that "rfkill block" call from
>> > userspace just change LED on/off state and nothing more...
>> >
>> > If above ACPI call just change LED, then should not be this in LED
>> > subsystem instead rfkill one? Or why do you prefer to use rfkill
>> > interface instead led?
>>
>> It indeed controls LED only at the moment. My intention was to have
>> have everything work without the need to modify any userspace
>> applications. Current it is 1) aus-rbtn issues KEY_RFKILL 2) an
>> userspace application changes rfkill states, and 3) both radio and LED
>> work. It will also work when a user enable/disable wireless devices
>> on a user application which uses rfkill interface.
>>
>> Come to think about it now, I may have to handle LED with WLAN and BT
>> but I will have to find a system with both devices later.
>>
>> I am not too familiar with userspace applications v.s. LED. Is it
>> possible to do the same (i.e. without touching userspace)? I think
>> rfkill is good interface to handle whatever needs doing when changing
>> wireless states, such as LED controls. However, if other approach can
>> meet the need I am happy to investigate.
>>
>
> There are triggers for led which automatically enable/disable led. I
> think that configuring default wifi/bluetooth trigger for that new led
> could work...
>
> --
> Pali Rohár
> [email protected]
--
Cheers,
Alex Hung
Hi!
Ideally, try to touch led trigger configuration from userspace yourself,
so you will see how it works. Take some machine which has some
configurable led exported in /sys/class/leds/ and try to set some
trigger via "trigger" entry.
I think that default trigger for led device (from kernel) can be set via
"default_trigger" property in struct led_classdev. See file linux/leds.h
On Tuesday 30 June 2015 16:38:18 Alex Hung wrote:
> Pali,
>
> Thanks for comments, but will you be able to provide more details so
> it is more clear how this works?
>
> On Mon, Jun 29, 2015 at 8:29 PM, Pali Rohár <[email protected]> wrote:
> > On Friday 26 June 2015 23:24:10 Alex Hung wrote:
> >> On Fri, Jun 26, 2015 at 10:56 PM, Pali Rohár <[email protected]> wrote:
> >> > Hi!
> >> >
> >> > On Wednesday 24 June 2015 10:57:51 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 <[email protected]>
> >> >
> >> > ...
> >> >
> >> >> +static int asus_radio_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_rbtn_device->handle, "HSWC",
> >> >> + &args, &output);
> >> >
> >> > What is this ACPI call doing? Just set LED control? Or something more?
> >> >
> >> >> + if (!ACPI_SUCCESS(status) || output == 0) {
> >> >> + pr_err("fail to change wireless LED.\n");
> >> >> + return -EINVAL;
> >> >> + }
> >> >> +
> >> >> + return 0;
> >> >> +}
> >> >> +
> >> >> +static int asus_rfkill_set(void *data, bool blocked)
> >> >> +{
> >> >> + radio_led_state = blocked ? 0 : 1;
> >> >> +
> >> >> + return asus_radio_led_set(radio_led_state);
> >> >> +}
> >> >
> >> > In my opinion this is not good idea that "rfkill block" call from
> >> > userspace just change LED on/off state and nothing more...
> >> >
> >> > If above ACPI call just change LED, then should not be this in LED
> >> > subsystem instead rfkill one? Or why do you prefer to use rfkill
> >> > interface instead led?
> >>
> >> It indeed controls LED only at the moment. My intention was to have
> >> have everything work without the need to modify any userspace
> >> applications. Current it is 1) aus-rbtn issues KEY_RFKILL 2) an
> >> userspace application changes rfkill states, and 3) both radio and LED
> >> work. It will also work when a user enable/disable wireless devices
> >> on a user application which uses rfkill interface.
> >>
> >> Come to think about it now, I may have to handle LED with WLAN and BT
> >> but I will have to find a system with both devices later.
> >>
> >> I am not too familiar with userspace applications v.s. LED. Is it
> >> possible to do the same (i.e. without touching userspace)? I think
> >> rfkill is good interface to handle whatever needs doing when changing
> >> wireless states, such as LED controls. However, if other approach can
> >> meet the need I am happy to investigate.
> >>
> >
> > There are triggers for led which automatically enable/disable led. I
> > think that configuring default wifi/bluetooth trigger for that new led
> > could work...
> >
> > --
> > Pali Rohár
> > [email protected]
>
>
>
--
Pali Rohár
[email protected]
Thanks for the information, and I really appreciate it.
I took a quick look at my HP laptop and it has a led as below:
/sys/class/leds/hp::hddprotect$ cat trigger
[none] AC-online BAT0-charging-or-full BAT0-charging BAT0-full
BAT0-charging-blink-full-solid usb-gadget usb-host cpu0 cpu1 cpu2 cpu3
cpu4 cpu5 cpu6 cpu7 mmc0 rfkill1 rfkill2 rfkill8
and I learned that LED can be triggered by rfkill. I also checked
asus-wmi and its default_trigger is its rfkill name "asus-wlan".
ATK4001 is an independent ACPI device, and Method(HSWC) is its method
to control LED (actually it has other functions but only LED is needed
so far). asus-rbtn does not have anything to be triggered because it
only translate an ACPI event to KEY_RFKILL unless a rfkill is created,
but this wouldn't make sense that I use both rfkill and led when I can
only use one.
The other concern is that I'd like the LED to be ORed by both WLAN and
BT in long term. default_trigger seems to be linked to one trigger.
On Tue, Jun 30, 2015 at 4:58 PM, Pali Rohár <[email protected]> wrote:
> Hi!
>
> Ideally, try to touch led trigger configuration from userspace yourself,
> so you will see how it works. Take some machine which has some
> configurable led exported in /sys/class/leds/ and try to set some
> trigger via "trigger" entry.
>
> I think that default trigger for led device (from kernel) can be set via
> "default_trigger" property in struct led_classdev. See file linux/leds.h
>
> On Tuesday 30 June 2015 16:38:18 Alex Hung wrote:
>> Pali,
>>
>> Thanks for comments, but will you be able to provide more details so
>> it is more clear how this works?
>>
>> On Mon, Jun 29, 2015 at 8:29 PM, Pali Rohár <[email protected]> wrote:
>> > On Friday 26 June 2015 23:24:10 Alex Hung wrote:
>> >> On Fri, Jun 26, 2015 at 10:56 PM, Pali Rohár <[email protected]> wrote:
>> >> > Hi!
>> >> >
>> >> > On Wednesday 24 June 2015 10:57:51 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 <[email protected]>
>> >> >
>> >> > ...
>> >> >
>> >> >> +static int asus_radio_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_rbtn_device->handle, "HSWC",
>> >> >> + &args, &output);
>> >> >
>> >> > What is this ACPI call doing? Just set LED control? Or something more?
>> >> >
>> >> >> + if (!ACPI_SUCCESS(status) || output == 0) {
>> >> >> + pr_err("fail to change wireless LED.\n");
>> >> >> + return -EINVAL;
>> >> >> + }
>> >> >> +
>> >> >> + return 0;
>> >> >> +}
>> >> >> +
>> >> >> +static int asus_rfkill_set(void *data, bool blocked)
>> >> >> +{
>> >> >> + radio_led_state = blocked ? 0 : 1;
>> >> >> +
>> >> >> + return asus_radio_led_set(radio_led_state);
>> >> >> +}
>> >> >
>> >> > In my opinion this is not good idea that "rfkill block" call from
>> >> > userspace just change LED on/off state and nothing more...
>> >> >
>> >> > If above ACPI call just change LED, then should not be this in LED
>> >> > subsystem instead rfkill one? Or why do you prefer to use rfkill
>> >> > interface instead led?
>> >>
>> >> It indeed controls LED only at the moment. My intention was to have
>> >> have everything work without the need to modify any userspace
>> >> applications. Current it is 1) aus-rbtn issues KEY_RFKILL 2) an
>> >> userspace application changes rfkill states, and 3) both radio and LED
>> >> work. It will also work when a user enable/disable wireless devices
>> >> on a user application which uses rfkill interface.
>> >>
>> >> Come to think about it now, I may have to handle LED with WLAN and BT
>> >> but I will have to find a system with both devices later.
>> >>
>> >> I am not too familiar with userspace applications v.s. LED. Is it
>> >> possible to do the same (i.e. without touching userspace)? I think
>> >> rfkill is good interface to handle whatever needs doing when changing
>> >> wireless states, such as LED controls. However, if other approach can
>> >> meet the need I am happy to investigate.
>> >>
>> >
>> > There are triggers for led which automatically enable/disable led. I
>> > think that configuring default wifi/bluetooth trigger for that new led
>> > could work...
>> >
>> > --
>> > Pali Rohár
>> > [email protected]
>>
>>
>>
>
> --
> Pali Rohár
> [email protected]
--
Cheers,
Alex Hung
On Mon, Jun 29, 2015 at 02:29:53PM +0200, Pali Roh?r wrote:
> On Friday 26 June 2015 23:24:10 Alex Hung wrote:
> > On Fri, Jun 26, 2015 at 10:56 PM, Pali Roh?r <[email protected]> wrote:
> > > Hi!
> > >
> > > On Wednesday 24 June 2015 10:57:51 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 <[email protected]>
> > >
> > > ...
> > >
> > >> +static int asus_radio_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_rbtn_device->handle, "HSWC",
> > >> + &args, &output);
> > >
> > > What is this ACPI call doing? Just set LED control? Or something more?
> > >
> > >> + if (!ACPI_SUCCESS(status) || output == 0) {
> > >> + pr_err("fail to change wireless LED.\n");
> > >> + return -EINVAL;
> > >> + }
> > >> +
> > >> + return 0;
> > >> +}
> > >> +
> > >> +static int asus_rfkill_set(void *data, bool blocked)
> > >> +{
> > >> + radio_led_state = blocked ? 0 : 1;
> > >> +
> > >> + return asus_radio_led_set(radio_led_state);
> > >> +}
> > >
> > > In my opinion this is not good idea that "rfkill block" call from
> > > userspace just change LED on/off state and nothing more...
> > >
> > > If above ACPI call just change LED, then should not be this in LED
> > > subsystem instead rfkill one? Or why do you prefer to use rfkill
> > > interface instead led?
> >
> > It indeed controls LED only at the moment. My intention was to have
> > have everything work without the need to modify any userspace
> > applications. Current it is 1) aus-rbtn issues KEY_RFKILL 2) an
> > userspace application changes rfkill states, and 3) both radio and LED
> > work. It will also work when a user enable/disable wireless devices
> > on a user application which uses rfkill interface.
> >
> > Come to think about it now, I may have to handle LED with WLAN and BT
> > but I will have to find a system with both devices later.
> >
> > I am not too familiar with userspace applications v.s. LED. Is it
> > possible to do the same (i.e. without touching userspace)? I think
> > rfkill is good interface to handle whatever needs doing when changing
> > wireless states, such as LED controls. However, if other approach can
> > meet the need I am happy to investigate.
> >
>
> There are triggers for led which automatically enable/disable led. I
> think that configuring default wifi/bluetooth trigger for that new led
> could work...
>
I agree with Pali. If all we're doing is changing LED state, this sounds like a
job for a LEDs trigger.
--
Darren Hart
Intel Open Source Technology Center
Hello, you can create new trigger and use that...
On Tuesday 30 June 2015 18:09:41 Alex Hung wrote:
> Thanks for the information, and I really appreciate it.
>
> I took a quick look at my HP laptop and it has a led as below:
>
> /sys/class/leds/hp::hddprotect$ cat trigger
> [none] AC-online BAT0-charging-or-full BAT0-charging BAT0-full
> BAT0-charging-blink-full-solid usb-gadget usb-host cpu0 cpu1 cpu2
> cpu3 cpu4 cpu5 cpu6 cpu7 mmc0 rfkill1 rfkill2 rfkill8
>
> and I learned that LED can be triggered by rfkill. I also checked
> asus-wmi and its default_trigger is its rfkill name "asus-wlan".
>
> ATK4001 is an independent ACPI device, and Method(HSWC) is its method
> to control LED (actually it has other functions but only LED is
> needed so far). asus-rbtn does not have anything to be triggered
> because it only translate an ACPI event to KEY_RFKILL unless a
> rfkill is created, but this wouldn't make sense that I use both
> rfkill and led when I can only use one.
>
> The other concern is that I'd like the LED to be ORed by both WLAN
> and BT in long term. default_trigger seems to be linked to one
> trigger.
>
> On Tue, Jun 30, 2015 at 4:58 PM, Pali Rohár <[email protected]>
> wrote:
> > Hi!
> >
> > Ideally, try to touch led trigger configuration from userspace
> > yourself, so you will see how it works. Take some machine which
> > has some configurable led exported in /sys/class/leds/ and try to
> > set some trigger via "trigger" entry.
> >
> > I think that default trigger for led device (from kernel) can be
> > set via "default_trigger" property in struct led_classdev. See
> > file linux/leds.h
> >
> > On Tuesday 30 June 2015 16:38:18 Alex Hung wrote:
> >> Pali,
> >>
> >> Thanks for comments, but will you be able to provide more details
> >> so it is more clear how this works?
> >>
> >> On Mon, Jun 29, 2015 at 8:29 PM, Pali Rohár <[email protected]>
> >> wrote:
> >> > On Friday 26 June 2015 23:24:10 Alex Hung wrote:
> >> >> On Fri, Jun 26, 2015 at 10:56 PM, Pali Rohár
> >> >> <[email protected]> wrote:
> >> >> > Hi!
> >> >> >
> >> >> > On Wednesday 24 June 2015 10:57:51 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 <[email protected]>
> >> >> >
> >> >> > ...
> >> >> >
> >> >> >> +static int asus_radio_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_rbtn_device->handle, "HSWC", +
> >> >> >> &args, &output);
> >> >> >
> >> >> > What is this ACPI call doing? Just set LED control? Or
> >> >> > something more?
> >> >> >
> >> >> >> + if (!ACPI_SUCCESS(status) || output == 0) {
> >> >> >> + pr_err("fail to change wireless LED.\n");
> >> >> >> + return -EINVAL;
> >> >> >> + }
> >> >> >> +
> >> >> >> + return 0;
> >> >> >> +}
> >> >> >> +
> >> >> >> +static int asus_rfkill_set(void *data, bool blocked)
> >> >> >> +{
> >> >> >> + radio_led_state = blocked ? 0 : 1;
> >> >> >> +
> >> >> >> + return asus_radio_led_set(radio_led_state);
> >> >> >> +}
> >> >> >
> >> >> > In my opinion this is not good idea that "rfkill block" call
> >> >> > from userspace just change LED on/off state and nothing
> >> >> > more...
> >> >> >
> >> >> > If above ACPI call just change LED, then should not be this
> >> >> > in LED subsystem instead rfkill one? Or why do you prefer to
> >> >> > use rfkill interface instead led?
> >> >>
> >> >> It indeed controls LED only at the moment. My intention was to
> >> >> have have everything work without the need to modify any
> >> >> userspace applications. Current it is 1) aus-rbtn issues
> >> >> KEY_RFKILL 2) an userspace application changes rfkill states,
> >> >> and 3) both radio and LED work. It will also work when a user
> >> >> enable/disable wireless devices on a user application which
> >> >> uses rfkill interface.
> >> >>
> >> >> Come to think about it now, I may have to handle LED with WLAN
> >> >> and BT but I will have to find a system with both devices
> >> >> later.
> >> >>
> >> >> I am not too familiar with userspace applications v.s. LED. Is
> >> >> it possible to do the same (i.e. without touching userspace)?
> >> >> I think rfkill is good interface to handle whatever needs
> >> >> doing when changing wireless states, such as LED controls.
> >> >> However, if other approach can meet the need I am happy to
> >> >> investigate.
> >> >
> >> > There are triggers for led which automatically enable/disable
> >> > led. I think that configuring default wifi/bluetooth trigger
> >> > for that new led could work...
> >> >
> >> > --
> >> > Pali Rohár
> >> > [email protected]
> >
> > --
> > Pali Rohár
> > [email protected]
--
Pali Rohár
[email protected]
On Wednesday 01 July 2015 00:09:41 Alex Hung wrote:
> ATK4001 is an independent ACPI device, and Method(HSWC) is its method
> to control LED (actually it has other functions but only LED is needed
> so far).
If this driver is for supporting ACPI ATK4001 device (which has couple
of methods, not only LED) it is really good idea to name it asus-rbtn?
Darren, you as maintainer of platform drivers, what do you think? I
believe in future this driver will be extended to support more functions
and so name asus-rbtn will be confusing.
--
Pali Rohár
[email protected]
Thanks for the support. I will create v3 based with LED triggers.
Just for information. ASUS's wording is as below:
Fn+F2 can be used to turn on or off all radio capabilities in the
device (as known as airplane mode switch). I don't have any
preferences on the name. We may use the term airplane-mode do if
asus-rbtn is not preferred. Let me know if there are any suggestions.
On Wed, Jul 1, 2015 at 7:48 PM, Pali Rohár <[email protected]> wrote:
> On Wednesday 01 July 2015 00:09:41 Alex Hung wrote:
>> ATK4001 is an independent ACPI device, and Method(HSWC) is its method
>> to control LED (actually it has other functions but only LED is needed
>> so far).
>
> If this driver is for supporting ACPI ATK4001 device (which has couple
> of methods, not only LED) it is really good idea to name it asus-rbtn?
>
> Darren, you as maintainer of platform drivers, what do you think? I
> believe in future this driver will be extended to support more functions
> and so name asus-rbtn will be confusing.
>
> --
> Pali Rohár
> [email protected]
--
Cheers,
Alex Hung
On Thursday 02 July 2015 15:10:39 Alex Hung wrote:
> Thanks for the support. I will create v3 based with LED triggers.
>
> Just for information. ASUS's wording is as below:
>
> Fn+F2 can be used to turn on or off all radio capabilities in the
> device (as known as airplane mode switch). I don't have any
> preferences on the name. We may use the term airplane-mode do if
> asus-rbtn is not preferred. Let me know if there are any suggestions.
>
This is OK, but I though that ACPI ATK4001 device is some
multifunctional device, which is doing more then LED control... (At
least I understand it from your comments). So in this case I would not
call driver led/radio related. Right?
>
> On Wed, Jul 1, 2015 at 7:48 PM, Pali Rohár <[email protected]> wrote:
> > On Wednesday 01 July 2015 00:09:41 Alex Hung wrote:
> >> ATK4001 is an independent ACPI device, and Method(HSWC) is its method
> >> to control LED (actually it has other functions but only LED is needed
> >> so far).
> >
> > If this driver is for supporting ACPI ATK4001 device (which has couple
> > of methods, not only LED) it is really good idea to name it asus-rbtn?
> >
> > Darren, you as maintainer of platform drivers, what do you think? I
> > believe in future this driver will be extended to support more functions
> > and so name asus-rbtn will be confusing.
> >
> > --
> > Pali Rohár
> > [email protected]
>
--
Pali Rohár
[email protected]
ATK4001 is an ACPI device for wireless hotkey, similar to how Dell and
HP are doing it. It is just ASUS who decides LED should be controlled
by software unlike HP whose LED is driven by hardware pins on mini
card.
On Fri, Jul 3, 2015 at 3:25 PM, Pali Rohár <[email protected]> wrote:
> On Thursday 02 July 2015 15:10:39 Alex Hung wrote:
>> Thanks for the support. I will create v3 based with LED triggers.
>>
>> Just for information. ASUS's wording is as below:
>>
>> Fn+F2 can be used to turn on or off all radio capabilities in the
>> device (as known as airplane mode switch). I don't have any
>> preferences on the name. We may use the term airplane-mode do if
>> asus-rbtn is not preferred. Let me know if there are any suggestions.
>>
>
> This is OK, but I though that ACPI ATK4001 device is some
> multifunctional device, which is doing more then LED control... (At
> least I understand it from your comments). So in this case I would not
> call driver led/radio related. Right?
>
>>
>> On Wed, Jul 1, 2015 at 7:48 PM, Pali Rohár <[email protected]> wrote:
>> > On Wednesday 01 July 2015 00:09:41 Alex Hung wrote:
>> >> ATK4001 is an independent ACPI device, and Method(HSWC) is its method
>> >> to control LED (actually it has other functions but only LED is needed
>> >> so far).
>> >
>> > If this driver is for supporting ACPI ATK4001 device (which has couple
>> > of methods, not only LED) it is really good idea to name it asus-rbtn?
>> >
>> > Darren, you as maintainer of platform drivers, what do you think? I
>> > believe in future this driver will be extended to support more functions
>> > and so name asus-rbtn will be confusing.
>> >
>> > --
>> > Pali Rohár
>> > [email protected]
>>
>
> --
> Pali Rohár
> [email protected]
--
Cheers,
Alex Hung
On Mon, Jul 06, 2015 at 09:35:40AM +0800, Alex Hung wrote:
> ATK4001 is an ACPI device for wireless hotkey, similar to how Dell and
> HP are doing it. It is just ASUS who decides LED should be controlled
> by software unlike HP whose LED is driven by hardware pins on mini
> card.
Alex, please refrain from top posting on Linux kernel mailing lists, it breaks
the established practice the readers are setup for.
Regarding the ATK4001 device, it did sound like it did more than control the
radios and the associcated LED. If that is all it does, then asus-rbtn is fine.
If it does something beyond that, we need understand what that is, as a more
platform-centric name would be more appropriate.
Thanks,
>
> On Fri, Jul 3, 2015 at 3:25 PM, Pali Roh?r <[email protected]> wrote:
> > On Thursday 02 July 2015 15:10:39 Alex Hung wrote:
> >> Thanks for the support. I will create v3 based with LED triggers.
> >>
> >> Just for information. ASUS's wording is as below:
> >>
> >> Fn+F2 can be used to turn on or off all radio capabilities in the
> >> device (as known as airplane mode switch). I don't have any
> >> preferences on the name. We may use the term airplane-mode do if
> >> asus-rbtn is not preferred. Let me know if there are any suggestions.
> >>
> >
> > This is OK, but I though that ACPI ATK4001 device is some
> > multifunctional device, which is doing more then LED control... (At
> > least I understand it from your comments). So in this case I would not
> > call driver led/radio related. Right?
> >
> >>
> >> On Wed, Jul 1, 2015 at 7:48 PM, Pali Roh?r <[email protected]> wrote:
> >> > On Wednesday 01 July 2015 00:09:41 Alex Hung wrote:
> >> >> ATK4001 is an independent ACPI device, and Method(HSWC) is its method
> >> >> to control LED (actually it has other functions but only LED is needed
> >> >> so far).
> >> >
> >> > If this driver is for supporting ACPI ATK4001 device (which has couple
> >> > of methods, not only LED) it is really good idea to name it asus-rbtn?
> >> >
> >> > Darren, you as maintainer of platform drivers, what do you think? I
> >> > believe in future this driver will be extended to support more functions
> >> > and so name asus-rbtn will be confusing.
> >> >
> >> > --
> >> > Pali Roh?r
> >> > [email protected]
> >>
> >
> > --
> > Pali Roh?r
> > [email protected]
>
>
>
> --
> Cheers,
> Alex Hung
>
--
Darren Hart
Intel Open Source Technology Center
On Monday 06 July 2015 15:43:28 Darren Hart wrote:
> On Mon, Jul 06, 2015 at 09:35:40AM +0800, Alex Hung wrote:
> > ATK4001 is an ACPI device for wireless hotkey, similar to how Dell and
> > HP are doing it. It is just ASUS who decides LED should be controlled
> > by software unlike HP whose LED is driven by hardware pins on mini
> > card.
>
> Alex, please refrain from top posting on Linux kernel mailing lists, it breaks
> the established practice the readers are setup for.
>
> Regarding the ATK4001 device, it did sound like it did more than control the
> radios and the associcated LED. If that is all it does, then asus-rbtn is fine.
> If it does something beyond that, we need understand what that is, as a more
> platform-centric name would be more appropriate.
>
> Thanks,
Yes, I understand too that ATK4001 device has more functions as one for
LED control. And in this case it is not good to use name from one
specific functionality.
And about name:
I chose name rbtn for dell driver because ACPI device in DSDT table is
named RBTN and acpi id is DELRBTN.
I think that acpi-rbtn.c (radio button) is not ideal name for driver
which at SW level controls LED device associated with wireless devices.
I would rather follow acpi device name, but in this case ATK4001 is even
worse name which does not say anything...
Maybe better name could be asus-wireless? Still I do not have any good
name, so choose something...
--
Pali Rohár
[email protected]
On Tue, Jul 07, 2015 at 04:25:18PM +0200, Pali Roh?r wrote:
> On Monday 06 July 2015 15:43:28 Darren Hart wrote:
> > On Mon, Jul 06, 2015 at 09:35:40AM +0800, Alex Hung wrote:
> > > ATK4001 is an ACPI device for wireless hotkey, similar to how Dell and
> > > HP are doing it. It is just ASUS who decides LED should be controlled
> > > by software unlike HP whose LED is driven by hardware pins on mini
> > > card.
> >
> > Alex, please refrain from top posting on Linux kernel mailing lists, it breaks
> > the established practice the readers are setup for.
> >
> > Regarding the ATK4001 device, it did sound like it did more than control the
> > radios and the associcated LED. If that is all it does, then asus-rbtn is fine.
> > If it does something beyond that, we need understand what that is, as a more
> > platform-centric name would be more appropriate.
> >
> > Thanks,
>
> Yes, I understand too that ATK4001 device has more functions as one for
> LED control. And in this case it is not good to use name from one
> specific functionality.
>
> And about name:
>
> I chose name rbtn for dell driver because ACPI device in DSDT table is
> named RBTN and acpi id is DELRBTN.
>
> I think that acpi-rbtn.c (radio button) is not ideal name for driver
> which at SW level controls LED device associated with wireless devices.
>
> I would rather follow acpi device name, but in this case ATK4001 is even
> worse name which does not say anything...
>
> Maybe better name could be asus-wireless? Still I do not have any good
> name, so choose something...
I suggested rbtn to try and start showing some kind of consistency in the
directory - however, it's no better than many of the other existing options,
including -rfkill -wireless -laptop, etc.
Speaking of -laptop, the asus-laptop driver supports ATK0100 and ATK0101. This
ATK4001 device appears to perform the same type of features. The question is
whether or not it can be reasonably incorporated into the existing asus-laptop
driver or not.
(I intended to do that review myself, but I haven't found the time in 48 hours,
so I'm just going to send this out and ask that Alex have a look and provide his
thoughts on asus-laptop)
--
Darren Hart
Intel Open Source Technology Center
On Fri, Jul 10, 2015 at 4:52 AM, Darren Hart <[email protected]> wrote:
> On Tue, Jul 07, 2015 at 04:25:18PM +0200, Pali Rohár wrote:
>> On Monday 06 July 2015 15:43:28 Darren Hart wrote:
>> > On Mon, Jul 06, 2015 at 09:35:40AM +0800, Alex Hung wrote:
>> > > ATK4001 is an ACPI device for wireless hotkey, similar to how Dell and
>> > > HP are doing it. It is just ASUS who decides LED should be controlled
>> > > by software unlike HP whose LED is driven by hardware pins on mini
>> > > card.
>> >
>> > Alex, please refrain from top posting on Linux kernel mailing lists, it breaks
>> > the established practice the readers are setup for.
>> >
>> > Regarding the ATK4001 device, it did sound like it did more than control the
>> > radios and the associcated LED. If that is all it does, then asus-rbtn is fine.
>> > If it does something beyond that, we need understand what that is, as a more
>> > platform-centric name would be more appropriate.
>> >
>> > Thanks,
>>
>> Yes, I understand too that ATK4001 device has more functions as one for
>> LED control. And in this case it is not good to use name from one
>> specific functionality.
>>
>> And about name:
>>
>> I chose name rbtn for dell driver because ACPI device in DSDT table is
>> named RBTN and acpi id is DELRBTN.
>>
>> I think that acpi-rbtn.c (radio button) is not ideal name for driver
>> which at SW level controls LED device associated with wireless devices.
>>
>> I would rather follow acpi device name, but in this case ATK4001 is even
>> worse name which does not say anything...
>>
>> Maybe better name could be asus-wireless? Still I do not have any good
>> name, so choose something...
>
> I suggested rbtn to try and start showing some kind of consistency in the
> directory - however, it's no better than many of the other existing options,
> including -rfkill -wireless -laptop, etc.
>
> Speaking of -laptop, the asus-laptop driver supports ATK0100 and ATK0101. This
> ATK4001 device appears to perform the same type of features. The question is
> whether or not it can be reasonably incorporated into the existing asus-laptop
> driver or not.
>
> (I intended to do that review myself, but I haven't found the time in 48 hours,
> so I'm just going to send this out and ask that Alex have a look and provide his
> thoughts on asus-laptop)
>
> --
> Darren Hart
> Intel Open Source Technology Center
Certainly, I should have time to look into asus-laptop next week.
--
Cheers,
Alex Hung
On Fri, Jul 10, 2015 at 3:52 AM, Alex Hung <[email protected]> wrote:
> On Fri, Jul 10, 2015 at 4:52 AM, Darren Hart <[email protected]> wrote:
>> On Tue, Jul 07, 2015 at 04:25:18PM +0200, Pali Rohár wrote:
>>> On Monday 06 July 2015 15:43:28 Darren Hart wrote:
>>> > On Mon, Jul 06, 2015 at 09:35:40AM +0800, Alex Hung wrote:
>>> > > ATK4001 is an ACPI device for wireless hotkey, similar to how Dell and
>>> > > HP are doing it. It is just ASUS who decides LED should be controlled
>>> > > by software unlike HP whose LED is driven by hardware pins on mini
>>> > > card.
>>> >
>>> > Alex, please refrain from top posting on Linux kernel mailing lists, it breaks
>>> > the established practice the readers are setup for.
>>> >
>>> > Regarding the ATK4001 device, it did sound like it did more than control the
>>> > radios and the associcated LED. If that is all it does, then asus-rbtn is fine.
>>> > If it does something beyond that, we need understand what that is, as a more
>>> > platform-centric name would be more appropriate.
>>> >
>>> > Thanks,
>>>
>>> Yes, I understand too that ATK4001 device has more functions as one for
>>> LED control. And in this case it is not good to use name from one
>>> specific functionality.
>>>
>>> And about name:
>>>
>>> I chose name rbtn for dell driver because ACPI device in DSDT table is
>>> named RBTN and acpi id is DELRBTN.
>>>
>>> I think that acpi-rbtn.c (radio button) is not ideal name for driver
>>> which at SW level controls LED device associated with wireless devices.
>>>
>>> I would rather follow acpi device name, but in this case ATK4001 is even
>>> worse name which does not say anything...
>>>
>>> Maybe better name could be asus-wireless? Still I do not have any good
>>> name, so choose something...
>>
>> I suggested rbtn to try and start showing some kind of consistency in the
>> directory - however, it's no better than many of the other existing options,
>> including -rfkill -wireless -laptop, etc.
>>
>> Speaking of -laptop, the asus-laptop driver supports ATK0100 and ATK0101. This
>> ATK4001 device appears to perform the same type of features. The question is
>> whether or not it can be reasonably incorporated into the existing asus-laptop
>> driver or not.
>>
>> (I intended to do that review myself, but I haven't found the time in 48 hours,
>> so I'm just going to send this out and ask that Alex have a look and provide his
>> thoughts on asus-laptop)
>>
>> --
>> Darren Hart
>> Intel Open Source Technology Center
>
> Certainly, I should have time to look into asus-laptop next week.
>
>
> --
> Cheers,
> Alex Hung
asus-laptop has some assumption regarding the methosd provided by the
ACPI device, but you may be able to integrate with it.
Hard to know if it make sense though, really depend how similar these
interfaces are.
--
Corentin Chary
http://xf.iksaif.net