* [PATCH v4] platform/x86: add lenovo generic wmi driver
@ 2024-03-05 12:13 Ai Chao
2024-03-05 14:12 ` Kuppuswamy Sathyanarayanan
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Ai Chao @ 2024-03-05 12:13 UTC (permalink / raw
To: hdegoede, ilpo.jarvinen, linux-kernel, platform-driver-x86; +Cc: 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 button pressed.
Signed-off-by: Ai Chao <aichao@kylinos.cn>
---
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 | 118 +++++++++++++++++++++++
3 files changed, 131 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..079f5aa5910c 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
+ button pressed.
+
+ 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..77084266829c
--- /dev/null
+++ b/drivers/platform/x86/lenovo-wmi-camera.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Lenovo WMI Camera Button Driver
+ *
+ * Author: Ai Chao <aichao@kylinos.cn>
+ * 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;
+ struct device *dev;
+ u8 camera_mode;
+};
+
+enum {
+ CAMERA_BUTTON_PRESSED = 1,
+};
+
+static ssize_t camerabutton_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct lenovo_wmi_priv *priv = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%u\n", priv->camera_mode);
+}
+DEVICE_ATTR_RO(camerabutton);
+
+static struct attribute *lenovo_wmi_attrs[] = {
+ &dev_attr_camerabutton.attr,
+ NULL,
+};
+
+static const struct attribute_group lenovo_wmi_group = {
+ .attrs = lenovo_wmi_attrs,
+};
+
+const struct attribute_group *lenovo_wmi_groups[] = {
+ &lenovo_wmi_group,
+ NULL,
+};
+
+static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
+{
+ struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
+
+ if (obj->type == ACPI_TYPE_BUFFER &&
+ obj->buffer.pointer[0] <= CAMERA_BUTTON_PRESSED) {
+ /* Camera mode:
+ * 0 camera close
+ * 1 camera open
+ */
+ priv->camera_mode = obj->buffer.pointer[0];
+
+ input_report_key(priv->idev, KEY_CAMERA, 1);
+ input_sync(priv->idev);
+ input_report_key(priv->idev, KEY_CAMERA, 0);
+ input_sync(priv->idev);
+ } else {
+ dev_dbg(&wdev->dev, "Bad response type %d\n", obj->type);
+ }
+}
+
+static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
+{
+ struct lenovo_wmi_priv *priv;
+
+ priv = devm_kzalloc(&wdev->dev, sizeof(*priv),
+ GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ dev_set_drvdata(&wdev->dev, priv);
+
+ priv->idev = devm_input_allocate_device(&wdev->dev);
+ if (!priv->idev)
+ return -ENOMEM;
+
+ priv->idev->name = "Lenovo WMI Camera Button";
+ priv->idev->phys = "wmi/input0";
+ priv->idev->id.bustype = BUS_HOST;
+ priv->idev->dev.parent = &wdev->dev;
+ set_bit(EV_KEY, priv->idev->evbit);
+ set_bit(KEY_CAMERA, priv->idev->keybit);
+
+ return input_register_device(priv->idev);
+}
+
+static const struct wmi_device_id lenovo_wmi_id_table[] = {
+ { .guid_string = WMI_LENOVO_CAMERABUTTON_EVENT_GUID },
+ { }
+};
+
+static struct wmi_driver lenovo_wmi_driver = {
+ .driver = {
+ .name = "lenovo-wmi-camera",
+ .dev_groups = lenovo_wmi_groups,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .id_table = lenovo_wmi_id_table,
+ .no_singleton = false,
+ .probe = lenovo_wmi_probe,
+ .notify = lenovo_wmi_notify,
+};
+
+module_wmi_driver(lenovo_wmi_driver);
+
+MODULE_DEVICE_TABLE(wmi, lenovo_wmi_id_table);
+MODULE_AUTHOR("Ai Chao <aichao@kylinos.cn>");
+MODULE_DESCRIPTION("Lenovo Generic WMI Driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4] platform/x86: add lenovo generic wmi driver
2024-03-05 12:13 [PATCH v4] platform/x86: add lenovo generic wmi driver Ai Chao
@ 2024-03-05 14:12 ` Kuppuswamy Sathyanarayanan
2024-03-05 14:29 ` Gergo Koteles
2024-03-05 14:48 ` Thomas Weißschuh
2 siblings, 0 replies; 7+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-03-05 14:12 UTC (permalink / raw
To: Ai Chao, hdegoede, ilpo.jarvinen, linux-kernel,
platform-driver-x86
On 3/5/24 4:13 AM, Ai Chao wrote:
> Add lenovo generic wmi driver to support camera button.
> The Camera button is a GPIO device. This driver receives ACPI notifyi
/s/notifyi/notification/
> when the button pressed.
>
> Signed-off-by: Ai Chao <aichao@kylinos.cn>
> ---
Other than this, it looks fine to me.
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 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 | 118 +++++++++++++++++++++++
> 3 files changed, 131 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..079f5aa5910c 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
> + button pressed.
> +
> + 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..77084266829c
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Lenovo WMI Camera Button Driver
> + *
> + * Author: Ai Chao <aichao@kylinos.cn>
> + * 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;
> + struct device *dev;
> + u8 camera_mode;
> +};
> +
> +enum {
> + CAMERA_BUTTON_PRESSED = 1,
> +};
> +
> +static ssize_t camerabutton_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct lenovo_wmi_priv *priv = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%u\n", priv->camera_mode);
> +}
> +DEVICE_ATTR_RO(camerabutton);
> +
> +static struct attribute *lenovo_wmi_attrs[] = {
> + &dev_attr_camerabutton.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group lenovo_wmi_group = {
> + .attrs = lenovo_wmi_attrs,
> +};
> +
> +const struct attribute_group *lenovo_wmi_groups[] = {
> + &lenovo_wmi_group,
> + NULL,
> +};
> +
> +static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
> +{
> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
> +
> + if (obj->type == ACPI_TYPE_BUFFER &&
> + obj->buffer.pointer[0] <= CAMERA_BUTTON_PRESSED) {
> + /* Camera mode:
> + * 0 camera close
> + * 1 camera open
> + */
> + priv->camera_mode = obj->buffer.pointer[0];
> +
> + input_report_key(priv->idev, KEY_CAMERA, 1);
> + input_sync(priv->idev);
> + input_report_key(priv->idev, KEY_CAMERA, 0);
> + input_sync(priv->idev);
> + } else {
> + dev_dbg(&wdev->dev, "Bad response type %d\n", obj->type);
> + }
> +}
> +
> +static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> + struct lenovo_wmi_priv *priv;
> +
> + priv = devm_kzalloc(&wdev->dev, sizeof(*priv),
> + GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&wdev->dev, priv);
> +
> + priv->idev = devm_input_allocate_device(&wdev->dev);
> + if (!priv->idev)
> + return -ENOMEM;
> +
> + priv->idev->name = "Lenovo WMI Camera Button";
> + priv->idev->phys = "wmi/input0";
> + priv->idev->id.bustype = BUS_HOST;
> + priv->idev->dev.parent = &wdev->dev;
> + set_bit(EV_KEY, priv->idev->evbit);
> + set_bit(KEY_CAMERA, priv->idev->keybit);
> +
> + return input_register_device(priv->idev);
> +}
> +
> +static const struct wmi_device_id lenovo_wmi_id_table[] = {
> + { .guid_string = WMI_LENOVO_CAMERABUTTON_EVENT_GUID },
> + { }
> +};
> +
> +static struct wmi_driver lenovo_wmi_driver = {
> + .driver = {
> + .name = "lenovo-wmi-camera",
> + .dev_groups = lenovo_wmi_groups,
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> + .id_table = lenovo_wmi_id_table,
> + .no_singleton = false,
> + .probe = lenovo_wmi_probe,
> + .notify = lenovo_wmi_notify,
> +};
> +
> +module_wmi_driver(lenovo_wmi_driver);
> +
> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_id_table);
> +MODULE_AUTHOR("Ai Chao <aichao@kylinos.cn>");
> +MODULE_DESCRIPTION("Lenovo Generic WMI Driver");
> +MODULE_LICENSE("GPL");
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] platform/x86: add lenovo generic wmi driver
2024-03-05 12:13 [PATCH v4] platform/x86: add lenovo generic wmi driver Ai Chao
2024-03-05 14:12 ` Kuppuswamy Sathyanarayanan
@ 2024-03-05 14:29 ` Gergo Koteles
2024-03-05 14:48 ` Thomas Weißschuh
2 siblings, 0 replies; 7+ messages in thread
From: Gergo Koteles @ 2024-03-05 14:29 UTC (permalink / raw
To: Ai Chao, hdegoede, ilpo.jarvinen, linux-kernel,
platform-driver-x86
Hi Ai,
On Tue, 2024-03-05 at 20:13 +0800, Ai Chao wrote:
>
> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_id_table);
> +MODULE_AUTHOR("Ai Chao <aichao@kylinos.cn>");
> +MODULE_DESCRIPTION("Lenovo Generic WMI Driver");
This is forgotten.
It should be "Lenovo WMI Camera Button driver"
> +MODULE_LICENSE("GPL");
Thanks,
Gergo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] platform/x86: add lenovo generic wmi driver
2024-03-05 12:13 [PATCH v4] platform/x86: add lenovo generic wmi driver Ai Chao
2024-03-05 14:12 ` Kuppuswamy Sathyanarayanan
2024-03-05 14:29 ` Gergo Koteles
@ 2024-03-05 14:48 ` Thomas Weißschuh
2024-03-05 15:35 ` Armin Wolf
2 siblings, 1 reply; 7+ messages in thread
From: Thomas Weißschuh @ 2024-03-05 14:48 UTC (permalink / raw
To: Ai Chao; +Cc: hdegoede, ilpo.jarvinen, linux-kernel, platform-driver-x86
On Tue, Mar 05, 2024 at 08:13:15PM +0800, Ai Chao wrote:
> Add lenovo generic wmi driver to support camera button.
> The Camera button is a GPIO device. This driver receives ACPI notifyi
> when the button pressed.
the button *is* pressed.
The subject does not mention that it is only about te camera button.
I would be interested on which devices this driver was tested and is
expected to work with.
>
> Signed-off-by: Ai Chao <aichao@kylinos.cn>
> ---
> 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 | 118 +++++++++++++++++++++++
> 3 files changed, 131 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..079f5aa5910c 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
> + button pressed.
> +
> + 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..77084266829c
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Lenovo WMI Camera Button Driver
> + *
> + * Author: Ai Chao <aichao@kylinos.cn>
> + * 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;
> + struct device *dev;
> + u8 camera_mode;
> +};
> +
> +enum {
> + CAMERA_BUTTON_PRESSED = 1,
> +};
> +
> +static ssize_t camerabutton_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct lenovo_wmi_priv *priv = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%u\n", priv->camera_mode);
> +}
> +DEVICE_ATTR_RO(camerabutton);
> +
> +static struct attribute *lenovo_wmi_attrs[] = {
> + &dev_attr_camerabutton.attr,
> + NULL,
No trailing comma after sentinel elements.
> +};
> +
> +static const struct attribute_group lenovo_wmi_group = {
> + .attrs = lenovo_wmi_attrs,
> +};
> +
> +const struct attribute_group *lenovo_wmi_groups[] = {
> + &lenovo_wmi_group,
> + NULL,
Also no trailing comma.
> +};
> +
> +static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
> +{
> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
> +
> + if (obj->type == ACPI_TYPE_BUFFER &&
> + obj->buffer.pointer[0] <= CAMERA_BUTTON_PRESSED) {
> + /* Camera mode:
> + * 0 camera close
> + * 1 camera open
> + */
> + priv->camera_mode = obj->buffer.pointer[0];
This looks similar to a switch.
Would it be more useful for the user to report a standard switch instead
of a key event which needs to be correlated with the sysfs file?
> +
> + input_report_key(priv->idev, KEY_CAMERA, 1);
> + input_sync(priv->idev);
> + input_report_key(priv->idev, KEY_CAMERA, 0);
> + input_sync(priv->idev);
> + } else {
> + dev_dbg(&wdev->dev, "Bad response type %d\n", obj->type);
> + }
> +}
> +
> +static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> + struct lenovo_wmi_priv *priv;
> +
> + priv = devm_kzalloc(&wdev->dev, sizeof(*priv),
> + GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&wdev->dev, priv);
> +
> + priv->idev = devm_input_allocate_device(&wdev->dev);
> + if (!priv->idev)
> + return -ENOMEM;
> +
> + priv->idev->name = "Lenovo WMI Camera Button";
> + priv->idev->phys = "wmi/input0";
> + priv->idev->id.bustype = BUS_HOST;
> + priv->idev->dev.parent = &wdev->dev;
> + set_bit(EV_KEY, priv->idev->evbit);
> + set_bit(KEY_CAMERA, priv->idev->keybit);
input_set_capability()?
> +
> + return input_register_device(priv->idev);
> +}
> +
> +static const struct wmi_device_id lenovo_wmi_id_table[] = {
> + { .guid_string = WMI_LENOVO_CAMERABUTTON_EVENT_GUID },
> + { }
> +};
> +
> +static struct wmi_driver lenovo_wmi_driver = {
> + .driver = {
> + .name = "lenovo-wmi-camera",
> + .dev_groups = lenovo_wmi_groups,
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> + .id_table = lenovo_wmi_id_table,
> + .no_singleton = false,
> + .probe = lenovo_wmi_probe,
> + .notify = lenovo_wmi_notify,
> +};
> +
> +module_wmi_driver(lenovo_wmi_driver);
> +
> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_id_table);
> +MODULE_AUTHOR("Ai Chao <aichao@kylinos.cn>");
> +MODULE_DESCRIPTION("Lenovo Generic WMI Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] platform/x86: add lenovo generic wmi driver
2024-03-05 14:48 ` Thomas Weißschuh
@ 2024-03-05 15:35 ` Armin Wolf
0 siblings, 0 replies; 7+ messages in thread
From: Armin Wolf @ 2024-03-05 15:35 UTC (permalink / raw
To: Thomas Weißschuh, Ai Chao
Cc: hdegoede, ilpo.jarvinen, linux-kernel, platform-driver-x86
Am 05.03.24 um 15:48 schrieb Thomas Weißschuh:
> On Tue, Mar 05, 2024 at 08:13:15PM +0800, Ai Chao wrote:
>> Add lenovo generic wmi driver to support camera button.
>> The Camera button is a GPIO device. This driver receives ACPI notifyi
>> when the button pressed.
> the button *is* pressed.
>
>
> The subject does not mention that it is only about te camera button.
>
> I would be interested on which devices this driver was tested and is
> expected to work with.
>
>> Signed-off-by: Ai Chao <aichao@kylinos.cn>
>> ---
>> 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 | 118 +++++++++++++++++++++++
>> 3 files changed, 131 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..079f5aa5910c 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
>> + button pressed.
>> +
>> + 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..77084266829c
>> --- /dev/null
>> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
>> @@ -0,0 +1,118 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Lenovo WMI Camera Button Driver
>> + *
>> + * Author: Ai Chao <aichao@kylinos.cn>
>> + * 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;
>> + struct device *dev;
>> + u8 camera_mode;
>> +};
>> +
>> +enum {
>> + CAMERA_BUTTON_PRESSED = 1,
>> +};
>> +
>> +static ssize_t camerabutton_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct lenovo_wmi_priv *priv = dev_get_drvdata(dev);
>> +
>> + return sysfs_emit(buf, "%u\n", priv->camera_mode);
>> +}
>> +DEVICE_ATTR_RO(camerabutton);
>> +
>> +static struct attribute *lenovo_wmi_attrs[] = {
>> + &dev_attr_camerabutton.attr,
>> + NULL,
> No trailing comma after sentinel elements.
>
>> +};
>> +
>> +static const struct attribute_group lenovo_wmi_group = {
>> + .attrs = lenovo_wmi_attrs,
>> +};
>> +
>> +const struct attribute_group *lenovo_wmi_groups[] = {
>> + &lenovo_wmi_group,
>> + NULL,
> Also no trailing comma.
>
>> +};
>> +
>> +static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
>> +{
>> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
>> +
>> + if (obj->type == ACPI_TYPE_BUFFER &&
>> + obj->buffer.pointer[0] <= CAMERA_BUTTON_PRESSED) {
>> + /* Camera mode:
>> + * 0 camera close
>> + * 1 camera open
>> + */
>> + priv->camera_mode = obj->buffer.pointer[0];
> This looks similar to a switch.
> Would it be more useful for the user to report a standard switch instead
> of a key event which needs to be correlated with the sysfs file?
I agree, maybe SW_CAMERA_LENS_COVER might be the right thing to use here,
if those camera states (open/closed) are meant to symbolize camera shutter states.
In such a case the initial switch state has to be retrieved, or else the input device
cannot be registered until the first event is received (similar how the hp-wmi driver
handles SW_CAMERA_LENS_COVER events).
Ai Chao, can you tell us if those two camera states are meant to act like a switch (camera switched off,
camera switched on) or meant to act like a key (camera button pressed, camera button released)?
>> +
>> + input_report_key(priv->idev, KEY_CAMERA, 1);
>> + input_sync(priv->idev);
>> + input_report_key(priv->idev, KEY_CAMERA, 0);
>> + input_sync(priv->idev);
>> + } else {
>> + dev_dbg(&wdev->dev, "Bad response type %d\n", obj->type);
>> + }
>> +}
>> +
>> +static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
>> +{
>> + struct lenovo_wmi_priv *priv;
>> +
>> + priv = devm_kzalloc(&wdev->dev, sizeof(*priv),
>> + GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + dev_set_drvdata(&wdev->dev, priv);
>> +
>> + priv->idev = devm_input_allocate_device(&wdev->dev);
>> + if (!priv->idev)
>> + return -ENOMEM;
>> +
>> + priv->idev->name = "Lenovo WMI Camera Button";
>> + priv->idev->phys = "wmi/input0";
>> + priv->idev->id.bustype = BUS_HOST;
>> + priv->idev->dev.parent = &wdev->dev;
>> + set_bit(EV_KEY, priv->idev->evbit);
>> + set_bit(KEY_CAMERA, priv->idev->keybit);
> input_set_capability()?
>
>> +
>> + return input_register_device(priv->idev);
>> +}
>> +
>> +static const struct wmi_device_id lenovo_wmi_id_table[] = {
>> + { .guid_string = WMI_LENOVO_CAMERABUTTON_EVENT_GUID },
>> + { }
>> +};
>> +
>> +static struct wmi_driver lenovo_wmi_driver = {
>> + .driver = {
>> + .name = "lenovo-wmi-camera",
>> + .dev_groups = lenovo_wmi_groups,
>> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>> + },
>> + .id_table = lenovo_wmi_id_table,
>> + .no_singleton = false,
The correct setting in this case would be ".no_singleton = true",
since camera_mode now lives inside struct lenovo_wmi_priv.
Thanks,
Armin Wolf
>> + .probe = lenovo_wmi_probe,
>> + .notify = lenovo_wmi_notify,
>> +};
>> +
>> +module_wmi_driver(lenovo_wmi_driver);
>> +
>> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_id_table);
>> +MODULE_AUTHOR("Ai Chao <aichao@kylinos.cn>");
>> +MODULE_DESCRIPTION("Lenovo Generic WMI Driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] platform/x86: add lenovo generic wmi driver
[not found] <2o0aznm5pjb-2o0c9lfkrd4@nsmail7.0.0--kylin--1>
@ 2024-03-06 8:36 ` Thomas Weißschuh
2024-03-06 11:01 ` Hans de Goede
1 sibling, 0 replies; 7+ messages in thread
From: Thomas Weißschuh @ 2024-03-06 8:36 UTC (permalink / raw
To: 艾超
Cc: Thomas Weißschuh, W_Armin, hdegoede, ilpo.jarvinen,
linux-kernel, platform-driver-x86
On Wed, Mar 06, 2024 at 04:17:43PM +0800, 艾超 wrote:
> > I would be interested on which devices this driver was tested and is
> > expected to work with.
>
> Lenovo A70，it is a Computer integrated machine.
Could you mention this in your next commit message?
Do you have any idea on which other devices this should work?
> >> This looks similar to a switch.
> >> Would it be more useful for the user to report a standard switch
> instead
> >> of a key event which needs to be correlated with the sysfs file?
> > I agree, maybe SW_CAMERA_LENS_COVER might be the right thing to use
> here,
> > if those camera states (open/closed) are meant to symbolize camera
> shutter states.
> > In such a case the initial switch state has to be retrieved, or else
> the input device
> > cannot be registered until the first event is received (similar how
> the hp-wmi driver
> > handles SW_CAMERA_LENS_COVER events).
> > Ai Chao, can you tell us if those two camera states are meant to act
> like a switch (camera switched off,
> > camera switched on) or meant to act like a key (camera button pressed,
> camera button released)?
>
> The camera button is like a switch. I can used SW_CAMERA_LENS_COVER to
> report input event , but the OS
>
> can't to show camera OSD. OS need a key value map to the camera OSD.
Why can't the OS do this?
The switch should report its value.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] platform/x86: add lenovo generic wmi driver
[not found] <2o0aznm5pjb-2o0c9lfkrd4@nsmail7.0.0--kylin--1>
2024-03-06 8:36 ` Thomas Weißschuh
@ 2024-03-06 11:01 ` Hans de Goede
1 sibling, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2024-03-06 11:01 UTC (permalink / raw
To: 艾超, Thomas Weißschuh, W_Armin
Cc: ilpo.jarvinen, linux-kernel, platform-driver-x86
Hi,
On 3/6/24 09:17, 艾超 wrote:
> Hi
>
>
>
>> I would be interested on which devices this driver was tested and is
>> expected to work with.
>
>
>
> Lenovo A70，it is a Computer integrated machine.
>
>
>>> This looks similar to a switch.
>>> Would it be more useful for the user to report a standard switch instead
>>> of a key event which needs to be correlated with the sysfs file?
>
>> I agree, maybe SW_CAMERA_LENS_COVER might be the right thing to use here,
>> if those camera states (open/closed) are meant to symbolize camera shutter states.
>
>> In such a case the initial switch state has to be retrieved, or else the input device
>> cannot be registered until the first event is received (similar how the hp-wmi driver
>> handles SW_CAMERA_LENS_COVER events).
>
>> Ai Chao, can you tell us if those two camera states are meant to act like a switch (camera switched off,
>> camera switched on) or meant to act like a key (camera button pressed, camera button released)?
>
>
>
> The camera button is like a switch. I can used SW_CAMERA_LENS_COVER to report input event , but the OS
>
> can't to show camera OSD. OS need a key value map to the camera OSD.
Please use SW_CAMERA_LENS_COVER in the next version, that really is the correct
thing to do here, so that you not only report the event of the camera state
changing, but also the actual camera state (on/off) to userspace.
This will also allow you to remove the custom sysfs atribute since you
are now correctly reporting the value to userspace through the evdev event.
As for userspace not responding to SW_CAMERA_LENS_COVER, this is because
SW_CAMERA_LENS_COVER is relatively new and we need to add support to
userspace for this. SW_CAMERA_LENS_COVER is already used on whole a bunch of
Dell laptops for this, so we need to do this anyways.
Please file an issue against GNOME, lets say against gnome-settings-daemon
for this.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-06 11:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-05 12:13 [PATCH v4] platform/x86: add lenovo generic wmi driver Ai Chao
2024-03-05 14:12 ` Kuppuswamy Sathyanarayanan
2024-03-05 14:29 ` Gergo Koteles
2024-03-05 14:48 ` Thomas Weißschuh
2024-03-05 15:35 ` Armin Wolf
[not found] <2o0aznm5pjb-2o0c9lfkrd4@nsmail7.0.0--kylin--1>
2024-03-06 8:36 ` Thomas Weißschuh
2024-03-06 11:01 ` Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).