[v11] platform/x86: add lenovo wmi camera button driver

[PATCH v11] platform/x86: add lenovo wmi camera button driver

Posted by Ai Chao 1 year, 9 months ago

Add lenovo generic wmi driver to support camera button.
The Camera button is a GPIO device. This driver receives ACPI notifyi
when the camera button is switched on/off. This driver is used in
Lenovo A70, it is a Computer integrated machine.

Signed-off-by: Ai Chao <aichao@kylinos.cn>
---
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 <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/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 <aichao@kylinos.cn>");
+MODULE_DESCRIPTION("Lenovo WMI Camera Button Driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH v11] platform/x86: add lenovo wmi camera button driver

Posted by Andy Shevchenko 1 year, 8 months ago

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

Re: [PATCH v11] platform/x86: add lenovo wmi camera button driver

Posted by Armin Wolf 1 year, 8 months ago

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

Re: [PATCH v11] platform/x86: add lenovo wmi camera button driver

Posted by Hans de Goede 1 year, 8 months ago

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 <aichao@kylinos.cn>

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 <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/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 <aichao@kylinos.cn>");
> +MODULE_DESCRIPTION("Lenovo WMI Camera Button Driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH v11] platform/x86: add lenovo wmi camera button driver

Posted by Kuppuswamy Sathyanarayanan 1 year, 9 months ago

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 <aichao@kylinos.cn>
> ---
> 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 <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/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 <aichao@kylinos.cn>");
> +MODULE_DESCRIPTION("Lenovo WMI Camera Button Driver");
> +MODULE_LICENSE("GPL");

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH v11] platform/x86: add lenovo wmi camera button driver

Posted by Ilpo Järvinen 1 year, 9 months ago

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 <aichao@kylinos.cn>
> > ---
> > 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 <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/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.

Re: [PATCH v11] platform/x86: add lenovo wmi camera button driver

Posted by Kuppuswamy Sathyanarayanan 1 year, 9 months ago

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 <aichao@kylinos.cn>
>>> ---
>>> 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 <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/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

Re: [PATCH v11] platform/x86: add lenovo wmi camera button driver

Posted by Armin Wolf 1 year, 9 months ago

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 <aichao@kylinos.cn>
>>>> ---
>>>> 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 <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/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 <W_Armin@gmx.de>