Re: [PATCH v4 4/4] mfd: max8907: Convert to use device managed poweroff handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mfd: max8907: max8907_i2c_probe() drops maxim,system-power-controller check
--

commit 5deb6448298f2829c11e1c8af86067bd08a79d7f
Author: Svyatoslav Ryhel <clamor95@gmail.com>

mfd: max8907: Convert to use device managed poweroff handler

This commit replaces the global pm_power_off assignment in the max8907
MFD driver with devm_register_power_off_handler().

> diff --git a/drivers/mfd/max8907.c b/drivers/mfd/max8907.c
> index 2a38e00a10c3b..e282dd82ece69 100644
> --- a/drivers/mfd/max8907.c
> +++ b/drivers/mfd/max8907.c

[ ... ]

> @@ -185,8 +192,6 @@ static int max8907_i2c_probe(struct i2c_client *i2c)
>  	struct device *dev = &i2c->dev;
>  	struct max8907 *max8907;
>  	int ret;
> -	bool pm_off = of_property_read_bool(dev->of_node,
> -					"maxim,system-power-controller");
>  
>  	max8907 = devm_kzalloc(dev, sizeof(*max8907), GFP_KERNEL);
>  	if (!max8907)
> @@ -237,9 +242,10 @@ static int max8907_i2c_probe(struct i2c_client *i2c)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to add MFD devices\n");
>  
> -	if (pm_off && !pm_power_off) {
> -		max8907_pm_off = max8907;
> -		pm_power_off = max8907_power_off;
> +	if (of_device_is_system_power_controller(dev->of_node)) {
> +		ret = devm_register_power_off_handler(dev, max8907_power_off, max8907);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "failed to register power-off handler\n");
>  	}

[Severity: High]
Does replacing the maxim,system-power-controller property check with only
of_device_is_system_power_controller() break system power-off on existing
DTBs?

In include/linux/of.h, of_device_is_system_power_controller() only checks
for the unprefixed property:

static inline bool of_device_is_system_power_controller(const struct device_node *np)
{
	return of_property_read_bool(np, "system-power-controller");
}

In Documentation/devicetree/bindings/regulator/maxim,max8907.yaml (added in
commit c254fcc966ae), maxim,system-power-controller is still documented as a
deprecated property for backward compatibility alongside
system-power-controller:

  maxim,system-power-controller:
    type: boolean
    deprecated: true
    description:
      Telling whether or not this PMIC is controlling the system power.

  system-power-controller: true

Should max8907_i2c_probe() check both
of_device_is_system_power_controller(dev->of_node) and
of_property_read_bool(dev->of_node, "maxim,system-power-controller") so
existing DTBs using the vendor-prefixed property continue to register the
power-off handler?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027114634.79706-1-clamor95@gmail.com?part=4