Re: [PATCH 12/15] iio: imu: inv_mpu6050: don't disable the IRQ under lock

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 1 Nov 2026 02:14:22 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 27 Oct 2026 17:19:16 +0100
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > From: Josef Bacik <josef@toxicpanda.com>
> > 
> > If the chip is a wake-up source, inv_mpu_suspend() calls disable_irq() with
> > st->lock held.  disable_irq() waits for the threaded handler, and
> > inv_mpu6050_interrupt_handle() takes st->lock to read the interrupt
> > status.  If the chip raises an interrupt while the system is being
> > suspended, the handler waits for the lock, the suspend callback waits for
> > the handler, and the suspend hangs.
> > 
> > Disable the interrupt once the lock has been dropped.
> > 
> > Fixes: 305914d01807 ("iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode")
> > Cc: stable <stable@kernel.org>
> > Cc: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>  
> 
> This seems correct to me, but waiting on Jean-Baptiste for
> view from someone more familiar with the driver (and maybe
> able to give it a sanity check test!)

Ah well.  I guess Jean-Baptiste missed this or is snowed under.
Either way I'm confident enough that you are right (and the very
low chance of regression given the change) so applied to the fixes-togreg
branch of iio.git.

Thanks,

Jonathan

> 
> > Cc: David Lechner <dlechner@baylibre.com>
> > Cc: "Nuno Sá" <nuno.sa@analog.com>
> > Cc: Andy Shevchenko <andy@kernel.org>
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > index 38c3f3b398d4..0ae9d497ccfd 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > @@ -2166,7 +2166,7 @@ static int inv_mpu_resume(struct device *dev)
> >  	return result;
> >  }
> >  
> > -static int inv_mpu_suspend(struct device *dev)
> > +static int __inv_mpu_suspend(struct device *dev, bool *disable)
> >  {
> >  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> >  	struct inv_mpu6050_state *st = iio_priv(indio_dev);
> > @@ -2212,7 +2212,7 @@ static int inv_mpu_suspend(struct device *dev)
> >  		if (result)
> >  			return result;
> >  		enable_irq_wake(st->irq);
> > -		disable_irq(st->irq);
> > +		*disable = true;
> >  	} else {
> >  		result = inv_mpu6050_set_power_itg(st, false);
> >  		if (result)
> > @@ -2223,6 +2223,21 @@ static int inv_mpu_suspend(struct device *dev)
> >  	return 0;
> >  }
> >  
> > +static int inv_mpu_suspend(struct device *dev)
> > +{
> > +	struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> > +	bool disable = false;
> > +	int result;
> > +
> > +	result = __inv_mpu_suspend(dev, &disable);
> > +
> > +	/* Not under st->lock: this waits for the handler, which takes it */
> > +	if (disable)
> > +		disable_irq(st->irq);
> > +
> > +	return result;
> > +}
> > +
> >  static int inv_mpu_runtime_suspend(struct device *dev)
> >  {
> >  	struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> >   
> 
>