Re: [PATCH 4/9] iio: adc: ad7606: Move software functions into common file

[Jonathan Cameron <jic23@kernel.org>]

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

> Since the register are always the same, whatever bus is used, moving the
> software functions into the main file avoids the code to be duplicated
> in both SPI and parallel version of the driver.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
>  drivers/iio/adc/ad7606.c     | 128 ++++++++++++++++++++++++++++++++++++++++--
>  drivers/iio/adc/ad7606.h     |  37 ++++++++++--
>  drivers/iio/adc/ad7606_spi.c | 131 +------------------------------------------
>  3 files changed, 156 insertions(+), 140 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 828603ed18f6..df0e49bc4bdb 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -85,6 +85,10 @@ static const unsigned int ad7606_oversampling_avail[7] = {
>  	1, 2, 4, 8, 16, 32, 64,
>  };
>  
> +static const unsigned int ad7606B_oversampling_avail[9] = {

Same in original code, but why capital B?

I think you didn't remove this as intended from ad7606_spi.c
so we have a duplicate.


> +	1, 2, 4, 8, 16, 32, 64, 128, 256
> +};
> +
>  static const unsigned int ad7616_oversampling_avail[8] = {
>  	1, 2, 4, 8, 16, 32, 64, 128,
>  };
> @@ -187,6 +191,8 @@ static int ad7608_chan_scale_setup(struct iio_dev *indio_dev,
>  				   struct iio_chan_spec *chan, int ch);
>  static int ad7609_chan_scale_setup(struct iio_dev *indio_dev,
>  				   struct iio_chan_spec *chan, int ch);
> +static int ad7616_sw_mode_setup(struct iio_dev *indio_dev);
> +static int ad7606B_sw_mode_setup(struct iio_dev *indio_dev);
Similar question. Why capital B?  We make ad lowercase, so I'd think it makes
sense for the B as well.



> +
> +static int ad7616_write_os_sw(struct iio_dev *indio_dev, int val)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +
> +	return ad7606_write_mask(st, AD7616_CONFIGURATION_REGISTER,
> +				     AD7616_OS_MASK, val << 2);
> +}
> +
> +static int ad7606_write_scale_sw(struct iio_dev *indio_dev, int ch, int val)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +
> +	return ad7606_write_mask(st,
> +				     AD7606_RANGE_CH_ADDR(ch),
> +				     AD7606_RANGE_CH_MSK(ch),
> +				     AD7606_RANGE_CH_MODE(ch, val));

Odd alignment.

> +}