* Re: [PATCH] mfd: wm8350: return an error for AUXADC timeouts
2026-06-24 14:42 [PATCH] mfd: wm8350: return an error for AUXADC timeouts Pengpeng Hou
@ 2026-07-02 16:00 ` Lee Jones
0 siblings, 0 replies; 2+ messages in thread
From: Lee Jones @ 2026-07-02 16:00 UTC (permalink / raw)
To: Pengpeng Hou; +Cc: patches, linux-kernel
Please take this into consideration:
/* Sashiko Automation: Issues Found (3 Findings) */
On Wed, 24 Jun 2026, Pengpeng Hou wrote:
> wm8350_read_auxadc() logs when the AUXADC poll bit remains set after the
> conversion wait, but still returns result, which remains zero when no
> conversion result was read.
>
> Return -ETIMEDOUT after turning the ADC back off and releasing
> auxadc_mutex so callers do not treat a timed out conversion as a valid
> zero sample.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> drivers/mfd/wm8350-core.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/wm8350-core.c b/drivers/mfd/wm8350-core.c
> index fbc77b218..214826e16 100644
> --- a/drivers/mfd/wm8350-core.c
> +++ b/drivers/mfd/wm8350-core.c
> @@ -188,6 +188,7 @@ EXPORT_SYMBOL_GPL(wm8350_reg_unlock);
> int wm8350_read_auxadc(struct wm8350 *wm8350, int channel, int scale, int vref)
> {
> u16 reg, result = 0;
> + int ret = 0;
>
> if (channel < WM8350_AUXADC_AUX1 || channel > WM8350_AUXADC_TEMP)
> return -EINVAL;
> @@ -221,11 +222,13 @@ int wm8350_read_auxadc(struct wm8350 *wm8350, int channel, int scale, int vref)
> wait_for_completion_timeout(&wm8350->auxadc_done, msecs_to_jiffies(5));
>
> reg = wm8350_reg_read(wm8350, WM8350_DIGITISER_CONTROL_1);
> - if (reg & WM8350_AUXADC_POLL)
> + if (reg & WM8350_AUXADC_POLL) {
> dev_err(wm8350->dev, "adc chn %d read timeout\n", channel);
> - else
> + ret = -ETIMEDOUT;
> + } else {
> result = wm8350_reg_read(wm8350,
> WM8350_AUX1_READBACK + channel);
> + }
>
> /* Turn off the ADC */
> reg = wm8350_reg_read(wm8350, WM8350_POWER_MGMT_5);
[Severity: High]
This is a pre-existing issue, but does wm8350_reg_read() return uninitialized
memory if the underlying bus read fails?
drivers/mfd/wm8350-core.c:wm8350_reg_read() {
unsigned int data;
int err;
err = regmap_read(wm8350->regmap, reg, &data);
if (err)
dev_err(wm8350->dev, "read from reg R%d failed\n", reg);
return data;
}
If regmap_read() fails, the stack variable data is returned uninitialized,
and then written back to hardware:
reg = wm8350_reg_read(wm8350, WM8350_POWER_MGMT_5);
wm8350_reg_write(wm8350, WM8350_POWER_MGMT_5,
reg & ~WM8350_AUXADC_ENA);
Can this lead to unpredictable hardware state?
> @@ -234,6 +237,9 @@ int wm8350_read_auxadc(struct wm8350 *wm8350, int channel, int scale, int vref)
>
> mutex_unlock(&wm8350->auxadc_mutex);
>
> + if (ret)
> + return ret;
> +
[Severity: High]
Does returning an error code here cause callers to propagate negative voltages?
Since callers like show_voltage() in drivers/hwmon/wm8350-hwmon.c and
wm8350_read_battery_uvolts() in drivers/power/wm8350_power.c do not check for
negative error codes, they unconditionally multiply the return value:
drivers/hwmon/wm8350-hwmon.c:show_voltage() {
...
val = wm8350_read_auxadc(wm8350, channel, 0, 0) * WM8350_AUX_COEFF;
...
}
Will this cause mathematically meaningless negative values to be returned to
userspace via the power supply and hwmon frameworks?
> return result & WM8350_AUXADC_DATA1_MASK;
> }
> EXPORT_SYMBOL_GPL(wm8350_read_auxadc);
> --
> 2.50.1 (Apple Git-155)
>
--
Lee Jones
^ permalink raw reply [flat|nested] 2+ messages in thread