linux-kernel - Re: [PATCH v2 2/2] clocksource/drivers/sh_cmt: Do not power down channels used for events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8672a8c1-26b2-4b65-a30e-a4c3e0f1f32e@linaro.org>
Date: Wed, 24 Sep 2025 11:19:21 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Geert Uytterhoeven <geert@...ux-m68k.org>,
 Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
Cc: Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org,
 linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH v2 2/2] clocksource/drivers/sh_cmt: Do not power down
 channels used for events


Hi Geert,

On 23/09/2025 16:56, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Wed, 10 Sept 2025 at 16:27, Niklas Söderlund
> <niklas.soderlund+renesas@...natech.se> wrote:
>> The CMT do runtime PM and call clk_enable()/clk_disable() when a new
>> clock event is register and the CMT is not already started. This is not
>> always possible as a spinlock is also needed to sync the internals of
>> the CMT. Running with PROVE_LOCKING uncovers one such 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
>>
>> Fix this by unconditionally powering on and enabling the needed clocks
>> for all CMT channels which are used for clock events. Do this before
>> registering any clock source or event to avid having to take the
>> spin lock at probe time.
>>
>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
>> ---
>> * Changes since v1
>> - Move the unconditional power on case before registering any clock
>>    source or event to avoid having to use the spinlock to synchronize the
>>    powerup sequence in probe.
> 
> Thanks for your patch, which is now commit cfbc0f1d24030ff9
> ("clocksource/drivers/sh_cmt: Do not power down channels used for
> events") in clockevents/timers/drivers/next.
> 
> Unfortunately this commit introduces an s2ram regression on e.g.
> Atmark Techo Armadillo-800EVA with R-Mobile A1: the system wakes
> up immediately.  There is no evidence of a wake-up event shown in
> /sys/kernel/debug/wakeup_sources.  This happens with or without
> console_suspend enabled.
> 
> Reverting this commit fixes the issue.  I suspect the system wakes up
> because the periodic clock event fires, and causes an interrupt.

I'm about to send a PR.

Shall I drop this change which fixes a lock issue or keep it ?

What has the higher priority ?


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog