Re: [PATCH] Input: elan_i2c: Disable irq on shutdown

Dmitry Torokhov
2021-05-11T02:02:04Z

Hi Stephen,

On Mon, May 10, 2021 at 03:00:12PM -0700, Stephen Boyd wrote:
> Touching an elan trackpad while shutting down the system sometimes leads
> to the following warning from i2c core. This is because the irq is still
> active and working, but the i2c bus for the device has been shutdown
> already. If the bus has been taken down then we shouldn't expect
> transfers to work. Disable the irq on shutdown so that this driver
> doesn't try to get the report in the irq handler after the i2c bus is
> shutdown.
>
> i2c i2c-7: Transfer while suspended
> WARNING: CPU: 0 PID: 196 at drivers/i2c/i2c-core.h:54 __i2c_transfer+0xb8/0x38c
> Modules linked in: rfcomm algif_hash algif_skcipher af_alg uinput xt_cgroup
> CPU: 0 PID: 196 Comm: irq/166-ekth300 Not tainted 5.4.115 #96
> Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
> pstate: 60c00009 (nZCv daif +PAN +UAO)
> pc : __i2c_transfer+0xb8/0x38c
> lr : __i2c_transfer+0xb8/0x38c
> sp : ffffffc011793c20
> x29: ffffffc011793c20 x28: 0000000000000000
> x27: ffffff85efd60348 x26: ffffff85efdb8040
> x25: ffffffec39d579cc x24: ffffffec39d57bac
> x23: ffffffec3aab17b9 x22: ffffff85f02d6400
> x21: 0000000000000001 x20: ffffff85f02aa190
> x19: ffffff85f02aa100 x18: 00000000ffff0a10
> x17: 0000000000000044 x16: 00000000000000ec
> x15: ffffffec3a0b9174 x14: 0000000000000006
> x13: 00000000003fe680 x12: 0000000000000000
> x11: 0000000000000000 x10: 00000000ffffffff
> x9 : 806da3cb9f8c1d00 x8 : 806da3cb9f8c1d00
> x7 : 0000000000000000 x6 : ffffffec3afd3bef
> x5 : 0000000000000000 x4 : 0000000000000000
> x3 : 0000000000000000 x2 : fffffffffffffcc7
> x1 : 0000000000000000 x0 : 0000000000000023
> Call trace:
> __i2c_transfer+0xb8/0x38c
> i2c_transfer+0xa0/0xf4
> i2c_transfer_buffer_flags+0x64/0x98
> elan_i2c_get_report+0x2c/0x88
> elan_isr+0x68/0x3e4
> irq_thread_fn+0x2c/0x70
> irq_thread+0xf8/0x148
> kthread+0x140/0x17c
> ret_from_fork+0x10/0x18

This does not seem to me that it is Elan-specific issue. I wonder if
this should be pushed into I2C core to shut off client->irq in shutdown
for everyone.

>
> Cc: Jingle Wu <jingle.wu@emc.com.tw>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> drivers/input/mouse/elan_i2c_core.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index bef73822315d..6f64992e70d1 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -1338,6 +1338,22 @@ static int elan_probe(struct i2c_client *client,
> return 0;
> }
>
> +static void elan_shutdown(struct i2c_client *client)
> +{
> + struct elan_tp_data *data = i2c_get_clientdata(client);
> +
> + /*
> + * Make sure we don't access i2c bus after it is shutdown.
> + *
> + * We are taking the mutex to make sure sysfs operations are
> + * complete before we attempt to silence the device by disabling
> + * the irq.
> + */

I do not think important on shutdown as I expect we'll stop/kill
userspace first, and then fully reinitialize device on subsequent boot.

Thanks.

--
Dmitry