[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 12 Dec 2017 20:08:53 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Stefan Brüns <stefan.bruens@...h-aachen.de>
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>,
Javier Martinez Canillas <javier@....samsung.com>
Subject: Re: [PATCH v1 0/7] iio: adc: ina2xx: Rework CNVR alignment, fix
busy loops
On Sun, 10 Dec 2017 21:22:13 +0100
Stefan Brüns <stefan.bruens@...h-aachen.de> wrote:
> On Sunday, December 10, 2017 6:36:54 PM CET Jonathan Cameron wrote:
> > On Fri, 8 Dec 2017 18:41:45 +0100
> >
> > Stefan Brüns <stefan.bruens@...h-aachen.de> wrote:
> > > Currently, the INA2xx driver may end up causing 100% load on a single core
> > > and fully loading the I2C bus, which is caused by two different issues:
> > >
> > > The code uses a udelay to bridge the gab between two subsequent samples.
> > > As the sampling interval may be up to 16 seconds, the CPU is busy
> > > waiting most of the time.
> > >
> > > The second issue manifests when using the (default) "synchronous" mode.
> > > The code polls for a set conversion ready flag, but fails to align the
> > > sampling interval to the raising flag. The polling interval is
> > > (rightfully) slighly shorter than the sampling interval, so after some
> > > samples the sampling thread is continously polling.
> >
> > I'm confused. Would you mind doing an asci art example perhaps?
>
> Lets assume the conversion interval is set to 2 ms. If the polling is done at
> the sampling frequency, it might be slightly to long due to differences
> between the host and device clocks, so the polling has to run somewhat faster.
> Somewhat faster means 200 us, this is kept unchanged.
>
> In case the CNVR flag is not set, the status register is read again until the
> flag raises. The time instant the flag raises is the reference for later
> reads.
>
> The following shows the timing for the fixed code. Each character corresponds
> to 200us, first row: real time (ms), second row: conversion finished by
> device, third row: register read
> s: status, CNVR not set
> S: status, CNVR set
> S: value register, e.g. shunt voltage
> _: bus busy (each reads needs 400 us @ 100 kBit/s)
>
> __0____1____2____3____4____5____6____7____8____9
> ...C.........C.........C.........C.........C....
> ..s_S_V_.....S_V_......S_V_....s_S_V_......S_V_.
>
> At 0 ms, the conversion has not yet finished, but a 0.4 it has. Further reads
> are done at 0.4 + n * (2 - 0.2) ms. The next read happens at 2.2 ms, the third
> should at 4.0, but happens slightly late at 4.2. The read at 5.8 gets an unset
> CNVR flag, so the sampling is readjusted to happen at 6.2 + n' (2 - 0.2) ms.
>
> The old code does the following:
> __0____1____2____3____4____5____6____7____8____9
> ...C.........C.........C.........C.........C....
> ..s_S_V_...s_S_V_...s_s_S_V_.s_s_S_V_.s_s_s_S_V_
>
> The first read happens at 0 ms, it measures the time for the reading (1.2 ms),
> sleeps for the remainder (0.6 ms) and reads again. The third read takes 1.6,
> so sleep for 0.2 ms.
>
> The old code does not differentiate between time spent in the status poll and
> time spent for reading. Time spent in the status poll should not be subtracted
> from the delay until the next read (well, halfway, only the time spent with
> the poll returning !CNVR).
Thanks - got it now. Makes complete sense. There are probably other
drivers where we can do something similar but certainly good to
improve this one.
Jonathan
>
> Kind regards,
>
> Stefan
>
Powered by blists - more mailing lists