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

[Anand Moon <linux.amoon@gmail.com>]

Hi Troy. Aurelien,

Thanks for your review comments.

On Wed, 11 Nov 2026 at 14:59, Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> 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.
> >

I had previously tested the OpenSBI P1 PMIC driver in a Yocto-based environment:

[1] https://lore.kernel.org/all/20260419150857.2705843-3-aurelien@aurel32.net/

With this driver registered in OpenSBI, Linux can delegate reboot and power-off
operations to firmware through the SBI SRST extension.

The power-off sequence is:
    Linux kernel_power_off()
      -> RISC-V machine power-off
      -> SBI SRST shutdown
      -> OpenSBI system_reset()
      -> SpacemiT P1 PMIC
      -> Power off

I have now rebuilt the Linux kernel with:

 # CONFIG_POWER_RESET_SPACEMIT_P1 is not set

Both reboot and power-off work correctly on the Banana Pi BPI-F3,
and I have not observed any issues.

alarm@k1:~$ sudo poweroff

Broadcast message from root@k1 on pts/1 (Wed 2026-11-11 10:34:03 UTC):

The system will power off now!

[ 1744.735287] systemd-shutdown[1]: Sending SIGTERM to remaining processes...
[ 1745.071304] systemd-journald[245]: Received SIGTERM from PID 1
(systemd-shutdow).
[ 1745.504595] systemd-shutdown[1]: Sending SIGKILL to remaining processes...
[ 1745.790159] systemd-shutdown[1]: Unmounting file systems.
[ 1745.822902] (sd-umount)[658]: Unmounting
'/run/credentials/systemd-journald.service'.
[ 1745.861214] (sd-remount)[659]: Remounting '/' read-only with options ''.
[ 1746.079291] EXT4-fs (mmcblk1p7): re-mounted
ccb95ca0-2804-4ade-938f-23116f90c193 ro.
[ 1746.272104] systemd-shutdown[1]: All filesystems unmounted.
[ 1746.272471] systemd-shutdown[1]: Deactivating swaps.
[ 1746.273712] systemd-shutdown[1]: All swaps deactivated.
[ 1746.273956] systemd-shutdown[1]: Detaching loop devices.
[ 1746.464867] systemd-shutdown[1]: All loop devices detached.
[ 1746.465346] systemd-shutdown[1]: Stopping MD devices.
[ 1746.471149] systemd-shutdown[1]: All MD devices stopped.
[ 1746.471417] systemd-shutdown[1]: Detaching DM devices.
[ 1746.476047] systemd-shutdown[1]: All DM devices detached.
[ 1746.476377] systemd-shutdown[1]: All filesystems, swaps, loop
devices, MD devices and DM devices detached.
[ 1746.533569] systemd-shutdown[1]: Successfully changed into root pivot.
[ 1746.533862] systemd-shutdown[1]: Entering exitrd...
[ 1746.686127] shutdown[1]: Syncing filesystems and block devices.
[ 1746.701832] shutdown[1]: Sending SIGTERM to remaining processes...
[ 1746.934370] shutdown[1]: Sending SIGKILL to remaining processes...
[ 1747.164219] shutdown[1]: Unmounting file systems.
[ 1747.203935] (sd-umoun[662]: Unmounting '/run/credentials'.
[ 1747.232643] (sd-umoun[663]: Unmounting '/oldroot/sys/kernel/config'.
[ 1747.269882] (sd-umoun[664]: Unmounting '/oldroot/sys/kernel/tracing'.
[ 1747.298218] (sd-umoun[665]: Unmounting '/oldroot/sys/kernel/debug'.
[ 1747.327488] (sd-umoun[666]: Unmounting '/oldroot/dev/hugepages'.
[ 1747.359053] (sd-umoun[667]: Unmounting '/oldroot/dev/mqueue'.
[ 1747.387798] (sd-umoun[668]: Unmounting '/oldroot/run'.
[ 1747.425939] (sd-umoun[669]: Unmounting '/oldroot/proc'.
[ 1747.473927] (sd-umoun[670]: Unmounting '/oldroot/sys/fs/bpf'.
[ 1747.527910] (sd-umoun[671]: Unmounting '/oldroot/sys/fs/pstore'.
[ 1747.588367] (sd-umoun[672]: Unmounting '/oldroot/sys/fs/cgroup'.
[ 1747.630489] (sd-umoun[673]: Unmounting '/oldroot/sys/kernel/security'.
[ 1747.664244] (sd-umoun[674]: Unmounting '/oldroot/sys'.
[ 1747.704716] (sd-umoun[675]: Unmounting '/oldroot/dev/pts'.
[ 1747.755218] (sd-umoun[676]: Unmounting '/oldroot/dev/shm'.
[ 1747.791652] (sd-umoun[677]: Unmounting '/oldroot/dev'.
[ 1747.826501] (sd-umoun[678]: Unmounting '/oldroot'.
[ 1748.451336] EXT4-fs (mmcblk1p7): unmounting filesystem
ccb95ca0-2804-4ade-938f-23116f90c193.
[ 1748.643359] shutdown[1]: All filesystems unmounted.
[ 1748.643640] shutdown[1]: Deactivating swaps.
[ 1748.644512] shutdown[1]: All swaps deactivated.
[ 1748.644722] shutdown[1]: Detaching loop devices.
[ 1748.732490] shutdown[1]: All loop devices detached.
[ 1748.732764] shutdown[1]: Stopping MD devices.
[ 1748.754002] shutdown[1]: All MD devices stopped.
[ 1748.754286] shutdown[1]: Detaching DM devices.
[ 1748.759617] shutdown[1]: All DM devices detached.
[ 1748.759885] shutdown[1]: All filesystems, swaps, loop devices, MD
devices and DM devices detached.
[ 1748.770510] shutdown[1]: Syncing filesystems and block devices.
[ 1748.791699] shutdown[1]: Powering off.
[ 1749.009246] reboot: Power down

Given that OpenSBI already handles both operations,
I agree that the Linux P1 power-off and reset driver may no longer be necessary,
at least on systems using firmware with the appropriate SBI SRST support.

> > 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.
>
Maybe we need an opensbi driver for VisionFive 2 dev boards; any thoughts
> Aurelien
>
> --
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net                     http://aurel32.net

Thanks
-Anand