Re: [PATCH] iio: accel: bma180: use devm_iio_triggered_buffer_setup()

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 21 Mar 2026 17:18:48 +0530
Shi Hao <i.shihao.999@gmail.com> wrote:

> Use devm_iio_triggered_buffer_setup() instead of
> iio_triggered_buffer_setup(). This removes the need for manual cleanup in
> both probe and remove callbacks, simplifying resource management.

This also changes the ordering so that the remove() path no longer
does things in the reverse order of probe()

Whilst that might not introduce bugs in this case, it makes reasoning
about race conditions much harder so I won't take code that does this.

The basic 'rule' for devm usage is that there must be only one transition
in the probe() from using it to not using it. You should never go back
to using it after that transition.  That way the handling in remove()
and the unwinding of the devm_* happen in reverse order of probe()
and all is easy to reason about

So to make any devm_ related changes in this driver requires a
more comprehensive approach.  Note that even if the change here didn't
suffer this ordering problem I'd be pushing back because of the partial
nature of applying devm in this driver.  + a complete solution would
not run into the ordering issue.

Thanks

Jonathan


> 
> Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
> ---
>  drivers/iio/accel/bma180.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
> index 7bc6761f5135..429d5a5c7672 100644
> --- a/drivers/iio/accel/bma180.c
> +++ b/drivers/iio/accel/bma180.c
> @@ -1004,7 +1004,7 @@ static int bma180_probe(struct i2c_client *client)
> 		indio_dev->trig = iio_trigger_get(data->trig);
> 	}
> 
> -	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> 			bma180_trigger_handler, NULL);
> 	if (ret < 0) {
> 		dev_err(dev, "unable to setup iio triggered buffer\n");
> @@ -1014,13 +1014,11 @@ static int bma180_probe(struct i2c_client *client)
> 	ret = iio_device_register(indio_dev);
> 	if (ret < 0) {
> 		dev_err(dev, "unable to register iio device\n");
> -		goto err_buffer_cleanup;
> +		goto err_trigger_unregister;
> 	}
> 
> 	return 0;
> 
> -err_buffer_cleanup:
> -	iio_triggered_buffer_cleanup(indio_dev);
>  err_trigger_unregister:
> 	if (data->trig)
> 		iio_trigger_unregister(data->trig);
> @@ -1041,7 +1039,6 @@ static void bma180_remove(struct i2c_client *client)
> 	struct bma180_data *data = iio_priv(indio_dev);
> 
> 	iio_device_unregister(indio_dev);
> -	iio_triggered_buffer_cleanup(indio_dev);
> 	if (data->trig) {
> 		iio_trigger_unregister(data->trig);
> 		iio_trigger_free(data->trig);
> --
> 2.53.0