On Sun, Jul 10, 2016 at 7:20 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On 05/07/16 22:27, Matt Ranostay wrote: >> On Tue, Jul 5, 2016 at 12:56 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: >>> On 03/07/16 23:24, Matt Ranostay wrote: >>>> On Sun, Jul 3, 2016 at 6:41 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: >>>>> On 02/07/16 23:13, Matt Ranostay wrote: >>>>>> On Sat, Jul 2, 2016 at 3:05 PM, Matt Ranostay <mranostay@xxxxxxxxx> 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? >>>>> >>>> >>>> Probably over-engineering unless we actually find a need to do this in >>>> the future. >>> I get the feeling we'll end up with a high performance system needing this >>> at some point. >>>> >>>>> 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. >>>> >>>> Makes sense but there is a slight issue of the settling time for the >>>> temp channel is 2-3 milliseconds. Can't assume all mux reads are going >>>> to take the same time constant. >>> That's down to the mux driver to handle it. Only trigger once it's known >>> to be stable.. >> >> Also how would the ADC report the data back it would almost need >> dynamically setup iio channels after mux gets setup, correct? >> >> 1) ADC driver probe >> 2) MUX driver probe >> 3) MUX registers it's data channels >> 4) ADC driver needs to enumerate them > Why does the ADC driver care? > > The Mux driver is the only bit that knows what the ADC is actually capturing > as it controls both the actual wire connections and the reporting to userspace > of results. > > So. > 1) ADC driver probe > 2) Mux driver probe (gets provided ADC channels - however many it controls > the inputs for). > 3) ADC trigger set to trigger provided by Mux. > > To give a simple example, lets consider a 2 input single channel mux going into > a single channel ADC. Mux trigger called (imaginatively) mux_trig0 > > Mux is consumer of the ADC channel. > > Setup: > 1) Mux registers as a 'buffer' consumer of the ADC channel. > 2) Mux has a trigger exposed (which is how it controls the capture.) > 3) ADC trigger set to the mux exposed one. > > A scan. (triggered say by a high resolution timer trigger). > 1) Mux picks channel 1 and waits for it to stabilize. > 2) Mux 'fires' trigger to initiate a capture and gets the resulting callback > call with the value. Stashes it somewhere > 3) Mux selects channel 2 and waits for it to stabilize > 4) Mux 'fires' trigger to initiate a capture and gets the resulting callback > call with the value. Stashes it somewhere. > 5) Mux driver can then combine the two values to form the 'scan' and push > that to it's buffer complete with whatever timestamp makes sense. > This Mux driver controlled buffer is the one userspace uses to get the data. > Only question is how does the callback come into play here with a trigger? Not sure I have seen this in the API so far. > Missing bit of all this is a consumer being able to control the providers > trigger. Doubt that would be hard to add. > Worse case initially it will have to be manually set. > I think that covers all possible circumstances where we have explicit > control of the mux or at least the ability to set it to a known state. > > Jonathan > >> >>>> >>>>> >>>>> 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. >>>> >>>> Of course in this case the ADC and LMP91000 device are using both the >>>> hrtimer trigger, albeit of course you can't do it at the same time. So >>>> it is polling no matter what. >>> Fair point. >>>> >>>>> >>>>> 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!) >>>> >>>> Yeah the sample response of the sensor isn't that high speed. Maybe a >>>> few dozen hertz. >>>> >>>>> >>>>>> * 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@xxxxxxxxxxxxxxx >>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>> >>>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >>>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
