[PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp

[PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp

[Salah Triki]

In mt6358_read_imp(), the return value of regmap_read() is currently
ignored. This is problematic because if the bus read fails the variable
val_v remains uninitialized.

The function subsequently assigns this uninitialized stack value to
*vbat, leading to incorrect measurement results being reported to
the IIO subsystem without any error indication.

Update the function to check the return value of regmap_read(). Ensure
that mt6358_stop_imp_conv() is still called to clean up the hardware
state before returning the error code.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 drivers/iio/adc/mt6359-auxadc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mt6359-auxadc.c b/drivers/iio/adc/mt6359-auxadc.c
index 6b9ed9b1fde2..f927bff4a26a 100644
--- a/drivers/iio/adc/mt6359-auxadc.c
+++ b/drivers/iio/adc/mt6359-auxadc.c
@@ -497,10 +497,13 @@ static int mt6358_read_imp(struct mt6359_auxadc *adc_dev,
 		return ret;
 
 	/* Read the params before stopping */
-	regmap_read(regmap, reg_adc0 + (cinfo->imp_adc_num << 1), &val_v);
+	ret = regmap_read(regmap, reg_adc0 + (cinfo->imp_adc_num << 1), &val_v);
 
 	mt6358_stop_imp_conv(adc_dev);
 
+	if (ret)
+		return ret;
+
 	if (vbat)
 		*vbat = val_v;
 	if (ibat)
-- 
2.43.0

Re: [PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp

[Andy Shevchenko]

On Mon, Apr 27, 2026 at 09:54:57AM +0100, Salah Triki wrote:
> In mt6358_read_imp(), the return value of regmap_read() is currently
> ignored. This is problematic because if the bus read fails the variable
> val_v remains uninitialized.
> 
> The function subsequently assigns this uninitialized stack value to
> *vbat, leading to incorrect measurement results being reported to
> the IIO subsystem without any error indication.
> 
> Update the function to check the return value of regmap_read(). Ensure
> that mt6358_stop_imp_conv() is still called to clean up the hardware
> state before returning the error code.

Sounds like this deserves a Fixes tag, but the problem is that the whole driver
is written like this. Why does having this fixed make it special?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp

[Salah Triki]

On Mon, Apr 27, 2026 at 12:14:40PM +0300, Andy Shevchenko wrote:
> On Mon, Apr 27, 2026 at 09:54:57AM +0100, Salah Triki wrote:
> > In mt6358_read_imp(), the return value of regmap_read() is currently
> > ignored. This is problematic because if the bus read fails the variable
> > val_v remains uninitialized.
> > 
> > The function subsequently assigns this uninitialized stack value to
> > *vbat, leading to incorrect measurement results being reported to
> > the IIO subsystem without any error indication.
> > 
> > Update the function to check the return value of regmap_read(). Ensure
> > that mt6358_stop_imp_conv() is still called to clean up the hardware
> > state before returning the error code.
> 
> Sounds like this deserves a Fixes tag, but the problem is that the whole driver
> is written like this. Why does having this fixed make it special?
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

You are right, Andy. I noticed that several other functions in this driver
share the same issue. I will send a V2 that addresses all unchecked
regmap_read() calls in this file and I will include the Fixes tag. Thanks
for the feedback.

Best Regards
Salah Triki

[PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp

[Salah Triki]

In mt6358_read_imp(), the return value of regmap_read() is currently
ignored. This is problematic because if the bus read fails the variable
val_v remains uninitialized.

The function subsequently assigns this uninitialized stack value to
*vbat, leading to incorrect measurement results being reported to
the IIO subsystem without any error indication.

Update the function to check the return value of regmap_read(). Ensure
that mt6358_stop_imp_conv() is still called to clean up the hardware
state before returning the error code.

Fixes: 3587914bf61 ("iio: adc: Add support for MediaTek MT6357/8/9 Auxiliary ADC")
Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes in v2:
- Added Fixes tag.
- Re-examined the entire driver for unchecked regmap operations.
  While several regmap_write() and regmap_set_bits() calls also ignore
  return values, I focused on this specific regmap_read() in 
  mt6358_read_imp() because it leads to an uninitialized variable usage
  (val_v). This makes this fix critical for reporting correct data 
  to userspace.

 drivers/iio/adc/mt6359-auxadc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mt6359-auxadc.c b/drivers/iio/adc/mt6359-auxadc.c
index 6b9ed9b1fde2..f927bff4a26a 100644
--- a/drivers/iio/adc/mt6359-auxadc.c
+++ b/drivers/iio/adc/mt6359-auxadc.c
@@ -497,10 +497,13 @@ static int mt6358_read_imp(struct mt6359_auxadc *adc_dev,
 		return ret;
 
 	/* Read the params before stopping */
-	regmap_read(regmap, reg_adc0 + (cinfo->imp_adc_num << 1), &val_v);
+	ret = regmap_read(regmap, reg_adc0 + (cinfo->imp_adc_num << 1), &val_v);
 
 	mt6358_stop_imp_conv(adc_dev);
 
+	if (ret)
+		return ret;
+
 	if (vbat)
 		*vbat = val_v;
 	if (ibat)
-- 
2.43.0

Re: [PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp

[Jonathan Cameron]

On Mon, 27 Apr 2026 12:31:34 +0100
Salah Triki <salah.triki@gmail.com> wrote:

> On Mon, Apr 27, 2026 at 12:14:40PM +0300, Andy Shevchenko wrote:
> > On Mon, Apr 27, 2026 at 09:54:57AM +0100, Salah Triki wrote:  
> > > In mt6358_read_imp(), the return value of regmap_read() is currently
> > > ignored. This is problematic because if the bus read fails the variable
> > > val_v remains uninitialized.
> > > 
> > > The function subsequently assigns this uninitialized stack value to
> > > *vbat, leading to incorrect measurement results being reported to
> > > the IIO subsystem without any error indication.
> > > 
> > > Update the function to check the return value of regmap_read(). Ensure
> > > that mt6358_stop_imp_conv() is still called to clean up the hardware
> > > state before returning the error code.  
> > 
> > Sounds like this deserves a Fixes tag, but the problem is that the whole driver
> > is written like this. Why does having this fixed make it special?
> > 
> > -- 
> > With Best Regards,
> > Andy Shevchenko
> > 
> >   
> 
> You are right, Andy. I noticed that several other functions in this driver
> share the same issue. I will send a V2 that addresses all unchecked
> regmap_read() calls in this file and I will include the Fixes tag. Thanks
> for the feedback.
What's the bus in this case?

Whilst maybe we should be checking them, it is not uncommon to forgo those
checks in internal busses.  In general they should be more reliable than
something going over PCB tracks like spi or i2c.

I'd be tempted to fix the uninitialized variable by setting it at declaration
rather than worry too much about checking all the regmap return values.

> 
> Best Regards
> Salah Triki