Re: [PATCH v2 3/4] iio: adc: ti-ads7950: switch to using guard() notation

[Dmitry Torokhov]

On Sat, Feb 21, 2026 at 11:34:33AM -0600, David Lechner wrote:
> On 2/18/26 8:29 PM, Dmitry Torokhov wrote:
> > +	scoped_guard(mutex, &st->slock) {
> > +		error = spi_sync(st->spi, &st->ring_msg);
> > +		if (error)
> > +			break;
> 
> I'm not a fan of scoped_guard() because of the hidden for loop in it.
> It hides the fact that the break; is breaking out of that for loop.
> 
> It would be more clear/obvious written as:
> 
> 	do {
> 		guard(mutex)(&st->slock);
> 
> 		ret = spi_sync(st->spi, &st->ring_msg);
> 		if (ret)
> 			break;
> 
> 		iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf[2],
> 						   iio_get_time_ns(indio_dev));
> 	} while (0);

OK.

I could also make it

	scoped_guard(mutex, &st->slock) {
		ret = spi_sync(st->spi, &st->ring_msg);
		if (!ret)
			iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf[2],
							   iio_get_time_ns(indio_dev));
	}

to avoid using "break".

I think you will find that scoped_guard() will gain the foothold in the
kernel so having implementation that does not follow common pattern
might not be the best option.

> >  
> >  	/* If set as output, return the output */
> >  	if (st->gpio_cmd_settings_bitmask & BIT(offset)) {
> >  		state = st->cmd_settings_bitmask & BIT(offset);
> > -		goto out;
> > +		return state;
> 
> This can return directly instead of using local variable.

This will require the explicitly normalizing, which we avoided by
introducing "bool state" to begin with...

> >  
> >  	st->single_tx = TI_ADS7950_GPIO_CMD_SETTINGS(st);
> > -	ret = spi_sync(st->spi, &st->scan_single_msg);
> > +	error = spi_sync(st->spi, &st->scan_single_msg);
> 
> Can just return directly here now.

I think there is benefit in explicitly calling out the error paths and
explicitly return 0 on success. It removes the doubt whether a function
can return positive value on success.

Thanks.

-- 
Dmitry