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@xxxxxxxxx> 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@xxxxxxxxxx>
> Reviewed-by: Linus Walleij <linusw@xxxxxxxxxx>
> Signed-off-by: Ethan Tidmore <ethantidmore06@xxxxxxxxx>
> ---
> 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);
>  }
>