Re: [PATCH 02/10] iio: dummy: Use automatic lock and direct mode cleanup.

From: andy.shevchenko@gmail.com
To: Jonathan Cameron <jic23@kernel.org>
Cc: linux-iio@vger.kernel.org, David Lechner <dlechner@baylibre.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>
Subject: Re: [PATCH 02/10] iio: dummy: Use automatic lock and direct mode cleanup.
Date: Sun, 4 Feb 2024 18:29:25 +0200	[thread overview]
Message-ID: <Zb-7ZUbZ65G-LoNW@surfacebook.localdomain> (raw)
In-Reply-To: <20240128150537.44592-3-jic23@kernel.org>

Sun, Jan 28, 2024 at 03:05:29PM +0000, Jonathan Cameron kirjoitti:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Given we now have iio_device_claim_direct_scoped() to perform automatic
> releasing of direct mode at exit from the scope that follows it, this can
> be used in conjunction with guard(mutex) etc remove a lot of special case
> handling.
> 
> Note that in this particular example code, there is no real reason you can't
> read channels via sysfs at the same time as filling the software buffer.
> To make it look more like a real driver constrain raw and processed
> channel reads from occurring whilst the buffer is in use.

...

> +		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> +			guard(mutex)(&st->lock);
> +			switch (chan->type) {
> +			case IIO_VOLTAGE:
> +				if (chan->output) {
> +					/* Set integer part to cached value */
> +					*val = st->dac_val;
> +					return IIO_VAL_INT;
> +				} else if (chan->differential) {
> +					if (chan->channel == 1)
> +						*val = st->differential_adc_val[0];
> +					else
> +						*val = st->differential_adc_val[1];
> +					return IIO_VAL_INT;
> +				} else {
> +					*val = st->single_ended_adc_val;
> +					return IIO_VAL_INT;
> +				}

Now you may go further and use only single return statement here.

> +			case IIO_ACCEL:
> +				*val = st->accel_val;
> +				return IIO_VAL_INT;
> +			default:
> +				return -EINVAL;
>  			}

...

> +		unreachable();

Hmm... Is it really required? Why?

...

P.S> I hope you are using --histogram diff algo when preparing patches.

-- 
With Best Regards,
Andy Shevchenko