Re: [PATCH 2/2] iio: adc: Add Nuvoton MA35D1 EADC driver

[Chi-Wen Weng]

Hi Jonathan,

Thanks for the review.

> Look at the ACQUIRE() macros for claim direct stuff (in iio.h)
> and then use guard() for this.  Be careful to add {} to define scope
> to being this case block.  May not save much code but it will be easeir
> to read than this currently is.

Understood. I will switch the direct read path to
IIO_DEV_ACQUIRE_DIRECT_MODE() / IIO_DEV_ACQUIRE_FAILED() and use
guard(mutex) for the driver lock. I will also add a scope around the
IIO_CHAN_INFO_RAW case block.

> Move assignment down to just above the loop.  Makes it easier for
> reviewers to associate the initial value with what is going on.

Will do.

> For new code
>     ret = devm_mutex_init(&adc->lock);
>     if (ret)
>         return ret;

Will fix this in v2.

> Second part of this is set by the triggered_buffer call later. So 
don't set it
> here.

Understood. I will keep only INDIO_DIRECT_MODE here and let
devm_iio_triggered_buffer_setup() set the triggered-buffer mode.

> Failing suspend because a buffer is enabled is unlikely to be popular.
> Can you not save necessary state and restore so buffered capture 
continues
> on resume?

Yes, I will rework the PM callbacks so suspend does not fail just because
the buffer is enabled.

For v2, I plan to suspend the IIO triggering path, stop the EADC
conversion/interrupt path, save the state needed for buffered capture,
and then disable the ADC and its clock. On resume, I will re-enable the
clock, reinitialize the ADC, restore the scan configuration, and restart
the buffered conversion path if it was active before suspend.

Thanks,
Chi-Wen