From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 320782D0602 for ; Thu, 2 Jul 2026 16:00:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783008024; cv=none; b=WplI6prhI589P/om/Gg0LU8zCS2smi3VKLeyvPOJYTmtSRKq+iTgzNiVzS66k65pa6yJdTvZhzuv5dwiZe+2VgGu56/RchzOmYu5QVD/24H344uAfrnKFDR6prkSVReUpXFGA6yVUWc4Md7zIbGL9K1oOTLQG6NeAkRVHRDLmtA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783008024; c=relaxed/simple; bh=j4S1oqUo9L/IpOQpAI+jE0nNOSDiSQWzM+QMhxDPc7E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=UhZlXGmpjqW2+5gslfwggAJodG6Nbcd1LKNnfwGVpNo9Of2fK45PNGIJ0JFl36tYpbxI73BP6rx7rbXMmRg/o9I4jO9nK5et+LtRWydPmaEPMXWajBRale4GWHUeM9erkZfzdkbc+aR/5oEFpod4w+L6Mmx7l714dD1g2wEh63Q= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=l7QKADnt; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="l7QKADnt" Received: by smtp.kernel.org (Postfix) with ESMTPSA id EC5AB1F000E9; Thu, 2 Jul 2026 16:00:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783008022; bh=dsMhnTSdsesnO9ddH+J1x2lgBqx3A4X2Gr6F+u2Bsco=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=l7QKADntlXTNYZA0kft9b40G+CegRR4vsMZgd/pUse6nll1HLpHbnxdVSOq3yfXYm 3Kl0RaSdh7bVaOWSLu5nneixLnmMcqckRTlVLcZJIlowbKl2qye2Q3RMsa6RgI0fex jmdKRL8VvpVUkZUQavz77RoGjY1+uP0CdHuMQbKwNEGS5Tq+TQAuWdd+rlPAItWC9j x5+IU+1zIBIPU0lXnehc11PZeKeZKSSjA4FmAYq9BlDmEHGU3o2Ig1s8Xy+MQmaeQa D5gzg9Bzj5ciKuNMmgXLZ3CIyV8F5xM+FgdrinBIRO+td+aR8oXvDCNapuksrU96rR m2qS0Lq+IF0qw== Date: Thu, 2 Jul 2026 17:00:19 +0100 From: Lee Jones To: Pengpeng Hou Cc: patches@opensource.cirrus.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mfd: wm8350: return an error for AUXADC timeouts Message-ID: <20260702160019.GZ2108533@google.com> References: <20260624144227.75230-1-pengpeng@iscas.ac.cn> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260624144227.75230-1-pengpeng@iscas.ac.cn> 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 > --- > 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