[PATCH 2/2] iio: adc: ti-ads1015: add 10% to conversion wait time

[PATCH 2/2] iio: adc: ti-ads1015: add 10% to conversion wait time

[Ladislav Michl]

As user's guide "ADS1015EVM, ADS1115EVM, ADS1015EVM-PDK, ADS1115EVM-PDK
User Guide (Rev. B)" (http://www.ti.com/lit/ug/sbau157b/sbau157b.pdf)
states at page 16:
"Note that both the ADS1115 and ADS1015 have internal clocks with a ±10%
accuracy. If performing FFT tests, frequencies may appear to be incorrect
as a result of this tolerance range.", add those 10% to converion wait
time.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 drivers/iio/adc/ti-ads1015.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 034e48566ab5..de8f44127fe2 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -268,6 +268,7 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 		dr_old = (old & ADS1015_CFG_DR_MASK) >> ADS1015_CFG_DR_SHIFT;
 		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr_old]);
 		conv_time += DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
+		conv_time += conv_time / 10;
 		usleep_range(conv_time, conv_time + 1);
 		data->conv_invalid = false;
 	}
-- 
2.11.0

Re: [PATCH 2/2] iio: adc: ti-ads1015: add 10% to conversion wait time

[Akinobu Mita]

2017-08-24 17:27 GMT+09:00 Ladislav Michl <ladis@linux-mips.org>:
> As user's guide "ADS1015EVM, ADS1115EVM, ADS1015EVM-PDK, ADS1115EVM-PDK
> User Guide (Rev. B)" (http://www.ti.com/lit/ug/sbau157b/sbau157b.pdf)
> states at page 16:
> "Note that both the ADS1115 and ADS1015 have internal clocks with a =C2=
=B110%
> accuracy. If performing FFT tests, frequencies may appear to be incorrect
> as a result of this tolerance range.", add those 10% to converion wait
> time.
>
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  drivers/iio/adc/ti-ads1015.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 034e48566ab5..de8f44127fe2 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -268,6 +268,7 @@ int ads1015_get_adc_result(struct ads1015_data *data,=
 int chan, int *val)
>                 dr_old =3D (old & ADS1015_CFG_DR_MASK) >> ADS1015_CFG_DR_=
SHIFT;
>                 conv_time =3D DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[=
dr_old]);
>                 conv_time +=3D DIV_ROUND_UP(USEC_PER_SEC, data->data_rate=
[dr]);
> +               conv_time +=3D conv_time / 10;

A comment explaining why we need this line helps someone who read it
as you described in the commit log.

>                 usleep_range(conv_time, conv_time + 1);
>                 data->conv_invalid =3D false;
>         }
> --
> 2.11.0
>

[PATCH v2] iio: adc: ti-ads1015: add 10% to conversion wait time

[Ladislav Michl]

As user's guide "ADS1015EVM, ADS1115EVM, ADS1015EVM-PDK, ADS1115EVM-PDK
User Guide (Rev. B)" (http://www.ti.com/lit/ug/sbau157b/sbau157b.pdf)
states at page 16:
"Note that both the ADS1115 and ADS1015 have internal clocks with a ±10%
accuracy. If performing FFT tests, frequencies may appear to be incorrect
as a result of this tolerance range.", add those 10% to converion wait
time.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: Add comment explaining why those 10% are needed

 drivers/iio/adc/ti-ads1015.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 034e48566ab5..ae29dd87c597 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -268,6 +268,7 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 		dr_old = (old & ADS1015_CFG_DR_MASK) >> ADS1015_CFG_DR_SHIFT;
 		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr_old]);
 		conv_time += DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
+		conv_time += conv_time / 10; /* 10% internal clock inaccuracy */
 		usleep_range(conv_time, conv_time + 1);
 		data->conv_invalid = false;
 	}
-- 
2.11.0

Re: [PATCH v2] iio: adc: ti-ads1015: add 10% to conversion wait time

[Akinobu Mita]

2017-08-25 14:39 GMT+09:00 Ladislav Michl <ladis@linux-mips.org>:
> As user's guide "ADS1015EVM, ADS1115EVM, ADS1015EVM-PDK, ADS1115EVM-PDK
> User Guide (Rev. B)" (http://www.ti.com/lit/ug/sbau157b/sbau157b.pdf)
> states at page 16:
> "Note that both the ADS1115 and ADS1015 have internal clocks with a =C2=
=B110%
> accuracy. If performing FFT tests, frequencies may appear to be incorrect
> as a result of this tolerance range.", add those 10% to converion wait
> time.
>
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  Changes:
>  -v2: Add comment explaining why those 10% are needed
>
>  drivers/iio/adc/ti-ads1015.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 034e48566ab5..ae29dd87c597 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -268,6 +268,7 @@ int ads1015_get_adc_result(struct ads1015_data *data,=
 int chan, int *val)
>                 dr_old =3D (old & ADS1015_CFG_DR_MASK) >> ADS1015_CFG_DR_=
SHIFT;
>                 conv_time =3D DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[=
dr_old]);
>                 conv_time +=3D DIV_ROUND_UP(USEC_PER_SEC, data->data_rate=
[dr]);
> +               conv_time +=3D conv_time / 10; /* 10% internal clock inac=
curacy */

Perhaps we'd better use DIV_ROUND_UP to avoid the worst case.

Anyway, this and patch 2/2 look good to me.

Reviewed-by: Akinobu Mita <akinobu.mita@gmail.com>

>                 usleep_range(conv_time, conv_time + 1);
>                 data->conv_invalid =3D false;
>         }
> --
> 2.11.0
>

Re: [PATCH v2] iio: adc: ti-ads1015: add 10% to conversion wait time

[Jonathan Cameron]

On Mon, 28 Aug 2017 00:01:22 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:

> 2017-08-25 14:39 GMT+09:00 Ladislav Michl <ladis@linux-mips.org>:
> > As user's guide "ADS1015EVM, ADS1115EVM, ADS1015EVM-PDK, ADS1115EVM-PDK
> > User Guide (Rev. B)" (http://www.ti.com/lit/ug/sbau157b/sbau157b.pdf)
> > states at page 16:
> > "Note that both the ADS1115 and ADS1015 have internal clocks with a ±10%
> > accuracy. If performing FFT tests, frequencies may appear to be incorrect
> > as a result of this tolerance range.", add those 10% to converion wait
> > time.
> >
> > Cc: Daniel Baluta <daniel.baluta@gmail.com>
> > Cc: Jonathan Cameron <jic23@kernel.org>
> > Cc: Akinobu Mita <akinobu.mita@gmail.com>
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > ---
> >  Changes:
> >  -v2: Add comment explaining why those 10% are needed
> >
> >  drivers/iio/adc/ti-ads1015.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> > index 034e48566ab5..ae29dd87c597 100644
> > --- a/drivers/iio/adc/ti-ads1015.c
> > +++ b/drivers/iio/adc/ti-ads1015.c
> > @@ -268,6 +268,7 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
> >                 dr_old = (old & ADS1015_CFG_DR_MASK) >> ADS1015_CFG_DR_SHIFT;
> >                 conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr_old]);
> >                 conv_time += DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
> > +               conv_time += conv_time / 10; /* 10% internal clock inaccuracy */  
> 
> Perhaps we'd better use DIV_ROUND_UP to avoid the worst case.
> 
> Anyway, this and patch 2/2 look good to me.

I'm guessing you mean 1/2 as this is the updated version of 2/2.

Which brings me to...
Ladislav,

Please don't do this.  If you update one patch in a series, a new
posting of the entire series - not as a reply to the previous version but
as a new thread, is the what you should be doing.

The whole replying to the thread etc might seem to make more sense but
it doesn't scale to complex patch series, so we would prefer everything
to be done in a fashion that does, however simple the series is.

Just makes things easier for maintainers and we can get awfully
grumpy about things that make our lives harder!

On another note, I've applied this though the togreg branch which
means it will take quite sometime to reach a mainline release.

If you wanted this fix in place quicker it should have been as
a separate patch rather than tied up with what was a performance
optimization (patch 1/2).  Anyhow, done now.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> 
> Reviewed-by: Akinobu Mita <akinobu.mita@gmail.com>
> 
> >                 usleep_range(conv_time, conv_time + 1);
> >                 data->conv_invalid = false;
> >         }
> > --
> > 2.11.0
> >  
> --
> 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