| Message ID | 12b5a27e-ffdc-4a89-bd4f-4c48c3ec9717@rwthex-w2-a.rwth-ad.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 21 Dec 2017 19:31:38 +0100 Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote: > Currently, the registers are read out once per conversion interval. If > the reading is delayed as the conversion has not yet finished, this extra > time is treated as being part of the readout, although it should delay > the start of the poll interval. This results in the interval starting > slightly earlier in each iteration, until all time between reads is > spent polling the status registers instead of sleeping. > > To fix this, the delay has to account for the state of the conversion > ready flag. Whenever the conversion is already finished, schedule the next > read on the regular interval, otherwise schedule it one interval after the > flag bit has been set. > > Split the work function in two functions, one for the status poll and one > for reading the values, to be able to note down the time when the flag > bit is raised. > > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> Great thanks. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > > --- > > Changes in v2: > - Describe old behaviour in commit message more clearly > > drivers/iio/adc/ina2xx-adc.c | 57 +++++++++++++++++++++++++++++--------------- > 1 file changed, 38 insertions(+), 19 deletions(-) > > diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c > index b7407ac91b59..80af0d2322a3 100644 > --- a/drivers/iio/adc/ina2xx-adc.c > +++ b/drivers/iio/adc/ina2xx-adc.c > @@ -697,13 +697,10 @@ static const struct iio_chan_spec ina219_channels[] = { > IIO_CHAN_SOFT_TIMESTAMP(4), > }; > > -static int ina2xx_work_buffer(struct iio_dev *indio_dev) > +static int ina2xx_conversion_ready(struct iio_dev *indio_dev) > { > struct ina2xx_chip_info *chip = iio_priv(indio_dev); > - /* data buffer needs space for channel data and timestap */ > - unsigned short data[4 + sizeof(s64)/sizeof(short)]; > - int bit, ret, i = 0; > - s64 time; > + int ret; > unsigned int alert; > > /* > @@ -717,22 +714,29 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev) > * For now, we do an extra read of the MASK_ENABLE register (INA226) > * resp. the BUS_VOLTAGE register (INA219). > */ > - if (!chip->allow_async_readout) > - do { > - if (chip->config->chip_id == ina226) { > - ret = regmap_read(chip->regmap, > - INA226_MASK_ENABLE, &alert); > - alert &= INA226_CVRF; > - } else { > - ret = regmap_read(chip->regmap, > - INA2XX_BUS_VOLTAGE, &alert); > - alert &= INA219_CNVR; > - } > + if (chip->config->chip_id == ina226) { > + ret = regmap_read(chip->regmap, > + INA226_MASK_ENABLE, &alert); > + alert &= INA226_CVRF; > + } else { > + ret = regmap_read(chip->regmap, > + INA2XX_BUS_VOLTAGE, &alert); > + alert &= INA219_CNVR; > + } > > - if (ret < 0) > - return ret; > + if (ret < 0) > + return ret; > > - } while (!alert); > + return !!alert; > +} > + > +static int ina2xx_work_buffer(struct iio_dev *indio_dev) > +{ > + struct ina2xx_chip_info *chip = iio_priv(indio_dev); > + /* data buffer needs space for channel data and timestap */ > + unsigned short data[4 + sizeof(s64)/sizeof(short)]; > + int bit, ret, i = 0; > + s64 time; > > time = iio_get_time_ns(indio_dev); > > @@ -776,6 +780,21 @@ static int ina2xx_capture_thread(void *data) > ktime_get_ts64(&next); > > do { > + while (!chip->allow_async_readout) { > + ret = ina2xx_conversion_ready(indio_dev); > + if (ret < 0) > + return ret; > + > + /* > + * If the conversion was not yet finished, > + * reset the reference timestamp. > + */ > + if (ret == 0) > + ktime_get_ts64(&next); > + else > + break; > + } > + > ret = ina2xx_work_buffer(indio_dev); > if (ret < 0) > return ret; -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c index b7407ac91b59..80af0d2322a3 100644 --- a/drivers/iio/adc/ina2xx-adc.c +++ b/drivers/iio/adc/ina2xx-adc.c @@ -697,13 +697,10 @@ static const struct iio_chan_spec ina219_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(4), }; -static int ina2xx_work_buffer(struct iio_dev *indio_dev) +static int ina2xx_conversion_ready(struct iio_dev *indio_dev) { struct ina2xx_chip_info *chip = iio_priv(indio_dev); - /* data buffer needs space for channel data and timestap */ - unsigned short data[4 + sizeof(s64)/sizeof(short)]; - int bit, ret, i = 0; - s64 time; + int ret; unsigned int alert; /* @@ -717,22 +714,29 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev) * For now, we do an extra read of the MASK_ENABLE register (INA226) * resp. the BUS_VOLTAGE register (INA219). */ - if (!chip->allow_async_readout) - do { - if (chip->config->chip_id == ina226) { - ret = regmap_read(chip->regmap, - INA226_MASK_ENABLE, &alert); - alert &= INA226_CVRF; - } else { - ret = regmap_read(chip->regmap, - INA2XX_BUS_VOLTAGE, &alert); - alert &= INA219_CNVR; - } + if (chip->config->chip_id == ina226) { + ret = regmap_read(chip->regmap, + INA226_MASK_ENABLE, &alert); + alert &= INA226_CVRF; + } else { + ret = regmap_read(chip->regmap, + INA2XX_BUS_VOLTAGE, &alert); + alert &= INA219_CNVR; + } - if (ret < 0) - return ret; + if (ret < 0) + return ret; - } while (!alert); + return !!alert; +} + +static int ina2xx_work_buffer(struct iio_dev *indio_dev) +{ + struct ina2xx_chip_info *chip = iio_priv(indio_dev); + /* data buffer needs space for channel data and timestap */ + unsigned short data[4 + sizeof(s64)/sizeof(short)]; + int bit, ret, i = 0; + s64 time; time = iio_get_time_ns(indio_dev); @@ -776,6 +780,21 @@ static int ina2xx_capture_thread(void *data) ktime_get_ts64(&next); do { + while (!chip->allow_async_readout) { + ret = ina2xx_conversion_ready(indio_dev); + if (ret < 0) + return ret; + + /* + * If the conversion was not yet finished, + * reset the reference timestamp. + */ + if (ret == 0) + ktime_get_ts64(&next); + else + break; + } + ret = ina2xx_work_buffer(indio_dev); if (ret < 0) return ret;
Currently, the registers are read out once per conversion interval. If the reading is delayed as the conversion has not yet finished, this extra time is treated as being part of the readout, although it should delay the start of the poll interval. This results in the interval starting slightly earlier in each iteration, until all time between reads is spent polling the status registers instead of sleeping. To fix this, the delay has to account for the state of the conversion ready flag. Whenever the conversion is already finished, schedule the next read on the regular interval, otherwise schedule it one interval after the flag bit has been set. Split the work function in two functions, one for the status poll and one for reading the values, to be able to note down the time when the flag bit is raised. Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> --- Changes in v2: - Describe old behaviour in commit message more clearly drivers/iio/adc/ina2xx-adc.c | 57 +++++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 19 deletions(-)