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/647053/2378282
Signed-off-by: Stefan Brüns <[email protected]>
---
drivers/iio/adc/ina2xx-adc.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index 8c8120406f52..b027d485398b 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -705,7 +705,6 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev)
int bit, ret, i = 0;
s64 time_a, time_b;
unsigned int alert;
- int cnvr_need_clear = 0;
time_a = iio_get_time_ns(indio_dev);
@@ -730,7 +729,6 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev)
ret = regmap_read(chip->regmap,
INA2XX_BUS_VOLTAGE, &alert);
alert &= INA219_CNVR;
- cnvr_need_clear = alert;
}
if (ret < 0)
@@ -752,18 +750,6 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev)
return ret;
data[i++] = val;
-
- if (INA2XX_SHUNT_VOLTAGE + bit == INA2XX_POWER)
- cnvr_need_clear = 0;
- }
-
- /* Dummy read on INA219 power register to clear CNVR flag */
- if (cnvr_need_clear && chip->config->chip_id == ina219) {
- unsigned int val;
-
- ret = regmap_read(chip->regmap, INA2XX_POWER, &val);
- if (ret < 0)
- return ret;
}
time_b = iio_get_time_ns(indio_dev);
--
2.15.1
On Fri, 8 Dec 2017 18:41:48 +0100
Stefan Brüns <[email protected]> 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/647053/2378282
>
> Signed-off-by: Stefan Brüns <[email protected]>
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.
Jonathan
> ---
>
> drivers/iio/adc/ina2xx-adc.c | 14 --------------
> 1 file changed, 14 deletions(-)
>
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 8c8120406f52..b027d485398b 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -705,7 +705,6 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev)
> int bit, ret, i = 0;
> s64 time_a, time_b;
> unsigned int alert;
> - int cnvr_need_clear = 0;
>
> time_a = iio_get_time_ns(indio_dev);
>
> @@ -730,7 +729,6 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev)
> ret = regmap_read(chip->regmap,
> INA2XX_BUS_VOLTAGE, &alert);
> alert &= INA219_CNVR;
> - cnvr_need_clear = alert;
> }
>
> if (ret < 0)
> @@ -752,18 +750,6 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev)
> return ret;
>
> data[i++] = val;
> -
> - if (INA2XX_SHUNT_VOLTAGE + bit == INA2XX_POWER)
> - cnvr_need_clear = 0;
> - }
> -
> - /* Dummy read on INA219 power register to clear CNVR flag */
> - if (cnvr_need_clear && chip->config->chip_id == ina219) {
> - unsigned int val;
> -
> - ret = regmap_read(chip->regmap, INA2XX_POWER, &val);
> - if (ret < 0)
> - return ret;
> }
>
> time_b = iio_get_time_ns(indio_dev);
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 <[email protected]> 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/647053
> > /2378282
> >
> > Signed-off-by: Stefan Br?ns <[email protected]>
>
> 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?
This only removes a redundant read.
All channel combinations (w/ and w/o timestamp) work, but combinations not
including the power register use less bus time now.
Kind regards,
Stefan
--
Stefan Br?ns / Bergstra?e 21 / 52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019
On Sun, 10 Dec 2017 21:53:42 +0100
Stefan Brüns <[email protected]> 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 <[email protected]> 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/647053
> > > /2378282
> > >
> > > Signed-off-by: Stefan Brüns <[email protected]>
> >
> > 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.
>
> This only removes a redundant read.
The question is whether it is redundant if we have no non timestamp
registers enabled.
I'll be honest, whilst I can't immediately spot any protection against
this in the core (and it definitely used to be possible), I'm not totally
sure it now is and don't have a system to hand to test against.
We had some debate a long time back on whether it made sense to have
only timestamps and I think we concluded it did as you might in theory
only care about the timing and not the data in some obscure cases.
Jonathan
>
> All channel combinations (w/ and w/o timestamp) work, but combinations not
> including the power register use less bus time now.
>
> Kind regards,
>
> Stefan
>
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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > >
> > > 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
On Wed, 13 Dec 2017 00:48:40 +0100
Stefan Brüns <[email protected]> wrote:
> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > >
> > > > 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.
Ah, fine then. I thought we were talking about having to read a channel
not just the register we are anyway polling for status.
Applied to the togreg branch of iio.git.
Thanks,
Jonathan
>
> Regards,
>
> Stefan
>
>