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

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

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]

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
-- 
2.47.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 1/2] iio: bu27034: simplify using guard(mutex)

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 5449 bytes --]

The BU27034 uses mutex for protecting the gain / time / scale changes.
The clean-up for a few of the functions can be slightly simplified by
removing the goto-based error handling/unlocking and by utilizing the
guard(mutex) scoped mutex handling instead.

Simplify driver by using the scoped mutexes.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 drivers/iio/light/rohm-bu27034.c | 73 +++++++++++---------------------
 1 file changed, 25 insertions(+), 48 deletions(-)

diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
index 76711c3cdf7c..d2ed92d5ec92 100644
--- a/drivers/iio/light/rohm-bu27034.c
+++ b/drivers/iio/light/rohm-bu27034.c
@@ -7,6 +7,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
@@ -395,30 +396,26 @@ static int bu27034_try_set_int_time(struct bu27034_data *data, int time_us)
 	int numg = ARRAY_SIZE(gains);
 	int ret, int_time_old, i;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 	ret = bu27034_get_int_time(data);
 	if (ret < 0)
-		goto unlock_out;
+		return ret;
 
 	int_time_old = ret;
 
 	if (!iio_gts_valid_time(&data->gts, time_us)) {
 		dev_err(data->dev, "Unsupported integration time %u\n",
 			time_us);
-		ret = -EINVAL;
-
-		goto unlock_out;
+		return -EINVAL;
 	}
 
-	if (time_us == int_time_old) {
-		ret = 0;
-		goto unlock_out;
-	}
+	if (time_us == int_time_old)
+		return 0;
 
 	for (i = 0; i < numg; i++) {
 		ret = bu27034_get_gain(data, gains[i].chan, &gains[i].old_gain);
 		if (ret)
-			goto unlock_out;
+			return 0;
 
 		ret = iio_gts_find_new_gain_by_old_gain_time(&data->gts,
 							     gains[i].old_gain,
@@ -434,7 +431,7 @@ static int bu27034_try_set_int_time(struct bu27034_data *data, int time_us)
 				gains[i].chan, time_us, scale1, scale2);
 
 			if (gains[i].new_gain < 0)
-				goto unlock_out;
+				return ret;
 
 			/*
 			 * If caller requests for integration time change and we
@@ -455,7 +452,7 @@ static int bu27034_try_set_int_time(struct bu27034_data *data, int time_us)
 					 "Total gain increase. Risk of saturation");
 				ret = iio_gts_get_min_gain(&data->gts);
 				if (ret < 0)
-					goto unlock_out;
+					return ret;
 			}
 			dev_dbg(data->dev, "chan %u scale changed\n",
 				 gains[i].chan);
@@ -468,15 +465,10 @@ static int bu27034_try_set_int_time(struct bu27034_data *data, int time_us)
 	for (i = 0; i < numg; i++) {
 		ret = bu27034_set_gain(data, gains[i].chan, gains[i].new_gain);
 		if (ret)
-			goto unlock_out;
+			return ret;
 	}
 
-	ret = bu27034_set_int_time(data, time_us);
-
-unlock_out:
-	mutex_unlock(&data->mutex);
-
-	return ret;
+	return bu27034_set_int_time(data, time_us);
 }
 
 static int bu27034_set_scale(struct bu27034_data *data, int chan,
@@ -492,10 +484,10 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 		return -EINVAL;
 	}
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 	ret = regmap_read(data->regmap, BU27034_REG_MODE_CONTROL1, &time_sel);
 	if (ret)
-		goto unlock_out;
+		return ret;
 
 	ret = iio_gts_find_gain_sel_for_scale_using_time(&data->gts, time_sel,
 						val, val2, &gain_sel);
@@ -518,7 +510,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 
 		ret = bu27034_get_gain(data, gain.chan, &gain.old_gain);
 		if (ret)
-			goto unlock_out;
+			return ret;
 
 		/*
 		 * Iterate through all the times to see if we find one which
@@ -551,26 +543,20 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 		if (!found) {
 			dev_dbg(data->dev,
 				"Can't set scale maintaining other channel\n");
-			ret = -EINVAL;
-
-			goto unlock_out;
+			return -EINVAL;
 		}
 
 		ret = bu27034_set_gain(data, gain.chan, gain.new_gain);
 		if (ret)
-			goto unlock_out;
+			return ret;
 
 		ret = regmap_update_bits(data->regmap, BU27034_REG_MODE_CONTROL1,
 				  BU27034_MASK_MEAS_MODE, new_time_sel);
 		if (ret)
-			goto unlock_out;
+			return ret;
 	}
 
-	ret = bu27034_write_gain_sel(data, chan, gain_sel);
-unlock_out:
-	mutex_unlock(&data->mutex);
-
-	return ret;
+	return bu27034_write_gain_sel(data, chan, gain_sel);
 }
 
 /*
@@ -1221,42 +1207,33 @@ static int bu27034_buffer_enable(struct iio_dev *idev)
 	struct task_struct *task;
 	int ret;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 	ret = bu27034_meas_set(data, true);
 	if (ret)
-		goto unlock_out;
+		return ret;
 
 	task = kthread_run(bu27034_buffer_thread, idev,
 				 "bu27034-buffering-%u",
 				 iio_device_id(idev));
-	if (IS_ERR(task)) {
-		ret = PTR_ERR(task);
-		goto unlock_out;
-	}
+	if (IS_ERR(task))
+		return PTR_ERR(task);
 
 	data->task = task;
 
-unlock_out:
-	mutex_unlock(&data->mutex);
-
-	return ret;
+	return 0;
 }
 
 static int bu27034_buffer_disable(struct iio_dev *idev)
 {
 	struct bu27034_data *data = iio_priv(idev);
-	int ret;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 	if (data->task) {
 		kthread_stop(data->task);
 		data->task = NULL;
 	}
 
-	ret = bu27034_meas_set(data, false);
-	mutex_unlock(&data->mutex);
-
-	return ret;
+	return bu27034_meas_set(data, false);
 }
 
 static const struct iio_buffer_setup_ops bu27034_buffer_ops = {
-- 
2.47.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] iio: bu27034: simplify using guard(mutex)

[Javier Carrasco]

On 21/11/2024 14:05, Matti Vaittinen wrote:
> The BU27034 uses mutex for protecting the gain / time / scale changes.
> The clean-up for a few of the functions can be slightly simplified by
> removing the goto-based error handling/unlocking and by utilizing the
> guard(mutex) scoped mutex handling instead.
> 
> Simplify driver by using the scoped mutexes.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
>  drivers/iio/light/rohm-bu27034.c | 73 +++++++++++---------------------
>  1 file changed, 25 insertions(+), 48 deletions(-)
> 

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

[PATCH 2/2] iio: bm1390: simplify using guard(mutex)

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 5752 bytes --]

The BM1390 uses mutex for protecting the fifo read sequence. The clean-up
for a few of the functions can be slightly simplified by removing the
goto-based error handling/unlocking and by utilizing the guard(mutex)
scoped mutex handling instead.

Simplify driver by using the scoped mutexes.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 drivers/iio/pressure/rohm-bm1390.c | 78 ++++++++++++------------------
 1 file changed, 30 insertions(+), 48 deletions(-)

diff --git a/drivers/iio/pressure/rohm-bm1390.c b/drivers/iio/pressure/rohm-bm1390.c
index ccaa07a569c9..fa1a477151d0 100644
--- a/drivers/iio/pressure/rohm-bm1390.c
+++ b/drivers/iio/pressure/rohm-bm1390.c
@@ -8,6 +8,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
@@ -263,14 +264,14 @@ static int bm1390_read_data(struct bm1390_data *data,
 {
 	int ret, warn;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 	/*
 	 * We use 'continuous mode' even for raw read because according to the
 	 * data-sheet an one-shot mode can't be used with IIR filter.
 	 */
 	ret = bm1390_meas_set(data, BM1390_MEAS_MODE_CONTINUOUS);
 	if (ret)
-		goto unlock_out;
+		return ret;
 
 	switch (chan->type) {
 	case IIO_PRESSURE:
@@ -287,10 +288,8 @@ static int bm1390_read_data(struct bm1390_data *data,
 	warn = bm1390_meas_set(data, BM1390_MEAS_MODE_STOP);
 	if (warn)
 		dev_warn(data->dev, "Failed to stop measurement (%d)\n", warn);
-unlock_out:
-	mutex_unlock(&data->mutex);
 
-	return ret;
+	return 0;
 }
 
 static int bm1390_read_raw(struct iio_dev *idev,
@@ -546,38 +545,33 @@ static int bm1390_fifo_enable(struct iio_dev *idev)
 	if (data->irq <= 0)
 		return -EINVAL;
 
-	mutex_lock(&data->mutex);
-	if (data->trigger_enabled) {
-		ret = -EBUSY;
-		goto unlock_out;
-	}
+	guard(mutex)(&data->mutex);
+
+	if (data->trigger_enabled)
+		return -EBUSY;
 
 	/* Update watermark to HW */
 	ret = bm1390_fifo_set_wmi(data);
 	if (ret)
-		goto unlock_out;
+		return ret;
 
 	/* Enable WMI_IRQ */
 	ret = regmap_set_bits(data->regmap, BM1390_REG_MODE_CTRL,
 			      BM1390_MASK_WMI_EN);
 	if (ret)
-		goto unlock_out;
+		return ret;
 
 	/* Enable FIFO */
 	ret = regmap_set_bits(data->regmap, BM1390_REG_FIFO_CTRL,
 			      BM1390_MASK_FIFO_EN);
 	if (ret)
-		goto unlock_out;
+		return ret;
 
 	data->state = BM1390_STATE_FIFO;
 
 	data->old_timestamp = iio_get_time_ns(idev);
-	ret = bm1390_meas_set(data, BM1390_MEAS_MODE_CONTINUOUS);
 
-unlock_out:
-	mutex_unlock(&data->mutex);
-
-	return ret;
+	return bm1390_meas_set(data, BM1390_MEAS_MODE_CONTINUOUS);
 }
 
 static int bm1390_fifo_disable(struct iio_dev *idev)
@@ -587,27 +581,22 @@ static int bm1390_fifo_disable(struct iio_dev *idev)
 
 	msleep(1);
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 	ret = bm1390_meas_set(data, BM1390_MEAS_MODE_STOP);
 	if (ret)
-		goto unlock_out;
+		return ret;
 
 	/* Disable FIFO */
 	ret = regmap_clear_bits(data->regmap, BM1390_REG_FIFO_CTRL,
 				BM1390_MASK_FIFO_EN);
 	if (ret)
-		goto unlock_out;
+		return ret;
 
 	data->state = BM1390_STATE_SAMPLE;
 
 	/* Disable WMI_IRQ */
-	ret = regmap_clear_bits(data->regmap, BM1390_REG_MODE_CTRL,
+	return regmap_clear_bits(data->regmap, BM1390_REG_MODE_CTRL,
 				 BM1390_MASK_WMI_EN);
-
-unlock_out:
-	mutex_unlock(&data->mutex);
-
-	return ret;
 }
 
 static int bm1390_buffer_postenable(struct iio_dev *idev)
@@ -691,25 +680,24 @@ static irqreturn_t bm1390_irq_thread_handler(int irq, void *private)
 {
 	struct iio_dev *idev = private;
 	struct bm1390_data *data = iio_priv(idev);
-	int ret = IRQ_NONE;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (data->trigger_enabled) {
 		iio_trigger_poll_nested(data->trig);
-		ret = IRQ_HANDLED;
-	} else if (data->state == BM1390_STATE_FIFO) {
+		return IRQ_HANDLED;
+	}
+
+	if (data->state == BM1390_STATE_FIFO) {
 		int ok;
 
 		ok = __bm1390_fifo_flush(idev, BM1390_FIFO_LENGTH,
 					 data->timestamp);
 		if (ok > 0)
-			ret = IRQ_HANDLED;
+			return IRQ_HANDLED;
 	}
 
-	mutex_unlock(&data->mutex);
-
-	return ret;
+	return IRQ_NONE;
 }
 
 static int bm1390_set_drdy_irq(struct bm1390_data *data, bool en)
@@ -725,17 +713,16 @@ static int bm1390_trigger_set_state(struct iio_trigger *trig,
 				    bool state)
 {
 	struct bm1390_data *data = iio_trigger_get_drvdata(trig);
-	int ret = 0;
+	int ret;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (data->trigger_enabled == state)
-		goto unlock_out;
+		return 0;
 
 	if (data->state == BM1390_STATE_FIFO) {
 		dev_warn(data->dev, "Can't set trigger when FIFO enabled\n");
-		ret = -EBUSY;
-		goto unlock_out;
+		return -EBUSY;
 	}
 
 	data->trigger_enabled = state;
@@ -743,13 +730,13 @@ static int bm1390_trigger_set_state(struct iio_trigger *trig,
 	if (state) {
 		ret = bm1390_meas_set(data, BM1390_MEAS_MODE_CONTINUOUS);
 		if (ret)
-			goto unlock_out;
+			return ret;
 	} else {
 		int dummy;
 
 		ret = bm1390_meas_set(data, BM1390_MEAS_MODE_STOP);
 		if (ret)
-			goto unlock_out;
+			return ret;
 
 		/*
 		 * We need to read the status register in order to ACK the
@@ -761,12 +748,7 @@ static int bm1390_trigger_set_state(struct iio_trigger *trig,
 			dev_warn(data->dev, "status read failed\n");
 	}
 
-	ret = bm1390_set_drdy_irq(data, state);
-
-unlock_out:
-	mutex_unlock(&data->mutex);
-
-	return ret;
+	return bm1390_set_drdy_irq(data, state);
 }
 
 static const struct iio_trigger_ops bm1390_trigger_ops = {
-- 
2.47.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] iio: bm1390: simplify using guard(mutex)

[Javier Carrasco]

On 21/11/2024 14:05, Matti Vaittinen wrote:
> The BM1390 uses mutex for protecting the fifo read sequence. The clean-up
> for a few of the functions can be slightly simplified by removing the
> goto-based error handling/unlocking and by utilizing the guard(mutex)
> scoped mutex handling instead.
> 
> Simplify driver by using the scoped mutexes.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
>  drivers/iio/pressure/rohm-bm1390.c | 78 ++++++++++++------------------
>  1 file changed, 30 insertions(+), 48 deletions(-)
> 

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

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

[Javier Carrasco]

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?

Best regards,
Javier Carrasco

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

[Matti Vaittinen]

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

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

[Javier Carrasco]

On 22/11/2024 07:10, Matti Vaittinen wrote:
> 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
> 

Yes, the cases I saw had very restricted scope. I just wanted to make
sure that you left them untouched on purpose. Often such refactoring of
mutex handling opts for removing all calls of mutex_lock/unlock to avoid
mixing both approaches in the same driver.

Personally, I like the scoped_guard() for short scopes too because it is
more robust if new code is added in that scope. But that is just a
preference :)

Best regards,
Javier Carrasco

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

[Jonathan Cameron]

On Thu, 21 Nov 2024 15:04:53 +0200
Matti Vaittinen <mazziesaccount@gmail.com> 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
> 
Applied. Thanks,

J
> ---
> 
> 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