1. Patchew
  2. linux
  3. View series

[PATCH] iio: gyro: bmg160: wait full startup time after mode change at probe

Stepan Ionichev posted 1 patch 1 month ago
There is a newer version of this series
drivers/iio/gyro/bmg160_core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH] iio: gyro: bmg160: wait full startup time after mode change at probe
Posted by Stepan Ionichev 1 month ago 
bmg160_chip_init() calls bmg160_set_mode(BMG160_MODE_NORMAL) and
then waits only 500-1000 us. Per the BMG160 datasheet
(BST-BMG160-DS000-07 Rev. 1.0, May 2013), the start-up and wake-up
times (tsu, twusm) are 30 ms.

The same file already waits BMG160_MAX_STARTUP_TIME_MS (80 ms)
in bmg160_runtime_resume() after the same set_mode(NORMAL)
operation. The 500 us value at probe was likely a unit mix-up;
the old comment said "500 ms" while the code used microseconds.

Reuse the same constant via msleep_interruptible() to match the
runtime resume path. Without this, register writes that follow
the mode change can hit the chip before it is ready.

Fixes: 22b46c45fb9b ("iio:gyro:bmg160 Gyro Sensor driver")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
 drivers/iio/gyro/bmg160_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
index 38394b5f3..44e90af5c 100644
--- a/drivers/iio/gyro/bmg160_core.c
+++ b/drivers/iio/gyro/bmg160_core.c
@@ -258,8 +258,7 @@ static int bmg160_chip_init(struct bmg160_data *data)
 	if (ret < 0)
 		return ret;
 
-	/* Wait upto 500 ms to be ready after changing mode */
-	usleep_range(500, 1000);
+	msleep_interruptible(BMG160_MAX_STARTUP_TIME_MS);
 
 	/* Set Bandwidth */
 	ret = bmg160_set_bw(data, BMG160_DEF_BW);
-- 
2.43.0
Re: [PATCH] iio: gyro: bmg160: wait full startup time after mode change at probe
Posted by Andy Shevchenko 1 month ago 
On Mon, May 11, 2026 at 11:27:55AM +0500, Stepan Ionichev wrote:
> bmg160_chip_init() calls bmg160_set_mode(BMG160_MODE_NORMAL) and
> then waits only 500-1000 us. Per the BMG160 datasheet
> (BST-BMG160-DS000-07 Rev. 1.0, May 2013), the start-up and wake-up
> times (tsu, twusm) are 30 ms.
> 
> The same file already waits BMG160_MAX_STARTUP_TIME_MS (80 ms)
> in bmg160_runtime_resume() after the same set_mode(NORMAL)
> operation. The 500 us value at probe was likely a unit mix-up;
> the old comment said "500 ms" while the code used microseconds.
> 
> Reuse the same constant via msleep_interruptible() to match the
> runtime resume path. Without this, register writes that follow
> the mode change can hit the chip before it is ready.

...

> -	/* Wait upto 500 ms to be ready after changing mode */

First of all, the same comment, put better description on "why?"
based on the datasheet explanation.

> -	usleep_range(500, 1000);
> +	msleep_interruptible(BMG160_MAX_STARTUP_TIME_MS);

Hmm... This returns an error in case it's aborted by a signal or other means.
What are you supposed to do with that?

-- 
With Best Regards,
Andy Shevchenko
[PATCH v2] iio: gyro: bmg160: wait full startup time after mode change at probe
Posted by Stepan Ionichev 1 month ago 
bmg160_chip_init() calls bmg160_set_mode(BMG160_MODE_NORMAL) and
then waits only 500-1000 us. Per the BMG160 datasheet
(BST-BMG160-DS000-07 Rev. 1.0, May 2013), the start-up and wake-up
times (tsu, twusm) are 30 ms.

The same file already waits BMG160_MAX_STARTUP_TIME_MS (80 ms)
in bmg160_runtime_resume() after the same set_mode(NORMAL)
operation. The 500 us value at probe was likely a unit mix-up;
the old comment said "500 ms" while the code used microseconds.

Reuse the same constant via msleep() and add a code comment
explaining the datasheet basis for the wait. Without this,
register writes that follow the mode change can hit the chip
before it is ready.

Fixes: 22b46c45fb9b ("iio:gyro:bmg160 Gyro Sensor driver")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v2:
- Use msleep() instead of msleep_interruptible() so the wait is not
  cut short by signals during probe (per Andy)
- Add a code comment with the datasheet basis for the 80 ms wait
  (per Andy)

 drivers/iio/gyro/bmg160_core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
index 38394b5f3..6d9019451 100644
--- a/drivers/iio/gyro/bmg160_core.c
+++ b/drivers/iio/gyro/bmg160_core.c
@@ -258,8 +258,14 @@ static int bmg160_chip_init(struct bmg160_data *data)
 	if (ret < 0)
 		return ret;
 
-	/* Wait upto 500 ms to be ready after changing mode */
-	usleep_range(500, 1000);
+	/*
+	 * Wait for the chip to be ready after switching to normal mode.
+	 * The BMG160 datasheet (BST-BMG160-DS000-07 Rev. 1.0, May 2013)
+	 * specifies a start-up / wake-up time (tsu, twusm) of 30 ms; use
+	 * BMG160_MAX_STARTUP_TIME_MS (80 ms) as a safety margin, matching
+	 * what bmg160_runtime_resume() already does.
+	 */
+	msleep(BMG160_MAX_STARTUP_TIME_MS);
 
 	/* Set Bandwidth */
 	ret = bmg160_set_bw(data, BMG160_DEF_BW);
-- 
2.43.0
Re: [PATCH v2] iio: gyro: bmg160: wait full startup time after mode change at probe
Posted by Jonathan Cameron 1 month ago 
On Mon, 11 May 2026 11:40:20 +0500
Stepan Ionichev <sozdayvek@gmail.com> wrote:

> bmg160_chip_init() calls bmg160_set_mode(BMG160_MODE_NORMAL) and
> then waits only 500-1000 us. Per the BMG160 datasheet
> (BST-BMG160-DS000-07 Rev. 1.0, May 2013), the start-up and wake-up
> times (tsu, twusm) are 30 ms.
> 
> The same file already waits BMG160_MAX_STARTUP_TIME_MS (80 ms)
> in bmg160_runtime_resume() after the same set_mode(NORMAL)
> operation. The 500 us value at probe was likely a unit mix-up;
> the old comment said "500 ms" while the code used microseconds.
> 
> Reuse the same constant via msleep() and add a code comment
> explaining the datasheet basis for the wait. Without this,
> register writes that follow the mode change can hit the chip
> before it is ready.
> 
> Fixes: 22b46c45fb9b ("iio:gyro:bmg160 Gyro Sensor driver")
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
Some process stuff.

Never send a new version in reply to the older one.  Always
a fresh thread - the reason is mainly that the threads become unreadable
if you got to more than one or two versions.

Also, don't send a new version for a reasonable period of time.
Something small like this maybe a few days, a bigger patch 1 week.

That lets multiple reviewers have time to take a look.
If you have a lot on list already then slow down in general and
spend some time helping to review patches coming from others.
The biggest bottleneck in IIO is reviewer time.

Anyhow, I'm going ignore this for a little while at least...

Jonathan

> ---
> v2:
> - Use msleep() instead of msleep_interruptible() so the wait is not
>   cut short by signals during probe (per Andy)
> - Add a code comment with the datasheet basis for the 80 ms wait
>   (per Andy)
> 
>  drivers/iio/gyro/bmg160_core.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
> index 38394b5f3..6d9019451 100644
> --- a/drivers/iio/gyro/bmg160_core.c
> +++ b/drivers/iio/gyro/bmg160_core.c
> @@ -258,8 +258,14 @@ static int bmg160_chip_init(struct bmg160_data *data)
>  	if (ret < 0)
>  		return ret;
>  
> -	/* Wait upto 500 ms to be ready after changing mode */
> -	usleep_range(500, 1000);
> +	/*
> +	 * Wait for the chip to be ready after switching to normal mode.
> +	 * The BMG160 datasheet (BST-BMG160-DS000-07 Rev. 1.0, May 2013)
> +	 * specifies a start-up / wake-up time (tsu, twusm) of 30 ms; use
> +	 * BMG160_MAX_STARTUP_TIME_MS (80 ms) as a safety margin, matching
> +	 * what bmg160_runtime_resume() already does.
> +	 */
> +	msleep(BMG160_MAX_STARTUP_TIME_MS);
>  
>  	/* Set Bandwidth */
>  	ret = bmg160_set_bw(data, BMG160_DEF_BW);
Re: [PATCH v2] iio: gyro: bmg160: wait full startup time after mode change at probe
Posted by Jonathan Cameron 3 weeks, 6 days ago 
On Mon, 11 May 2026 15:47:04 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 11 May 2026 11:40:20 +0500
> Stepan Ionichev <sozdayvek@gmail.com> wrote:
> 
> > bmg160_chip_init() calls bmg160_set_mode(BMG160_MODE_NORMAL) and
> > then waits only 500-1000 us. Per the BMG160 datasheet
> > (BST-BMG160-DS000-07 Rev. 1.0, May 2013), the start-up and wake-up
> > times (tsu, twusm) are 30 ms.
> > 
> > The same file already waits BMG160_MAX_STARTUP_TIME_MS (80 ms)
> > in bmg160_runtime_resume() after the same set_mode(NORMAL)
> > operation. The 500 us value at probe was likely a unit mix-up;
> > the old comment said "500 ms" while the code used microseconds.
> > 
> > Reuse the same constant via msleep() and add a code comment
> > explaining the datasheet basis for the wait. Without this,
> > register writes that follow the mode change can hit the chip
> > before it is ready.
> > 
> > Fixes: 22b46c45fb9b ("iio:gyro:bmg160 Gyro Sensor driver")
> > Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>  
> Some process stuff.
> 
> Never send a new version in reply to the older one.  Always
> a fresh thread - the reason is mainly that the threads become unreadable
> if you got to more than one or two versions.
> 
> Also, don't send a new version for a reasonable period of time.
> Something small like this maybe a few days, a bigger patch 1 week.
> 
> That lets multiple reviewers have time to take a look.
> If you have a lot on list already then slow down in general and
> spend some time helping to review patches coming from others.
> The biggest bottleneck in IIO is reviewer time.
> 
> Anyhow, I'm going ignore this for a little while at least...

Applied to the fixes-togreg branch of iio.git and marked for stable.

As fixes go it's pretty safe given it just extends a sleep, so
I'll grab it now rather than waiting longer.

Jonathan

> 
> Jonathan
> 
> > ---
> > v2:
> > - Use msleep() instead of msleep_interruptible() so the wait is not
> >   cut short by signals during probe (per Andy)
> > - Add a code comment with the datasheet basis for the 80 ms wait
> >   (per Andy)
> > 
> >  drivers/iio/gyro/bmg160_core.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
> > index 38394b5f3..6d9019451 100644
> > --- a/drivers/iio/gyro/bmg160_core.c
> > +++ b/drivers/iio/gyro/bmg160_core.c
> > @@ -258,8 +258,14 @@ static int bmg160_chip_init(struct bmg160_data *data)
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	/* Wait upto 500 ms to be ready after changing mode */
> > -	usleep_range(500, 1000);
> > +	/*
> > +	 * Wait for the chip to be ready after switching to normal mode.
> > +	 * The BMG160 datasheet (BST-BMG160-DS000-07 Rev. 1.0, May 2013)
> > +	 * specifies a start-up / wake-up time (tsu, twusm) of 30 ms; use
> > +	 * BMG160_MAX_STARTUP_TIME_MS (80 ms) as a safety margin, matching
> > +	 * what bmg160_runtime_resume() already does.
> > +	 */
> > +	msleep(BMG160_MAX_STARTUP_TIME_MS);
> >  
> >  	/* Set Bandwidth */
> >  	ret = bmg160_set_bw(data, BMG160_DEF_BW);  
> 
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.