2023-10-09 12:12:29

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH] Input: cyapa - add missing input core locking to suspend/resume functions

Grab input->mutex during suspend/resume functions like it is done in
other input drivers. This fixes the following warning during system
suspend/resume cycle on Samsung Exynos5250-based Snow Chromebook:

------------[ cut here ]------------
WARNING: CPU: 1 PID: 1680 at drivers/input/input.c:2291 input_device_enabled+0x68/0x6c
Modules linked in: ...
CPU: 1 PID: 1680 Comm: kworker/u4:12 Tainted: G W 6.6.0-rc5-next-20231009 #14109
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: events_unbound async_run_entry_fn
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x58/0x70
dump_stack_lvl from __warn+0x1a8/0x1cc
__warn from warn_slowpath_fmt+0x18c/0x1b4
warn_slowpath_fmt from input_device_enabled+0x68/0x6c
input_device_enabled from cyapa_gen3_set_power_mode+0x13c/0x1dc
cyapa_gen3_set_power_mode from cyapa_reinitialize+0x10c/0x15c
cyapa_reinitialize from cyapa_resume+0x48/0x98
cyapa_resume from dpm_run_callback+0x90/0x298
dpm_run_callback from device_resume+0xb4/0x258
device_resume from async_resume+0x20/0x64
async_resume from async_run_entry_fn+0x40/0x15c
async_run_entry_fn from process_scheduled_works+0xbc/0x6a8
process_scheduled_works from worker_thread+0x188/0x454
worker_thread from kthread+0x108/0x140
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf1625fb0 to 0xf1625ff8)
...
---[ end trace 0000000000000000 ]---
...
------------[ cut here ]------------
WARNING: CPU: 1 PID: 1680 at drivers/input/input.c:2291 input_device_enabled+0x68/0x6c
Modules linked in: ...
CPU: 1 PID: 1680 Comm: kworker/u4:12 Tainted: G W 6.6.0-rc5-next-20231009 #14109
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: events_unbound async_run_entry_fn
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x58/0x70
dump_stack_lvl from __warn+0x1a8/0x1cc
__warn from warn_slowpath_fmt+0x18c/0x1b4
warn_slowpath_fmt from input_device_enabled+0x68/0x6c
input_device_enabled from cyapa_gen3_set_power_mode+0x13c/0x1dc
cyapa_gen3_set_power_mode from cyapa_reinitialize+0x10c/0x15c
cyapa_reinitialize from cyapa_resume+0x48/0x98
cyapa_resume from dpm_run_callback+0x90/0x298
dpm_run_callback from device_resume+0xb4/0x258
device_resume from async_resume+0x20/0x64
async_resume from async_run_entry_fn+0x40/0x15c
async_run_entry_fn from process_scheduled_works+0xbc/0x6a8
process_scheduled_works from worker_thread+0x188/0x454
worker_thread from kthread+0x108/0x140
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf1625fb0 to 0xf1625ff8)
...
---[ end trace 0000000000000000 ]---

Fixes: d69f0a43c677 ("Input: use input_device_enabled()")
Signed-off-by: Marek Szyprowski <[email protected]>
---
drivers/input/mouse/cyapa.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
index a84098448f5b..cf23f95b5f11 100644
--- a/drivers/input/mouse/cyapa.c
+++ b/drivers/input/mouse/cyapa.c
@@ -1347,10 +1347,16 @@ static int cyapa_suspend(struct device *dev)
u8 power_mode;
int error;

- error = mutex_lock_interruptible(&cyapa->state_sync_lock);
+ error = mutex_lock_interruptible(&cyapa->input->mutex);
if (error)
return error;

+ error = mutex_lock_interruptible(&cyapa->state_sync_lock);
+ if (error) {
+ mutex_unlock(&cyapa->input->mutex);
+ return error;
+ }
+
/*
* Runtime PM is enable only when device is in operational mode and
* users in use, so need check it before disable it to
@@ -1385,6 +1391,8 @@ static int cyapa_suspend(struct device *dev)
cyapa->irq_wake = (enable_irq_wake(client->irq) == 0);

mutex_unlock(&cyapa->state_sync_lock);
+ mutex_unlock(&cyapa->input->mutex);
+
return 0;
}

@@ -1394,6 +1402,7 @@ static int cyapa_resume(struct device *dev)
struct cyapa *cyapa = i2c_get_clientdata(client);
int error;

+ mutex_lock(&cyapa->input->mutex);
mutex_lock(&cyapa->state_sync_lock);

if (device_may_wakeup(dev) && cyapa->irq_wake) {
@@ -1412,6 +1421,7 @@ static int cyapa_resume(struct device *dev)
enable_irq(client->irq);

mutex_unlock(&cyapa->state_sync_lock);
+ mutex_unlock(&cyapa->input->mutex);
return 0;
}

--
2.34.1


2024-04-25 10:02:39

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH] Input: cyapa - add missing input core locking to suspend/resume functions

On 09.10.2023 14:10, Marek Szyprowski wrote:
> Grab input->mutex during suspend/resume functions like it is done in
> other input drivers. This fixes the following warning during system
> suspend/resume cycle on Samsung Exynos5250-based Snow Chromebook:
>
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1680 at drivers/input/input.c:2291 input_device_enabled+0x68/0x6c
> Modules linked in: ...
> CPU: 1 PID: 1680 Comm: kworker/u4:12 Tainted: G W 6.6.0-rc5-next-20231009 #14109
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: events_unbound async_run_entry_fn
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x58/0x70
> dump_stack_lvl from __warn+0x1a8/0x1cc
> __warn from warn_slowpath_fmt+0x18c/0x1b4
> warn_slowpath_fmt from input_device_enabled+0x68/0x6c
> input_device_enabled from cyapa_gen3_set_power_mode+0x13c/0x1dc
> cyapa_gen3_set_power_mode from cyapa_reinitialize+0x10c/0x15c
> cyapa_reinitialize from cyapa_resume+0x48/0x98
> cyapa_resume from dpm_run_callback+0x90/0x298
> dpm_run_callback from device_resume+0xb4/0x258
> device_resume from async_resume+0x20/0x64
> async_resume from async_run_entry_fn+0x40/0x15c
> async_run_entry_fn from process_scheduled_works+0xbc/0x6a8
> process_scheduled_works from worker_thread+0x188/0x454
> worker_thread from kthread+0x108/0x140
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf1625fb0 to 0xf1625ff8)
> ...
> ---[ end trace 0000000000000000 ]---
> ...
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1680 at drivers/input/input.c:2291 input_device_enabled+0x68/0x6c
> Modules linked in: ...
> CPU: 1 PID: 1680 Comm: kworker/u4:12 Tainted: G W 6.6.0-rc5-next-20231009 #14109
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: events_unbound async_run_entry_fn
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x58/0x70
> dump_stack_lvl from __warn+0x1a8/0x1cc
> __warn from warn_slowpath_fmt+0x18c/0x1b4
> warn_slowpath_fmt from input_device_enabled+0x68/0x6c
> input_device_enabled from cyapa_gen3_set_power_mode+0x13c/0x1dc
> cyapa_gen3_set_power_mode from cyapa_reinitialize+0x10c/0x15c
> cyapa_reinitialize from cyapa_resume+0x48/0x98
> cyapa_resume from dpm_run_callback+0x90/0x298
> dpm_run_callback from device_resume+0xb4/0x258
> device_resume from async_resume+0x20/0x64
> async_resume from async_run_entry_fn+0x40/0x15c
> async_run_entry_fn from process_scheduled_works+0xbc/0x6a8
> process_scheduled_works from worker_thread+0x188/0x454
> worker_thread from kthread+0x108/0x140
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf1625fb0 to 0xf1625ff8)
> ...
> ---[ end trace 0000000000000000 ]---
>
> Fixes: d69f0a43c677 ("Input: use input_device_enabled()")
> Signed-off-by: Marek Szyprowski <[email protected]>

Gentle ping?

> ---
> drivers/input/mouse/cyapa.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> index a84098448f5b..cf23f95b5f11 100644
> --- a/drivers/input/mouse/cyapa.c
> +++ b/drivers/input/mouse/cyapa.c
> @@ -1347,10 +1347,16 @@ static int cyapa_suspend(struct device *dev)
> u8 power_mode;
> int error;
>
> - error = mutex_lock_interruptible(&cyapa->state_sync_lock);
> + error = mutex_lock_interruptible(&cyapa->input->mutex);
> if (error)
> return error;
>
> + error = mutex_lock_interruptible(&cyapa->state_sync_lock);
> + if (error) {
> + mutex_unlock(&cyapa->input->mutex);
> + return error;
> + }
> +
> /*
> * Runtime PM is enable only when device is in operational mode and
> * users in use, so need check it before disable it to
> @@ -1385,6 +1391,8 @@ static int cyapa_suspend(struct device *dev)
> cyapa->irq_wake = (enable_irq_wake(client->irq) == 0);
>
> mutex_unlock(&cyapa->state_sync_lock);
> + mutex_unlock(&cyapa->input->mutex);
> +
> return 0;
> }
>
> @@ -1394,6 +1402,7 @@ static int cyapa_resume(struct device *dev)
> struct cyapa *cyapa = i2c_get_clientdata(client);
> int error;
>
> + mutex_lock(&cyapa->input->mutex);
> mutex_lock(&cyapa->state_sync_lock);
>
> if (device_may_wakeup(dev) && cyapa->irq_wake) {
> @@ -1412,6 +1421,7 @@ static int cyapa_resume(struct device *dev)
> enable_irq(client->irq);
>
> mutex_unlock(&cyapa->state_sync_lock);
> + mutex_unlock(&cyapa->input->mutex);
> return 0;
> }
>

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland


2024-04-25 17:04:54

by Andrzej Pietrasiewicz

[permalink] [raw]
Subject: Re: [PATCH] Input: cyapa - add missing input core locking to suspend/resume functions

Hi Marek,

W dniu 25.04.2024 o 12:02, Marek Szyprowski pisze:
> On 09.10.2023 14:10, Marek Szyprowski wrote:
>> Grab input->mutex during suspend/resume functions like it is done in
>> other input drivers. This fixes the following warning during system
>> suspend/resume cycle on Samsung Exynos5250-based Snow Chromebook:
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 1 PID: 1680 at drivers/input/input.c:2291 input_device_enabled+0x68/0x6c
>> Modules linked in: ...
>> CPU: 1 PID: 1680 Comm: kworker/u4:12 Tainted: G W 6.6.0-rc5-next-20231009 #14109
>> Hardware name: Samsung Exynos (Flattened Device Tree)
>> Workqueue: events_unbound async_run_entry_fn
>> unwind_backtrace from show_stack+0x10/0x14
>> show_stack from dump_stack_lvl+0x58/0x70
>> dump_stack_lvl from __warn+0x1a8/0x1cc
>> __warn from warn_slowpath_fmt+0x18c/0x1b4
>> warn_slowpath_fmt from input_device_enabled+0x68/0x6c
>> input_device_enabled from cyapa_gen3_set_power_mode+0x13c/0x1dc
>> cyapa_gen3_set_power_mode from cyapa_reinitialize+0x10c/0x15c
>> cyapa_reinitialize from cyapa_resume+0x48/0x98
>> cyapa_resume from dpm_run_callback+0x90/0x298
>> dpm_run_callback from device_resume+0xb4/0x258
>> device_resume from async_resume+0x20/0x64
>> async_resume from async_run_entry_fn+0x40/0x15c
>> async_run_entry_fn from process_scheduled_works+0xbc/0x6a8
>> process_scheduled_works from worker_thread+0x188/0x454
>> worker_thread from kthread+0x108/0x140
>> kthread from ret_from_fork+0x14/0x28
>> Exception stack(0xf1625fb0 to 0xf1625ff8)
>> ...
>> ---[ end trace 0000000000000000 ]---
>> ...
>> ------------[ cut here ]------------
>> WARNING: CPU: 1 PID: 1680 at drivers/input/input.c:2291 input_device_enabled+0x68/0x6c
>> Modules linked in: ...
>> CPU: 1 PID: 1680 Comm: kworker/u4:12 Tainted: G W 6.6.0-rc5-next-20231009 #14109
>> Hardware name: Samsung Exynos (Flattened Device Tree)
>> Workqueue: events_unbound async_run_entry_fn
>> unwind_backtrace from show_stack+0x10/0x14
>> show_stack from dump_stack_lvl+0x58/0x70
>> dump_stack_lvl from __warn+0x1a8/0x1cc
>> __warn from warn_slowpath_fmt+0x18c/0x1b4
>> warn_slowpath_fmt from input_device_enabled+0x68/0x6c
>> input_device_enabled from cyapa_gen3_set_power_mode+0x13c/0x1dc
>> cyapa_gen3_set_power_mode from cyapa_reinitialize+0x10c/0x15c
>> cyapa_reinitialize from cyapa_resume+0x48/0x98
>> cyapa_resume from dpm_run_callback+0x90/0x298
>> dpm_run_callback from device_resume+0xb4/0x258
>> device_resume from async_resume+0x20/0x64
>> async_resume from async_run_entry_fn+0x40/0x15c
>> async_run_entry_fn from process_scheduled_works+0xbc/0x6a8
>> process_scheduled_works from worker_thread+0x188/0x454
>> worker_thread from kthread+0x108/0x140
>> kthread from ret_from_fork+0x14/0x28
>> Exception stack(0xf1625fb0 to 0xf1625ff8)
>> ...
>> ---[ end trace 0000000000000000 ]---
>>
>> Fixes: d69f0a43c677 ("Input: use input_device_enabled()")
>> Signed-off-by: Marek Szyprowski <[email protected]>
>
> Gentle ping?

Thanks for pinging this gently ;)

Unfortunately I am unable to test it now, but I assume this
fixes the problem for you. If so, you can add my

Reviewed-by: Andrzej Pietrasiewicz <[email protected]>

>
>> ---
>> drivers/input/mouse/cyapa.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
>> index a84098448f5b..cf23f95b5f11 100644
>> --- a/drivers/input/mouse/cyapa.c
>> +++ b/drivers/input/mouse/cyapa.c
>> @@ -1347,10 +1347,16 @@ static int cyapa_suspend(struct device *dev)
>> u8 power_mode;
>> int error;
>>
>> - error = mutex_lock_interruptible(&cyapa->state_sync_lock);
>> + error = mutex_lock_interruptible(&cyapa->input->mutex);
>> if (error)
>> return error;
>>
>> + error = mutex_lock_interruptible(&cyapa->state_sync_lock);
>> + if (error) {
>> + mutex_unlock(&cyapa->input->mutex);
>> + return error;
>> + }
>> +
>> /*
>> * Runtime PM is enable only when device is in operational mode and
>> * users in use, so need check it before disable it to
>> @@ -1385,6 +1391,8 @@ static int cyapa_suspend(struct device *dev)
>> cyapa->irq_wake = (enable_irq_wake(client->irq) == 0);
>>
>> mutex_unlock(&cyapa->state_sync_lock);
>> + mutex_unlock(&cyapa->input->mutex);
>> +
>> return 0;
>> }
>>
>> @@ -1394,6 +1402,7 @@ static int cyapa_resume(struct device *dev)
>> struct cyapa *cyapa = i2c_get_clientdata(client);
>> int error;
>>
>> + mutex_lock(&cyapa->input->mutex);
>> mutex_lock(&cyapa->state_sync_lock);
>>
>> if (device_may_wakeup(dev) && cyapa->irq_wake) {
>> @@ -1412,6 +1421,7 @@ static int cyapa_resume(struct device *dev)
>> enable_irq(client->irq);
>>
>> mutex_unlock(&cyapa->state_sync_lock);
>> + mutex_unlock(&cyapa->input->mutex);
>> return 0;
>> }
>>
>
> Best regards