[PATCH v3 0/4] iio: gyro: mpu3050: Multiple bug fixes

[PATCH v3 0/4] iio: gyro: mpu3050: Multiple bug fixes

[Ethan Tidmore]

This series fixes 4 bugs:
- Patch 1: Fix free_irq() being called with wrong handler.
- Patch 2: Added missing free_irq() in an error path.
- Patch 3: Moved iio_device_register() to end of probe function, in
  doing so had to implement correct error cleanups and place
  iio_device_unregister in beginning of remove callback.
- Patch 4: free_irq() is after called iio_triggered_buffer_cleanup()
  which breaks LIFO, place it before it.

I plan on doing a proper devm_ conversion of this driver, but went ahead
and did these fixes so they could be backported.

v3:
- Patch 1 & 2 remove unneeded code snippets.
- Patch 3 remove stray change and tidy up grammar.
- Patch 4 clarify patch purpose.

Ethan Tidmore (4):
  iio: gyro: mpu3050: Fix incorrect free_irq() variable
  iio: gyro: mpu3050: Fix irq resource leak
  iio: gyro: mpu3050: Move iio_device_register() to correct location
  iio: gyro: mpu3050: Fix out-of-sequence free_irq()

 drivers/iio/gyro/mpu3050-core.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

-- 
2.53.0

[PATCH v3 1/4] iio: gyro: mpu3050: Fix incorrect free_irq() variable

[Ethan Tidmore]

The handler for the IRQ part of this driver is mpu3050->trig but,
in the teardown free_irq() is called with handler mpu3050.

Use correct IRQ handler when calling free_irq().

Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v3: 
- Remove unneeded code snippet.
v2:
- Patch added to series.

 drivers/iio/gyro/mpu3050-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index ee2fcd20545d..06162d886b59 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1261,7 +1261,7 @@ void mpu3050_common_remove(struct device *dev)
 	pm_runtime_disable(dev);
 	iio_triggered_buffer_cleanup(indio_dev);
 	if (mpu3050->irq)
-		free_irq(mpu3050->irq, mpu3050);
+		free_irq(mpu3050->irq, mpu3050->trig);
 	iio_device_unregister(indio_dev);
 	mpu3050_power_down(mpu3050);
 }
-- 
2.53.0

[PATCH v3 2/4] iio: gyro: mpu3050: Fix irq resource leak

[Ethan Tidmore]

The interrupt handler is setup but only a few lines down if
iio_trigger_register() fails the function returns without properly
releasing the handler.

Add cleanup goto to resolve resource leak.

Detected by Smatch:
drivers/iio/gyro/mpu3050-core.c:1128 mpu3050_trigger_probe() warn:
'irq' from request_threaded_irq() not released on lines: 1124.

Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v3: 
- Remove unneeded code snippet.
v2:
- Use manual cleanup instead of devm_.

 drivers/iio/gyro/mpu3050-core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index 06162d886b59..b6e05afbe512 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1121,11 +1121,16 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
 
 	ret = iio_trigger_register(mpu3050->trig);
 	if (ret)
-		return ret;
+		goto err_iio_trigger;
 
 	indio_dev->trig = iio_trigger_get(mpu3050->trig);
 
 	return 0;
+
+err_iio_trigger:
+	free_irq(mpu3050->irq, mpu3050->trig);
+
+	return ret;
 }
 
 int mpu3050_common_probe(struct device *dev,
-- 
2.53.0

[PATCH v3 3/4] iio: gyro: mpu3050: Move iio_device_register() to correct location

[Ethan Tidmore]

iio_device_register() should be at the end of the probe function to
prevent race conditions.

Place iio_device_register() at the end of the probe function and place
iio_device_unregister() accordingly.

Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v3: 
- Remove stray change.
- Fix grammar with "the".
v2:
- Patch added to series.

 drivers/iio/gyro/mpu3050-core.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index b6e05afbe512..b590cf6709b4 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1218,12 +1218,6 @@ int mpu3050_common_probe(struct device *dev,
 		goto err_power_down;
 	}
 
-	ret = iio_device_register(indio_dev);
-	if (ret) {
-		dev_err(dev, "device register failed\n");
-		goto err_cleanup_buffer;
-	}
-
 	dev_set_drvdata(dev, indio_dev);
 
 	/* Check if we have an assigned IRQ to use as trigger */
@@ -1246,9 +1240,20 @@ int mpu3050_common_probe(struct device *dev,
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_put(dev);
 
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "device register failed\n");
+		goto err_iio_device_register;
+	}
+
 	return 0;
 
-err_cleanup_buffer:
+err_iio_device_register:
+	pm_runtime_get_sync(dev);
+	pm_runtime_put_noidle(dev);
+	pm_runtime_disable(dev);
+	if (irq)
+		free_irq(mpu3050->irq, mpu3050->trig);
 	iio_triggered_buffer_cleanup(indio_dev);
 err_power_down:
 	mpu3050_power_down(mpu3050);
@@ -1261,13 +1266,13 @@ void mpu3050_common_remove(struct device *dev)
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct mpu3050 *mpu3050 = iio_priv(indio_dev);
 
+	iio_device_unregister(indio_dev);
 	pm_runtime_get_sync(dev);
 	pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
 	iio_triggered_buffer_cleanup(indio_dev);
 	if (mpu3050->irq)
 		free_irq(mpu3050->irq, mpu3050->trig);
-	iio_device_unregister(indio_dev);
 	mpu3050_power_down(mpu3050);
 }
 
-- 
2.53.0

[PATCH v3 4/4] iio: gyro: mpu3050: Fix out-of-sequence free_irq()

[Ethan Tidmore]

The triggered buffer is initialized before the IRQ is requested. The
removal path currently calls  iio_triggered_buffer_cleanup() before 
free_irq(). This violates the expected LIFO. 

Place free_irq() in correct location.

Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v3: 
- Clarify patch purpose
v2:
- Patch added to series.

 drivers/iio/gyro/mpu3050-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index b590cf6709b4..f29c9ce63029 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1270,9 +1270,9 @@ void mpu3050_common_remove(struct device *dev)
 	pm_runtime_get_sync(dev);
 	pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
-	iio_triggered_buffer_cleanup(indio_dev);
 	if (mpu3050->irq)
 		free_irq(mpu3050->irq, mpu3050->trig);
+	iio_triggered_buffer_cleanup(indio_dev);
 	mpu3050_power_down(mpu3050);
 }
 
-- 
2.53.0

Re: [PATCH v3 4/4] iio: gyro: mpu3050: Fix out-of-sequence free_irq()

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 04:48:18PM -0600, Ethan Tidmore wrote:
> The triggered buffer is initialized before the IRQ is requested. The
> removal path currently calls  iio_triggered_buffer_cleanup() before 

Double space and trailing space.

> free_irq(). This violates the expected LIFO. 

Trailing space.

> 
> Place free_irq() in correct location.

...

No need to resend unless asked for.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/4] iio: gyro: mpu3050: Fix out-of-sequence free_irq()

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 04:48:18PM -0600, Ethan Tidmore wrote:
> The triggered buffer is initialized before the IRQ is requested. The
> removal path currently calls  iio_triggered_buffer_cleanup() before 
> free_irq(). This violates the expected LIFO. 
> 
> Place free_irq() in correct location.

"Place free_irq() in the correct location."


> - Clarify patch purpose

Still doesn't give me enough clarity.

The patch as I read it doesn't move free_irq().

...

But I leave it to native speaker and maintainer to decide. No need to resend.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/4] iio: gyro: mpu3050: Multiple bug fixes

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 04:48:14PM -0600, Ethan Tidmore wrote:
> This series fixes 4 bugs:
> - Patch 1: Fix free_irq() being called with wrong handler.
> - Patch 2: Added missing free_irq() in an error path.
> - Patch 3: Moved iio_device_register() to end of probe function, in
>   doing so had to implement correct error cleanups and place
>   iio_device_unregister in beginning of remove callback.
> - Patch 4: free_irq() is after called iio_triggered_buffer_cleanup()
>   which breaks LIFO, place it before it.
> 
> I plan on doing a proper devm_ conversion of this driver, but went ahead
> and did these fixes so they could be backported.

LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

The commit message in the patch 4 sounds to me as needs an improve,
but I leave this to Jonathan.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/4] iio: gyro: mpu3050: Fix out-of-sequence free_irq()

[Jonathan Cameron]

On Wed, 25 Feb 2026 13:59:20 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Feb 24, 2026 at 04:48:18PM -0600, Ethan Tidmore wrote:
> > The triggered buffer is initialized before the IRQ is requested. The
> > removal path currently calls  iio_triggered_buffer_cleanup() before 
> > free_irq(). This violates the expected LIFO. 
> > 
> > Place free_irq() in correct location.  
> 
> "Place free_irq() in the correct location."
> 
> 
> > - Clarify patch purpose  
> 
> Still doesn't give me enough clarity.
> 
> The patch as I read it doesn't move free_irq().

I went with
Place free_irq() in the correct location relative to
iio_triggered_buffer_cleanup().

As a Englishman I do always appreciate Andy adding the missing "the" which
I think of as an American thing :)  You really don't want to know how
many editing wars I've had over that in some specifications.

Jonathan

> 
> ...
> 
> But I leave it to native speaker and maintainer to decide. No need to resend.
> 

Re: [PATCH v3 0/4] iio: gyro: mpu3050: Multiple bug fixes

[Jonathan Cameron]

On Wed, 25 Feb 2026 14:01:44 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Feb 24, 2026 at 04:48:14PM -0600, Ethan Tidmore wrote:
> > This series fixes 4 bugs:
> > - Patch 1: Fix free_irq() being called with wrong handler.
> > - Patch 2: Added missing free_irq() in an error path.
> > - Patch 3: Moved iio_device_register() to end of probe function, in
> >   doing so had to implement correct error cleanups and place
> >   iio_device_unregister in beginning of remove callback.
> > - Patch 4: free_irq() is after called iio_triggered_buffer_cleanup()
> >   which breaks LIFO, place it before it.
> > 
> > I plan on doing a proper devm_ conversion of this driver, but went ahead
> > and did these fixes so they could be backported.  
> 
> LGTM,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 
> The commit message in the patch 4 sounds to me as needs an improve,
> but I leave this to Jonathan.
> 
> 

Series applied to the fixes-togreg branch of iio.git and marked for
stable inclusion.

Thanks,

Jonathan

Re: [PATCH v3 3/4] iio: gyro: mpu3050: Move iio_device_register() to correct location

[Jonathan Cameron]

On Tue, 24 Feb 2026 16:48:17 -0600
Ethan Tidmore <ethantidmore06@gmail.com> wrote:

> iio_device_register() should be at the end of the probe function to
> prevent race conditions.
Ah. I perhaps over stated this...
See below.

> 
> Place iio_device_register() at the end of the probe function and place
> iio_device_unregister() accordingly.
> 
> Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Reviewed-by: Linus Walleij <linusw@kernel.org>
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
> ---
> v3: 
> - Remove stray change.
> - Fix grammar with "the".
> v2:
> - Patch added to series.
> 
>  drivers/iio/gyro/mpu3050-core.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index b6e05afbe512..b590cf6709b4 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -1218,12 +1218,6 @@ int mpu3050_common_probe(struct device *dev,
>  		goto err_power_down;
>  	}
>  
> -	ret = iio_device_register(indio_dev);
> -	if (ret) {
> -		dev_err(dev, "device register failed\n");
> -		goto err_cleanup_buffer;
> -	}
> -
>  	dev_set_drvdata(dev, indio_dev);
>  
>  	/* Check if we have an assigned IRQ to use as trigger */
> @@ -1246,9 +1240,20 @@ int mpu3050_common_probe(struct device *dev,
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_put(dev);
>  
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(dev, "device register failed\n");
> +		goto err_iio_device_register;
> +	}
> +
>  	return 0;
>  
> -err_cleanup_buffer:
> +err_iio_device_register:
> +	pm_runtime_get_sync(dev);
> +	pm_runtime_put_noidle(dev);
> +	pm_runtime_disable(dev);
These are a rare thing that is fine either before or after iio_device_register()
as any races just result in elevated reference counts (which drop again when
the racing access finishes).

Having said that, the order you have here should be fine.

Jonathan


> +	if (irq)
> +		free_irq(mpu3050->irq, mpu3050->trig);
>  	iio_triggered_buffer_cleanup(indio_dev);
>  err_power_down:
>  	mpu3050_power_down(mpu3050);
> @@ -1261,13 +1266,13 @@ void mpu3050_common_remove(struct device *dev)
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct mpu3050 *mpu3050 = iio_priv(indio_dev);
>  
> +	iio_device_unregister(indio_dev);
>  	pm_runtime_get_sync(dev);
>  	pm_runtime_put_noidle(dev);
>  	pm_runtime_disable(dev);
>  	iio_triggered_buffer_cleanup(indio_dev);
>  	if (mpu3050->irq)
>  		free_irq(mpu3050->irq, mpu3050->trig);
> -	iio_device_unregister(indio_dev);
>  	mpu3050_power_down(mpu3050);
>  }
>