Re: [PATCH v2 06/13] regulator: ab8500: Propagate mode enable read errors

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Uninitialized stack variable used and hardware error ignored in `ab8500_regulator_get_mode`.
--

commit e3e13ae0ac9e7a8bd4d65f25709df32c5a1808c8
Author: Linus Walleij <linusw@kernel.org>

regulator: ab8500: Propagate mode enable read errors

For regulators whose enable and mode share a state field, set_mode()
first reads that field so changing the requested mode does not enable a
disabled rail. A register read error is currently treated as true and
the driver proceeds to write the new mode, potentially enabling a rail
whose state is unknown. This patch returns the read error instead of
changing the register or cached mode.

> diff --git a/drivers/regulator/ab8500.c b/drivers/regulator/ab8500.c
> index 05c6b9c7af0c7..e4c3502c8dbcc 100644
> --- a/drivers/regulator/ab8500.c
> +++ b/drivers/regulator/ab8500.c
[ ... ]
> @@ -490,7 +490,17 @@ static int ab8500_regulator_set_mode(struct regulator_dev *rdev,
>  		goto out_unlock;
>  	}
>  
> -	if (info->mode_mask || ab8500_regulator_is_enabled(rdev)) {
> +	if (info->mode_mask) {
> +		enabled = 1;
> +	} else {
> +		enabled = ab8500_regulator_is_enabled(rdev);
> +		if (enabled < 0) {
> +			ret = enabled;
> +			goto out_unlock;
> +		}
> +	}
> +
> +	if (enabled) {
>  		ret = abx500_mask_and_set_register_interruptible(info->dev,
>  			bank, reg, mask, val);
>  		if (ret < 0) {

[Severity: High]
This isn't a bug introduced by this patch, but is there a similar logic issue
in ab8500_regulator_get_mode() just below this?

If a hardware or bus error occurs when reading the mode register:

drivers/regulator/ab8500.c:ab8500_regulator_get_mode() {
    ...
    if (info->mode_mask) {
        ret = abx500_get_register_interruptible(info->dev,
        info->mode_bank, info->mode_reg, &val);
        val = val & info->mode_mask;
    ...
}

Does this code ignore the hardware error code in ret and proceed to read the
uninitialized stack variable val? By not checking if the read succeeded, it
appears to return either a random regulator mode or -EINVAL to the caller,
which could mask the real error and feed incorrect states to the regulator
core.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-ux500-dts-snowball-regulator-v2-0-fe88b01829bf@kernel.org?part=6