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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241126182919.2b0aabb6@jic23-huawei>
Date: Tue, 26 Nov 2024 18:29:19 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Guillaume Stols <gstols@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Nuno Sa <nuno.sa@...log.com>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, dlechner@...libre.com, jstephan@...libre.com,
 aardelean@...libre.com, adureghello@...libre.com
Subject: Re: [PATCH 4/9] iio: adc: ad7606: Move software functions into
 common file

On Thu, 21 Nov 2024 10:18:26 +0000
Guillaume Stols <gstols@...libre.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@...libre.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.

> +}