All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/4] Asus Wireless Radio Control driver
@ 2015-12-15 15:30 João Paulo Rechi Vita
  2015-12-15 15:30 ` [PATCH 1/4] platform/x86: Add " João Paulo Rechi Vita
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-15 15:30 UTC (permalink / raw)
  To: Corentin Chary
  Cc: platform-driver-x86, acpi4asus-user, jprvita, João Paulo Rechi Vita

This series is a request for comments on the driver for the "Asus Wireless
Radio Control" device, as named on the vendor website, which handles
notifications from the airplane mode hotkey and drives the airplane mode
indicator LED. This device appears in the DSDT as "ASHS".

When the airplane mode hotkey is pressed a query 0x0B is started in the
Embedded Controller, and all this query does is a notify ASHS with the value
0x88. ASHS also has one method HSWC, which can drive the airplane mode LED and
query its status, according to its parameter.

To properly drive the LED a new RFKill trigger was implemented, to track the
global state of all RFKill switches in the system, and turn the LED ON when all
are blocked, and OFF otherwise. This code will have to go through review on
linux-wireless, but first I wanted to get feedback on the WRC driver itself.

There is one known issue: the airplane mode LED uses the same ID as the WLAN
LED in asus-wmi, so at the moment to have the LED being driven correctly by
asus-wrc I have to disable asus-wmi. Even with wapf=0, which does not register
a WLAN LED in asus-wmi, the conflict still occurs because
ASUS_WMI_DSTS_USER_BIT is set. Please advise on what is the best way to solve
this problem.

I've chosen to make this a separate module because this is a separate entry in
the DSDT, and checkpatch told me to add an entry in MAINTAINERS in this case.
Please let me know if any of this should have been done differently.

I have access to two Asus laptops which need this driver for the airplane mode
hotkey to work: E202SA (which does not have a LED) and X555UB (which have a
LED), and on https://bugzilla.kernel.org/show_bug.cgi?id=98931#c22 there is
report of a 3rd laptop that gets fixed with this driver. This should also fix
the mentioned bug report.

Regards,

Joao Paulo.

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

 MAINTAINERS                     |   6 ++
 drivers/platform/x86/Kconfig    |  15 +++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/asus-wrc.c | 200 ++++++++++++++++++++++++++++++++++++++++
 net/rfkill/core.c               |  30 ++++++
 5 files changed, 252 insertions(+)
 create mode 100644 drivers/platform/x86/asus-wrc.c

-- 
2.5.0

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver
  2015-12-15 15:30 [RFC 0/4] Asus Wireless Radio Control driver João Paulo Rechi Vita
@ 2015-12-15 15:30 ` João Paulo Rechi Vita
  2015-12-19  6:39   ` Darren Hart
  2015-12-19  7:23   ` Darren Hart
  2015-12-15 15:30 ` [PATCH 2/4] asus-wrc: Add ACPI HID ATK4001 João Paulo Rechi Vita
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-15 15:30 UTC (permalink / raw)
  To: Corentin Chary
  Cc: platform-driver-x86, acpi4asus-user, jprvita, 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/4] asus-wrc: Add ACPI HID ATK4001
  2015-12-15 15:30 [RFC 0/4] Asus Wireless Radio Control driver João Paulo Rechi Vita
  2015-12-15 15:30 ` [PATCH 1/4] platform/x86: Add " João Paulo Rechi Vita
@ 2015-12-15 15:30 ` João Paulo Rechi Vita
  2015-12-19  6:40   ` Darren Hart
  2015-12-15 15:30 ` [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger João Paulo Rechi Vita
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-15 15:30 UTC (permalink / raw)
  To: Corentin Chary
  Cc: platform-driver-x86, acpi4asus-user, jprvita, 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-wrc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/asus-wrc.c b/drivers/platform/x86/asus-wrc.c
index 13a2022..941ad73 100644
--- a/drivers/platform/x86/asus-wrc.c
+++ b/drivers/platform/x86/asus-wrc.c
@@ -96,6 +96,7 @@ static int asus_wrc_remove(struct acpi_device *device)
 }
 
 static const struct acpi_device_id device_ids[] = {
+	{"ATK4001", 0},
 	{"ATK4002", 0},
 	{"", 0},
 };
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger
  2015-12-15 15:30 [RFC 0/4] Asus Wireless Radio Control driver João Paulo Rechi Vita
  2015-12-15 15:30 ` [PATCH 1/4] platform/x86: Add " João Paulo Rechi Vita
  2015-12-15 15:30 ` [PATCH 2/4] asus-wrc: Add ACPI HID ATK4001 João Paulo Rechi Vita
@ 2015-12-15 15:30 ` João Paulo Rechi Vita
  2015-12-19  0:22   ` Darren Hart
  2015-12-15 15:30 ` [PATCH 4/4] asus-wrc: Toggle airplane mode LED João Paulo Rechi Vita
  2015-12-19  8:00 ` [RFC 0/4] Asus Wireless Radio Control driver Darren Hart
  4 siblings, 1 reply; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-15 15:30 UTC (permalink / raw)
  To: Corentin Chary
  Cc: platform-driver-x86, acpi4asus-user, jprvita, 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/4] asus-wrc: Toggle airplane mode LED
  2015-12-15 15:30 [RFC 0/4] Asus Wireless Radio Control driver João Paulo Rechi Vita
                   ` (2 preceding siblings ...)
  2015-12-15 15:30 ` [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger João Paulo Rechi Vita
@ 2015-12-15 15:30 ` João Paulo Rechi Vita
  2015-12-19  8:00 ` [RFC 0/4] Asus Wireless Radio Control driver Darren Hart
  4 siblings, 0 replies; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-15 15:30 UTC (permalink / raw)
  To: Corentin Chary
  Cc: platform-driver-x86, acpi4asus-user, jprvita, 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/asus-wrc.c | 81 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/drivers/platform/x86/asus-wrc.c b/drivers/platform/x86/asus-wrc.c
index 941ad73..6acb36e 100644
--- a/drivers/platform/x86/asus-wrc.c
+++ b/drivers/platform/x86/asus-wrc.c
@@ -17,11 +17,21 @@
 #include <linux/acpi.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
+#include <linux/leds.h>
 
 #define ASUS_WRC_MODULE_NAME "Asus Wireless Radio Control Driver"
 
+#define ASUS_WRC_LED_STATUS 0x2
+#define ASUS_WRC_LED_OFF 0x4
+#define ASUS_WRC_LED_ON 0x5
+
 struct asus_wrc_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 const struct key_entry asus_wrc_keymap[] = {
@@ -29,6 +39,57 @@ static const struct key_entry asus_wrc_keymap[] = {
 	{ KE_END, 0 }
 };
 
+static u64 asus_wrc_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_wrc_led_get(struct led_classdev *led)
+{
+	struct asus_wrc_data *data = container_of(led, struct asus_wrc_data,
+						  led);
+	int s = asus_wrc_method(data->acpidev->handle, "HSWC",
+				ASUS_WRC_LED_STATUS);
+	if (s == ASUS_WRC_LED_ON)
+		return LED_FULL;
+	return LED_OFF;
+}
+
+static void asus_wrc_led_update(struct work_struct *work)
+{
+	struct asus_wrc_data *data = container_of(work, struct asus_wrc_data,
+						  led_work);
+	asus_wrc_method(data->acpidev->handle, "HSWC", data->led_state);
+}
+
+static void asus_wrc_led_set(struct led_classdev *led,
+			     enum led_brightness value)
+{
+	struct asus_wrc_data *data = container_of(led, struct asus_wrc_data,
+						  led);
+	data->led_state = value == LED_OFF ? ASUS_WRC_LED_OFF : ASUS_WRC_LED_ON;
+	queue_work(data->wq, &data->led_work);
+}
+
 static void asus_wrc_notify(struct acpi_device *device, u32 event)
 {
 	struct asus_wrc_data *data = acpi_driver_data(device);
@@ -50,6 +111,8 @@ static int asus_wrc_add(struct acpi_device *device)
 	if (!data)
 		return -ENOMEM;
 
+	data->acpidev = device;
+
 	data->inputdev = input_allocate_device();
 	if (!data->inputdev)
 		goto fail;
@@ -60,6 +123,10 @@ static int asus_wrc_add(struct acpi_device *device)
 	data->inputdev->dev.parent = &device->dev;
 	set_bit(EV_REP, data->inputdev->evbit);
 
+	data->wq = create_singlethread_workqueue("asus_wrc_workqueue");
+	if (!data->wq)
+		goto fail;
+
 	err = sparse_keymap_setup(data->inputdev, asus_wrc_keymap, NULL);
 	if (err)
 		goto fail;
@@ -68,6 +135,17 @@ static int asus_wrc_add(struct acpi_device *device)
 	if (err)
 		goto fail;
 
+	INIT_WORK(&data->led_work, asus_wrc_led_update);
+	data->led.name = "asus-wrc::airplane_mode";
+	data->led.brightness_set = asus_wrc_led_set;
+	data->led.brightness_get = asus_wrc_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;
+
 	device->driver_data = data;
 	return 0;
 
@@ -89,6 +167,9 @@ static int asus_wrc_remove(struct acpi_device *device)
 			data->inputdev = NULL;
 		}
 
+		if (data->wq)
+			destroy_workqueue(data->wq);
+
 		kfree(data);
 		data = NULL;
 	}
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger
  2015-12-15 15:30 ` [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger João Paulo Rechi Vita
@ 2015-12-19  0:22   ` Darren Hart
  2015-12-19  0:36     ` João Paulo Rechi Vita
  2015-12-19  2:25     ` David Miller
  0 siblings, 2 replies; 17+ messages in thread
From: Darren Hart @ 2015-12-19  0:22 UTC (permalink / raw)
  To: João Paulo Rechi Vita, Johannes Berg, David S. Miller
  Cc: Corentin Chary, platform-driver-x86, acpi4asus-user,
	João Paulo Rechi Vita, linux-wireless, netdev

On Tue, Dec 15, 2015 at 10:30:41AM -0500, João Paulo Rechi Vita wrote:
> 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.

So this will need to go through Johannes and David per get_maintainer.pl before
we can use it in platform drivers.

+Johannes
+David
+wireless
+netdev

-- 
Darren Hart
Intel Open Source Technology Center

> 
> 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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger
  2015-12-19  0:22   ` Darren Hart
@ 2015-12-19  0:36     ` João Paulo Rechi Vita
  2015-12-19  2:25     ` David Miller
  1 sibling, 0 replies; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-19  0:36 UTC (permalink / raw)
  To: Darren Hart
  Cc: Johannes Berg, David S. Miller, Corentin Chary,
	platform-driver-x86, acpi4asus-user, João Paulo Rechi Vita,
	linux-wireless, netdev

On 18 December 2015 at 19:22, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Dec 15, 2015 at 10:30:41AM -0500, João Paulo Rechi Vita wrote:
>> 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.
>
> So this will need to go through Johannes and David per get_maintainer.pl before
> we can use it in platform drivers.
>

Yes, I am aware of that. I just wanted to get feedback and validate
the new platform driver that makes use of this new RFKill LED trigger
before proposing it, since I expect that will be their first
questioning.

For those who were not in the original message, this is patch is part
of a series implementing a new platform driver for the airplane mode
hotkey and LED present in some Asus laptops, which is a separate ACPI
device (not part of WMI) with ACPI _HID "ASHS" and named "Wireless
Radio Control" by Asus.

Thanks for your feedback!

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger
  2015-12-19  0:22   ` Darren Hart
  2015-12-19  0:36     ` João Paulo Rechi Vita
@ 2015-12-19  2:25     ` David Miller
  1 sibling, 0 replies; 17+ messages in thread
From: David Miller @ 2015-12-19  2:25 UTC (permalink / raw)
  To: dvhart
  Cc: jprvita, johannes, corentin.chary, platform-driver-x86,
	acpi4asus-user, jprvita, linux-wireless, netdev

From: Darren Hart <dvhart@infradead.org>
Date: Fri, 18 Dec 2015 16:22:12 -0800

> On Tue, Dec 15, 2015 at 10:30:41AM -0500, João Paulo Rechi Vita wrote:
>> 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.
> 
> So this will need to go through Johannes and David per get_maintainer.pl before
> we can use it in platform drivers.
> 
> +Johannes
> +David
> +wireless
> +netdev

RFKILL changes go via the wireless tree.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver
  2015-12-15 15:30 ` [PATCH 1/4] platform/x86: Add " João Paulo Rechi Vita
@ 2015-12-19  6:39   ` Darren Hart
       [not found]     ` <CA+A7VXWGTAsen2V29ifJ9nGCuHRnQnZi=Q_oEi1fkmeo=t1gxQ@mail.gmail.com>
  2015-12-19  7:23   ` Darren Hart
  1 sibling, 1 reply; 17+ messages in thread
From: Darren Hart @ 2015-12-19  6:39 UTC (permalink / raw)
  To: João Paulo Rechi Vita
  Cc: Corentin Chary, platform-driver-x86, acpi4asus-user,
	João Paulo Rechi Vita

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] asus-wrc: Add ACPI HID ATK4001
  2015-12-15 15:30 ` [PATCH 2/4] asus-wrc: Add ACPI HID ATK4001 João Paulo Rechi Vita
@ 2015-12-19  6:40   ` Darren Hart
  2015-12-20 18:57     ` João Paulo Rechi Vita
  0 siblings, 1 reply; 17+ messages in thread
From: Darren Hart @ 2015-12-19  6:40 UTC (permalink / raw)
  To: João Paulo Rechi Vita
  Cc: Corentin Chary, platform-driver-x86, acpi4asus-user,
	João Paulo Rechi Vita

On Tue, Dec 15, 2015 at 10:30:40AM -0500, João Paulo Rechi Vita wrote:
> 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".

Do you happen to have an ASL dump of this ID? Does it differ in any significant
way?

> 
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
> Reported-by: Tasev Nikola <tasev.stefanoska@skynet.be>
> ---
>  drivers/platform/x86/asus-wrc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/x86/asus-wrc.c b/drivers/platform/x86/asus-wrc.c
> index 13a2022..941ad73 100644
> --- a/drivers/platform/x86/asus-wrc.c
> +++ b/drivers/platform/x86/asus-wrc.c
> @@ -96,6 +96,7 @@ static int asus_wrc_remove(struct acpi_device *device)
>  }
>  
>  static const struct acpi_device_id device_ids[] = {
> +	{"ATK4001", 0},
>  	{"ATK4002", 0},
>  	{"", 0},
>  };
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver
  2015-12-15 15:30 ` [PATCH 1/4] platform/x86: Add " João Paulo Rechi Vita
  2015-12-19  6:39   ` Darren Hart
@ 2015-12-19  7:23   ` Darren Hart
       [not found]     ` <CA+A7VXU9MepWmWL6BS+XyjbLs2s_ONR5Yy3tqT--cbYS7p1P1Q@mail.gmail.com>
  1 sibling, 1 reply; 17+ messages in thread
From: Darren Hart @ 2015-12-19  7:23 UTC (permalink / raw)
  To: João Paulo Rechi Vita, Mousou Yuu
  Cc: Corentin Chary, platform-driver-x86, acpi4asus-user,
	João Paulo Rechi Vita

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/4] Asus Wireless Radio Control driver
  2015-12-15 15:30 [RFC 0/4] Asus Wireless Radio Control driver João Paulo Rechi Vita
                   ` (3 preceding siblings ...)
  2015-12-15 15:30 ` [PATCH 4/4] asus-wrc: Toggle airplane mode LED João Paulo Rechi Vita
@ 2015-12-19  8:00 ` Darren Hart
  2015-12-20 19:01   ` João Paulo Rechi Vita
  4 siblings, 1 reply; 17+ messages in thread
From: Darren Hart @ 2015-12-19  8:00 UTC (permalink / raw)
  To: João Paulo Rechi Vita
  Cc: Corentin Chary, platform-driver-x86, acpi4asus-user,
	João Paulo Rechi Vita

On Tue, Dec 15, 2015 at 10:30:38AM -0500, João Paulo Rechi Vita wrote:
> This series is a request for comments on the driver for the "Asus Wireless
> Radio Control" device, as named on the vendor website, which handles
> notifications from the airplane mode hotkey and drives the airplane mode
> indicator LED. This device appears in the DSDT as "ASHS".
> 
> When the airplane mode hotkey is pressed a query 0x0B is started in the
> Embedded Controller, and all this query does is a notify ASHS with the value
> 0x88. ASHS also has one method HSWC, which can drive the airplane mode LED and
> query its status, according to its parameter.

Hi Joao,

Thanks for a detailed introduction, very helpful.

> 
> To properly drive the LED a new RFKill trigger was implemented, to track the
> global state of all RFKill switches in the system, and turn the LED ON when all
> are blocked, and OFF otherwise. This code will have to go through review on
> linux-wireless, but first I wanted to get feedback on the WRC driver itself.

Apologies, I see I jumped the gun on this. I was rushed this afternoon and was
trying to avoid being the bottleneck. At least we won't have to wait long for
Johannes to decide after we're done with the review here.

> 
> There is one known issue: the airplane mode LED uses the same ID as the WLAN
> LED in asus-wmi, so at the moment to have the LED being driven correctly by
> asus-wrc I have to disable asus-wmi. Even with wapf=0, which does not register
> a WLAN LED in asus-wmi, the conflict still occurs because
> ASUS_WMI_DSTS_USER_BIT is set. Please advise on what is the best way to solve
> this problem.

I'm trying to make sense of these two, what is the common ID?

I see asus-wmi.c uses the dev_id ASUS_WMI_DEVID_WLAN_LED 0x0010012. The  ASL you
reference addresses 0x0203000F in OWGD and OWGS, but I don't yet see how they're
the same thing? Is this buried in the guts of the wmi system?

Regardless, it seems an update to asus nb wmi quirks so we don't setup rfkill
with the wmi driver at all. It would be preferable not to have to do it based on
DMI strings. Corentin, any insight you can offer here could be helpful in how to
identify these types of systems so we don't have to enumerate them one at a
time and make asus-wmi and asus-wrc play nice together.

Joao, it would be nice to consolidate on a naming scheme. hp-wireless,
dell-rbtn, *-rfill, etc. One of these should work for asus. Mousou used
asus-wireless which I'd prefer over another new term like wrc.

> 
> I've chosen to make this a separate module because this is a separate entry in
> the DSDT, and checkpatch told me to add an entry in MAINTAINERS in this case.
> Please let me know if any of this should have been done differently.
> 

Are you willing to be the maintainer for this driver? A response to patches to
this list within a week or so is all that's really required. This subsystem
depends on people with the hardware to help keep it it good shape.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver
       [not found]     ` <CA+A7VXWGTAsen2V29ifJ9nGCuHRnQnZi=Q_oEi1fkmeo=t1gxQ@mail.gmail.com>
@ 2015-12-20 16:58       ` João Paulo Rechi Vita
  0 siblings, 0 replies; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-20 16:58 UTC (permalink / raw)
  To: Darren Hart
  Cc: Corentin Chary, platform-driver-x86, acpi4asus-user,
	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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] platform/x86: Add Asus Wireless Radio Control driver
       [not found]     ` <CA+A7VXU9MepWmWL6BS+XyjbLs2s_ONR5Yy3tqT--cbYS7p1P1Q@mail.gmail.com>
@ 2015-12-20 17:00       ` João Paulo Rechi Vita
  0 siblings, 0 replies; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-20 17:00 UTC (permalink / raw)
  To: Darren Hart
  Cc: Mousou Yuu, Corentin Chary, platform-driver-x86, acpi4asus-user,
	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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] asus-wrc: Add ACPI HID ATK4001
  2015-12-19  6:40   ` Darren Hart
@ 2015-12-20 18:57     ` João Paulo Rechi Vita
  0 siblings, 0 replies; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-20 18:57 UTC (permalink / raw)
  To: Darren Hart
  Cc: Corentin Chary, platform-driver-x86, acpi4asus-user,
	João Paulo Rechi Vita, tasev.stefanoska

On 19 December 2015 at 01:40, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Dec 15, 2015 at 10:30:40AM -0500, João Paulo Rechi Vita wrote:
>> 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".
>
> Do you happen to have an ASL dump of this ID? Does it differ in any significant
> way?
>

The DSDT is attached [1] to the bug report. The ASHS node looks
exactly the same as the one I posted in the commit message, but the
LED id is different. I'm not sure if that is the reason it woks fine
there. I'll elaborate more on that topic on the answer to your
comments on the cover letter of this series.

[1] https://bugzilla.kernel.org/attachment.cgi?id=177811

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/4] Asus Wireless Radio Control driver
  2015-12-19  8:00 ` [RFC 0/4] Asus Wireless Radio Control driver Darren Hart
@ 2015-12-20 19:01   ` João Paulo Rechi Vita
  0 siblings, 0 replies; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-20 19:01 UTC (permalink / raw)
  To: Darren Hart
  Cc: Corentin Chary, platform-driver-x86, acpi4asus-user,
	João Paulo Rechi Vita

On 19 December 2015 at 03:00, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Dec 15, 2015 at 10:30:38AM -0500, João Paulo Rechi Vita wrote:
>> This series is a request for comments on the driver for the "Asus Wireless
>> Radio Control" device, as named on the vendor website, which handles
>> notifications from the airplane mode hotkey and drives the airplane mode
>> indicator LED. This device appears in the DSDT as "ASHS".
>>
>> When the airplane mode hotkey is pressed a query 0x0B is started in the
>> Embedded Controller, and all this query does is a notify ASHS with the value
>> 0x88. ASHS also has one method HSWC, which can drive the airplane mode LED and
>> query its status, according to its parameter.
>
> Hi Joao,
>
> Thanks for a detailed introduction, very helpful.
>

Thanks!

>>
>> To properly drive the LED a new RFKill trigger was implemented, to track the
>> global state of all RFKill switches in the system, and turn the LED ON when all
>> are blocked, and OFF otherwise. This code will have to go through review on
>> linux-wireless, but first I wanted to get feedback on the WRC driver itself.
>
> Apologies, I see I jumped the gun on this. I was rushed this afternoon and was
> trying to avoid being the bottleneck. At least we won't have to wait long for
> Johannes to decide after we're done with the review here.
>

Yes, that will be helpful.

>>
>> There is one known issue: the airplane mode LED uses the same ID as the WLAN
>> LED in asus-wmi, so at the moment to have the LED being driven correctly by
>> asus-wrc I have to disable asus-wmi. Even with wapf=0, which does not register
>> a WLAN LED in asus-wmi, the conflict still occurs because
>> ASUS_WMI_DSTS_USER_BIT is set. Please advise on what is the best way to solve
>> this problem.
>
> I'm trying to make sense of these two, what is the common ID?
>
> I see asus-wmi.c uses the dev_id ASUS_WMI_DEVID_WLAN_LED 0x0010012. The  ASL you
> reference addresses 0x0203000F in OWGD and OWGS, but I don't yet see how they're
> the same thing? Is this buried in the guts of the wmi system?
>

Sorry, I meant to also add a link to the DSDT in this letter but
forgot, here it goes:
https://gist.githubusercontent.com/jprvita/7dbd3a6aa1a9ac085b62/raw/03f83b94dbb73c8572012f7ca7621ea3d862fed1/asus-x555ub-dsdt.dsl

Bellow is the relevant part that I think causes the conflict. In the
WMNB method of the wmi device, we have:

                    If (LEqual (IIA0, 0x00010002))
                    {
                        OWGD (IIA1)
                        Return (One)
                    }

Because ASUS_WMI_DSTS_USER_BIT is set, asus-wmi uses
ASUS_WMI_DEVID_WLAN_LED to store the RFKill state, which ends up
calling OWGD with the WLAN state (which is the opposite of the
airplane mode status). At least that's what I could follow, but I
certainly appreciate any input on that.

> Regardless, it seems an update to asus nb wmi quirks so we don't setup rfkill
> with the wmi driver at all. It would be preferable not to have to do it based on
> DMI strings. Corentin, any insight you can offer here could be helpful in how to
> identify these types of systems so we don't have to enumerate them one at a
> time and make asus-wmi and asus-wrc play nice together.
>

I was actually thinking of something like checking for the ASHS _STA
method, or checking for ASHS presence, although I'm not sure if that's
easy or doable at all. A DMI quirk would certainly work, but as you
said, we would have to list all conflicting models.

> Joao, it would be nice to consolidate on a naming scheme. hp-wireless,
> dell-rbtn, *-rfill, etc. One of these should work for asus. Mousou used
> asus-wireless which I'd prefer over another new term like wrc.
>

The name of the Windows driver for this is "Asus Wireless Remote
Control", that's where I took WRC from. But I don't mind changing it,
asus-wireless seems appropriate to me, and unless you have any other
preference, I'll go with that for now.

>>
>> I've chosen to make this a separate module because this is a separate entry in
>> the DSDT, and checkpatch told me to add an entry in MAINTAINERS in this case.
>> Please let me know if any of this should have been done differently.
>>
>
> Are you willing to be the maintainer for this driver? A response to patches to
> this list within a week or so is all that's really required. This subsystem
> depends on people with the hardware to help keep it it good shape.
>

Yes, I am. Also, it seems the company I'm working for [1] will keep
enabling a few Asus laptops in the near future. This means I'll get my
hands on new Asus laptops from time to time, or will be able to ask
for other people on the team to do some tests on them, but it also
means I'll not keep any of these forever. I feel that this will work
fine, but I just wanted to be clear about that.

[1] http://endlessm.com

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger
  2015-12-26 14:56 [RFCv2 " João Paulo Rechi Vita
@ 2015-12-26 14:56 ` João Paulo Rechi Vita
  0 siblings, 0 replies; 17+ messages in thread
From: João Paulo Rechi Vita @ 2015-12-26 14:56 UTC (permalink / raw)
  To: Darren Hart
  Cc: Corentin Chary, platform-driver-x86, acpi4asus-user,
	linux-kernel, 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-12-26 14:57 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-15 15:30 [RFC 0/4] Asus Wireless Radio Control driver João Paulo Rechi Vita
2015-12-15 15:30 ` [PATCH 1/4] platform/x86: Add " João Paulo Rechi Vita
2015-12-19  6:39   ` Darren Hart
     [not found]     ` <CA+A7VXWGTAsen2V29ifJ9nGCuHRnQnZi=Q_oEi1fkmeo=t1gxQ@mail.gmail.com>
2015-12-20 16:58       ` João Paulo Rechi Vita
2015-12-19  7:23   ` Darren Hart
     [not found]     ` <CA+A7VXU9MepWmWL6BS+XyjbLs2s_ONR5Yy3tqT--cbYS7p1P1Q@mail.gmail.com>
2015-12-20 17:00       ` João Paulo Rechi Vita
2015-12-15 15:30 ` [PATCH 2/4] asus-wrc: Add ACPI HID ATK4001 João Paulo Rechi Vita
2015-12-19  6:40   ` Darren Hart
2015-12-20 18:57     ` João Paulo Rechi Vita
2015-12-15 15:30 ` [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger João Paulo Rechi Vita
2015-12-19  0:22   ` Darren Hart
2015-12-19  0:36     ` João Paulo Rechi Vita
2015-12-19  2:25     ` David Miller
2015-12-15 15:30 ` [PATCH 4/4] asus-wrc: Toggle airplane mode LED João Paulo Rechi Vita
2015-12-19  8:00 ` [RFC 0/4] Asus Wireless Radio Control driver Darren Hart
2015-12-20 19:01   ` João Paulo Rechi Vita
2015-12-26 14:56 [RFCv2 " João Paulo Rechi Vita
2015-12-26 14:56 ` [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger João Paulo Rechi Vita

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.