* [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context
@ 2026-11-10 6:14 Anand Moon
2026-11-10 6:14 ` [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart Anand Moon
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Anand Moon @ 2026-11-10 6:14 UTC (permalink / raw)
To: Troy Mitchell, Andi Shyti, Sebastian Reichel, Yixun Lan,
Aurelien Jarno, open list:I2C SUBSYSTEM HOST DRIVERS, open list,
open list:SYSTEM RESET/SHUTDOWN DRIVERS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
Cc: Anand Moon, Alex Elder
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 <troy.mitchell@linux.spacemit.com>
Cc: Alex Elder <elder@riscstar.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
drivers/i2c/busses/i2c-k1.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
index ff32876e24d3..f4b21d7efaaf 100644
--- a/drivers/i2c/busses/i2c-k1.c
+++ b/drivers/i2c/busses/i2c-k1.c
@@ -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
--
2.56.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart
2026-11-10 6:14 [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context Anand Moon
@ 2026-11-10 6:14 ` Anand Moon
2026-11-10 21:25 ` Aurelien Jarno
2026-11-10 6:41 ` [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context Troy Mitchell
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Anand Moon @ 2026-11-10 6:14 UTC (permalink / raw)
To: Troy Mitchell, Andi Shyti, Sebastian Reichel, Yixun Lan,
Aurelien Jarno, open list:I2C SUBSYSTEM HOST DRIVERS, open list,
open list:SYSTEM RESET/SHUTDOWN DRIVERS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
Cc: 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.
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).
Assisted-by: LLM
Cc: Troy Mitchell <troy.mitchell@linux.spacemit.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Fixes: 28124cc0fb8c ("driver: reset: spacemit-p1: add driver for poweroff/reboot")
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
drivers/power/reset/spacemit-p1-reboot.c | 86 ++++++++++++++++++++----
1 file changed, 74 insertions(+), 12 deletions(-)
diff --git a/drivers/power/reset/spacemit-p1-reboot.c b/drivers/power/reset/spacemit-p1-reboot.c
index c0454b547718..8fcb3eadddf3 100644
--- a/drivers/power/reset/spacemit-p1-reboot.c
+++ b/drivers/power/reset/spacemit-p1-reboot.c
@@ -4,8 +4,9 @@
*/
#include <linux/bits.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
#include <linux/platform_device.h>
-#include <linux/regmap.h>
#include <linux/reboot.h>
/* Power Control Register 2 */
@@ -13,13 +14,68 @@
#define PWR_CTRL2_SHUTDOWN BIT(2) /* Shutdown request */
#define PWR_CTRL2_RST BIT(1) /* Reset request */
+static int spacemit_p1_read_atomic(struct i2c_client *client,
+ u8 reg, u8 *val)
+{
+ struct i2c_msg msgs[] = {
+ {
+ .addr = client->addr,
+ .flags = client->flags & I2C_M_TEN,
+ .len = 1,
+ .buf = ®,
+ },
+ {
+ .addr = client->addr,
+ .flags = (client->flags & I2C_M_TEN) | I2C_M_RD,
+ .len = 1,
+ .buf = val,
+ },
+ };
+ int ret;
+
+ ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+
+ return ret == ARRAY_SIZE(msgs) ? 0 : ret < 0 ? ret : -EIO;
+}
+
+static int spacemit_p1_write_atomic(struct i2c_client *client,
+ u8 reg, u8 val)
+{
+ u8 buf[] = { reg, val };
+ struct i2c_msg msg = {
+ .addr = client->addr,
+ .flags = client->flags & I2C_M_TEN,
+ .len = sizeof(buf),
+ .buf = buf,
+ };
+ int ret;
+
+ ret = i2c_transfer(client->adapter, &msg, 1);
+
+ return ret == 1 ? 0 : ret < 0 ? ret : -EIO;
+}
+
+static int spacemit_p1_update_bits_atomic(struct i2c_client *client,
+ u8 reg, u8 mask)
+{
+ u8 val;
+ int ret;
+
+ ret = spacemit_p1_read_atomic(client, reg, &val);
+ if (ret)
+ return ret;
+
+ return spacemit_p1_write_atomic(client, reg, val | mask);
+}
+
static int spacemit_p1_pwroff_handler(struct sys_off_data *data)
{
- struct regmap *regmap = data->cb_data;
+ struct i2c_client *client = data->cb_data;
int ret;
/* Put the PMIC into shutdown state */
- ret = regmap_set_bits(regmap, PWR_CTRL2, PWR_CTRL2_SHUTDOWN);
+ ret = spacemit_p1_update_bits_atomic(client, PWR_CTRL2,
+ PWR_CTRL2_SHUTDOWN);
if (ret) {
dev_err(data->dev, "shutdown failed: %d\n", ret);
return notifier_from_errno(ret);
@@ -30,11 +86,12 @@ static int spacemit_p1_pwroff_handler(struct sys_off_data *data)
static int spacemit_p1_restart_handler(struct sys_off_data *data)
{
- struct regmap *regmap = data->cb_data;
+ struct i2c_client *client = data->cb_data;
int ret;
/* Put the PMIC into reset state */
- ret = regmap_set_bits(regmap, PWR_CTRL2, PWR_CTRL2_RST);
+ ret = spacemit_p1_update_bits_atomic(client, PWR_CTRL2,
+ PWR_CTRL2_RST);
if (ret) {
dev_err(data->dev, "restart failed: %d\n", ret);
return notifier_from_errno(ret);
@@ -46,21 +103,26 @@ static int spacemit_p1_restart_handler(struct sys_off_data *data)
static int spacemit_p1_reboot_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct regmap *regmap;
+ struct i2c_client *client;
int ret;
- regmap = dev_get_regmap(dev->parent, NULL);
- if (!regmap)
- return -ENODEV;
+ client = i2c_verify_client(dev->parent);
+ if (!client)
+ return dev_err_probe(dev, -ENODEV,
+ "Failed to find parent I2C client\n");
+
+ if (!client->adapter->algo->xfer_atomic)
+ return dev_err_probe(dev, -EOPNOTSUPP,
+ "Atomic I2C transfers not supported\n");
- ret = devm_register_power_off_handler(dev, &spacemit_p1_pwroff_handler,
- regmap);
+ ret = devm_register_power_off_handler(dev, spacemit_p1_pwroff_handler,
+ client);
if (ret)
return dev_err_probe(dev, ret,
"Failed to register power off handler\n");
ret = devm_register_restart_handler(dev, spacemit_p1_restart_handler,
- regmap);
+ client);
if (ret)
return dev_err_probe(dev, ret,
"Failed to register restart handler\n");
--
2.56.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context
2026-11-10 6:14 [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context Anand Moon
2026-11-10 6:14 ` [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart Anand Moon
@ 2026-11-10 6:41 ` Troy Mitchell
2026-11-10 6:49 ` Troy Mitchell
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Troy Mitchell @ 2026-11-10 6:41 UTC (permalink / raw)
To: Anand Moon
Cc: Troy Mitchell, Andi Shyti, Sebastian Reichel, Yixun Lan,
Aurelien Jarno, linux-i2c, linux-kernel, linux-pm, linux-riscv,
spacemit, Alex Elder
[-- Attachment #1: Type: text/plain, Size: 834 bytes --]
On Tue, 10 Nov 2026 11:44:26 +0530, Anand Moon wrote:
> 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.
This usleep_range() was left behind when the PIO path and
spacemit_i2c_delay() were added, so please add:
Fixes: 5dd75dac1b35 ("i2c: spacemit: introduce pio for k1")
Otherwise, LGTM.
Reviewed-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
--
Troy Mitchell
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context
2026-11-10 6:14 [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context Anand Moon
2026-11-10 6:14 ` [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart Anand Moon
2026-11-10 6:41 ` [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context Troy Mitchell
@ 2026-11-10 6:49 ` Troy Mitchell
2026-11-10 7:04 ` Anand Moon
2026-11-10 20:49 ` Aurelien Jarno
2026-11-10 21:15 ` Alex Elder
4 siblings, 1 reply; 10+ messages in thread
From: Troy Mitchell @ 2026-11-10 6:49 UTC (permalink / raw)
To: Anand Moon, Troy Mitchell, Andi Shyti, Sebastian Reichel,
Yixun Lan, Aurelien Jarno, open list:I2C SUBSYSTEM HOST DRIVERS,
open list, open list:SYSTEM RESET/SHUTDOWN DRIVERS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
Cc: Alex Elder
[-- Attachment #1: Type: text/plain, Size: 691 bytes --]
On Tue Nov 10, 2026 at 6:14 AM GMT, Anand Moon wrote:
> 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
Also, please drop this tag. The same applies to patch 2.
See commit 1e035297ffda.
--
Troy Mitchell
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context
2026-11-10 6:49 ` Troy Mitchell
@ 2026-11-10 7:04 ` Anand Moon
0 siblings, 0 replies; 10+ messages in thread
From: Anand Moon @ 2026-11-10 7:04 UTC (permalink / raw)
To: Troy Mitchell
Cc: Andi Shyti, Sebastian Reichel, Yixun Lan, Aurelien Jarno,
open list:I2C SUBSYSTEM HOST DRIVERS, open list,
open list:SYSTEM RESET/SHUTDOWN DRIVERS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support, Alex Elder
Hi Troy,
On Tue, 10 Nov 2026 at 12:19, Troy Mitchell
<troy.mitchell@linux.spacemit.com> wrote:
>
> On Tue Nov 10, 2026 at 6:14 AM GMT, Anand Moon wrote:
> > 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
> Also, please drop this tag. The same applies to patch 2.
> See commit 1e035297ffda.
>
Ok once I get positive feedback on both patches, I will drop this tag
and resubmit.
> --
> Troy Mitchell
>
Thanks
-Anand
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context
2026-11-10 6:14 [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context Anand Moon
` (2 preceding siblings ...)
2026-11-10 6:49 ` Troy Mitchell
@ 2026-11-10 20:49 ` Aurelien Jarno
2026-11-10 21:15 ` Alex Elder
4 siblings, 0 replies; 10+ messages in thread
From: Aurelien Jarno @ 2026-11-10 20:49 UTC (permalink / raw)
To: Anand Moon
Cc: Troy Mitchell, Andi Shyti, Sebastian Reichel, Yixun Lan,
open list:I2C SUBSYSTEM HOST DRIVERS, open list,
open list:SYSTEM RESET/SHUTDOWN DRIVERS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support, Alex Elder
On 2026-11-10 11:44, Anand Moon wrote:
> 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 <troy.mitchell@linux.spacemit.com>
> Cc: Alex Elder <elder@riscstar.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> drivers/i2c/busses/i2c-k1.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
> index ff32876e24d3..f4b21d7efaaf 100644
> --- a/drivers/i2c/busses/i2c-k1.c
> +++ b/drivers/i2c/busses/i2c-k1.c
> @@ -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
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context
2026-11-10 6:14 [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context Anand Moon
` (3 preceding siblings ...)
2026-11-10 20:49 ` Aurelien Jarno
@ 2026-11-10 21:15 ` Alex Elder
4 siblings, 0 replies; 10+ messages in thread
From: Alex Elder @ 2026-11-10 21:15 UTC (permalink / raw)
To: Anand Moon, Troy Mitchell, Andi Shyti, Sebastian Reichel,
Yixun Lan, Aurelien Jarno, open list:I2C SUBSYSTEM HOST DRIVERS,
open list, open list:SYSTEM RESET/SHUTDOWN DRIVERS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
On 11/10/26 12:14 AM, Anand Moon wrote:
> 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 <troy.mitchell@linux.spacemit.com>
> Cc: Alex Elder <elder@riscstar.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> drivers/i2c/busses/i2c-k1.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
> index ff32876e24d3..f4b21d7efaaf 100644
> --- a/drivers/i2c/busses/i2c-k1.c
> +++ b/drivers/i2c/busses/i2c-k1.c
> @@ -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 use_pio flag is set for atomic transfers, so spacemit_i2c_delay()
will use udelay() regardless of the delay period.
Looks good.
Reviewed-by: Alex Elder <elder@riscstar.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart
2026-11-10 6:14 ` [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart Anand Moon
@ 2026-11-10 21:25 ` Aurelien Jarno
2026-11-11 1:16 ` Troy Mitchell
0 siblings, 1 reply; 10+ messages in thread
From: Aurelien Jarno @ 2026-11-10 21:25 UTC (permalink / raw)
To: Anand Moon
Cc: Troy Mitchell, Andi Shyti, Sebastian Reichel, Yixun Lan,
open list:I2C SUBSYSTEM HOST DRIVERS, open list,
open list:SYSTEM RESET/SHUTDOWN DRIVERS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart
2026-11-10 21:25 ` Aurelien Jarno
@ 2026-11-11 1:16 ` Troy Mitchell
2026-11-11 9:29 ` Aurelien Jarno
0 siblings, 1 reply; 10+ messages in thread
From: Troy Mitchell @ 2026-11-11 1:16 UTC (permalink / raw)
To: Aurelien Jarno, Anand Moon
Cc: Troy Mitchell, Andi Shyti, Sebastian Reichel, Yixun Lan,
open list:I2C SUBSYSTEM HOST DRIVERS, open list,
open list:SYSTEM RESET/SHUTDOWN DRIVERS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
[-- 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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart
2026-11-11 1:16 ` Troy Mitchell
@ 2026-11-11 9:29 ` Aurelien Jarno
0 siblings, 0 replies; 10+ messages in thread
From: Aurelien Jarno @ 2026-11-11 9:29 UTC (permalink / raw)
To: Troy Mitchell
Cc: Anand Moon, Andi Shyti, Sebastian Reichel, Yixun Lan,
open list:I2C SUBSYSTEM HOST DRIVERS, open list,
open list:SYSTEM RESET/SHUTDOWN DRIVERS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
[-- 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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-11-11 9:30 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-10 6:14 [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context Anand Moon
2026-11-10 6:14 ` [PATCH v1 2/2] power: reset: spacemit-p1: Use atomic I2C transfers for power-off and restart Anand Moon
2026-11-10 21:25 ` Aurelien Jarno
2026-11-11 1:16 ` Troy Mitchell
2026-11-11 9:29 ` Aurelien Jarno
2026-11-10 6:41 ` [PATCH v1 1/2] i2c: spacemit: Avoid sleeping during bus reset in atomic context Troy Mitchell
2026-11-10 6:49 ` Troy Mitchell
2026-11-10 7:04 ` Anand Moon
2026-11-10 20:49 ` Aurelien Jarno
2026-11-10 21:15 ` Alex Elder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®