Re: [PATCH 2/2] iio: bmi160: use all devm functions in probe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/25/18 5:51 AM, Jonathan Cameron wrote:
On Mon, 19 Nov 2018 10:48:20 +0200
Daniel Baluta <daniel.baluta@xxxxxxxxx> wrote:

On Mon, Nov 19, 2018 at 2:55 AM Martin Kelly <martin@xxxxxxxxxxxxxxxx> wrote:

From: Martin Kelly <martin@xxxxxxxxxxxxxxxx>

Currently, we're using the devm version of some but not all functions.
Switch to the devm version of iio_triggered_buffer_setup and
iio_device_register to simplify the code a bit and decrease the chance of
bugs.

Yes, it makes sense.

Acked-by: Daniel Baluta <daniel.baluta@xxxxxxxxx>
Here we disagree :).  This results in the chip being uninitialized
via bmi160_chip_uninit before we remove the user space interfaces.
So the combination that is there right now was very deliberate
and is correct - that's not to say it can't be improved upon.

So if you do want to do this you need to ensure that operation
automatically occurs in the correct point in the remove and error
flows.

devm_add_action_or_reset is your friend here as it'll let you call
that uninit from the automatic unwinding path and hence at the
correct point.

Thanks,

Jonathan


Thank you and good catch. I'll send a v2 using devm_add_action_or_reset.



Signed-off-by: Martin Kelly <martin@xxxxxxxxxxxxxxxx>
---
  drivers/iio/imu/bmi160/bmi160_core.c | 12 ++++--------
  1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index 4d9d59d9e3a9..5e4efaed5e88 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -577,18 +577,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
         indio_dev->modes = INDIO_DIRECT_MODE;
         indio_dev->info = &bmi160_info;

-       ret = iio_triggered_buffer_setup(indio_dev, NULL,
-                                        bmi160_trigger_handler, NULL);
+       ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+                                             bmi160_trigger_handler, NULL);
         if (ret < 0)
                 goto uninit;

-       ret = iio_device_register(indio_dev);
+       ret = devm_iio_device_register(dev, indio_dev);
         if (ret < 0)
-               goto buffer_cleanup;
+               goto uninit;

         return 0;
-buffer_cleanup:
-       iio_triggered_buffer_cleanup(indio_dev);
  uninit:
         bmi160_chip_uninit(data);
         return ret;
@@ -600,8 +598,6 @@ void bmi160_core_remove(struct device *dev)
         struct iio_dev *indio_dev = dev_get_drvdata(dev);
         struct bmi160_data *data = iio_priv(indio_dev);

-       iio_device_unregister(indio_dev);
-       iio_triggered_buffer_cleanup(indio_dev);
         bmi160_chip_uninit(data);
  }
  EXPORT_SYMBOL_GPL(bmi160_core_remove);
--
2.11.0





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux