[PATCH 1/5] platform/x86: wmi: Replace read_takes_no_args with a flags field

[PATCH 1/5] platform/x86: wmi: Replace read_takes_no_args with a flags field

[Hans de Goede]

Replace the wmi_block.read_takes_no_args bool field with
an unsigned long flags field, used together with test_bit()
and friends.

This is a preparation patch for fixing a driver->notify() vs ->probe()
race, which requires atomic flag handling.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/wmi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index c34341f4da76..46178e03aeca 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -57,6 +57,10 @@ static_assert(sizeof(typeof_member(struct guid_block, guid)) == 16);
 static_assert(sizeof(struct guid_block) == 20);
 static_assert(__alignof__(struct guid_block) == 1);
 
+enum {	/* wmi_block flags */
+	WMI_READ_TAKES_NO_ARGS,
+};
+
 struct wmi_block {
 	struct wmi_device dev;
 	struct list_head list;
@@ -67,8 +71,7 @@ struct wmi_block {
 	wmi_notify_handler handler;
 	void *handler_data;
 	u64 req_buf_size;
-
-	bool read_takes_no_args;
+	unsigned long flags;
 };
 
 
@@ -367,7 +370,7 @@ static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
 	wq_params[0].type = ACPI_TYPE_INTEGER;
 	wq_params[0].integer.value = instance;
 
-	if (instance == 0 && wblock->read_takes_no_args)
+	if (instance == 0 && test_bit(WMI_READ_TAKES_NO_ARGS, &wblock->flags))
 		input.count = 0;
 
 	/*
@@ -1113,7 +1116,7 @@ static int wmi_create_device(struct device *wmi_bus_dev,
 	 * laptops, WQxx may not be a method at all.)
 	 */
 	if (info->type != ACPI_TYPE_METHOD || info->param_count == 0)
-		wblock->read_takes_no_args = true;
+		set_bit(WMI_READ_TAKES_NO_ARGS, &wblock->flags);
 
 	kfree(info);
 
-- 
2.33.1

[PATCH 2/5] platform/x86: wmi: Fix driver->notify() vs ->probe() race

[Hans de Goede]

The driver core sets struct device->driver before calling out
to the bus' probe() method, this leaves a window where an ACPI
notify may happen on the WMI object before the driver's
probe() method has completed running, causing e.g. the
driver's notify() callback to get called with drvdata
not yet being set leading to a NULL pointer deref.

At a check for this to the WMI core, ensuring that the notify()
callback is not called before the driver is ready.

Fixes: 1686f5444546 ("platform/x86: wmi: Incorporate acpi_install_notify_handler")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/wmi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 46178e03aeca..02aba274c4bc 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -59,6 +59,7 @@ static_assert(__alignof__(struct guid_block) == 1);
 
 enum {	/* wmi_block flags */
 	WMI_READ_TAKES_NO_ARGS,
+	WMI_PROBED,
 };
 
 struct wmi_block {
@@ -1008,6 +1009,7 @@ static int wmi_dev_probe(struct device *dev)
 		}
 	}
 
+	set_bit(WMI_PROBED, &wblock->flags);
 	return 0;
 
 probe_misc_failure:
@@ -1025,6 +1027,8 @@ static void wmi_dev_remove(struct device *dev)
 	struct wmi_block *wblock = dev_to_wblock(dev);
 	struct wmi_driver *wdriver = drv_to_wdrv(dev->driver);
 
+	clear_bit(WMI_PROBED, &wblock->flags);
+
 	if (wdriver->filter_callback) {
 		misc_deregister(&wblock->char_dev);
 		kfree(wblock->char_dev.name);
@@ -1322,7 +1326,7 @@ static void acpi_wmi_notify_handler(acpi_handle handle, u32 event,
 		return;
 
 	/* If a driver is bound, then notify the driver. */
-	if (wblock->dev.dev.driver) {
+	if (test_bit(WMI_PROBED, &wblock->flags) && wblock->dev.dev.driver) {
 		struct wmi_driver *driver = drv_to_wdrv(wblock->dev.dev.driver);
 		struct acpi_buffer evdata = { ACPI_ALLOCATE_BUFFER, NULL };
 		acpi_status status;
-- 
2.33.1

[PATCH 3/5] platform/x86: wmi: Add no_notify_data flag to struct wmi_driver

[Hans de Goede]

Some WMI implementations do notifies on WMI objects without a _WED method
allow WMI drivers to indicate that _WED should not be called for notifies
on the WMI objects the driver is bound to.

Instead the driver's notify callback will simply be called with a NULL
data argument.

Reported-by: Yauhen Kharuzhy <jekhor@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/wmi.c | 10 ++++++----
 include/linux/wmi.h        |  1 +
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 02aba274c4bc..58a23a9adbef 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -1331,10 +1331,12 @@ static void acpi_wmi_notify_handler(acpi_handle handle, u32 event,
 		struct acpi_buffer evdata = { ACPI_ALLOCATE_BUFFER, NULL };
 		acpi_status status;
 
-		status = get_event_data(wblock, &evdata);
-		if (ACPI_FAILURE(status)) {
-			dev_warn(&wblock->dev.dev, "failed to get event data\n");
-			return;
+		if (!driver->no_notify_data) {
+			status = get_event_data(wblock, &evdata);
+			if (ACPI_FAILURE(status)) {
+				dev_warn(&wblock->dev.dev, "failed to get event data\n");
+				return;
+			}
 		}
 
 		if (driver->notify)
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index 2cb3913c1f50..b88d7b58e61e 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -35,6 +35,7 @@ extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
 struct wmi_driver {
 	struct device_driver driver;
 	const struct wmi_device_id *id_table;
+	bool no_notify_data;
 
 	int (*probe)(struct wmi_device *wdev, const void *context);
 	void (*remove)(struct wmi_device *wdev);
-- 
2.33.1

[PATCH 4/5] platform/x86: lenovo-yogabook-wmi: Add driver for Lenovo Yoga Book

[Hans de Goede]

From: Yauhen Kharuzhy <jekhor@gmail.com>

Add driver to handle WMI events, control the keyboard backlight and
bind/unbind the keyboard-touch / digitizer driver so that only one
is active at a time.

It may seem a bit weird to handle the toggling of the modes in the
kernel, but the hw actually expects only 1 device to be active
at a time.

Changes by Hans de Goede:
- Whole bunch of cleanups
- Make the kernel do the driver bind/unbind itself instead of
  sending events to userspace and requiring a special userspace
  daemon to deal with this

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
Co-developed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/Kconfig               |  13 +
 drivers/platform/x86/Makefile              |   1 +
 drivers/platform/x86/lenovo-yogabook-wmi.c | 339 +++++++++++++++++++++
 3 files changed, 353 insertions(+)
 create mode 100644 drivers/platform/x86/lenovo-yogabook-wmi.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index ad1c45682f0e..a1dd5e0676d1 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -127,6 +127,19 @@ config GIGABYTE_WMI
 	  To compile this driver as a module, choose M here: the module will
 	  be called gigabyte-wmi.
 
+config YOGABOOK_WMI
+	tristate "Lenovo Yoga Book tablet WMI key driver"
+	depends on ACPI_WMI
+	depends on INPUT
+	select LEDS_CLASS
+	select NEW_LEDS
+	help
+	  Say Y here if you want to support the 'Pen' key and keyboard backlight
+	  control on the Lenovo Yoga Book tablets.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called lenovo-yogabook-wmi.
+
 config ACERHDF
 	tristate "Acer Aspire One temperature and fan driver"
 	depends on ACPI && THERMAL
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index d6bd2ff5f76d..30047df8f701 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT)	+= nvidia-wmi-ec-backlight.o
 obj-$(CONFIG_PEAQ_WMI)			+= peaq-wmi.o
 obj-$(CONFIG_XIAOMI_WMI)		+= xiaomi-wmi.o
 obj-$(CONFIG_GIGABYTE_WMI)		+= gigabyte-wmi.o
+obj-$(CONFIG_YOGABOOK_WMI)		+= lenovo-yogabook-wmi.o
 
 # Acer
 obj-$(CONFIG_ACERHDF)		+= acerhdf.o
diff --git a/drivers/platform/x86/lenovo-yogabook-wmi.c b/drivers/platform/x86/lenovo-yogabook-wmi.c
new file mode 100644
index 000000000000..ecd624d9108f
--- /dev/null
+++ b/drivers/platform/x86/lenovo-yogabook-wmi.c
@@ -0,0 +1,339 @@
+// SPDX-License-Identifier: GPL-2.0
+/* WMI driver for Lenovo Yoga Book YB1-X90* / -X91* tablets */
+
+#include <linux/acpi.h>
+#include <linux/devm-helpers.h>
+#include <linux/module.h>
+#include <linux/leds.h>
+#include <linux/wmi.h>
+#include <linux/workqueue.h>
+
+#define YB_MBTN_EVENT_GUID	"243FEC1D-1963-41C1-8100-06A9D82A94B4"
+#define YB_MBTN_METHOD_GUID	"742B0CA1-0B20-404B-9CAA-AEFCABF30CE0"
+
+#define YB_PAD_ENABLE	1
+#define YB_PAD_DISABLE	2
+#define YB_LIGHTUP_BTN	3
+
+#define YB_KBD_BL_DEFAULT 128
+
+/* flags */
+enum {
+	YB_KBD_IS_ON,
+	YB_DIGITIZER_IS_ON,
+	YB_DIGITIZER_MODE,
+	YB_SUSPENDED,
+};
+
+struct yogabook_wmi {
+	struct wmi_device *wdev;
+	struct acpi_device *kbd_adev;
+	struct acpi_device *dig_adev;
+	struct device *kbd_dev;
+	struct device *dig_dev;
+	struct work_struct work;
+	struct led_classdev kbd_bl_led;
+	unsigned long flags;
+	uint8_t brightness;
+};
+
+static int yogabook_wmi_do_action(struct wmi_device *wdev, int action)
+{
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_buffer input;
+	acpi_status status;
+	u32 dummy_arg = 0;
+
+	dev_dbg(&wdev->dev, "Do action: %d\n", action);
+
+	input.pointer = &dummy_arg;
+	input.length = sizeof(dummy_arg);
+
+	status = wmi_evaluate_method(YB_MBTN_METHOD_GUID, 0, action, &input,
+				     &output);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&wdev->dev, "Calling WMI method failure: 0x%x\n",
+			status);
+		return status;
+	}
+
+	kfree(output.pointer);
+
+	return 0;
+}
+
+/*
+ * To control keyboard backlight, call the method KBLC() of the TCS1 ACPI
+ * device (Goodix touchpad acts as virtual sensor keyboard).
+ */
+static int yogabook_wmi_set_kbd_backlight(struct wmi_device *wdev,
+					  uint8_t level)
+{
+	struct yogabook_wmi *data = dev_get_drvdata(&wdev->dev);
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_object_list input;
+	union acpi_object param;
+	acpi_status status;
+
+	if (data->kbd_adev->power.state != ACPI_STATE_D0) {
+		dev_warn(&wdev->dev, "keyboard touchscreen not in D0, cannot set brightness\n");
+		return -ENXIO;
+	}
+
+	dev_dbg(&wdev->dev, "Set KBLC level to %u\n", level);
+
+	input.count = 1;
+	input.pointer = &param;
+
+	param.type = ACPI_TYPE_INTEGER;
+	param.integer.value = 255 - level;
+
+	status = acpi_evaluate_object(acpi_device_handle(data->kbd_adev), "KBLC",
+				      &input, &output);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&wdev->dev, "Failed to call KBLC method: 0x%x\n", status);
+		return status;
+	}
+
+	kfree(output.pointer);
+	return 0;
+}
+
+static void yogabook_wmi_work(struct work_struct *work)
+{
+	struct yogabook_wmi *data = container_of(work, struct yogabook_wmi, work);
+	struct device *dev = &data->wdev->dev;
+	bool kbd_on, digitizer_on;
+	int r;
+
+	if (test_bit(YB_SUSPENDED, &data->flags))
+		return;
+
+	if (test_bit(YB_DIGITIZER_MODE, &data->flags)) {
+		digitizer_on = true;
+		kbd_on = false;
+	} else {
+		kbd_on = true;
+		digitizer_on = false;
+	}
+
+	if (!kbd_on && test_bit(YB_KBD_IS_ON, &data->flags)) {
+		/*
+		 * Must be done before releasing the keyboard touchscreen driver,
+		 * so that the keyboard touchscreen dev is still in D0.
+		 */
+		yogabook_wmi_set_kbd_backlight(data->wdev, 0);
+		device_release_driver(data->kbd_dev);
+		clear_bit(YB_KBD_IS_ON, &data->flags);
+	}
+
+	if (!digitizer_on && test_bit(YB_DIGITIZER_IS_ON, &data->flags)) {
+		yogabook_wmi_do_action(data->wdev, YB_PAD_DISABLE);
+		device_release_driver(data->dig_dev);
+		clear_bit(YB_DIGITIZER_IS_ON, &data->flags);
+	}
+
+	if (kbd_on && !test_bit(YB_KBD_IS_ON, &data->flags)) {
+		r = device_reprobe(data->kbd_dev);
+		if (r)
+			dev_warn(dev, "Reprobe of keyboard touchscreen failed: %d\n", r);
+
+		yogabook_wmi_set_kbd_backlight(data->wdev, data->brightness);
+		set_bit(YB_KBD_IS_ON, &data->flags);
+	}
+
+	if (digitizer_on && !test_bit(YB_DIGITIZER_IS_ON, &data->flags)) {
+		r = device_reprobe(data->dig_dev);
+		if (r)
+			dev_warn(dev, "Reprobe of digitizer failed: %d\n", r);
+
+		yogabook_wmi_do_action(data->wdev, YB_PAD_ENABLE);
+		set_bit(YB_DIGITIZER_IS_ON, &data->flags);
+	}
+}
+
+static void yogabook_wmi_notify(struct wmi_device *wdev, union acpi_object *dummy)
+{
+	struct yogabook_wmi *data = dev_get_drvdata(&wdev->dev);
+
+	if (test_bit(YB_SUSPENDED, &data->flags))
+		return;
+
+	if (test_bit(YB_DIGITIZER_MODE, &data->flags))
+		clear_bit(YB_DIGITIZER_MODE, &data->flags);
+	else
+		set_bit(YB_DIGITIZER_MODE, &data->flags);
+
+	/*
+	 * We are called from the ACPI core and the driver [un]binding which is
+	 * done also needs ACPI functions, use a workqueue to avoid deadlocking.
+	 */
+	schedule_work(&data->work);
+}
+
+static enum led_brightness kbd_brightness_get(struct led_classdev *cdev)
+{
+	struct yogabook_wmi *data =
+		container_of(cdev, struct yogabook_wmi, kbd_bl_led);
+
+	return data->brightness;
+}
+
+static int kbd_brightness_set(struct led_classdev *cdev,
+			      enum led_brightness value)
+{
+	struct yogabook_wmi *data =
+		container_of(cdev, struct yogabook_wmi, kbd_bl_led);
+	struct wmi_device *wdev = data->wdev;
+
+	if ((value < 0) || (value > 255))
+		return -EINVAL;
+
+	data->brightness = value;
+
+	if (data->kbd_adev->power.state != ACPI_STATE_D0)
+		return 0;
+
+	return yogabook_wmi_set_kbd_backlight(wdev, data->brightness);
+}
+
+static int yogabook_wmi_probe(struct wmi_device *wdev, const void *context)
+{
+	struct yogabook_wmi *data;
+	int r;
+
+	data = devm_kzalloc(&wdev->dev, sizeof(struct yogabook_wmi), GFP_KERNEL);
+	if (data == NULL)
+		return -ENOMEM;
+
+	dev_set_drvdata(&wdev->dev, data);
+
+	data->wdev = wdev;
+	data->brightness = YB_KBD_BL_DEFAULT;
+	set_bit(YB_KBD_IS_ON, &data->flags);
+	set_bit(YB_DIGITIZER_IS_ON, &data->flags);
+
+	r = devm_work_autocancel(&wdev->dev, &data->work, yogabook_wmi_work);
+	if (r)
+		return r;
+
+	data->kbd_adev = acpi_dev_get_first_match_dev("GDIX1001", NULL, -1);
+	if (!data->kbd_adev) {
+		dev_err(&wdev->dev, "Cannot find the touchpad device in ACPI tables\n");
+		return -ENODEV;
+	}
+
+	data->dig_adev = acpi_dev_get_first_match_dev("WCOM0019", NULL, -1);
+	if (!data->dig_adev) {
+		dev_err(&wdev->dev, "Cannot find the digitizer device in ACPI tables\n");
+		r = -ENODEV;
+		goto error_put_devs;
+	}
+
+	data->kbd_dev = get_device(acpi_get_first_physical_node(data->kbd_adev));
+	if (!data->kbd_dev || !data->kbd_dev->driver) {
+		r = -EPROBE_DEFER;
+		goto error_put_devs;
+	}
+
+	data->dig_dev = get_device(acpi_get_first_physical_node(data->dig_adev));
+	if (!data->dig_dev || !data->dig_dev->driver) {
+		r = -EPROBE_DEFER;
+		goto error_put_devs;
+	}
+
+	schedule_work(&data->work);
+
+	data->kbd_bl_led.name = "ybwmi::kbd_backlight";
+	data->kbd_bl_led.brightness_set_blocking = kbd_brightness_set;
+	data->kbd_bl_led.brightness_get = kbd_brightness_get;
+	data->kbd_bl_led.max_brightness = 255;
+
+	r = devm_led_classdev_register(&wdev->dev, &data->kbd_bl_led);
+	if (r < 0) {
+		dev_err_probe(&wdev->dev, r, "Registering backlight LED device\n");
+		goto error_put_devs;
+	}
+
+	return 0;
+
+error_put_devs:
+	put_device(data->dig_dev);
+	put_device(data->kbd_dev);
+	acpi_dev_put(data->dig_adev);
+	acpi_dev_put(data->kbd_adev);
+	return r;
+}
+
+static void yogabook_wmi_remove(struct wmi_device *wdev)
+{
+	struct yogabook_wmi *data = dev_get_drvdata(&wdev->dev);
+
+	put_device(data->dig_dev);
+	put_device(data->kbd_dev);
+	acpi_dev_put(data->dig_adev);
+	acpi_dev_put(data->kbd_adev);
+}
+
+__maybe_unused int yogabook_wmi_suspend(struct device *dev)
+{
+	struct wmi_device *wdev = container_of(dev, struct wmi_device, dev);
+	struct yogabook_wmi *data = dev_get_drvdata(dev);
+
+	set_bit(YB_SUSPENDED, &data->flags);
+
+	flush_work(&data->work);
+
+	/* Turn off the pen button at sleep */
+	if (test_bit(YB_DIGITIZER_IS_ON, &data->flags))
+		yogabook_wmi_do_action(wdev, YB_PAD_DISABLE);
+
+	return 0;
+}
+
+__maybe_unused int yogabook_wmi_resume(struct device *dev)
+{
+	struct wmi_device *wdev = container_of(dev, struct wmi_device, dev);
+	struct yogabook_wmi *data = dev_get_drvdata(dev);
+
+	if (test_bit(YB_KBD_IS_ON, &data->flags)) {
+		/* Ensure keyboard touchpad is on before we call KBLC() */
+		acpi_device_set_power(data->kbd_adev, ACPI_STATE_D0);
+		yogabook_wmi_set_kbd_backlight(wdev, data->brightness);
+	}
+
+	if (test_bit(YB_DIGITIZER_IS_ON, &data->flags))
+		yogabook_wmi_do_action(wdev, YB_PAD_ENABLE);
+
+	clear_bit(YB_SUSPENDED, &data->flags);
+
+	return 0;
+}
+
+static const struct wmi_device_id yogabook_wmi_id_table[] = {
+	{
+		.guid_string = YB_MBTN_EVENT_GUID,
+	},
+	{ } /* Terminating entry */
+};
+
+static SIMPLE_DEV_PM_OPS(yogabook_wmi_pm_ops,
+			 yogabook_wmi_suspend, yogabook_wmi_resume);
+
+static struct wmi_driver yogabook_wmi_driver = {
+	.driver = {
+		.name = "yogabook-wmi",
+		.pm = &yogabook_wmi_pm_ops,
+	},
+	.no_notify_data = true,
+	.id_table = yogabook_wmi_id_table,
+	.probe = yogabook_wmi_probe,
+	.remove = yogabook_wmi_remove,
+	.notify = yogabook_wmi_notify,
+};
+module_wmi_driver(yogabook_wmi_driver);
+
+MODULE_DEVICE_TABLE(wmi, yogabook_wmi_id_table);
+MODULE_AUTHOR("Yauhen Kharuzhy");
+MODULE_DESCRIPTION("Lenovo Yoga Book WMI driver");
+MODULE_LICENSE("GPL v2");
-- 
2.33.1

Re: [PATCH 4/5] platform/x86: lenovo-yogabook-wmi: Add driver for Lenovo Yoga Book

[Hans de Goede]

Hi,

On 11/28/21 20:00, Hans de Goede wrote:
> From: Yauhen Kharuzhy <jekhor@gmail.com>
> 
> Add driver to handle WMI events, control the keyboard backlight and
> bind/unbind the keyboard-touch / digitizer driver so that only one
> is active at a time.
> 
> It may seem a bit weird to handle the toggling of the modes in the
> kernel, but the hw actually expects only 1 device to be active
> at a time.
> 
> Changes by Hans de Goede:
> - Whole bunch of cleanups
> - Make the kernel do the driver bind/unbind itself instead of
>   sending events to userspace and requiring a special userspace
>   daemon to deal with this
> 
> Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
> Co-developed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Note this new driver relies on my:

"pinctrl: cherryview: Do not allow the same interrupt line to be used by 2 pins"

patch to also be in the kernel, otherwise when the digitizer is
disabled during driver load (leaving only the touchscreen-kbd
active), this will result in disabling the "shared" interrupt
line breaking the touchscreen-kbd functionality until the users
toggles to digitizer mode and back to touchscreen-kbd again.

Regards,

Hans



> ---
>  drivers/platform/x86/Kconfig               |  13 +
>  drivers/platform/x86/Makefile              |   1 +
>  drivers/platform/x86/lenovo-yogabook-wmi.c | 339 +++++++++++++++++++++
>  3 files changed, 353 insertions(+)
>  create mode 100644 drivers/platform/x86/lenovo-yogabook-wmi.c
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index ad1c45682f0e..a1dd5e0676d1 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -127,6 +127,19 @@ config GIGABYTE_WMI
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called gigabyte-wmi.
>  
> +config YOGABOOK_WMI
> +	tristate "Lenovo Yoga Book tablet WMI key driver"
> +	depends on ACPI_WMI
> +	depends on INPUT
> +	select LEDS_CLASS
> +	select NEW_LEDS
> +	help
> +	  Say Y here if you want to support the 'Pen' key and keyboard backlight
> +	  control on the Lenovo Yoga Book tablets.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called lenovo-yogabook-wmi.
> +
>  config ACERHDF
>  	tristate "Acer Aspire One temperature and fan driver"
>  	depends on ACPI && THERMAL
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index d6bd2ff5f76d..30047df8f701 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT)	+= nvidia-wmi-ec-backlight.o
>  obj-$(CONFIG_PEAQ_WMI)			+= peaq-wmi.o
>  obj-$(CONFIG_XIAOMI_WMI)		+= xiaomi-wmi.o
>  obj-$(CONFIG_GIGABYTE_WMI)		+= gigabyte-wmi.o
> +obj-$(CONFIG_YOGABOOK_WMI)		+= lenovo-yogabook-wmi.o
>  
>  # Acer
>  obj-$(CONFIG_ACERHDF)		+= acerhdf.o
> diff --git a/drivers/platform/x86/lenovo-yogabook-wmi.c b/drivers/platform/x86/lenovo-yogabook-wmi.c
> new file mode 100644
> index 000000000000..ecd624d9108f
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo-yogabook-wmi.c
> @@ -0,0 +1,339 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* WMI driver for Lenovo Yoga Book YB1-X90* / -X91* tablets */
> +
> +#include <linux/acpi.h>
> +#include <linux/devm-helpers.h>
> +#include <linux/module.h>
> +#include <linux/leds.h>
> +#include <linux/wmi.h>
> +#include <linux/workqueue.h>
> +
> +#define YB_MBTN_EVENT_GUID	"243FEC1D-1963-41C1-8100-06A9D82A94B4"
> +#define YB_MBTN_METHOD_GUID	"742B0CA1-0B20-404B-9CAA-AEFCABF30CE0"
> +
> +#define YB_PAD_ENABLE	1
> +#define YB_PAD_DISABLE	2
> +#define YB_LIGHTUP_BTN	3
> +
> +#define YB_KBD_BL_DEFAULT 128
> +
> +/* flags */
> +enum {
> +	YB_KBD_IS_ON,
> +	YB_DIGITIZER_IS_ON,
> +	YB_DIGITIZER_MODE,
> +	YB_SUSPENDED,
> +};
> +
> +struct yogabook_wmi {
> +	struct wmi_device *wdev;
> +	struct acpi_device *kbd_adev;
> +	struct acpi_device *dig_adev;
> +	struct device *kbd_dev;
> +	struct device *dig_dev;
> +	struct work_struct work;
> +	struct led_classdev kbd_bl_led;
> +	unsigned long flags;
> +	uint8_t brightness;
> +};
> +
> +static int yogabook_wmi_do_action(struct wmi_device *wdev, int action)
> +{
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_buffer input;
> +	acpi_status status;
> +	u32 dummy_arg = 0;
> +
> +	dev_dbg(&wdev->dev, "Do action: %d\n", action);
> +
> +	input.pointer = &dummy_arg;
> +	input.length = sizeof(dummy_arg);
> +
> +	status = wmi_evaluate_method(YB_MBTN_METHOD_GUID, 0, action, &input,
> +				     &output);
> +	if (ACPI_FAILURE(status)) {
> +		dev_err(&wdev->dev, "Calling WMI method failure: 0x%x\n",
> +			status);
> +		return status;
> +	}
> +
> +	kfree(output.pointer);
> +
> +	return 0;
> +}
> +
> +/*
> + * To control keyboard backlight, call the method KBLC() of the TCS1 ACPI
> + * device (Goodix touchpad acts as virtual sensor keyboard).
> + */
> +static int yogabook_wmi_set_kbd_backlight(struct wmi_device *wdev,
> +					  uint8_t level)
> +{
> +	struct yogabook_wmi *data = dev_get_drvdata(&wdev->dev);
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_object_list input;
> +	union acpi_object param;
> +	acpi_status status;
> +
> +	if (data->kbd_adev->power.state != ACPI_STATE_D0) {
> +		dev_warn(&wdev->dev, "keyboard touchscreen not in D0, cannot set brightness\n");
> +		return -ENXIO;
> +	}
> +
> +	dev_dbg(&wdev->dev, "Set KBLC level to %u\n", level);
> +
> +	input.count = 1;
> +	input.pointer = &param;
> +
> +	param.type = ACPI_TYPE_INTEGER;
> +	param.integer.value = 255 - level;
> +
> +	status = acpi_evaluate_object(acpi_device_handle(data->kbd_adev), "KBLC",
> +				      &input, &output);
> +	if (ACPI_FAILURE(status)) {
> +		dev_err(&wdev->dev, "Failed to call KBLC method: 0x%x\n", status);
> +		return status;
> +	}
> +
> +	kfree(output.pointer);
> +	return 0;
> +}
> +
> +static void yogabook_wmi_work(struct work_struct *work)
> +{
> +	struct yogabook_wmi *data = container_of(work, struct yogabook_wmi, work);
> +	struct device *dev = &data->wdev->dev;
> +	bool kbd_on, digitizer_on;
> +	int r;
> +
> +	if (test_bit(YB_SUSPENDED, &data->flags))
> +		return;
> +
> +	if (test_bit(YB_DIGITIZER_MODE, &data->flags)) {
> +		digitizer_on = true;
> +		kbd_on = false;
> +	} else {
> +		kbd_on = true;
> +		digitizer_on = false;
> +	}
> +
> +	if (!kbd_on && test_bit(YB_KBD_IS_ON, &data->flags)) {
> +		/*
> +		 * Must be done before releasing the keyboard touchscreen driver,
> +		 * so that the keyboard touchscreen dev is still in D0.
> +		 */
> +		yogabook_wmi_set_kbd_backlight(data->wdev, 0);
> +		device_release_driver(data->kbd_dev);
> +		clear_bit(YB_KBD_IS_ON, &data->flags);
> +	}
> +
> +	if (!digitizer_on && test_bit(YB_DIGITIZER_IS_ON, &data->flags)) {
> +		yogabook_wmi_do_action(data->wdev, YB_PAD_DISABLE);
> +		device_release_driver(data->dig_dev);
> +		clear_bit(YB_DIGITIZER_IS_ON, &data->flags);
> +	}
> +
> +	if (kbd_on && !test_bit(YB_KBD_IS_ON, &data->flags)) {
> +		r = device_reprobe(data->kbd_dev);
> +		if (r)
> +			dev_warn(dev, "Reprobe of keyboard touchscreen failed: %d\n", r);
> +
> +		yogabook_wmi_set_kbd_backlight(data->wdev, data->brightness);
> +		set_bit(YB_KBD_IS_ON, &data->flags);
> +	}
> +
> +	if (digitizer_on && !test_bit(YB_DIGITIZER_IS_ON, &data->flags)) {
> +		r = device_reprobe(data->dig_dev);
> +		if (r)
> +			dev_warn(dev, "Reprobe of digitizer failed: %d\n", r);
> +
> +		yogabook_wmi_do_action(data->wdev, YB_PAD_ENABLE);
> +		set_bit(YB_DIGITIZER_IS_ON, &data->flags);
> +	}
> +}
> +
> +static void yogabook_wmi_notify(struct wmi_device *wdev, union acpi_object *dummy)
> +{
> +	struct yogabook_wmi *data = dev_get_drvdata(&wdev->dev);
> +
> +	if (test_bit(YB_SUSPENDED, &data->flags))
> +		return;
> +
> +	if (test_bit(YB_DIGITIZER_MODE, &data->flags))
> +		clear_bit(YB_DIGITIZER_MODE, &data->flags);
> +	else
> +		set_bit(YB_DIGITIZER_MODE, &data->flags);
> +
> +	/*
> +	 * We are called from the ACPI core and the driver [un]binding which is
> +	 * done also needs ACPI functions, use a workqueue to avoid deadlocking.
> +	 */
> +	schedule_work(&data->work);
> +}
> +
> +static enum led_brightness kbd_brightness_get(struct led_classdev *cdev)
> +{
> +	struct yogabook_wmi *data =
> +		container_of(cdev, struct yogabook_wmi, kbd_bl_led);
> +
> +	return data->brightness;
> +}
> +
> +static int kbd_brightness_set(struct led_classdev *cdev,
> +			      enum led_brightness value)
> +{
> +	struct yogabook_wmi *data =
> +		container_of(cdev, struct yogabook_wmi, kbd_bl_led);
> +	struct wmi_device *wdev = data->wdev;
> +
> +	if ((value < 0) || (value > 255))
> +		return -EINVAL;
> +
> +	data->brightness = value;
> +
> +	if (data->kbd_adev->power.state != ACPI_STATE_D0)
> +		return 0;
> +
> +	return yogabook_wmi_set_kbd_backlight(wdev, data->brightness);
> +}
> +
> +static int yogabook_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> +	struct yogabook_wmi *data;
> +	int r;
> +
> +	data = devm_kzalloc(&wdev->dev, sizeof(struct yogabook_wmi), GFP_KERNEL);
> +	if (data == NULL)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&wdev->dev, data);
> +
> +	data->wdev = wdev;
> +	data->brightness = YB_KBD_BL_DEFAULT;
> +	set_bit(YB_KBD_IS_ON, &data->flags);
> +	set_bit(YB_DIGITIZER_IS_ON, &data->flags);
> +
> +	r = devm_work_autocancel(&wdev->dev, &data->work, yogabook_wmi_work);
> +	if (r)
> +		return r;
> +
> +	data->kbd_adev = acpi_dev_get_first_match_dev("GDIX1001", NULL, -1);
> +	if (!data->kbd_adev) {
> +		dev_err(&wdev->dev, "Cannot find the touchpad device in ACPI tables\n");
> +		return -ENODEV;
> +	}
> +
> +	data->dig_adev = acpi_dev_get_first_match_dev("WCOM0019", NULL, -1);
> +	if (!data->dig_adev) {
> +		dev_err(&wdev->dev, "Cannot find the digitizer device in ACPI tables\n");
> +		r = -ENODEV;
> +		goto error_put_devs;
> +	}
> +
> +	data->kbd_dev = get_device(acpi_get_first_physical_node(data->kbd_adev));
> +	if (!data->kbd_dev || !data->kbd_dev->driver) {
> +		r = -EPROBE_DEFER;
> +		goto error_put_devs;
> +	}
> +
> +	data->dig_dev = get_device(acpi_get_first_physical_node(data->dig_adev));
> +	if (!data->dig_dev || !data->dig_dev->driver) {
> +		r = -EPROBE_DEFER;
> +		goto error_put_devs;
> +	}
> +
> +	schedule_work(&data->work);
> +
> +	data->kbd_bl_led.name = "ybwmi::kbd_backlight";
> +	data->kbd_bl_led.brightness_set_blocking = kbd_brightness_set;
> +	data->kbd_bl_led.brightness_get = kbd_brightness_get;
> +	data->kbd_bl_led.max_brightness = 255;
> +
> +	r = devm_led_classdev_register(&wdev->dev, &data->kbd_bl_led);
> +	if (r < 0) {
> +		dev_err_probe(&wdev->dev, r, "Registering backlight LED device\n");
> +		goto error_put_devs;
> +	}
> +
> +	return 0;
> +
> +error_put_devs:
> +	put_device(data->dig_dev);
> +	put_device(data->kbd_dev);
> +	acpi_dev_put(data->dig_adev);
> +	acpi_dev_put(data->kbd_adev);
> +	return r;
> +}
> +
> +static void yogabook_wmi_remove(struct wmi_device *wdev)
> +{
> +	struct yogabook_wmi *data = dev_get_drvdata(&wdev->dev);
> +
> +	put_device(data->dig_dev);
> +	put_device(data->kbd_dev);
> +	acpi_dev_put(data->dig_adev);
> +	acpi_dev_put(data->kbd_adev);
> +}
> +
> +__maybe_unused int yogabook_wmi_suspend(struct device *dev)
> +{
> +	struct wmi_device *wdev = container_of(dev, struct wmi_device, dev);
> +	struct yogabook_wmi *data = dev_get_drvdata(dev);
> +
> +	set_bit(YB_SUSPENDED, &data->flags);
> +
> +	flush_work(&data->work);
> +
> +	/* Turn off the pen button at sleep */
> +	if (test_bit(YB_DIGITIZER_IS_ON, &data->flags))
> +		yogabook_wmi_do_action(wdev, YB_PAD_DISABLE);
> +
> +	return 0;
> +}
> +
> +__maybe_unused int yogabook_wmi_resume(struct device *dev)
> +{
> +	struct wmi_device *wdev = container_of(dev, struct wmi_device, dev);
> +	struct yogabook_wmi *data = dev_get_drvdata(dev);
> +
> +	if (test_bit(YB_KBD_IS_ON, &data->flags)) {
> +		/* Ensure keyboard touchpad is on before we call KBLC() */
> +		acpi_device_set_power(data->kbd_adev, ACPI_STATE_D0);
> +		yogabook_wmi_set_kbd_backlight(wdev, data->brightness);
> +	}
> +
> +	if (test_bit(YB_DIGITIZER_IS_ON, &data->flags))
> +		yogabook_wmi_do_action(wdev, YB_PAD_ENABLE);
> +
> +	clear_bit(YB_SUSPENDED, &data->flags);
> +
> +	return 0;
> +}
> +
> +static const struct wmi_device_id yogabook_wmi_id_table[] = {
> +	{
> +		.guid_string = YB_MBTN_EVENT_GUID,
> +	},
> +	{ } /* Terminating entry */
> +};
> +
> +static SIMPLE_DEV_PM_OPS(yogabook_wmi_pm_ops,
> +			 yogabook_wmi_suspend, yogabook_wmi_resume);
> +
> +static struct wmi_driver yogabook_wmi_driver = {
> +	.driver = {
> +		.name = "yogabook-wmi",
> +		.pm = &yogabook_wmi_pm_ops,
> +	},
> +	.no_notify_data = true,
> +	.id_table = yogabook_wmi_id_table,
> +	.probe = yogabook_wmi_probe,
> +	.remove = yogabook_wmi_remove,
> +	.notify = yogabook_wmi_notify,
> +};
> +module_wmi_driver(yogabook_wmi_driver);
> +
> +MODULE_DEVICE_TABLE(wmi, yogabook_wmi_id_table);
> +MODULE_AUTHOR("Yauhen Kharuzhy");
> +MODULE_DESCRIPTION("Lenovo Yoga Book WMI driver");
> +MODULE_LICENSE("GPL v2");
> 

[PATCH 5/5] platform/x86: lenovo-yogabook-wmi: Add support for hall sensor on the back

[Hans de Goede]

On the back of the device there is a hall sensor connected to the
"INT33FF:02" GPIO controller pin 18, which gets triggered when the
device is fully folded into tablet-mode (when the back of the display
touched the back of the keyboard).

Use this to disable both the touch-keyboard and the digitizer when
the tablet is fully folded into tablet-mode.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/lenovo-yogabook-wmi.c | 71 +++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/lenovo-yogabook-wmi.c b/drivers/platform/x86/lenovo-yogabook-wmi.c
index ecd624d9108f..65dd1ed97762 100644
--- a/drivers/platform/x86/lenovo-yogabook-wmi.c
+++ b/drivers/platform/x86/lenovo-yogabook-wmi.c
@@ -3,6 +3,9 @@
 
 #include <linux/acpi.h>
 #include <linux/devm-helpers.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
+#include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/leds.h>
 #include <linux/wmi.h>
@@ -22,6 +25,7 @@ enum {
 	YB_KBD_IS_ON,
 	YB_DIGITIZER_IS_ON,
 	YB_DIGITIZER_MODE,
+	YB_TABLET_MODE,
 	YB_SUSPENDED,
 };
 
@@ -31,6 +35,8 @@ struct yogabook_wmi {
 	struct acpi_device *dig_adev;
 	struct device *kbd_dev;
 	struct device *dig_dev;
+	struct gpio_desc *backside_hall_gpio;
+	int backside_hall_irq;
 	struct work_struct work;
 	struct led_classdev kbd_bl_led;
 	unsigned long flags;
@@ -109,7 +115,10 @@ static void yogabook_wmi_work(struct work_struct *work)
 	if (test_bit(YB_SUSPENDED, &data->flags))
 		return;
 
-	if (test_bit(YB_DIGITIZER_MODE, &data->flags)) {
+	if (test_bit(YB_TABLET_MODE, &data->flags)) {
+		kbd_on = false;
+		digitizer_on = false;
+	} else if (test_bit(YB_DIGITIZER_MODE, &data->flags)) {
 		digitizer_on = true;
 		kbd_on = false;
 	} else {
@@ -171,6 +180,20 @@ static void yogabook_wmi_notify(struct wmi_device *wdev, union acpi_object *dumm
 	schedule_work(&data->work);
 }
 
+static irqreturn_t yogabook_backside_hall_irq(int irq, void *_data)
+{
+	struct yogabook_wmi *data = _data;
+
+	if (gpiod_get_value(data->backside_hall_gpio))
+		set_bit(YB_TABLET_MODE, &data->flags);
+	else
+		clear_bit(YB_TABLET_MODE, &data->flags);
+
+	schedule_work(&data->work);
+
+	return IRQ_HANDLED;
+}
+
 static enum led_brightness kbd_brightness_get(struct led_classdev *cdev)
 {
 	struct yogabook_wmi *data =
@@ -197,6 +220,19 @@ static int kbd_brightness_set(struct led_classdev *cdev,
 	return yogabook_wmi_set_kbd_backlight(wdev, data->brightness);
 }
 
+static struct gpiod_lookup_table yogabook_wmi_gpios = {
+	.dev_id		= "243FEC1D-1963-41C1-8100-06A9D82A94B4",
+	.table		= {
+		GPIO_LOOKUP("INT33FF:02", 18, "backside_hall_sw", GPIO_ACTIVE_LOW),
+		{}
+	},
+};
+
+static void yogabook_wmi_rm_gpio_lookup(void *unused)
+{
+	gpiod_remove_lookup_table(&yogabook_wmi_gpios);
+}
+
 static int yogabook_wmi_probe(struct wmi_device *wdev, const void *context)
 {
 	struct yogabook_wmi *data;
@@ -242,6 +278,36 @@ static int yogabook_wmi_probe(struct wmi_device *wdev, const void *context)
 		goto error_put_devs;
 	}
 
+	gpiod_add_lookup_table(&yogabook_wmi_gpios);
+
+	r = devm_add_action_or_reset(&wdev->dev, yogabook_wmi_rm_gpio_lookup, NULL);
+	if (r)
+		goto error_put_devs;
+
+	data->backside_hall_gpio =
+		devm_gpiod_get(&wdev->dev, "backside_hall_sw", GPIOD_IN);
+	if (IS_ERR(data->backside_hall_gpio)) {
+		r = PTR_ERR(data->backside_hall_gpio);
+		dev_err_probe(&wdev->dev, r, "Getting backside_hall_sw GPIO\n");
+		goto error_put_devs;
+	}
+
+	r = gpiod_to_irq(data->backside_hall_gpio);
+	if (r < 0) {
+		dev_err_probe(&wdev->dev, r, "Getting backside_hall_sw IRQ\n");
+		goto error_put_devs;
+	}
+	data->backside_hall_irq = r;
+
+	r = devm_request_irq(&wdev->dev, data->backside_hall_irq,
+			     yogabook_backside_hall_irq,
+			     IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			     "backside_hall_sw", data);
+	if (r) {
+		dev_err_probe(&wdev->dev, r, "Requesting backside_hall_sw IRQ\n");
+		goto error_put_devs;
+	}
+
 	schedule_work(&data->work);
 
 	data->kbd_bl_led.name = "ybwmi::kbd_backlight";
@@ -307,6 +373,9 @@ __maybe_unused int yogabook_wmi_resume(struct device *dev)
 
 	clear_bit(YB_SUSPENDED, &data->flags);
 
+	/* Check for YB_TABLET_MODE changes made during suspend */
+	schedule_work(&data->work);
+
 	return 0;
 }
 
-- 
2.33.1

Re: [PATCH 5/5] platform/x86: lenovo-yogabook-wmi: Add support for hall sensor on the back

[Yauhen Kharuzhy]

On Sun, Nov 28, 2021 at 08:00:31PM +0100, Hans de Goede wrote:
> On the back of the device there is a hall sensor connected to the
> "INT33FF:02" GPIO controller pin 18, which gets triggered when the
> device is fully folded into tablet-mode (when the back of the display
> touched the back of the keyboard).
> 
> Use this to disable both the touch-keyboard and the digitizer when
> the tablet is fully folded into tablet-mode.

Wonderful. I knew that this device would still bring surprises...I
didn't find this sensor in my investigation :)

In your opinion, is it possible to use this sensor to correct of the custom
hinge angle sensor readings from the Intel sensor hub? The hinge angle sensor
uses accelerometers to calculate the angle and cannot reliably
distinquish between 0 and 360 degrees. I used the lid switch status in a my
patch for iio-sensors-proxy but this sensor looks like a yet another candidate.
The lid switch is more generic soultion, of course.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/platform/x86/lenovo-yogabook-wmi.c | 71 +++++++++++++++++++++-
>  1 file changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/lenovo-yogabook-wmi.c b/drivers/platform/x86/lenovo-yogabook-wmi.c
> index ecd624d9108f..65dd1ed97762 100644
> --- a/drivers/platform/x86/lenovo-yogabook-wmi.c
> +++ b/drivers/platform/x86/lenovo-yogabook-wmi.c
> @@ -3,6 +3,9 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/devm-helpers.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/machine.h>
> +#include <linux/interrupt.h>
>  #include <linux/module.h>
>  #include <linux/leds.h>
>  #include <linux/wmi.h>
> @@ -22,6 +25,7 @@ enum {
>  	YB_KBD_IS_ON,
>  	YB_DIGITIZER_IS_ON,
>  	YB_DIGITIZER_MODE,
> +	YB_TABLET_MODE,
>  	YB_SUSPENDED,
>  };
>  
> @@ -31,6 +35,8 @@ struct yogabook_wmi {
>  	struct acpi_device *dig_adev;
>  	struct device *kbd_dev;
>  	struct device *dig_dev;
> +	struct gpio_desc *backside_hall_gpio;
> +	int backside_hall_irq;
>  	struct work_struct work;
>  	struct led_classdev kbd_bl_led;
>  	unsigned long flags;
> @@ -109,7 +115,10 @@ static void yogabook_wmi_work(struct work_struct *work)
>  	if (test_bit(YB_SUSPENDED, &data->flags))
>  		return;
>  
> -	if (test_bit(YB_DIGITIZER_MODE, &data->flags)) {
> +	if (test_bit(YB_TABLET_MODE, &data->flags)) {
> +		kbd_on = false;
> +		digitizer_on = false;
> +	} else if (test_bit(YB_DIGITIZER_MODE, &data->flags)) {
>  		digitizer_on = true;
>  		kbd_on = false;
>  	} else {
> @@ -171,6 +180,20 @@ static void yogabook_wmi_notify(struct wmi_device *wdev, union acpi_object *dumm
>  	schedule_work(&data->work);
>  }
>  
> +static irqreturn_t yogabook_backside_hall_irq(int irq, void *_data)
> +{
> +	struct yogabook_wmi *data = _data;
> +
> +	if (gpiod_get_value(data->backside_hall_gpio))
> +		set_bit(YB_TABLET_MODE, &data->flags);
> +	else
> +		clear_bit(YB_TABLET_MODE, &data->flags);
> +
> +	schedule_work(&data->work);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static enum led_brightness kbd_brightness_get(struct led_classdev *cdev)
>  {
>  	struct yogabook_wmi *data =
> @@ -197,6 +220,19 @@ static int kbd_brightness_set(struct led_classdev *cdev,
>  	return yogabook_wmi_set_kbd_backlight(wdev, data->brightness);
>  }
>  
> +static struct gpiod_lookup_table yogabook_wmi_gpios = {
> +	.dev_id		= "243FEC1D-1963-41C1-8100-06A9D82A94B4",
> +	.table		= {
> +		GPIO_LOOKUP("INT33FF:02", 18, "backside_hall_sw", GPIO_ACTIVE_LOW),
> +		{}
> +	},
> +};
> +
> +static void yogabook_wmi_rm_gpio_lookup(void *unused)
> +{
> +	gpiod_remove_lookup_table(&yogabook_wmi_gpios);
> +}
> +
>  static int yogabook_wmi_probe(struct wmi_device *wdev, const void *context)
>  {
>  	struct yogabook_wmi *data;
> @@ -242,6 +278,36 @@ static int yogabook_wmi_probe(struct wmi_device *wdev, const void *context)
>  		goto error_put_devs;
>  	}
>  
> +	gpiod_add_lookup_table(&yogabook_wmi_gpios);
> +
> +	r = devm_add_action_or_reset(&wdev->dev, yogabook_wmi_rm_gpio_lookup, NULL);
> +	if (r)
> +		goto error_put_devs;
> +
> +	data->backside_hall_gpio =
> +		devm_gpiod_get(&wdev->dev, "backside_hall_sw", GPIOD_IN);
> +	if (IS_ERR(data->backside_hall_gpio)) {
> +		r = PTR_ERR(data->backside_hall_gpio);
> +		dev_err_probe(&wdev->dev, r, "Getting backside_hall_sw GPIO\n");
> +		goto error_put_devs;
> +	}
> +
> +	r = gpiod_to_irq(data->backside_hall_gpio);
> +	if (r < 0) {
> +		dev_err_probe(&wdev->dev, r, "Getting backside_hall_sw IRQ\n");
> +		goto error_put_devs;
> +	}
> +	data->backside_hall_irq = r;
> +
> +	r = devm_request_irq(&wdev->dev, data->backside_hall_irq,
> +			     yogabook_backside_hall_irq,
> +			     IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			     "backside_hall_sw", data);
> +	if (r) {
> +		dev_err_probe(&wdev->dev, r, "Requesting backside_hall_sw IRQ\n");
> +		goto error_put_devs;
> +	}
> +
>  	schedule_work(&data->work);
>  
>  	data->kbd_bl_led.name = "ybwmi::kbd_backlight";
> @@ -307,6 +373,9 @@ __maybe_unused int yogabook_wmi_resume(struct device *dev)
>  
>  	clear_bit(YB_SUSPENDED, &data->flags);
>  
> +	/* Check for YB_TABLET_MODE changes made during suspend */
> +	schedule_work(&data->work);
> +
>  	return 0;
>  }
>  
> -- 
> 2.33.1
> 

-- 
Yauhen Kharuzhy

Re: [PATCH 5/5] platform/x86: lenovo-yogabook-wmi: Add support for hall sensor on the back

[Hans de Goede]

Hi,

On 11/28/21 22:09, Yauhen Kharuzhy wrote:
> On Sun, Nov 28, 2021 at 08:00:31PM +0100, Hans de Goede wrote:
>> On the back of the device there is a hall sensor connected to the
>> "INT33FF:02" GPIO controller pin 18, which gets triggered when the
>> device is fully folded into tablet-mode (when the back of the display
>> touched the back of the keyboard).
>>
>> Use this to disable both the touch-keyboard and the digitizer when
>> the tablet is fully folded into tablet-mode.
> 
> Wonderful. I knew that this device would still bring surprises...I
> didn't find this sensor in my investigation :)
> 
> In your opinion, is it possible to use this sensor to correct of the custom
> hinge angle sensor readings from the Intel sensor hub? The hinge angle sensor
> uses accelerometers to calculate the angle and cannot reliably
> distinquish between 0 and 360 degrees. I used the lid switch status in a my
> patch for iio-sensors-proxy but this sensor looks like a yet another candidate.
> The lid switch is more generic soultion, of course.

Right, there are quite a few yoga / 360° hinges devices which have 2
accelerometers, one in each half. So any solutions in iio-sensors-proxy
really should be generic.

I did think about this before, because I'm interested in the iio-sensor-proxy
solution for this in general and my thoughts on this are that it is fine to make
iio-sensor-proxy send SW_TABLET_MODE=1 for any angle below 5° (and above 185°).

The idea being that if the lid is almost closed in laptop mode then the device is
not being interacted with anyways and then SW_TABLET_MODE=1 does not matter.
In practice what SW_TABLET_MODE=1 does is:

1. Enable auto-rotation of the display, does not matter if the display is almost
fully closed, since clearly the user is not looking at it.

2. Enable automatic pop-up of on-screen-keyboard when selecting text input-fields,
again this does not matter since with the lid almost closed the user is not likely
to select any text-input fields.

3. Suppress *built-in* keyboard and touchpad events to avoid accidental pressed
on the keyboard / touchpad to register, and again with the lid almost fully closed
the user is unlikely to actually want to use the touchpad/keyboard so the false-positive
SW_TABLET_MODE=1 reporting for angles below 5° should not be an issue.

Using the lid switch is an interesting idea. But I believe that the angle accuracy
won't be all that great, so we would need to check the lid-switch for any angle below
say 5° then and in many cases the lid-switch will not report 1 until the lid is
very close to being fully closed, so then their will still be a window where we
do a false-positive reporting of SW_TABLET_MODE=1 . At which point we might just
as well live with the false-positive reporting of SW_TABLET_MODE=1 and not bother
with the lid-switch. At least that is my 2 cents.

Feel free to copy and paste parts of this email to a big comment in the code
explaining why angles below 5° report SW_TABLET_MODE=1, assuming you implement
that idea.

Regards,

Hans




> 
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/platform/x86/lenovo-yogabook-wmi.c | 71 +++++++++++++++++++++-
>>  1 file changed, 70 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/lenovo-yogabook-wmi.c b/drivers/platform/x86/lenovo-yogabook-wmi.c
>> index ecd624d9108f..65dd1ed97762 100644
>> --- a/drivers/platform/x86/lenovo-yogabook-wmi.c
>> +++ b/drivers/platform/x86/lenovo-yogabook-wmi.c
>> @@ -3,6 +3,9 @@
>>  
>>  #include <linux/acpi.h>
>>  #include <linux/devm-helpers.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/gpio/machine.h>
>> +#include <linux/interrupt.h>
>>  #include <linux/module.h>
>>  #include <linux/leds.h>
>>  #include <linux/wmi.h>
>> @@ -22,6 +25,7 @@ enum {
>>  	YB_KBD_IS_ON,
>>  	YB_DIGITIZER_IS_ON,
>>  	YB_DIGITIZER_MODE,
>> +	YB_TABLET_MODE,
>>  	YB_SUSPENDED,
>>  };
>>  
>> @@ -31,6 +35,8 @@ struct yogabook_wmi {
>>  	struct acpi_device *dig_adev;
>>  	struct device *kbd_dev;
>>  	struct device *dig_dev;
>> +	struct gpio_desc *backside_hall_gpio;
>> +	int backside_hall_irq;
>>  	struct work_struct work;
>>  	struct led_classdev kbd_bl_led;
>>  	unsigned long flags;
>> @@ -109,7 +115,10 @@ static void yogabook_wmi_work(struct work_struct *work)
>>  	if (test_bit(YB_SUSPENDED, &data->flags))
>>  		return;
>>  
>> -	if (test_bit(YB_DIGITIZER_MODE, &data->flags)) {
>> +	if (test_bit(YB_TABLET_MODE, &data->flags)) {
>> +		kbd_on = false;
>> +		digitizer_on = false;
>> +	} else if (test_bit(YB_DIGITIZER_MODE, &data->flags)) {
>>  		digitizer_on = true;
>>  		kbd_on = false;
>>  	} else {
>> @@ -171,6 +180,20 @@ static void yogabook_wmi_notify(struct wmi_device *wdev, union acpi_object *dumm
>>  	schedule_work(&data->work);
>>  }
>>  
>> +static irqreturn_t yogabook_backside_hall_irq(int irq, void *_data)
>> +{
>> +	struct yogabook_wmi *data = _data;
>> +
>> +	if (gpiod_get_value(data->backside_hall_gpio))
>> +		set_bit(YB_TABLET_MODE, &data->flags);
>> +	else
>> +		clear_bit(YB_TABLET_MODE, &data->flags);
>> +
>> +	schedule_work(&data->work);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>>  static enum led_brightness kbd_brightness_get(struct led_classdev *cdev)
>>  {
>>  	struct yogabook_wmi *data =
>> @@ -197,6 +220,19 @@ static int kbd_brightness_set(struct led_classdev *cdev,
>>  	return yogabook_wmi_set_kbd_backlight(wdev, data->brightness);
>>  }
>>  
>> +static struct gpiod_lookup_table yogabook_wmi_gpios = {
>> +	.dev_id		= "243FEC1D-1963-41C1-8100-06A9D82A94B4",
>> +	.table		= {
>> +		GPIO_LOOKUP("INT33FF:02", 18, "backside_hall_sw", GPIO_ACTIVE_LOW),
>> +		{}
>> +	},
>> +};
>> +
>> +static void yogabook_wmi_rm_gpio_lookup(void *unused)
>> +{
>> +	gpiod_remove_lookup_table(&yogabook_wmi_gpios);
>> +}
>> +
>>  static int yogabook_wmi_probe(struct wmi_device *wdev, const void *context)
>>  {
>>  	struct yogabook_wmi *data;
>> @@ -242,6 +278,36 @@ static int yogabook_wmi_probe(struct wmi_device *wdev, const void *context)
>>  		goto error_put_devs;
>>  	}
>>  
>> +	gpiod_add_lookup_table(&yogabook_wmi_gpios);
>> +
>> +	r = devm_add_action_or_reset(&wdev->dev, yogabook_wmi_rm_gpio_lookup, NULL);
>> +	if (r)
>> +		goto error_put_devs;
>> +
>> +	data->backside_hall_gpio =
>> +		devm_gpiod_get(&wdev->dev, "backside_hall_sw", GPIOD_IN);
>> +	if (IS_ERR(data->backside_hall_gpio)) {
>> +		r = PTR_ERR(data->backside_hall_gpio);
>> +		dev_err_probe(&wdev->dev, r, "Getting backside_hall_sw GPIO\n");
>> +		goto error_put_devs;
>> +	}
>> +
>> +	r = gpiod_to_irq(data->backside_hall_gpio);
>> +	if (r < 0) {
>> +		dev_err_probe(&wdev->dev, r, "Getting backside_hall_sw IRQ\n");
>> +		goto error_put_devs;
>> +	}
>> +	data->backside_hall_irq = r;
>> +
>> +	r = devm_request_irq(&wdev->dev, data->backside_hall_irq,
>> +			     yogabook_backside_hall_irq,
>> +			     IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
>> +			     "backside_hall_sw", data);
>> +	if (r) {
>> +		dev_err_probe(&wdev->dev, r, "Requesting backside_hall_sw IRQ\n");
>> +		goto error_put_devs;
>> +	}
>> +
>>  	schedule_work(&data->work);
>>  
>>  	data->kbd_bl_led.name = "ybwmi::kbd_backlight";
>> @@ -307,6 +373,9 @@ __maybe_unused int yogabook_wmi_resume(struct device *dev)
>>  
>>  	clear_bit(YB_SUSPENDED, &data->flags);
>>  
>> +	/* Check for YB_TABLET_MODE changes made during suspend */
>> +	schedule_work(&data->work);
>> +
>>  	return 0;
>>  }
>>  
>> -- 
>> 2.33.1
>>
> 

Re: [PATCH 5/5] platform/x86: lenovo-yogabook-wmi: Add support for hall sensor on the back

[Andy Shevchenko]

On Sun, Nov 28, 2021 at 9:00 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> On the back of the device there is a hall sensor connected to the

Hall (it's name)

> "INT33FF:02" GPIO controller pin 18, which gets triggered when the
> device is fully folded into tablet-mode (when the back of the display
> touched the back of the keyboard).

touches?

> Use this to disable both the touch-keyboard and the digitizer when
> the tablet is fully folded into tablet-mode.

...

> +static irqreturn_t yogabook_backside_hall_irq(int irq, void *_data)
> +{
> +       struct yogabook_wmi *data = _data;

> +       if (gpiod_get_value(data->backside_hall_gpio))
> +               set_bit(YB_TABLET_MODE, &data->flags);
> +       else
> +               clear_bit(YB_TABLET_MODE, &data->flags);

assign_bit()?

> +       schedule_work(&data->work);
> +
> +       return IRQ_HANDLED;
> +}

...

> +       data->backside_hall_gpio =
> +               devm_gpiod_get(&wdev->dev, "backside_hall_sw", GPIOD_IN);
> +       if (IS_ERR(data->backside_hall_gpio)) {

> +               r = PTR_ERR(data->backside_hall_gpio);
> +               dev_err_probe(&wdev->dev, r, "Getting backside_hall_sw GPIO\n");

Can be

  r = dev_err_probe(...);

but I think you did that on purpose to make lines shorter.

> +               goto error_put_devs;
> +       }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 5/5] platform/x86: lenovo-yogabook-wmi: Add support for hall sensor on the back

[Hans de Goede]

Hi,

On 11/29/21 10:28, Andy Shevchenko wrote:
> On Sun, Nov 28, 2021 at 9:00 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> On the back of the device there is a hall sensor connected to the
> 
> Hall (it's name)

Ack, will fix.

> 
>> "INT33FF:02" GPIO controller pin 18, which gets triggered when the
>> device is fully folded into tablet-mode (when the back of the display
>> touched the back of the keyboard).
> 
> touches?

Ack, will fix.

> 
>> Use this to disable both the touch-keyboard and the digitizer when
>> the tablet is fully folded into tablet-mode.
> 
> ...
> 
>> +static irqreturn_t yogabook_backside_hall_irq(int irq, void *_data)
>> +{
>> +       struct yogabook_wmi *data = _data;
> 
>> +       if (gpiod_get_value(data->backside_hall_gpio))
>> +               set_bit(YB_TABLET_MODE, &data->flags);
>> +       else
>> +               clear_bit(YB_TABLET_MODE, &data->flags);
> 
> assign_bit()?

Interesting I did not know about that one. I see that it
expands to exactly the same code though and I find
the current version more readable, so I'm going to keep
this as is.

> 
>> +       schedule_work(&data->work);
>> +
>> +       return IRQ_HANDLED;
>> +}
> 
> ...
> 
>> +       data->backside_hall_gpio =
>> +               devm_gpiod_get(&wdev->dev, "backside_hall_sw", GPIOD_IN);
>> +       if (IS_ERR(data->backside_hall_gpio)) {
> 
>> +               r = PTR_ERR(data->backside_hall_gpio);
>> +               dev_err_probe(&wdev->dev, r, "Getting backside_hall_sw GPIO\n");
> 
> Can be
> 
>   r = dev_err_probe(...);
> 
> but I think you did that on purpose to make lines shorter.

Right, I did this this way to not make the lines too long.

Regards,

Hans



> 
>> +               goto error_put_devs;
>> +       }
> 

Re: [PATCH 1/5] platform/x86: wmi: Replace read_takes_no_args with a flags field

[Andy Shevchenko]

On Sun, Nov 28, 2021 at 9:00 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Replace the wmi_block.read_takes_no_args bool field with
> an unsigned long flags field, used together with test_bit()
> and friends.
>
> This is a preparation patch for fixing a driver->notify() vs ->probe()
> race, which requires atomic flag handling.

For patches 1,2,3 and 5 (after addressing minor spelling issues) are fine to me,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/platform/x86/wmi.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index c34341f4da76..46178e03aeca 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -57,6 +57,10 @@ static_assert(sizeof(typeof_member(struct guid_block, guid)) == 16);
>  static_assert(sizeof(struct guid_block) == 20);
>  static_assert(__alignof__(struct guid_block) == 1);
>
> +enum { /* wmi_block flags */
> +       WMI_READ_TAKES_NO_ARGS,
> +};
> +
>  struct wmi_block {
>         struct wmi_device dev;
>         struct list_head list;
> @@ -67,8 +71,7 @@ struct wmi_block {
>         wmi_notify_handler handler;
>         void *handler_data;
>         u64 req_buf_size;
> -
> -       bool read_takes_no_args;
> +       unsigned long flags;
>  };
>
>
> @@ -367,7 +370,7 @@ static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
>         wq_params[0].type = ACPI_TYPE_INTEGER;
>         wq_params[0].integer.value = instance;
>
> -       if (instance == 0 && wblock->read_takes_no_args)
> +       if (instance == 0 && test_bit(WMI_READ_TAKES_NO_ARGS, &wblock->flags))
>                 input.count = 0;
>
>         /*
> @@ -1113,7 +1116,7 @@ static int wmi_create_device(struct device *wmi_bus_dev,
>          * laptops, WQxx may not be a method at all.)
>          */
>         if (info->type != ACPI_TYPE_METHOD || info->param_count == 0)
> -               wblock->read_takes_no_args = true;
> +               set_bit(WMI_READ_TAKES_NO_ARGS, &wblock->flags);
>
>         kfree(info);
>
> --
> 2.33.1
>


--
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] platform/x86: wmi: Replace read_takes_no_args with a flags field

[Hans de Goede]

Hi,

On 11/30/21 15:52, Andy Shevchenko wrote:
> On Sun, Nov 28, 2021 at 9:00 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Replace the wmi_block.read_takes_no_args bool field with
>> an unsigned long flags field, used together with test_bit()
>> and friends.
>>
>> This is a preparation patch for fixing a driver->notify() vs ->probe()
>> race, which requires atomic flag handling.
> 
> For patches 1,2,3 and 5 (after addressing minor spelling issues) are fine to me,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thank you. I've added this series to my review-hans branch now, with
your Reviewed-by added and the spelling issues addressed.

Regards,

Hans



> 
> 
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/platform/x86/wmi.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
>> index c34341f4da76..46178e03aeca 100644
>> --- a/drivers/platform/x86/wmi.c
>> +++ b/drivers/platform/x86/wmi.c
>> @@ -57,6 +57,10 @@ static_assert(sizeof(typeof_member(struct guid_block, guid)) == 16);
>>  static_assert(sizeof(struct guid_block) == 20);
>>  static_assert(__alignof__(struct guid_block) == 1);
>>
>> +enum { /* wmi_block flags */
>> +       WMI_READ_TAKES_NO_ARGS,
>> +};
>> +
>>  struct wmi_block {
>>         struct wmi_device dev;
>>         struct list_head list;
>> @@ -67,8 +71,7 @@ struct wmi_block {
>>         wmi_notify_handler handler;
>>         void *handler_data;
>>         u64 req_buf_size;
>> -
>> -       bool read_takes_no_args;
>> +       unsigned long flags;
>>  };
>>
>>
>> @@ -367,7 +370,7 @@ static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
>>         wq_params[0].type = ACPI_TYPE_INTEGER;
>>         wq_params[0].integer.value = instance;
>>
>> -       if (instance == 0 && wblock->read_takes_no_args)
>> +       if (instance == 0 && test_bit(WMI_READ_TAKES_NO_ARGS, &wblock->flags))
>>                 input.count = 0;
>>
>>         /*
>> @@ -1113,7 +1116,7 @@ static int wmi_create_device(struct device *wmi_bus_dev,
>>          * laptops, WQxx may not be a method at all.)
>>          */
>>         if (info->type != ACPI_TYPE_METHOD || info->param_count == 0)
>> -               wblock->read_takes_no_args = true;
>> +               set_bit(WMI_READ_TAKES_NO_ARGS, &wblock->flags);
>>
>>         kfree(info);
>>
>> --
>> 2.33.1
>>
> 
> 
> --
> With Best Regards,
> Andy Shevchenko
>