Re: [RFC 0/2] iio: add support for LMP91000EVM potentiostat board

[Jonathan Cameron <jic23@kernel.org>]

On 02/07/16 23:13, Matt Ranostay wrote:
> On Sat, Jul 2, 2016 at 3:05 PM, Matt Ranostay <mranostay@gmail.com> wrote:
>> LMP91000EVM evaluation board has LMP91000 potentiostat along with an
>> 16-bit ADC for chemical sensoring applications.
>>
>>  * add support for the TI LMP91000 potentiostat
>>  * add support for ADC141S626 and ADC161S626 ADC chips
> 
> Probably should have put what I am RFC'ing :).
> 
> * does this belong in a new path drivers/iio/potentiostat ?
I'm going for drivers/iio/AFE/potentiostat with drivers/iio/AFE/amplifiers
as well to take the only similar driver we already have.

> * first example of a iio consumer within drivers/iio, does it seems sane?
It's 'interesting'.  You've worked around the whole question of how to handle
a mux by putting a push interface equipped client on top of the polled interface
of the ADC.  It's an elegant solution that I'd never considered.

By the very nature of a mux interface, unless we are piping the mux switching
out on the same trigger system as the read back, the actual read out must be
polled rather than self clocked. Only the mux knows when it is ready.
The triggered version has all sorts of additional complexity even if we had
output buffers already to go (such as requiring the output buffering to
'lead' the input buffering to give the mux time to switch.

Question to my mind is whether this is a generic and flexible enough approach
to use for this sort of device in the future... I think we have two classes
of 'analog device' that we need to support:

1) Simple all channels always there devices such as analog accelerometers
feeding into an ADC with a sequencer (or a software based sequencer).
In that case the data flow is clearly going to go over the buffered interface.
The accelerometer driver is just massaging the data for types / scale adjustments.
It has no influence on the sampling of the data.

2) The 'smart' front end with a mux.  In this case the 'when to read' question
is driven by the front end, not the ADC.  Games could be played to push the
sampling of data over to the ADC, but is it worth doing?

So if we wanted to do this, the AFE could itself export a trigger that is then
used by the ADC which in turn pushes data back to the AFE driver via the buffered
data interface.  The AFE driver would then have to handle the demux of this
data stream into a coherent form to push out in it's own buffer.
This approach gains the following:
 - quick data transfers, particularly if we are dealing with a multiple output
   mux.  e.g we might have a 16 to 4 mux into a 4 channel simultaneous sampling
   (or sequenced) ADC. So in this case if the mux was set to provide all 16
   channels in order we'd do 4 reads of the ADC getting 0 1 2 3, 4 5 6 7 etc. 
   The mux driver would then have to recombine these 16 channels before kicking
   them out.

To do this we'd need to add an interface to allow the AFE/mux driver to set
the trigger for the ADC to its own. 

If we want to do this quickish I think that's about the lightest weight option
we can do.

Now, the question is, what are the disadvantages of going with what you
have here for this driver but keeping in mind the above for when it matters?

I'm guessing we never need to run this particularly driver very fast...
I'm inclined to say yes but would like some other opinions on this one!
(hence the expanded Cc list - please do pull in anyone else you think
might be interested!)

> * ADC driver has no IIO_CHAN_INFO_SCALE due to no regulators being defined
Should be some defined. That was easy ;)
> * Should ADC value be signed or unsigned?   -16636 is 0V, 0 is 2/VA ,
> 16635 is ~VA. Of course true zero is defined by the VREF voltage.
err. Odd. Go with signed I think.
> 
>>
>> Matt Ranostay (2):
>>   iio: adc: ti-adc1x1s: add support for TI 1-channel differential ADCs
>>   iio: potentiostat: add LMP91000 support
>>
>>  .../devicetree/bindings/iio/adc/ti-adc1x1s.txt     |  16 ++
>>  .../bindings/iio/potentiostat/lmp91000.txt         |  28 ++
>>  drivers/iio/Kconfig                                |   1 +
>>  drivers/iio/Makefile                               |   1 +
>>  drivers/iio/adc/Kconfig                            |  12 +
>>  drivers/iio/adc/Makefile                           |   1 +
>>  drivers/iio/adc/ti-adc1x1s.c                       | 233 ++++++++++++++++
>>  drivers/iio/potentiostat/Kconfig                   |  21 ++
>>  drivers/iio/potentiostat/Makefile                  |   6 +
>>  drivers/iio/potentiostat/lmp91000.c                | 303 +++++++++++++++++++++
>>  10 files changed, 622 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc1x1s.txt
>>  create mode 100644 Documentation/devicetree/bindings/iio/potentiostat/lmp91000.txt
>>  create mode 100644 drivers/iio/adc/ti-adc1x1s.c
>>  create mode 100644 drivers/iio/potentiostat/Kconfig
>>  create mode 100644 drivers/iio/potentiostat/Makefile
>>  create mode 100644 drivers/iio/potentiostat/lmp91000.c
>>
>> --
>> 2.7.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>