On 01/06/16 13:34, Laxman Dewangan wrote:
Add support for INA3221 SW driver via IIO ADC interface. The device isHi Laxman,
register as iio-device and provides interface for voltage/current and power
monitor. Also provide interface for setting oneshot/continuous mode and
critical/warning threshold for the shunt voltage drop.
Signed-off-by: Laxman Dewangan <ldewangan@xxxxxxxxxx>
As ever with any driver lying on the border of IIO and hwmon, please include
a short justification of why you need an IIO driver and also cc the
hwmon list + maintainers. (cc'd on this reply).
I simply won't take a driver where the hwmon maintainers aren't happy.
As it stands I'm not seeing obvious reasons in the code for why this
should be an IIO device.
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.