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

["Troy Mitchell" <troy.mitchell@linux.spacemit.com>]

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

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]


-- 
Troy Mitchell


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