Re: [PATCH 9/9] iio: adc: ad7606: Add support for writing registers when using backend

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 21 Nov 2024 10:18:31 +0000
Guillaume Stols <gstols@baylibre.com> wrote:

> Adds the logic for effectively enabling the software mode for the
> iio-backend, i.e enabling the software mode channel configuration and
> implementing the register writing functions.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>

A few comments inline, but basically looks fine to me.

Thanks,

Jonathan

> diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
> index a25182a3daa7..0c1177f436f3 100644
> --- a/drivers/iio/adc/ad7606_par.c
> +++ b/drivers/iio/adc/ad7606_par.c

>  static int ad7606_bi_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask)
>  {
>  	struct ad7606_state *st = iio_priv(indio_dev);
> @@ -70,7 +83,7 @@ static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_iio_backend_enable(dev, st->back);
> +	ret = devm_iio_backend_enable(st->dev, st->back);

Is that a different dev? That's not obvious...

>  	if (ret)
>  		return ret;
>  
> @@ -86,9 +99,52 @@ static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio
>  	return 0;
>  }
>  
> +static int ad7606_bi_reg_read(struct iio_dev *indio_dev, unsigned int addr)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +	struct ad7606_platform_data *pdata =  st->dev->platform_data;
> +
> +	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> +		ret = pdata->bus_reg_read(st->back,
> +					addr,
> +					&val);

As below.

> +	}
> +	if (ret < 0)
> +		return ret;
> +
> +	return val;
> +}
> +
> +static int ad7606_bi_reg_write(struct iio_dev *indio_dev,
> +			       unsigned int addr,
> +			       unsigned int val)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +	struct ad7606_platform_data *pdata =  st->dev->platform_data;
> +	int ret;
> +
> +	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {

Given David's if_not_cond_guard() should land shortly I'd prefer
to use that going forwards for cases like this.

> +	ret = pdata->bus_reg_write(st->back,
> +					addr,
> +					val);
Put parameters all on one line.
+ return here (which needs the new if_not_cond_guard() to avoid
confusing the compiler).

> +	}
> +	return ret;
> +}