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]>
---
v11: remove input_unregister_device.
v10: Add lenovo_wmi_remove and mutex_destroy.
v9: Add mutex for wmi notify.
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 | 126 +++++++++++++++++++++++
3 files changed, 139 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..fda936e2f37c
--- /dev/null
+++ b/drivers/platform/x86/lenovo-wmi-camera.c
@@ -0,0 +1,126 @@
+// 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/mutex.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 mutex notify_lock; /* lenovo wmi camera button notify lock */
+};
+
+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);
+ unsigned int keycode;
+ 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;
+ }
+
+ mutex_lock(&priv->notify_lock);
+
+ 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);
+
+ mutex_unlock(&priv->notify_lock);
+}
+
+static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
+{
+ struct lenovo_wmi_priv *priv;
+ int ret;
+
+ 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);
+
+ ret = input_register_device(priv->idev);
+ if (ret)
+ return ret;
+
+ mutex_init(&priv->notify_lock);
+
+ return 0;
+}
+
+static void lenovo_wmi_remove(struct wmi_device *wdev)
+{
+ struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
+
+ mutex_destroy(&priv->notify_lock);
+}
+
+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,
+ .remove = lenovo_wmi_remove,
+};
+
+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
On 3/21/24 11:47 PM, 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 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]>
> ---
> v11: remove input_unregister_device.
> v10: Add lenovo_wmi_remove and mutex_destroy.
> v9: Add mutex for wmi notify.
> 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 | 126 +++++++++++++++++++++++
> 3 files changed, 139 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..fda936e2f37c
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
> @@ -0,0 +1,126 @@
> +// 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/mutex.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 mutex notify_lock; /* lenovo wmi camera button notify lock */
> +};
> +
> +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);
> + unsigned int keycode;
> + 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;
> + }
> +
> + mutex_lock(&priv->notify_lock);
> +
> + 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);
> +
> + mutex_unlock(&priv->notify_lock);
> +}
> +
> +static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> + struct lenovo_wmi_priv *priv;
> + int ret;
> +
> + 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);
> +
> + ret = input_register_device(priv->idev);
> + if (ret)
> + return ret;
> +
> + mutex_init(&priv->notify_lock);
> +
> + return 0;
> +}
> +
> +static void lenovo_wmi_remove(struct wmi_device *wdev)
> +{
> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
> +
> + mutex_destroy(&priv->notify_lock);
Do you really need to call mutex_destroy() during the module unload?
I don't think kernel allocates any memory during mutex_init() that needs
be freed.
> +}
> +
> +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,
> + .remove = lenovo_wmi_remove,
> +};
> +
> +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");
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
On Fri, 22 Mar 2024, Kuppuswamy Sathyanarayanan wrote:
> On 3/21/24 11:47 PM, 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 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]>
> > ---
> > v11: remove input_unregister_device.
> > v10: Add lenovo_wmi_remove and mutex_destroy.
> > v9: Add mutex for wmi notify.
> > 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 | 126 +++++++++++++++++++++++
> > 3 files changed, 139 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..fda936e2f37c
> > --- /dev/null
> > +++ b/drivers/platform/x86/lenovo-wmi-camera.c
> > @@ -0,0 +1,126 @@
> > +// 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/mutex.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 mutex notify_lock; /* lenovo wmi camera button notify lock */
> > +};
> > +
> > +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);
> > + unsigned int keycode;
> > + 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;
> > + }
> > +
> > + mutex_lock(&priv->notify_lock);
> > +
> > + 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);
> > +
> > + mutex_unlock(&priv->notify_lock);
> > +}
> > +
> > +static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
> > +{
> > + struct lenovo_wmi_priv *priv;
> > + int ret;
> > +
> > + 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);
> > +
> > + ret = input_register_device(priv->idev);
> > + if (ret)
> > + return ret;
> > +
> > + mutex_init(&priv->notify_lock);
> > +
> > + return 0;
> > +}
> > +
> > +static void lenovo_wmi_remove(struct wmi_device *wdev)
> > +{
> > + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
> > +
> > + mutex_destroy(&priv->notify_lock);
>
> Do you really need to call mutex_destroy() during the module unload?
>
> I don't think kernel allocates any memory during mutex_init() that needs
> be freed.
Is all debug code going to be happy if it's not called?
--
i.
Am 22.03.24 um 17:47 schrieb Kuppuswamy Sathyanarayanan:
> On 3/22/24 7:39 AM, Ilpo Järvinen wrote:
>> On Fri, 22 Mar 2024, Kuppuswamy Sathyanarayanan wrote:
>>> On 3/21/24 11:47 PM, 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 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]>
>>>> ---
>>>> v11: remove input_unregister_device.
>>>> v10: Add lenovo_wmi_remove and mutex_destroy.
>>>> v9: Add mutex for wmi notify.
>>>> 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 | 126 +++++++++++++++++++++++
>>>> 3 files changed, 139 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..fda936e2f37c
>>>> --- /dev/null
>>>> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
>>>> @@ -0,0 +1,126 @@
>>>> +// 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/mutex.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 mutex notify_lock; /* lenovo wmi camera button notify lock */
>>>> +};
>>>> +
>>>> +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);
>>>> + unsigned int keycode;
>>>> + 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;
>>>> + }
>>>> +
>>>> + mutex_lock(&priv->notify_lock);
>>>> +
>>>> + 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);
>>>> +
>>>> + mutex_unlock(&priv->notify_lock);
>>>> +}
>>>> +
>>>> +static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
>>>> +{
>>>> + struct lenovo_wmi_priv *priv;
>>>> + int ret;
>>>> +
>>>> + 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);
>>>> +
>>>> + ret = input_register_device(priv->idev);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + mutex_init(&priv->notify_lock);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void lenovo_wmi_remove(struct wmi_device *wdev)
>>>> +{
>>>> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
>>>> +
>>>> + mutex_destroy(&priv->notify_lock);
>>> Do you really need to call mutex_destroy() during the module unload?
>>>
>>> I don't think kernel allocates any memory during mutex_init() that needs
>>> be freed.
>> Is all debug code going to be happy if it's not called?
>>
> I am not aware of any issue. Do you have any details about it?
>
> From the comments, it looks like mutex_destroy() is used to mark a
> mutex unusable. Not sure why we need to mark a device priv lock
> unusable during the unload process.
Hi,
AFAIK calling mutex_destroy() allows the lock debugging code to catch
attempts to access the mutex afterwards, so this would allow us to spot
such issues when enabling lock debugging.
For the whole driver:
Reviewed-by: Armin Wolf <[email protected]>
On 3/22/24 7:39 AM, Ilpo Järvinen wrote:
> On Fri, 22 Mar 2024, Kuppuswamy Sathyanarayanan wrote:
>> On 3/21/24 11:47 PM, 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 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]>
>>> ---
>>> v11: remove input_unregister_device.
>>> v10: Add lenovo_wmi_remove and mutex_destroy.
>>> v9: Add mutex for wmi notify.
>>> 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 | 126 +++++++++++++++++++++++
>>> 3 files changed, 139 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..fda936e2f37c
>>> --- /dev/null
>>> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
>>> @@ -0,0 +1,126 @@
>>> +// 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/mutex.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 mutex notify_lock; /* lenovo wmi camera button notify lock */
>>> +};
>>> +
>>> +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);
>>> + unsigned int keycode;
>>> + 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;
>>> + }
>>> +
>>> + mutex_lock(&priv->notify_lock);
>>> +
>>> + 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);
>>> +
>>> + mutex_unlock(&priv->notify_lock);
>>> +}
>>> +
>>> +static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
>>> +{
>>> + struct lenovo_wmi_priv *priv;
>>> + int ret;
>>> +
>>> + 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);
>>> +
>>> + ret = input_register_device(priv->idev);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + mutex_init(&priv->notify_lock);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void lenovo_wmi_remove(struct wmi_device *wdev)
>>> +{
>>> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
>>> +
>>> + mutex_destroy(&priv->notify_lock);
>> Do you really need to call mutex_destroy() during the module unload?
>>
>> I don't think kernel allocates any memory during mutex_init() that needs
>> be freed.
> Is all debug code going to be happy if it's not called?
>
I am not aware of any issue. Do you have any details about it?
From the comments, it looks like mutex_destroy() is used to mark a
mutex unusable. Not sure why we need to mark a device priv lock
unusable during the unload process.
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Hi,
On 3/22/24 7:47 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
> 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]>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> v11: remove input_unregister_device.
> v10: Add lenovo_wmi_remove and mutex_destroy.
> v9: Add mutex for wmi notify.
> 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 | 126 +++++++++++++++++++++++
> 3 files changed, 139 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..fda936e2f37c
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
> @@ -0,0 +1,126 @@
> +// 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/mutex.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 mutex notify_lock; /* lenovo wmi camera button notify lock */
> +};
> +
> +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);
> + unsigned int keycode;
> + 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;
> + }
> +
> + mutex_lock(&priv->notify_lock);
> +
> + 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);
> +
> + mutex_unlock(&priv->notify_lock);
> +}
> +
> +static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> + struct lenovo_wmi_priv *priv;
> + int ret;
> +
> + 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);
> +
> + ret = input_register_device(priv->idev);
> + if (ret)
> + return ret;
> +
> + mutex_init(&priv->notify_lock);
> +
> + return 0;
> +}
> +
> +static void lenovo_wmi_remove(struct wmi_device *wdev)
> +{
> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
> +
> + mutex_destroy(&priv->notify_lock);
> +}
> +
> +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,
> + .remove = lenovo_wmi_remove,
> +};
> +
> +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");
On Fri, Mar 22, 2024 at 02:47:50PM +0800, Ai Chao wrote:
> Add lenovo generic wmi driver to support camera button.
WMI
> 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.
> +config LENOVO_WMI_CAMERA
> + tristate "Lenovo WMI Camera Button driver"
> + depends on ACPI_WMI
> + depends on INPUT
No COMPILE_TEST?
> + 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.
..
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
+ types.h
> +#include <linux/wmi.h>
..
> +struct lenovo_wmi_priv {
> + struct input_dev *idev;
> + struct mutex notify_lock; /* lenovo wmi camera button notify lock */
WMI
> +};
..
> + /* obj->buffer.pointer[0] is camera mode:
> + * 0 camera close
> + * 1 camera open
> + */
/*
* The correct multi-line comment style
* is depicted here.
*/
..
> + keycode = (camera_mode == SW_CAMERA_ON ?
> + KEY_CAMERA_ACCESS_ENABLE : KEY_CAMERA_ACCESS_DISABLE);
Useless parentheses.
..
> + ret = input_register_device(priv->idev);
> + if (ret)
> + return ret;
> + mutex_init(&priv->notify_lock);
Your mutex should be initialized before use. Have you tested that?
..
> +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,
> + .remove = lenovo_wmi_remove,
> +};
> +
Unneeded blank line.
> +module_wmi_driver(lenovo_wmi_driver);
..
> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_id_table);
Please, move it closer to the respective table.
--
With Best Regards,
Andy Shevchenko
Am 25.03.24 um 17:29 schrieb Andy Shevchenko:
> On Fri, Mar 22, 2024 at 02:47:50PM +0800, Ai Chao wrote:
>> Add lenovo generic wmi driver to support camera button.
> WMI
>
>> 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.
>> +config LENOVO_WMI_CAMERA
>> + tristate "Lenovo WMI Camera Button driver"
>> + depends on ACPI_WMI
>> + depends on INPUT
> No COMPILE_TEST?
>
>> + 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.
> ...
>
>> +#include <linux/acpi.h>
>> +#include <linux/device.h>
>> +#include <linux/input.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
> + types.h
>
>> +#include <linux/wmi.h>
> ...
>
>> +struct lenovo_wmi_priv {
>> + struct input_dev *idev;
>> + struct mutex notify_lock; /* lenovo wmi camera button notify lock */
> WMI
>
>> +};
> ...
>
>> + /* obj->buffer.pointer[0] is camera mode:
>> + * 0 camera close
>> + * 1 camera open
>> + */
> /*
> * The correct multi-line comment style
> * is depicted here.
> */
>
> ...
>
>> + keycode = (camera_mode == SW_CAMERA_ON ?
>> + KEY_CAMERA_ACCESS_ENABLE : KEY_CAMERA_ACCESS_DISABLE);
> Useless parentheses.
>
> ...
>
>> + ret = input_register_device(priv->idev);
>> + if (ret)
>> + return ret;
>> + mutex_init(&priv->notify_lock);
> Your mutex should be initialized before use. Have you tested that?
Hi,
i suggested that the mutex be initialized after calling input_register_device().
The reason for this is that the mutex is only used inside the WMI notify callback,
and the WMI driver core will only call it after probe() has returned.
So imho it should be safe.
Thanks,
Armin Wolf
>
> ...
>
>> +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,
>> + .remove = lenovo_wmi_remove,
>> +};
>> +
> Unneeded blank line.
>
>> +module_wmi_driver(lenovo_wmi_driver);
> ...
>
>> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_id_table);
> Please, move it closer to the respective table.
>
Hi Ai Chao,
On 3/26/24 3:54 AM, 艾超 wrote:
> Hi
>
>
>
> WMI
>
>> > 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.
>
>> > +config LENOVO_WMI_CAMERA
>> > + tristate "Lenovo WMI Camera Button driver"
>> > + depends on ACPI_WMI
>> > + depends on INPUT
>
>> No COMPILE_TEST?
>
>
>
> I compile this driver and used Evtest tool to test it on lenovo A70.
>
>
> ...
>
>> > + /* obj->buffer.pointer[0] is camera mode:
>> > + * 0 camera close
>> > + * 1 camera open
>> > + */
>
>> /*
>> * The correct multi-line comment style
>> * is depicted here.
>> */
>
>
>
> Thanks, I will modify it.
> ...
>
>> > + keycode = (camera_mode == SW_CAMERA_ON ?
>> > + KEY_CAMERA_ACCESS_ENABLE : KEY_CAMERA_ACCESS_DISABLE);
>
>> Useless parentheses.
>
>
>
> I think the parentheses is a good programming style and beneficial for reading.
>
>
>
> ...
>
>> > + ret = input_register_device(priv->idev);
>> > + if (ret)
>> > + return ret;
>
>> > + mutex_init(&priv->notify_lock);
>
>> Your mutex should be initialized before use. Have you tested that?
>
>
>
> Yes, I tested it.
>
>
> ...
>
>> > +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,
>> > + .remove = lenovo_wmi_remove,
>> > +};
>> > +
>
>> Unneeded blank line.
>
>
>
> Thanks, I will modify it.
>
>
>> > +module_wmi_driver(lenovo_wmi_driver);
>
> ...
>
>> > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_id_table);
>
>> Please, move it closer to the respective table.
>
>
>
> Thanks, I will modify it.
I have already merged this. I'll squash in fixes for the few
small code style remarks from Andy, so there is no need
to send a new version.
Regards,
Hans
On Wed, Mar 27, 2024 at 11:54:55AM +0100, Hans de Goede wrote:
> On 3/26/24 3:54 AM, 艾超 wrote:
..
> >> > +config LENOVO_WMI_CAMERA
> >> > + tristate "Lenovo WMI Camera Button driver"
> >> > + depends on ACPI_WMI
> >> > + depends on INPUT
> >
> >> No COMPILE_TEST?
> >
> > I compile this driver and used Evtest tool to test it on lenovo A70.
What I meant here is to add respective COMPILE_TEST to the dependency(ies).
--
With Best Regards,
Andy Shevchenko
Hi,
On 3/27/24 2:14 PM, Andy Shevchenko wrote:
> On Wed, Mar 27, 2024 at 11:54:55AM +0100, Hans de Goede wrote:
>> On 3/26/24 3:54 AM, 艾超 wrote:
>
> ...
>
>>>>> +config LENOVO_WMI_CAMERA
>>>>> + tristate "Lenovo WMI Camera Button driver"
>>>>> + depends on ACPI_WMI
>>>>> + depends on INPUT
>>>
>>>> No COMPILE_TEST?
>>>
>>> I compile this driver and used Evtest tool to test it on lenovo A70.
>
> What I meant here is to add respective COMPILE_TEST to the dependency(ies).
Neither include/linux/input.h nor include/linux/wmi.h offer
stubs when disabled so this will not work.
Besides x86 has a lot of compile test coverage in general,
so I don't see much value in adding || COMPILE_TEST to
dependencies.
Regards,
Hans
On Wed, Mar 27, 2024 at 09:03:38PM +0100, Hans de Goede wrote:
> On 3/27/24 2:14 PM, Andy Shevchenko wrote:
> > On Wed, Mar 27, 2024 at 11:54:55AM +0100, Hans de Goede wrote:
> >> On 3/26/24 3:54 AM, 艾超 wrote:
..
> >>>>> +config LENOVO_WMI_CAMERA
> >>>>> + tristate "Lenovo WMI Camera Button driver"
> >>>>> + depends on ACPI_WMI
> >>>>> + depends on INPUT
> >>>
> >>>> No COMPILE_TEST?
> >>>
> >>> I compile this driver and used Evtest tool to test it on lenovo A70.
> >
> > What I meant here is to add respective COMPILE_TEST to the dependency(ies).
>
> Neither include/linux/input.h nor include/linux/wmi.h offer
> stubs when disabled so this will not work.
Oh, I didn't realize that!
> Besides x86 has a lot of compile test coverage in general,
> so I don't see much value in adding || COMPILE_TEST to
> dependencies.
Agree.
--
With Best Regards,
Andy Shevchenko