i2c: spacemit: Avoid sleeping during bus reset in atomic context

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).

Hi Anand,

Quoted 22 lines · Anand Moon

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.

Quoted 4 lines

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.

Hi Aurelien,

Quoted 39 lines · Aurelien Jarno

Hi Anand,

Quoted 22 lines · Anand Moon

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.

Quoted 4 lines

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.

Quoted 4 lines

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] open here

Hi Troy,

Quoted 53 lines · Troy Mitchell

Hi Aurelien,

Quoted 39 lines · Aurelien Jarno

Hi Anand,

Quoted 22 lines · Anand Moon

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.

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 4 lines

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] open here

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
[email protected] http://aurel32.net

The spacemit_i2c_conditionally_reset_bus() function uses usleep_range() to wait after triggering a bus reset request. However, this function can be invoked during atomic I2C transfers (such as power-off or restart handlers running in atomic notifier chains), where sleeping is illegal.

Replace the sleep operation with the driver's internal helper, spacemit_i2c_delay(), which conditionally uses udelay() when running in an atomic context, avoiding "scheduling while atomic" bugs.

The spacemit_i2c_conditionally_reset_bus() function uses usleep_range() to wait after triggering a bus reset request. However, this function can be invoked during atomic I2C transfers (such as power-off or restart handlers running in atomic notifier chains), where sleeping is illegal.

Replace the sleep operation with the driver's internal helper, spacemit_i2c_delay(), which conditionally uses udelay() when running in an atomic context, avoiding "scheduling while atomic" bugs.

Assisted-by LLM

Quoted 5 lines · Anand Moon

The spacemit_i2c_conditionally_reset_bus() function uses usleep_range() to wait after triggering a bus reset request. However, this function can be invoked during atomic I2C transfers (such as power-off or restart handlers running in atomic notifier chains), where sleeping is illegal.

Quoted 5 lines

Replace the sleep operation with the driver's internal helper, spacemit_i2c_delay(), which conditionally uses udelay() when running in an atomic context, avoiding "scheduling while atomic" bugs.

Assisted-by LLM

Also, please drop this tag. The same applies to patch 2. See commit 1e035297ffda.

--
Troy Mitchell

The spacemit_i2c_conditionally_reset_bus() function uses usleep_range() to wait after triggering a bus reset request. However, this function can be invoked during atomic I2C transfers (such as power-off or restart handlers running in atomic notifier chains), where sleeping is illegal.

Replace the sleep operation with the driver's internal helper, spacemit_i2c_delay(), which conditionally uses udelay() when running in an atomic context, avoiding "scheduling while atomic" bugs.

Assisted-by LLM

Cc Troy Mitchell <[email protected]>

Cc Alex Elder <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
 drivers/i2c/busses/i2c-k1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/i2c/busses/i2c-k1.c +1 -1

index ff32876e24d3..f4b21d7efaaf 100644
@@ -332,7 +332,7 @@ static void spacemit_i2c_conditionally_reset_bus(struct spacemit_i2c_dev *i2c)
 		writel(FIELD_PREP(SPACEMIT_RCR_FIELD_RST_CYC, 1),
 		       i2c->base + SPACEMIT_IRCR);
 		writel(SPACEMIT_CR_RSTREQ, i2c->base + SPACEMIT_ICR);
-		usleep_range(20, 30);
+		spacemit_i2c_delay(i2c, 20);
 	}
 
 	/* check sda again here */

base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9

The spacemit_i2c_conditionally_reset_bus() function uses usleep_range() to wait after triggering a bus reset request. However, this function can be invoked during atomic I2C transfers (such as power-off or restart handlers running in atomic notifier chains), where sleeping is illegal.

Replace the sleep operation with the driver's internal helper, spacemit_i2c_delay(), which conditionally uses udelay() when running in an atomic context, avoiding "scheduling while atomic" bugs.

Assisted-by LLM

Cc Troy Mitchell <[email protected]>

Cc Alex Elder <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
  drivers/i2c/busses/i2c-k1.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/i2c/busses/i2c-k1.c +1 -1

index ff32876e24d3..f4b21d7efaaf 100644
@@ -332,7 +332,7 @@ static void spacemit_i2c_conditionally_reset_bus(struct spacemit_i2c_dev *i2c)
  		writel(FIELD_PREP(SPACEMIT_RCR_FIELD_RST_CYC, 1),
  		       i2c->base + SPACEMIT_IRCR);
  		writel(SPACEMIT_CR_RSTREQ, i2c->base + SPACEMIT_ICR);
-		usleep_range(20, 30);
+		spacemit_i2c_delay(i2c, 20);
  	}
 
  	/* check sda again here */

base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9