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

Jonathan Cameron
2024-11-26T18:29:28Z

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.

> +}