Re: [PATCH v1 4/5] iio: adc: mt6359: Add support for MediaTek MT6363 PMIC AUXADC

[Andy Shevchenko]

On Wed, Jun 25, 2025 at 03:29:47PM +0200, AngeloGioacchino Del Regno wrote:
> Il 23/06/25 16:30, Andy Shevchenko ha scritto:
> > On Mon, Jun 23, 2025 at 02:00:27PM +0200, AngeloGioacchino Del Regno wrote:

...

> > > +	if (MTK_AUXADC_HAS_FLAG(cinfo, IS_SPMI)) {
> > > +		/* If the previous read succeeded, this can't fail */
> > > +		regmap_read(regmap, reg - 1, &lval);
> > 
> > No error check? lval may contain garbage here, right?
> 
> No, because if the previous read succeeded, this can't fail, and also cannot ever
> possibly contain garbage (and if it does, - but again, that can't happen - there is
> no way to validate that because valid values are [0x00..0xff] anyway).

Never say never. Any regmap_*() call that performs I/O might fail. You can't
predict with 100% guarantee the HW behaviour in all possible scenarios.

> > > +		val = (val << 8) | lval;
> > 
> > Is it guaranteed that lval is always less than 256 (if unsigned)?
> 
> Yes, with SPMI that is guaranteed.
> 
> > > +	}

...

> > > +		regmap_update_bits(regmap, cinfo->regs[desc->ext_sel_idx],
> > > +				   MT6363_EXT_PURES_MASK, ext_sel);
> > 
> > No  error check?
> 
> No, because if the previous reads and/or writes succeeded, it is impossible for
> this to fail :-)

Ditto.

I.o.w. the failed regmap_*() call can be a signal that something on the
communication channel with the HW went wrong, Depending on the severity of this
call the device driver may decide what to do next.

-- 
With Best Regards,
Andy Shevchenko