Re: [PATCH v6 2/3] mfd: rk8xx: Release the power hold GPIOs before powering off

[Heiko Stuebner]

Am Sonntag, 1. November 2026, 16:40:51 Mitteleuropäische Normalzeit schrieb Yaozhong Li:
> rk808_power_off() writes the PMIC's shutdown bit directly. On boards where
> lines driven by the SoC gate the PMIC's power-off path, a shutdown
> performed this way does not stick: the rails drop and immediately come
> back up, so the board restarts instead of staying off. U-Boot reports the
> result as a power-on reset.
> 
> Take those lines as an optional GPIO array, hold them asserted while the
> system runs and release them - waiting power-hold-delay-ms - before
> writing the shutdown bit. Device trees without the property retain the
> existing behaviour.
> 
> The array is claimed before any child device is registered, so that a
> -EPROBE_DEFER from the GPIO provider does not tear down the regulators,
> RTC and clocks that devm_mfd_add_devices() has already registered.
> 
> On a Firefly-RK3399 the shutdown stuck when such a line went from high to
> low before the shutdown bit was written, and did not when the line was
> never driven.
> 
> Assisted-by: LLM
> Signed-off-by: Yaozhong Li <yaozhonguwl@xxxxxxxxx>
> ---
>  drivers/mfd/rk8xx-core.c  | 49 +++++++++++++++++++++++++++++++++++++--
>  include/linux/mfd/rk808.h |  3 +++
>  2 files changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/rk8xx-core.c b/drivers/mfd/rk8xx-core.c
> index 3dcf6ab..137ab20 100644
> --- a/drivers/mfd/rk8xx-core.c
> +++ b/drivers/mfd/rk8xx-core.c
> @@ -11,6 +11,8 @@
>   */
>  
>  #include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/interrupt.h>
>  #include <linux/mfd/rk808.h>
>  #include <linux/mfd/core.h>
> @@ -703,6 +705,22 @@ static int rk808_power_off(struct sys_off_data *data)
>  	default:
>  		return NOTIFY_DONE;
>  	}
> +
> +	/*
> +	 * Some boards route "power hold" lines from the SoC into the board's
> +	 * power circuitry. They have to be held asserted while the system is
> +	 * running and released before the PMIC shutdown request is written,
> +	 * otherwise the rails can drop and come back up instead of staying
> +	 * off. Releasing them here rather than from a separate handler keeps
> +	 * the ordering against the I2C write explicit.

this will be called from both the I2C and SPI variants of the controller.
Also please shorten that comment block.

> +	 */
> +	if (rk808->power_hold_gpios) {

unsigned int line; here not in for() ... could also just be "i", as it's
simply counting

> +		for (unsigned int line = 0; line < rk808->power_hold_gpios->ndescs; line++)
> +			gpiod_set_value_cansleep(rk808->power_hold_gpios->desc[line], 0);

This code is (now again and more correctly) running when irqs have
already been turned off, so nothing is allowed to sleep here

When you enable
CONFIG_DEBUG_ATOMIC_SLEEP=y
you should get nice warning splat about that.


Heiko