LinuxLists.cc - [PATCH] Input: elan_i2c - Implement inhibit/uninhibit functions.

2023-05-31 09:15:41

Subject: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit functions.

Add inhibit/uninhibit functions.

Signed-off-by: Jingle.wu <[email protected]>
---
drivers/input/mouse/elan_i2c_core.c | 207 ++++++++++++++++++++++++++++
1 file changed, 207 insertions(+)

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 5f0d75a45c80..4ea57f4c7bd4 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -56,6 +56,7 @@ struct elan_tp_data {
struct input_dev *input;
struct input_dev *tp_input; /* trackpoint input node */
struct regulator *vcc;
+ struct list_head list; /* for list of devices needing input handler */

const struct elan_transport_ops *ops;

@@ -63,6 +64,11 @@ struct elan_tp_data {
struct completion fw_completion;
bool in_fw_update;

+ struct work_struct lid_work;
+ bool lid_switch;
+ int lid_value;
+ bool in_inhibit;
+
struct mutex sysfs_mutex;

unsigned int max_x;
@@ -96,6 +102,9 @@ struct elan_tp_data {
u32 quirks; /* Various quirks */
};

+static struct workqueue_struct *elan_mode_wq;
+static LIST_HEAD(elan_devices_with_lid_handler);
+
static u32 elan_i2c_lookup_quirks(u16 ic_type, u16 product_id)
{
static const struct {
@@ -329,6 +338,74 @@ static int elan_initialize(struct elan_tp_data *data, bool skip_reset)
return error;
}

+static int elan_reactivate(struct elan_tp_data *data)
+{
+ struct device *dev = &data->client->dev;
+ int error;
+
+ error = elan_set_power(data, true);
+ if (error)
+ dev_err(dev, "failed to restore power: %d\n", error);
+
+ error = data->ops->sleep_control(data->client, false);
+ if (error) {
+ dev_err(dev,
+ "failed to wake device up: %d\n", error);
+ return error;
+ }
+
+ return error;
+}
+
+static int elan_inhibit(struct input_dev *input_dev)
+{
+ struct elan_tp_data *data = input_get_drvdata(input_dev);
+ struct i2c_client *client = data->client;
+ int error;
+
+ dev_dbg(&client->dev, "inhibiting\n");
+ /*
+ * We are taking the mutex to make sure sysfs operations are
+ * complete before we attempt to bring the device into low[er]
+ * power mode.
+ */
+ error = mutex_lock_interruptible(&data->sysfs_mutex);
+ if (error)
+ return error;
+
+ disable_irq(client->irq);
+
+ error = elan_set_power(data, false);
+ if (error)
+ enable_irq(client->irq);
+
+ data->in_inhibit = true;
+ mutex_unlock(&data->sysfs_mutex);
+
+ return error;
+}
+
+static int elan_uninhibit(struct input_dev *input_dev)
+{
+ struct elan_tp_data *data = input_get_drvdata(input_dev);
+ struct i2c_client *client = data->client;
+ int error;
+
+ dev_dbg(&client->dev, "uninhibiting\n");
+ error = mutex_lock_interruptible(&data->sysfs_mutex);
+ if (error)
+ return error;
+
+ error = elan_reactivate(data);
+ if (error == 0)
+ enable_irq(client->irq);
+
+ data->in_inhibit = false;
+ mutex_unlock(&data->sysfs_mutex);
+
+ return error;
+}
+
static int elan_query_device_info(struct elan_tp_data *data)
{
int error;
@@ -1187,6 +1264,124 @@ static void elan_disable_regulator(void *_data)
regulator_disable(data->vcc);
}

+static void lid_work_handler(struct work_struct *work)
+{
+ struct elan_tp_data *data = container_of(work, struct elan_tp_data,
+ lid_work);
+
+ if (data->lid_value)
+ elan_inhibit(data->input);
+ else
+ elan_uninhibit(data->input);
+
+}
+
+static void elan_input_lid_event(struct input_handle *handle, unsigned int type,
+ unsigned int code, int value)
+{
+ struct elan_tp_data *data, *n;
+
+ if (type == EV_SW && code == SW_LID) {
+ list_for_each_entry_safe(data, n, &elan_devices_with_lid_handler, list) {
+ data->lid_value = value;
+ queue_work(elan_mode_wq, &data->lid_work);
+ }
+ }
+
+}
+
+struct elan_input_lid {
+ struct input_handle handle;
+};
+
+static int elan_input_lid_connect(struct input_handler *handler,
+ struct input_dev *dev,
+ const struct input_device_id *id)
+{
+ struct elan_input_lid *lid;
+ char *name;
+ int error;
+
+ lid = kzalloc(sizeof(*lid), GFP_KERNEL);
+ if (!lid)
+ return -ENOMEM;
+ name = kasprintf(GFP_KERNEL, "elan-i2c-lid-%s", dev_name(&dev->dev));
+ if (!name) {
+ error = -ENOMEM;
+ goto err_free_lid;
+ }
+ lid->handle.dev = dev;
+ lid->handle.handler = handler;
+ lid->handle.name = name;
+ lid->handle.private = lid;
+ error = input_register_handle(&lid->handle);
+ if (error)
+ goto err_free_name;
+ error = input_open_device(&lid->handle);
+ if (error)
+ goto err_unregister_handle;
+ return 0;
+err_unregister_handle:
+ input_unregister_handle(&lid->handle);
+err_free_name:
+ kfree(name);
+err_free_lid:
+ kfree(lid);
+ return error;
+}
+
+static void elan_input_lid_disconnect(struct input_handle *handle)
+{
+ struct elan_input_lid *lid = handle->private;
+
+ input_close_device(handle);
+ input_unregister_handle(handle);
+ kfree(handle->name);
+ kfree(lid);
+}
+
+static const struct input_device_id elan_input_lid_ids[] = {
+ {
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_SWBIT,
+ .evbit = { BIT_MASK(EV_SW) },
+ .swbit = { [BIT_WORD(SW_LID)] = BIT_MASK(SW_LID) },
+ },
+ { },
+};
+
+static struct input_handler elan_input_lid_handler = {
+ .event = elan_input_lid_event,
+ .connect = elan_input_lid_connect,
+ .disconnect = elan_input_lid_disconnect,
+ .name = "elan-i2c-lid",
+ .id_table = elan_input_lid_ids,
+};
+
+static int elan_create_lid_handler(struct elan_tp_data *data)
+{
+ int error = 0;
+
+ elan_mode_wq = create_singlethread_workqueue("elan-i2c-lid");
+ if (elan_mode_wq == NULL)
+ return -ENOMEM;
+ error = input_register_handler(&elan_input_lid_handler);
+ if (error)
+ goto remove_wq;
+
+ data->lid_switch = true;
+ INIT_LIST_HEAD(&data->list);
+ INIT_WORK(&data->lid_work, lid_work_handler);
+ list_add_tail(&data->list, &elan_devices_with_lid_handler);
+
+ return 0;
+
+remove_wq:
+ data->lid_switch = false;
+ destroy_workqueue(elan_mode_wq);
+ elan_mode_wq = NULL;
+ return error;
+}
+
static int elan_probe(struct i2c_client *client)
{
const struct elan_transport_ops *transport_ops;
@@ -1325,6 +1520,10 @@ static int elan_probe(struct i2c_client *client)
}
}

+ error = elan_create_lid_handler(data);
+ if (error)
+ dev_err(dev, "failed to create lid handler: %d\n", error);
+
return 0;
}

@@ -1334,6 +1533,10 @@ static int elan_suspend(struct device *dev)
struct elan_tp_data *data = i2c_get_clientdata(client);
int ret;

+ /* Wait for switch on completion */
+ if (data->lid_switch)
+ flush_workqueue(elan_mode_wq);
+
/*
* We are taking the mutex to make sure sysfs operations are
* complete before we attempt to bring the device into low[er]
@@ -1371,6 +1574,10 @@ static int elan_resume(struct device *dev)
struct elan_tp_data *data = i2c_get_clientdata(client);
int error;

+ /* Wait for switch on completion */
+ if (data->lid_switch)
+ flush_workqueue(elan_mode_wq);
+
if (!device_may_wakeup(dev)) {
error = regulator_enable(data->vcc);
if (error) {
--
2.34.1

2023-06-29 23:06:50

by Dmitry Torokhov

[permalink] [raw]

Subject: Re: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit functions.

Hi Jingle,

On Wed, May 31, 2023 at 05:03:40PM +0800, jingle.wu wrote:
> Add inhibit/uninhibit functions.

You need to provide justification for this change, i.e. explain why you
need this/what is currently not working or working sub-optimally.

>
> Signed-off-by: Jingle.wu <[email protected]>
> ---
> drivers/input/mouse/elan_i2c_core.c | 207 ++++++++++++++++++++++++++++
> 1 file changed, 207 insertions(+)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index 5f0d75a45c80..4ea57f4c7bd4 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -56,6 +56,7 @@ struct elan_tp_data {
> struct input_dev *input;
> struct input_dev *tp_input; /* trackpoint input node */
> struct regulator *vcc;
> + struct list_head list; /* for list of devices needing input handler */
>
> const struct elan_transport_ops *ops;
>
> @@ -63,6 +64,11 @@ struct elan_tp_data {
> struct completion fw_completion;
> bool in_fw_update;
>
> + struct work_struct lid_work;
> + bool lid_switch;
> + int lid_value;
> + bool in_inhibit;
> +
> struct mutex sysfs_mutex;
>
> unsigned int max_x;
> @@ -96,6 +102,9 @@ struct elan_tp_data {
> u32 quirks; /* Various quirks */
> };
>
> +static struct workqueue_struct *elan_mode_wq;
> +static LIST_HEAD(elan_devices_with_lid_handler);
> +
> static u32 elan_i2c_lookup_quirks(u16 ic_type, u16 product_id)
> {
> static const struct {
> @@ -329,6 +338,74 @@ static int elan_initialize(struct elan_tp_data *data, bool skip_reset)
> return error;
> }
>
> +static int elan_reactivate(struct elan_tp_data *data)
> +{
> + struct device *dev = &data->client->dev;
> + int error;
> +
> + error = elan_set_power(data, true);
> + if (error)
> + dev_err(dev, "failed to restore power: %d\n", error);
> +
> + error = data->ops->sleep_control(data->client, false);
> + if (error) {
> + dev_err(dev,
> + "failed to wake device up: %d\n", error);
> + return error;
> + }
> +
> + return error;
> +}
> +
> +static int elan_inhibit(struct input_dev *input_dev)

I would rather you did not call it inhibit/uninhibit because this is not
what it does.

Please split the logic for recalibration from logic of powering on and
off the device. I also hope in the future firmware revisions the
distinction will be more clear (i.e. have a separate method/command to
recalibrate baseline on demand).

> +{
> + struct elan_tp_data *data = input_get_drvdata(input_dev);
> + struct i2c_client *client = data->client;
> + int error;
> +
> + dev_dbg(&client->dev, "inhibiting\n");
> + /*
> + * We are taking the mutex to make sure sysfs operations are
> + * complete before we attempt to bring the device into low[er]
> + * power mode.
> + */
> + error = mutex_lock_interruptible(&data->sysfs_mutex);
> + if (error)
> + return error;
> +
> + disable_irq(client->irq);
> +
> + error = elan_set_power(data, false);
> + if (error)
> + enable_irq(client->irq);
> +
> + data->in_inhibit = true;
> + mutex_unlock(&data->sysfs_mutex);
> +
> + return error;
> +}
> +
> +static int elan_uninhibit(struct input_dev *input_dev)
> +{
> + struct elan_tp_data *data = input_get_drvdata(input_dev);
> + struct i2c_client *client = data->client;
> + int error;
> +
> + dev_dbg(&client->dev, "uninhibiting\n");
> + error = mutex_lock_interruptible(&data->sysfs_mutex);
> + if (error)
> + return error;
> +
> + error = elan_reactivate(data);
> + if (error == 0)
> + enable_irq(client->irq);
> +
> + data->in_inhibit = false;
> + mutex_unlock(&data->sysfs_mutex);
> +
> + return error;
> +}
> +
> static int elan_query_device_info(struct elan_tp_data *data)
> {
> int error;
> @@ -1187,6 +1264,124 @@ static void elan_disable_regulator(void *_data)
> regulator_disable(data->vcc);
> }
>
> +static void lid_work_handler(struct work_struct *work)
> +{
> + struct elan_tp_data *data = container_of(work, struct elan_tp_data,
> + lid_work);
> +
> + if (data->lid_value)
> + elan_inhibit(data->input);
> + else
> + elan_uninhibit(data->input);
> +
> +}
> +
> +static void elan_input_lid_event(struct input_handle *handle, unsigned int type,
> + unsigned int code, int value)
> +{
> + struct elan_tp_data *data, *n;
> +
> + if (type == EV_SW && code == SW_LID) {
> + list_for_each_entry_safe(data, n, &elan_devices_with_lid_handler, list) {

Why do you need the "_safe()" variant here?

> + data->lid_value = value;
> + queue_work(elan_mode_wq, &data->lid_work);
> + }
> + }
> +
> +}
> +
> +struct elan_input_lid {
> + struct input_handle handle;
> +};
> +
> +static int elan_input_lid_connect(struct input_handler *handler,
> + struct input_dev *dev,
> + const struct input_device_id *id)
> +{
> + struct elan_input_lid *lid;
> + char *name;
> + int error;
> +
> + lid = kzalloc(sizeof(*lid), GFP_KERNEL);
> + if (!lid)
> + return -ENOMEM;
> + name = kasprintf(GFP_KERNEL, "elan-i2c-lid-%s", dev_name(&dev->dev));
> + if (!name) {
> + error = -ENOMEM;
> + goto err_free_lid;
> + }
> + lid->handle.dev = dev;
> + lid->handle.handler = handler;
> + lid->handle.name = name;
> + lid->handle.private = lid;
> + error = input_register_handle(&lid->handle);
> + if (error)
> + goto err_free_name;
> + error = input_open_device(&lid->handle);
> + if (error)
> + goto err_unregister_handle;
> + return 0;
> +err_unregister_handle:
> + input_unregister_handle(&lid->handle);
> +err_free_name:
> + kfree(name);
> +err_free_lid:
> + kfree(lid);
> + return error;
> +}
> +
> +static void elan_input_lid_disconnect(struct input_handle *handle)
> +{
> + struct elan_input_lid *lid = handle->private;
> +
> + input_close_device(handle);
> + input_unregister_handle(handle);
> + kfree(handle->name);
> + kfree(lid);
> +}
> +
> +static const struct input_device_id elan_input_lid_ids[] = {
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_SWBIT,
> + .evbit = { BIT_MASK(EV_SW) },
> + .swbit = { [BIT_WORD(SW_LID)] = BIT_MASK(SW_LID) },
> + },
> + { },
> +};
> +
> +static struct input_handler elan_input_lid_handler = {
> + .event = elan_input_lid_event,
> + .connect = elan_input_lid_connect,
> + .disconnect = elan_input_lid_disconnect,
> + .name = "elan-i2c-lid",
> + .id_table = elan_input_lid_ids,
> +};
> +
> +static int elan_create_lid_handler(struct elan_tp_data *data)
> +{
> + int error = 0;
> +
> + elan_mode_wq = create_singlethread_workqueue("elan-i2c-lid");
> + if (elan_mode_wq == NULL)
> + return -ENOMEM;
> + error = input_register_handler(&elan_input_lid_handler);
> + if (error)
> + goto remove_wq;
> +
> + data->lid_switch = true;
> + INIT_LIST_HEAD(&data->list);
> + INIT_WORK(&data->lid_work, lid_work_handler);
> + list_add_tail(&data->list, &elan_devices_with_lid_handler);

It looks like you call elan_create_lid_handler() from elan_probe() which
means it can be called several times (we should not assume there is only
one controller), I do not see it being destroyed in remove() either, so
it will break if you bind/unbind the driver.

I also not sure why you need the list of you have a handler per device.

> +
> + return 0;
> +
> +remove_wq:
> + data->lid_switch = false;
> + destroy_workqueue(elan_mode_wq);
> + elan_mode_wq = NULL;
> + return error;
> +}
> +
> static int elan_probe(struct i2c_client *client)
> {
> const struct elan_transport_ops *transport_ops;
> @@ -1325,6 +1520,10 @@ static int elan_probe(struct i2c_client *client)
> }
> }
>
> + error = elan_create_lid_handler(data);
> + if (error)
> + dev_err(dev, "failed to create lid handler: %d\n", error);

Do we need this on _ALL_ devices with ELan controllers, or just certain
ones? If we need this on all devices how did it work before?

> +
> return 0;
> }
>
> @@ -1334,6 +1533,10 @@ static int elan_suspend(struct device *dev)
> struct elan_tp_data *data = i2c_get_clientdata(client);
> int ret;
>
> + /* Wait for switch on completion */
> + if (data->lid_switch)
> + flush_workqueue(elan_mode_wq);
> +
> /*
> * We are taking the mutex to make sure sysfs operations are
> * complete before we attempt to bring the device into low[er]
> @@ -1371,6 +1574,10 @@ static int elan_resume(struct device *dev)
> struct elan_tp_data *data = i2c_get_clientdata(client);
> int error;
>
> + /* Wait for switch on completion */
> + if (data->lid_switch)
> + flush_workqueue(elan_mode_wq);
> +
> if (!device_may_wakeup(dev)) {
> error = regulator_enable(data->vcc);
> if (error) {
> --
> 2.34.1
>

Thanks.

--
Dmitry

2023-07-05 06:23:15

by Jingle.Wu

[permalink] [raw]

Subject: RE: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit functions.

HI Dmitry:

1.
> +static void elan_input_lid_event(struct input_handle *handle,
> +unsigned
int type,
> + unsigned int code, int value) {
> + struct elan_tp_data *data, *n;
> +
> + if (type == EV_SW && code == SW_LID) {
> + list_for_each_entry_safe(data, n,
&elan_devices_with_lid_handler,
> +list) {

Why do you need the "_safe()" variant here?
-> Because I took the above link as reference.
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3
578852/7/drivers/hid/hid-multitouch.c#394

2.
> + data->lid_switch = true;
> + INIT_LIST_HEAD(&data->list);
> + INIT_WORK(&data->lid_work, lid_work_handler);
> + list_add_tail(&data->list, &elan_devices_with_lid_handler);

It looks like you call elan_create_lid_handler() from elan_probe() which
means it can be called several times (we should not assume there is only one
controller), I do not see it being destroyed in remove() either, so it will
break if you bind/unbind the driver.

I also not sure why you need the list of you have a handler per device.

-> If the elan_create_lid_handler() function not be put into probe(), the
lid
handler would be invalid in elan private data("struct elan_tp_data *data").
Or do you have any suggestions for it? Thanks.

3.
> + error = elan_create_lid_handler(data);
> + if (error)
> + dev_err(dev, "failed to create lid handler: %d\n", error);

Do we need this on _ALL_ devices with ELan controllers, or just certain
ones? If we need this on all devices how did it work before?

-> Yes, we need this on all device.
In Chromeos kernel v5.4 before, the elan_inhibit()/uninhibit function was
built and worked
successfully in elan_i2c_driver.
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads
/chromeos-5.4/drivers/input/mouse/elan_i2c_core.c#353

Thanks
JINGLE

-----Original Message-----
From: Dmitry Torokhov [mailto:[email protected]]
Sent: Friday, June 30, 2023 6:57 AM
To: jingle.wu <[email protected]>
Cc: [email protected]; [email protected];
[email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit
functions.

Hi Jingle,

On Wed, May 31, 2023 at 05:03:40PM +0800, jingle.wu wrote:
> Add inhibit/uninhibit functions.

You need to provide justification for this change, i.e. explain why you need
this/what is currently not working or working sub-optimally.

>
> Signed-off-by: Jingle.wu <[email protected]>
> ---
> drivers/input/mouse/elan_i2c_core.c | 207
> ++++++++++++++++++++++++++++
> 1 file changed, 207 insertions(+)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c
> b/drivers/input/mouse/elan_i2c_core.c
> index 5f0d75a45c80..4ea57f4c7bd4 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -56,6 +56,7 @@ struct elan_tp_data {
> struct input_dev *input;
> struct input_dev *tp_input; /* trackpoint input node */
> struct regulator *vcc;
> + struct list_head list; /* for list of devices needing input handler
*/
>
> const struct elan_transport_ops *ops;
>
> @@ -63,6 +64,11 @@ struct elan_tp_data {
> struct completion fw_completion;
> bool in_fw_update;
>
> + struct work_struct lid_work;
> + bool lid_switch;
> + int lid_value;
> + bool in_inhibit;
> +
> struct mutex sysfs_mutex;
>
> unsigned int max_x;
> @@ -96,6 +102,9 @@ struct elan_tp_data {
> u32 quirks; /* Various quirks */
> };
>
> +static struct workqueue_struct *elan_mode_wq; static
> +LIST_HEAD(elan_devices_with_lid_handler);
> +
> static u32 elan_i2c_lookup_quirks(u16 ic_type, u16 product_id) {
> static const struct {
> @@ -329,6 +338,74 @@ static int elan_initialize(struct elan_tp_data *data,
bool skip_reset)
> return error;
> }
>
> +static int elan_reactivate(struct elan_tp_data *data) {
> + struct device *dev = &data->client->dev;
> + int error;
> +
> + error = elan_set_power(data, true);
> + if (error)
> + dev_err(dev, "failed to restore power: %d\n", error);
> +
> + error = data->ops->sleep_control(data->client, false);
> + if (error) {
> + dev_err(dev,
> + "failed to wake device up: %d\n", error);
> + return error;
> + }
> +
> + return error;
> +}
> +
> +static int elan_inhibit(struct input_dev *input_dev)

I would rather you did not call it inhibit/uninhibit because this is not
what it does.

Please split the logic for recalibration from logic of powering on and off
the device. I also hope in the future firmware revisions the distinction
will be more clear (i.e. have a separate method/command to recalibrate
baseline on demand).

> +{
> + struct elan_tp_data *data = input_get_drvdata(input_dev);
> + struct i2c_client *client = data->client;
> + int error;
> +
> + dev_dbg(&client->dev, "inhibiting\n");
> + /*
> + * We are taking the mutex to make sure sysfs operations are
> + * complete before we attempt to bring the device into low[er]
> + * power mode.
> + */
> + error = mutex_lock_interruptible(&data->sysfs_mutex);
> + if (error)
> + return error;
> +
> + disable_irq(client->irq);
> +
> + error = elan_set_power(data, false);
> + if (error)
> + enable_irq(client->irq);
> +
> + data->in_inhibit = true;
> + mutex_unlock(&data->sysfs_mutex);
> +
> + return error;
> +}
> +
> +static int elan_uninhibit(struct input_dev *input_dev) {
> + struct elan_tp_data *data = input_get_drvdata(input_dev);
> + struct i2c_client *client = data->client;
> + int error;
> +
> + dev_dbg(&client->dev, "uninhibiting\n");
> + error = mutex_lock_interruptible(&data->sysfs_mutex);
> + if (error)
> + return error;
> +
> + error = elan_reactivate(data);
> + if (error == 0)
> + enable_irq(client->irq);
> +
> + data->in_inhibit = false;
> + mutex_unlock(&data->sysfs_mutex);
> +
> + return error;
> +}
> +
> static int elan_query_device_info(struct elan_tp_data *data) {
> int error;
> @@ -1187,6 +1264,124 @@ static void elan_disable_regulator(void *_data)
> regulator_disable(data->vcc);
> }
>
> +static void lid_work_handler(struct work_struct *work) {
> + struct elan_tp_data *data = container_of(work, struct elan_tp_data,
> + lid_work);
> +
> + if (data->lid_value)
> + elan_inhibit(data->input);
> + else
> + elan_uninhibit(data->input);
> +
> +}
> +
> +static void elan_input_lid_event(struct input_handle *handle, unsigned
int type,
> + unsigned int code, int value) {
> + struct elan_tp_data *data, *n;
> +
> + if (type == EV_SW && code == SW_LID) {
> + list_for_each_entry_safe(data, n,
&elan_devices_with_lid_handler,
> +list) {

Why do you need the "_safe()" variant here?

> + data->lid_value = value;
> + queue_work(elan_mode_wq, &data->lid_work);
> + }
> + }
> +
> +}
> +
> +struct elan_input_lid {
> + struct input_handle handle;
> +};
> +
> +static int elan_input_lid_connect(struct input_handler *handler,
> + struct input_dev *dev,
> + const struct input_device_id *id) {
> + struct elan_input_lid *lid;
> + char *name;
> + int error;
> +
> + lid = kzalloc(sizeof(*lid), GFP_KERNEL);
> + if (!lid)
> + return -ENOMEM;
> + name = kasprintf(GFP_KERNEL, "elan-i2c-lid-%s",
dev_name(&dev->dev));
> + if (!name) {
> + error = -ENOMEM;
> + goto err_free_lid;
> + }
> + lid->handle.dev = dev;
> + lid->handle.handler = handler;
> + lid->handle.name = name;
> + lid->handle.private = lid;
> + error = input_register_handle(&lid->handle);
> + if (error)
> + goto err_free_name;
> + error = input_open_device(&lid->handle);
> + if (error)
> + goto err_unregister_handle;
> + return 0;
> +err_unregister_handle:
> + input_unregister_handle(&lid->handle);
> +err_free_name:
> + kfree(name);
> +err_free_lid:
> + kfree(lid);
> + return error;
> +}
> +
> +static void elan_input_lid_disconnect(struct input_handle *handle) {
> + struct elan_input_lid *lid = handle->private;
> +
> + input_close_device(handle);
> + input_unregister_handle(handle);
> + kfree(handle->name);
> + kfree(lid);
> +}
> +
> +static const struct input_device_id elan_input_lid_ids[] = {
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
INPUT_DEVICE_ID_MATCH_SWBIT,
> + .evbit = { BIT_MASK(EV_SW) },
> + .swbit = { [BIT_WORD(SW_LID)] = BIT_MASK(SW_LID) },
> + },
> + { },
> +};
> +
> +static struct input_handler elan_input_lid_handler = {
> + .event = elan_input_lid_event,
> + .connect = elan_input_lid_connect,
> + .disconnect = elan_input_lid_disconnect,
> + .name = "elan-i2c-lid",
> + .id_table = elan_input_lid_ids,
> +};
> +
> +static int elan_create_lid_handler(struct elan_tp_data *data) {
> + int error = 0;
> +
> + elan_mode_wq = create_singlethread_workqueue("elan-i2c-lid");
> + if (elan_mode_wq == NULL)
> + return -ENOMEM;
> + error = input_register_handler(&elan_input_lid_handler);
> + if (error)
> + goto remove_wq;
> +
> + data->lid_switch = true;
> + INIT_LIST_HEAD(&data->list);
> + INIT_WORK(&data->lid_work, lid_work_handler);
> + list_add_tail(&data->list, &elan_devices_with_lid_handler);

It looks like you call elan_create_lid_handler() from elan_probe() which
means it can be called several times (we should not assume there is only one
controller), I do not see it being destroyed in remove() either, so it will
break if you bind/unbind the driver.

I also not sure why you need the list of you have a handler per device.

> +
> + return 0;
> +
> +remove_wq:
> + data->lid_switch = false;
> + destroy_workqueue(elan_mode_wq);
> + elan_mode_wq = NULL;
> + return error;
> +}
> +
> static int elan_probe(struct i2c_client *client) {
> const struct elan_transport_ops *transport_ops; @@ -1325,6 +1520,10
> @@ static int elan_probe(struct i2c_client *client)
> }
> }
>
> + error = elan_create_lid_handler(data);
> + if (error)
> + dev_err(dev, "failed to create lid handler: %d\n", error);

Do we need this on _ALL_ devices with ELan controllers, or just certain
ones? If we need this on all devices how did it work before?

> +
> return 0;
> }
>
> @@ -1334,6 +1533,10 @@ static int elan_suspend(struct device *dev)
> struct elan_tp_data *data = i2c_get_clientdata(client);
> int ret;
>
> + /* Wait for switch on completion */
> + if (data->lid_switch)
> + flush_workqueue(elan_mode_wq);
> +
> /*
> * We are taking the mutex to make sure sysfs operations are
> * complete before we attempt to bring the device into low[er] @@
> -1371,6 +1574,10 @@ static int elan_resume(struct device *dev)
> struct elan_tp_data *data = i2c_get_clientdata(client);
> int error;
>
> + /* Wait for switch on completion */
> + if (data->lid_switch)
> + flush_workqueue(elan_mode_wq);
> +
> if (!device_may_wakeup(dev)) {
> error = regulator_enable(data->vcc);
> if (error) {
> --
> 2.34.1
>

Thanks.

--
Dmitry

2023-07-07 06:15:52

by Jingle.Wu

[permalink] [raw]

Subject: RE: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit functions.

HI Dmitry:

...
module_i2c_driver(elan_driver);

static int __init elan_init(void)
{
return elan_create_lid_handler();
}

static void __exit elan_exit(void)
{
if (elan_mode_wq)
elan_destroy_lid_handler();
}

module_init(elan_init);
module_exit(elan_exit);
...

If I change to elan_create_lid_handler() at module_init, complie error as
follow.

<pre>./include/linux/module.h:130:49: error:
redefinition of '__inittest'
130 | static inline initcall_t __maybe_unused __inittest(void) \
| ^~~~~~~~~~
drivers/input/mouse/elan_i2c_core.c:1666:1: note: in expansion of macro
'module_init'
1666 | module_init(elan_init);
| ^~~~~~~~~~~
./include/linux/module.h:130:49: note:
previous definition of '__inittest' with type 'int
(*(void))(void)'
130 | static inline initcall_t __maybe_unused __inittest(void) \
| ^~~~~~~~~~
./include/linux/device/driver.h:266:1: note:
in expansion of macro 'module_init'
266 | module_init(__driver##_init); \
| ^~~~~~~~~~~
./include/linux/i2c.h:956:9: note:
in expansion of macro 'module_driver'
956 | module_driver(__i2c_driver, i2c_add_driver, \
| ^~~~~~~~~~~~~
drivers/input/mouse/elan_i2c_core.c:1653:1: note: in expansion of macro
'module_i2c_driver'
1653 | module_i2c_driver(elan_driver);
| ^~~~~~~~~~~~~~~~~
./include/linux/module.h:132:13: error:
redefinition of 'init_module'
132 | int init_module(void)
__copy(initfn) \
| ^~~~~~~~~~~
drivers/input/mouse/elan_i2c_core.c:1666:1: note: in expansion of macro
'module_init'
1666 | module_init(elan_init);
| ^~~~~~~~~~~
./include/linux/module.h:132:13: note:
previous definition of 'init_module' with type
'int(void)'
132 | int init_module(void)
__copy(initfn) \
| ^~~~~~~~~~~
./include/linux/device/driver.h:266:1: note:
in expansion of macro 'module_init'
266 | module_init(__driver##_init); \
| ^~~~~~~~~~~
./include/linux/i2c.h:956:9: note:
in expansion of macro 'module_driver'
956 | module_driver(__i2c_driver, i2c_add_driver, \
| ^~~~~~~~~~~~~
drivers/input/mouse/elan_i2c_core.c:1653:1: note: in expansion of macro
'module_i2c_driver'
1653 | module_i2c_driver(elan_driver);
| ^~~~~~~~~~~~~~~~~
./include/linux/module.h:138:49: error:
redefinition of '__exittest'
138 | static inline exitcall_t __maybe_unused __exittest(void) \
| ^~~~~~~~~~
drivers/input/mouse/elan_i2c_core.c:1667:1: note: in expansion of macro
'module_exit'
1667 | module_exit(elan_exit);
| ^~~~~~~~~~~
./include/linux/module.h:138:49: note:
previous definition of '__exittest' with type 'void
(*(void))(void)'
138 | static inline exitcall_t __maybe_unused __exittest(void) \
| ^~~~~~~~~~
./include/linux/device/driver.h:271:1: note:
in expansion of macro 'module_exit'
271 | module_exit(__driver##_exit);
| ^~~~~~~~~~~
./include/linux/i2c.h:956:9: note:
in expansion of macro 'module_driver'
956 | module_driver(__i2c_driver, i2c_add_driver, \
| ^~~~~~~~~~~~~
drivers/input/mouse/elan_i2c_core.c:1653:1: note: in expansion of macro
'module_i2c_driver'
1653 | module_i2c_driver(elan_driver);
| ^~~~~~~~~~~~~~~~~
./include/linux/module.h:140:14: error:
redefinition of 'cleanup_module'
140 | void cleanup_module(void) __copy(exitfn)
\
| ^~~~~~~~~~~~~~
drivers/input/mouse/elan_i2c_core.c:1667:1: note: in expansion of macro
'module_exit'
1667 | module_exit(elan_exit);
| ^~~~~~~~~~~
./include/linux/module.h:140:14: note:
previous definition of 'cleanup_module' with type
'void(void)'
140 | void cleanup_module(void) __copy(exitfn)
\
| ^~~~~~~~~~~~~~
./include/linux/device/driver.h:271:1: note:
in expansion of macro 'module_exit'
271 | module_exit(__driver##_exit);
| ^~~~~~~~~~~
./include/linux/i2c.h:956:9: note:
in expansion of macro 'module_driver'
956 | module_driver(__i2c_driver, i2c_add_driver, \
| ^~~~~~~~~~~~~
drivers/input/mouse/elan_i2c_core.c:1653:1: note: in expansion of macro
'module_i2c_driver'
1653 | module_i2c_driver(elan_driver);
| ^~~~~~~~~~~~~~~~~
</pre>

THANJS
JINGLE
-----Original Message-----
From: Jingle.Wu [mailto:[email protected]]
Sent: Wednesday, July 5, 2023 2:09 PM
To: 'Dmitry Torokhov' <[email protected]>
Cc: '[email protected]' <[email protected]>;
'[email protected]' <[email protected]>;
'[email protected]' <[email protected]>; '[email protected]'
<[email protected]>; '[email protected]' <[email protected]>
Subject: RE: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit
functions.

HI Dmitry:

1.
> +static void elan_input_lid_event(struct input_handle *handle,
> +unsigned
int type,
> + unsigned int code, int value) {
> + struct elan_tp_data *data, *n;
> +
> + if (type == EV_SW && code == SW_LID) {
> + list_for_each_entry_safe(data, n,
&elan_devices_with_lid_handler,
> +list) {

Why do you need the "_safe()" variant here?
-> Because I took the above link as reference.
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3
578852/7/drivers/hid/hid-multitouch.c#394

2.
> + data->lid_switch = true;
> + INIT_LIST_HEAD(&data->list);
> + INIT_WORK(&data->lid_work, lid_work_handler);
> + list_add_tail(&data->list, &elan_devices_with_lid_handler);

It looks like you call elan_create_lid_handler() from elan_probe() which
means it can be called several times (we should not assume there is only one
controller), I do not see it being destroyed in remove() either, so it will
break if you bind/unbind the driver.

I also not sure why you need the list of you have a handler per device.

-> If the elan_create_lid_handler() function not be put into probe(),
-> the lid
handler would be invalid in elan private data("struct elan_tp_data *data").
Or do you have any suggestions for it? Thanks.

3.
> + error = elan_create_lid_handler(data);
> + if (error)
> + dev_err(dev, "failed to create lid handler: %d\n", error);

Do we need this on _ALL_ devices with ELan controllers, or just certain
ones? If we need this on all devices how did it work before?

-> Yes, we need this on all device.
In Chromeos kernel v5.4 before, the elan_inhibit()/uninhibit function was
built and worked successfully in elan_i2c_driver.
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads
/chromeos-5.4/drivers/input/mouse/elan_i2c_core.c#353

Thanks
JINGLE

-----Original Message-----
From: Dmitry Torokhov [mailto:[email protected]]
Sent: Friday, June 30, 2023 6:57 AM
To: jingle.wu <[email protected]>
Cc: [email protected]; [email protected];
[email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit
functions.

Hi Jingle,

On Wed, May 31, 2023 at 05:03:40PM +0800, jingle.wu wrote:
> Add inhibit/uninhibit functions.

You need to provide justification for this change, i.e. explain why you need
this/what is currently not working or working sub-optimally.

>
> Signed-off-by: Jingle.wu <[email protected]>
> ---
> drivers/input/mouse/elan_i2c_core.c | 207
> ++++++++++++++++++++++++++++
> 1 file changed, 207 insertions(+)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c
> b/drivers/input/mouse/elan_i2c_core.c
> index 5f0d75a45c80..4ea57f4c7bd4 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -56,6 +56,7 @@ struct elan_tp_data {
> struct input_dev *input;
> struct input_dev *tp_input; /* trackpoint input node */
> struct regulator *vcc;
> + struct list_head list; /* for list of devices needing input handler
*/
>
> const struct elan_transport_ops *ops;
>
> @@ -63,6 +64,11 @@ struct elan_tp_data {
> struct completion fw_completion;
> bool in_fw_update;
>
> + struct work_struct lid_work;
> + bool lid_switch;
> + int lid_value;
> + bool in_inhibit;
> +
> struct mutex sysfs_mutex;
>
> unsigned int max_x;
> @@ -96,6 +102,9 @@ struct elan_tp_data {
> u32 quirks; /* Various quirks */
> };
>
> +static struct workqueue_struct *elan_mode_wq; static
> +LIST_HEAD(elan_devices_with_lid_handler);
> +
> static u32 elan_i2c_lookup_quirks(u16 ic_type, u16 product_id) {
> static const struct {
> @@ -329,6 +338,74 @@ static int elan_initialize(struct elan_tp_data *data,
bool skip_reset)
> return error;
> }
>
> +static int elan_reactivate(struct elan_tp_data *data) {
> + struct device *dev = &data->client->dev;
> + int error;
> +
> + error = elan_set_power(data, true);
> + if (error)
> + dev_err(dev, "failed to restore power: %d\n", error);
> +
> + error = data->ops->sleep_control(data->client, false);
> + if (error) {
> + dev_err(dev,
> + "failed to wake device up: %d\n", error);
> + return error;
> + }
> +
> + return error;
> +}
> +
> +static int elan_inhibit(struct input_dev *input_dev)

I would rather you did not call it inhibit/uninhibit because this is not
what it does.

Please split the logic for recalibration from logic of powering on and off
the device. I also hope in the future firmware revisions the distinction
will be more clear (i.e. have a separate method/command to recalibrate
baseline on demand).

> +{
> + struct elan_tp_data *data = input_get_drvdata(input_dev);
> + struct i2c_client *client = data->client;
> + int error;
> +
> + dev_dbg(&client->dev, "inhibiting\n");
> + /*
> + * We are taking the mutex to make sure sysfs operations are
> + * complete before we attempt to bring the device into low[er]
> + * power mode.
> + */
> + error = mutex_lock_interruptible(&data->sysfs_mutex);
> + if (error)
> + return error;
> +
> + disable_irq(client->irq);
> +
> + error = elan_set_power(data, false);
> + if (error)
> + enable_irq(client->irq);
> +
> + data->in_inhibit = true;
> + mutex_unlock(&data->sysfs_mutex);
> +
> + return error;
> +}
> +
> +static int elan_uninhibit(struct input_dev *input_dev) {
> + struct elan_tp_data *data = input_get_drvdata(input_dev);
> + struct i2c_client *client = data->client;
> + int error;
> +
> + dev_dbg(&client->dev, "uninhibiting\n");
> + error = mutex_lock_interruptible(&data->sysfs_mutex);
> + if (error)
> + return error;
> +
> + error = elan_reactivate(data);
> + if (error == 0)
> + enable_irq(client->irq);
> +
> + data->in_inhibit = false;
> + mutex_unlock(&data->sysfs_mutex);
> +
> + return error;
> +}
> +
> static int elan_query_device_info(struct elan_tp_data *data) {
> int error;
> @@ -1187,6 +1264,124 @@ static void elan_disable_regulator(void *_data)
> regulator_disable(data->vcc);
> }
>
> +static void lid_work_handler(struct work_struct *work) {
> + struct elan_tp_data *data = container_of(work, struct elan_tp_data,
> + lid_work);
> +
> + if (data->lid_value)
> + elan_inhibit(data->input);
> + else
> + elan_uninhibit(data->input);
> +
> +}
> +
> +static void elan_input_lid_event(struct input_handle *handle, unsigned
int type,
> + unsigned int code, int value) {
> + struct elan_tp_data *data, *n;
> +
> + if (type == EV_SW && code == SW_LID) {
> + list_for_each_entry_safe(data, n,
&elan_devices_with_lid_handler,
> +list) {

Why do you need the "_safe()" variant here?

> + data->lid_value = value;
> + queue_work(elan_mode_wq, &data->lid_work);
> + }
> + }
> +
> +}
> +
> +struct elan_input_lid {
> + struct input_handle handle;
> +};
> +
> +static int elan_input_lid_connect(struct input_handler *handler,
> + struct input_dev *dev,
> + const struct input_device_id *id) {
> + struct elan_input_lid *lid;
> + char *name;
> + int error;
> +
> + lid = kzalloc(sizeof(*lid), GFP_KERNEL);
> + if (!lid)
> + return -ENOMEM;
> + name = kasprintf(GFP_KERNEL, "elan-i2c-lid-%s",
dev_name(&dev->dev));
> + if (!name) {
> + error = -ENOMEM;
> + goto err_free_lid;
> + }
> + lid->handle.dev = dev;
> + lid->handle.handler = handler;
> + lid->handle.name = name;
> + lid->handle.private = lid;
> + error = input_register_handle(&lid->handle);
> + if (error)
> + goto err_free_name;
> + error = input_open_device(&lid->handle);
> + if (error)
> + goto err_unregister_handle;
> + return 0;
> +err_unregister_handle:
> + input_unregister_handle(&lid->handle);
> +err_free_name:
> + kfree(name);
> +err_free_lid:
> + kfree(lid);
> + return error;
> +}
> +
> +static void elan_input_lid_disconnect(struct input_handle *handle) {
> + struct elan_input_lid *lid = handle->private;
> +
> + input_close_device(handle);
> + input_unregister_handle(handle);
> + kfree(handle->name);
> + kfree(lid);
> +}
> +
> +static const struct input_device_id elan_input_lid_ids[] = {
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
INPUT_DEVICE_ID_MATCH_SWBIT,
> + .evbit = { BIT_MASK(EV_SW) },
> + .swbit = { [BIT_WORD(SW_LID)] = BIT_MASK(SW_LID) },
> + },
> + { },
> +};
> +
> +static struct input_handler elan_input_lid_handler = {
> + .event = elan_input_lid_event,
> + .connect = elan_input_lid_connect,
> + .disconnect = elan_input_lid_disconnect,
> + .name = "elan-i2c-lid",
> + .id_table = elan_input_lid_ids,
> +};
> +
> +static int elan_create_lid_handler(struct elan_tp_data *data) {
> + int error = 0;
> +
> + elan_mode_wq = create_singlethread_workqueue("elan-i2c-lid");
> + if (elan_mode_wq == NULL)
> + return -ENOMEM;
> + error = input_register_handler(&elan_input_lid_handler);
> + if (error)
> + goto remove_wq;
> +
> + data->lid_switch = true;
> + INIT_LIST_HEAD(&data->list);
> + INIT_WORK(&data->lid_work, lid_work_handler);
> + list_add_tail(&data->list, &elan_devices_with_lid_handler);

It looks like you call elan_create_lid_handler() from elan_probe() which
means it can be called several times (we should not assume there is only one
controller), I do not see it being destroyed in remove() either, so it will
break if you bind/unbind the driver.

I also not sure why you need the list of you have a handler per device.

> +
> + return 0;
> +
> +remove_wq:
> + data->lid_switch = false;
> + destroy_workqueue(elan_mode_wq);
> + elan_mode_wq = NULL;
> + return error;
> +}
> +
> static int elan_probe(struct i2c_client *client) {
> const struct elan_transport_ops *transport_ops; @@ -1325,6 +1520,10
> @@ static int elan_probe(struct i2c_client *client)
> }
> }
>
> + error = elan_create_lid_handler(data);
> + if (error)
> + dev_err(dev, "failed to create lid handler: %d\n", error);

Do we need this on _ALL_ devices with ELan controllers, or just certain
ones? If we need this on all devices how did it work before?

> +
> return 0;
> }
>
> @@ -1334,6 +1533,10 @@ static int elan_suspend(struct device *dev)
> struct elan_tp_data *data = i2c_get_clientdata(client);
> int ret;
>
> + /* Wait for switch on completion */
> + if (data->lid_switch)
> + flush_workqueue(elan_mode_wq);
> +
> /*
> * We are taking the mutex to make sure sysfs operations are
> * complete before we attempt to bring the device into low[er] @@
> -1371,6 +1574,10 @@ static int elan_resume(struct device *dev)
> struct elan_tp_data *data = i2c_get_clientdata(client);
> int error;
>
> + /* Wait for switch on completion */
> + if (data->lid_switch)
> + flush_workqueue(elan_mode_wq);
> +
> if (!device_may_wakeup(dev)) {
> error = regulator_enable(data->vcc);
> if (error) {
> --
> 2.34.1
>

Thanks.

--
Dmitry

2023-07-31 07:11:18

by Dmitry Torokhov

[permalink] [raw]

Subject: Re: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit functions.

Hi Jingle,

On Wed, Jul 05, 2023 at 02:09:18PM +0800, Jingle.Wu wrote:
> HI Dmitry:
>
> 1.
> > +static void elan_input_lid_event(struct input_handle *handle,
> > +unsigned
> int type,
> > + unsigned int code, int value) {
> > + struct elan_tp_data *data, *n;
> > +
> > + if (type == EV_SW && code == SW_LID) {
> > + list_for_each_entry_safe(data, n,
> &elan_devices_with_lid_handler,
> > +list) {
>
> Why do you need the "_safe()" variant here?
> -> Because I took the above link as reference.
> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3
> 578852/7/drivers/hid/hid-multitouch.c#394

It is wrong there too. The _safe() variant protects list traversal when
the list is being modified by the same thread, here we do not do that.

>
> 2.
> > + data->lid_switch = true;
> > + INIT_LIST_HEAD(&data->list);
> > + INIT_WORK(&data->lid_work, lid_work_handler);
> > + list_add_tail(&data->list, &elan_devices_with_lid_handler);
>
> It looks like you call elan_create_lid_handler() from elan_probe() which
> means it can be called several times (we should not assume there is only one
> controller), I do not see it being destroyed in remove() either, so it will
> break if you bind/unbind the driver.
>
> I also not sure why you need the list of you have a handler per device.
>
> -> If the elan_create_lid_handler() function not be put into probe(), the
> lid
> handler would be invalid in elan private data("struct elan_tp_data *data").
> Or do you have any suggestions for it? Thanks.

The handler's connect() is called for each matching device so you can
tie it up at that time.

>
> 3.
> > + error = elan_create_lid_handler(data);
> > + if (error)
> > + dev_err(dev, "failed to create lid handler: %d\n", error);
>
> Do we need this on _ALL_ devices with ELan controllers, or just certain
> ones? If we need this on all devices how did it work before?
>
> -> Yes, we need this on all device.
> In Chromeos kernel v5.4 before, the elan_inhibit()/uninhibit function was
> built and worked
> successfully in elan_i2c_driver.
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads
> /chromeos-5.4/drivers/input/mouse/elan_i2c_core.c#353

We have the Elan touchpad driver without this functionality for many
years. I am aware that certain devices need this, but the fact that
Chrome OS kernel 5.4 (which is only used on a subset of Chromebooks) has
it does not necessarily mean that this functionality is needed on _ALL_
devices.

Thanks.

--
Dmitry