[PATCH v2 0/4] Multiple bug fixes

[PATCH v2 0/4] 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.

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 | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

-- 
2.53.0

[PATCH v2 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:

	ret = request_threaded_irq(irq,
				   mpu3050_irq_handler,
				   mpu3050_irq_thread,
				   irq_trig,
				   mpu3050->trig->name,
				   mpu3050->trig);

...

void mpu3050_common_remove(struct device *dev)
{

...

	if (mpu3050->irq)
		free_irq(mpu3050->irq, mpu3050);
	iio_device_unregister(indio_dev);
	mpu3050_power_down(mpu3050);
}

Use correct IRQ handler when calling free_irq().

Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 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

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

[Linus Walleij]

On Tue, Feb 24, 2026 at 3:35 AM Ethan Tidmore <ethantidmore06@gmail.com> wrote:

> The handler for the IRQ part of this driver is mpu3050->trig but,
> in the teardown free_irq() is called with handler mpu3050:
>
>         ret = request_threaded_irq(irq,
>                                    mpu3050_irq_handler,
>                                    mpu3050_irq_thread,
>                                    irq_trig,
>                                    mpu3050->trig->name,
>                                    mpu3050->trig);
>
> ...
>
> void mpu3050_common_remove(struct device *dev)
> {
>
> ...
>
>         if (mpu3050->irq)
>                 free_irq(mpu3050->irq, mpu3050);
>         iio_device_unregister(indio_dev);
>         mpu3050_power_down(mpu3050);
> }
>
> Use correct IRQ handler when calling free_irq().
>
> Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

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

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 08:35:08PM -0600, Ethan Tidmore wrote:
> The handler for the IRQ part of this driver is mpu3050->trig but,
> in the teardown free_irq() is called with handler mpu3050:
> 
> 	ret = request_threaded_irq(irq,
> 				   mpu3050_irq_handler,
> 				   mpu3050_irq_thread,
> 				   irq_trig,
> 				   mpu3050->trig->name,
> 				   mpu3050->trig);

No need to cite the full snippet (we may easily open the file and read that),
it's enough to mention like

	request_threaded_irq(..., mpu3050->trig)

or just in free words without any code.

...

> void mpu3050_common_remove(struct device *dev)
> {
> 
> ...
> 
> 	if (mpu3050->irq)
> 		free_irq(mpu3050->irq, mpu3050);
> 	iio_device_unregister(indio_dev);
> 	mpu3050_power_down(mpu3050);
> }

Ditto.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 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:

        ret = request_threaded_irq(irq,
                                   mpu3050_irq_handler,
                                   mpu3050_irq_thread,
                                   irq_trig,
                                   mpu3050->trig->name,
                                   mpu3050->trig);

...

        ret = iio_trigger_register(mpu3050->trig);
        if (ret)
                return ret;

        indio_dev->trig = iio_trigger_get(mpu3050->trig);

        return 0;
}

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")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 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

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

[Linus Walleij]

On Tue, Feb 24, 2026 at 3:35 AM Ethan Tidmore <ethantidmore06@gmail.com> wrote:

> The interrupt handler is setup but only a few lines down if
> iio_trigger_register() fails the function returns without properly
> releasing the handler:
>
>         ret = request_threaded_irq(irq,
>                                    mpu3050_irq_handler,
>                                    mpu3050_irq_thread,
>                                    irq_trig,
>                                    mpu3050->trig->name,
>                                    mpu3050->trig);
>
> ...
>
>         ret = iio_trigger_register(mpu3050->trig);
>         if (ret)
>                 return ret;
>
>         indio_dev->trig = iio_trigger_get(mpu3050->trig);
>
>         return 0;
> }
>
> 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")
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

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

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 08:35:09PM -0600, Ethan Tidmore wrote:
> The interrupt handler is setup but only a few lines down if
> iio_trigger_register() fails the function returns without properly
> releasing the handler:

>         ret = request_threaded_irq(irq,
>                                    mpu3050_irq_handler,
>                                    mpu3050_irq_thread,
>                                    irq_trig,
>                                    mpu3050->trig->name,
>                                    mpu3050->trig);
> 
> ...
> 
>         ret = iio_trigger_register(mpu3050->trig);
>         if (ret)
>                 return ret;
> 
>         indio_dev->trig = iio_trigger_get(mpu3050->trig);
> 
>         return 0;
> }

Same comment, try to avoid citing the existing information (id est
the contents of the file).

> 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.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 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 end of 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>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 drivers/iio/gyro/mpu3050-core.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index b6e05afbe512..73dc7ea1f075 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 */
@@ -1231,6 +1225,7 @@ int mpu3050_common_probe(struct device *dev,
 		ret = mpu3050_trigger_probe(indio_dev, irq);
 		if (ret)
 			dev_err(dev, "failed to register trigger\n");
+
 	}
 
 	/* Enable runtime PM */
@@ -1246,9 +1241,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 +1267,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

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

[Linus Walleij]

On Tue, Feb 24, 2026 at 3:35 AM Ethan Tidmore <ethantidmore06@gmail.com> wrote:

> iio_device_register() should be at the end of the probe function to
> prevent race conditions.
>
> Place iio_device_register() at end of 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>
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

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

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 08:35:10PM -0600, Ethan Tidmore wrote:
> iio_device_register() should be at the end of the probe function to
> prevent race conditions.
> 
> Place iio_device_register() at end of probe function and place

at the end

> iio_device_unregister() accordingly.

...

> @@ -1231,6 +1225,7 @@ int mpu3050_common_probe(struct device *dev,
>  		ret = mpu3050_trigger_probe(indio_dev, irq);
>  		if (ret)
>  			dev_err(dev, "failed to register trigger\n");
> +
>  	}
>  
>  	/* Enable runtime PM */

Stray change.

-- 
With Best Regards,
Andy Shevchenko

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

[Ethan Tidmore]

The function free_irq() is placed after iio_triggered_buffer_cleanup()
which breaks LIFO because the IRQ was setup after
iio_triggered_buffer_setup().

Place free_irq() in correct location.

Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 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 73dc7ea1f075..3517522e3a51 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1271,9 +1271,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 v2 4/4] iio: gyro: mpu3050: Fix out-of-sequence free_irq()

[Linus Walleij]

On Tue, Feb 24, 2026 at 3:35 AM Ethan Tidmore <ethantidmore06@gmail.com> wrote:

> The function free_irq() is placed after iio_triggered_buffer_cleanup()
> which breaks LIFO because the IRQ was setup after
> iio_triggered_buffer_setup().
>
> Place free_irq() in correct location.
>
> Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

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

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 08:35:11PM -0600, Ethan Tidmore wrote:
> The function free_irq() is placed after iio_triggered_buffer_cleanup()
> which breaks LIFO because the IRQ was setup after
> iio_triggered_buffer_setup().
> 
> Place free_irq() in correct location.

With this commit message it's unclear why it's not a part of the patch 1
to begin with. You need to rephrase to make sure it's all about
iio_triggered_buffer_cleanup() in the first place.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/4] Multiple bug fixes

[Ethan Tidmore]

On Mon Feb 23, 2026 at 8:35 PM CST, Ethan Tidmore wrote:

Just realized I didn't put a changelog, the only actual changes is the
implementation of devm_ to manual cleanup with Patch 2 [1]. The rest are
new patches.

[1] https://lore.kernel.org/all/20260220200522.19967-1-ethantidmore06@gmail.com/

Thanks,

ET

Re: [PATCH v2 0/4] Multiple bug fixes

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 08:35:07PM -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.

All good, except one stray change and some work needed in the commit messages.

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

This would be nice!

-- 
With Best Regards,
Andy Shevchenko