Re: [PATCH 2/4] iio: adc: mt6323-auxadc: add mt6323 PMIC AUXADC driver

[Roman Vivchar]

Hi Nuno,

On Tuesday, June 2nd, 2026 at 7:42 PM, Nuno Sá <noname.nuno@xxxxxxxxx> wrote:

> On Tue, 2026-06-02 at 15:46 +0300, Roman Vivchar via B4 Relay wrote:

...

> >
> > +MEDIATEK MT6323 PMIC AUXADC DRIVER
> > +M:	Roman Vivchar <rva333@xxxxxxxxxxxxxx>
> > +L:	linux-iio@xxxxxxxxxxxxxxx
> > +L:	linux-mediatek@xxxxxxxxxxxxxxxxxxx (moderated for non-subscribers)
> > +S:	Maintained
> > +F:	drivers/iio/adc/mt6323-auxadc.c
> > +F:	include/dt-bindings/iio/adc/mediatek,mt6323-auxadc.h
>
> The above file was not added in this patch

The header file is added in patch 1 (dt-bindings). Following Krzysztof's
feedback on the previous version, I squashed the MAINTAINERS into this patch.
Please let me know if I misunderstood anything.

...

> > +	MTK_PMIC_IIO_CHAN(accdet,    MT6323_AUXADC_ACCDET,    MT6323_AUXADC_ADC7,
> > IIO_VOLTAGE),
> > +};
>
> All of the above are IIO_VOLTAGE. Just remove _ch_type then.

Ack.

> > +
> > +/**
> > + * struct mt6323_auxadc - Main driver structure
> > + * @regmap:        Regmap from PWRAP
> > + * @lock:          Mutex to serialize AUXADC reading vs configuration
> > + *
> > + * The MediaTek MT6323 (as well as a lot of other PMICs) has the following
> > hierarchy:
> > + * PMIC AUXADC <- PMIC MFD <- SoC PWRAP (wrapper for PWRAP FSM)
> > + *
> > + * Therefore, PWRAP regmap should be obtained using dev->parent->parent.
> > + */
>
> The above kerneldoc seems unnecessary to me.

Ack.

...

> > +	case IIO_CHAN_INFO_RAW:
> > +		scoped_guard(mutex, &auxadc->lock) {
> > +			ret = mt6323_auxadc_prepare_channel(auxadc);
> > +			if (ret)
> > +				return ret;
> > +
> > +			ret = mt6323_auxadc_request(auxadc, chan->channel);
> > +			if (ret)
> > +				return ret;
> > +
> > +			/* Hardware limitation: the AUXADC needs a delay to become
> > ready. */
> > +			fsleep(300);
> > +
> > +			ret = mt6323_auxadc_read(auxadc, chan, val);
> > +			if (ret)
> > +				return ret;
>
> Could be return mt6323_auxadc_read(...)

The mt6323_auxadc_read returns 0, while IIO expects IIO_VAL_INT (defined as 1).
Should the mt6323_auxadc_read function return 1 for success?

Best regards,
Roman