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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5751753D.40107@nvidia.com>
Date:	Fri, 3 Jun 2016 17:47:01 +0530
From:	Laxman Dewangan <ldewangan@...dia.com>
To:	Jonathan Cameron <jic23@...nel.org>, <robh+dt@...nel.org>,
	<corbet@....net>, <lars@...afoo.de>
CC:	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-doc@...r.kernel.org>, <linux-iio@...r.kernel.org>,
	<linux-hwmon@...r.kernel.org>, Jean Delvare <khali@...ux-fr.org>,
	"Guenter Roeck" <linux@...ck-us.net>
Subject: Re: [PATCH 2/3] iio: adc: ina3221: Add support for IIO ADC driver
 for TI INA3221


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.