Re: [PATCH] iio: gyro: mpu3050: avoid double free_irq after trigger failure

[Guangshuo Li]

Hi Jonathan,

Thank you for pointing this out.

On Thu, 4 Jun 2026 at 01:18, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Wed,  3 Jun 2026 22:16:33 +0800
> Guangshuo Li <lgs201920130244@xxxxxxxxx> wrote:
>
> > commit 4216db1043a3b ("iio: gyro: mpu3050: Fix irq resource leak") made
> > mpu3050_trigger_probe() release the IRQ when iio_trigger_register() fails
> > after request_threaded_irq() succeeds.
> >
> > However, that failure is non-fatal to mpu3050_common_probe(). The caller
> > only logs the trigger setup failure and continues to register the IIO
> > device. Since mpu3050_trigger_probe() leaves mpu3050->irq set after
> > freeing the IRQ, later cleanup still believes the driver owns that IRQ.
> >
> > If iio_device_register() fails, the probe error path frees the IRQ again.
> > If probe succeeds, mpu3050_common_remove() frees it again during unbind.
> >
> > Clear mpu3050->irq after releasing the IRQ, and use mpu3050->irq rather
> > than the original irq argument to decide whether the probe error path
> > still has an IRQ to release.
> >
> > Fixes: 4216db1043a3b ("iio: gyro: mpu3050: Fix irq resource leak")
> > Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> Please check the list for similar fixes.
> https://lore.kernel.org/all/20260520152236.2308686-1-xinyuili@xxxxxxx/
> has a discussion of the right solution here being to just fail the probe
> if the trigger registration fails.
>
> The current attempt to paper over that is nasty and not something
> we normally do.
>
> Jonathan
>
> > ---
> >  drivers/iio/gyro/mpu3050-core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> > index d84e04e4b431..0fd881fdd1ad 100644
> > --- a/drivers/iio/gyro/mpu3050-core.c
> > +++ b/drivers/iio/gyro/mpu3050-core.c
> > @@ -1137,6 +1137,7 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
> >
> >  err_iio_trigger:
> >       free_irq(mpu3050->irq, mpu3050->trig);
> > +     mpu3050->irq = 0;
> >
> >       return ret;
> >  }
> > @@ -1260,7 +1261,7 @@ int mpu3050_common_probe(struct device *dev,
> >       pm_runtime_get_sync(dev);
> >       pm_runtime_put_noidle(dev);
> >       pm_runtime_disable(dev);
> > -     if (irq)
> > +     if (mpu3050->irq)
> >               free_irq(mpu3050->irq, mpu3050->trig);
> >       iio_triggered_buffer_cleanup(indio_dev);
> >  err_power_down:
>

I missed that previous discussion. The patch you mentioned already fixes
the issue I was trying to address.

Thanks,
Guangshuo