Re: [PATCH 2/4] mfd: max77620: convert poweroff support to sys-off API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] max77620_pm_power_off() acquires a mutex inside an atomic context (RCU read-side critical section and disabled IRQs), leading to a potential deadlock during system power-off.
--

commit b3bd2ca1c0c28ed17c99ae76d84c114260a5a844
Author: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>

mfd: max77620: convert poweroff support to sys-off API

This commit converts max77620_pm_power_off() to the sys-off callback prototype
and registers it with the sys-off API when the device tree marks the PMIC as
a system power controller. It also removes the global max77620_scratch pointer
by passing the chip instance through the callback data.

> diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c
> --- a/drivers/mfd/max77620.c
> +++ b/drivers/mfd/max77620.c
[ ... ]
> @@ -484,13 +483,15 @@ static int max77620_read_es_version(struct max77620_chip *chip)
>  	return ret;
>  }
>  
> -static void max77620_pm_power_off(void)
> +static int max77620_pm_power_off(struct sys_off_data *data)
>  {
> -	struct max77620_chip *chip = max77620_scratch;
> +	struct max77620_chip *chip = data->cb_data;
>  
>  	regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG1,
>  			   MAX77620_ONOFFCNFG1_SFT_RST,
>  			   MAX77620_ONOFFCNFG1_SFT_RST);
> +
> +	return NOTIFY_DONE;
>  }
[ ... ]
> @@ -573,10 +573,14 @@ static int max77620_probe(struct i2c_client *client)
>  		return ret;
>  	}
>  
> -	pm_off = of_device_is_system_power_controller(client->dev.of_node);
> -	if (pm_off && !pm_power_off) {
> -		max77620_scratch = chip;
> -		pm_power_off = max77620_pm_power_off;
> +	if (of_device_is_system_power_controller(client->dev.of_node)) {
> +		ret = devm_register_sys_off_handler(&client->dev,
> +						    SYS_OFF_MODE_POWER_OFF,

Does this code risk deadlocking during system power-off?

Handlers registered with SYS_OFF_MODE_POWER_OFF execute via an atomic
notifier call chain inside do_kernel_power_off() holding rcu_read_lock(),
and with local IRQs completely disabled.

However, the max77620_pm_power_off() callback invokes regmap_update_bits()
on an I2C regmap, which relies on a standard mutex.

If another CPU held this PMIC regmap mutex when smp_send_stop() was issued,
attempting to acquire it here will hang the power-off sequence indefinitely.

Would it be safer to register this handler using
SYS_OFF_MODE_POWER_OFF_PREPARE instead? That mode uses a blocking notifier
call chain and executes before CPUs are stopped and IRQs are disabled.

> +						    SYS_OFF_PRIO_DEFAULT,
> +						    max77620_pm_power_off, chip);
> +		if (ret)
> +			return dev_err_probe(&client->dev, ret,
> +					"failed to register power-off handler\n");
>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514-smaug-poweroff-v1-0-30f9a4688966@tecnico.ulisboa.pt?part=2