d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Ensure a measurement is complete before reading the result in ads111x.

Browse Source 
Also, disable the comparator by default; it's not used for anything.

The previous logic would start a measurement, and then pause_sbt() for the
averaging time currently configured in the chip.  After waiting that long,
the code would blindly read the measurement register and return its value.
The problem is that the chip's idea of averaging time is based on its
internal free-running 1MHz oscillator, which may be running at a wildly
different rate than the kernel clock.  If the chip's internal timer was
running slower than the kernel clock, we'd end up grabbing a stale result
from an old measurement.

The driver now still uses pause_sbt() to yield the cpu while waiting for
the measurement to complete, but after sleeping it checks the chip's status
register to ensure the measurement engine is idle.  If it's not, the driver
uses a retry loop to wait a bit (5% of the original wait time) then check
again for completion.
This commit is contained in:
ian 2019-09-05 19:07:48 +00:00
parent d41edcbb5f
commit fe9f05cf09
1 changed files with 25 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
sys/dev/iicbus/ads111x.c
View File

@ -62,6 +62,7 @@ __FBSDID("$FreeBSD$");
#define ADS111x_CONF_GAIN_SHIFT 9 /* Programmable gain amp */
#define ADS111x_CONF_MODE_SHIFT 8 /* Operational mode */
#define ADS111x_CONF_RATE_SHIFT 5 /* Sample rate */
#define ADS111x_CONF_COMP_DISABLE 3 /* Comparator disable */
#define ADS111x_LOTHRESH 2 /* Compare lo threshold (rw) */
@ -81,7 +82,8 @@ __FBSDID("$FreeBSD$");
* comparator and we'll leave it alone if they do. That allows them connect the
* alert pin to something and use the feature without any help from this driver.
*/
#define ADS111x_CONF_DEFAULT (1 << ADS111x_CONF_MODE_SHIFT)
#define ADS111x_CONF_DEFAULT \
((1 << ADS111x_CONF_MODE_SHIFT) | ADS111x_CONF_COMP_DISABLE)
#define ADS111x_CONF_USERMASK 0x001f
/*
@ -189,7 +191,7 @@ static int
ads111x_sample_voltage(struct ads111x_softc *sc, int channum, int *voltage)
{
struct ads111x_channel *chan;
int err, cfgword, convword, rate, waitns;
int err, cfgword, convword, rate, retries, waitns;
int64_t fsrange;
chan = &sc->channels[channum];
@ -205,7 +207,8 @@ ads111x_sample_voltage(struct ads111x_softc *sc, int channum, int *voltage)
/*
* Calculate how long it will take to make the measurement at the
* current sampling rate (round up), and sleep at least that long.
* current sampling rate (round up). The measurement averaging time
* ranges from 300us to 125ms, so we yield the cpu while waiting.
*/
rate = sc->chipinfo->ratetab[chan->rateidx];
waitns = (1000000000 + rate - 1) / rate;
@ -213,20 +216,27 @@ ads111x_sample_voltage(struct ads111x_softc *sc, int channum, int *voltage)
if (err != 0 && err != EWOULDBLOCK)
return (err);
#if 0
/*
* Sanity-check that the measurement is complete. Not enabled by
* default because checking wastes 200-800us just in moving the status
* command and result across the i2c bus, which could double the time it
* takes to get one measurement. Unlike most i2c slaves, this device
* does not auto-increment the register number on reads, so we can't
* read both status and measurement in one operation.
* In theory the measurement should be available now; we waited long
* enough. However, the chip times its averaging intervals using an
* internal 1 MHz oscillator which likely isn't running at the same rate
* as the system clock, so we have to double-check that the measurement
* is complete before reading the result. If it's not ready yet, yield
* the cpu again for 5% of the time we originally calculated.
*
* Unlike most i2c slaves, this device does not auto-increment the
* register number on reads, so we can't read both status and
* measurement in one operation.
*/
if ((err = ads111x_read_2(sc, ADS111x_CONF, &cfgword)) != 0)
return (err);
if (!(cfgword & ADS111x_CONF_IDLE))
return (EIO);
#endif
for (retries = 5; ; --retries) {
if (retries == 0)
return (EWOULDBLOCK);
if ((err = ads111x_read_2(sc, ADS111x_CONF, &cfgword)) != 0)
return (err);
if (cfgword & ADS111x_CONF_IDLE)
break;
pause_sbt("ads111x", nstosbt(waitns / 20), 0, C_PREL(2));
}
/* Retrieve the sample and convert it to microvolts. */
if ((err = ads111x_read_2(sc, ADS111x_CONV, &convword)) != 0)