Re: [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart

[Aurelien Jarno <aurelien@aurel32.net>]

[-- Attachment #1: Type: text/plain, Size: 3247 bytes --]

Hi Troy,

On 2026-11-11 01:16, Troy Mitchell wrote:
> Hi Aurelien,
> 
> On Tue Nov 10, 2026 at 9:25 PM GMT, Aurelien Jarno wrote:
> > Hi Anand,
> >
> > On 2026-11-10 11:44, Anand Moon wrote:
> >> The SpacemiT P1 power-off and restart handlers use regmap_set_bits()
> >> to access the PMIC. However, these handlers run in an atomic notifier
> >> context (specifically within an rcu_read_lock() critical section), while
> >> the I2C-backed regmap uses a sleepable mutex.
> >> 
> >> This triggers a "WARNING: suspicious RCU usage" warning due to an illegal
> >> context switch in the RCU read-side critical section during shutdown on the
> >> Banana Pi BPI-F3:
> >> 
> >> [  563.803475] WARNING: suspicious RCU usage
> >> [  563.807813] 7.3.1-ARCH-RISC-V #1 Not tainted
> >> [  563.816614] ./include/linux/rcupdate.h:387 Illegal context switch in RCU read-side critical section!
> >> ...
> >> [  563.841445] locks held by shutdown/1: 2, last CPU#0:
> >> [  563.846694]  #0: ffffffff8404fb20 (system_transition_mutex){+.+.}-{4:4}, at: __do_sys_reboot+0x230/0x29e
> >> [  563.856806]  #1: ffffffff84167860 (rcu_read_lock){....}-{1:3}, at: atomic_notifier_call_chain+0x4a/0x1c2
> >> 
> >> Replace the regmap operations with atomic I2C transfers to avoid sleeping
> >> in an atomic context. Preserve unrelated register bits through
> >> read-modify-write operations and check for atomic I2C support before
> >> registering the handlers.
> >> 
> >> Obtain the I2C client from the parent device and verify that the adapter
> >> supports atomic transfers before registering the handlers.
> >> 
> >> Tested on the Banana Pi BPI-F3 (SpacemiT K1).
> >
> > While the patch looks correct, I am not sure this is the correct 
> > approach, as opposed to the one proposed by Troy [1].
> >
> > Indeed bypassing regmap and using i2c atomic transfers needs to be done 
> > by all affected drivers. For instance the VisionFive 2 board, which use 
> > an axp15060 PMIC is also affected by this issue. And the corresponding 
> > driver is shared between architectures, which make this kind of change 
> > more difficult.
> >
> > For the BPI-F3, note that with OpenSBI 1.9, the reboot is handled 
> > through the SBI SRST extension, preventing this issue to happen. On the 
> > long term, I am not sure we should keep this driver.
> >
> That only covers restart. The SBI restart handler is registered at
> priority 192 and runs before the P1 one at SYS_OFF_PRIO_DEFAULT. Power
> off is the other way around: sbi_srst_power_off() goes through
> register_platform_power_off(), i.e. SYS_OFF_PRIO_PLATFORM, so the P1
> power-off handler still runs first and still hits this on shutdown,
> even with OpenSBI 1.9.
> 
> So until this driver goes away, BPI-F3 still needs the arch fix [1]
> for power-off.
> 
> Link: https://lore.kernel.org/linux-riscv/20261104-v7-0-rc1-rv-dis-int-before-restart-v5-1-956ec09c900d@linux.dev/ [1]

We also need a more global fix for the other boards like the VisionFive 
2, so definitely we need the above fix instead of patching the drivers.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                     http://aurel32.net

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]