[PATCH] mfd: stpmic1: Attempt system shutdown twice in case PMIC is confused

[PATCH] mfd: stpmic1: Attempt system shutdown twice in case PMIC is confused

[Marek Vasut]

Attempt to shut down again, in case the first attempt failed.
The STPMIC1 might get confused and the first regmap_update_bits()
returns with -ETIMEDOUT / -110 . If that or similar transient
failure occurs, try to shut down again. If the second attempt
fails, there is some bigger problem, report it to user.

Fixes: 6e9df38f359a ("mfd: stpmic1: Add PMIC poweroff via sys-off handler")
Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Lee Jones <lee@kernel.org>
Cc: Pascal PAILLET-LME <p.paillet@st.com>
Cc: Paul Cercueil <paul@crapouillou.net>
Cc: Sean Nyekjaer <sean@geanix.com>
Cc: kernel@dh-electronics.com
---
 drivers/mfd/stpmic1.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/stpmic1.c b/drivers/mfd/stpmic1.c
index 081827bc05961..9ef4049d7db10 100644
--- a/drivers/mfd/stpmic1.c
+++ b/drivers/mfd/stpmic1.c
@@ -121,9 +121,23 @@ static const struct regmap_irq_chip stpmic1_regmap_irq_chip = {
 static int stpmic1_power_off(struct sys_off_data *data)
 {
 	struct stpmic1 *ddata = data->cb_data;
+	int ret;
+
+	ret = regmap_update_bits(ddata->regmap, MAIN_CR, SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
+	if (!ret)
+		return NOTIFY_DONE;
+
+	/*
+	 * Attempt to shut down again, in case the first attempt failed.
+	 * The STPMIC1 might get confused and the first regmap_update_bits()
+	 * returns with -ETIMEDOUT / -110 . If that or similar transient
+	 * failure occurs, try to shut down again. If the second attempt
+	 * fails, there is some bigger problem, report it to user.
+	 */
+	ret = regmap_update_bits(ddata->regmap, MAIN_CR, SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
+	if (ret)
+		dev_err(ddata->dev, "Failed to access PMIC I2C bus (%d)\n", ret);
 
-	regmap_update_bits(ddata->regmap, MAIN_CR,
-			   SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
 
 	return NOTIFY_DONE;
 }
-- 
2.51.0

Re: [PATCH] mfd: stpmic1: Attempt system shutdown twice in case PMIC is confused

[Sean Nyekjaer]

On Fri, Jan 09, 2026 at 05:12:09AM +0100, Marek Vasut wrote:
> Attempt to shut down again, in case the first attempt failed.
> The STPMIC1 might get confused and the first regmap_update_bits()
> returns with -ETIMEDOUT / -110 . If that or similar transient
> failure occurs, try to shut down again. If the second attempt
> fails, there is some bigger problem, report it to user.

I don't think I have ever seen my devices fail to shutdown.
Have you ever seen the second call fail?

> 
> Fixes: 6e9df38f359a ("mfd: stpmic1: Add PMIC poweroff via sys-off handler")
> Signed-off-by: Marek Vasut <marex@nabladev.com>
Reviewed-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Cc: Lee Jones <lee@kernel.org>
> Cc: Pascal PAILLET-LME <p.paillet@st.com>
> Cc: Paul Cercueil <paul@crapouillou.net>
> Cc: Sean Nyekjaer <sean@geanix.com>
> Cc: kernel@dh-electronics.com
> ---
>  drivers/mfd/stpmic1.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/stpmic1.c b/drivers/mfd/stpmic1.c
> index 081827bc05961..9ef4049d7db10 100644
> --- a/drivers/mfd/stpmic1.c
> +++ b/drivers/mfd/stpmic1.c
> @@ -121,9 +121,23 @@ static const struct regmap_irq_chip stpmic1_regmap_irq_chip = {
>  static int stpmic1_power_off(struct sys_off_data *data)
>  {
>  	struct stpmic1 *ddata = data->cb_data;
> +	int ret;
> +
> +	ret = regmap_update_bits(ddata->regmap, MAIN_CR, SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
> +	if (!ret)
> +		return NOTIFY_DONE;
> +
> +	/*
> +	 * Attempt to shut down again, in case the first attempt failed.
> +	 * The STPMIC1 might get confused and the first regmap_update_bits()
> +	 * returns with -ETIMEDOUT / -110 . If that or similar transient
> +	 * failure occurs, try to shut down again. If the second attempt
> +	 * fails, there is some bigger problem, report it to user.
> +	 */
> +	ret = regmap_update_bits(ddata->regmap, MAIN_CR, SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
> +	if (ret)
> +		dev_err(ddata->dev, "Failed to access PMIC I2C bus (%d)\n", ret);
> 
> -	regmap_update_bits(ddata->regmap, MAIN_CR,
> -			   SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
> 
>  	return NOTIFY_DONE;
>  }
> --
> 2.51.0
> 

Re: [PATCH] mfd: stpmic1: Attempt system shutdown twice in case PMIC is confused

[Marek Vasut]

On 1/9/26 3:54 PM, Sean Nyekjaer wrote:
> On Fri, Jan 09, 2026 at 05:12:09AM +0100, Marek Vasut wrote:
>> Attempt to shut down again, in case the first attempt failed.
>> The STPMIC1 might get confused and the first regmap_update_bits()
>> returns with -ETIMEDOUT / -110 . If that or similar transient
>> failure occurs, try to shut down again. If the second attempt
>> fails, there is some bigger problem, report it to user.
> 
> I don't think I have ever seen my devices fail to shutdown.
> Have you ever seen the second call fail?
I have sporadically seen the first call fail with timeout, not the 
second one yet. If the second call fails, I think we have a real bad 
hardware problem.

Maybe these issues are hidden on devices which use SCMI too, so they 
never get properly fixed in the direct hardware access kernel drivers 
anymore, sigh.

Re: [PATCH] mfd: stpmic1: Attempt system shutdown twice in case PMIC is confused

[Lee Jones]

On Fri, 09 Jan 2026, Marek Vasut wrote:

> Attempt to shut down again, in case the first attempt failed.
> The STPMIC1 might get confused and the first regmap_update_bits()
> returns with -ETIMEDOUT / -110 . If that or similar transient
> failure occurs, try to shut down again. If the second attempt
> fails, there is some bigger problem, report it to user.
> 
> Fixes: 6e9df38f359a ("mfd: stpmic1: Add PMIC poweroff via sys-off handler")
> Signed-off-by: Marek Vasut <marex@nabladev.com>
> ---
> Cc: Lee Jones <lee@kernel.org>
> Cc: Pascal PAILLET-LME <p.paillet@st.com>
> Cc: Paul Cercueil <paul@crapouillou.net>
> Cc: Sean Nyekjaer <sean@geanix.com>
> Cc: kernel@dh-electronics.com
> ---
>  drivers/mfd/stpmic1.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/stpmic1.c b/drivers/mfd/stpmic1.c
> index 081827bc05961..9ef4049d7db10 100644
> --- a/drivers/mfd/stpmic1.c
> +++ b/drivers/mfd/stpmic1.c
> @@ -121,9 +121,23 @@ static const struct regmap_irq_chip stpmic1_regmap_irq_chip = {
>  static int stpmic1_power_off(struct sys_off_data *data)
>  {
>  	struct stpmic1 *ddata = data->cb_data;
> +	int ret;
> +
> +	ret = regmap_update_bits(ddata->regmap, MAIN_CR, SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
> +	if (!ret)
> +		return NOTIFY_DONE;
> +
> +	/*
> +	 * Attempt to shut down again, in case the first attempt failed.
> +	 * The STPMIC1 might get confused and the first regmap_update_bits()
> +	 * returns with -ETIMEDOUT / -110 . If that or similar transient
> +	 * failure occurs, try to shut down again. If the second attempt
> +	 * fails, there is some bigger problem, report it to user.
> +	 */
> +	ret = regmap_update_bits(ddata->regmap, MAIN_CR, SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);

I think it would be better to put it into a loop with 2 retries.

> +	if (ret)
> +		dev_err(ddata->dev, "Failed to access PMIC I2C bus (%d)\n", ret);
>  
> -	regmap_update_bits(ddata->regmap, MAIN_CR,
> -			   SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
>  
>  	return NOTIFY_DONE;
>  }
> -- 
> 2.51.0
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH] mfd: stpmic1: Attempt system shutdown twice in case PMIC is confused

[Marek Vasut]

On 1/13/26 1:33 PM, Lee Jones wrote:
> On Fri, 09 Jan 2026, Marek Vasut wrote:
> 
>> Attempt to shut down again, in case the first attempt failed.
>> The STPMIC1 might get confused and the first regmap_update_bits()
>> returns with -ETIMEDOUT / -110 . If that or similar transient
>> failure occurs, try to shut down again. If the second attempt
>> fails, there is some bigger problem, report it to user.
>>
>> Fixes: 6e9df38f359a ("mfd: stpmic1: Add PMIC poweroff via sys-off handler")
>> Signed-off-by: Marek Vasut <marex@nabladev.com>
>> ---
>> Cc: Lee Jones <lee@kernel.org>
>> Cc: Pascal PAILLET-LME <p.paillet@st.com>
>> Cc: Paul Cercueil <paul@crapouillou.net>
>> Cc: Sean Nyekjaer <sean@geanix.com>
>> Cc: kernel@dh-electronics.com
>> ---
>>   drivers/mfd/stpmic1.c | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mfd/stpmic1.c b/drivers/mfd/stpmic1.c
>> index 081827bc05961..9ef4049d7db10 100644
>> --- a/drivers/mfd/stpmic1.c
>> +++ b/drivers/mfd/stpmic1.c
>> @@ -121,9 +121,23 @@ static const struct regmap_irq_chip stpmic1_regmap_irq_chip = {
>>   static int stpmic1_power_off(struct sys_off_data *data)
>>   {
>>   	struct stpmic1 *ddata = data->cb_data;
>> +	int ret;
>> +
>> +	ret = regmap_update_bits(ddata->regmap, MAIN_CR, SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
>> +	if (!ret)
>> +		return NOTIFY_DONE;
>> +
>> +	/*
>> +	 * Attempt to shut down again, in case the first attempt failed.
>> +	 * The STPMIC1 might get confused and the first regmap_update_bits()
>> +	 * returns with -ETIMEDOUT / -110 . If that or similar transient
>> +	 * failure occurs, try to shut down again. If the second attempt
>> +	 * fails, there is some bigger problem, report it to user.
>> +	 */
>> +	ret = regmap_update_bits(ddata->regmap, MAIN_CR, SOFTWARE_SWITCH_OFF, SOFTWARE_SWITCH_OFF);
> 
> I think it would be better to put it into a loop with 2 retries.
I sent V2 like that now.