[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZgGmfV9ciPdtbGO1@smile.fi.intel.com>
Date: Mon, 25 Mar 2024 18:29:49 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Ai Chao <aichao@...inos.cn>
Cc: hdegoede@...hat.com, ilpo.jarvinen@...ux.intel.com,
linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v11] platform/x86: add lenovo wmi camera button driver
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
Powered by blists - more mailing lists