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

[Lee Jones <lee@kernel.org>]

On Tue, 27 Oct 2026, sashiko-bot@kernel.org wrote:

> 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

Did this get an answer?

> 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

-- 
Lee Jones