Re: [PATCH v3 4/4] regulator: fan53555: Add support for mode operations on Silergy devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The function `fan53555_set_mode` unconditionally returns 0 and ignores the error return value from `regmap_update_bits`, falsely informing the regulator core that mode changes were successful even when underlying I2C transfers fail.
--

--- Patch [4]: [PATCH v3 4/4] regulator: fan53555: Add support for mode operations on Silergy devices ---
commit 0810cb3eb74c80e6af381ca179af7f69456b49a8
Author: Victor Krawiec <victor.krawiec@arturia.com>

    regulator: fan53555: Add support for mode operations on Silergy devices
    
    Make the PWM mode configurable from devicetree. Some boards require forced
    PWM mode to keep the supply ripple within acceptable limits under light
    load conditions
    
    Support is restricted to Silergy manufacturer as it is the only one
    currently tested.

> diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
> index c282236959b18..85f46874eb49d 100644
> --- a/drivers/regulator/fan53555.c
> +++ b/drivers/regulator/fan53555.c

[ ... ]

> @@ -389,6 +390,18 @@ static int rk8602_voltages_setup_rockchip(struct fan53555_device_info *di)
>  	return 0;
>  }
>  
> +static inline unsigned int fan53555_map_mode(unsigned int mode)
> +{
> +	switch (mode) {
> +	case FAN53555_REGULATOR_MODE_FORCE_PWM:
> +		return REGULATOR_MODE_FAST;
> +	case FAN53555_REGULATOR_MODE_AUTO:
> +		return REGULATOR_MODE_NORMAL;
> +	default:
> +		return REGULATOR_MODE_INVALID;
> +	}
> +}
> +

[Severity: High]
This is a pre-existing issue, but since this patch enables setting the
mode from the device tree via fan53555_map_mode(), should fan53555_set_mode()
check the return value of regmap_update_bits()?

Looking at fan53555_set_mode() in drivers/regulator/fan53555.c, it
unconditionally returns 0 and ignores the error return value from
regmap_update_bits():

drivers/regulator/fan53555.c:fan53555_set_mode() {
    ...
	switch (mode) {
	case REGULATOR_MODE_FAST:
		regmap_update_bits(rdev->regmap, di->mode_reg,
				   di->mode_mask, di->mode_mask);
		break;
	case REGULATOR_MODE_NORMAL:
		regmap_update_bits(rdev->regmap, di->vol_reg, di->mode_mask, 0);
		break;
	default:
		return -EINVAL;
	}
	return 0;
}

If an I2C transfer fails here, the regulator hardware could remain in
auto mode while the kernel core falsely believes it was successfully set to
forced PWM mode. Could this lead to the unacceptable voltage ripple under
light load conditions mentioned in the commit message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723094001.120264-1-victor.krawiec@arturia.com?part=4