[PATCH 0/5] iio: adc: ad4030: scan_type cleanups

[PATCH 0/5] iio: adc: ad4030: scan_type cleanups

[David Lechner]

This started as patch to address an unchecked error return of
iio_get_current_scan_type(). Then while looking at other code related
to getting the scan_type, I noticed some opportunities to simplify the
driver a bit by removing some redundant code and clear up some things
that were not so obvious to me.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
David Lechner (5):
      iio: adc: ad4030: check scan_type for error
      iio: adc: ad4030: remove some duplicate code
      iio: adc: ad4030: move setting mode to update_scan_mode
      iio: adc: ad4030: don't store scan_type in state
      iio: adc: ad4030: explain rearranging raw sample data

 drivers/iio/adc/ad4030.c | 60 +++++++++++++++++++++++++-----------------------
 1 file changed, 31 insertions(+), 29 deletions(-)
---
base-commit: 66cadadbc94e18070245af7053f115061a73f016
change-id: 20250310-iio-adc-ad4030-check-scan-type-err-0858c3e3519c

Best regards,
-- 
David Lechner <dlechner@baylibre.com>

[PATCH 1/5] iio: adc: ad4030: check scan_type for error

[David Lechner]

Check scan_type for error ad4030_get_chan_scale(). Currently, this
should never fail, but it is good practice to always check for errors
in case of future changes.

Calling iio_get_current_scan_type() is moved out of the if statement
also to avoid potential issues with future changes instead of assuming
that the non-differential case does not use extended scan_type.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202503040954.n6MhjSsV-lkp@intel.com/
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad4030.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index 9a020680885d93f4da8922e5cfeecc0c7ce23f4d..af7a817e8273496e8856a5ba1a9c2e66a11f0a84 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -390,16 +390,18 @@ static int ad4030_get_chan_scale(struct iio_dev *indio_dev,
 	struct ad4030_state *st = iio_priv(indio_dev);
 	const struct iio_scan_type *scan_type;
 
+	scan_type = iio_get_current_scan_type(indio_dev, st->chip->channels);
+	if (IS_ERR(scan_type))
+		return PTR_ERR(scan_type);
+
 	if (chan->differential) {
-		scan_type = iio_get_current_scan_type(indio_dev,
-						      st->chip->channels);
 		*val = (st->vref_uv * 2) / MILLI;
 		*val2 = scan_type->realbits;
 		return IIO_VAL_FRACTIONAL_LOG2;
 	}
 
 	*val = st->vref_uv / MILLI;
-	*val2 = chan->scan_type.realbits;
+	*val2 = scan_type->realbits;
 	return IIO_VAL_FRACTIONAL_LOG2;
 }
 

-- 
2.43.0

Re: [PATCH 1/5] iio: adc: ad4030: check scan_type for error

[Nuno Sá]

On Mon, 2025-03-10 at 15:43 -0500, David Lechner wrote:
> Check scan_type for error ad4030_get_chan_scale(). Currently, this
> should never fail, but it is good practice to always check for errors
> in case of future changes.
> 
> Calling iio_get_current_scan_type() is moved out of the if statement
> also to avoid potential issues with future changes instead of assuming
> that the non-differential case does not use extended scan_type.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202503040954.n6MhjSsV-lkp@intel.com/
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/ad4030.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> index
> 9a020680885d93f4da8922e5cfeecc0c7ce23f4d..af7a817e8273496e8856a5ba1a9c2e66a11f
> 0a84 100644
> --- a/drivers/iio/adc/ad4030.c
> +++ b/drivers/iio/adc/ad4030.c
> @@ -390,16 +390,18 @@ static int ad4030_get_chan_scale(struct iio_dev
> *indio_dev,
>  	struct ad4030_state *st = iio_priv(indio_dev);
>  	const struct iio_scan_type *scan_type;
>  
> +	scan_type = iio_get_current_scan_type(indio_dev, st->chip->channels);
> +	if (IS_ERR(scan_type))
> +		return PTR_ERR(scan_type);
> +
>  	if (chan->differential) {
> -		scan_type = iio_get_current_scan_type(indio_dev,
> -						      st->chip->channels);
>  		*val = (st->vref_uv * 2) / MILLI;
>  		*val2 = scan_type->realbits;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	}
>  
>  	*val = st->vref_uv / MILLI;
> -	*val2 = chan->scan_type.realbits;
> +	*val2 = scan_type->realbits;
>  	return IIO_VAL_FRACTIONAL_LOG2;
>  }
>  
> 

[PATCH 2/5] iio: adc: ad4030: remove some duplicate code

[David Lechner]

Remove some duplicate code in the ad4030_get_chan_scale() function by
simplifying the if statement.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad4030.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index af7a817e8273496e8856a5ba1a9c2e66a11f0a84..f24b46164a477f9b6b5c93ffeba0a335f7b3de5a 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -394,14 +394,13 @@ static int ad4030_get_chan_scale(struct iio_dev *indio_dev,
 	if (IS_ERR(scan_type))
 		return PTR_ERR(scan_type);
 
-	if (chan->differential) {
+	if (chan->differential)
 		*val = (st->vref_uv * 2) / MILLI;
-		*val2 = scan_type->realbits;
-		return IIO_VAL_FRACTIONAL_LOG2;
-	}
+	else
+		*val = st->vref_uv / MILLI;
 
-	*val = st->vref_uv / MILLI;
 	*val2 = scan_type->realbits;
+
 	return IIO_VAL_FRACTIONAL_LOG2;
 }
 

-- 
2.43.0

Re: [PATCH 2/5] iio: adc: ad4030: remove some duplicate code

[Nuno Sá]

On Mon, 2025-03-10 at 15:43 -0500, David Lechner wrote:
> Remove some duplicate code in the ad4030_get_chan_scale() function by
> simplifying the if statement.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/ad4030.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> index
> af7a817e8273496e8856a5ba1a9c2e66a11f0a84..f24b46164a477f9b6b5c93ffeba0a335f7b3
> de5a 100644
> --- a/drivers/iio/adc/ad4030.c
> +++ b/drivers/iio/adc/ad4030.c
> @@ -394,14 +394,13 @@ static int ad4030_get_chan_scale(struct iio_dev
> *indio_dev,
>  	if (IS_ERR(scan_type))
>  		return PTR_ERR(scan_type);
>  
> -	if (chan->differential) {
> +	if (chan->differential)
>  		*val = (st->vref_uv * 2) / MILLI;
> -		*val2 = scan_type->realbits;
> -		return IIO_VAL_FRACTIONAL_LOG2;
> -	}
> +	else
> +		*val = st->vref_uv / MILLI;
>  
> -	*val = st->vref_uv / MILLI;
>  	*val2 = scan_type->realbits;
> +
>  	return IIO_VAL_FRACTIONAL_LOG2;
>  }
>  
> 

[PATCH 3/5] iio: adc: ad4030: move setting mode to update_scan_mode

[David Lechner]

Move calling ad4030_set_mode() from the buffer preenable callback to
the update_scan_mode callback. This doesn't change any functionality
but is more logical since setting the mode is a function of the scan
mask and doesn't require an "undo" operation when the buffer is
disabled.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad4030.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index f24b46164a477f9b6b5c93ffeba0a335f7b3de5a..c2117c7a296f22aeeec6911c8a8c74ed576296a0 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -868,6 +868,12 @@ static int ad4030_get_current_scan_type(const struct iio_dev *indio_dev,
 	return st->avg_log2 ? AD4030_SCAN_TYPE_AVG : AD4030_SCAN_TYPE_NORMAL;
 }
 
+static int ad4030_update_scan_mode(struct iio_dev *indio_dev,
+				   const unsigned long *scan_mask)
+{
+	return ad4030_set_mode(indio_dev, *scan_mask);
+}
+
 static const struct iio_info ad4030_iio_info = {
 	.read_avail = ad4030_read_avail,
 	.read_raw = ad4030_read_raw,
@@ -875,13 +881,9 @@ static const struct iio_info ad4030_iio_info = {
 	.debugfs_reg_access = ad4030_reg_access,
 	.read_label = ad4030_read_label,
 	.get_current_scan_type = ad4030_get_current_scan_type,
+	.update_scan_mode  = ad4030_update_scan_mode,
 };
 
-static int ad4030_buffer_preenable(struct iio_dev *indio_dev)
-{
-	return ad4030_set_mode(indio_dev, *indio_dev->active_scan_mask);
-}
-
 static bool ad4030_validate_scan_mask(struct iio_dev *indio_dev,
 				      const unsigned long *scan_mask)
 {
@@ -895,7 +897,6 @@ static bool ad4030_validate_scan_mask(struct iio_dev *indio_dev,
 }
 
 static const struct iio_buffer_setup_ops ad4030_buffer_setup_ops = {
-	.preenable = ad4030_buffer_preenable,
 	.validate_scan_mask = ad4030_validate_scan_mask,
 };
 

-- 
2.43.0

Re: [PATCH 3/5] iio: adc: ad4030: move setting mode to update_scan_mode

[Nuno Sá]

On Mon, 2025-03-10 at 15:43 -0500, David Lechner wrote:
> Move calling ad4030_set_mode() from the buffer preenable callback to
> the update_scan_mode callback. This doesn't change any functionality
> but is more logical since setting the mode is a function of the scan
> mask and doesn't require an "undo" operation when the buffer is
> disabled.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/ad4030.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> index
> f24b46164a477f9b6b5c93ffeba0a335f7b3de5a..c2117c7a296f22aeeec6911c8a8c74ed5762
> 96a0 100644
> --- a/drivers/iio/adc/ad4030.c
> +++ b/drivers/iio/adc/ad4030.c
> @@ -868,6 +868,12 @@ static int ad4030_get_current_scan_type(const struct
> iio_dev *indio_dev,
>  	return st->avg_log2 ? AD4030_SCAN_TYPE_AVG : AD4030_SCAN_TYPE_NORMAL;
>  }
>  
> +static int ad4030_update_scan_mode(struct iio_dev *indio_dev,
> +				   const unsigned long *scan_mask)
> +{
> +	return ad4030_set_mode(indio_dev, *scan_mask);
> +}
> +
>  static const struct iio_info ad4030_iio_info = {
>  	.read_avail = ad4030_read_avail,
>  	.read_raw = ad4030_read_raw,
> @@ -875,13 +881,9 @@ static const struct iio_info ad4030_iio_info = {
>  	.debugfs_reg_access = ad4030_reg_access,
>  	.read_label = ad4030_read_label,
>  	.get_current_scan_type = ad4030_get_current_scan_type,
> +	.update_scan_mode  = ad4030_update_scan_mode,
>  };
>  
> -static int ad4030_buffer_preenable(struct iio_dev *indio_dev)
> -{
> -	return ad4030_set_mode(indio_dev, *indio_dev->active_scan_mask);
> -}
> -
>  static bool ad4030_validate_scan_mask(struct iio_dev *indio_dev,
>  				      const unsigned long *scan_mask)
>  {
> @@ -895,7 +897,6 @@ static bool ad4030_validate_scan_mask(struct iio_dev
> *indio_dev,
>  }
>  
>  static const struct iio_buffer_setup_ops ad4030_buffer_setup_ops = {
> -	.preenable = ad4030_buffer_preenable,
>  	.validate_scan_mask = ad4030_validate_scan_mask,
>  };
>  
> 

[PATCH 4/5] iio: adc: ad4030: don't store scan_type in state

[David Lechner]

Move getting the scan_type to ad4030_conversion(). Previously, we were
getting the scan_type in two other places, then storing it in the
state struct before using it in ad4030_conversion(). This was a bit
fragile against potential future changes since it isn't obvious that
anything that could potentially change the scan_type would need to
also update the state struct. Also, the non-obviousness of this led to
a redundant call to iio_get_current_scan_type() in
ad4030_single_conversion() since it also calls ad4030_set_mode() which
in turn calls ad4030_conversion().

To simplify things, just call iio_get_current_scan_type() in
ad4030_conversion() where the returned struct is actually used and
don't bother storing it in the state struct.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad4030.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index c2117c7a296f22aeeec6911c8a8c74ed576296a0..54ad74b96c9f256a67848330f875379edc828b0b 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -147,7 +147,6 @@ struct ad4030_state {
 	struct spi_device *spi;
 	struct regmap *regmap;
 	const struct ad4030_chip_info *chip;
-	const struct iio_scan_type *current_scan_type;
 	struct gpio_desc *cnv_gpio;
 	int vref_uv;
 	int vio_uv;
@@ -562,11 +561,6 @@ static int ad4030_set_mode(struct iio_dev *indio_dev, unsigned long mask)
 		st->mode = AD4030_OUT_DATA_MD_DIFF;
 	}
 
-	st->current_scan_type = iio_get_current_scan_type(indio_dev,
-							  st->chip->channels);
-	if (IS_ERR(st->current_scan_type))
-		return PTR_ERR(st->current_scan_type);
-
 	return regmap_update_bits(st->regmap, AD4030_REG_MODES,
 				  AD4030_REG_MODES_MASK_OUT_DATA_MODE,
 				  st->mode);
@@ -614,15 +608,20 @@ static void ad4030_extract_interleaved(u8 *src, u32 *ch0, u32 *ch1)
 static int ad4030_conversion(struct iio_dev *indio_dev)
 {
 	struct ad4030_state *st = iio_priv(indio_dev);
-	unsigned char diff_realbytes =
-		BITS_TO_BYTES(st->current_scan_type->realbits);
-	unsigned char diff_storagebytes =
-		BITS_TO_BYTES(st->current_scan_type->storagebits);
+	const struct iio_scan_type *scan_type;
+	unsigned char diff_realbytes, diff_storagebytes;
 	unsigned int bytes_to_read;
 	unsigned long cnv_nb = BIT(st->avg_log2);
 	unsigned int i;
 	int ret;
 
+	scan_type = iio_get_current_scan_type(indio_dev, st->chip->channels);
+	if (IS_ERR(scan_type))
+		return PTR_ERR(scan_type);
+
+	diff_realbytes = BITS_TO_BYTES(scan_type->realbits);
+	diff_storagebytes = BITS_TO_BYTES(scan_type->storagebits);
+
 	/* Number of bytes for one differential channel */
 	bytes_to_read = diff_realbytes;
 	/* Add one byte if we are using a differential + common byte mode */
@@ -673,11 +672,6 @@ static int ad4030_single_conversion(struct iio_dev *indio_dev,
 	if (ret)
 		return ret;
 
-	st->current_scan_type = iio_get_current_scan_type(indio_dev,
-							  st->chip->channels);
-	if (IS_ERR(st->current_scan_type))
-		return PTR_ERR(st->current_scan_type);
-
 	ret = ad4030_conversion(indio_dev);
 	if (ret)
 		return ret;

-- 
2.43.0

Re: [PATCH 4/5] iio: adc: ad4030: don't store scan_type in state

[Nuno Sá]

On Mon, 2025-03-10 at 15:43 -0500, David Lechner wrote:
> Move getting the scan_type to ad4030_conversion(). Previously, we were
> getting the scan_type in two other places, then storing it in the
> state struct before using it in ad4030_conversion(). This was a bit
> fragile against potential future changes since it isn't obvious that
> anything that could potentially change the scan_type would need to
> also update the state struct. Also, the non-obviousness of this led to
> a redundant call to iio_get_current_scan_type() in
> ad4030_single_conversion() since it also calls ad4030_set_mode() which
> in turn calls ad4030_conversion().
> 
> To simplify things, just call iio_get_current_scan_type() in
> ad4030_conversion() where the returned struct is actually used and
> don't bother storing it in the state struct.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/ad4030.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> index
> c2117c7a296f22aeeec6911c8a8c74ed576296a0..54ad74b96c9f256a67848330f875379edc82
> 8b0b 100644
> --- a/drivers/iio/adc/ad4030.c
> +++ b/drivers/iio/adc/ad4030.c
> @@ -147,7 +147,6 @@ struct ad4030_state {
>  	struct spi_device *spi;
>  	struct regmap *regmap;
>  	const struct ad4030_chip_info *chip;
> -	const struct iio_scan_type *current_scan_type;
>  	struct gpio_desc *cnv_gpio;
>  	int vref_uv;
>  	int vio_uv;
> @@ -562,11 +561,6 @@ static int ad4030_set_mode(struct iio_dev *indio_dev,
> unsigned long mask)
>  		st->mode = AD4030_OUT_DATA_MD_DIFF;
>  	}
>  
> -	st->current_scan_type = iio_get_current_scan_type(indio_dev,
> -							  st->chip-
> >channels);
> -	if (IS_ERR(st->current_scan_type))
> -		return PTR_ERR(st->current_scan_type);
> -
>  	return regmap_update_bits(st->regmap, AD4030_REG_MODES,
>  				  AD4030_REG_MODES_MASK_OUT_DATA_MODE,
>  				  st->mode);
> @@ -614,15 +608,20 @@ static void ad4030_extract_interleaved(u8 *src, u32
> *ch0, u32 *ch1)
>  static int ad4030_conversion(struct iio_dev *indio_dev)
>  {
>  	struct ad4030_state *st = iio_priv(indio_dev);
> -	unsigned char diff_realbytes =
> -		BITS_TO_BYTES(st->current_scan_type->realbits);
> -	unsigned char diff_storagebytes =
> -		BITS_TO_BYTES(st->current_scan_type->storagebits);
> +	const struct iio_scan_type *scan_type;
> +	unsigned char diff_realbytes, diff_storagebytes;
>  	unsigned int bytes_to_read;
>  	unsigned long cnv_nb = BIT(st->avg_log2);
>  	unsigned int i;
>  	int ret;
>  
> +	scan_type = iio_get_current_scan_type(indio_dev, st->chip->channels);
> +	if (IS_ERR(scan_type))
> +		return PTR_ERR(scan_type);
> +
> +	diff_realbytes = BITS_TO_BYTES(scan_type->realbits);
> +	diff_storagebytes = BITS_TO_BYTES(scan_type->storagebits);
> +
>  	/* Number of bytes for one differential channel */
>  	bytes_to_read = diff_realbytes;
>  	/* Add one byte if we are using a differential + common byte mode */
> @@ -673,11 +672,6 @@ static int ad4030_single_conversion(struct iio_dev
> *indio_dev,
>  	if (ret)
>  		return ret;
>  
> -	st->current_scan_type = iio_get_current_scan_type(indio_dev,
> -							  st->chip-
> >channels);
> -	if (IS_ERR(st->current_scan_type))
> -		return PTR_ERR(st->current_scan_type);
> -
>  	ret = ad4030_conversion(indio_dev);
>  	if (ret)
>  		return ret;
> 

[PATCH 5/5] iio: adc: ad4030: explain rearranging raw sample data

[David Lechner]

Add a comment explaining why the raw sample data is rearranged in the
in the ad4030_conversion() function. It is not so obvious from the code
why this is done.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad4030.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index 54ad74b96c9f256a67848330f875379edc828b0b..636f9f33e66af73d102722b984dc1230e1417d1e 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -646,6 +646,12 @@ static int ad4030_conversion(struct iio_dev *indio_dev)
 					   &st->rx_data.dual.diff[0],
 					   &st->rx_data.dual.diff[1]);
 
+	/*
+	 * If no common mode voltage channel is enabled, we can use the raw
+	 * data as is. Otherwise, we need to rearrange the data a bit to match
+	 * the natural alignment of the IIO buffer.
+	 */
+
 	if (st->mode != AD4030_OUT_DATA_MD_16_DIFF_8_COM &&
 	    st->mode != AD4030_OUT_DATA_MD_24_DIFF_8_COM)
 		return 0;

-- 
2.43.0

Re: [PATCH 5/5] iio: adc: ad4030: explain rearranging raw sample data

[Nuno Sá]

On Mon, 2025-03-10 at 15:43 -0500, David Lechner wrote:
> Add a comment explaining why the raw sample data is rearranged in the
> in the ad4030_conversion() function. It is not so obvious from the code
> why this is done.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

(BTW, for some reason I started to send the tags without first checking the
complete series. So I could have just replied to the cover :facepalm:

- Nuno Sá

>  drivers/iio/adc/ad4030.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> index
> 54ad74b96c9f256a67848330f875379edc828b0b..636f9f33e66af73d102722b984dc1230e141
> 7d1e 100644
> --- a/drivers/iio/adc/ad4030.c
> +++ b/drivers/iio/adc/ad4030.c
> @@ -646,6 +646,12 @@ static int ad4030_conversion(struct iio_dev *indio_dev)
>  					   &st->rx_data.dual.diff[0],
>  					   &st->rx_data.dual.diff[1]);
>  
> +	/*
> +	 * If no common mode voltage channel is enabled, we can use the raw
> +	 * data as is. Otherwise, we need to rearrange the data a bit to
> match
> +	 * the natural alignment of the IIO buffer.
> +	 */
> +
>  	if (st->mode != AD4030_OUT_DATA_MD_16_DIFF_8_COM &&
>  	    st->mode != AD4030_OUT_DATA_MD_24_DIFF_8_COM)
>  		return 0;
> 

Re: [PATCH 5/5] iio: adc: ad4030: explain rearranging raw sample data

[Jonathan Cameron]

On Tue, 11 Mar 2025 09:30:32 +0000
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Mon, 2025-03-10 at 15:43 -0500, David Lechner wrote:
> > Add a comment explaining why the raw sample data is rearranged in the
> > in the ad4030_conversion() function. It is not so obvious from the code
> > why this is done.
> > 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---  
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> 
> (BTW, for some reason I started to send the tags without first checking the
> complete series. So I could have just replied to the cover :facepalm:

I do that sometimes when I know I might not get to the end of the series :)

Just to add to the confusion...

Series applied to the testing branch of iio.git which I'll rebase after
rc1 is available.

Thanks

Jonathan

> 
> - Nuno Sá
> 
> >  drivers/iio/adc/ad4030.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> > index
> > 54ad74b96c9f256a67848330f875379edc828b0b..636f9f33e66af73d102722b984dc1230e141
> > 7d1e 100644
> > --- a/drivers/iio/adc/ad4030.c
> > +++ b/drivers/iio/adc/ad4030.c
> > @@ -646,6 +646,12 @@ static int ad4030_conversion(struct iio_dev *indio_dev)
> >  					   &st->rx_data.dual.diff[0],
> >  					   &st->rx_data.dual.diff[1]);
> >  
> > +	/*
> > +	 * If no common mode voltage channel is enabled, we can use the raw
> > +	 * data as is. Otherwise, we need to rearrange the data a bit to
> > match
> > +	 * the natural alignment of the IIO buffer.
> > +	 */
> > +
> >  	if (st->mode != AD4030_OUT_DATA_MD_16_DIFF_8_COM &&
> >  	    st->mode != AD4030_OUT_DATA_MD_24_DIFF_8_COM)
> >  		return 0;
> >   
>