Re: [PATCH v3 2/3] iio: accel: adxl355: convert to guard(mutex)

[Jonathan Cameron <jic23@kernel.org>]

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


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