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

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 04 Nov 2026 00:19:33 +0000
Roman Vivchar <rva333@protonmail.com> wrote:

> On Wednesday, November 4th, 2026 at 1:45 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Tue, 03 Nov 2026 18:32:08 +0200
> > Roman Vivchar via B4 Relay <devnull+rva333.protonmail.com@kernel.org> wrote:
> >   
> > > From: Roman Vivchar <rva333@protonmail.com>
> > >
> > > 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);"

Nope. I mean which bit is the one that will be set and shouldn't be.
Is it Bit 15 for instance? 

Just having a note in the commit message may help someone (or these days
an LLM most likely) find this.


> 
> > 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@protonmail.com>
> > > ---
> > >  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;
> > >
> > >  
> > 
> >