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

[David Lechner <dlechner@baylibre.com>]

On 6/3/26 6:24 AM, Roman Vivchar wrote:
> Hi Nuno,
> 
> On Tuesday, June 2nd, 2026 at 7:42 PM, Nuno Sá <noname.nuno@gmail.com> 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@protonmail.com>
>>> +L:	linux-iio@vger.kernel.org
>>> +L:	linux-mediatek@lists.infradead.org (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.

Usually, we want the MAINTAINERS entry in the same patch that
the file was added. It is fine to include MAINTAINERS changes
in the dt-bindings patch.

>>> +	case IIO_CHAN_INFO_RAW:
>>> +		scoped_guard(mutex, &auxadc->lock) {

We can avoid extra indent by making this:

	case IIO_CHAN_INFO_RAW: {
		guard(mutex)(&auxadc->lock);

		...
		
		return IIO_VAL_INT;
	}

>>> +			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?

mt6323_auxadc_read() would need to return IIO_VAL_INT.