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

From: Aurelien Jarno <aurelien@aurel32.net>
To: Anand Moon <linux.amoon@gmail.com>
Cc: Troy Mitchell <troy.mitchell@linux.spacemit.com>,
	Andi Shyti <andi.shyti@kernel.org>,
	Sebastian Reichel <sre@kernel.org>, Yixun Lan <dlan@kernel.org>,
	"open list:I2C SUBSYSTEM HOST DRIVERS"
	<linux-i2c@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:SYSTEM RESET/SHUTDOWN DRIVERS"
	<linux-pm@vger.kernel.org>,
	"open list:RISC-V SPACEMIT SoC Support"
	<linux-riscv@lists.infradead.org>,
	"open list:RISC-V SPACEMIT SoC Support"
	<spacemit@lists.linux.dev>
Subject: Re: [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart
Date: Tue, 10 Nov 2026 22:25:42 +0100	[thread overview]
Message-ID: <avOL1mCkI4YSaDeO@aurel32.net> (raw)
In-Reply-To: <20261110061429.2036-2-linux.amoon@gmail.com>

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.

Regards
Aurelien

[1] https://lore.kernel.org/linux-riscv/20261104-v7-0-rc1-rv-dis-int-before-restart-v5-1-956ec09c900d@linux.dev/

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