Messages in this thread | | | Date | Tue, 15 Feb 2022 14:20:27 -0800 | From | Guenter Roeck <> | Subject | Re: [PATCH v2] ARM: dts: sun8i: Adjust power key nodes |
| |
On 2/15/22 12:34, Jernej Škrabec wrote: > Hi! > > Dne torek, 15. februar 2022 ob 01:27:32 CET je Guenter Roeck napisal(a): >> Hi, >> >> On Mon, Nov 29, 2021 at 05:55:10PM +0100, Jernej Skrabec wrote: >>> Several H3 and one H2+ board have power key nodes, which are slightly >>> off. Some are missing wakeup-source property and some have BTN_0 code >>> assigned instead of KEY_POWER. >>> >>> Adjust them, so they can function as intended by designer. >>> >>> Co-developed-by: Michael Klein <michael@fossekall.de> >>> Signed-off-by: Michael Klein <michael@fossekall.de> >>> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> >> >> This patch results in the following traceback when rebooting an >> orangepi-pc qemu emulation. >> >> [ 30.899594] >> [ 30.899685] ============================================ >> [ 30.899757] WARNING: possible recursive locking detected >> [ 30.899938] 5.17.0-rc3-00394-gc849047c2473 #1 Not tainted >> [ 30.900055] -------------------------------------------- >> [ 30.900124] init/307 is trying to acquire lock: >> [ 30.900246] c2dfe27c (&irq_desc_lock_class){-.-.}-{2:2}, at: > __irq_get_desc_lock+0x58/0xa0 >> [ 30.900900] >> [ 30.900900] but task is already holding lock: >> [ 30.900974] c3c0ac7c (&irq_desc_lock_class){-.-.}-{2:2}, at: > __irq_get_desc_lock+0x58/0xa0 >> [ 30.901101] >> [ 30.901101] other info that might help us debug this: >> [ 30.901188] Possible unsafe locking scenario: >> [ 30.901188] >> [ 30.901262] CPU0 >> [ 30.901301] ---- >> [ 30.901339] lock(&irq_desc_lock_class); >> [ 30.901411] lock(&irq_desc_lock_class); >> [ 30.901480] >> [ 30.901480] *** DEADLOCK *** >> [ 30.901480] >> [ 30.901554] May be due to missing lock nesting notation >> [ 30.901554] >> [ 30.901657] 4 locks held by init/307: >> [ 30.901724] #0: c1f29f18 (system_transition_mutex){+.+.}-{3:3}, at: > __do_sys_reboot+0x90/0x23c >> [ 30.901889] #1: c20f7760 (&dev->mutex){....}-{3:3}, at: > device_shutdown+0xf4/0x224 >> [ 30.902016] #2: c2e804d8 (&dev->mutex){....}-{3:3}, at: > device_shutdown+0x104/0x224 >> [ 30.902138] #3: c3c0ac7c (&irq_desc_lock_class){-.-.}-{2:2}, at: > __irq_get_desc_lock+0x58/0xa0 >> [ 30.902281] >> [ 30.902281] stack backtrace: >> [ 30.902462] CPU: 0 PID: 307 Comm: init Not tainted 5.17.0-rc3-00394- > gc849047c2473 #1 >> [ 30.902572] Hardware name: Allwinner sun8i Family >> [ 30.902781] unwind_backtrace from show_stack+0x10/0x14 >> [ 30.902895] show_stack from dump_stack_lvl+0x68/0x90 >> [ 30.902970] dump_stack_lvl from __lock_acquire+0x1680/0x31a0 >> [ 30.903047] __lock_acquire from lock_acquire+0x148/0x3dc >> [ 30.903118] lock_acquire from _raw_spin_lock_irqsave+0x50/0x6c >> [ 30.903197] _raw_spin_lock_irqsave from __irq_get_desc_lock+0x58/0xa0 >> [ 30.903282] __irq_get_desc_lock from irq_set_irq_wake+0x2c/0x19c >> [ 30.903366] irq_set_irq_wake from irq_set_irq_wake+0x13c/0x19c >> [ 30.903442] irq_set_irq_wake from gpio_keys_suspend+0x80/0x1a4 >> [ 30.903523] gpio_keys_suspend from gpio_keys_shutdown+0x10/0x2c >> [ 30.903603] gpio_keys_shutdown from device_shutdown+0x180/0x224 >> [ 30.903685] device_shutdown from __do_sys_reboot+0x134/0x23c >> [ 30.903764] __do_sys_reboot from ret_fast_syscall+0x0/0x1c >> [ 30.903894] Exception stack(0xc584ffa8 to 0xc584fff0) >> [ 30.904013] ffa0: 01234567 000c623f fee1dead 28121969 > 01234567 00000000 >> [ 30.904117] ffc0: 01234567 000c623f 00000001 00000058 000d85c0 00000000 > 00000000 00000000 >> [ 30.904213] ffe0: 000d8298 be84ddf4 000918bc b6eb0edc >> [ 30.905189] reboot: Restarting system >> >> The warning is no longer seen after reverting this patch. >> >> The problem exists but is not seen in v5.17-rc4 because a bug in commit >> 8df89a7cbc63 ("pinctrl-sunxi: don't call pinctrl_gpio_direction()") >> hides it. That problem is fixed with commit 3c5412cdec9f ("pinctrl-sunxi: >> sunxi_pinctrl_gpio_direction_in/output: use correct offset") in linux-next, >> and the traceback is seen there. > > Hm... These DT changes were tested with many users on older kernels for some > time now and new properties conform to bindings. Should we revert pinctrl > changes? >
I don't think those changes were tested with orangepi-pc on real hardware. Maybe I didn't explain it clearly enough: Commit 8df89a7cbc63 does _not_ introduce the problem. It hides the problem. Reverting commit 8df89a7cbc63 won't help but result in exactly the same backtrace (I tried).
Some more details: This commit introduces "wakeup-source;" to various orangepi-pc nodes. This triggers in a call to sunxi_pinctrl_irq_set_wake(), which did not happen before and which may result in the traceback.
Guenter
| |