Re: [RFC] clocksource/drivers/sh_cmt: Always leave device running after probe

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Geert,

Thanks for your feedback and test!

On 2025-10-15 14:43:03 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> Thanks for your patch!
> 
> On Sun, 12 Oct 2025 at 11:10, Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > The CMT device can be used as both a clocksource and a clockevent
> > provider. The driver tries to be smart and power itself on and off, as
> > well as enabling and disabling its clock when it's not in operation.
> > This behavior is slightly altered if the CMT is used as an early
> > platform device in which case the device is left powered on after probe,
> > but the clock is still enabled and disabled at runtime.
> >
> > This have worked for a long time, but recent improvements in PREEMPT_RT
> 
> has
> 
> > and PROVE_LOCKING have highlighted an issue. As the CMT register itself
> 
> registers
> 
> > as a clockevent provided, clockevents_register_device(), it needs to use
> 
> provider
> 
> > raw spinlocks internally as this is the context of which the clockevent
> > framework interacts with the CMT driver. However in the context of
> > holding a raw spinlock the CMT driver can't really manage its power
> > state or clock with calls to pm_runtime_*() and clk_*() as these calls
> > end up in other platform drivers using regular spinlocks to control
> > power and clocks.
> >
> > This mix of spinlock contexts trips a lockdep warning.
> >
> >     =============================
> >     [ 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
> >
> > For non-PREEMPT_RT builds this is not really an issue, but for
> > PREEMPT_RT builds where normal spinlocks can sleep this might be an
> > issue. Be cautious and always leave the power and clock running after
> > probe.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> Thanks, I gave this a try on various boards, no issues seen.
> I also force-disabled the TMU on R-Mobile A1, to make sure its known
> splat is not hiding any other locking problems.

Once we are happy with the CMT fix I will do something similar for TMU 
too.

> 
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks!

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> 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