On Wed, Oct 12, 2022 at 6:15 PM Nuno Sá <nuno.sa@xxxxxxxxxx> wrote:
>
> This patchset cleans all the drivers directly using the iio_device 'mlock'.
> This lock is private and should not be used outside the core (or by using
> proper helpers).
>
> Most of the conversions where straight, but there are some that really need
> extra looking. Mainly patches [13/15] and [14/15] were a bit hacky since
> iio_device_claim_direct_mode() does not fit 100%. The reason is that we
> want to check if the device is buffering and do something if it is (in
> which case the API return -EBUSY and released the lock. I just used a
> combinations of locks to get around this (hopefully I did not messed up).
>
> Note that this series was only compiled tested using allyesconfig for
> ARM. I ran 'git grep' to make sure there were no more users of 'mlock'.
> Hopefully I covered them all...
Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
I haven't seen any serious issues, some small ones regarding spelling,
indentation and comment are per individual patches.
> v2:
>
> [PATCH 1-8, 10-12/16]
> * Mention the inclusion of mutex.h in the commit message.
>
> [PATCH 1-8, 10, 12/16]
> * Initialize mutex as late as possible.
> Note that [PATCH 11/16] was not included since the code to do so was not
> direct enough. Would need to get a pointer to the private struture
> outside of scmi_alloc_iiodev() to do it. While not hard, the added changes
> in the code is not really worth it (IMO of course).
>
> [PATCH 1/16]
> * Refactored the commit message a bit. I guess this one will still needs
> more discussion...
>
> [PATCH 9/16]
> * New patch to add an helper function to read the samples.
>
> [PATCH 13/16]
> * New patch to introduce iio_device_{claim|release}_buffer_mode() APIs.
>
> [PATCH 14/16]
> * Make use of the new iio_device_{claim|release}_buffer_mode() helpers
>
> [PATCH 15/16]
> * Make use of the new iio_device_{claim|release}_buffer_mode() helpers
> in combination with claim_direct_mode(). This is needed so that we make sure
> we always get one of the modes (and hence the iio_dev lock) to safely call
> max30102_get_temp(). Note that I'm not particular "happy" with the code but
> OTOH, it does not look as bad as I thought :). Anyways, if there are no
> complains with it, I'm ok to leave it as-is. Otherwise, I think we can think
> on the flag approach (briefly discussed in the first series).
>
> v3:
>
> [PATCH 1/4]
> * fix 'make W=1' warning about prototypes mismatch.
>
> [PATCH 2/4]
> * improved commit message to explain why we are shadowing error codes.
>
> [PATCH 4/4]
> * minor English fix on the commit message (as suggested by Andy).
>
> Nuno Sá (4):
> iio: core: introduce iio_device_{claim|release}_buffer_mode() APIs
> iio: health: max30100: do not use internal iio_dev lock
> iio: health: max30102: do not use internal iio_dev lock
> iio: core: move 'mlock' to 'struct iio_dev_opaque'
>
> drivers/iio/TODO | 3 --
> drivers/iio/health/max30100.c | 9 ++---
> drivers/iio/health/max30102.c | 19 +++++++---
> drivers/iio/industrialio-buffer.c | 29 ++++++++-------
> drivers/iio/industrialio-core.c | 58 +++++++++++++++++++++++++-----
> drivers/iio/industrialio-event.c | 4 +--
> drivers/iio/industrialio-trigger.c | 12 +++----
> include/linux/iio/iio-opaque.h | 2 ++
> include/linux/iio/iio.h | 5 ++-
> 9 files changed, 97 insertions(+), 44 deletions(-)
>
> --
> 2.38.0
>
--
With Best Regards,
Andy Shevchenko
