Re: [PATCH] hwmon: (ads7828) Properly handle -EPROBE_DEFER from devm_regulator_get_optional()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver masks genuine errors from `devm_regulator_get_optional()` by explicitly only checking for `-EPROBE_DEFER`.
- [High] The driver fetches an optional external VREF regulator but never calls `regulator_enable()` to turn it on.
--

--- Patch [1]: [PATCH] hwmon: (ads7828) Properly handle -EPROBE_DEFER from devm_regulator_get_optional() ---
commit 78230277b5f05df9416cedbd8b4b4816183523e1
Author: Qingshuang Fu <fuqingshuang@kylinos.cn>

hwmon: (ads7828) Properly handle -EPROBE_DEFER from devm_regulator_get_optional()

This commit explicitly checks for -EPROBE_DEFER when calling
devm_regulator_get_optional() in ads7828_probe(). It ensures the error is
propagated to trigger probe deferral, rather than incorrectly falling back
to the internal VREF.

> diff --git a/drivers/hwmon/ads7828.c b/drivers/hwmon/ads7828.c
> index 149cfcec78dcc..1c4a04cb023c0 100644
> --- a/drivers/hwmon/ads7828.c
> +++ b/drivers/hwmon/ads7828.c
> @@ -133,6 +133,8 @@ static int ads7828_probe(struct i2c_client *client)
>  			    vref_mv > ADS7828_EXT_VREF_MV_MAX)
>  				return -EINVAL;
>  			ext_vref = true;

[Severity: High]
This is a pre-existing issue, but does this code need to enable the external
regulator?

After retrieving the optional regulator, the driver queries its voltage but
never calls regulator_enable() to turn it on. If the external regulator is
not configured as 'always-on' in the device tree, the VREF pin will remain
unpowered, causing the ADC to return zero or floating conversion results.

> +		} else if (PTR_ERR(reg) == -EPROBE_DEFER) {
> +			return PTR_ERR(reg);

[Severity: High]
This isn't a bug introduced by this patch, but does this error handling mask
genuine failures from the regulator core?

By only propagating -EPROBE_DEFER, legitimate errors from
devm_regulator_get_optional() like -ENOMEM or -EINVAL are treated as if no
regulator exists (-ENODEV). This causes the driver to incorrectly fall back
to the internal VREF.

If the pin is physically connected to an external regulator, driving it
internally could create a voltage conflict and lead to hardware damage.
Should we check for -ENODEV to decide whether to fall back, and propagate
all other errors?

>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805020506.958722-1-fffsqian@163.com?part=1