Re: [RFC PATCH v1 0/4] Add support for Allwinner GPADC on D1/T113s/R329 SoCs

[Jonathan Cameron]

On Wed, 24 May 2023 14:36:28 +0300
Maxim Kiselev <bigunclemax@xxxxxxxxx> wrote:

> Hi Andre,
> 
> thanks for you comments
> 
> > This may sound kind of obvious, but wouldn't it be easier to model this
> > with one compatible string, and have the number of channels as a DT
> > property?  
> 
> Yes, I completely agree that using separate config for each SoCs is looks
> overcomplicated because the only difference is the number of channels.
> I thought about a DT property with channels number but I didn't find
> another ADC driver with the same approach (except i2c ADC's with child nodes).
If you are 100% sure that that devices are either
1) Detectable at runtime
2) Identical in functionality.

So that in neither case will any changes on driver support expose differences
in the future then a single compatible is fine.

The back up is that you use fallback compatibles - list more than one.
Whilst it doesn't matter (as no differences found) the driver can use
the first one.  If differences become apparent later, others may be used.

I'm not however keen on a simple channel count parameter.  If you want
to go that way, it's better to provide the fine control of individual channel
child nodes (see Documentation/devicetree/bindings/iio/adc/adc.yaml)

That way the control is on which channels are wired to something useful, rather
than whether the device can read them or not (which is pointless if no one
wired them up.


> 
> > Or, alternatively, using iio/multiplexer/io-channel-mux.yaml, since it's
> > only one ADC anyway?  
> I'm sorry, I didn't quite understand what you're suggesting.

That's normally only used for a separate MUX where we need a separate driver
to handle it.  If used on a device like this it would expose additional complexity
to userspace with no benefits in generality etc.

> 
> > And btw: it seems that the T507 (the H616 die with a different pinout) has
> > the same IP, with four channels:
> > http://dl.linux-sunxi.org/T507/  
> 
> Oh, thanks for pointing that. I'll add it to the list in the next version.