Re: [PATCH 3/9] iio:adc: ad7606: Move the software mode configuration

[Jonathan Cameron <jic23@kernel.org>]

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

> This is a preparation for the intoduction of the sofware functions in
> the iio backend version of the driver.
> The software mode configuration must be executed once the channels are
> configured, and the number of channels is known. This is not the case
> before iio-backend's configuration is called, and iio backend version of
> the driver does not have a timestamp channel.
> Also the sw_mode_config callback is configured during the
> iio-backend configuration.
> For clarity purpose, I moved the entire block instead of just the
> concerned function calls.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
>  drivers/iio/adc/ad7606.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 893b93b86aa7..828603ed18f6 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -1246,17 +1246,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  			return -ERESTARTSYS;
>  	}
>  
> -	st->write_scale = ad7606_write_scale_hw;
> -	st->write_os = ad7606_write_os_hw;
> -
> -	ret = ad7606_sw_mode_setup(indio_dev);
Isn't this the only call to this function?

> -	if (ret)
> -		return ret;
> -
> -	ret = ad7606_chan_scales_setup(indio_dev);
> -	if (ret)
> -		return ret;
> -
>  	/* If convst pin is not defined, setup PWM. */
>  	if (!st->gpio_convst) {
>  		st->cnvst_pwm = devm_pwm_get(dev, NULL);
> @@ -1334,6 +1323,20 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  			return ret;
>  	}
>  
> +	st->write_scale = ad7606_write_scale_hw;
> +	st->write_os = ad7606_write_os_hw;
> +
> +	st->sw_mode_en = st->chip_info->sw_setup_cb &&
> +			 device_property_present(st->dev, "adi,sw-mode");
> +	if (st->sw_mode_en) {
> +		indio_dev->info = &ad7606_info_sw_mode;
> +		st->chip_info->sw_setup_cb(indio_dev);

Where did this callback come from?

Looks like this and the next patch need tidying up so each does
one distinct thing.

> +	}
> +
> +	ret = ad7606_chan_scales_setup(indio_dev);
> +	if (ret)
> +		return ret;
> +
>  	return devm_iio_device_register(dev, indio_dev);
>  }
>  EXPORT_SYMBOL_NS_GPL(ad7606_probe, IIO_AD7606);
>