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

[Jonathan Cameron <jic23@kernel.org>]

> >   
> 
> Hi Jonathan,
> 
> Thank you for the detailed review.
> 
> I will address these comments in v4.
Hi,

A couple of process things.

Most of this didn't need a reply at all.  When you agree with feedback
and plan to modify the code, keep any description of that for
the change log of v2.

Also, reply inline to individual comments. The reviewer has probably 
forgotten most of the details of what they commented on and so
has to do the tiresome step of reading their own reply and then correlating
that with what you have at the end.  If you reply inline as I did during
the review, the context they need is right there.  Keep to tight
focused comments where you want further feedback or disagree with the
review.

...

> 
> Regarding the one-hot scan-mask handling, the MA35D1 hardware itself is
> not limited to one sample module at a time. The EADC has one SAR
> converter shared by multiple sample modules. Multiple sample modules can
> be triggered together, after which the hardware priority arbitrator
> performs their conversions sequentially.
> 
> The initial driver, however, currently uses only sample module 0 and
> performs one voltage conversion for each buffered trigger. Therefore
> the buffered interface exposed by this version of the driver supports
> one active voltage channel per scan. I will make this limitation
> explicit using the standard one-hot scan-mask validation callback and
> use iio_for_each_active_channel() when selecting the active channel.
> Support for using multiple sample modules for multi-channel buffered
> scans can be added separately in the future.

This part would have been helpful inline next to the comment it is addressing.

> 
> Since the scan layout is therefore fixed to one u16 sample plus an
> optional timestamp, I will replace IIO_DECLARE_BUFFER_WITH_TS() with a
> fixed scan structure containing one u16 sample and an aligned timestamp.

Jonathan