2018-11-19 06:14:06

by Jian-Hong Pan

[permalink] [raw]
Subject: [PATCH] iio: st_sensors: Fix the sleep time for sampling

According to the description of st_sensor_settings and st_sensor_data
structures' comments:
- bootime: samples to discard when sensor passing from power-down to
power-up.
- odr: Output data rate of the sensor [Hz].

The sleep time should be
sdata->sensor_settings->bootime + 1000 / sdata->odr ms.

Signed-off-by: Jian-Hong Pan <[email protected]>
---
drivers/iio/common/st_sensors/st_sensors_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 26fbd1bd9413..6b87ea657a92 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -594,7 +594,7 @@ int st_sensors_read_info_raw(struct iio_dev *indio_dev,
if (err < 0)
goto out;

- msleep((sdata->sensor_settings->bootime * 1000) / sdata->odr);
+ msleep(sdata->sensor_settings->bootime + 1000 / sdata->odr);
err = st_sensors_read_axis_data(indio_dev, ch, val);
if (err < 0)
goto out;
--
2.11.0



2018-11-19 19:06:51

by Denis CIOCCA

[permalink] [raw]
Subject: RE: [PATCH] iio: st_sensors: Fix the sleep time for sampling

Hi Jian,

Not clear to me why should be + instead of *.

ODR is expressed in Hz, so (1/Hz) = period in seconds (1 sample sampling time) [s]
1000 * (1/Hz) = period in milliseconds (1 sample sampling time) [ms]
n * 1000 * (1/Hz) = n times period in milliseconds (n times sample sampling time) [ms]

In your case you assume bootime is in milliseconds. Maybe we can change the comment and use 'number of samples ...'.

Br,
Denis



-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Jian-Hong Pan
Sent: Sunday, November 18, 2018 10:12 PM
To: Jonathan Cameron <[email protected]>; Hartmut Knaack <[email protected]>; Lars-Peter Clausen <[email protected]>; Peter Meerwald-Stadler <[email protected]>; Dominique Martinet <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Jian-Hong Pan <[email protected]>
Subject: [PATCH] iio: st_sensors: Fix the sleep time for sampling

According to the description of st_sensor_settings and st_sensor_data structures' comments:
- bootime: samples to discard when sensor passing from power-down to power-up.
- odr: Output data rate of the sensor [Hz].

The sleep time should be
sdata->sensor_settings->bootime + 1000 / sdata->odr ms.

Signed-off-by: Jian-Hong Pan <[email protected]>
---
drivers/iio/common/st_sensors/st_sensors_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 26fbd1bd9413..6b87ea657a92 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -594,7 +594,7 @@ int st_sensors_read_info_raw(struct iio_dev *indio_dev,
if (err < 0)
goto out;

- msleep((sdata->sensor_settings->bootime * 1000) / sdata->odr);
+ msleep(sdata->sensor_settings->bootime + 1000 / sdata->odr);
err = st_sensors_read_axis_data(indio_dev, ch, val);
if (err < 0)
goto out;
--
2.11.0


2018-11-21 05:42:34

by Jian-Hong Pan

[permalink] [raw]
Subject: Re: [PATCH] iio: st_sensors: Fix the sleep time for sampling

Denis CIOCCA <[email protected]> 於 2018年11月20日 週二 上午3:05寫道:
>
> Hi Jian,
>
> Not clear to me why should be + instead of *.
>
> ODR is expressed in Hz, so (1/Hz) = period in seconds (1 sample sampling time) [s]
> 1000 * (1/Hz) = period in milliseconds (1 sample sampling time) [ms]
> n * 1000 * (1/Hz) = n times period in milliseconds (n times sample sampling time) [ms]
>
> In your case you assume bootime is in milliseconds.

Yes, I assume that according to the original comment.

>Maybe we can change the comment and use 'number of samples ...'.

Making the meaning more clear is better.

However, does the bootime of the measurement need as the long time to
be enabled?
If the sampling rate is 1Hz and n is 2, then they will do msleep with
2000 ms for each st_sensors_read_info_raw.

Regards,
Jian-Hong Pan

>
>
> -----Original Message-----
> From: [email protected] <[email protected]> On Behalf Of Jian-Hong Pan
> Sent: Sunday, November 18, 2018 10:12 PM
> To: Jonathan Cameron <[email protected]>; Hartmut Knaack <[email protected]>; Lars-Peter Clausen <[email protected]>; Peter Meerwald-Stadler <[email protected]>; Dominique Martinet <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; Jian-Hong Pan <[email protected]>
> Subject: [PATCH] iio: st_sensors: Fix the sleep time for sampling
>
> According to the description of st_sensor_settings and st_sensor_data structures' comments:
> - bootime: samples to discard when sensor passing from power-down to power-up.
> - odr: Output data rate of the sensor [Hz].
>
> The sleep time should be
> sdata->sensor_settings->bootime + 1000 / sdata->odr ms.
>
> Signed-off-by: Jian-Hong Pan <[email protected]>
> ---
> drivers/iio/common/st_sensors/st_sensors_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 26fbd1bd9413..6b87ea657a92 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -594,7 +594,7 @@ int st_sensors_read_info_raw(struct iio_dev *indio_dev,
> if (err < 0)
> goto out;
>
> - msleep((sdata->sensor_settings->bootime * 1000) / sdata->odr);
> + msleep(sdata->sensor_settings->bootime + 1000 / sdata->odr);
> err = st_sensors_read_axis_data(indio_dev, ch, val);
> if (err < 0)
> goto out;
> --
> 2.11.0
>

2018-11-25 13:23:53

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] iio: st_sensors: Fix the sleep time for sampling

On Wed, 21 Nov 2018 13:13:40 +0800
Jian-Hong Pan <[email protected]> wrote:

> Denis CIOCCA <[email protected]> 於 2018年11月20日 週二 上午3:05寫道:
> >
> > Hi Jian,
> >
> > Not clear to me why should be + instead of *.
> >
> > ODR is expressed in Hz, so (1/Hz) = period in seconds (1 sample sampling time) [s]
> > 1000 * (1/Hz) = period in milliseconds (1 sample sampling time) [ms]
> > n * 1000 * (1/Hz) = n times period in milliseconds (n times sample sampling time) [ms]
> >
> > In your case you assume bootime is in milliseconds.
>
> Yes, I assume that according to the original comment.
>
> >Maybe we can change the comment and use 'number of samples ...'.
>
> Making the meaning more clear is better.
>
> However, does the bootime of the measurement need as the long time to
> be enabled?
> If the sampling rate is 1Hz and n is 2, then they will do msleep with
> 2000 ms for each st_sensors_read_info_raw.

Superficially that seems correct as we need to be sure that a reading
has occurred. If you want it to be quicker than the ODR should be set
faster so that the reading shows up reasonably quickly. At 1Hz and
you want to drop 2 samples, it will indeed take 2 seconds.

I'm not understanding why this is unexpected?

Jonathan

>
> Regards,
> Jian-Hong Pan
>
> >
> >
> > -----Original Message-----
> > From: [email protected] <[email protected]> On Behalf Of Jian-Hong Pan
> > Sent: Sunday, November 18, 2018 10:12 PM
> > To: Jonathan Cameron <[email protected]>; Hartmut Knaack <[email protected]>; Lars-Peter Clausen <[email protected]>; Peter Meerwald-Stadler <[email protected]>; Dominique Martinet <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected]; Jian-Hong Pan <[email protected]>
> > Subject: [PATCH] iio: st_sensors: Fix the sleep time for sampling
> >
> > According to the description of st_sensor_settings and st_sensor_data structures' comments:
> > - bootime: samples to discard when sensor passing from power-down to power-up.
> > - odr: Output data rate of the sensor [Hz].
> >
> > The sleep time should be
> > sdata->sensor_settings->bootime + 1000 / sdata->odr ms.
> >
> > Signed-off-by: Jian-Hong Pan <[email protected]>
> > ---
> > drivers/iio/common/st_sensors/st_sensors_core.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> > index 26fbd1bd9413..6b87ea657a92 100644
> > --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> > +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> > @@ -594,7 +594,7 @@ int st_sensors_read_info_raw(struct iio_dev *indio_dev,
> > if (err < 0)
> > goto out;
> >
> > - msleep((sdata->sensor_settings->bootime * 1000) / sdata->odr);
> > + msleep(sdata->sensor_settings->bootime + 1000 / sdata->odr);
> > err = st_sensors_read_axis_data(indio_dev, ch, val);
> > if (err < 0)
> > goto out;
> > --
> > 2.11.0
> >


2018-11-27 10:31:38

by Jian-Hong Pan

[permalink] [raw]
Subject: Re: [PATCH] iio: st_sensors: Fix the sleep time for sampling

Jonathan Cameron <[email protected]> 於 2018年11月25日 週日 下午9:23寫道:
>
> On Wed, 21 Nov 2018 13:13:40 +0800
> Jian-Hong Pan <[email protected]> wrote:
>
> > Denis CIOCCA <[email protected]> 於 2018年11月20日 週二 上午3:05寫道:
> > >
> > > Hi Jian,
> > >
> > > Not clear to me why should be + instead of *.
> > >
> > > ODR is expressed in Hz, so (1/Hz) = period in seconds (1 sample sampling time) [s]
> > > 1000 * (1/Hz) = period in milliseconds (1 sample sampling time) [ms]
> > > n * 1000 * (1/Hz) = n times period in milliseconds (n times sample sampling time) [ms]
> > >
> > > In your case you assume bootime is in milliseconds.
> >
> > Yes, I assume that according to the original comment.
> >
> > >Maybe we can change the comment and use 'number of samples ...'.
> >
> > Making the meaning more clear is better.
> >
> > However, does the bootime of the measurement need as the long time to
> > be enabled?
> > If the sampling rate is 1Hz and n is 2, then they will do msleep with
> > 2000 ms for each st_sensors_read_info_raw.
>
> Superficially that seems correct as we need to be sure that a reading
> has occurred. If you want it to be quicker than the ODR should be set
> faster so that the reading shows up reasonably quickly. At 1Hz and
> you want to drop 2 samples, it will indeed take 2 seconds.

Now, I understand with the description. Thank you.

Jian-Hong Pan

> > > -----Original Message-----
> > > From: [email protected] <[email protected]> On Behalf Of Jian-Hong Pan
> > > Sent: Sunday, November 18, 2018 10:12 PM
> > > To: Jonathan Cameron <[email protected]>; Hartmut Knaack <[email protected]>; Lars-Peter Clausen <[email protected]>; Peter Meerwald-Stadler <[email protected]>; Dominique Martinet <[email protected]>
> > > Cc: [email protected]; [email protected]; [email protected]; Jian-Hong Pan <[email protected]>
> > > Subject: [PATCH] iio: st_sensors: Fix the sleep time for sampling
> > >
> > > According to the description of st_sensor_settings and st_sensor_data structures' comments:
> > > - bootime: samples to discard when sensor passing from power-down to power-up.
> > > - odr: Output data rate of the sensor [Hz].
> > >
> > > The sleep time should be
> > > sdata->sensor_settings->bootime + 1000 / sdata->odr ms.
> > >
> > > Signed-off-by: Jian-Hong Pan <[email protected]>
> > > ---
> > > drivers/iio/common/st_sensors/st_sensors_core.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> > > index 26fbd1bd9413..6b87ea657a92 100644
> > > --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> > > +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> > > @@ -594,7 +594,7 @@ int st_sensors_read_info_raw(struct iio_dev *indio_dev,
> > > if (err < 0)
> > > goto out;
> > >
> > > - msleep((sdata->sensor_settings->bootime * 1000) / sdata->odr);
> > > + msleep(sdata->sensor_settings->bootime + 1000 / sdata->odr);
> > > err = st_sensors_read_axis_data(indio_dev, ch, val);
> > > if (err < 0)
> > > goto out;
> > > --
> > > 2.11.0
> > >
>