Re: [PATCH v3 10/10] iio: adc: ad7606: add support for writing registers when using backend

[Jonathan Cameron]

On Wed, 29 Jan 2025 12:03:11 +0100
Angelo Dureghello <adureghello@xxxxxxxxxxxx> wrote:

> From: Guillaume Stols <gstols@xxxxxxxxxxxx>
> 
> Add 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@xxxxxxxxxxxx>
> Co-developed-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
> Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>

A few trivial things inline.

> diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
> index 64733b607aa8..19d93ae49e1d 100644
> --- a/drivers/iio/adc/ad7606_par.c
> +++ b/drivers/iio/adc/ad7606_par.c
> @@ -19,6 +19,7 @@

>  
> -static int ad7606_bi_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask)
> +static const struct iio_chan_spec ad7606b_bi_sw_channels[] = {
> +	AD7606_BI_SW_CHANNEL(0),
> +	AD7606_BI_SW_CHANNEL(1),
> +	AD7606_BI_SW_CHANNEL(2),
> +	AD7606_BI_SW_CHANNEL(3),
> +	AD7606_BI_SW_CHANNEL(4),
> +	AD7606_BI_SW_CHANNEL(5),
> +	AD7606_BI_SW_CHANNEL(6),
> +	AD7606_BI_SW_CHANNEL(7),
> +};
> +
> +static int ad7606_par_bus_update_scan_mode(struct iio_dev *indio_dev,
> +					   const unsigned long *scan_mask)
>  {
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  	unsigned int c, ret;
> @@ -48,7 +61,8 @@ static int ad7606_bi_update_scan_mode(struct iio_dev *indio_dev, const unsigned
>  	return 0;
>  }
>  
> -static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio_dev)
> +static int ad7606_par_bus_setup_iio_backend(struct device *dev,
> +					    struct iio_dev *indio_dev)
>  {
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  	unsigned int ret, c;
> @@ -86,9 +100,56 @@ static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio
>  	return 0;
>  }
>  
> +static int ad7606_par_bus_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;
Bonus space before st that shouldn't be there.

I'd also reorder this to have int val, ret last.

> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = pdata->bus_reg_read(st->back, addr, &val);
> +
> +	iio_device_release_direct_mode(indio_dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return val;
> +}
> +
> +static int ad7606_par_bus_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;

Same bonus space.

> +	int ret;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = pdata->bus_reg_write(st->back, addr, val);
> +
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}