Re: [PATCH] mfd: wm8350: return an error for AUXADC timeouts

[Lee Jones <lee@kernel.org>]

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