Re: [PATCH 2/3] iio: adc: ina3221: Add support for IIO ADC driver for TI INA3221

From: Laxman Dewangan
Date: Fri Jun 03 2016 - 08:29:51 EST

On Friday 03 June 2016 05:39 PM, Jonathan Cameron wrote:
On 03/06/16 12:26, Laxman Dewangan wrote:
On Friday 03 June 2016 03:36 PM, Jonathan Cameron wrote:

I thought that all ADC or monitors are going to be part of IIO device
framework. I saw the ina2xx which is same (single channel) which was
my reference point.
That had a rather specific use case IIRC - they needed the buffered support
to get the data fast enough.

I think in our particular requirements, we dont need the buffering support but HW keep monitor and check with warning/critical threshold to generate HW signal.
Funily enough I know this datasheet a little as was evaluating
it for use on some boards at the day job a week or so ago.

Various comments inline. Major points are:
* Don't use 'fake' channels to control events. If the events infrastructure
doesn't handle your events, then fix that rather than working around it.
* There is a lot of ABI in here concerned with oneshot vs continuous.
This seems to me to be more than it should be. We wouldn't expect to
see stuff changing as a result of switching between these modes other
than wrt to when the data shows up. So I'd expect to not see this
directly exposed at all - but rather sit in oneshot unless either:
1) Buffered mode is running (not currently supported)
2) Alerts are on - which I think requires it to be in continuous mode.

Other question to my mind is whether we should be reporting vshunt or
(using device tree to pass resistance) current.
This is bus and shunt voltage device for power monitoring. In our
platforms, we use this device for bus current and so power monitor.

We have two usecases, one is one shot, read when it needs it. And
other continuous when we have multiple core running then continuous
mode to get the power consumption by rail.
That's fine, but continuous should be using the buffered interfaces
really as that's there explicitly to support groups of channels
captured using a sequencer.

Then the abi ends up much more standard which is nice. Also allows
for high speed ish continuous monitoring which is what the was
I think the point of the single channel driver.

The requirement for continuous monitoring is to ADC generate alert when the current on bus cross the threshold of warning/critical level so that alert signal can be used for throttling.
So in my this particular usecase, we may not need buffered data.

Yaah, alert is used only on continuous mode and mainly used for
throttling when rail power goes beyond some limit.
Of interesting in Linux, or routed directly to hardware?

Yaah, In some platform this is routed to the hardware for throttling.