Re: [PATCH v2] iio: gyro: adxrs290: Use guard(mutex) in lieu of manual lock+unlock

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 24 Apr 2026 12:24:07 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Apr 23, 2026 at 07:37:51PM -0300, Guilherme Dias wrote:
> > Use guard(mutex) to automatically release the lock on scope exit,
> > simplifying the error handling path and removing the need for
> > explicit unlock and goto-based cleanup.  
> 
> ...
> 
> >  static int adxrs290_get_rate_data(struct iio_dev *indio_dev, const u8 cmd, int *val)
> >  {
> >  	struct adxrs290_state *st = iio_priv(indio_dev);
> > -	int ret = 0;
> >  	int temp;
> >  
> > -	mutex_lock(&st->lock);
> > +	guard(mutex)(&st->lock);  
> 
> + blank line.
> 
> We usually consider guard()() as not semantically linked to any code above
> or below
> 
> >  	temp = spi_w8r16(st->spi, cmd);
> > -	if (temp < 0) {
> > -		ret = temp;
> > -		goto err_unlock;
> > -	}
> > +	if (temp < 0)
> > +		return temp;
> >  
> >  	*val = sign_extend32(temp, 15);
> > -
> > -err_unlock:
> > -	mutex_unlock(&st->lock);
> > -	return ret;
> > +	return 0;
> >  }  
> 
> Ditto for the similar cases below.
> 
> ...
> 
> > static int adxrs290_set_mode(struct iio_dev *indio_dev, enum adxrs290_mode mode)  
> 
> >  	default:
> >  		ret = -EINVAL;
> > -		goto out_unlock;
> > +		return ret;  
> 
> Just return directly the given error code.
> 
> ...
> 
> > static irqreturn_t adxrs290_trigger_handler(int irq, void *p)  
> 
> > -	mutex_lock(&st->lock);
> > +	scoped_guard(mutex, &st->lock){  
> 
> No. Besides wrong style of this line, this has a very broken indentation now.
> 
> >  	/* exercise a bulk data capture starting from reg DATAX0... */
> >  	ret = spi_write_then_read(st->spi, &tx, sizeof(tx), st->buffer.channels,
> >  				  sizeof(st->buffer.channels));
> > -	if (ret < 0)
> > -		goto out_unlock_notify;
> > -
> > -	iio_push_to_buffers_with_timestamp(indio_dev, &st->buffer,
> > +	if (ret >= 0)
No to this as well. Keep the error out of line.  Which makes the use
of guard() here tricky.  You could do a do { guard(); } while(0) loop so that
you can use break with out it being odd looking.

(I don't like breaks out of scoped_guard() because it's non obvious scoped
guard is actually a loop.

> > +		iio_push_to_buffers_with_timestamp(indio_dev, &st->buffer,
> >  					   pf->timestamp);
> > +	}

> >  
> > -out_unlock_notify:
> > -	mutex_unlock(&st->lock);  
> 
> >  	iio_trigger_notify_done(indio_dev->trig);
> > -
> >  	return IRQ_HANDLED;  
> 
> Stray change.
> 
> >  }  
>