Re: [PATCH v8] platform/x86: add lenovo wmi camera button driver
From: Armin Wolf
Date: Fri Mar 15 2024 - 14:03:44 EST
Am 15.03.24 um 18:54 schrieb Ilpo Järvinen:
On Fri, 15 Mar 2024, Armin Wolf wrote:
Am 15.03.24 um 12:51 schrieb Ilpo Järvinen:
On Thu, 14 Mar 2024, Armin Wolf wrote:
Am 14.03.24 um 06:03 schrieb Ai Chao:
Add lenovo generic wmi driver to support camera button.
The Camera button is a GPIO device. This driver receives ACPI notifyi
when the camera button is switched on/off. This driver is used in
Lenovo A70, it is a Computer integrated machine.
Signed-off-by: Ai Chao <aichao@xxxxxxxxxx>
---
v8: Dev_deb convert to dev_err.
v7: Add dev_dbg and remove unused dev in struct.
v6: Modify SW_CAMERA_LENS_COVER to
KEY_CAMERA_ACCESS_ENABLE/KEY_CAMERA_ACCESS_DISABLE.
v5: Remove camera button groups, modify KEY_CAMERA to
SW_CAMERA_LENS_COVER.
v4: Remove lenovo_wmi_input_setup, move camera_mode into struct
lenovo_wmi_priv.
v3: Remove lenovo_wmi_remove function.
v2: Adjust GPL v2 to GPL, adjust sprintf to sysfs_emit.
drivers/platform/x86/Kconfig | 12 +++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/lenovo-wmi-camera.c | 108
+++++++++++++++++++++++
3 files changed, 121 insertions(+)
create mode 100644 drivers/platform/x86/lenovo-wmi-camera.c
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index bdd302274b9a..9506a455b547 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1001,6 +1001,18 @@ config INSPUR_PLATFORM_PROFILE
To compile this driver as a module, choose M here: the module
will be called inspur-platform-profile.
+config LENOVO_WMI_CAMERA
+ tristate "Lenovo WMI Camera Button driver"
+ depends on ACPI_WMI
+ depends on INPUT
+ help
+ This driver provides support for Lenovo camera button. The Camera
+ button is a GPIO device. This driver receives ACPI notify when the
+ camera button is switched on/off.
+
+ To compile this driver as a module, choose M here: the module
+ will be called lenovo-wmi-camera.
+
source "drivers/platform/x86/x86-android-tablets/Kconfig"
config FW_ATTR_CLASS
diff --git a/drivers/platform/x86/Makefile
b/drivers/platform/x86/Makefile
index 1de432e8861e..217e94d7c877 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o
obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o
obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o
obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o
+obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o
# Intel
obj-y += intel/
diff --git a/drivers/platform/x86/lenovo-wmi-camera.c
b/drivers/platform/x86/lenovo-wmi-camera.c
new file mode 100644
index 000000000000..f83e3ccd9189
--- /dev/null
+++ b/drivers/platform/x86/lenovo-wmi-camera.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Lenovo WMI Camera Button Driver
+ *
+ * Author: Ai Chao <aichao@xxxxxxxxxx>
+ * Copyright (C) 2024 KylinSoft Corporation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/wmi.h>
+
+#define WMI_LENOVO_CAMERABUTTON_EVENT_GUID
"50C76F1F-D8E4-D895-0A3D-62F4EA400013"
+
+struct lenovo_wmi_priv {
+ struct input_dev *idev;
+};
+
+enum {
+ SW_CAMERA_OFF = 0,
+ SW_CAMERA_ON = 1,
+};
+
+static void lenovo_wmi_notify(struct wmi_device *wdev, union
acpi_object
*obj)
+{
+ struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
+ u8 camera_mode;
+
+ if (obj->type != ACPI_TYPE_BUFFER) {
+ dev_err(&wdev->dev, "Bad response type %u\n", obj->type);
+ return;
+ }
+
+ if (obj->buffer.length != 1) {
+ dev_err(&wdev->dev, "Invalid buffer length %u\n",
obj->buffer.length);
+ return;
+ }
+
+ /* obj->buffer.pointer[0] is camera mode:
+ * 0 camera close
+ * 1 camera open
+ */
+ camera_mode = obj->buffer.pointer[0];
+ if (camera_mode > SW_CAMERA_ON) {
+ dev_err(&wdev->dev, "Unknown camera mode %u\n", camera_mode);
+ return;
+ }
+
+ if (camera_mode == SW_CAMERA_ON) {
+ input_report_key(priv->idev, KEY_CAMERA_ACCESS_ENABLE, 1);
+ input_sync(priv->idev);
+ input_report_key(priv->idev, KEY_CAMERA_ACCESS_ENABLE, 0);
+ } else {
+ input_report_key(priv->idev, KEY_CAMERA_ACCESS_DISABLE, 1);
+ input_sync(priv->idev);
+ input_report_key(priv->idev, KEY_CAMERA_ACCESS_DISABLE, 0);
+ }
While not exactly wrong the if seems unnecessary, you could do:
unsigned int keycode;
...
keycode = camera_mode == SW_CAMERA_ON ? KEY_CAMERA_ACCESS_ENABLE :
KEY_CAMERA_ACCESS_DISABLE;
input_report_key(priv->idev, keycode, 1);
input_sync(priv->idev);
input_report_key(priv->idev, keycode, 0);
+ input_sync(priv->idev);
+}
Armin,
I tried to figure out the concurrency rules for the WMI notify handler but
came up basically nothing. I suppose it boils down on ACPI notify handling
and I couldn't find useful documentation about that either. :-/
Could you perhaps add this information into WMI documentation?
As far as i know, the ACPI notify handlers can be scheduled concurrently on
all CPUs,
see https://lore.kernel.org/all/7617703.EvYhyI6sBW@kreacher/ for details.
Hi,
I meant this from the point of view whether the same notify handler can
only have one instance active at a time because otherwise one would
need locking to protect e.g. that input injection sequence.
The same WMI notify handler can have multiple instances active at the
same time, so the input sequence indeed needs some locking.
Chao, can you please add a mutex to the struct lenovo_wmi_priv and use this to
protect the input sequence?
Thanks,
Armin Wolf
I will add a short note about this to the WMI driver guide which i plan to
upstream soon (after the EC handler stuff is finished).
Thanks.