[RFCv2 0/4] Asus Wireless Radio Control driver

[RFCv2 0/4] Asus Wireless Radio Control driver

[João Paulo Rechi Vita]

This is the 2nd RFC for the "Asus Wireless Radio Control" device, addressing
the comments on the previous RFC. The differences from the previous RFC are:

 - Generate input events accessing the input layer directly instead of using
   sparse_keymap, as this was considered overkill for a device which has only
   one event type.

 - Set the input device vendor id.

 - Clean-up the driver to make a little shorter: now the only extra bits
   comparing to Mousou's proposal is the "struct asus_wireless_data", which is
   used to keep all the driver's data (input device pointer, acpi device
   pointer and LEDs data).

 - Compile the driver as a module by default.

 - Select the necessary config options to compile the led_class subsystem when
   this driver is selected.

 - Change the module name from asus-wrc to asus-wireless.

Since I am currently travelling I did not have a chance to test this changes,
but I'll do so as soon as I get back to my home office on Jan 1st. I decided to
send this updated version anyway to get feedback since this can't be merged
until we have the airplane mode RFKill LED trigger merged in the wireless tree.
I'll also send that patch to linux-wireless for feedback and integration later
today.

Regards,

Joao Paulo

João Paulo Rechi Vita (4):
  platform/x86: Add Asus Wireless Radio Control driver
  asus-wireless: Add ACPI HID ATK4001
  net/rfkill: Create "airplane mode" LED trigger
  asus-wireless: Toggle airplane mode LED

 MAINTAINERS                          |   6 ++
 drivers/platform/x86/Kconfig         |  17 ++++
 drivers/platform/x86/Makefile        |   1 +
 drivers/platform/x86/asus-wireless.c | 187 +++++++++++++++++++++++++++++++++++
 net/rfkill/core.c                    |  30 ++++++
 5 files changed, 241 insertions(+)
 create mode 100644 drivers/platform/x86/asus-wireless.c

-- 
2.5.0

[PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver

[João Paulo Rechi Vita]

Some Asus notebooks like the Asus E202SA and the Asus X555UB have a
separate ACPI device for notifications from the airplane mode hotkey.
This device is called "Wireless Radio Control" in Asus websites and ASHS
in the DSDT, and its ACPI _HID is ATK4002 in the two models mentioned
above.

For these models, when the airplane mode hotkey (Fn+F2) is pressed, a
query 0x0B is started in the Embedded Controller, and all this query does
is a notify ASHS with the value 0x88 (for acpi_osi >= "Windows 2012"):

	Scope (_SB.PCI0.SBRG.EC0)
	{
		(...)
		Method (_Q0B, 0, NotSerialized)  // _Qxx: EC Query
		{
			If ((MSOS () >= OSW8))
			{
				Notify (ASHS, 0x88) // Device-Specific
			}
			Else
			{
				(...)
			}
		}
	}

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 MAINTAINERS                          |   6 ++
 drivers/platform/x86/Kconfig         |  15 +++++
 drivers/platform/x86/Makefile        |   1 +
 drivers/platform/x86/asus-wireless.c | 105 +++++++++++++++++++++++++++++++++++
 4 files changed, 127 insertions(+)
 create mode 100644 drivers/platform/x86/asus-wireless.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c984136..e1860f2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1791,6 +1791,12 @@ S:	Maintained
 F:	drivers/platform/x86/asus*.c
 F:	drivers/platform/x86/eeepc*.c
 
+ASUS WIRELESS RADIO CONTROL DRIVER
+M:	João Paulo Rechi Vita <jprvita@gmail.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/x86/asus-wireless.c
+
 ASYNCHRONOUS TRANSFERS/TRANSFORMS (IOAT) API
 R:	Dan Williams <dan.j.williams@intel.com>
 W:	http://sourceforge.net/projects/xscaleiop
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index d0bfcf8..d3a088b 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -587,6 +587,21 @@ config EEEPC_WMI
 	  If you have an ACPI-WMI compatible Eee PC laptop (>= 1000), say Y or M
 	  here.
 
+config ASUS_WIRELESS
+	tristate "Asus Wireless Radio Control Driver"
+	depends on ACPI
+	depends on INPUT
+	default m
+	---help---
+	  The Asus Wireless Radio Control handles the airplane mode hotkey
+	  present on some Asus laptops.
+
+	  Say Y or M here if you have an ASUS notebook with an airplane mode
+	  hotkey.
+
+	  If you choose to compile this driver as a module the module will be
+	  called asus-wireless.
+
 config ACPI_WMI
 	tristate "WMI"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 4410e91..8b8df29 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_ASUS_LAPTOP)	+= asus-laptop.o
 obj-$(CONFIG_ASUS_WMI)		+= asus-wmi.o
 obj-$(CONFIG_ASUS_NB_WMI)	+= asus-nb-wmi.o
+obj-$(CONFIG_ASUS_WIRELESS)	+= asus-wireless.o
 obj-$(CONFIG_EEEPC_LAPTOP)	+= eeepc-laptop.o
 obj-$(CONFIG_EEEPC_WMI)		+= eeepc-wmi.o
 obj-$(CONFIG_MSI_LAPTOP)	+= msi-laptop.o
diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
new file mode 100644
index 0000000..ef7dba5
--- /dev/null
+++ b/drivers/platform/x86/asus-wireless.c
@@ -0,0 +1,105 @@
+/*
+ * Asus Wireless Radio Control Driver
+ *
+ * Copyright (C) 2015 Endless Mobile, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/acpi.h>
+#include <linux/input.h>
+#include <linux/pci_ids.h>
+
+#define ASUS_WIRELESS_MODULE_NAME "Asus Wireless Radio Control Driver"
+
+struct asus_wireless_data {
+	struct input_dev *inputdev;
+};
+
+static void asus_wireless_notify(struct acpi_device *device, u32 event)
+{
+	struct asus_wireless_data *data = acpi_driver_data(device);
+
+	pr_debug("event=0x%X\n", event);
+	if (event != 0x88) {
+		pr_info("Unknown ASHS event: 0x%X\n", event);
+		return;
+	}
+	input_report_key(data->inputdev, KEY_RFKILL, 1);
+	input_report_key(data->inputdev, KEY_RFKILL, 0);
+	input_sync(data->inputdev);
+}
+
+static int asus_wireless_add(struct acpi_device *device)
+{
+	struct asus_wireless_data *data;
+	int err = -ENOMEM;
+
+	pr_info(ASUS_WIRELESS_MODULE_NAME"\n");
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+	device->driver_data = data;
+
+	data->inputdev = input_allocate_device();
+	if (!data->inputdev)
+		goto fail;
+
+	data->inputdev->name = "Asus Wireless Radio Control";
+	data->inputdev->phys = "asus-wireless/input0";
+	data->inputdev->id.bustype = BUS_HOST;
+	data->inputdev->id.vendor = PCI_VENDOR_ID_ASUSTEK;
+	data->inputdev->dev.parent = &device->dev;
+	set_bit(EV_REP, data->inputdev->evbit);
+	set_bit(KEY_RFKILL, data->inputdev->keybit);
+
+	err = input_register_device(data->inputdev);
+	if (err)
+		goto fail;
+	return 0;
+
+fail:
+	device->driver->ops.remove(device);
+	return err;
+}
+
+static int asus_wireless_remove(struct acpi_device *device)
+{
+	struct asus_wireless_data *data = acpi_driver_data(device);
+
+	pr_info("Removing "ASUS_WIRELESS_MODULE_NAME"\n");
+	if (data->inputdev)
+		input_unregister_device(data->inputdev);
+	kfree(data);
+	return 0;
+}
+
+static const struct acpi_device_id device_ids[] = {
+	{"ATK4002", 0},
+	{"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, device_ids);
+
+static struct acpi_driver asus_wireless_driver = {
+	.name = ASUS_WIRELESS_MODULE_NAME,
+	.class = "hotkey",
+	.ids = device_ids,
+	.ops = {
+		.add = asus_wireless_add,
+		.remove = asus_wireless_remove,
+		.notify = asus_wireless_notify,
+	},
+};
+module_acpi_driver(asus_wireless_driver);
+
+MODULE_DESCRIPTION(ASUS_WIRELESS_MODULE_NAME);
+MODULE_AUTHOR("João Paulo Rechi Vita <jprvita@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
2.5.0

[PATCH 2/4] asus-wireless: Add ACPI HID ATK4001

[João Paulo Rechi Vita]

As reported in https://bugzilla.kernel.org/show_bug.cgi?id=98931#c22 in
the Asus UX31A the Asus Wireless Radio Control device (ASHS) uses the
HID "ATK4001".

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
Reported-by: Tasev Nikola <tasev.stefanoska@skynet.be>
---
 drivers/platform/x86/asus-wireless.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
index ef7dba5..7928efd 100644
--- a/drivers/platform/x86/asus-wireless.c
+++ b/drivers/platform/x86/asus-wireless.c
@@ -83,6 +83,7 @@ static int asus_wireless_remove(struct acpi_device *device)
 }
 
 static const struct acpi_device_id device_ids[] = {
+	{"ATK4001", 0},
 	{"ATK4002", 0},
 	{"", 0},
 };
-- 
2.5.0

[PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger

[João Paulo Rechi Vita]

For platform drivers to be able to correctly drive the "Airplane Mode"
indicative LED there needs to be a RFKill LED trigger tied to the global
state of RFKILL_TYPE_ALL (instead of to a specific RFKill) and that
works in an inverted manner of regular RFKill LED triggers, that is, the
LED is ON when the state is blocked, and OFF otherwise.

This commit implements such a trigger, which will be used by the
asus-wrc x86 platform driver.

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 net/rfkill/core.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index b41e9ea..3effc29 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -124,6 +124,26 @@ static bool rfkill_epo_lock_active;
 
 
 #ifdef CONFIG_RFKILL_LEDS
+static void airplane_mode_led_trigger_activate(struct led_classdev *led);
+
+static struct led_trigger airplane_mode_led_trigger = {
+	.name     = "rfkill-airplane-mode",
+	.activate = airplane_mode_led_trigger_activate,
+};
+
+static void airplane_mode_led_trigger_event(void)
+{
+	if (rfkill_global_states[RFKILL_TYPE_ALL].cur & RFKILL_BLOCK_ANY)
+		led_trigger_event(&airplane_mode_led_trigger, LED_FULL);
+	else
+		led_trigger_event(&airplane_mode_led_trigger, LED_OFF);
+}
+
+static void airplane_mode_led_trigger_activate(struct led_classdev *led)
+{
+	airplane_mode_led_trigger_event();
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
 	struct led_trigger *trigger;
@@ -175,6 +195,10 @@ static void rfkill_led_trigger_unregister(struct rfkill *rfkill)
 	led_trigger_unregister(&rfkill->led_trigger);
 }
 #else
+static void airplane_mode_led_trigger_event(void)
+{
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
 }
@@ -346,6 +370,7 @@ static void __rfkill_switch_all(const enum rfkill_type type, bool blocked)
 
 		for (i = 0; i < NUM_RFKILL_TYPES; i++)
 			rfkill_global_states[i].cur = blocked;
+		airplane_mode_led_trigger_event();
 	} else {
 		rfkill_global_states[type].cur = blocked;
 	}
@@ -1177,6 +1202,7 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 			enum rfkill_type i;
 			for (i = 0; i < NUM_RFKILL_TYPES; i++)
 				rfkill_global_states[i].cur = ev.soft;
+			airplane_mode_led_trigger_event();
 		} else {
 			rfkill_global_states[ev.type].cur = ev.soft;
 		}
@@ -1293,6 +1319,10 @@ static int __init rfkill_init(void)
 	}
 #endif
 
+#ifdef CONFIG_RFKILL_LEDS
+	led_trigger_register(&airplane_mode_led_trigger);
+#endif
+
  out:
 	return error;
 }
-- 
2.5.0

[PATCH 4/4] asus-wireless: Toggle airplane mode LED

[João Paulo Rechi Vita]

In the ASHS device we have the HSWC method, which basically calls either
OWGD or OWGS, depending on its parameter:

	Device (ASHS)
	{
		Name (_HID, "ATK4002")  // _HID: Hardware ID
		Method (HSWC, 1, Serialized)
		{
			If ((Arg0 < 0x02))
			{
				OWGD (Arg0)
				Return (One)
			}
			If ((Arg0 == 0x02))
			{
				Local0 = OWGS ()
				If (Local0)
				{
					Return (0x05)
				}
				Else
				{
					Return (0x04)
				}
			}
			If ((Arg0 == 0x03))
			{
				Return (0xFF)
			}
			If ((Arg0 == 0x04))
			{
				OWGD (Zero)
				Return (One)
			}
			If ((Arg0 == 0x05))
			{
				OWGD (One)
				Return (One)
			}
			If ((Arg0 == 0x80))
			{
				Return (One)
			}
		}
		Method (_STA, 0, NotSerialized)  // _STA: Status
		{
			If ((MSOS () >= OSW8))
			{
				Return (0x0F)
			}
			Else
			{
				Return (Zero)
			}
		}
	}

On the Asus E202SA laptop, which does not have an airplane mode LED,
OWGD has an empty implementation and OWGS simply returns 0. On the Asus
X555UB these methods have the following implementation:

	Method (OWGD, 1, Serialized)
	{
		SGPL (0x0203000F, Arg0)
		SGPL (0x0203000F, Arg0)
	}

	Method (OWGS, 0, Serialized)
	{
		Store (RGPL (0x0203000F), Local0)
		Return (Local0)
	}

Where OWGD(1) sets the airplane mode LED ON, OWGD(0) set it off, and
OWGS() returns its state.

This commit makes use of a newly implemented RFKill LED trigger to
trigger the LED when the system enters or exits "Airplane Mode", there
is, when all radios are blocked.

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 drivers/platform/x86/Kconfig         |  2 +
 drivers/platform/x86/asus-wireless.c | 81 ++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index d3a088b..3d8dc0b 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -592,6 +592,8 @@ config ASUS_WIRELESS
 	depends on ACPI
 	depends on INPUT
 	default m
+	select NEW_LEDS
+	select LEDS_CLASS
 	---help---
 	  The Asus Wireless Radio Control handles the airplane mode hotkey
 	  present on some Asus laptops.
diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
index 7928efd..489ef83 100644
--- a/drivers/platform/x86/asus-wireless.c
+++ b/drivers/platform/x86/asus-wireless.c
@@ -17,13 +17,76 @@
 #include <linux/acpi.h>
 #include <linux/input.h>
 #include <linux/pci_ids.h>
+#include <linux/leds.h>
 
 #define ASUS_WIRELESS_MODULE_NAME "Asus Wireless Radio Control Driver"
+#define ASUS_WIRELESS_LED_STATUS 0x2
+#define ASUS_WIRELESS_LED_OFF 0x4
+#define ASUS_WIRELESS_LED_ON 0x5
 
 struct asus_wireless_data {
 	struct input_dev *inputdev;
+	struct acpi_device *acpidev;
+	struct workqueue_struct *wq;
+	struct work_struct led_work;
+	struct led_classdev led;
+	int led_state;
 };
 
+static u64 asus_wireless_method(acpi_handle handle, const char *method,
+				int param)
+{
+	union acpi_object obj;
+	struct acpi_object_list p;
+	acpi_status s;
+	u64 ret;
+
+	pr_debug("Evaluating method %s, parameter 0x%X\n", method, param);
+	obj.type = ACPI_TYPE_INTEGER;
+	obj.integer.value = param;
+	p.count = 1;
+	p.pointer = &obj;
+
+	s = acpi_evaluate_integer(handle, (acpi_string) method, &p, &ret);
+	if (!ACPI_SUCCESS(s))
+		pr_err("Failed to evaluate method %s, parameter 0x%X (%d)\n",
+		       method, param, s);
+	pr_debug("%s returned 0x%X\n", method, (uint) ret);
+	return ret;
+}
+
+static enum led_brightness asus_wireless_led_get(struct led_classdev *led)
+{
+	struct asus_wireless_data *data;
+	int s;
+
+	data = container_of(led, struct asus_wireless_data, led);
+	s = asus_wireless_method(data->acpidev->handle, "HSWC",
+				 ASUS_WIRELESS_LED_STATUS);
+	if (s == ASUS_WIRELESS_LED_ON)
+		return LED_FULL;
+	return LED_OFF;
+}
+
+static void asus_wireless_led_update(struct work_struct *work)
+{
+	struct asus_wireless_data *data;
+
+	data = container_of(work, struct asus_wireless_data, led_work);
+	asus_wireless_method(data->acpidev->handle, "HSWC", data->led_state);
+}
+
+static void asus_wireless_led_set(struct led_classdev *led,
+				  enum led_brightness value)
+{
+	struct asus_wireless_data *data;
+
+	data = container_of(led, struct asus_wireless_data, led);
+	data->led_state = value == LED_OFF ? ASUS_WIRELESS_LED_OFF :
+					     ASUS_WIRELESS_LED_ON;
+	queue_work(data->wq, &data->led_work);
+}
+
 static void asus_wireless_notify(struct acpi_device *device, u32 event)
 {
 	struct asus_wireless_data *data = acpi_driver_data(device);
@@ -49,6 +112,7 @@ static int asus_wireless_add(struct acpi_device *device)
 		return -ENOMEM;
 	device->driver_data = data;
 
+	data->acpidev = device;
 	data->inputdev = input_allocate_device();
 	if (!data->inputdev)
 		goto fail;
@@ -64,6 +128,21 @@ static int asus_wireless_add(struct acpi_device *device)
 	err = input_register_device(data->inputdev);
 	if (err)
 		goto fail;
+
+	data->wq = create_singlethread_workqueue("asus_wireless_workqueue");
+	if (!data->wq)
+		goto fail;
+
+	INIT_WORK(&data->led_work, asus_wireless_led_update);
+	data->led.name = "asus-wireless::airplane_mode";
+	data->led.brightness_set = asus_wireless_led_set;
+	data->led.brightness_get = asus_wireless_led_get;
+	data->led.flags = LED_CORE_SUSPENDRESUME;
+	data->led.max_brightness = 1;
+	data->led.default_trigger = "rfkill-airplane-mode";
+	err = led_classdev_register(&device->dev, &data->led);
+	if (err)
+		goto fail;
 	return 0;
 
 fail:
@@ -78,6 +157,8 @@ static int asus_wireless_remove(struct acpi_device *device)
 	pr_info("Removing "ASUS_WIRELESS_MODULE_NAME"\n");
 	if (data->inputdev)
 		input_unregister_device(data->inputdev);
+	if (data->wq)
+		destroy_workqueue(data->wq);
 	kfree(data);
 	return 0;
 }
-- 
2.5.0

Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver

[Andy Shevchenko]

On Sat, Dec 26, 2015 at 4:56 PM, João Paulo Rechi Vita
<jprvita@gmail.com> wrote:
> Some Asus notebooks like the Asus E202SA and the Asus X555UB have a
> separate ACPI device for notifications from the airplane mode hotkey.
> This device is called "Wireless Radio Control" in Asus websites and ASHS
> in the DSDT, and its ACPI _HID is ATK4002 in the two models mentioned
> above.
>
> For these models, when the airplane mode hotkey (Fn+F2) is pressed, a
> query 0x0B is started in the Embedded Controller, and all this query does
> is a notify ASHS with the value 0x88 (for acpi_osi >= "Windows 2012"):
>
>         Scope (_SB.PCI0.SBRG.EC0)
>         {
>                 (...)
>                 Method (_Q0B, 0, NotSerialized)  // _Qxx: EC Query
>                 {
>                         If ((MSOS () >= OSW8))
>                         {
>                                 Notify (ASHS, 0x88) // Device-Specific
>                         }
>                         Else
>                         {
>                                 (...)
>                         }
>                 }
>         }
>
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
> ---
>  MAINTAINERS                          |   6 ++
>  drivers/platform/x86/Kconfig         |  15 +++++
>  drivers/platform/x86/Makefile        |   1 +
>  drivers/platform/x86/asus-wireless.c | 105 +++++++++++++++++++++++++++++++++++
>  4 files changed, 127 insertions(+)
>  create mode 100644 drivers/platform/x86/asus-wireless.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c984136..e1860f2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1791,6 +1791,12 @@ S:       Maintained
>  F:     drivers/platform/x86/asus*.c
>  F:     drivers/platform/x86/eeepc*.c
>
> +ASUS WIRELESS RADIO CONTROL DRIVER
> +M:     João Paulo Rechi Vita <jprvita@gmail.com>
> +L:     platform-driver-x86@vger.kernel.org
> +S:     Maintained
> +F:     drivers/platform/x86/asus-wireless.c
> +
>  ASYNCHRONOUS TRANSFERS/TRANSFORMS (IOAT) API
>  R:     Dan Williams <dan.j.williams@intel.com>
>  W:     http://sourceforge.net/projects/xscaleiop
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index d0bfcf8..d3a088b 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -587,6 +587,21 @@ config EEEPC_WMI
>           If you have an ACPI-WMI compatible Eee PC laptop (>= 1000), say Y or M
>           here.
>
> +config ASUS_WIRELESS
> +       tristate "Asus Wireless Radio Control Driver"
> +       depends on ACPI
> +       depends on INPUT
> +       default m
> +       ---help---
> +         The Asus Wireless Radio Control handles the airplane mode hotkey
> +         present on some Asus laptops.
> +
> +         Say Y or M here if you have an ASUS notebook with an airplane mode
> +         hotkey.
> +
> +         If you choose to compile this driver as a module the module will be
> +         called asus-wireless.
> +
>  config ACPI_WMI
>         tristate "WMI"
>         depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 4410e91..8b8df29 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -5,6 +5,7 @@
>  obj-$(CONFIG_ASUS_LAPTOP)      += asus-laptop.o
>  obj-$(CONFIG_ASUS_WMI)         += asus-wmi.o
>  obj-$(CONFIG_ASUS_NB_WMI)      += asus-nb-wmi.o
> +obj-$(CONFIG_ASUS_WIRELESS)    += asus-wireless.o
>  obj-$(CONFIG_EEEPC_LAPTOP)     += eeepc-laptop.o
>  obj-$(CONFIG_EEEPC_WMI)                += eeepc-wmi.o
>  obj-$(CONFIG_MSI_LAPTOP)       += msi-laptop.o
> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
> new file mode 100644
> index 0000000..ef7dba5
> --- /dev/null
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -0,0 +1,105 @@
> +/*
> + * Asus Wireless Radio Control Driver
> + *
> + * Copyright (C) 2015 Endless Mobile, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/acpi.h>
> +#include <linux/input.h>

> +#include <linux/pci_ids.h>

Didn't notice exatcly how this one is used.

> +
> +#define ASUS_WIRELESS_MODULE_NAME "Asus Wireless Radio Control Driver"
> +
> +struct asus_wireless_data {
> +       struct input_dev *inputdev;
> +};
> +
> +static void asus_wireless_notify(struct acpi_device *device, u32 event)
> +{
> +       struct asus_wireless_data *data = acpi_driver_data(device);
> +
> +       pr_debug("event=0x%X\n", event);

dev_, please.

> +       if (event != 0x88) {
> +               pr_info("Unknown ASHS event: 0x%X\n", event);

Ditto. Moreover, this is apparently not an info, I can put it either
to debug or to warn depending how much those I produced in erroneous
state.

> +               return;
> +       }
> +       input_report_key(data->inputdev, KEY_RFKILL, 1);
> +       input_report_key(data->inputdev, KEY_RFKILL, 0);
> +       input_sync(data->inputdev);
> +}
> +
> +static int asus_wireless_add(struct acpi_device *device)
> +{
> +       struct asus_wireless_data *data;
> +       int err = -ENOMEM;
> +

> +       pr_info(ASUS_WIRELESS_MODULE_NAME"\n");

Kinda useless,

> +       data = kzalloc(sizeof(*data), GFP_KERNEL);

devm_kzalloc();

> +       if (!data)
> +               return -ENOMEM;
> +       device->driver_data = data;
> +
> +       data->inputdev = input_allocate_device();

Use devm_*

> +       if (!data->inputdev)
> +               goto fail;
> +
> +       data->inputdev->name = "Asus Wireless Radio Control";
> +       data->inputdev->phys = "asus-wireless/input0";
> +       data->inputdev->id.bustype = BUS_HOST;
> +       data->inputdev->id.vendor = PCI_VENDOR_ID_ASUSTEK;
> +       data->inputdev->dev.parent = &device->dev;
> +       set_bit(EV_REP, data->inputdev->evbit);
> +       set_bit(KEY_RFKILL, data->inputdev->keybit);
> +
> +       err = input_register_device(data->inputdev);
> +       if (err)
> +               goto fail;
> +       return 0;
> +
> +fail:
> +       device->driver->ops.remove(device);

Better to show explicitly what you do here. Currently I might say it's
a strange way I've ever seen to express the error path.

> +       return err;
> +}
> +
> +static int asus_wireless_remove(struct acpi_device *device)
> +{
> +       struct asus_wireless_data *data = acpi_driver_data(device);
> +

> +       pr_info("Removing "ASUS_WIRELESS_MODULE_NAME"\n");

Kinda useless.

> +       if (data->inputdev)

How is it possible?

> +               input_unregister_device(data->inputdev);

Redundant after switching to devm_

> +       kfree(data);

Ditto.

> +       return 0;
> +}
> +
> +static const struct acpi_device_id device_ids[] = {
> +       {"ATK4002", 0},
> +       {"", 0},
> +};
> +MODULE_DEVICE_TABLE(acpi, device_ids);
> +
> +static struct acpi_driver asus_wireless_driver = {
> +       .name = ASUS_WIRELESS_MODULE_NAME,
> +       .class = "hotkey",
> +       .ids = device_ids,
> +       .ops = {
> +               .add = asus_wireless_add,
> +               .remove = asus_wireless_remove,
> +               .notify = asus_wireless_notify,
> +       },
> +};
> +module_acpi_driver(asus_wireless_driver);
> +
> +MODULE_DESCRIPTION(ASUS_WIRELESS_MODULE_NAME);
> +MODULE_AUTHOR("João Paulo Rechi Vita <jprvita@gmail.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] asus-wireless: Toggle airplane mode LED

[Andy Shevchenko]

On Sat, Dec 26, 2015 at 4:56 PM, João Paulo Rechi Vita
<jprvita@gmail.com> wrote:
> In the ASHS device we have the HSWC method, which basically calls either
> OWGD or OWGS, depending on its parameter:
>
>         Device (ASHS)
>         {
>                 Name (_HID, "ATK4002")  // _HID: Hardware ID
>                 Method (HSWC, 1, Serialized)
>                 {
>                         If ((Arg0 < 0x02))
>                         {
>                                 OWGD (Arg0)
>                                 Return (One)
>                         }
>                         If ((Arg0 == 0x02))
>                         {
>                                 Local0 = OWGS ()
>                                 If (Local0)
>                                 {
>                                         Return (0x05)
>                                 }
>                                 Else
>                                 {
>                                         Return (0x04)
>                                 }
>                         }
>                         If ((Arg0 == 0x03))
>                         {
>                                 Return (0xFF)
>                         }
>                         If ((Arg0 == 0x04))
>                         {
>                                 OWGD (Zero)
>                                 Return (One)
>                         }
>                         If ((Arg0 == 0x05))
>                         {
>                                 OWGD (One)
>                                 Return (One)
>                         }
>                         If ((Arg0 == 0x80))
>                         {
>                                 Return (One)
>                         }
>                 }
>                 Method (_STA, 0, NotSerialized)  // _STA: Status
>                 {
>                         If ((MSOS () >= OSW8))
>                         {
>                                 Return (0x0F)
>                         }
>                         Else
>                         {
>                                 Return (Zero)
>                         }
>                 }
>         }
>
> On the Asus E202SA laptop, which does not have an airplane mode LED,
> OWGD has an empty implementation and OWGS simply returns 0. On the Asus
> X555UB these methods have the following implementation:
>
>         Method (OWGD, 1, Serialized)
>         {
>                 SGPL (0x0203000F, Arg0)
>                 SGPL (0x0203000F, Arg0)
>         }
>
>         Method (OWGS, 0, Serialized)
>         {
>                 Store (RGPL (0x0203000F), Local0)
>                 Return (Local0)
>         }
>
> Where OWGD(1) sets the airplane mode LED ON, OWGD(0) set it off, and
> OWGS() returns its state.
>
> This commit makes use of a newly implemented RFKill LED trigger to
> trigger the LED when the system enters or exits "Airplane Mode", there
> is, when all radios are blocked.
>
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
> ---
>  drivers/platform/x86/Kconfig         |  2 +
>  drivers/platform/x86/asus-wireless.c | 81 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 83 insertions(+)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index d3a088b..3d8dc0b 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -592,6 +592,8 @@ config ASUS_WIRELESS
>         depends on ACPI
>         depends on INPUT
>         default m
> +       select NEW_LEDS
> +       select LEDS_CLASS
>         ---help---
>           The Asus Wireless Radio Control handles the airplane mode hotkey
>           present on some Asus laptops.
> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
> index 7928efd..489ef83 100644
> --- a/drivers/platform/x86/asus-wireless.c
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -17,13 +17,76 @@
>  #include <linux/acpi.h>
>  #include <linux/input.h>
>  #include <linux/pci_ids.h>
> +#include <linux/leds.h>
>
>  #define ASUS_WIRELESS_MODULE_NAME "Asus Wireless Radio Control Driver"
> +#define ASUS_WIRELESS_LED_STATUS 0x2
> +#define ASUS_WIRELESS_LED_OFF 0x4
> +#define ASUS_WIRELESS_LED_ON 0x5
>
>  struct asus_wireless_data {
>         struct input_dev *inputdev;
> +       struct acpi_device *acpidev;

You can get this easily from struct device.

> +       struct workqueue_struct *wq;
> +       struct work_struct led_work;
> +       struct led_classdev led;
> +       int led_state;
>  };
>
> +static u64 asus_wireless_method(acpi_handle handle, const char *method,
> +                               int param)
> +{
> +       union acpi_object obj;
> +       struct acpi_object_list p;
> +       acpi_status s;
> +       u64 ret;
> +
> +       pr_debug("Evaluating method %s, parameter 0x%X\n", method, param);

acpi_handle_* in such cases.

> +       obj.type = ACPI_TYPE_INTEGER;
> +       obj.integer.value = param;
> +       p.count = 1;
> +       p.pointer = &obj;
> +
> +       s = acpi_evaluate_integer(handle, (acpi_string) method, &p, &ret);
> +       if (!ACPI_SUCCESS(s))

ACPI_FAILURE()

> +               pr_err("Failed to evaluate method %s, parameter 0x%X (%d)\n",
> +                      method, param, s);
> +       pr_debug("%s returned 0x%X\n", method, (uint) ret);
> +       return ret;
> +}
> +
> +static enum led_brightness asus_wireless_led_get(struct led_classdev *led)
> +{
> +       struct asus_wireless_data *data;
> +       int s;
> +
> +       data = container_of(led, struct asus_wireless_data, led);
> +       s = asus_wireless_method(data->acpidev->handle, "HSWC",

Usually we get a handle through specific macro ACPI_HANDLE from a
struct device (see above).

> +                                ASUS_WIRELESS_LED_STATUS);
> +       if (s == ASUS_WIRELESS_LED_ON)
> +               return LED_FULL;
> +       return LED_OFF;
> +}
> +
> +static void asus_wireless_led_update(struct work_struct *work)
> +{
> +       struct asus_wireless_data *data;
> +
> +       data = container_of(work, struct asus_wireless_data, led_work);
> +       asus_wireless_method(data->acpidev->handle, "HSWC", data->led_state);

Ditto.

> +}
> +
> +static void asus_wireless_led_set(struct led_classdev *led,
> +                                 enum led_brightness value)
> +{
> +       struct asus_wireless_data *data;
> +
> +       data = container_of(led, struct asus_wireless_data, led);
> +       data->led_state = value == LED_OFF ? ASUS_WIRELESS_LED_OFF :
> +                                            ASUS_WIRELESS_LED_ON;
> +       queue_work(data->wq, &data->led_work);
> +}
> +
>  static void asus_wireless_notify(struct acpi_device *device, u32 event)
>  {
>         struct asus_wireless_data *data = acpi_driver_data(device);
> @@ -49,6 +112,7 @@ static int asus_wireless_add(struct acpi_device *device)
>                 return -ENOMEM;
>         device->driver_data = data;
>
> +       data->acpidev = device;
>         data->inputdev = input_allocate_device();
>         if (!data->inputdev)
>                 goto fail;
> @@ -64,6 +128,21 @@ static int asus_wireless_add(struct acpi_device *device)
>         err = input_register_device(data->inputdev);
>         if (err)
>                 goto fail;
> +
> +       data->wq = create_singlethread_workqueue("asus_wireless_workqueue");
> +       if (!data->wq)
> +               goto fail;
> +
> +       INIT_WORK(&data->led_work, asus_wireless_led_update);
> +       data->led.name = "asus-wireless::airplane_mode";
> +       data->led.brightness_set = asus_wireless_led_set;
> +       data->led.brightness_get = asus_wireless_led_get;
> +       data->led.flags = LED_CORE_SUSPENDRESUME;
> +       data->led.max_brightness = 1;
> +       data->led.default_trigger = "rfkill-airplane-mode";
> +       err = led_classdev_register(&device->dev, &data->led);
> +       if (err)
> +               goto fail;
>         return 0;
>
>  fail:
> @@ -78,6 +157,8 @@ static int asus_wireless_remove(struct acpi_device *device)
>         pr_info("Removing "ASUS_WIRELESS_MODULE_NAME"\n");
>         if (data->inputdev)
>                 input_unregister_device(data->inputdev);
> +       if (data->wq)
> +               destroy_workqueue(data->wq);
>         kfree(data);
>         return 0;
>  }
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver

[João Paulo Rechi Vita]

On 27 December 2015 at 08:21, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Sat, Dec 26, 2015 at 4:56 PM, João Paulo Rechi Vita
> <jprvita@gmail.com> wrote:
>> Some Asus notebooks like the Asus E202SA and the Asus X555UB have a
>> separate ACPI device for notifications from the airplane mode hotkey.
>> This device is called "Wireless Radio Control" in Asus websites and ASHS
>> in the DSDT, and its ACPI _HID is ATK4002 in the two models mentioned
>> above.
>>
>> For these models, when the airplane mode hotkey (Fn+F2) is pressed, a
>> query 0x0B is started in the Embedded Controller, and all this query does
>> is a notify ASHS with the value 0x88 (for acpi_osi >= "Windows 2012"):
>>
>>         Scope (_SB.PCI0.SBRG.EC0)
>>         {
>>                 (...)
>>                 Method (_Q0B, 0, NotSerialized)  // _Qxx: EC Query
>>                 {
>>                         If ((MSOS () >= OSW8))
>>                         {
>>                                 Notify (ASHS, 0x88) // Device-Specific
>>                         }
>>                         Else
>>                         {
>>                                 (...)
>>                         }
>>                 }
>>         }
>>
>> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
>> ---
>>  MAINTAINERS                          |   6 ++
>>  drivers/platform/x86/Kconfig         |  15 +++++
>>  drivers/platform/x86/Makefile        |   1 +
>>  drivers/platform/x86/asus-wireless.c | 105 +++++++++++++++++++++++++++++++++++
>>  4 files changed, 127 insertions(+)
>>  create mode 100644 drivers/platform/x86/asus-wireless.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index c984136..e1860f2 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1791,6 +1791,12 @@ S:       Maintained
>>  F:     drivers/platform/x86/asus*.c
>>  F:     drivers/platform/x86/eeepc*.c
>>
>> +ASUS WIRELESS RADIO CONTROL DRIVER
>> +M:     João Paulo Rechi Vita <jprvita@gmail.com>
>> +L:     platform-driver-x86@vger.kernel.org
>> +S:     Maintained
>> +F:     drivers/platform/x86/asus-wireless.c
>> +
>>  ASYNCHRONOUS TRANSFERS/TRANSFORMS (IOAT) API
>>  R:     Dan Williams <dan.j.williams@intel.com>
>>  W:     http://sourceforge.net/projects/xscaleiop
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index d0bfcf8..d3a088b 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -587,6 +587,21 @@ config EEEPC_WMI
>>           If you have an ACPI-WMI compatible Eee PC laptop (>= 1000), say Y or M
>>           here.
>>
>> +config ASUS_WIRELESS
>> +       tristate "Asus Wireless Radio Control Driver"
>> +       depends on ACPI
>> +       depends on INPUT
>> +       default m
>> +       ---help---
>> +         The Asus Wireless Radio Control handles the airplane mode hotkey
>> +         present on some Asus laptops.
>> +
>> +         Say Y or M here if you have an ASUS notebook with an airplane mode
>> +         hotkey.
>> +
>> +         If you choose to compile this driver as a module the module will be
>> +         called asus-wireless.
>> +
>>  config ACPI_WMI
>>         tristate "WMI"
>>         depends on ACPI
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index 4410e91..8b8df29 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -5,6 +5,7 @@
>>  obj-$(CONFIG_ASUS_LAPTOP)      += asus-laptop.o
>>  obj-$(CONFIG_ASUS_WMI)         += asus-wmi.o
>>  obj-$(CONFIG_ASUS_NB_WMI)      += asus-nb-wmi.o
>> +obj-$(CONFIG_ASUS_WIRELESS)    += asus-wireless.o
>>  obj-$(CONFIG_EEEPC_LAPTOP)     += eeepc-laptop.o
>>  obj-$(CONFIG_EEEPC_WMI)                += eeepc-wmi.o
>>  obj-$(CONFIG_MSI_LAPTOP)       += msi-laptop.o
>> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
>> new file mode 100644
>> index 0000000..ef7dba5
>> --- /dev/null
>> +++ b/drivers/platform/x86/asus-wireless.c
>> @@ -0,0 +1,105 @@
>> +/*
>> + * Asus Wireless Radio Control Driver
>> + *
>> + * Copyright (C) 2015 Endless Mobile, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/types.h>
>> +#include <linux/acpi.h>
>> +#include <linux/input.h>
>
>> +#include <linux/pci_ids.h>
>
> Didn't notice exatcly how this one is used.
>

For PCI_VENDOR_ID_ASUSTEK, which is used to set the vendor id field in
the input device structure.

>> +
>> +#define ASUS_WIRELESS_MODULE_NAME "Asus Wireless Radio Control Driver"
>> +
>> +struct asus_wireless_data {
>> +       struct input_dev *inputdev;
>> +};
>> +
>> +static void asus_wireless_notify(struct acpi_device *device, u32 event)
>> +{
>> +       struct asus_wireless_data *data = acpi_driver_data(device);
>> +
>> +       pr_debug("event=0x%X\n", event);
>
> dev_, please.
>

Ok, I'll fix it.

>> +       if (event != 0x88) {
>> +               pr_info("Unknown ASHS event: 0x%X\n", event);
>
> Ditto. Moreover, this is apparently not an info, I can put it either
> to debug or to warn depending how much those I produced in erroneous
> state.
>

I never seen anything else than 0x88 come from the hardware, just
added this to catch potential updates of this device. I don't think it
makes sense to move it to debug because we already print every event
when on debug level. I'm moving it to notice, since I don't think it
is serious enough for a warning (it will not lead to errors), but it
is something out of the ordinary that it seems worth reporting.

>> +               return;
>> +       }
>> +       input_report_key(data->inputdev, KEY_RFKILL, 1);
>> +       input_report_key(data->inputdev, KEY_RFKILL, 0);
>> +       input_sync(data->inputdev);
>> +}
>> +
>> +static int asus_wireless_add(struct acpi_device *device)
>> +{
>> +       struct asus_wireless_data *data;
>> +       int err = -ENOMEM;
>> +
>
>> +       pr_info(ASUS_WIRELESS_MODULE_NAME"\n");
>
> Kinda useless,
>

Right, makes sense.

>> +       data = kzalloc(sizeof(*data), GFP_KERNEL);
>
> devm_kzalloc();
>

Ok, I'm switching this and the other suggestions to their
resource-managed counterparts.

>> +       if (!data)
>> +               return -ENOMEM;
>> +       device->driver_data = data;
>> +
>> +       data->inputdev = input_allocate_device();
>
> Use devm_*
>

Same here.

>> +       if (!data->inputdev)
>> +               goto fail;
>> +
>> +       data->inputdev->name = "Asus Wireless Radio Control";
>> +       data->inputdev->phys = "asus-wireless/input0";
>> +       data->inputdev->id.bustype = BUS_HOST;
>> +       data->inputdev->id.vendor = PCI_VENDOR_ID_ASUSTEK;
>> +       data->inputdev->dev.parent = &device->dev;
>> +       set_bit(EV_REP, data->inputdev->evbit);
>> +       set_bit(KEY_RFKILL, data->inputdev->keybit);
>> +
>> +       err = input_register_device(data->inputdev);
>> +       if (err)
>> +               goto fail;
>> +       return 0;
>> +
>> +fail:
>> +       device->driver->ops.remove(device);
>
> Better to show explicitly what you do here. Currently I might say it's
> a strange way I've ever seen to express the error path.
>

After switching to managed resources, there is nothing left to be done
here (at least for this patch).

>> +       return err;
>> +}
>> +
>> +static int asus_wireless_remove(struct acpi_device *device)
>> +{
>> +       struct asus_wireless_data *data = acpi_driver_data(device);
>> +
>
>> +       pr_info("Removing "ASUS_WIRELESS_MODULE_NAME"\n");
>
> Kinda useless.
>

Makes sense.

>> +       if (data->inputdev)
>
> How is it possible?
>
>> +               input_unregister_device(data->inputdev);
>
> Redundant after switching to devm_
>
>> +       kfree(data);
>
> Ditto.
>

After switching to managed resources, there is nothing left to be done
in the remove path (at least for now).

>> +       return 0;
>> +}
>> +
>> +static const struct acpi_device_id device_ids[] = {
>> +       {"ATK4002", 0},
>> +       {"", 0},
>> +};
>> +MODULE_DEVICE_TABLE(acpi, device_ids);
>> +
>> +static struct acpi_driver asus_wireless_driver = {
>> +       .name = ASUS_WIRELESS_MODULE_NAME,
>> +       .class = "hotkey",
>> +       .ids = device_ids,
>> +       .ops = {
>> +               .add = asus_wireless_add,
>> +               .remove = asus_wireless_remove,
>> +               .notify = asus_wireless_notify,
>> +       },
>> +};
>> +module_acpi_driver(asus_wireless_driver);
>> +
>> +MODULE_DESCRIPTION(ASUS_WIRELESS_MODULE_NAME);
>> +MODULE_AUTHOR("João Paulo Rechi Vita <jprvita@gmail.com>");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.5.0
>>

Thanks a lot for your feedback, Andy. I'll send a revised version shortly.

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 4/4] asus-wireless: Toggle airplane mode LED

[João Paulo Rechi Vita]

On 27 December 2015 at 08:32, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Sat, Dec 26, 2015 at 4:56 PM, João Paulo Rechi Vita
> <jprvita@gmail.com> wrote:
>> In the ASHS device we have the HSWC method, which basically calls either
>> OWGD or OWGS, depending on its parameter:
>>
>>         Device (ASHS)
>>         {
>>                 Name (_HID, "ATK4002")  // _HID: Hardware ID
>>                 Method (HSWC, 1, Serialized)
>>                 {
>>                         If ((Arg0 < 0x02))
>>                         {
>>                                 OWGD (Arg0)
>>                                 Return (One)
>>                         }
>>                         If ((Arg0 == 0x02))
>>                         {
>>                                 Local0 = OWGS ()
>>                                 If (Local0)
>>                                 {
>>                                         Return (0x05)
>>                                 }
>>                                 Else
>>                                 {
>>                                         Return (0x04)
>>                                 }
>>                         }
>>                         If ((Arg0 == 0x03))
>>                         {
>>                                 Return (0xFF)
>>                         }
>>                         If ((Arg0 == 0x04))
>>                         {
>>                                 OWGD (Zero)
>>                                 Return (One)
>>                         }
>>                         If ((Arg0 == 0x05))
>>                         {
>>                                 OWGD (One)
>>                                 Return (One)
>>                         }
>>                         If ((Arg0 == 0x80))
>>                         {
>>                                 Return (One)
>>                         }
>>                 }
>>                 Method (_STA, 0, NotSerialized)  // _STA: Status
>>                 {
>>                         If ((MSOS () >= OSW8))
>>                         {
>>                                 Return (0x0F)
>>                         }
>>                         Else
>>                         {
>>                                 Return (Zero)
>>                         }
>>                 }
>>         }
>>
>> On the Asus E202SA laptop, which does not have an airplane mode LED,
>> OWGD has an empty implementation and OWGS simply returns 0. On the Asus
>> X555UB these methods have the following implementation:
>>
>>         Method (OWGD, 1, Serialized)
>>         {
>>                 SGPL (0x0203000F, Arg0)
>>                 SGPL (0x0203000F, Arg0)
>>         }
>>
>>         Method (OWGS, 0, Serialized)
>>         {
>>                 Store (RGPL (0x0203000F), Local0)
>>                 Return (Local0)
>>         }
>>
>> Where OWGD(1) sets the airplane mode LED ON, OWGD(0) set it off, and
>> OWGS() returns its state.
>>
>> This commit makes use of a newly implemented RFKill LED trigger to
>> trigger the LED when the system enters or exits "Airplane Mode", there
>> is, when all radios are blocked.
>>
>> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
>> ---
>>  drivers/platform/x86/Kconfig         |  2 +
>>  drivers/platform/x86/asus-wireless.c | 81 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 83 insertions(+)
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index d3a088b..3d8dc0b 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -592,6 +592,8 @@ config ASUS_WIRELESS
>>         depends on ACPI
>>         depends on INPUT
>>         default m
>> +       select NEW_LEDS
>> +       select LEDS_CLASS
>>         ---help---
>>           The Asus Wireless Radio Control handles the airplane mode hotkey
>>           present on some Asus laptops.
>> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
>> index 7928efd..489ef83 100644
>> --- a/drivers/platform/x86/asus-wireless.c
>> +++ b/drivers/platform/x86/asus-wireless.c
>> @@ -17,13 +17,76 @@
>>  #include <linux/acpi.h>
>>  #include <linux/input.h>
>>  #include <linux/pci_ids.h>
>> +#include <linux/leds.h>
>>
>>  #define ASUS_WIRELESS_MODULE_NAME "Asus Wireless Radio Control Driver"
>> +#define ASUS_WIRELESS_LED_STATUS 0x2
>> +#define ASUS_WIRELESS_LED_OFF 0x4
>> +#define ASUS_WIRELESS_LED_ON 0x5
>>
>>  struct asus_wireless_data {
>>         struct input_dev *inputdev;
>> +       struct acpi_device *acpidev;
>
> You can get this easily from struct device.
>

If I'm understanding it correctly that only works when the acpi device
is a companion to a physical device, which is not the case here
(ACPI_COMPANION(acpi_device->dev)==NULL). I'm keeping this for now,
please let me know if I'm missing something.

>> +       struct workqueue_struct *wq;
>> +       struct work_struct led_work;
>> +       struct led_classdev led;
>> +       int led_state;
>>  };
>>
>> +static u64 asus_wireless_method(acpi_handle handle, const char *method,
>> +                               int param)
>> +{
>> +       union acpi_object obj;
>> +       struct acpi_object_list p;
>> +       acpi_status s;
>> +       u64 ret;
>> +
>> +       pr_debug("Evaluating method %s, parameter 0x%X\n", method, param);
>
> acpi_handle_* in such cases.
>

Ok.

>> +       obj.type = ACPI_TYPE_INTEGER;
>> +       obj.integer.value = param;
>> +       p.count = 1;
>> +       p.pointer = &obj;
>> +
>> +       s = acpi_evaluate_integer(handle, (acpi_string) method, &p, &ret);
>> +       if (!ACPI_SUCCESS(s))
>
> ACPI_FAILURE()
>

Ok.

>> +               pr_err("Failed to evaluate method %s, parameter 0x%X (%d)\n",
>> +                      method, param, s);
>> +       pr_debug("%s returned 0x%X\n", method, (uint) ret);
>> +       return ret;
>> +}
>> +
>> +static enum led_brightness asus_wireless_led_get(struct led_classdev *led)
>> +{
>> +       struct asus_wireless_data *data;
>> +       int s;
>> +
>> +       data = container_of(led, struct asus_wireless_data, led);
>> +       s = asus_wireless_method(data->acpidev->handle, "HSWC",
>
> Usually we get a handle through specific macro ACPI_HANDLE from a
> struct device (see above).
>
>> +                                ASUS_WIRELESS_LED_STATUS);
>> +       if (s == ASUS_WIRELESS_LED_ON)
>> +               return LED_FULL;
>> +       return LED_OFF;
>> +}
>> +
>> +static void asus_wireless_led_update(struct work_struct *work)
>> +{
>> +       struct asus_wireless_data *data;
>> +
>> +       data = container_of(work, struct asus_wireless_data, led_work);
>> +       asus_wireless_method(data->acpidev->handle, "HSWC", data->led_state);
>
> Ditto.
>

These last two depend on ACPI_COMPANION as well. I'm changing this to
call acpi_device_handle() instead of accessing the pointer directly.

I will send an updated version shortly. Thanks again for your feedback.

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver

[João Paulo Rechi Vita]

On 19 December 2015 at 02:23, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Dec 15, 2015 at 10:30:39AM -0500, João Paulo Rechi Vita wrote:
>
> ...
>
>> +static void asus_wrc_notify(struct acpi_device *device, u32 event)
>> +{
>> + struct asus_wrc_data *data = acpi_driver_data(device);
>> +
>> + pr_debug("event=0x%X\n", event);
>> +
>> + if (!sparse_keymap_report_event(data->inputdev, event, 1, true))
>> + pr_info("Unknown ASHS event: 0x%X\n", event);
>> +}
>> +
>> +static int asus_wrc_add(struct acpi_device *device)
>> +{
>> + struct asus_wrc_data *data;
>> + int err = -ENOMEM;
>> +
>> + pr_info(ASUS_WRC_MODULE_NAME"\n");
>> +
>> + data = kzalloc(sizeof(struct asus_wrc_data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + data->inputdev = input_allocate_device();
>> + if (!data->inputdev)
>> + goto fail;
>> +
>> + data->inputdev->name = "Asus Wireless Radio Control";
>> + data->inputdev->phys = "asus-wrc/input0";
>> + data->inputdev->id.bustype = BUS_HOST;
>> + data->inputdev->dev.parent = &device->dev;
>> + set_bit(EV_REP, data->inputdev->evbit);
>
> The version Mousou includes some minor differences:
>
> + switch_dev->id.vendor = PCI_VENDOR_ID_ASUSTEK;
>
> The vendor addition seems appropriate.

 Agree, I'll add that.

> The rest appears to be a slightly more directly approach to a sparse
> keymap
> which seems like overkill for a driver with a single key.
>
> + set_bit(EV_KEY, switch_dev->evbit);
> + set_bit(KEY_RFKILL, switch_dev->keybit);
>
> Mousou's driver results in about 30 less lines as well. Please compare and
> see
> if we might be able to merge the best of each version.
>

I only had the time to skim through Mousou's patch yet, but I'll check it in
detail and try to incorporate things that I might have missed.

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver

[João Paulo Rechi Vita]

On 19 December 2015 at 01:39, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Dec 15, 2015 at 10:30:39AM -0500, João Paulo Rechi Vita wrote:
>> Some Asus notebooks like the Asus E202SA and the Asus X555UB have a
>> separate ACPI device for notifications from the airplane mode hotkey.
>> This device is called "Wireless Radio Control" in Asus websites and ASHS
>> in the DSDT, and its ACPI _HID is ATK4002 in the two models mentioned
>> above.
>>
>> For these models, when the airplane mode hotkey (Fn+F2) is pressed, a
>> query 0x0B is started in the Embedded Controller, and all this query does
>> is a notify ASHS with the value 0x88 (for acpi_osi >= "Windows 2012"):
>>
>> Scope (_SB.PCI0.SBRG.EC0)
>> {
>> (...)
>> Method (_Q0B, 0, NotSerialized) // _Qxx: EC Query
>> {
>> If ((MSOS () >= OSW8))
>> {
>> Notify (ASHS, 0x88) // Device-Specific
>> }
>> Else
>> {
>> (...)
>> }
>> }
>> }
>>
>> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
>
> Hi Joao,
>
> Nice work!
>
> In the future, [please be sure to include all the maintainers listed by
> get_maintainer.pl when submitting patches for review.
>
> No concerns from me on this portion of the series.
>

Great news, thank you! I'll make sure to check get_maintainer.pl for my next
submissions, I have not worked with that script before.

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver

[Darren Hart]

On Tue, Dec 15, 2015 at 10:30:39AM -0500, João Paulo Rechi Vita wrote:

...

> +static void asus_wrc_notify(struct acpi_device *device, u32 event)
> +{
> +	struct asus_wrc_data *data = acpi_driver_data(device);
> +
> +	pr_debug("event=0x%X\n", event);
> +
> +	if (!sparse_keymap_report_event(data->inputdev, event, 1, true))
> +		pr_info("Unknown ASHS event: 0x%X\n", event);
> +}
> +
> +static int asus_wrc_add(struct acpi_device *device)
> +{
> +	struct asus_wrc_data *data;
> +	int err = -ENOMEM;
> +
> +	pr_info(ASUS_WRC_MODULE_NAME"\n");
> +
> +	data = kzalloc(sizeof(struct asus_wrc_data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->inputdev = input_allocate_device();
> +	if (!data->inputdev)
> +		goto fail;
> +
> +	data->inputdev->name = "Asus Wireless Radio Control";
> +	data->inputdev->phys = "asus-wrc/input0";
> +	data->inputdev->id.bustype = BUS_HOST;
> +	data->inputdev->dev.parent = &device->dev;
> +	set_bit(EV_REP, data->inputdev->evbit);

The version Mousou includes some minor differences:

+	switch_dev->id.vendor = PCI_VENDOR_ID_ASUSTEK;

The vendor addition seems appropriate.

The rest appears to be a slightly more directly approach to a sparse keymap
which seems like overkill for a driver with a single key.

+	set_bit(EV_KEY, switch_dev->evbit);
+	set_bit(KEY_RFKILL, switch_dev->keybit);

Mousou's driver results in about 30 less lines as well. Please compare and see
if we might be able to merge the best of each version.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver

[Darren Hart]

On Tue, Dec 15, 2015 at 10:30:39AM -0500, João Paulo Rechi Vita wrote:
> Some Asus notebooks like the Asus E202SA and the Asus X555UB have a
> separate ACPI device for notifications from the airplane mode hotkey.
> This device is called "Wireless Radio Control" in Asus websites and ASHS
> in the DSDT, and its ACPI _HID is ATK4002 in the two models mentioned
> above.
> 
> For these models, when the airplane mode hotkey (Fn+F2) is pressed, a
> query 0x0B is started in the Embedded Controller, and all this query does
> is a notify ASHS with the value 0x88 (for acpi_osi >= "Windows 2012"):
> 
> 	Scope (_SB.PCI0.SBRG.EC0)
> 	{
> 		(...)
> 		Method (_Q0B, 0, NotSerialized)  // _Qxx: EC Query
> 		{
> 			If ((MSOS () >= OSW8))
> 			{
> 				Notify (ASHS, 0x88) // Device-Specific
> 			}
> 			Else
> 			{
> 				(...)
> 			}
> 		}
> 	}
> 
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>

Hi Joao,

Nice work!

In the future, [please be sure to include all the maintainers listed by
get_maintainer.pl when submitting patches for review.

No concerns from me on this portion of the series.

-- 
Darren Hart
Intel Open Source Technology Center

[PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver

[João Paulo Rechi Vita]

Some Asus notebooks like the Asus E202SA and the Asus X555UB have a
separate ACPI device for notifications from the airplane mode hotkey.
This device is called "Wireless Radio Control" in Asus websites and ASHS
in the DSDT, and its ACPI _HID is ATK4002 in the two models mentioned
above.

For these models, when the airplane mode hotkey (Fn+F2) is pressed, a
query 0x0B is started in the Embedded Controller, and all this query does
is a notify ASHS with the value 0x88 (for acpi_osi >= "Windows 2012"):

	Scope (_SB.PCI0.SBRG.EC0)
	{
		(...)
		Method (_Q0B, 0, NotSerialized)  // _Qxx: EC Query
		{
			If ((MSOS () >= OSW8))
			{
				Notify (ASHS, 0x88) // Device-Specific
			}
			Else
			{
				(...)
			}
		}
	}

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 MAINTAINERS                     |   6 ++
 drivers/platform/x86/Kconfig    |  15 +++++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/asus-wrc.c | 118 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 140 insertions(+)
 create mode 100644 drivers/platform/x86/asus-wrc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9bff63c..b9849ea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1791,6 +1791,12 @@ S:	Maintained
 F:	drivers/platform/x86/asus*.c
 F:	drivers/platform/x86/eeepc*.c
 
+ASUS WIRELESS RADIO CONTROL DRIVER
+M:	João Paulo Rechi Vita <jprvita@gmail.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/x86/asus-wrc.c
+
 ASYNCHRONOUS TRANSFERS/TRANSFORMS (IOAT) API
 R:	Dan Williams <dan.j.williams@intel.com>
 W:	http://sourceforge.net/projects/xscaleiop
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 1089eaa..edd5de4 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -587,6 +587,21 @@ config EEEPC_WMI
 	  If you have an ACPI-WMI compatible Eee PC laptop (>= 1000), say Y or M
 	  here.
 
+config ASUS_WRC
+	tristate "Asus Wireless Radio Control Driver"
+	depends on ACPI
+	depends on INPUT
+	select INPUT_SPARSEKMAP
+	---help---
+	  The Asus Wireless Radio Control handles the airplane mode hotkey
+	  present on some Asus laptops, known as ASHS in the DSDT.
+
+	  Say Y or M here if you have an ASUS notebook with an airplane mode
+	  hotkey.
+
+	  If you choose to compile this driver as a module the module will be
+	  called asus-wrc.
+
 config ACPI_WMI
 	tristate "WMI"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 3ca78a3..98b5c2d 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_ASUS_LAPTOP)	+= asus-laptop.o
 obj-$(CONFIG_ASUS_WMI)		+= asus-wmi.o
 obj-$(CONFIG_ASUS_NB_WMI)	+= asus-nb-wmi.o
+obj-$(CONFIG_ASUS_WRC)		+= asus-wrc.o
 obj-$(CONFIG_EEEPC_LAPTOP)	+= eeepc-laptop.o
 obj-$(CONFIG_EEEPC_WMI)		+= eeepc-wmi.o
 obj-$(CONFIG_MSI_LAPTOP)	+= msi-laptop.o
diff --git a/drivers/platform/x86/asus-wrc.c b/drivers/platform/x86/asus-wrc.c
new file mode 100644
index 0000000..13a2022
--- /dev/null
+++ b/drivers/platform/x86/asus-wrc.c
@@ -0,0 +1,118 @@
+/*
+ * Asus Wireless Radio Control Driver
+ *
+ * Copyright (C) 2015 Endless Mobile, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/acpi.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+
+#define ASUS_WRC_MODULE_NAME "Asus Wireless Radio Control Driver"
+
+struct asus_wrc_data {
+	struct input_dev *inputdev;
+};
+
+static const struct key_entry asus_wrc_keymap[] = {
+	{ KE_KEY, 0x88, { KEY_RFKILL } },
+	{ KE_END, 0 }
+};
+
+static void asus_wrc_notify(struct acpi_device *device, u32 event)
+{
+	struct asus_wrc_data *data = acpi_driver_data(device);
+
+	pr_debug("event=0x%X\n", event);
+
+	if (!sparse_keymap_report_event(data->inputdev, event, 1, true))
+		pr_info("Unknown ASHS event: 0x%X\n", event);
+}
+
+static int asus_wrc_add(struct acpi_device *device)
+{
+	struct asus_wrc_data *data;
+	int err = -ENOMEM;
+
+	pr_info(ASUS_WRC_MODULE_NAME"\n");
+
+	data = kzalloc(sizeof(struct asus_wrc_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->inputdev = input_allocate_device();
+	if (!data->inputdev)
+		goto fail;
+
+	data->inputdev->name = "Asus Wireless Radio Control";
+	data->inputdev->phys = "asus-wrc/input0";
+	data->inputdev->id.bustype = BUS_HOST;
+	data->inputdev->dev.parent = &device->dev;
+	set_bit(EV_REP, data->inputdev->evbit);
+
+	err = sparse_keymap_setup(data->inputdev, asus_wrc_keymap, NULL);
+	if (err)
+		goto fail;
+
+	err = input_register_device(data->inputdev);
+	if (err)
+		goto fail;
+
+	device->driver_data = data;
+	return 0;
+
+fail:
+	device->driver->ops.remove(device);
+	return err;
+}
+
+static int asus_wrc_remove(struct acpi_device *device)
+{
+	struct asus_wrc_data *data = acpi_driver_data(device);
+
+	pr_info("Removing "ASUS_WRC_MODULE_NAME"\n");
+
+	if (data) {
+		if (data->inputdev) {
+			sparse_keymap_free(data->inputdev);
+			input_unregister_device(data->inputdev);
+			data->inputdev = NULL;
+		}
+
+		kfree(data);
+		data = NULL;
+	}
+	return 0;
+}
+
+static const struct acpi_device_id device_ids[] = {
+	{"ATK4002", 0},
+	{"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, device_ids);
+
+static struct acpi_driver asus_wrc_driver = {
+	.name = ASUS_WRC_MODULE_NAME,
+	.class = "hotkey",
+	.ids = device_ids,
+	.ops = {
+		.add = asus_wrc_add,
+		.remove = asus_wrc_remove,
+		.notify = asus_wrc_notify,
+	},
+};
+module_acpi_driver(asus_wrc_driver);
+
+MODULE_DESCRIPTION(ASUS_WRC_MODULE_NAME);
+MODULE_AUTHOR("João Paulo Rechi Vita <jprvita@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
2.5.0