[PATCH v3 0/6] Topstar U931/RVP7 ACPI LED Workaround

[PATCH v3 0/6] Topstar U931/RVP7 ACPI LED Workaround

[Guillaume Douézan-Grard]

Hi Andy,

Thank you for your review. Changes since the previous submission are
highlighted below.

On Topstar U931/RVP7 Notebooks, an issue prevents the WLAN toggle key
from being properly managed by the Embedded Controller and successfully
disconnect the adapter. A specific ACPI method allows to toggle the WLAN
LED state regardless.

Based on a DMI check, the LED is registered within the corresponding
subsystem, making possible to use a software-based trigger (rfkill for
instance, to synchronize the LED with the network interface state).

Thank you for your time,

--
Guillaume Douézan-Grard


Changes since v2:

* revert an older conversion from a generic module to an acpi driver
module instead of undoing it manually,
* separate commits for the renaming and the spliting phases,
* DMI-based check instead of a module parameter,
* copyright and minor fixes put at the end.


Changes since v1:

* leave devices names unchanged for ABI compatibility,
* fix label names,
* separate module authors definition,
* add commit description to Patch 3.


Guillaume Douézan-Grard (6):
  revert "topstar-laptop: convert to module_acpi_driver()"
  platform/x86: topstar-laptop: use consistent naming scheme
  platform/x86: topstar-laptop: split ACPI events and input handling
  platform/x86: topstar-laptop: add platform device
  platform/x86: topstar-laptop: add Topstar U931/RVP7 WLAN LED
    workaround
  platform/x86: topstar-laptop: update copyright and fix some comments

 drivers/platform/x86/Kconfig          |   2 +
 drivers/platform/x86/topstar-laptop.c | 358 +++++++++++++++++++++++++++-------
 2 files changed, 290 insertions(+), 70 deletions(-)

-- 
2.16.1

[PATCH v3 1/6] revert "topstar-laptop: convert to module_acpi_driver()"

[Guillaume Douézan-Grard]

Revert commit 15165594da65 ("topstar-laptop: convert to
module_acpi_driver()") to later add a platform device.

Signed-off-by: Guillaume Douézan-Grard <gdouezangrard@gmail.com>
---
 drivers/platform/x86/topstar-laptop.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/topstar-laptop.c b/drivers/platform/x86/topstar-laptop.c
index 1032c00b907b..b4807b868a69 100644
--- a/drivers/platform/x86/topstar-laptop.c
+++ b/drivers/platform/x86/topstar-laptop.c
@@ -178,7 +178,27 @@ static struct acpi_driver acpi_topstar_driver = {
 		.notify = acpi_topstar_notify,
 	},
 };
-module_acpi_driver(acpi_topstar_driver);
+
+static int __init topstar_laptop_init(void)
+{
+	int ret;
+
+	ret = acpi_bus_register_driver(&acpi_topstar_driver);
+	if (ret < 0)
+		return ret;
+
+	pr_info("ACPI extras driver loaded\n");
+
+	return 0;
+}
+
+static void __exit topstar_laptop_exit(void)
+{
+	acpi_bus_unregister_driver(&acpi_topstar_driver);
+}
+
+module_init(topstar_laptop_init);
+module_exit(topstar_laptop_exit);
 
 MODULE_AUTHOR("Herton Ronaldo Krzesinski");
 MODULE_DESCRIPTION("Topstar Laptop ACPI Extras driver");
-- 
2.16.1

Re: [PATCH v3 1/6] revert "topstar-laptop: convert to module_acpi_driver()"

[Andy Shevchenko]

On Wed, Feb 21, 2018 at 5:12 PM, Guillaume Douézan-Grard
<gdouezangrard@gmail.com> wrote:
> Revert commit 15165594da65 ("topstar-laptop: convert to
> module_acpi_driver()") to later add a platform device.

I would rather reindent this like

Revert commit
  15165594da65 ("topstar-laptop: convert to module_acpi_driver()")
to later add a platform device.

Subject should be
platform/x86: topstar-laptop: Revert "convert to module_acpi_driver()"

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 2/6] platform/x86: topstar-laptop: use consistent naming scheme

[Guillaume Douézan-Grard]

* use module prefix naming scheme for functions and programming
constructs,

* consistent label names

Signed-off-by: Guillaume Douézan-Grard <gdouezangrard@gmail.com>
---
 drivers/platform/x86/topstar-laptop.c | 82 +++++++++++++++++------------------
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/drivers/platform/x86/topstar-laptop.c b/drivers/platform/x86/topstar-laptop.c
index b4807b868a69..17f22a085d9a 100644
--- a/drivers/platform/x86/topstar-laptop.c
+++ b/drivers/platform/x86/topstar-laptop.c
@@ -21,10 +21,10 @@
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
 
-#define ACPI_TOPSTAR_CLASS "topstar"
+#define TOPSTAR_LAPTOP_CLASS "topstar"
 
-struct topstar_hkey {
-	struct input_dev *inputdev;
+struct topstar_laptop {
+	struct input_dev *input;
 };
 
 static const struct key_entry topstar_keymap[] = {
@@ -57,11 +57,11 @@ static const struct key_entry topstar_keymap[] = {
 	{ KE_END, 0 }
 };
 
-static void acpi_topstar_notify(struct acpi_device *device, u32 event)
+static void topstar_acpi_notify(struct acpi_device *device, u32 event)
 {
 	static bool dup_evnt[2];
 	bool *dup;
-	struct topstar_hkey *hkey = acpi_driver_data(device);
+	struct topstar_laptop *topstar = acpi_driver_data(device);
 
 	/* 0x83 and 0x84 key events comes duplicated... */
 	if (event == 0x83 || event == 0x84) {
@@ -73,11 +73,11 @@ static void acpi_topstar_notify(struct acpi_device *device, u32 event)
 		*dup = true;
 	}
 
-	if (!sparse_keymap_report_event(hkey->inputdev, event, 1, true))
+	if (!sparse_keymap_report_event(topstar->input, event, 1, true))
 		pr_info("unknown event = 0x%02x\n", event);
 }
 
-static int acpi_topstar_fncx_switch(struct acpi_device *device, bool state)
+static int topstar_acpi_fncx_switch(struct acpi_device *device, bool state)
 {
 	acpi_status status;
 
@@ -91,72 +91,72 @@ static int acpi_topstar_fncx_switch(struct acpi_device *device, bool state)
 	return 0;
 }
 
-static int acpi_topstar_init_hkey(struct topstar_hkey *hkey)
+static int topstar_input_init(struct topstar_laptop *topstar)
 {
 	struct input_dev *input;
-	int error;
+	int err;
 
 	input = input_allocate_device();
 	if (!input)
 		return -ENOMEM;
 
 	input->name = "Topstar Laptop extra buttons";
-	input->phys = "topstar/input0";
+	input->phys = TOPSTAR_LAPTOP_CLASS "/input0";
 	input->id.bustype = BUS_HOST;
 
-	error = sparse_keymap_setup(input, topstar_keymap, NULL);
-	if (error) {
+	err = sparse_keymap_setup(input, topstar_keymap, NULL);
+	if (err) {
 		pr_err("Unable to setup input device keymap\n");
 		goto err_free_dev;
 	}
 
-	error = input_register_device(input);
-	if (error) {
+	err = input_register_device(input);
+	if (err) {
 		pr_err("Unable to register input device\n");
 		goto err_free_dev;
 	}
 
-	hkey->inputdev = input;
+	topstar->input = input;
 	return 0;
 
- err_free_dev:
+err_free_dev:
 	input_free_device(input);
-	return error;
+	return err;
 }
 
-static int acpi_topstar_add(struct acpi_device *device)
+static int topstar_acpi_add(struct acpi_device *device)
 {
-	struct topstar_hkey *tps_hkey;
+	struct topstar_laptop *topstar;
 
-	tps_hkey = kzalloc(sizeof(struct topstar_hkey), GFP_KERNEL);
-	if (!tps_hkey)
+	topstar = kzalloc(sizeof(struct topstar_laptop), GFP_KERNEL);
+	if (!topstar)
 		return -ENOMEM;
 
 	strcpy(acpi_device_name(device), "Topstar TPSACPI");
-	strcpy(acpi_device_class(device), ACPI_TOPSTAR_CLASS);
+	strcpy(acpi_device_class(device), TOPSTAR_LAPTOP_CLASS);
 
-	if (acpi_topstar_fncx_switch(device, true))
-		goto add_err;
+	if (topstar_acpi_fncx_switch(device, true))
+		goto err_free;
 
-	if (acpi_topstar_init_hkey(tps_hkey))
-		goto add_err;
+	if (topstar_input_init(topstar))
+		goto err_free;
 
-	device->driver_data = tps_hkey;
+	device->driver_data = topstar;
 	return 0;
 
-add_err:
-	kfree(tps_hkey);
+err_free:
+	kfree(topstar);
 	return -ENODEV;
 }
 
-static int acpi_topstar_remove(struct acpi_device *device)
+static int topstar_acpi_remove(struct acpi_device *device)
 {
-	struct topstar_hkey *tps_hkey = acpi_driver_data(device);
+	struct topstar_laptop *topstar = acpi_driver_data(device);
 
-	acpi_topstar_fncx_switch(device, false);
+	topstar_acpi_fncx_switch(device, false);
 
-	input_unregister_device(tps_hkey->inputdev);
-	kfree(tps_hkey);
+	input_unregister_device(topstar->input);
+	kfree(topstar);
 
 	return 0;
 }
@@ -168,14 +168,14 @@ static const struct acpi_device_id topstar_device_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, topstar_device_ids);
 
-static struct acpi_driver acpi_topstar_driver = {
+static struct acpi_driver topstar_acpi_driver = {
 	.name = "Topstar laptop ACPI driver",
-	.class = ACPI_TOPSTAR_CLASS,
+	.class = TOPSTAR_LAPTOP_CLASS,
 	.ids = topstar_device_ids,
 	.ops = {
-		.add = acpi_topstar_add,
-		.remove = acpi_topstar_remove,
-		.notify = acpi_topstar_notify,
+		.add = topstar_acpi_add,
+		.remove = topstar_acpi_remove,
+		.notify = topstar_acpi_notify,
 	},
 };
 
@@ -183,7 +183,7 @@ static int __init topstar_laptop_init(void)
 {
 	int ret;
 
-	ret = acpi_bus_register_driver(&acpi_topstar_driver);
+	ret = acpi_bus_register_driver(&topstar_acpi_driver);
 	if (ret < 0)
 		return ret;
 
@@ -194,7 +194,7 @@ static int __init topstar_laptop_init(void)
 
 static void __exit topstar_laptop_exit(void)
 {
-	acpi_bus_unregister_driver(&acpi_topstar_driver);
+	acpi_bus_unregister_driver(&topstar_acpi_driver);
 }
 
 module_init(topstar_laptop_init);
-- 
2.16.1

[PATCH v3 3/6] platform/x86: topstar-laptop: split ACPI events and input handling

[Guillaume Douézan-Grard]

* get the `acpi_device` from the `topstar_laptop` struct,

* split input registering and `sparse_keymap` events from ACPI events
handling,

* use notify, init and exit functions for ACPI and input handling

Signed-off-by: Guillaume Douézan-Grard <gdouezangrard@gmail.com>
---
 drivers/platform/x86/topstar-laptop.c | 109 ++++++++++++++++++++++------------
 1 file changed, 71 insertions(+), 38 deletions(-)

diff --git a/drivers/platform/x86/topstar-laptop.c b/drivers/platform/x86/topstar-laptop.c
index 17f22a085d9a..fbf020e8a0e1 100644
--- a/drivers/platform/x86/topstar-laptop.c
+++ b/drivers/platform/x86/topstar-laptop.c
@@ -24,9 +24,14 @@
 #define TOPSTAR_LAPTOP_CLASS "topstar"
 
 struct topstar_laptop {
+	struct acpi_device *device;
 	struct input_dev *input;
 };
 
+/*
+ * Input
+ */
+
 static const struct key_entry topstar_keymap[] = {
 	{ KE_KEY, 0x80, { KEY_BRIGHTNESSUP } },
 	{ KE_KEY, 0x81, { KEY_BRIGHTNESSDOWN } },
@@ -57,40 +62,12 @@ static const struct key_entry topstar_keymap[] = {
 	{ KE_END, 0 }
 };
 
-static void topstar_acpi_notify(struct acpi_device *device, u32 event)
+static void topstar_input_notify(struct topstar_laptop *topstar, int event)
 {
-	static bool dup_evnt[2];
-	bool *dup;
-	struct topstar_laptop *topstar = acpi_driver_data(device);
-
-	/* 0x83 and 0x84 key events comes duplicated... */
-	if (event == 0x83 || event == 0x84) {
-		dup = &dup_evnt[event - 0x83];
-		if (*dup) {
-			*dup = false;
-			return;
-		}
-		*dup = true;
-	}
-
 	if (!sparse_keymap_report_event(topstar->input, event, 1, true))
 		pr_info("unknown event = 0x%02x\n", event);
 }
 
-static int topstar_acpi_fncx_switch(struct acpi_device *device, bool state)
-{
-	acpi_status status;
-
-	status = acpi_execute_simple_method(device->handle, "FNCX",
-						state ? 0x86 : 0x87);
-	if (ACPI_FAILURE(status)) {
-		pr_err("Unable to switch FNCX notifications\n");
-		return -ENODEV;
-	}
-
-	return 0;
-}
-
 static int topstar_input_init(struct topstar_laptop *topstar)
 {
 	struct input_dev *input;
@@ -124,9 +101,62 @@ static int topstar_input_init(struct topstar_laptop *topstar)
 	return err;
 }
 
+static void topstar_input_exit(struct topstar_laptop *topstar)
+{
+	input_unregister_device(topstar->input);
+}
+
+/*
+ * ACPI
+ */
+
+static int topstar_acpi_fncx_switch(struct acpi_device *device, bool state)
+{
+	acpi_status status;
+	u64 arg = state ? 0x86 : 0x87;
+
+	status = acpi_execute_simple_method(device->handle, "FNCX", arg);
+	if (ACPI_FAILURE(status)) {
+		pr_err("Unable to switch FNCX notifications\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static void topstar_acpi_notify(struct acpi_device *device, u32 event)
+{
+	struct topstar_laptop *topstar = acpi_driver_data(device);
+	static bool dup_evnt[2];
+	bool *dup;
+
+	/* 0x83 and 0x84 key events comes duplicated... */
+	if (event == 0x83 || event == 0x84) {
+		dup = &dup_evnt[event - 0x83];
+		if (*dup) {
+			*dup = false;
+			return;
+		}
+		*dup = true;
+	}
+
+	topstar_input_notify(topstar, event);
+}
+
+static int topstar_acpi_init(struct topstar_laptop *topstar)
+{
+	return topstar_acpi_fncx_switch(topstar->device, true);
+}
+
+static void topstar_acpi_exit(struct topstar_laptop *topstar)
+{
+	topstar_acpi_fncx_switch(topstar->device, false);
+}
+
 static int topstar_acpi_add(struct acpi_device *device)
 {
 	struct topstar_laptop *topstar;
+	int err;
 
 	topstar = kzalloc(sizeof(struct topstar_laptop), GFP_KERNEL);
 	if (!topstar)
@@ -134,30 +164,34 @@ static int topstar_acpi_add(struct acpi_device *device)
 
 	strcpy(acpi_device_name(device), "Topstar TPSACPI");
 	strcpy(acpi_device_class(device), TOPSTAR_LAPTOP_CLASS);
+	device->driver_data = topstar;
+	topstar->device = device;
 
-	if (topstar_acpi_fncx_switch(device, true))
+	err = topstar_acpi_init(topstar);
+	if (err)
 		goto err_free;
 
-	if (topstar_input_init(topstar))
-		goto err_free;
+	err = topstar_input_init(topstar);
+	if (err)
+		goto err_acpi_exit;
 
-	device->driver_data = topstar;
 	return 0;
 
+err_acpi_exit:
+	topstar_acpi_exit(topstar);
 err_free:
 	kfree(topstar);
-	return -ENODEV;
+	return err;
 }
 
 static int topstar_acpi_remove(struct acpi_device *device)
 {
 	struct topstar_laptop *topstar = acpi_driver_data(device);
 
-	topstar_acpi_fncx_switch(device, false);
+	topstar_input_exit(topstar);
+	topstar_acpi_exit(topstar);
 
-	input_unregister_device(topstar->input);
 	kfree(topstar);
-
 	return 0;
 }
 
@@ -188,7 +222,6 @@ static int __init topstar_laptop_init(void)
 		return ret;
 
 	pr_info("ACPI extras driver loaded\n");
-
 	return 0;
 }
 
-- 
2.16.1

[PATCH v3 4/6] platform/x86: topstar-laptop: add platform device

[Guillaume Douézan-Grard]

* add a platform device to support further addition of a led subsystem,

* register the existing input device to this platform device

Signed-off-by: Guillaume Douézan-Grard <gdouezangrard@gmail.com>
---
 drivers/platform/x86/topstar-laptop.c | 59 +++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/topstar-laptop.c b/drivers/platform/x86/topstar-laptop.c
index fbf020e8a0e1..792c90d62724 100644
--- a/drivers/platform/x86/topstar-laptop.c
+++ b/drivers/platform/x86/topstar-laptop.c
@@ -20,11 +20,13 @@
 #include <linux/acpi.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
+#include <linux/platform_device.h>
 
 #define TOPSTAR_LAPTOP_CLASS "topstar"
 
 struct topstar_laptop {
 	struct acpi_device *device;
+	struct platform_device *platform;
 	struct input_dev *input;
 };
 
@@ -80,6 +82,7 @@ static int topstar_input_init(struct topstar_laptop *topstar)
 	input->name = "Topstar Laptop extra buttons";
 	input->phys = TOPSTAR_LAPTOP_CLASS "/input0";
 	input->id.bustype = BUS_HOST;
+	input->dev.parent = &topstar->platform->dev;
 
 	err = sparse_keymap_setup(input, topstar_keymap, NULL);
 	if (err) {
@@ -106,6 +109,42 @@ static void topstar_input_exit(struct topstar_laptop *topstar)
 	input_unregister_device(topstar->input);
 }
 
+/*
+ * Platform
+ */
+
+static struct platform_driver topstar_platform_driver = {
+	.driver = {
+		.name = TOPSTAR_LAPTOP_CLASS,
+	},
+};
+
+static int topstar_platform_init(struct topstar_laptop *topstar)
+{
+	int err;
+
+	topstar->platform = platform_device_alloc(TOPSTAR_LAPTOP_CLASS, -1);
+	if (!topstar->platform)
+		return -ENOMEM;
+
+	platform_set_drvdata(topstar->platform, topstar);
+
+	err = platform_device_add(topstar->platform);
+	if (err)
+		goto err_device_put;
+
+	return 0;
+
+err_device_put:
+	platform_device_put(topstar->platform);
+	return err;
+}
+
+static void topstar_platform_exit(struct topstar_laptop *topstar)
+{
+	platform_device_unregister(topstar->platform);
+}
+
 /*
  * ACPI
  */
@@ -171,12 +210,18 @@ static int topstar_acpi_add(struct acpi_device *device)
 	if (err)
 		goto err_free;
 
-	err = topstar_input_init(topstar);
+	err = topstar_platform_init(topstar);
 	if (err)
 		goto err_acpi_exit;
 
+	err = topstar_input_init(topstar);
+	if (err)
+		goto err_platform_exit;
+
 	return 0;
 
+err_platform_exit:
+	topstar_platform_exit(topstar);
 err_acpi_exit:
 	topstar_acpi_exit(topstar);
 err_free:
@@ -189,6 +234,7 @@ static int topstar_acpi_remove(struct acpi_device *device)
 	struct topstar_laptop *topstar = acpi_driver_data(device);
 
 	topstar_input_exit(topstar);
+	topstar_platform_exit(topstar);
 	topstar_acpi_exit(topstar);
 
 	kfree(topstar);
@@ -217,17 +263,26 @@ static int __init topstar_laptop_init(void)
 {
 	int ret;
 
-	ret = acpi_bus_register_driver(&topstar_acpi_driver);
+	ret = platform_driver_register(&topstar_platform_driver);
 	if (ret < 0)
 		return ret;
 
+	ret = acpi_bus_register_driver(&topstar_acpi_driver);
+	if (ret < 0)
+		goto err_driver_unreg;
+
 	pr_info("ACPI extras driver loaded\n");
 	return 0;
+
+err_driver_unreg:
+	platform_driver_unregister(&topstar_platform_driver);
+	return ret;
 }
 
 static void __exit topstar_laptop_exit(void)
 {
 	acpi_bus_unregister_driver(&topstar_acpi_driver);
+	platform_driver_unregister(&topstar_platform_driver);
 }
 
 module_init(topstar_laptop_init);
-- 
2.16.1

[PATCH v3 5/6] platform/x86: topstar-laptop: add Topstar U931/RVP7 WLAN LED workaround

[Guillaume Douézan-Grard]

Topstar U931 laptops provide a LED synced with the WLAN adapter
hard-blocking state. Unfortunately, some models seem to be defective,
making impossible to hard-block the adapter with the WLAN switch and
thus the LED is useless.

An ACPI method is available to programmatically control this switch and
it indirectly allows to control the LED.

This commit registers the LED within the corresponding subsystem, making
possible for instance to use an rfkill-based trigger to synchronize the
LED with the device state.

This workaround is enabled automatically for Topstar U931/RVP7 laptops
based on a DMI check.

Signed-off-by: Guillaume Douézan-Grard <gdouezangrard@gmail.com>
---
 drivers/platform/x86/Kconfig          |   2 +
 drivers/platform/x86/topstar-laptop.c | 108 ++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 9a8f96465cdc..bc66a31757db 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -742,6 +742,8 @@ config TOPSTAR_LAPTOP
 	depends on ACPI
 	depends on INPUT
 	select INPUT_SPARSEKMAP
+	select LEDS_CLASS
+	select NEW_LEDS
 	---help---
 	  This driver adds support for hotkeys found on Topstar laptops.
 
diff --git a/drivers/platform/x86/topstar-laptop.c b/drivers/platform/x86/topstar-laptop.c
index 792c90d62724..bf9761092c7e 100644
--- a/drivers/platform/x86/topstar-laptop.c
+++ b/drivers/platform/x86/topstar-laptop.c
@@ -18,8 +18,10 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
+#include <linux/dmi.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
+#include <linux/leds.h>
 #include <linux/platform_device.h>
 
 #define TOPSTAR_LAPTOP_CLASS "topstar"
@@ -28,8 +30,77 @@ struct topstar_laptop {
 	struct acpi_device *device;
 	struct platform_device *platform;
 	struct input_dev *input;
+	struct led_classdev led;
 };
 
+/*
+ * LED
+ */
+
+static enum led_brightness topstar_led_get(struct led_classdev *led)
+{
+	return led->brightness;
+}
+
+static int topstar_led_set(struct led_classdev *led,
+		enum led_brightness state)
+{
+	struct topstar_laptop *topstar = container_of(led,
+			struct topstar_laptop, led);
+
+	struct acpi_object_list params;
+	union acpi_object in_obj;
+	unsigned long long int ret;
+	acpi_status status;
+
+	params.count = 1;
+	params.pointer = &in_obj;
+	in_obj.type = ACPI_TYPE_INTEGER;
+	in_obj.integer.value = 0x83;
+
+	/*
+	 * Topstar ACPI returns 0x30001 when the LED is ON and 0x30000 when it
+	 * is OFF.
+	 */
+	status = acpi_evaluate_integer(topstar->device->handle,
+			"GETX", &params, &ret);
+	if (ACPI_FAILURE(status))
+		return -1;
+
+	/*
+	 * FNCX(0x83) toggles the LED (more precisely, it is supposed to
+	 * act as an hardware switch and disconnect the WLAN adapter but
+	 * it seems to be faulty on some models like the Topstar U931
+	 * Notebook).
+	 */
+	if ((ret == 0x30001 && state == LED_OFF)
+			|| (ret == 0x30000 && state != LED_OFF)) {
+		status = acpi_execute_simple_method(topstar->device->handle,
+				"FNCX", 0x83);
+		if (ACPI_FAILURE(status))
+			return -1;
+	}
+
+	return 0;
+}
+
+static int topstar_led_init(struct topstar_laptop *topstar)
+{
+	topstar->led = (struct led_classdev) {
+		.default_trigger = "rfkill0",
+		.brightness_get = topstar_led_get,
+		.brightness_set_blocking = topstar_led_set,
+		.name = TOPSTAR_LAPTOP_CLASS "::wlan",
+	};
+
+	return led_classdev_register(&topstar->platform->dev, &topstar->led);
+}
+
+static void topstar_led_exit(struct topstar_laptop *topstar)
+{
+	led_classdev_unregister(&topstar->led);
+}
+
 /*
  * Input
  */
@@ -192,11 +263,37 @@ static void topstar_acpi_exit(struct topstar_laptop *topstar)
 	topstar_acpi_fncx_switch(topstar->device, false);
 }
 
+/*
+ * Enable software-based WLAN LED control on systems with defective
+ * hardware switch.
+ */
+static bool led_workaround;
+
+static int dmi_led_workaround(const struct dmi_system_id *id)
+{
+	led_workaround = true;
+	return 0;
+}
+
+static const struct dmi_system_id topstar_dmi_ids[] = {
+	{
+		.callback = dmi_led_workaround,
+		.ident = "Topstar U931/RVP7",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "U931"),
+			DMI_MATCH(DMI_BOARD_VERSION, "RVP7"),
+		},
+	},
+	{}
+};
+
 static int topstar_acpi_add(struct acpi_device *device)
 {
 	struct topstar_laptop *topstar;
 	int err;
 
+	dmi_check_system(topstar_dmi_ids);
+
 	topstar = kzalloc(sizeof(struct topstar_laptop), GFP_KERNEL);
 	if (!topstar)
 		return -ENOMEM;
@@ -218,8 +315,16 @@ static int topstar_acpi_add(struct acpi_device *device)
 	if (err)
 		goto err_platform_exit;
 
+	if (led_workaround) {
+		err = topstar_led_init(topstar);
+		if (err)
+			goto err_input_exit;
+	}
+
 	return 0;
 
+err_input_exit:
+	topstar_input_exit(topstar);
 err_platform_exit:
 	topstar_platform_exit(topstar);
 err_acpi_exit:
@@ -233,6 +338,9 @@ static int topstar_acpi_remove(struct acpi_device *device)
 {
 	struct topstar_laptop *topstar = acpi_driver_data(device);
 
+	if (led_workaround)
+		topstar_led_exit(topstar);
+
 	topstar_input_exit(topstar);
 	topstar_platform_exit(topstar);
 	topstar_acpi_exit(topstar);
-- 
2.16.1

[PATCH v3 6/6] platform/x86: topstar-laptop: update copyright and fix some comments

[Guillaume Douézan-Grard]

Signed-off-by: Guillaume Douézan-Grard <gdouezangrard@gmail.com>
---
 drivers/platform/x86/topstar-laptop.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/topstar-laptop.c b/drivers/platform/x86/topstar-laptop.c
index bf9761092c7e..294729c98267 100644
--- a/drivers/platform/x86/topstar-laptop.c
+++ b/drivers/platform/x86/topstar-laptop.c
@@ -1,10 +1,11 @@
 /*
- * ACPI driver for Topstar notebooks (hotkeys support only)
+ * Topstar Laptop ACPI Extras driver
  *
  * Copyright (c) 2009 Herton Ronaldo Krzesinski <herton@mandriva.com.br>
+ * Copyright (c) 2018 Guillaume Douézan-Grard
  *
  * Implementation inspired by existing x86 platform drivers, in special
- * asus/eepc/fujitsu-laptop, thanks to their authors
+ * asus/eepc/fujitsu-laptop, thanks to their authors.
  *
  * 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
@@ -397,5 +398,6 @@ module_init(topstar_laptop_init);
 module_exit(topstar_laptop_exit);
 
 MODULE_AUTHOR("Herton Ronaldo Krzesinski");
+MODULE_AUTHOR("Guillaume Douézan-Grard");
 MODULE_DESCRIPTION("Topstar Laptop ACPI Extras driver");
 MODULE_LICENSE("GPL");
-- 
2.16.1

Re: [PATCH v3 6/6] platform/x86: topstar-laptop: update copyright and fix some comments

[Andy Shevchenko]

On Wed, Feb 21, 2018 at 5:13 PM, Guillaume Douézan-Grard
<gdouezangrard@gmail.com> wrote:

Commit message?

AFAICS,
"Add myself as an author of the driver."

> Signed-off-by: Guillaume Douézan-Grard <gdouezangrard@gmail.com>

>  /*
> - * ACPI driver for Topstar notebooks (hotkeys support only)
> + * Topstar Laptop ACPI Extras driver

This should go to the patch which does the actual extension.


>   * Copyright (c) 2009 Herton Ronaldo Krzesinski <herton@mandriva.com.br>
> + * Copyright (c) 2018 Guillaume Douézan-Grard
>   *
>   * Implementation inspired by existing x86 platform drivers, in special
> - * asus/eepc/fujitsu-laptop, thanks to their authors
> + * asus/eepc/fujitsu-laptop, thanks to their authors.
>   *

>   * 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

If you want, you can consider to do another one to switch to SDPX identifier.

>  MODULE_AUTHOR("Herton Ronaldo Krzesinski");
> +MODULE_AUTHOR("Guillaume Douézan-Grard");

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/6] Topstar U931/RVP7 ACPI LED Workaround

[Andy Shevchenko]

On Wed, Feb 21, 2018 at 5:12 PM, Guillaume Douézan-Grard
<gdouezangrard@gmail.com> wrote:
> Hi Andy,
>
> Thank you for your review. Changes since the previous submission are
> highlighted below.
>
> On Topstar U931/RVP7 Notebooks, an issue prevents the WLAN toggle key
> from being properly managed by the Embedded Controller and successfully
> disconnect the adapter. A specific ACPI method allows to toggle the WLAN
> LED state regardless.
>
> Based on a DMI check, the LED is registered within the corresponding
> subsystem, making possible to use a software-based trigger (rfkill for
> instance, to synchronize the LED with the network interface state).
>
> Thank you for your time,

Couple of style things. otherwise looks good to me.

>
> --
> Guillaume Douézan-Grard
>
>
> Changes since v2:
>
> * revert an older conversion from a generic module to an acpi driver
> module instead of undoing it manually,
> * separate commits for the renaming and the spliting phases,
> * DMI-based check instead of a module parameter,
> * copyright and minor fixes put at the end.
>
>
> Changes since v1:
>
> * leave devices names unchanged for ABI compatibility,
> * fix label names,
> * separate module authors definition,
> * add commit description to Patch 3.
>
>
> Guillaume Douézan-Grard (6):
>   revert "topstar-laptop: convert to module_acpi_driver()"
>   platform/x86: topstar-laptop: use consistent naming scheme
>   platform/x86: topstar-laptop: split ACPI events and input handling
>   platform/x86: topstar-laptop: add platform device
>   platform/x86: topstar-laptop: add Topstar U931/RVP7 WLAN LED
>     workaround
>   platform/x86: topstar-laptop: update copyright and fix some comments
>
>  drivers/platform/x86/Kconfig          |   2 +
>  drivers/platform/x86/topstar-laptop.c | 358 +++++++++++++++++++++++++++-------
>  2 files changed, 290 insertions(+), 70 deletions(-)
>
> --
> 2.16.1
>



-- 
With Best Regards,
Andy Shevchenko