drivers/iio/accel/adxl313_core.c | 7 +++---- drivers/iio/accel/adxl355_core.c | 27 ++++++++------------------- drivers/iio/accel/adxl372.c | 8 +++----- 3 files changed, 14 insertions(+), 28 deletions(-)
This series converts manual mutex_lock/mutex_unlock pairs to
guard(mutex) in three ADXL accelerometer drivers. Each conversion
also simplifies error handling by removing goto labels and
returning directly.
v2: Split into one patch per driver as requested by David Lechner.
Ensured each conversion also simplifies code flow.
Rajveer Chaudhari (3):
iio: accel: adxl313: convert to guard(mutex)
iio: accel: adxl355: convert to guard(mutex)
iio: accel: adxl372: convert to guard(mutex)
drivers/iio/accel/adxl313_core.c | 7 +++----
drivers/iio/accel/adxl355_core.c | 27 ++++++++-------------------
drivers/iio/accel/adxl372.c | 8 +++-----
3 files changed, 14 insertions(+), 28 deletions(-)
--
2.53.0
This series converts manual mutex_lock/mutex_unlock pairs to
guard(mutex) in three ADXL accelerometer drivers. Each conversion
also simplifies error handling by removing goto labels and
returning directly where possible.
v3: Remove remaining goto labels completely as requested by David Lechner.
Return directly from functions where ret variable is not needed.
v2: Split into one patch per driver as requested by David Lechner.
Ensured each conversion also simplifies code flow.
Rajveer Chaudhari (3):
iio: accel: adxl313: convert to guard(mutex)
iio: accel: adxl355: convert to guard(mutex)
iio: accel: adxl372: convert to guard(mutex)
drivers/iio/accel/adxl313_core.c | 15 ++----
drivers/iio/accel/adxl355_core.c | 81 ++++++++++++++------------------
drivers/iio/accel/adxl372.c | 16 +++----
3 files changed, 45 insertions(+), 67 deletions(-)
--
2.53.0
On Sat, 7 Mar 2026 15:47:55 +0530 Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote: > This series converts manual mutex_lock/mutex_unlock pairs to > guard(mutex) in three ADXL accelerometer drivers. Each conversion > also simplifies error handling by removing goto labels and > returning directly where possible. > > v3: Remove remaining goto labels completely as requested by David Lechner. > Return directly from functions where ret variable is not needed. > v2: Split into one patch per driver as requested by David Lechner. > Ensured each conversion also simplifies code flow. > > Rajveer Chaudhari (3): > iio: accel: adxl313: convert to guard(mutex) > iio: accel: adxl355: convert to guard(mutex) > iio: accel: adxl372: convert to guard(mutex) > > drivers/iio/accel/adxl313_core.c | 15 ++---- > drivers/iio/accel/adxl355_core.c | 81 ++++++++++++++------------------ > drivers/iio/accel/adxl372.c | 16 +++---- > 3 files changed, 45 insertions(+), 67 deletions(-) > Hi Rajveer, Small process thing. Please send each new version as a separate email thread. It can get very hard to follow when we end up with deeply nested threads so most areas of the kernel prefer to just associate them via name of the thread and possibly a link in the cover letter to the previous version. Jonathan
Hello Jonathan, I am sorry for making things confusing, do you want me to re-send V3 on a new separate email thread? Or I should keep this in mind for future patches. Thankyou, Rajveer On Sat, Mar 7, 2026 at 3:56 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sat, 7 Mar 2026 15:47:55 +0530 > Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote: > > > This series converts manual mutex_lock/mutex_unlock pairs to > > guard(mutex) in three ADXL accelerometer drivers. Each conversion > > also simplifies error handling by removing goto labels and > > returning directly where possible. > > > > v3: Remove remaining goto labels completely as requested by David Lechner. > > Return directly from functions where ret variable is not needed. > > v2: Split into one patch per driver as requested by David Lechner. > > Ensured each conversion also simplifies code flow. > > > > Rajveer Chaudhari (3): > > iio: accel: adxl313: convert to guard(mutex) > > iio: accel: adxl355: convert to guard(mutex) > > iio: accel: adxl372: convert to guard(mutex) > > > > drivers/iio/accel/adxl313_core.c | 15 ++---- > > drivers/iio/accel/adxl355_core.c | 81 ++++++++++++++------------------ > > drivers/iio/accel/adxl372.c | 16 +++---- > > 3 files changed, 45 insertions(+), 67 deletions(-) > > > > Hi Rajveer, > > Small process thing. Please send each new version as a separate email > thread. It can get very hard to follow when we end up with deeply > nested threads so most areas of the kernel prefer to just associate > them via name of the thread and possibly a link in the cover letter > to the previous version. > > Jonathan
On Sat, 7 Mar 2026 16:03:58 +0530 Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote: > Hello Jonathan, > > I am sorry for making things confusing, do you want me to re-send V3 > on a new separate email thread? > Or I should keep this in mind for future patches. Keep in mind for future patch sets / v4 of this one Thanks, J > > Thankyou, > Rajveer > > On Sat, Mar 7, 2026 at 3:56 PM Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Sat, 7 Mar 2026 15:47:55 +0530 > > Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote: > > > > > This series converts manual mutex_lock/mutex_unlock pairs to > > > guard(mutex) in three ADXL accelerometer drivers. Each conversion > > > also simplifies error handling by removing goto labels and > > > returning directly where possible. > > > > > > v3: Remove remaining goto labels completely as requested by David Lechner. > > > Return directly from functions where ret variable is not needed. > > > v2: Split into one patch per driver as requested by David Lechner. > > > Ensured each conversion also simplifies code flow. > > > > > > Rajveer Chaudhari (3): > > > iio: accel: adxl313: convert to guard(mutex) > > > iio: accel: adxl355: convert to guard(mutex) > > > iio: accel: adxl372: convert to guard(mutex) > > > > > > drivers/iio/accel/adxl313_core.c | 15 ++---- > > > drivers/iio/accel/adxl355_core.c | 81 ++++++++++++++------------------ > > > drivers/iio/accel/adxl372.c | 16 +++---- > > > 3 files changed, 45 insertions(+), 67 deletions(-) > > > > > > > Hi Rajveer, > > > > Small process thing. Please send each new version as a separate email > > thread. It can get very hard to follow when we end up with deeply > > nested threads so most areas of the kernel prefer to just associate > > them via name of the thread and possibly a link in the cover letter > > to the previous version. > > > > Jonathan
Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
adxl313_read_axis(). This ensures the mutex is released on all
return paths and allows returning directly without a goto label.
v3: Return directly from regmap_bulk_read error path.
v2: Split into separate patch per driver.
Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
---
drivers/iio/accel/adxl313_core.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index 9f5d4d2cb325..40f62c6f89b4 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -8,6 +8,7 @@
*/
#include <linux/bitfield.h>
+#include <linux/cleanup.h>
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/overflow.h>
@@ -354,21 +355,15 @@ static int adxl313_set_odr(struct adxl313_data *data,
static int adxl313_read_axis(struct adxl313_data *data,
struct iio_chan_spec const *chan)
{
- int ret;
-
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
- ret = regmap_bulk_read(data->regmap,
+ int ret = regmap_bulk_read(data->regmap,
ADXL313_REG_DATA_AXIS(chan->address),
&data->transf_buf, sizeof(data->transf_buf));
if (ret)
- goto unlock_ret;
-
- ret = le16_to_cpu(data->transf_buf);
+ return ret;
-unlock_ret:
- mutex_unlock(&data->lock);
- return ret;
+ return le16_to_cpu(data->transf_buf);
}
static int adxl313_read_freq_avail(struct iio_dev *indio_dev,
--
2.53.0On Sat, 7 Mar 2026 15:47:56 +0530
Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote:
> Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
> adxl313_read_axis(). This ensures the mutex is released on all
> return paths and allows returning directly without a goto label.
>
> v3: Return directly from regmap_bulk_read error path.
> v2: Split into separate patch per driver.
This change log belongs..
>
> Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
> ---
.. here so that it doesn't get picked up when the patch is applied.
The history of how a patch was modified isn't normally something we want
in the git log.
One minor comment inline.
> drivers/iio/accel/adxl313_core.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
> index 9f5d4d2cb325..40f62c6f89b4 100644
> --- a/drivers/iio/accel/adxl313_core.c
> +++ b/drivers/iio/accel/adxl313_core.c
> @@ -8,6 +8,7 @@
> */
>
> #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> #include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/overflow.h>
> @@ -354,21 +355,15 @@ static int adxl313_set_odr(struct adxl313_data *data,
> static int adxl313_read_axis(struct adxl313_data *data,
> struct iio_chan_spec const *chan)
> {
> - int ret;
> -
> - mutex_lock(&data->lock);
> + guard(mutex)(&data->lock);
>
> - ret = regmap_bulk_read(data->regmap,
> + int ret = regmap_bulk_read(data->regmap,
This is not a style used in the kernel. There are exceptions where we declare
variables inline, but those are mainly about __free() so where a destructor and
constructor want to be visible together. That doesn't apply here so please
leave the declaration of ret at the top.
Thanks,
Jonathan
> ADXL313_REG_DATA_AXIS(chan->address),
> &data->transf_buf, sizeof(data->transf_buf));
> if (ret)
> - goto unlock_ret;
> -
> - ret = le16_to_cpu(data->transf_buf);
> + return ret;
>
> -unlock_ret:
> - mutex_unlock(&data->lock);
> - return ret;
> + return le16_to_cpu(data->transf_buf);
> }
>
> static int adxl313_read_freq_avail(struct iio_dev *indio_dev,
Replace manual mutex_lock/mutex_unlock pairs with guard(mutex) in
adxl355_data_rdy_trigger_set_state(), adxl355_set_odr(),
adxl355_set_hpf_3db() and adxl355_set_calibbias(). Remove all
goto labels and return directly on error paths.
v3: Remove all remaining gotos and return directly where possible.
v2: Split into separate patch per driver.
Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
---
drivers/iio/accel/adxl355_core.c | 81 ++++++++++++++------------------
1 file changed, 34 insertions(+), 47 deletions(-)
diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
index 1c1d64d5cbcb..af606e2ab8d4 100644
--- a/drivers/iio/accel/adxl355_core.c
+++ b/drivers/iio/accel/adxl355_core.c
@@ -9,6 +9,7 @@
#include <linux/bits.h>
#include <linux/bitfield.h>
+#include <linux/cleanup.h>
#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
#include <linux/iio/trigger.h>
@@ -261,16 +262,13 @@ static int adxl355_data_rdy_trigger_set_state(struct iio_trigger *trig,
{
struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
struct adxl355_data *data = iio_priv(indio_dev);
- int ret;
- mutex_lock(&data->lock);
- ret = regmap_update_bits(data->regmap, ADXL355_POWER_CTL_REG,
+ guard(mutex)(&data->lock);
+
+ return regmap_update_bits(data->regmap, ADXL355_POWER_CTL_REG,
ADXL355_POWER_CTL_DRDY_MSK,
FIELD_PREP(ADXL355_POWER_CTL_DRDY_MSK,
state ? 0 : 1));
- mutex_unlock(&data->lock);
-
- return ret;
}
static void adxl355_fill_3db_frequency_table(struct adxl355_data *data)
@@ -409,38 +407,34 @@ static int adxl355_set_odr(struct adxl355_data *data,
{
int ret;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
if (data->odr == odr) {
- mutex_unlock(&data->lock);
return 0;
}
ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
if (ret)
- goto err_unlock;
+ return ret;
ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
ADXL355_FILTER_ODR_MSK,
FIELD_PREP(ADXL355_FILTER_ODR_MSK, odr));
- if (ret)
- goto err_set_opmode;
+ if (ret){
+ adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+ return ret;
+ }
data->odr = odr;
adxl355_fill_3db_frequency_table(data);
ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
- if (ret)
- goto err_set_opmode;
+ if (ret){
+ adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+ return ret;
+ }
- mutex_unlock(&data->lock);
return 0;
-
-err_set_opmode:
- adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
-err_unlock:
- mutex_unlock(&data->lock);
- return ret;
}
static int adxl355_set_hpf_3db(struct adxl355_data *data,
@@ -448,37 +442,33 @@ static int adxl355_set_hpf_3db(struct adxl355_data *data,
{
int ret;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
if (data->hpf_3db == hpf) {
- mutex_unlock(&data->lock);
return 0;
}
ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
if (ret)
- goto err_unlock;
+ return ret;
ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
ADXL355_FILTER_HPF_MSK,
FIELD_PREP(ADXL355_FILTER_HPF_MSK, hpf));
- if (ret)
- goto err_set_opmode;
+ if (ret){
+ adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+ return ret;
+ }
data->hpf_3db = hpf;
ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
- if (ret)
- goto err_set_opmode;
+ if (ret){
+ adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+ return ret;
+ }
- mutex_unlock(&data->lock);
return 0;
-
-err_set_opmode:
- adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
-err_unlock:
- mutex_unlock(&data->lock);
- return ret;
}
static int adxl355_set_calibbias(struct adxl355_data *data,
@@ -486,33 +476,30 @@ static int adxl355_set_calibbias(struct adxl355_data *data,
{
int ret;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
if (ret)
- goto err_unlock;
+ return ret;
put_unaligned_be16(calibbias, data->transf_buf);
ret = regmap_bulk_write(data->regmap,
adxl355_chans[chan].offset_reg,
data->transf_buf, 2);
- if (ret)
- goto err_set_opmode;
+ if (ret){
+ adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+ return ret;
+ }
data->calibbias[chan] = calibbias;
ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
- if (ret)
- goto err_set_opmode;
+ if (ret){
+ adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+ return ret;
+ }
- mutex_unlock(&data->lock);
return 0;
-
-err_set_opmode:
- adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
-err_unlock:
- mutex_unlock(&data->lock);
- return ret;
}
static int adxl355_read_raw(struct iio_dev *indio_dev,
--
2.53.0On Sat, 7 Mar 2026 15:47:57 +0530
Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote:
> Replace manual mutex_lock/mutex_unlock pairs with guard(mutex) in
> adxl355_data_rdy_trigger_set_state(), adxl355_set_odr(),
> adxl355_set_hpf_3db() and adxl355_set_calibbias(). Remove all
> goto labels and return directly on error paths.
>
> v3: Remove all remaining gotos and return directly where possible.
> v2: Split into separate patch per driver.
Same issue for the change log needing to be below the ---
>
> Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
> ---
> drivers/iio/accel/adxl355_core.c | 81 ++++++++++++++------------------
> 1 file changed, 34 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
> index 1c1d64d5cbcb..af606e2ab8d4 100644
> --- a/drivers/iio/accel/adxl355_core.c
> +++ b/drivers/iio/accel/adxl355_core.c
>
...
>
> static void adxl355_fill_3db_frequency_table(struct adxl355_data *data)
> @@ -409,38 +407,34 @@ static int adxl355_set_odr(struct adxl355_data *data,
> {
> int ret;
>
> - mutex_lock(&data->lock);
> + guard(mutex)(&data->lock);
>
> if (data->odr == odr) {
> - mutex_unlock(&data->lock);
> return 0;
> }
>
> ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
> if (ret)
> - goto err_unlock;
> + return ret;
>
> ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
> ADXL355_FILTER_ODR_MSK,
> FIELD_PREP(ADXL355_FILTER_ODR_MSK, odr));
> - if (ret)
> - goto err_set_opmode;
> + if (ret){
> + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
See below.
> + return ret;
> + }
>
> data->odr = odr;
> adxl355_fill_3db_frequency_table(data);
>
> ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> - if (ret)
> - goto err_set_opmode;
> + if (ret){
> + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> + return ret;
> + }
>
> - mutex_unlock(&data->lock);
> return 0;
> -
> -err_set_opmode:
> - adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> -err_unlock:
> - mutex_unlock(&data->lock);
> - return ret;
> }
>
> static int adxl355_set_hpf_3db(struct adxl355_data *data,
> @@ -448,37 +442,33 @@ static int adxl355_set_hpf_3db(struct adxl355_data *data,
> {
> int ret;
>
> - mutex_lock(&data->lock);
> + guard(mutex)(&data->lock);
>
> if (data->hpf_3db == hpf) {
> - mutex_unlock(&data->lock);
> return 0;
> }
>
> ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
> if (ret)
> - goto err_unlock;
> + return ret;
>
> ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
> ADXL355_FILTER_HPF_MSK,
> FIELD_PREP(ADXL355_FILTER_HPF_MSK, hpf));
> - if (ret)
> - goto err_set_opmode;
> + if (ret){
> + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
Similar to below.
> + return ret;
> + }
>
> data->hpf_3db = hpf;
>
> ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> - if (ret)
> - goto err_set_opmode;
> + if (ret){
> + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> + return ret;
> + }
>
> - mutex_unlock(&data->lock);
> return 0;
> -
> -err_set_opmode:
> - adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> -err_unlock:
> - mutex_unlock(&data->lock);
> - return ret;
> }
>
> static int adxl355_set_calibbias(struct adxl355_data *data,
> @@ -486,33 +476,30 @@ static int adxl355_set_calibbias(struct adxl355_data *data,
> {
> int ret;
>
> - mutex_lock(&data->lock);
> + guard(mutex)(&data->lock);
>
> ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
> if (ret)
> - goto err_unlock;
> + return ret;
>
> put_unaligned_be16(calibbias, data->transf_buf);
> ret = regmap_bulk_write(data->regmap,
> adxl355_chans[chan].offset_reg,
> data->transf_buf, 2);
> - if (ret)
> - goto err_set_opmode;
> + if (ret){
Whilst this does answer the feedback you got on v2 wrt to not mixing
gotos (see comments on this in cleanup.h) it leads to inelegant code
due to the duplication.
There are a couple of techniques to avoid this. The most appropriate
one here is to use a helper function called something like
do_adxl355_set_calibbias() which has all the code
done between set_op_mode and unsetting it.
Then you can unconditionally unset the op_mode before checking
if there as an error. You will need to be a little clever with
ret code handling though.
> + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> + return ret;
> + }
>
> data->calibbias[chan] = calibbias;
>
> ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> - if (ret)
> - goto err_set_opmode;
> + if (ret){
Run checkpatch.pl over your patch. Should be a space before that {
> + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> + return ret;
> + }
>
> - mutex_unlock(&data->lock);
> return 0;
> -
> -err_set_opmode:
> - adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> -err_unlock:
> - mutex_unlock(&data->lock);
> - return ret;
> }
>
> static int adxl355_read_raw(struct iio_dev *indio_dev,
On Sat, 7 Mar 2026 10:39:49 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Sat, 7 Mar 2026 15:47:57 +0530
> Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote:
>
> > Replace manual mutex_lock/mutex_unlock pairs with guard(mutex) in
> > adxl355_data_rdy_trigger_set_state(), adxl355_set_odr(),
> > adxl355_set_hpf_3db() and adxl355_set_calibbias(). Remove all
> > goto labels and return directly on error paths.
> >
> > v3: Remove all remaining gotos and return directly where possible.
> > v2: Split into separate patch per driver.
>
> Same issue for the change log needing to be below the ---
One more thing. Sometimes guard() brings enough extra complexity it isn't
the right tool for the job. Whilst you can explore the suggestions for
helper functions I make below, I'm not sure they will lead to easier to
read and less error prone code. So it might not be a good idea to use
them in this driver at all!
Jonathan
>
>
> >
> > Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
> > ---
> > drivers/iio/accel/adxl355_core.c | 81 ++++++++++++++------------------
> > 1 file changed, 34 insertions(+), 47 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
> > index 1c1d64d5cbcb..af606e2ab8d4 100644
> > --- a/drivers/iio/accel/adxl355_core.c
> > +++ b/drivers/iio/accel/adxl355_core.c
> >
>
> ...
>
> >
> > static void adxl355_fill_3db_frequency_table(struct adxl355_data *data)
> > @@ -409,38 +407,34 @@ static int adxl355_set_odr(struct adxl355_data *data,
> > {
> > int ret;
> >
> > - mutex_lock(&data->lock);
> > + guard(mutex)(&data->lock);
> >
> > if (data->odr == odr) {
> > - mutex_unlock(&data->lock);
> > return 0;
> > }
> >
> > ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
> > if (ret)
> > - goto err_unlock;
> > + return ret;
> >
> > ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
> > ADXL355_FILTER_ODR_MSK,
> > FIELD_PREP(ADXL355_FILTER_ODR_MSK, odr));
> > - if (ret)
> > - goto err_set_opmode;
> > + if (ret){
> > + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> See below.
>
> > + return ret;
> > + }
> >
> > data->odr = odr;
> > adxl355_fill_3db_frequency_table(data);
> >
> > ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > - if (ret)
> > - goto err_set_opmode;
> > + if (ret){
> > + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > + return ret;
> > + }
> >
> > - mutex_unlock(&data->lock);
> > return 0;
> > -
> > -err_set_opmode:
> > - adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > -err_unlock:
> > - mutex_unlock(&data->lock);
> > - return ret;
> > }
> >
> > static int adxl355_set_hpf_3db(struct adxl355_data *data,
> > @@ -448,37 +442,33 @@ static int adxl355_set_hpf_3db(struct adxl355_data *data,
> > {
> > int ret;
> >
> > - mutex_lock(&data->lock);
> > + guard(mutex)(&data->lock);
> >
> > if (data->hpf_3db == hpf) {
> > - mutex_unlock(&data->lock);
> > return 0;
> > }
> >
> > ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
> > if (ret)
> > - goto err_unlock;
> > + return ret;
> >
> > ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
> > ADXL355_FILTER_HPF_MSK,
> > FIELD_PREP(ADXL355_FILTER_HPF_MSK, hpf));
> > - if (ret)
> > - goto err_set_opmode;
> > + if (ret){
> > + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
>
> Similar to below.
>
> > + return ret;
> > + }
> >
> > data->hpf_3db = hpf;
> >
> > ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > - if (ret)
> > - goto err_set_opmode;
> > + if (ret){
> > + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > + return ret;
> > + }
> >
> > - mutex_unlock(&data->lock);
> > return 0;
> > -
> > -err_set_opmode:
> > - adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > -err_unlock:
> > - mutex_unlock(&data->lock);
> > - return ret;
> > }
> >
> > static int adxl355_set_calibbias(struct adxl355_data *data,
> > @@ -486,33 +476,30 @@ static int adxl355_set_calibbias(struct adxl355_data *data,
> > {
> > int ret;
> >
> > - mutex_lock(&data->lock);
> > + guard(mutex)(&data->lock);
> >
> > ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
> > if (ret)
> > - goto err_unlock;
> > + return ret;
> >
> > put_unaligned_be16(calibbias, data->transf_buf);
> > ret = regmap_bulk_write(data->regmap,
> > adxl355_chans[chan].offset_reg,
> > data->transf_buf, 2);
> > - if (ret)
> > - goto err_set_opmode;
> > + if (ret){
> Whilst this does answer the feedback you got on v2 wrt to not mixing
> gotos (see comments on this in cleanup.h) it leads to inelegant code
> due to the duplication.
>
> There are a couple of techniques to avoid this. The most appropriate
> one here is to use a helper function called something like
> do_adxl355_set_calibbias() which has all the code
> done between set_op_mode and unsetting it.
>
> Then you can unconditionally unset the op_mode before checking
> if there as an error. You will need to be a little clever with
> ret code handling though.
>
>
> > + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > + return ret;
> > + }
> >
> > data->calibbias[chan] = calibbias;
> >
> > ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > - if (ret)
> > - goto err_set_opmode;
> > + if (ret){
>
> Run checkpatch.pl over your patch. Should be a space before that {
>
> > + adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > + return ret;
> > + }
> >
> > - mutex_unlock(&data->lock);
> > return 0;
> > -
> > -err_set_opmode:
> > - adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> > -err_unlock:
> > - mutex_unlock(&data->lock);
> > - return ret;
> > }
> >
> > static int adxl355_read_raw(struct iio_dev *indio_dev,
>
Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
adxl372_write_threshold_value(). Remove goto label and return
directly on error path.
v3: Return directly from error path without goto.
v2: Split into separate patch per driver.
Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
---
drivers/iio/accel/adxl372.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 28a8793a53b6..340433782cf2 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -7,6 +7,7 @@
#include <linux/bitfield.h>
#include <linux/bitops.h>
+#include <linux/cleanup.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/module.h>
@@ -334,20 +335,15 @@ static ssize_t adxl372_write_threshold_value(struct iio_dev *indio_dev, unsigned
u16 threshold)
{
struct adxl372_state *st = iio_priv(indio_dev);
- int ret;
- mutex_lock(&st->threshold_m);
- ret = regmap_write(st->regmap, addr, ADXL372_THRESH_VAL_H_SEL(threshold));
+ guard(mutex)(&st->threshold_m);
+
+ int ret = regmap_write(st->regmap, addr, ADXL372_THRESH_VAL_H_SEL(threshold));
if (ret < 0)
- goto unlock;
+ return ret;
- ret = regmap_update_bits(st->regmap, addr + 1, GENMASK(7, 5),
+ return regmap_update_bits(st->regmap, addr + 1, GENMASK(7, 5),
ADXL372_THRESH_VAL_L_SEL(threshold) << 5);
-
-unlock:
- mutex_unlock(&st->threshold_m);
-
- return ret;
}
static int adxl372_read_axis(struct adxl372_state *st, u8 addr)
--
2.53.0On Sat, 7 Mar 2026 15:47:58 +0530
Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote:
> Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
> adxl372_write_threshold_value(). Remove goto label and return
> directly on error path.
>
> v3: Return directly from error path without goto.
> v2: Split into separate patch per driver.
As in previous, this needs to move down.
Otherwise this patch is fine.
Thanks,
Jonathan
>
> Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
> ---
> drivers/iio/accel/adxl372.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index 28a8793a53b6..340433782cf2 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -7,6 +7,7 @@
>
> #include <linux/bitfield.h>
> #include <linux/bitops.h>
> +#include <linux/cleanup.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> #include <linux/module.h>
> @@ -334,20 +335,15 @@ static ssize_t adxl372_write_threshold_value(struct iio_dev *indio_dev, unsigned
> u16 threshold)
> {
> struct adxl372_state *st = iio_priv(indio_dev);
> - int ret;
>
> - mutex_lock(&st->threshold_m);
> - ret = regmap_write(st->regmap, addr, ADXL372_THRESH_VAL_H_SEL(threshold));
> + guard(mutex)(&st->threshold_m);
> +
> + int ret = regmap_write(st->regmap, addr, ADXL372_THRESH_VAL_H_SEL(threshold));
> if (ret < 0)
> - goto unlock;
> + return ret;
>
> - ret = regmap_update_bits(st->regmap, addr + 1, GENMASK(7, 5),
> + return regmap_update_bits(st->regmap, addr + 1, GENMASK(7, 5),
> ADXL372_THRESH_VAL_L_SEL(threshold) << 5);
> -
> -unlock:
> - mutex_unlock(&st->threshold_m);
> -
> - return ret;
> }
>
> static int adxl372_read_axis(struct adxl372_state *st, u8 addr)
© 2016 - 2026 Red Hat, Inc.