Re: [PATCH 0/2] iio: Use __cleanup for a few ROHM sensors

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

 



Hi Javier,

On 21/11/2024 15:54, Javier Carrasco wrote:
On 21/11/2024 14:04, Matti Vaittinen wrote:
Use __cleanup.

The series converts the rest of the ROHM sensors (maintained by me) to
use guard(mutex). This simplifies the error paths.

As a note, kx022a accelerometer driver is handled in another series,
which also adds support for two new accelerometers. I did also patch the
driver for the BU27008 and BU27010 - but when I was testing the changes
I found that the BU27008 status is set to "obsolete". I'll try to dig
some information about the BU27010 and decide if having the driver
in-tree is still worth the effort, or if I should just send out patches
to drop it all. Hence patch to rohm-bu27008.c is not included in the
series. If someone is actually using the BU27008 or BU27010 and wants
to patch it - feel free to pick
131315de97ff ("iio: bu27008: simplify using guard(mutex)")
from
https://github.com/M-Vaittinen/linux/tree/bu27008-cleanup

---

Matti Vaittinen (2):
   iio: bu27034: simplify using guard(mutex)
   iio: bm1390: simplify using guard(mutex)

  drivers/iio/light/rohm-bu27034.c   | 73 ++++++++++------------------
  drivers/iio/pressure/rohm-bm1390.c | 78 ++++++++++++------------------
  2 files changed, 55 insertions(+), 96 deletions(-)


base-commit: adc218676eef25575469234709c2d87185ca223a

Hi Matti,

Both patches look good to me, but I noticed that you kept a few
mutex_lock() + mutex_unlock() in both drivers, in particular in the
cases where a scoped_guard() could simplify the code. Did you leave
those cases untouched on purpose?

Thanks for taking a look at the patches. Much appreciated :)

I remember leaving couple of direct calls to mutex_lock() and mutex_unlock() - but I think I left them only to places where I saw no real improvement by the use of guard() or scoped_guard(). It is likely I considered the locking in these cases being trivial. (Probably only for a duration of one or couple of function calls, with no error handling when a lock is held). The direct mutex_lock()/mutex_unlock() has no real room for usual errors (like leaving the function while lock was taken) in such case.

For me,

mutex_lock();
ret = foo();
mutex_unlock();

is as clear as it gets. I don't think scoped_guard() has benefits there. On the contrary, for me the scoped_guard() would be more complex and less obvious :)

Yours,
	-- Matti





[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