2024-03-14 05:03:50

by Ai Chao

[permalink] [raw]
Subject: [PATCH v8] platform/x86: add lenovo wmi camera button driver

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 <[email protected]>
---
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 <[email protected]>
+ * 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);
+ }
+ input_sync(priv->idev);
+}
+
+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;
+ input_set_capability(priv->idev, EV_KEY, KEY_CAMERA_ACCESS_ENABLE);
+ input_set_capability(priv->idev, EV_KEY, KEY_CAMERA_ACCESS_DISABLE);
+
+ 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",
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .id_table = lenovo_wmi_id_table,
+ .no_singleton = true,
+ .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 <[email protected]>");
+MODULE_DESCRIPTION("Lenovo WMI Camera Button Driver");
+MODULE_LICENSE("GPL");
--
2.25.1



2024-03-14 17:22:10

by Armin Wolf

[permalink] [raw]
Subject: Re: [PATCH v8] platform/x86: add lenovo wmi camera button driver

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 <[email protected]>
> ---
> 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 <[email protected]>
> + * 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);
> + }
> + input_sync(priv->idev);
> +}
> +
> +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;
> + input_set_capability(priv->idev, EV_KEY, KEY_CAMERA_ACCESS_ENABLE);
> + input_set_capability(priv->idev, EV_KEY, KEY_CAMERA_ACCESS_DISABLE);
> +
> + 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",
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> + .id_table = lenovo_wmi_id_table,
> + .no_singleton = true,
> + .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 <[email protected]>");
> +MODULE_DESCRIPTION("Lenovo WMI Camera Button Driver");
> +MODULE_LICENSE("GPL");

Looks good to me.

Reviewed-by: Armin Wolf <[email protected]>


2024-03-15 11:52:14

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v8] platform/x86: add lenovo wmi camera button driver

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 <[email protected]>
> > ---
> > 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 <[email protected]>
> > + * 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?

--
i.

> > +
> > +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;
> > + input_set_capability(priv->idev, EV_KEY, KEY_CAMERA_ACCESS_ENABLE);
> > + input_set_capability(priv->idev, EV_KEY, KEY_CAMERA_ACCESS_DISABLE);
> > +
> > + 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",
> > + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> > + },
> > + .id_table = lenovo_wmi_id_table,
> > + .no_singleton = true,
> > + .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 <[email protected]>");
> > +MODULE_DESCRIPTION("Lenovo WMI Camera Button Driver");
> > +MODULE_LICENSE("GPL");
>
> Looks good to me.
>
> Reviewed-by: Armin Wolf <[email protected]>
>

2024-03-15 17:32:18

by Armin Wolf

[permalink] [raw]
Subject: Re: [PATCH v8] platform/x86: add lenovo wmi camera button driver

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 <[email protected]>
>>> ---
>>> 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 <[email protected]>
>>> + * 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.

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,
Armin Wolf


2024-03-15 18:04:38

by Armin Wolf

[permalink] [raw]
Subject: Re: [PATCH v8] platform/x86: add lenovo wmi camera button driver

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 <[email protected]>
>>>>> ---
>>>>> 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 <[email protected]>
>>>>> + * 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.
>

2024-03-15 22:15:57

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v8] platform/x86: add lenovo wmi camera button driver

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 <[email protected]>
> > > > ---
> > > > 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 <[email protected]>
> > > > + * 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.

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

--
i.