Re: [PATCH 2/3] iio: adc: mt6359-auxadc: mask conversion ready bit in mt6358_read_imp

[Roman Vivchar]

On Wednesday, November 4th, 2026 at 1:45 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

> On Tue, 03 Nov 2026 18:32:08 +0200
> Roman Vivchar via B4 Relay <devnull+rva333.protonmail.com@xxxxxxxxxx> wrote:
> 
> > From: Roman Vivchar <rva333@xxxxxxxxxxxxxx>
> >
> > In mt6358_read_imp() the raw ADC value is returned directly without
> > masking out ready bit, resulting in incorrect battery voltage reporting.
> 
> Can you add where the ready bit is.  That might help someone
> looking at wrong values and wondering if this is what is causing them.

It's at the end of mt6359_auxadc_read_adc() function, which is included in
the commit message. Do you mean include the code part too?
"*out = val & GENMASK(chan->scan_type.realbits - 1, 0);"

> This one is pretty glaring as bugs go (given I think this means the
> top bit is set?) so any reason it wasn't obvious to anyone testing
> this device?

I can't really tell if the driver was properly tested. As far as I understand,
Chromebook devices use some embedded controller, which likely manages this
kind of stuff without PMIC ADC. There's no in-tree mobile SoCs with mt6357,
6358 or 6359, nor fuel gauge or thermal drivers (yet), so maybe that's why.

> Jonathan
> 
> 
> >
> > Apply the mask to strip the ready bit, matching the behavior in
> > mt6359_auxadc_read_adc() function.
> >
> > Fixes: 3587914bf61d ("iio: adc: Add support for MediaTek MT6357/8/9 Auxiliary ADC")
> > Signed-off-by: Roman Vivchar <rva333@xxxxxxxxxxxxxx>
> > ---
> >  drivers/iio/adc/mt6359-auxadc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/mt6359-auxadc.c b/drivers/iio/adc/mt6359-auxadc.c
> > index 951d236f972b..09432d0974cf 100644
> > --- a/drivers/iio/adc/mt6359-auxadc.c
> > +++ b/drivers/iio/adc/mt6359-auxadc.c
> > @@ -502,7 +502,7 @@ static int mt6358_read_imp(struct mt6359_auxadc *adc_dev,
> >  	mt6358_stop_imp_conv(adc_dev);
> >
> >  	if (vbat)
> > -		*vbat = val_v;
> > +		*vbat = val_v & GENMASK(chan->scan_type.realbits - 1, 0);
> >  	if (ibat)
> >  		*ibat = 0;
> >
> >
> 
>