On 14/07/16 04:36, Matt Ranostay wrote: > 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. The ABI has been there a long time - just not much used :) See buffers\industrialio-buffer-cb.c Was original written to support pushing data out to the iio-input bridge driver (which was last posted in 2012)... Just got used (slightly wrongly) in the sunxi touchscreen driver as well. > >> 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 we'd have to automate it or this would all be really clunky to use from userspace. > >> 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 > -- 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
