Re: [PATCH] hwmon: (ads7828) Properly handle -EPROBE_DEFER from devm_regulator_get_optional()
[email protected] Wed, 05 Aug 2026 02:11:58 +0000
| Newsgroups | org.kernel.vger.linux-hwmon |
|---|---|
| Message-ID | <[email protected]> |
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 c= alls `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 <[email protected]> hwmon: (ads7828) Properly handle -EPROBE_DEFER from devm_regulator_get_opti= onal() 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 =3D 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) =3D=3D -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? > } > } --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260805020506.9587= [email protected]?part=3D1