| Message ID | 20201130125915.1315667-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] iio: gyro: mpu3050: Use devm_ to set up buffer | expand |
On Mon, 30 Nov 2020 13:59:14 +0100 Linus Walleij <linus.walleij@linaro.org> wrote: > This makes use of devm_iio_triggered_buffer_setup() to > save some minor overhead. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Hi Linus, I'm very fussy about mixing devm and other cleanup. Unless everything that happens after this point is also managed, I'm not happy to see an individual function made managed. It may be safe, but if fails the 'obviously safe' test. Something odd going on here though. We are currently removing the buffer before we unregister the userspace interfaces to it. That's not a good idea. The remove order seems reverse from what it should be... Jonathan > --- > drivers/iio/gyro/mpu3050-core.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c > index 00e58060968c..0d0850945d3a 100644 > --- a/drivers/iio/gyro/mpu3050-core.c > +++ b/drivers/iio/gyro/mpu3050-core.c > @@ -1203,9 +1203,10 @@ int mpu3050_common_probe(struct device *dev, > indio_dev->modes = INDIO_DIRECT_MODE; > indio_dev->name = name; > > - ret = iio_triggered_buffer_setup(indio_dev, iio_pollfunc_store_time, > - mpu3050_trigger_handler, > - &mpu3050_buffer_setup_ops); > + ret = devm_iio_triggered_buffer_setup(dev, > + indio_dev, iio_pollfunc_store_time, > + mpu3050_trigger_handler, > + &mpu3050_buffer_setup_ops); > if (ret) { > dev_err(dev, "triggered buffer setup failed\n"); > goto err_power_down; > @@ -1214,7 +1215,7 @@ int mpu3050_common_probe(struct device *dev, > ret = iio_device_register(indio_dev); > if (ret) { > dev_err(dev, "device register failed\n"); > - goto err_cleanup_buffer; > + goto err_power_down; > } > > dev_set_drvdata(dev, indio_dev); > @@ -1241,8 +1242,6 @@ int mpu3050_common_probe(struct device *dev, > > return 0; > > -err_cleanup_buffer: > - iio_triggered_buffer_cleanup(indio_dev); > err_power_down: > mpu3050_power_down(mpu3050); > > @@ -1258,7 +1257,6 @@ int 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); > iio_device_unregister(indio_dev);
diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c index 00e58060968c..0d0850945d3a 100644 --- a/drivers/iio/gyro/mpu3050-core.c +++ b/drivers/iio/gyro/mpu3050-core.c @@ -1203,9 +1203,10 @@ int mpu3050_common_probe(struct device *dev, indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->name = name; - ret = iio_triggered_buffer_setup(indio_dev, iio_pollfunc_store_time, - mpu3050_trigger_handler, - &mpu3050_buffer_setup_ops); + ret = devm_iio_triggered_buffer_setup(dev, + indio_dev, iio_pollfunc_store_time, + mpu3050_trigger_handler, + &mpu3050_buffer_setup_ops); if (ret) { dev_err(dev, "triggered buffer setup failed\n"); goto err_power_down; @@ -1214,7 +1215,7 @@ int mpu3050_common_probe(struct device *dev, ret = iio_device_register(indio_dev); if (ret) { dev_err(dev, "device register failed\n"); - goto err_cleanup_buffer; + goto err_power_down; } dev_set_drvdata(dev, indio_dev); @@ -1241,8 +1242,6 @@ int mpu3050_common_probe(struct device *dev, return 0; -err_cleanup_buffer: - iio_triggered_buffer_cleanup(indio_dev); err_power_down: mpu3050_power_down(mpu3050); @@ -1258,7 +1257,6 @@ int 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); iio_device_unregister(indio_dev);
This makes use of devm_iio_triggered_buffer_setup() to save some minor overhead. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/iio/gyro/mpu3050-core.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)