* Re: [PATCH v5] platform/x86: lenovo-wmi-camera: Use SW_CAMERA_LENS_COVER instead of KEY_CAMERA_ACESS
2025-01-01 10:29 [PATCH v5] platform/x86: lenovo-wmi-camera: Use SW_CAMERA_LENS_COVER instead of KEY_CAMERA_ACESS Ai Chao
@ 2025-01-01 12:45 ` Armin Wolf
2025-01-06 10:47 ` Hans de Goede
2025-01-07 18:09 ` Ilpo Järvinen
2 siblings, 0 replies; 4+ messages in thread
From: Armin Wolf @ 2025-01-01 12:45 UTC (permalink / raw)
To: Ai Chao, hdegoede, ilpo.jarvinen, platform-driver-x86, linux-kernel
Am 01.01.25 um 11:29 schrieb Ai Chao:
> Use SW_CAMERA_LENS_COVER instead of KEY_CAMERA_ACESS_ENABLE and
> KEY_CAMERA_ACESS_DISABLE. When the camera toggle switch was hit,
> the lenovo-wmi-camera driver would report an event code.
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Ai Chao <aichao@kylinos.cn>
> ---
> change for v5
> -Add input_report_switch before input_register_device.
> change for v4
> -Add mutex_unlock and report a switch state of 0 if SW_CAMERA_ON.
> change for v3
> -Used input_register_device and input_allocate_device.
> change for v2
> -Only delays the input-device registration and switches to reporting SW_CAMERA_LENS_COVER.
>
> drivers/platform/x86/lenovo-wmi-camera.c | 67 ++++++++++++++++--------
> 1 file changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo-wmi-camera.c b/drivers/platform/x86/lenovo-wmi-camera.c
> index 0c0bedaf7407..ceed441a0161 100644
> --- a/drivers/platform/x86/lenovo-wmi-camera.c
> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
> @@ -26,10 +26,38 @@ enum {
> SW_CAMERA_ON = 1,
> };
>
> +static int camera_shutter_input_setup(struct wmi_device *wdev, u8 camera_mode)
> +{
> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
> + int err;
> +
> + priv->idev = input_allocate_device();
> + 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_SW, SW_CAMERA_LENS_COVER);
> +
> + input_report_switch(priv->idev, SW_CAMERA_LENS_COVER,
> + camera_mode == SW_CAMERA_ON ? 0 : 1);
> + input_sync(priv->idev);
> +
> + err = input_register_device(priv->idev);
> + if (err) {
> + input_free_device(priv->idev);
> + priv->idev = NULL;
> + }
> +
> + return err;
> +}
> +
> 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) {
> @@ -55,11 +83,18 @@ static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
>
> 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);
> + if (!priv->idev) {
> + if (camera_shutter_input_setup(wdev, camera_mode))
> + dev_warn(&wdev->dev, "Failed to register input device\n");
> +
> + mutex_unlock(&priv->notify_lock);
> + return;
> + }
> +
> + if (camera_mode == SW_CAMERA_ON)
> + input_report_switch(priv->idev, SW_CAMERA_LENS_COVER, 0);
> + else
> + input_report_switch(priv->idev, SW_CAMERA_LENS_COVER, 1);
> input_sync(priv->idev);
>
> mutex_unlock(&priv->notify_lock);
> @@ -68,29 +103,12 @@ static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
> 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;
> @@ -100,6 +118,9 @@ static void lenovo_wmi_remove(struct wmi_device *wdev)
> {
> struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
>
> + if (priv->idev)
> + input_unregister_device(priv->idev);
> +
> mutex_destroy(&priv->notify_lock);
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v5] platform/x86: lenovo-wmi-camera: Use SW_CAMERA_LENS_COVER instead of KEY_CAMERA_ACESS
2025-01-01 10:29 [PATCH v5] platform/x86: lenovo-wmi-camera: Use SW_CAMERA_LENS_COVER instead of KEY_CAMERA_ACESS Ai Chao
2025-01-01 12:45 ` Armin Wolf
@ 2025-01-06 10:47 ` Hans de Goede
2025-01-07 18:09 ` Ilpo Järvinen
2 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2025-01-06 10:47 UTC (permalink / raw)
To: Ai Chao, ilpo.jarvinen, platform-driver-x86, linux-kernel
Hi,
On 1-Jan-25 11:29 AM, Ai Chao wrote:
> Use SW_CAMERA_LENS_COVER instead of KEY_CAMERA_ACESS_ENABLE and
> KEY_CAMERA_ACESS_DISABLE. When the camera toggle switch was hit,
> the lenovo-wmi-camera driver would report an event code.
>
> Signed-off-by: Ai Chao <aichao@kylinos.cn>
> ---
> change for v5
> -Add input_report_switch before input_register_device.
> change for v4
> -Add mutex_unlock and report a switch state of 0 if SW_CAMERA_ON.
> change for v3
> -Used input_register_device and input_allocate_device.
> change for v2
> -Only delays the input-device registration and switches to reporting SW_CAMERA_LENS_COVER.
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
>
> drivers/platform/x86/lenovo-wmi-camera.c | 67 ++++++++++++++++--------
> 1 file changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo-wmi-camera.c b/drivers/platform/x86/lenovo-wmi-camera.c
> index 0c0bedaf7407..ceed441a0161 100644
> --- a/drivers/platform/x86/lenovo-wmi-camera.c
> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
> @@ -26,10 +26,38 @@ enum {
> SW_CAMERA_ON = 1,
> };
>
> +static int camera_shutter_input_setup(struct wmi_device *wdev, u8 camera_mode)
> +{
> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
> + int err;
> +
> + priv->idev = input_allocate_device();
> + 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_SW, SW_CAMERA_LENS_COVER);
> +
> + input_report_switch(priv->idev, SW_CAMERA_LENS_COVER,
> + camera_mode == SW_CAMERA_ON ? 0 : 1);
> + input_sync(priv->idev);
> +
> + err = input_register_device(priv->idev);
> + if (err) {
> + input_free_device(priv->idev);
> + priv->idev = NULL;
> + }
> +
> + return err;
> +}
> +
> 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) {
> @@ -55,11 +83,18 @@ static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
>
> 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);
> + if (!priv->idev) {
> + if (camera_shutter_input_setup(wdev, camera_mode))
> + dev_warn(&wdev->dev, "Failed to register input device\n");
> +
> + mutex_unlock(&priv->notify_lock);
> + return;
> + }
> +
> + if (camera_mode == SW_CAMERA_ON)
> + input_report_switch(priv->idev, SW_CAMERA_LENS_COVER, 0);
> + else
> + input_report_switch(priv->idev, SW_CAMERA_LENS_COVER, 1);
> input_sync(priv->idev);
>
> mutex_unlock(&priv->notify_lock);
> @@ -68,29 +103,12 @@ static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
> 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;
> @@ -100,6 +118,9 @@ static void lenovo_wmi_remove(struct wmi_device *wdev)
> {
> struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
>
> + if (priv->idev)
> + input_unregister_device(priv->idev);
> +
> mutex_destroy(&priv->notify_lock);
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v5] platform/x86: lenovo-wmi-camera: Use SW_CAMERA_LENS_COVER instead of KEY_CAMERA_ACESS
2025-01-01 10:29 [PATCH v5] platform/x86: lenovo-wmi-camera: Use SW_CAMERA_LENS_COVER instead of KEY_CAMERA_ACESS Ai Chao
2025-01-01 12:45 ` Armin Wolf
2025-01-06 10:47 ` Hans de Goede
@ 2025-01-07 18:09 ` Ilpo Järvinen
2 siblings, 0 replies; 4+ messages in thread
From: Ilpo Järvinen @ 2025-01-07 18:09 UTC (permalink / raw)
To: Ai Chao; +Cc: Hans de Goede, platform-driver-x86, LKML
On Wed, 1 Jan 2025, Ai Chao wrote:
> Use SW_CAMERA_LENS_COVER instead of KEY_CAMERA_ACESS_ENABLE and
> KEY_CAMERA_ACESS_DISABLE. When the camera toggle switch was hit,
> the lenovo-wmi-camera driver would report an event code.
>
> Signed-off-by: Ai Chao <aichao@kylinos.cn>
> ---
> change for v5
> -Add input_report_switch before input_register_device.
> change for v4
> -Add mutex_unlock and report a switch state of 0 if SW_CAMERA_ON.
> change for v3
> -Used input_register_device and input_allocate_device.
> change for v2
> -Only delays the input-device registration and switches to reporting SW_CAMERA_LENS_COVER.
>
> drivers/platform/x86/lenovo-wmi-camera.c | 67 ++++++++++++++++--------
> 1 file changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo-wmi-camera.c b/drivers/platform/x86/lenovo-wmi-camera.c
> index 0c0bedaf7407..ceed441a0161 100644
> --- a/drivers/platform/x86/lenovo-wmi-camera.c
> +++ b/drivers/platform/x86/lenovo-wmi-camera.c
> @@ -26,10 +26,38 @@ enum {
> SW_CAMERA_ON = 1,
> };
>
> +static int camera_shutter_input_setup(struct wmi_device *wdev, u8 camera_mode)
> +{
> + struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
> + int err;
> +
> + priv->idev = input_allocate_device();
> + 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_SW, SW_CAMERA_LENS_COVER);
> +
> + input_report_switch(priv->idev, SW_CAMERA_LENS_COVER,
> + camera_mode == SW_CAMERA_ON ? 0 : 1);
> + input_sync(priv->idev);
> +
> + err = input_register_device(priv->idev);
> + if (err) {
> + input_free_device(priv->idev);
> + priv->idev = NULL;
> + }
> +
> + return err;
> +}
> +
> 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) {
> @@ -55,11 +83,18 @@ static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
>
> 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);
> + if (!priv->idev) {
> + if (camera_shutter_input_setup(wdev, camera_mode))
> + dev_warn(&wdev->dev, "Failed to register input device\n");
> +
> + mutex_unlock(&priv->notify_lock);
> + return;
> + }
> +
> + if (camera_mode == SW_CAMERA_ON)
> + input_report_switch(priv->idev, SW_CAMERA_LENS_COVER, 0);
> + else
> + input_report_switch(priv->idev, SW_CAMERA_LENS_COVER, 1);
> input_sync(priv->idev);
>
> mutex_unlock(&priv->notify_lock);
Please just convert the mutex_lock() to use guard() and remove both
mutex_unlock()s as cleanup.h will then handle that automatically for you.
--
i.
> @@ -68,29 +103,12 @@ static void lenovo_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
> 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;
> @@ -100,6 +118,9 @@ static void lenovo_wmi_remove(struct wmi_device *wdev)
> {
> struct lenovo_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
>
> + if (priv->idev)
> + input_unregister_device(priv->idev);
> +
> mutex_destroy(&priv->notify_lock);
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread