On Sun, 10 Dec 2017 21:22:13 +0100 Stefan Brüns <stefan.bruens@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
