Hi Geert,
On 2025-09-09 14:17:16 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
>
> On Sat, 30 Aug 2025 at 23:10, Niklas Söderlund
> <niklas.soderlund+renesas@xxxxxxxxxxxx> wrote:
> > This RFC/PATCH tries to address an issue with the Renesas CMT driver
> > design. The driver do PM and clock handling in struct clock_event_device
> > callbacks. This leads to LOCKDEP warnings and I think hints at a larger
> > issue.
> >
> > =============================
> > [ BUG: Invalid wait context ]
> > 6.17.0-rc3-arm64-renesas-03071-gb3c4f4122b28-dirty #21 Not tainted
> > -----------------------------
> > swapper/1/0 is trying to lock:
> > ffff00000898d180 (&dev->power.lock){-...}-{3:3}, at: __pm_runtime_resume+0x38/0x88
> > ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version 0xAF400001/0xDCC63000, Driver version 5.0
> > other info that might help us debug this:
> > ccree e6601000.crypto: ARM ccree device initialized
> > context-{5:5}
> > 2 locks held by swapper/1/0:
> > #0: ffff80008173c298 (tick_broadcast_lock){-...}-{2:2}, at: __tick_broadcast_oneshot_control+0xa4/0x3a8
> > #1: ffff0000089a5858 (&ch->lock){....}-{2:2}
> > usbcore: registered new interface driver usbhid
> > , at: sh_cmt_start+0x30/0x364
> > stack backtrace:
> > CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 6.17.0-rc3-arm64-renesas-03071-gb3c4f4122b28-dirty #21 PREEMPT
> > Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
> > Call trace:
> > show_stack+0x14/0x1c (C)
> > dump_stack_lvl+0x6c/0x90
> > dump_stack+0x14/0x1c
> > __lock_acquire+0x904/0x1584
> > lock_acquire+0x220/0x34c
> > _raw_spin_lock_irqsave+0x58/0x80
> > __pm_runtime_resume+0x38/0x88
> > sh_cmt_start+0x54/0x364
> > sh_cmt_clock_event_set_oneshot+0x64/0xb8
> > clockevents_switch_state+0xfc/0x13c
> > tick_broadcast_set_event+0x30/0xa4
> > __tick_broadcast_oneshot_control+0x1e0/0x3a8
> > tick_broadcast_oneshot_control+0x30/0x40
> > cpuidle_enter_state+0x40c/0x680
> > cpuidle_enter+0x30/0x40
> > do_idle+0x1f4/0x26c
> > cpu_startup_entry+0x34/0x40
> > secondary_start_kernel+0x11c/0x13c
> > __secondary_switched+0x74/0x78
> >
> > This series tries to address this by instead doing PM and clock
> > management at probe time, and leaving them on for the CMT channels that
> > are used as clock events. The CMT design is a bit messy as channels can
> > be used both as clock sources and events. And the design to do the
> > housekeeping for clock sources seems to be valid and is kept.
> >
> > This is posted as an RFC as there is one other driver in-tree that
> > suffers form similar issues. I intend to try and refactor that away too,
> > but would like to try and get some feedback first.
> >
> > The work is tested on R-Car M3N.
> >
> > Niklas Söderlund (2):
> > clocksource/drivers/sh_cmt: Split start/stop of clock source and
> > events
> > clocksource/drivers/sh_cmt: Do not power down channels used for events
>
> Thanks for your series!
>
> I can confirm this fixes the issue on e.g. R-Car Gen3, where the CMT
> can be used for clock events or as a clock source, so
> Tested-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
Thanks for testing this.
>
> On R-Mobile A1, the CMT is also used for periodic clock events.
> Your series seems to fix one invalid context, and expose a "new" one
> (lockdep shuts down after the first report):
Interesting find. Indeed this patch solves on issue just to uncover the
next one. I have reworked the order of when the CMT is enabled in the
probe call path to also solve this new uncovered issue and will post a
v2.
Thanks for finding it, and for allowing me access to a R-Mobile A1 so I
could fix it.
Please note when you test v2 on R-Mobile A1 you will still hit a splat
"BUG: Invalid wait context" as fixing all CMT issues moves on and
exposes issue with TMU (a different driver)... I intend to fix them too
as soon as this CMT solution is acked as I suspect TMU have very similar
issue as I try to fix in CMT with this work.
>
> sh_cmt e6138000.timer: ch0: used for clock events
> sh_cmt e6138000.timer: ch0: used for periodic clock events
>
> =============================
> [ BUG: Invalid wait context ]
> 6.17.0-rc5-armadillo-06055-g57d9d685e295 #872 Not tainted
> -----------------------------
> swapper/0/1 is trying to lock:
> c0e5ae28 (enable_lock){....}-{3:3}, at: clk_enable_lock+0x38/0xc4
> other info that might help us debug this:
> context-{5:5}
> 2 locks held by swapper/0/1:
> #0: c218e488 (&dev->mutex){....}-{4:4}, at: __driver_attach+0xd8/0xf8
> #1: c2201038 (&ch->lock){....}-{2:2}, at: sh_cmt_probe+0x598/0x780
> stack backtrace:
> CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted
> 6.17.0-rc5-armadillo-06055-g57d9d685e295 #872 NONE
> Hardware name: Generic R8A7740 (Flattened Device Tree)
> Call trace:
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x44/0x74
> dump_stack_lvl from __lock_acquire+0x41c/0x17e8
> __lock_acquire from lock_acquire+0x208/0x28c
> lock_acquire from _raw_spin_lock_irqsave+0x50/0x64
>
> - _raw_spin_lock_irqsave from __pm_runtime_resume+0x54/0x80
> - __pm_runtime_resume from sh_cmt_start+0x60/0x26c
> - sh_cmt_start from sh_cmt_clock_event_set_state+0x60/0xe8
> - sh_cmt_clock_event_set_state from clockevents_switch_state+0x90/0x138
> - clockevents_switch_state from clockevents_register_device+0x78/0xe8
> - clockevents_register_device from sh_cmt_probe+0x588/0x700
> + _raw_spin_lock_irqsave from clk_enable_lock+0x38/0xc4
> + clk_enable_lock from clk_core_enable_lock+0xc/0x2c
> + clk_core_enable_lock from sh_cmt_enable+0x28/0x1c8
> + sh_cmt_enable from sh_cmt_probe+0x5a4/0x780
>
> sh_cmt_probe from platform_probe+0x58/0x90
> platform_probe from really_probe+0x128/0x28c
> really_probe from __driver_probe_device+0x16c/0x18c
> __driver_probe_device from driver_probe_device+0x2c/0xa8
> driver_probe_device from __driver_attach+0xe4/0xf8
> __driver_attach from bus_for_each_dev+0x84/0xc8
> bus_for_each_dev from bus_add_driver+0xd0/0x1d8
> bus_add_driver from driver_register+0xb8/0x100
> driver_register from do_one_initcall+0x74/0x1cc
> do_one_initcall from kernel_init_freeable+0x224/0x294
> kernel_init_freeable from kernel_init+0x14/0x12c
> kernel_init from ret_from_fork+0x14/0x28
> Exception stack(0xe0851fb0 to 0xe0851ff8)
> 1fa0: 00000000 00000000
> 00000000 00000000
> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> 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
--
Kind Regards,
Niklas Söderlund
