Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC

[Chi-Wen Weng <cwweng.linux@gmail.com>]

Hi Jonathan,

Thank you for the review.

 > Not sure this doc helps. What are these interrupts for?
 > The driver only uses one of them so why are there 4?
 >
 > May well need interrupt-names to allow gaps in the list to
 > work but hard to tell without more information.

The hardware has four EADC interrupt outputs, ADINT0 to ADINT3. The
conversion-complete source for each interrupt can be selected through
the EADC interrupt source registers.

However, this initial driver only uses sample module 0 and routes its
end-of-conversion event to ADINT0. So the binding does not need to
describe the unused interrupt outputs at this stage.

I will simplify this in v3 and document only one interrupt entry for
ADINT0. If support for the other ADINT lines is added later, the binding
can be extended with interrupt-names at that point.

I will also drop the redundant maxItems from the interrupts property, as
reported by Rob's bot.

 > Do you need that explicitly. I think adc.yaml already brings that in.

No, it is not needed. I will drop the explicit diff-channels property
from the binding.

 > For these take a look at the description of diff-channels.
 > It states that where the pairs are hard wired you can just use reg
 > and not specify diff-channels.  Seems to apply here and will greatly
 > simplify this binding.

Yes, the differential pairs are fixed in hardware. The valid pairs are
0-4, 1-5, 2-6 and 3-7.

I will rework the channel binding to avoid diff-channels for these fixed
pairs. For fixed differential channels, reg will identify the hardware
pair. For single-ended inputs, I will use single-channel so that the
single-ended input number is explicit and not confused with a fixed
differential pair index.

That should remove the large allOf block and make the binding much
simpler.

 > What are these channels?  There is a reference to one
 > internally wired one so what are the other 3?

Channels 4 to 7 are external ADC input pins. They can be used as
single-ended inputs, and in differential mode they are the fixed negative
inputs for channels 0 to 3.

The internally wired channel is channel 8, which is the VBAT input. This
binding only describes the external ADC input pins, so I will clarify the
description to avoid confusion.

Thanks again. I will address these issues in the next revision.

Best regards,
Chi-Wen