[PATCH v3 0/4] Make 'mlock' really private

[Andy Shevchenko]

On Wed, Oct 12, 2022 at 6:15 PM Nuno Sá <nuno.sa at analog.com> 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 at gmail.com>

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