From: sashiko-bot@kernel.org
To: "Qingshuang Fu" <fffsqian@163.com>
Cc: linux-hwmon@vger.kernel.org
Subject: Re: [PATCH v2] hwmon: (ads7828) Don't ignore errors from devm_regulator_get_optional()
Date: Wed, 05 Aug 2026 03:10:11 +0000 [thread overview]
Message-ID: <20260805031012.07F421F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260805030056.1082745-1-fffsqian@163.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver acquires the external 'vref' regulator and reads its voltage but never enables it.
--
--- Patch [1]: [PATCH v2] hwmon: (ads7828) Don't ignore errors from devm_regulator_get_optional() ---
commit 49f8d154ff184810c29b27c75b5602ac3ecd4f65
Author: Qingshuang Fu <fuqingshuang@kylinos.cn>
Subject: hwmon: (ads7828) Don't ignore errors from devm_regulator_get_optional()
The driver currently ignores all errors from devm_regulator_get_optional()
and falls back to using the internal VREF. This commit changes the driver
to propagate genuine errors other than -ENODEV to the caller instead of
silently ignoring them.
> diff --git a/drivers/hwmon/ads7828.c b/drivers/hwmon/ads7828.c
> index 149cfcec78dcc..1c3a8ce8afb5b 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;
> + } else if (PTR_ERR(reg) != -ENODEV) {
> + return PTR_ERR(reg);
> }
[Severity: High]
This is a pre-existing issue, but looking at ads7828_probe(), the driver
acquires the external vref regulator and reads its voltage but never actually
enables it:
reg = devm_regulator_get_optional(dev, "vref");
if (!IS_ERR(reg)) {
vref_uv = regulator_get_voltage(reg);
vref_mv = DIV_ROUND_CLOSEST(vref_uv, 1000);
...
If the external vref regulator is not marked as regulator-always-on in the
device tree, won't the regulator subsystem disable it during the late initcall
unused-regulator cleanup?
Could this cut the reference voltage to the ADC and cause it to return zeroed
or completely incorrect sensor readings?
Should the driver call regulator_enable() or perhaps use
devm_regulator_get_enable_read_voltage() instead?
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805030056.1082745-1-fffsqian@163.com?part=1
prev parent reply other threads:[~2026-08-05 3:10 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-05 3:00 [PATCH v2] hwmon: (ads7828) Don't ignore errors from devm_regulator_get_optional() Qingshuang Fu
2026-08-05 3:10 ` sashiko-bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260805031012.07F421F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=fffsqian@163.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.