gpio_keys module can either accept gpios or interrupts. The module
initializes delayed work in case of gpios only and not for interrupts,
so make sure cancel_delayed_work_sync() is called only when bdata->gpiod
is true.
This fixes the issue seen below when the gpio_keys module is unloaded and
an interrupt pin is used instead of GPIO:
[ 360.297569] ------------[ cut here ]------------
[ 360.302303] WARNING: CPU: 0 PID: 237 at kernel/workqueue.c:3066 __flush_work+0x414/0x470
[ 360.310531] Modules linked in: gpio_keys(-)
[ 360.314797] CPU: 0 PID: 237 Comm: rmmod Not tainted 5.18.0-rc5-arm64-renesas-00116-g73636105874d-dirty #166
[ 360.324662] Hardware name: Renesas SMARC EVK based on r9a07g054l2 (DT)
[ 360.331270] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 360.338318] pc : __flush_work+0x414/0x470
[ 360.342385] lr : __cancel_work_timer+0x140/0x1b0
[ 360.347065] sp : ffff80000a7fba00
[ 360.350423] x29: ffff80000a7fba00 x28: ffff000012b9c5c0 x27: 0000000000000000
[ 360.357664] x26: ffff80000a7fbb80 x25: ffff80000954d0a8 x24: 0000000000000001
[ 360.364904] x23: ffff800009757000 x22: 0000000000000000 x21: ffff80000919b000
[ 360.372143] x20: ffff00000f5974e0 x19: ffff00000f5974e0 x18: ffff8000097fcf48
[ 360.379382] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000053f40
[ 360.386622] x14: ffff800009850e88 x13: 0000000000000002 x12: 000000000000a60c
[ 360.393861] x11: 000000000000a610 x10: 0000000000000000 x9 : 0000000000000008
[ 360.401100] x8 : 0101010101010101 x7 : 00000000a473c394 x6 : 0080808080808080
[ 360.408339] x5 : 0000000000000001 x4 : 0000000000000000 x3 : ffff80000919b458
[ 360.415578] x2 : ffff8000097577f0 x1 : 0000000000000001 x0 : 0000000000000000
[ 360.422818] Call trace:
[ 360.425299] __flush_work+0x414/0x470
[ 360.429012] __cancel_work_timer+0x140/0x1b0
[ 360.433340] cancel_delayed_work_sync+0x10/0x18
[ 360.437931] gpio_keys_quiesce_key+0x28/0x58 [gpio_keys]
[ 360.443327] devm_action_release+0x10/0x18
[ 360.447481] release_nodes+0x8c/0x1a0
[ 360.451194] devres_release_all+0x90/0x100
[ 360.455346] device_unbind_cleanup+0x14/0x60
[ 360.459677] device_release_driver_internal+0xe8/0x168
[ 360.464883] driver_detach+0x4c/0x90
[ 360.468509] bus_remove_driver+0x54/0xb0
[ 360.472485] driver_unregister+0x2c/0x58
[ 360.476462] platform_driver_unregister+0x10/0x18
[ 360.481230] gpio_keys_exit+0x14/0x828 [gpio_keys]
[ 360.486088] __arm64_sys_delete_module+0x1e0/0x270
[ 360.490945] invoke_syscall+0x40/0xf8
[ 360.494661] el0_svc_common.constprop.3+0xf0/0x110
[ 360.499515] do_el0_svc+0x20/0x78
[ 360.502877] el0_svc+0x48/0xf8
[ 360.505977] el0t_64_sync_handler+0x88/0xb0
[ 360.510216] el0t_64_sync+0x148/0x14c
[ 360.513930] irq event stamp: 4306
[ 360.517288] hardirqs last enabled at (4305): [<ffff8000080b0300>] __cancel_work_timer+0x130/0x1b0
[ 360.526359] hardirqs last disabled at (4306): [<ffff800008d194fc>] el1_dbg+0x24/0x88
[ 360.534204] softirqs last enabled at (4278): [<ffff8000080104a0>] _stext+0x4a0/0x5e0
[ 360.542133] softirqs last disabled at (4267): [<ffff8000080932ac>] irq_exit_rcu+0x18c/0x1b0
[ 360.550591] ---[ end trace 0000000000000000 ]---
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/input/keyboard/gpio_keys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index d75a8b179a8a..ec9d50ddda42 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -133,7 +133,7 @@ static void gpio_keys_quiesce_key(void *data)
hrtimer_cancel(&bdata->release_timer);
if (bdata->debounce_use_hrtimer)
hrtimer_cancel(&bdata->debounce_timer);
- else
+ else if (bdata->gpiod)
cancel_delayed_work_sync(&bdata->work);
}
--
2.17.1
On Fri, May 13, 2022 at 6:44 PM Lad Prabhakar
<[email protected]> wrote:
> gpio_keys module can either accept gpios or interrupts. The module
> initializes delayed work in case of gpios only and not for interrupts,
> so make sure cancel_delayed_work_sync() is called only when bdata->gpiod
> is true.
>
> This fixes the issue seen below when the gpio_keys module is unloaded and
> an interrupt pin is used instead of GPIO:
>
> [ 360.297569] ------------[ cut here ]------------
> [ 360.302303] WARNING: CPU: 0 PID: 237 at kernel/workqueue.c:3066 __flush_work+0x414/0x470
> [ 360.310531] Modules linked in: gpio_keys(-)
> [ 360.314797] CPU: 0 PID: 237 Comm: rmmod Not tainted 5.18.0-rc5-arm64-renesas-00116-g73636105874d-dirty #166
> [ 360.324662] Hardware name: Renesas SMARC EVK based on r9a07g054l2 (DT)
> [ 360.331270] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 360.338318] pc : __flush_work+0x414/0x470
> [ 360.342385] lr : __cancel_work_timer+0x140/0x1b0
> [ 360.347065] sp : ffff80000a7fba00
> [ 360.350423] x29: ffff80000a7fba00 x28: ffff000012b9c5c0 x27: 0000000000000000
> [ 360.357664] x26: ffff80000a7fbb80 x25: ffff80000954d0a8 x24: 0000000000000001
> [ 360.364904] x23: ffff800009757000 x22: 0000000000000000 x21: ffff80000919b000
> [ 360.372143] x20: ffff00000f5974e0 x19: ffff00000f5974e0 x18: ffff8000097fcf48
> [ 360.379382] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000053f40
> [ 360.386622] x14: ffff800009850e88 x13: 0000000000000002 x12: 000000000000a60c
> [ 360.393861] x11: 000000000000a610 x10: 0000000000000000 x9 : 0000000000000008
> [ 360.401100] x8 : 0101010101010101 x7 : 00000000a473c394 x6 : 0080808080808080
> [ 360.408339] x5 : 0000000000000001 x4 : 0000000000000000 x3 : ffff80000919b458
> [ 360.415578] x2 : ffff8000097577f0 x1 : 0000000000000001 x0 : 0000000000000000
> [ 360.422818] Call trace:
> [ 360.425299] __flush_work+0x414/0x470
> [ 360.429012] __cancel_work_timer+0x140/0x1b0
> [ 360.433340] cancel_delayed_work_sync+0x10/0x18
> [ 360.437931] gpio_keys_quiesce_key+0x28/0x58 [gpio_keys]
> [ 360.443327] devm_action_release+0x10/0x18
> [ 360.447481] release_nodes+0x8c/0x1a0
> [ 360.451194] devres_release_all+0x90/0x100
> [ 360.455346] device_unbind_cleanup+0x14/0x60
> [ 360.459677] device_release_driver_internal+0xe8/0x168
> [ 360.464883] driver_detach+0x4c/0x90
> [ 360.468509] bus_remove_driver+0x54/0xb0
> [ 360.472485] driver_unregister+0x2c/0x58
> [ 360.476462] platform_driver_unregister+0x10/0x18
> [ 360.481230] gpio_keys_exit+0x14/0x828 [gpio_keys]
> [ 360.486088] __arm64_sys_delete_module+0x1e0/0x270
> [ 360.490945] invoke_syscall+0x40/0xf8
> [ 360.494661] el0_svc_common.constprop.3+0xf0/0x110
> [ 360.499515] do_el0_svc+0x20/0x78
> [ 360.502877] el0_svc+0x48/0xf8
> [ 360.505977] el0t_64_sync_handler+0x88/0xb0
> [ 360.510216] el0t_64_sync+0x148/0x14c
> [ 360.513930] irq event stamp: 4306
> [ 360.517288] hardirqs last enabled at (4305): [<ffff8000080b0300>] __cancel_work_timer+0x130/0x1b0
> [ 360.526359] hardirqs last disabled at (4306): [<ffff800008d194fc>] el1_dbg+0x24/0x88
> [ 360.534204] softirqs last enabled at (4278): [<ffff8000080104a0>] _stext+0x4a0/0x5e0
> [ 360.542133] softirqs last disabled at (4267): [<ffff8000080932ac>] irq_exit_rcu+0x18c/0x1b0
> [ 360.550591] ---[ end trace 0000000000000000 ]---
>
> Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Lad,
On Fri, May 13, 2022 at 02:25:00PM +0100, Lad Prabhakar wrote:
> gpio_keys module can either accept gpios or interrupts. The module
> initializes delayed work in case of gpios only and not for interrupts,
> so make sure cancel_delayed_work_sync() is called only when bdata->gpiod
> is true.
...
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index d75a8b179a8a..ec9d50ddda42 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -133,7 +133,7 @@ static void gpio_keys_quiesce_key(void *data)
> hrtimer_cancel(&bdata->release_timer);
> if (bdata->debounce_use_hrtimer)
> hrtimer_cancel(&bdata->debounce_timer);
> - else
> + else if (bdata->gpiod)
> cancel_delayed_work_sync(&bdata->work);
We already have a check for bdata->gpiod a couple lines above. I think
the chunk should look like this:
if (!bdata->gpiod)
hrtimer_cancel(&bdata->release_timer);
else if (bdata->debounce_use_hrtimer)
hrtimer_cancel(&bdata->debounce_timer);
else
cancel_delayed_work_sync(&bdata->work);
since we use debounce timer/work only when we deal with gpio-backed
keys.
Thanks.
--
Dmitry
Hi Dmitry,
Thank you for the review.
On Wed, May 18, 2022 at 5:46 AM Dmitry Torokhov
<[email protected]> wrote:
>
> Hi Lad,
>
> On Fri, May 13, 2022 at 02:25:00PM +0100, Lad Prabhakar wrote:
> > gpio_keys module can either accept gpios or interrupts. The module
> > initializes delayed work in case of gpios only and not for interrupts,
> > so make sure cancel_delayed_work_sync() is called only when bdata->gpiod
> > is true.
> ...
> > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> > index d75a8b179a8a..ec9d50ddda42 100644
> > --- a/drivers/input/keyboard/gpio_keys.c
> > +++ b/drivers/input/keyboard/gpio_keys.c
> > @@ -133,7 +133,7 @@ static void gpio_keys_quiesce_key(void *data)
> > hrtimer_cancel(&bdata->release_timer);
> > if (bdata->debounce_use_hrtimer)
> > hrtimer_cancel(&bdata->debounce_timer);
> > - else
> > + else if (bdata->gpiod)
> > cancel_delayed_work_sync(&bdata->work);
>
> We already have a check for bdata->gpiod a couple lines above. I think
> the chunk should look like this:
>
> if (!bdata->gpiod)
> hrtimer_cancel(&bdata->release_timer);
> else if (bdata->debounce_use_hrtimer)
> hrtimer_cancel(&bdata->debounce_timer);
> else
> cancel_delayed_work_sync(&bdata->work);
>
> since we use debounce timer/work only when we deal with gpio-backed
> keys.
>
Agreed, will fix that in v2.
Cheers,
Prabhakar