linux-kernel - Re: [PATCH v1 3/7] iio: adc: ina2xx: Remove unneeded dummy read to clear CNVR flag

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 13 Dec 2017 00:48:40 +0100
From:   Stefan Brüns <stefan.bruens@...h-aachen.de>
To:     Jonathan Cameron <jic23@...nel.org>
CC:     <linux-iio@...r.kernel.org>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Maciej Purski <m.purski@...sung.com>,
        <linux-kernel@...r.kernel.org>, "Andrew F . Davis" <afd@...com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Hartmut Knaack <knaack.h@....de>
Subject: Re: [PATCH v1 3/7] iio: adc: ina2xx: Remove unneeded dummy read to clear CNVR flag

On Tuesday, December 12, 2017 9:15:30 PM CET Jonathan Cameron wrote:
> On Sun, 10 Dec 2017 21:53:42 +0100
> 
> Stefan Brüns <stefan.bruens@...h-aachen.de> wrote:
> > On Sunday, December 10, 2017 6:27:33 PM CET Jonathan Cameron wrote:
> > > On Fri, 8 Dec 2017 18:41:48 +0100
> > > 
> > > Stefan Brüns <stefan.bruens@...h-aachen.de> wrote:
> > > > Although the datasheet states the CNVR flag is cleared by reading the
> > > > BUS_VOLTAGE register, it is actually cleared by reading any of the
> > > > voltage/current/power registers.
> > > > 
> > > > The behaviour has been confirmed by TI support:
> > > > http://e2e.ti.com/support/amplifiers/current-shunt-monitors/f/931/p/64
> > > > 7053
> > > > /2378282
> > > > 
> > > > Signed-off-by: Stefan Brüns <stefan.bruens@...h-aachen.de>
> > > 
> > > I haven't checked the code thoroughly so there may well be something
> > > stopping it but have you checked the case where the only channel enabled
> > > is
> > > the timestamp?
> > > 
> > > Obviously it makes little sense, but IIRC there is nothing in the core
> > > preventing that happening.
> > 
> > The timestamp is completely unrelated to the status register, so I fail to
> > understand your question. Can you please clarify?
> 
> If you only have a timestamp, the trigger will still fire (I think)
> but you'll do no reading at all from the device.  If configured in this,
> admittedly odd, way you should just get a stream of timestamps with no
> data.

If there are reads depends on the mode - if running asynchronously, it will 
just stream out 64 bits of timestamp each interval. In synchronous mode, the 
driver will read the status register (low bits of bus voltage register for 
INA219, msk register for INA226), which implicitly clears the CNVR flag.
 
> > This only removes a redundant read.
> 
> The question is whether it is redundant if we have no non timestamp
> registers enabled.

According to the documentation, INA219 and 226 had to be treated differently. 
As it turned out, both actually behave the same way regarding the CNVR flag, 
so we just poll the status register, which for both devices clears the flag.

Regards,

Stefan


-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019
Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)