[PATCH 0/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

[PATCH 0/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

[Alexandru Ardelean]

The AD7606C-16 and AD7606C-18 are pretty similar with the AD7606B.
The main difference between AD7606C-16 & AD7606C-18 is the precision in
bits (16 vs 18).
Because of that, some scales need to be defined for the 18-bit variants, as
they need to be computed against 2**18 (vs 2**16 for the 16 bit-variants).

Because the AD7606C-16,18 also supports bipolar & differential channels,
for SW-mode, the default range of 10 V or ±10V should be set at probe.
On reset, the default range (in the registers) is set to value 0x3 which
corresponds to '±10 V single-ended range', regardless of bipolar or
differential configuration.

Aside from the scale/ranges, the AD7606C-16 is similar to the AD7606B.

This changeset, does a bit of rework to the existing ad7606 driver and then
adds support for the AD7606C-16 & AD7606C-18 parts.

Datasheet links:
  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf
  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf


Alexandru Ardelean (7):
  iio: adc: ad7606: add 'bits' parameter to channels macros
  iio: adc: ad7606: move 'val' pointer to ad7606_scan_direct()
  iio: adc: ad7606: split a 'ad7606_sw_mode_setup()' from probe
  iio: adc: ad7606: wrap channel ranges & scales into struct
  iio: adc: ad7606: rework available attributes for SW channels
  dt-bindings: iio: adc: add adi,ad7606c-{16,18} compatible strings
  iio: adc: ad7606: add support for AD7606C-{16,18} parts

 .../bindings/iio/adc/adi,ad7606.yaml          |  83 ++++
 drivers/iio/adc/ad7606.c                      | 445 +++++++++++++++---
 drivers/iio/adc/ad7606.h                      |  77 ++-
 drivers/iio/adc/ad7606_spi.c                  | 110 ++++-
 4 files changed, 624 insertions(+), 91 deletions(-)

-- 
2.46.0

[PATCH 1/7] iio: adc: ad7606: add 'bits' parameter to channels macros

[Alexandru Ardelean]

There are some newer additions to the AD7606 family, which support 18 bit
precision.
Up until now, all chips were 16 bit.

This change adds a 'bits' parameter to the AD760X_CHANNEL macro and renames
'ad7606_channels' -> 'ad7606_channels_16bit' for the current devices.

The AD7606_CHANNEL_PER_CHAN_SCALE() macro is also introduced, as it will
also require that the number of bits be correctly adjusted (for 18 bit
parts).

Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
---
 drivers/iio/adc/ad7606.c     | 58 ++++++++++++++++++------------------
 drivers/iio/adc/ad7606.h     | 18 ++++++-----
 drivers/iio/adc/ad7606_spi.c | 16 +++++-----
 3 files changed, 47 insertions(+), 45 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 539e4a8621fe..dba1f28782e4 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -333,16 +333,16 @@ static const struct iio_chan_spec ad7605_channels[] = {
 	AD7605_CHANNEL(3),
 };
 
-static const struct iio_chan_spec ad7606_channels[] = {
+static const struct iio_chan_spec ad7606_channels_16bit[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(8),
-	AD7606_CHANNEL(0),
-	AD7606_CHANNEL(1),
-	AD7606_CHANNEL(2),
-	AD7606_CHANNEL(3),
-	AD7606_CHANNEL(4),
-	AD7606_CHANNEL(5),
-	AD7606_CHANNEL(6),
-	AD7606_CHANNEL(7),
+	AD7606_CHANNEL(0, 16),
+	AD7606_CHANNEL(1, 16),
+	AD7606_CHANNEL(2, 16),
+	AD7606_CHANNEL(3, 16),
+	AD7606_CHANNEL(4, 16),
+	AD7606_CHANNEL(5, 16),
+	AD7606_CHANNEL(6, 16),
+	AD7606_CHANNEL(7, 16),
 };
 
 /*
@@ -357,22 +357,22 @@ static const struct iio_chan_spec ad7606_channels[] = {
  */
 static const struct iio_chan_spec ad7616_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(16),
-	AD7606_CHANNEL(0),
-	AD7606_CHANNEL(1),
-	AD7606_CHANNEL(2),
-	AD7606_CHANNEL(3),
-	AD7606_CHANNEL(4),
-	AD7606_CHANNEL(5),
-	AD7606_CHANNEL(6),
-	AD7606_CHANNEL(7),
-	AD7606_CHANNEL(8),
-	AD7606_CHANNEL(9),
-	AD7606_CHANNEL(10),
-	AD7606_CHANNEL(11),
-	AD7606_CHANNEL(12),
-	AD7606_CHANNEL(13),
-	AD7606_CHANNEL(14),
-	AD7606_CHANNEL(15),
+	AD7606_CHANNEL(0, 16),
+	AD7606_CHANNEL(1, 16),
+	AD7606_CHANNEL(2, 16),
+	AD7606_CHANNEL(3, 16),
+	AD7606_CHANNEL(4, 16),
+	AD7606_CHANNEL(5, 16),
+	AD7606_CHANNEL(6, 16),
+	AD7606_CHANNEL(7, 16),
+	AD7606_CHANNEL(8, 16),
+	AD7606_CHANNEL(9, 16),
+	AD7606_CHANNEL(10, 16),
+	AD7606_CHANNEL(11, 16),
+	AD7606_CHANNEL(12, 16),
+	AD7606_CHANNEL(13, 16),
+	AD7606_CHANNEL(14, 16),
+	AD7606_CHANNEL(15, 16),
 };
 
 static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
@@ -382,25 +382,25 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
 		.num_channels = 5,
 	},
 	[ID_AD7606_8] = {
-		.channels = ad7606_channels,
+		.channels = ad7606_channels_16bit,
 		.num_channels = 9,
 		.oversampling_avail = ad7606_oversampling_avail,
 		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
 	},
 	[ID_AD7606_6] = {
-		.channels = ad7606_channels,
+		.channels = ad7606_channels_16bit,
 		.num_channels = 7,
 		.oversampling_avail = ad7606_oversampling_avail,
 		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
 	},
 	[ID_AD7606_4] = {
-		.channels = ad7606_channels,
+		.channels = ad7606_channels_16bit,
 		.num_channels = 5,
 		.oversampling_avail = ad7606_oversampling_avail,
 		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
 	},
 	[ID_AD7606B] = {
-		.channels = ad7606_channels,
+		.channels = ad7606_channels_16bit,
 		.num_channels = 9,
 		.oversampling_avail = ad7606_oversampling_avail,
 		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index 0c6a88cc4695..771121350f98 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -8,7 +8,7 @@
 #ifndef IIO_ADC_AD7606_H_
 #define IIO_ADC_AD7606_H_
 
-#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all) {	\
+#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) {	\
 		.type = IIO_VOLTAGE,				\
 		.indexed = 1,					\
 		.channel = num,					\
@@ -19,24 +19,26 @@
 		.scan_index = num,				\
 		.scan_type = {					\
 			.sign = 's',				\
-			.realbits = 16,				\
-			.storagebits = 16,			\
+			.realbits = (bits),			\
+			.storagebits = (bits),			\
 			.endianness = IIO_CPU,			\
 		},						\
 }
 
 #define AD7605_CHANNEL(num)				\
 	AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW),	\
-		BIT(IIO_CHAN_INFO_SCALE), 0)
+		BIT(IIO_CHAN_INFO_SCALE), 0, 16)
 
-#define AD7606_CHANNEL(num)				\
+#define AD7606_CHANNEL(num, bits)			\
 	AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW),	\
 		BIT(IIO_CHAN_INFO_SCALE),		\
-		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO))
+		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits)
 
-#define AD7616_CHANNEL(num)	\
+#define AD7606_SW_CHANNEL(num, bits)	\
 	AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\
-		0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO))
+		0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits)
+
+#define AD7616_CHANNEL(num)	AD7606_SW_CHANNEL(num, 16)
 
 /**
  * struct ad7606_chip_info - chip specific information
diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
index 287a0591533b..dd0075c97c24 100644
--- a/drivers/iio/adc/ad7606_spi.c
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -67,14 +67,14 @@ static const struct iio_chan_spec ad7616_sw_channels[] = {
 
 static const struct iio_chan_spec ad7606b_sw_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(8),
-	AD7616_CHANNEL(0),
-	AD7616_CHANNEL(1),
-	AD7616_CHANNEL(2),
-	AD7616_CHANNEL(3),
-	AD7616_CHANNEL(4),
-	AD7616_CHANNEL(5),
-	AD7616_CHANNEL(6),
-	AD7616_CHANNEL(7),
+	AD7606_SW_CHANNEL(0, 16),
+	AD7606_SW_CHANNEL(1, 16),
+	AD7606_SW_CHANNEL(2, 16),
+	AD7606_SW_CHANNEL(3, 16),
+	AD7606_SW_CHANNEL(4, 16),
+	AD7606_SW_CHANNEL(5, 16),
+	AD7606_SW_CHANNEL(6, 16),
+	AD7606_SW_CHANNEL(7, 16),
 };
 
 static const unsigned int ad7606B_oversampling_avail[9] = {
-- 
2.46.0

Re: [PATCH 1/7] iio: adc: ad7606: add 'bits' parameter to channels macros

[Jonathan Cameron]

On Mon, 19 Aug 2024 09:47:11 +0300
Alexandru Ardelean <aardelean@baylibre.com> wrote:

> There are some newer additions to the AD7606 family, which support 18 bit
> precision.

Hi Alexandru,

> Up until now, all chips were 16 bit.
> 
> This change adds a 'bits' parameter to the AD760X_CHANNEL macro and renames
> 'ad7606_channels' -> 'ad7606_channels_16bit' for the current devices.
> 
> The AD7606_CHANNEL_PER_CHAN_SCALE() macro is also introduced, as it will
> also require that the number of bits be correctly adjusted (for 18 bit
> parts).
Where is that introduced?  There is a _SW_ variant of one macro that isn't
mentioned...

J
> 
> Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
> ---
>  drivers/iio/adc/ad7606.c     | 58 ++++++++++++++++++------------------
>  drivers/iio/adc/ad7606.h     | 18 ++++++-----
>  drivers/iio/adc/ad7606_spi.c | 16 +++++-----
>  3 files changed, 47 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 539e4a8621fe..dba1f28782e4 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -333,16 +333,16 @@ static const struct iio_chan_spec ad7605_channels[] = {
>  	AD7605_CHANNEL(3),
>  };
>  
> -static const struct iio_chan_spec ad7606_channels[] = {
> +static const struct iio_chan_spec ad7606_channels_16bit[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(8),
> -	AD7606_CHANNEL(0),
> -	AD7606_CHANNEL(1),
> -	AD7606_CHANNEL(2),
> -	AD7606_CHANNEL(3),
> -	AD7606_CHANNEL(4),
> -	AD7606_CHANNEL(5),
> -	AD7606_CHANNEL(6),
> -	AD7606_CHANNEL(7),
> +	AD7606_CHANNEL(0, 16),
> +	AD7606_CHANNEL(1, 16),
> +	AD7606_CHANNEL(2, 16),
> +	AD7606_CHANNEL(3, 16),
> +	AD7606_CHANNEL(4, 16),
> +	AD7606_CHANNEL(5, 16),
> +	AD7606_CHANNEL(6, 16),
> +	AD7606_CHANNEL(7, 16),
>  };
>  
>  /*
> @@ -357,22 +357,22 @@ static const struct iio_chan_spec ad7606_channels[] = {
>   */
>  static const struct iio_chan_spec ad7616_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(16),
> -	AD7606_CHANNEL(0),
> -	AD7606_CHANNEL(1),
> -	AD7606_CHANNEL(2),
> -	AD7606_CHANNEL(3),
> -	AD7606_CHANNEL(4),
> -	AD7606_CHANNEL(5),
> -	AD7606_CHANNEL(6),
> -	AD7606_CHANNEL(7),
> -	AD7606_CHANNEL(8),
> -	AD7606_CHANNEL(9),
> -	AD7606_CHANNEL(10),
> -	AD7606_CHANNEL(11),
> -	AD7606_CHANNEL(12),
> -	AD7606_CHANNEL(13),
> -	AD7606_CHANNEL(14),
> -	AD7606_CHANNEL(15),
> +	AD7606_CHANNEL(0, 16),
> +	AD7606_CHANNEL(1, 16),
> +	AD7606_CHANNEL(2, 16),
> +	AD7606_CHANNEL(3, 16),
> +	AD7606_CHANNEL(4, 16),
> +	AD7606_CHANNEL(5, 16),
> +	AD7606_CHANNEL(6, 16),
> +	AD7606_CHANNEL(7, 16),
> +	AD7606_CHANNEL(8, 16),
> +	AD7606_CHANNEL(9, 16),
> +	AD7606_CHANNEL(10, 16),
> +	AD7606_CHANNEL(11, 16),
> +	AD7606_CHANNEL(12, 16),
> +	AD7606_CHANNEL(13, 16),
> +	AD7606_CHANNEL(14, 16),
> +	AD7606_CHANNEL(15, 16),
>  };
>  
>  static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
> @@ -382,25 +382,25 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
>  		.num_channels = 5,
>  	},
>  	[ID_AD7606_8] = {
> -		.channels = ad7606_channels,
> +		.channels = ad7606_channels_16bit,
>  		.num_channels = 9,
>  		.oversampling_avail = ad7606_oversampling_avail,
>  		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
>  	},
>  	[ID_AD7606_6] = {
> -		.channels = ad7606_channels,
> +		.channels = ad7606_channels_16bit,
>  		.num_channels = 7,
>  		.oversampling_avail = ad7606_oversampling_avail,
>  		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
>  	},
>  	[ID_AD7606_4] = {
> -		.channels = ad7606_channels,
> +		.channels = ad7606_channels_16bit,
>  		.num_channels = 5,
>  		.oversampling_avail = ad7606_oversampling_avail,
>  		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
>  	},
>  	[ID_AD7606B] = {
> -		.channels = ad7606_channels,
> +		.channels = ad7606_channels_16bit,
>  		.num_channels = 9,
>  		.oversampling_avail = ad7606_oversampling_avail,
>  		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
> diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
> index 0c6a88cc4695..771121350f98 100644
> --- a/drivers/iio/adc/ad7606.h
> +++ b/drivers/iio/adc/ad7606.h
> @@ -8,7 +8,7 @@
>  #ifndef IIO_ADC_AD7606_H_
>  #define IIO_ADC_AD7606_H_
>  
> -#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all) {	\
> +#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) {	\
>  		.type = IIO_VOLTAGE,				\
>  		.indexed = 1,					\
>  		.channel = num,					\
> @@ -19,24 +19,26 @@
>  		.scan_index = num,				\
>  		.scan_type = {					\
>  			.sign = 's',				\
> -			.realbits = 16,				\
> -			.storagebits = 16,			\
> +			.realbits = (bits),			\
> +			.storagebits = (bits),			\
>  			.endianness = IIO_CPU,			\
>  		},						\
>  }
>  
>  #define AD7605_CHANNEL(num)				\
>  	AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW),	\
> -		BIT(IIO_CHAN_INFO_SCALE), 0)
> +		BIT(IIO_CHAN_INFO_SCALE), 0, 16)
>  
> -#define AD7606_CHANNEL(num)				\
> +#define AD7606_CHANNEL(num, bits)			\
>  	AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW),	\
>  		BIT(IIO_CHAN_INFO_SCALE),		\
> -		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO))
> +		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits)
>  
> -#define AD7616_CHANNEL(num)	\
> +#define AD7606_SW_CHANNEL(num, bits)	\
>  	AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\
> -		0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO))
> +		0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits)
> +
> +#define AD7616_CHANNEL(num)	AD7606_SW_CHANNEL(num, 16)
>  
>  /**
>   * struct ad7606_chip_info - chip specific information
> diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
> index 287a0591533b..dd0075c97c24 100644
> --- a/drivers/iio/adc/ad7606_spi.c
> +++ b/drivers/iio/adc/ad7606_spi.c
> @@ -67,14 +67,14 @@ static const struct iio_chan_spec ad7616_sw_channels[] = {
>  
>  static const struct iio_chan_spec ad7606b_sw_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(8),
> -	AD7616_CHANNEL(0),
> -	AD7616_CHANNEL(1),
> -	AD7616_CHANNEL(2),
> -	AD7616_CHANNEL(3),
> -	AD7616_CHANNEL(4),
> -	AD7616_CHANNEL(5),
> -	AD7616_CHANNEL(6),
> -	AD7616_CHANNEL(7),
> +	AD7606_SW_CHANNEL(0, 16),
> +	AD7606_SW_CHANNEL(1, 16),
> +	AD7606_SW_CHANNEL(2, 16),
> +	AD7606_SW_CHANNEL(3, 16),
> +	AD7606_SW_CHANNEL(4, 16),
> +	AD7606_SW_CHANNEL(5, 16),
> +	AD7606_SW_CHANNEL(6, 16),
> +	AD7606_SW_CHANNEL(7, 16),
>  };
>  
>  static const unsigned int ad7606B_oversampling_avail[9] = {

Re: [PATCH 1/7] iio: adc: ad7606: add 'bits' parameter to channels macros

[Alexandru Ardelean]

On Fri, Aug 23, 2024 at 9:53 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 19 Aug 2024 09:47:11 +0300
> Alexandru Ardelean <aardelean@baylibre.com> wrote:
>
> > There are some newer additions to the AD7606 family, which support 18 bit
> > precision.
>
> Hi Alexandru,
>
> > Up until now, all chips were 16 bit.
> >
> > This change adds a 'bits' parameter to the AD760X_CHANNEL macro and renames
> > 'ad7606_channels' -> 'ad7606_channels_16bit' for the current devices.
> >
> > The AD7606_CHANNEL_PER_CHAN_SCALE() macro is also introduced, as it will
> > also require that the number of bits be correctly adjusted (for 18 bit
> > parts).
> Where is that introduced?  There is a _SW_ variant of one macro that isn't
> mentioned...

Right.
My famous forgetting to update commits after updating code a few times.
Will update.

>
> J
> >
> > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
> > ---
> >  drivers/iio/adc/ad7606.c     | 58 ++++++++++++++++++------------------
> >  drivers/iio/adc/ad7606.h     | 18 ++++++-----
> >  drivers/iio/adc/ad7606_spi.c | 16 +++++-----
> >  3 files changed, 47 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> > index 539e4a8621fe..dba1f28782e4 100644
> > --- a/drivers/iio/adc/ad7606.c
> > +++ b/drivers/iio/adc/ad7606.c
> > @@ -333,16 +333,16 @@ static const struct iio_chan_spec ad7605_channels[] = {
> >       AD7605_CHANNEL(3),
> >  };
> >
> > -static const struct iio_chan_spec ad7606_channels[] = {
> > +static const struct iio_chan_spec ad7606_channels_16bit[] = {
> >       IIO_CHAN_SOFT_TIMESTAMP(8),
> > -     AD7606_CHANNEL(0),
> > -     AD7606_CHANNEL(1),
> > -     AD7606_CHANNEL(2),
> > -     AD7606_CHANNEL(3),
> > -     AD7606_CHANNEL(4),
> > -     AD7606_CHANNEL(5),
> > -     AD7606_CHANNEL(6),
> > -     AD7606_CHANNEL(7),
> > +     AD7606_CHANNEL(0, 16),
> > +     AD7606_CHANNEL(1, 16),
> > +     AD7606_CHANNEL(2, 16),
> > +     AD7606_CHANNEL(3, 16),
> > +     AD7606_CHANNEL(4, 16),
> > +     AD7606_CHANNEL(5, 16),
> > +     AD7606_CHANNEL(6, 16),
> > +     AD7606_CHANNEL(7, 16),
> >  };
> >
> >  /*
> > @@ -357,22 +357,22 @@ static const struct iio_chan_spec ad7606_channels[] = {
> >   */
> >  static const struct iio_chan_spec ad7616_channels[] = {
> >       IIO_CHAN_SOFT_TIMESTAMP(16),
> > -     AD7606_CHANNEL(0),
> > -     AD7606_CHANNEL(1),
> > -     AD7606_CHANNEL(2),
> > -     AD7606_CHANNEL(3),
> > -     AD7606_CHANNEL(4),
> > -     AD7606_CHANNEL(5),
> > -     AD7606_CHANNEL(6),
> > -     AD7606_CHANNEL(7),
> > -     AD7606_CHANNEL(8),
> > -     AD7606_CHANNEL(9),
> > -     AD7606_CHANNEL(10),
> > -     AD7606_CHANNEL(11),
> > -     AD7606_CHANNEL(12),
> > -     AD7606_CHANNEL(13),
> > -     AD7606_CHANNEL(14),
> > -     AD7606_CHANNEL(15),
> > +     AD7606_CHANNEL(0, 16),
> > +     AD7606_CHANNEL(1, 16),
> > +     AD7606_CHANNEL(2, 16),
> > +     AD7606_CHANNEL(3, 16),
> > +     AD7606_CHANNEL(4, 16),
> > +     AD7606_CHANNEL(5, 16),
> > +     AD7606_CHANNEL(6, 16),
> > +     AD7606_CHANNEL(7, 16),
> > +     AD7606_CHANNEL(8, 16),
> > +     AD7606_CHANNEL(9, 16),
> > +     AD7606_CHANNEL(10, 16),
> > +     AD7606_CHANNEL(11, 16),
> > +     AD7606_CHANNEL(12, 16),
> > +     AD7606_CHANNEL(13, 16),
> > +     AD7606_CHANNEL(14, 16),
> > +     AD7606_CHANNEL(15, 16),
> >  };
> >
> >  static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
> > @@ -382,25 +382,25 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
> >               .num_channels = 5,
> >       },
> >       [ID_AD7606_8] = {
> > -             .channels = ad7606_channels,
> > +             .channels = ad7606_channels_16bit,
> >               .num_channels = 9,
> >               .oversampling_avail = ad7606_oversampling_avail,
> >               .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
> >       },
> >       [ID_AD7606_6] = {
> > -             .channels = ad7606_channels,
> > +             .channels = ad7606_channels_16bit,
> >               .num_channels = 7,
> >               .oversampling_avail = ad7606_oversampling_avail,
> >               .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
> >       },
> >       [ID_AD7606_4] = {
> > -             .channels = ad7606_channels,
> > +             .channels = ad7606_channels_16bit,
> >               .num_channels = 5,
> >               .oversampling_avail = ad7606_oversampling_avail,
> >               .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
> >       },
> >       [ID_AD7606B] = {
> > -             .channels = ad7606_channels,
> > +             .channels = ad7606_channels_16bit,
> >               .num_channels = 9,
> >               .oversampling_avail = ad7606_oversampling_avail,
> >               .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
> > diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
> > index 0c6a88cc4695..771121350f98 100644
> > --- a/drivers/iio/adc/ad7606.h
> > +++ b/drivers/iio/adc/ad7606.h
> > @@ -8,7 +8,7 @@
> >  #ifndef IIO_ADC_AD7606_H_
> >  #define IIO_ADC_AD7606_H_
> >
> > -#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all) { \
> > +#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) {   \
> >               .type = IIO_VOLTAGE,                            \
> >               .indexed = 1,                                   \
> >               .channel = num,                                 \
> > @@ -19,24 +19,26 @@
> >               .scan_index = num,                              \
> >               .scan_type = {                                  \
> >                       .sign = 's',                            \
> > -                     .realbits = 16,                         \
> > -                     .storagebits = 16,                      \
> > +                     .realbits = (bits),                     \
> > +                     .storagebits = (bits),                  \
> >                       .endianness = IIO_CPU,                  \
> >               },                                              \
> >  }
> >
> >  #define AD7605_CHANNEL(num)                          \
> >       AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW),     \
> > -             BIT(IIO_CHAN_INFO_SCALE), 0)
> > +             BIT(IIO_CHAN_INFO_SCALE), 0, 16)
> >
> > -#define AD7606_CHANNEL(num)                          \
> > +#define AD7606_CHANNEL(num, bits)                    \
> >       AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW),     \
> >               BIT(IIO_CHAN_INFO_SCALE),               \
> > -             BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO))
> > +             BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits)
> >
> > -#define AD7616_CHANNEL(num)  \
> > +#define AD7606_SW_CHANNEL(num, bits) \
> >       AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\
> > -             0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO))
> > +             0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits)
> > +
> > +#define AD7616_CHANNEL(num)  AD7606_SW_CHANNEL(num, 16)
> >
> >  /**
> >   * struct ad7606_chip_info - chip specific information
> > diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
> > index 287a0591533b..dd0075c97c24 100644
> > --- a/drivers/iio/adc/ad7606_spi.c
> > +++ b/drivers/iio/adc/ad7606_spi.c
> > @@ -67,14 +67,14 @@ static const struct iio_chan_spec ad7616_sw_channels[] = {
> >
> >  static const struct iio_chan_spec ad7606b_sw_channels[] = {
> >       IIO_CHAN_SOFT_TIMESTAMP(8),
> > -     AD7616_CHANNEL(0),
> > -     AD7616_CHANNEL(1),
> > -     AD7616_CHANNEL(2),
> > -     AD7616_CHANNEL(3),
> > -     AD7616_CHANNEL(4),
> > -     AD7616_CHANNEL(5),
> > -     AD7616_CHANNEL(6),
> > -     AD7616_CHANNEL(7),
> > +     AD7606_SW_CHANNEL(0, 16),
> > +     AD7606_SW_CHANNEL(1, 16),
> > +     AD7606_SW_CHANNEL(2, 16),
> > +     AD7606_SW_CHANNEL(3, 16),
> > +     AD7606_SW_CHANNEL(4, 16),
> > +     AD7606_SW_CHANNEL(5, 16),
> > +     AD7606_SW_CHANNEL(6, 16),
> > +     AD7606_SW_CHANNEL(7, 16),
> >  };
> >
> >  static const unsigned int ad7606B_oversampling_avail[9] = {
>

[PATCH 2/7] iio: adc: ad7606: move 'val' pointer to ad7606_scan_direct()

[Alexandru Ardelean]

The ad7606_scan_direct() function returns 'int', which is fine for 16-bit
samples.
But when going to 18-bit samples, these need to be implemented as 32-bit
(or int) type.

In that case when getting samples (which can be negative), we'd get random
error codes.
So, the easiest thing is to just move the 'val' pointer to
'ad7606_scan_direct()'. This doesn't qualify as a fix, it's just a
preparation for 18-bit ADCs (of the AD7606 family).

Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
---
 drivers/iio/adc/ad7606.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index dba1f28782e4..5049e37f8393 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -138,7 +138,8 @@ static irqreturn_t ad7606_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
-static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
+static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch,
+			      int *val)
 {
 	struct ad7606_state *st = iio_priv(indio_dev);
 	int ret;
@@ -153,7 +154,7 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 
 	ret = ad7606_read_samples(st);
 	if (ret == 0)
-		ret = st->data[ch];
+		*val = sign_extend32(st->data[ch], 15);
 
 error_ret:
 	gpiod_set_value(st->gpio_convst, 0);
@@ -173,10 +174,9 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
 		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			ret = ad7606_scan_direct(indio_dev, chan->address);
+			ret = ad7606_scan_direct(indio_dev, chan->address, val);
 			if (ret < 0)
 				return ret;
-			*val = (short) ret;
 			return IIO_VAL_INT;
 		}
 		unreachable();
-- 
2.46.0

[PATCH 3/7] iio: adc: ad7606: split a 'ad7606_sw_mode_setup()' from probe

[Alexandru Ardelean]

This change moves the logic for setting up SW mode (during probe) into it's
own function.

With the addition of some newer parts, the SW-mode part can get a little
more complicated.
So it's a bit better to have a separate function for this.

Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
---
 drivers/iio/adc/ad7606.c | 43 ++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 5049e37f8393..b400c9b2519d 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -545,6 +545,29 @@ static const struct iio_trigger_ops ad7606_trigger_ops = {
 	.validate_device = iio_trigger_validate_own_device,
 };
 
+static int ad7606_sw_mode_setup(struct iio_dev *indio_dev)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+
+	if (!st->bops->sw_mode_config)
+		return 0;
+
+	st->sw_mode_en = device_property_present(st->dev, "adi,sw-mode");
+	if (!st->sw_mode_en)
+		return 0;
+
+	indio_dev->info = &ad7606_info_os_range_and_debug;
+
+	/* Scale of 0.076293 is only available in sw mode */
+	st->scale_avail = ad7616_sw_scale_avail;
+	st->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail);
+
+	/* After reset, in software mode, ±10 V is set by default */
+	memset32(st->range, 2, ARRAY_SIZE(st->range));
+
+	return st->bops->sw_mode_config(indio_dev);
+}
+
 int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 		 const char *name, unsigned int id,
 		 const struct ad7606_bus_ops *bops)
@@ -617,23 +640,9 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 	st->write_scale = ad7606_write_scale_hw;
 	st->write_os = ad7606_write_os_hw;
 
-	if (st->bops->sw_mode_config)
-		st->sw_mode_en = device_property_present(st->dev,
-							 "adi,sw-mode");
-
-	if (st->sw_mode_en) {
-		/* Scale of 0.076293 is only available in sw mode */
-		st->scale_avail = ad7616_sw_scale_avail;
-		st->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail);
-
-		/* After reset, in software mode, ±10 V is set by default */
-		memset32(st->range, 2, ARRAY_SIZE(st->range));
-		indio_dev->info = &ad7606_info_os_range_and_debug;
-
-		ret = st->bops->sw_mode_config(indio_dev);
-		if (ret < 0)
-			return ret;
-	}
+	ret = ad7606_sw_mode_setup(indio_dev);
+	if (ret)
+		return ret;
 
 	st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
 					  indio_dev->name,
-- 
2.46.0

[PATCH 4/7] iio: adc: ad7606: wrap channel ranges & scales into struct

[Alexandru Ardelean]

With the addition of AD7606C-16,18 which have differential & bipolar
channels (and ranges), which can vary from channel to channel, we'll need
to keep more information about each channel range.

To do that, we'll add a 'struct ad7606_chan_scale' type to hold just
configuration for each channel.
This includes the scales per channel (which can be different with
AD7606C-16,18), as well as the range for each channel.
This driver was already keeping the range value for each channel before,
and since this is couple with the scales, it also makes sense to put them
in the same struct.

Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
---
 drivers/iio/adc/ad7606.c | 37 +++++++++++++++++++++++++------------
 drivers/iio/adc/ad7606.h | 22 ++++++++++++++++------
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index b400c9b2519d..2554a4a4a9c0 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -170,6 +170,7 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
 {
 	int ret, ch = 0;
 	struct ad7606_state *st = iio_priv(indio_dev);
+	struct ad7606_chan_scale *cs;
 
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
@@ -183,8 +184,9 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		if (st->sw_mode_en)
 			ch = chan->address;
+		cs = &st->chan_scales[ch];
 		*val = 0;
-		*val2 = st->scale_avail[st->range[ch]];
+		*val2 = cs->scale_avail[cs->range];
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		*val = st->oversampling;
@@ -214,8 +216,9 @@ static ssize_t in_voltage_scale_available_show(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7606_state *st = iio_priv(indio_dev);
+	struct ad7606_chan_scale *cs = &st->chan_scales[0];
 
-	return ad7606_show_avail(buf, st->scale_avail, st->num_scales, true);
+	return ad7606_show_avail(buf, cs->scale_avail, cs->num_scales, true);
 }
 
 static IIO_DEVICE_ATTR_RO(in_voltage_scale_available, 0);
@@ -253,19 +256,21 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 			    long mask)
 {
 	struct ad7606_state *st = iio_priv(indio_dev);
+	struct ad7606_chan_scale *cs;
 	int i, ret, ch = 0;
 
 	guard(mutex)(&st->lock);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		i = find_closest(val2, st->scale_avail, st->num_scales);
 		if (st->sw_mode_en)
 			ch = chan->address;
+		cs = &st->chan_scales[ch];
+		i = find_closest(val2, cs->scale_avail, cs->num_scales);
 		ret = st->write_scale(indio_dev, ch, i);
 		if (ret < 0)
 			return ret;
-		st->range[ch] = i;
+		cs->range = i;
 
 		return 0;
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -547,7 +552,9 @@ static const struct iio_trigger_ops ad7606_trigger_ops = {
 
 static int ad7606_sw_mode_setup(struct iio_dev *indio_dev)
 {
+	unsigned int num_channels = indio_dev->num_channels - 1;
 	struct ad7606_state *st = iio_priv(indio_dev);
+	int ch;
 
 	if (!st->bops->sw_mode_config)
 		return 0;
@@ -559,11 +566,14 @@ static int ad7606_sw_mode_setup(struct iio_dev *indio_dev)
 	indio_dev->info = &ad7606_info_os_range_and_debug;
 
 	/* Scale of 0.076293 is only available in sw mode */
-	st->scale_avail = ad7616_sw_scale_avail;
-	st->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail);
-
 	/* After reset, in software mode, ±10 V is set by default */
-	memset32(st->range, 2, ARRAY_SIZE(st->range));
+	for (ch = 0; ch < num_channels; ch++) {
+		struct ad7606_chan_scale *cs = &st->chan_scales[ch];
+
+		cs->scale_avail = ad7616_sw_scale_avail;
+		cs->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail);
+		cs->range = 2;
+	}
 
 	return st->bops->sw_mode_config(indio_dev);
 }
@@ -572,6 +582,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 		 const char *name, unsigned int id,
 		 const struct ad7606_bus_ops *bops)
 {
+	struct ad7606_chan_scale *cs;
 	struct ad7606_state *st;
 	int ret;
 	struct iio_dev *indio_dev;
@@ -588,10 +599,12 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 	st->bops = bops;
 	st->base_address = base_address;
 	/* tied to logic low, analog input range is +/- 5V */
-	st->range[0] = 0;
 	st->oversampling = 1;
-	st->scale_avail = ad7606_scale_avail;
-	st->num_scales = ARRAY_SIZE(ad7606_scale_avail);
+
+	cs = &st->chan_scales[0];
+	cs->range = 0;
+	cs->scale_avail = ad7606_scale_avail;
+	cs->num_scales = ARRAY_SIZE(ad7606_scale_avail);
 
 	ret = devm_regulator_get_enable(dev, "avcc");
 	if (ret)
@@ -698,7 +711,7 @@ static int ad7606_resume(struct device *dev)
 	struct ad7606_state *st = iio_priv(indio_dev);
 
 	if (st->gpio_standby) {
-		gpiod_set_value(st->gpio_range, st->range[0]);
+		gpiod_set_value(st->gpio_range, st->chan_scales[0].range);
 		gpiod_set_value(st->gpio_standby, 1);
 		ad7606_reset(st);
 	}
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index 771121350f98..afe6a4030e0e 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -8,6 +8,8 @@
 #ifndef IIO_ADC_AD7606_H_
 #define IIO_ADC_AD7606_H_
 
+#define AD760X_MAX_CHANNELS	16
+
 #define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) {	\
 		.type = IIO_VOLTAGE,				\
 		.indexed = 1,					\
@@ -60,17 +62,27 @@ struct ad7606_chip_info {
 	unsigned long			init_delay_ms;
 };
 
+/**
+ * struct ad7606_chan_scale - channel scale configuration
+ * @scale_avail		pointer to the array which stores the available scales
+ * @num_scales		number of elements stored in the scale_avail array
+ * @range		voltage range selection, selects which scale to apply
+ */
+struct ad7606_chan_scale {
+	const unsigned int		*scale_avail;
+	unsigned int			num_scales;
+	unsigned int			range;
+};
+
 /**
  * struct ad7606_state - driver instance specific data
  * @dev		pointer to kernel device
  * @chip_info		entry in the table of chips that describes this device
  * @bops		bus operations (SPI or parallel)
- * @range		voltage range selection, selects which scale to apply
+ * @chan_scales		scale configuration for channels
  * @oversampling	oversampling selection
  * @base_address	address from where to read data in parallel operation
  * @sw_mode_en		software mode enabled
- * @scale_avail		pointer to the array which stores the available scales
- * @num_scales		number of elements stored in the scale_avail array
  * @oversampling_avail	pointer to the array which stores the available
  *			oversampling ratios.
  * @num_os_ratios	number of elements stored in oversampling_avail array
@@ -94,12 +106,10 @@ struct ad7606_state {
 	struct device			*dev;
 	const struct ad7606_chip_info	*chip_info;
 	const struct ad7606_bus_ops	*bops;
-	unsigned int			range[16];
+	struct ad7606_chan_scale	chan_scales[AD760X_MAX_CHANNELS];
 	unsigned int			oversampling;
 	void __iomem			*base_address;
 	bool				sw_mode_en;
-	const unsigned int		*scale_avail;
-	unsigned int			num_scales;
 	const unsigned int		*oversampling_avail;
 	unsigned int			num_os_ratios;
 	int (*write_scale)(struct iio_dev *indio_dev, int ch, int val);
-- 
2.46.0

[PATCH 5/7] iio: adc: ad7606: rework available attributes for SW channels

[Alexandru Ardelean]

For SW mode, the oversampling and scales attributes are always present.
So, they can be implemented via a 'read_avail' hook in iio_info.

For HW mode, it's a bit tricky, as these attributes get assigned based on
GPIO definitions.

So, for SW mode, we define a separate AD7606_SW_CHANNEL() macro, and use
that for the SW channels.
And 'ad7606_info_os_range_and_debug' can be renamed to
'ad7606_info_sw_mode' as it is only used for SW mode.

For the 'read_avail' hook, we'll need to allocate the SW scales, so that
they are just returned userspace without any extra processing.
The oversampling available parameters don't need any extra processing.

Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
---
 drivers/iio/adc/ad7606.c | 75 +++++++++++++++++++++++++++++++++++++---
 drivers/iio/adc/ad7606.h | 30 +++++++++++++---
 2 files changed, 96 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 2554a4a4a9c0..7533aab4b7c8 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -507,6 +507,37 @@ static int ad7606_buffer_predisable(struct iio_dev *indio_dev)
 	return 0;
 }
 
+static int ad7606_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long info)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+	struct ad7606_chan_scale *cs;
+	unsigned int ch = 0;
+
+	switch (info) {
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*vals = st->oversampling_avail;
+		*length = st->num_os_ratios;
+		*type = IIO_VAL_INT;
+
+		return IIO_AVAIL_LIST;
+
+	case IIO_CHAN_INFO_SCALE:
+		if (st->sw_mode_en)
+			ch = chan->address;
+
+		cs = &st->chan_scales[ch];
+		*vals = cs->scale_avail_show;
+		*length = cs->num_scales * 2;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+
+		return IIO_AVAIL_LIST;
+	}
+	return -EINVAL;
+}
+
 static const struct iio_buffer_setup_ops ad7606_buffer_ops = {
 	.postenable = &ad7606_buffer_postenable,
 	.predisable = &ad7606_buffer_predisable,
@@ -524,11 +555,11 @@ static const struct iio_info ad7606_info_os_and_range = {
 	.validate_trigger = &ad7606_validate_trigger,
 };
 
-static const struct iio_info ad7606_info_os_range_and_debug = {
+static const struct iio_info ad7606_info_sw_mode = {
 	.read_raw = &ad7606_read_raw,
 	.write_raw = &ad7606_write_raw,
+	.read_avail = &ad7606_read_avail,
 	.debugfs_reg_access = &ad7606_reg_access,
-	.attrs = &ad7606_attribute_group_os_and_range,
 	.validate_trigger = &ad7606_validate_trigger,
 };
 
@@ -554,7 +585,8 @@ static int ad7606_sw_mode_setup(struct iio_dev *indio_dev)
 {
 	unsigned int num_channels = indio_dev->num_channels - 1;
 	struct ad7606_state *st = iio_priv(indio_dev);
-	int ch;
+	unsigned int *scale_avail_show, num_scales_avail_show;
+	int ret, ch;
 
 	if (!st->bops->sw_mode_config)
 		return 0;
@@ -563,7 +595,7 @@ static int ad7606_sw_mode_setup(struct iio_dev *indio_dev)
 	if (!st->sw_mode_en)
 		return 0;
 
-	indio_dev->info = &ad7606_info_os_range_and_debug;
+	indio_dev->info = &ad7606_info_sw_mode;
 
 	/* Scale of 0.076293 is only available in sw mode */
 	/* After reset, in software mode, ±10 V is set by default */
@@ -575,7 +607,40 @@ static int ad7606_sw_mode_setup(struct iio_dev *indio_dev)
 		cs->range = 2;
 	}
 
-	return st->bops->sw_mode_config(indio_dev);
+	ret = st->bops->sw_mode_config(indio_dev);
+	if (ret)
+		return ret;
+
+	num_scales_avail_show = 1;
+
+	for (ch = 0; ch < num_channels; ch++) {
+		struct ad7606_chan_scale *cs;
+		int i;
+
+		/* AD7606C supports different scales per channel */
+		cs = &st->chan_scales[ch];
+
+		if (num_scales_avail_show == 1 && ch > 0) {
+			cs->scale_avail_show = scale_avail_show;
+			continue;
+		}
+
+		scale_avail_show = devm_kcalloc(st->dev, cs->num_scales * 2,
+						sizeof(*scale_avail_show),
+						GFP_KERNEL);
+		if (!scale_avail_show)
+			return -ENOMEM;
+
+		/* Generate a scale_avail list for showing to userspace */
+		for (i = 0; i < cs->num_scales; i++) {
+			scale_avail_show[i * 2] = 0;
+			scale_avail_show[i * 2 + 1] = cs->scale_avail[i];
+		}
+
+		cs->scale_avail_show = scale_avail_show;
+	}
+
+	return 0;
 }
 
 int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index afe6a4030e0e..d71a843a5de5 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -27,6 +27,29 @@
 		},						\
 }
 
+#define AD7606_SW_CHANNEL(num, bits) {				\
+		.type = IIO_VOLTAGE,				\
+		.indexed = 1,					\
+		.channel = num,					\
+		.address = num,					\
+		.info_mask_separate =				\
+			BIT(IIO_CHAN_INFO_RAW) | 		\
+			BIT(IIO_CHAN_INFO_SCALE),		\
+		.info_mask_separate_available =			\
+			BIT(IIO_CHAN_INFO_SCALE),		\
+		.info_mask_shared_by_all =			\
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),	\
+		.info_mask_shared_by_all_available =		\
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),	\
+		.scan_index = num,				\
+		.scan_type = {					\
+			.sign = 's',				\
+			.realbits = (bits),			\
+			.storagebits = (bits),			\
+			.endianness = IIO_CPU,			\
+		},						\
+}
+
 #define AD7605_CHANNEL(num)				\
 	AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW),	\
 		BIT(IIO_CHAN_INFO_SCALE), 0, 16)
@@ -36,10 +59,6 @@
 		BIT(IIO_CHAN_INFO_SCALE),		\
 		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits)
 
-#define AD7606_SW_CHANNEL(num, bits)	\
-	AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\
-		0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits)
-
 #define AD7616_CHANNEL(num)	AD7606_SW_CHANNEL(num, 16)
 
 /**
@@ -65,11 +84,14 @@ struct ad7606_chip_info {
 /**
  * struct ad7606_chan_scale - channel scale configuration
  * @scale_avail		pointer to the array which stores the available scales
+ * @scale_avail_show	a duplicate of 'scale_avail' which is readily formatted
+ *			such that it can be read via the 'read_avail' hook
  * @num_scales		number of elements stored in the scale_avail array
  * @range		voltage range selection, selects which scale to apply
  */
 struct ad7606_chan_scale {
 	const unsigned int		*scale_avail;
+	const unsigned int		*scale_avail_show;
 	unsigned int			num_scales;
 	unsigned int			range;
 };
-- 
2.46.0

[PATCH 6/7] dt-bindings: iio: adc: add adi,ad7606c-{16,18} compatible strings

[Alexandru Ardelean]

The driver will support the AD7606C-16 and AD7606C-18.
This change adds the compatible strings for these devices.

The AD7606C-16,18 channels also support these (individually configurable)
types of channels:
 - bipolar single-ended
 - unipolar single-ended
 - bipolar differential

This DT adds support for 'channel@X' nodes'

Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
---
 .../bindings/iio/adc/adi,ad7606.yaml          | 83 +++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index 69408cae3db9..f9e177de3f8c 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -14,6 +14,8 @@ description: |
   https://www.analog.com/media/en/technical-documentation/data-sheets/AD7605-4.pdf
   https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
   https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf
   https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
 
 properties:
@@ -24,6 +26,8 @@ properties:
       - adi,ad7606-6
       - adi,ad7606-8  # Referred to as AD7606 (without -8) in the datasheet
       - adi,ad7606b
+      - adi,ad7606c-16
+      - adi,ad7606c-18
       - adi,ad7616
 
   reg:
@@ -114,6 +118,30 @@ properties:
       assumed that the pins are hardwired to VDD.
     type: boolean
 
+patternProperties:
+  "^channel@([0-9a-f])$":
+    type: object
+    $ref: adc.yaml
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        description: The channel number.
+        minimum: 0
+        maximum: 7
+
+      diff-channel:
+        description: Channel is bipolar differential.
+        type: boolean
+
+      bipolar:
+        description: |
+          Channel is bipolar single-ended. If 'diff-channel' is set, then
+          the value of this property will be ignored.
+        type: boolean
+    required:
+      - reg
+
 required:
   - compatible
   - reg
@@ -170,6 +198,21 @@ allOf:
         adi,conversion-start-gpios:
           maxItems: 1
 
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - adi,ad7606c-16
+                - adi,ad7606c-18
+    then:
+      patternProperties:
+        "^channel@([0-9a-f])$":
+          properties:
+            diff-channels: false
+            bipolar: true
+
 unevaluatedProperties: false
 
 examples:
@@ -202,4 +245,44 @@ examples:
             standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
         };
     };
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad7606-8";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+            spi-cpol;
+            spi-cpha;
+
+            avcc-supply = <&adc_vref>;
+            vdrive-supply = <&vdd_supply>;
+
+            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-parent = <&gpio>;
+
+            adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+
+            adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+            reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
+            adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
+            standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
+
+            adi,sw-mode;
+
+            channel@1 {
+                reg = <1>;
+                diff-channel;
+            };
+
+            channel@3 {
+                reg = <3>;
+                bipolar;
+            };
+        };
+    };
 ...
-- 
2.46.0

Re: [PATCH 6/7] dt-bindings: iio: adc: add adi,ad7606c-{16,18} compatible strings

[Krzysztof Kozlowski]

On Mon, Aug 19, 2024 at 09:47:16AM +0300, Alexandru Ardelean wrote:
> The driver will support the AD7606C-16 and AD7606C-18.
> This change adds the compatible strings for these devices.
> 
> The AD7606C-16,18 channels also support these (individually configurable)
> types of channels:
>  - bipolar single-ended
>  - unipolar single-ended
>  - bipolar differential
> 
> This DT adds support for 'channel@X' nodes'

I don't understand this sentence, suggest to drop it.


> 
> Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
> ---
>  .../bindings/iio/adc/adi,ad7606.yaml          | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> index 69408cae3db9..f9e177de3f8c 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> @@ -14,6 +14,8 @@ description: |
>    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7605-4.pdf
>    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
>    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf
>    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
>  
>  properties:
> @@ -24,6 +26,8 @@ properties:
>        - adi,ad7606-6
>        - adi,ad7606-8  # Referred to as AD7606 (without -8) in the datasheet
>        - adi,ad7606b
> +      - adi,ad7606c-16
> +      - adi,ad7606c-18
>        - adi,ad7616
>  
>    reg:
> @@ -114,6 +118,30 @@ properties:
>        assumed that the pins are hardwired to VDD.
>      type: boolean
>  
> +patternProperties:
> +  "^channel@([0-9a-f])$":

[0-7]

> +    type: object
> +    $ref: adc.yaml
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel number.
> +        minimum: 0
> +        maximum: 7
> +
> +      diff-channel:
> +        description: Channel is bipolar differential.

There is diff-channels property, why do we need one more?

> +        type: boolean
> +
> +      bipolar:
> +        description: |
> +          Channel is bipolar single-ended. If 'diff-channel' is set, then
> +          the value of this property will be ignored.

Then provide here allOf:if:then which makes it false if diff-channel(s)
is present. And then drop entire property, because you duplicate what's
in adc.yaml.


> +        type: boolean

Blank line.

> +    required:
> +      - reg
> +
>  required:
>    - compatible
>    - reg
> @@ -170,6 +198,21 @@ allOf:
>          adi,conversion-start-gpios:
>            maxItems: 1
>  
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - adi,ad7606c-16
> +                - adi,ad7606c-18
> +    then:
> +      patternProperties:
> +        "^channel@([0-9a-f])$":
> +          properties:
> +            diff-channels: false
> +            bipolar: true

? Drop, no clue what you want to say here. But more important, you are
now adding channels to other variants. Split your commit between new
device and new properties for existing devices.


Best regards,
Krzysztof

Re: [PATCH 6/7] dt-bindings: iio: adc: add adi,ad7606c-{16,18} compatible strings

[Alexandru Ardelean]

On Mon, Aug 19, 2024 at 4:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Aug 19, 2024 at 09:47:16AM +0300, Alexandru Ardelean wrote:
> > The driver will support the AD7606C-16 and AD7606C-18.
> > This change adds the compatible strings for these devices.
> >
> > The AD7606C-16,18 channels also support these (individually configurable)
> > types of channels:
> >  - bipolar single-ended
> >  - unipolar single-ended
> >  - bipolar differential
> >
> > This DT adds support for 'channel@X' nodes'
>
> I don't understand this sentence, suggest to drop it.

Huh?
I guess I'm developing more ADHD, where I forget to finish sentences(?)

>
>
> >
> > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
> > ---
> >  .../bindings/iio/adc/adi,ad7606.yaml          | 83 +++++++++++++++++++
> >  1 file changed, 83 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > index 69408cae3db9..f9e177de3f8c 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > @@ -14,6 +14,8 @@ description: |
> >    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7605-4.pdf
> >    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
> >    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf
> > +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf
> > +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf
> >    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
> >
> >  properties:
> > @@ -24,6 +26,8 @@ properties:
> >        - adi,ad7606-6
> >        - adi,ad7606-8  # Referred to as AD7606 (without -8) in the datasheet
> >        - adi,ad7606b
> > +      - adi,ad7606c-16
> > +      - adi,ad7606c-18
> >        - adi,ad7616
> >
> >    reg:
> > @@ -114,6 +118,30 @@ properties:
> >        assumed that the pins are hardwired to VDD.
> >      type: boolean
> >
> > +patternProperties:
> > +  "^channel@([0-9a-f])$":
>
> [0-7]
>
> > +    type: object
> > +    $ref: adc.yaml
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        description: The channel number.
> > +        minimum: 0
> > +        maximum: 7
> > +
> > +      diff-channel:
> > +        description: Channel is bipolar differential.
>
> There is diff-channels property, why do we need one more?

Yeah, I wanted to use that.
Maybe I will try another spin at that.
The thing with "diff-channels" is that it requires 2 ints.
So,  diff-channels = <0 0>.
To use it here, a rule would need to be put in place where  "reg ==
diff-channels[0] && reg == diff-channels[1]".
That also works from my side.
Part of the reason for this patchset, was to also get some feedback
(if this is the correct direction).

>
> > +        type: boolean
> > +
> > +      bipolar:
> > +        description: |
> > +          Channel is bipolar single-ended. If 'diff-channel' is set, then
> > +          the value of this property will be ignored.
>
> Then provide here allOf:if:then which makes it false if diff-channel(s)
> is present. And then drop entire property, because you duplicate what's
> in adc.yaml.

Ack.

>
>
> > +        type: boolean
>
> Blank line.
>
> > +    required:
> > +      - reg
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -170,6 +198,21 @@ allOf:
> >          adi,conversion-start-gpios:
> >            maxItems: 1
> >
> > +  - if:
> > +      not:
> > +        properties:
> > +          compatible:
> > +            contains:
> > +              enum:
> > +                - adi,ad7606c-16
> > +                - adi,ad7606c-18
> > +    then:
> > +      patternProperties:
> > +        "^channel@([0-9a-f])$":
> > +          properties:
> > +            diff-channels: false
> > +            bipolar: true
>
> ? Drop, no clue what you want to say here. But more important, you are
> now adding channels to other variants. Split your commit between new
> device and new properties for existing devices.

Ack.
Will do that.

>
>
> Best regards,
> Krzysztof
>

Re: [PATCH 6/7] dt-bindings: iio: adc: add adi,ad7606c-{16,18} compatible strings

[Jonathan Cameron]

> > > +    type: object
> > > +    $ref: adc.yaml
> > > +    unevaluatedProperties: false
> > > +
> > > +    properties:
> > > +      reg:
> > > +        description: The channel number.
> > > +        minimum: 0
> > > +        maximum: 7
> > > +
> > > +      diff-channel:
> > > +        description: Channel is bipolar differential.  
> >
> > There is diff-channels property, why do we need one more?  
> 
> Yeah, I wanted to use that.
> Maybe I will try another spin at that.
> The thing with "diff-channels" is that it requires 2 ints.
> So,  diff-channels = <0 0>.
> To use it here, a rule would need to be put in place where  "reg ==
> diff-channels[0] && reg == diff-channels[1]".
> That also works from my side.
> Part of the reason for this patchset, was to also get some feedback
> (if this is the correct direction).
> 
So I 'think' this is a datasheet matching thing.
In many cases, even for strictly differential devices, the pin
naming allows for a clear A - B channel description. Here
in the non differential modes, the negative pins are effectively
not used (from a really quick look at the datasheet)

So we 'could' introduce magic channels (give them high numbers) for
the negative ends. I think we may want to do that for the
userspace ABI (0-0 on the few times it has come up has been a
calibration / self check mode not what you have here - it
wires the actual inputs together).  Alternative is just present
them as a simple voltage and don't worry about the differential aspect
as it's not hugely different to bipolar (where the zero level is
effectively the negative input of a differential ADC.

For the binding I'm fine with the binding using A, A as you suggest
with an update to adc.yaml to cover this corner.

We never (I think) have bindings for the self check case where the input
is wired to both sides. It's just a mode that is applied to
any inputs that are wired. 

> >  
> > > +        type: boolean
> > > +
> > > +      bipolar:
> > > +        description: |
> > > +          Channel is bipolar single-ended. If 'diff-channel' is set, then
> > > +          the value of this property will be ignored.  
> >
> > Then provide here allOf:if:then which makes it false if diff-channel(s)
> > is present. And then drop entire property, because you duplicate what's
> > in adc.yaml.  
> 
> Ack.

Re: [PATCH 6/7] dt-bindings: iio: adc: add adi,ad7606c-{16,18} compatible strings

[Krzysztof Kozlowski]

On 21/08/2024 22:26, Jonathan Cameron wrote:
> 
>>>> +    type: object
>>>> +    $ref: adc.yaml
>>>> +    unevaluatedProperties: false
>>>> +
>>>> +    properties:
>>>> +      reg:
>>>> +        description: The channel number.
>>>> +        minimum: 0
>>>> +        maximum: 7
>>>> +
>>>> +      diff-channel:
>>>> +        description: Channel is bipolar differential.  
>>>
>>> There is diff-channels property, why do we need one more?  
>>
>> Yeah, I wanted to use that.
>> Maybe I will try another spin at that.
>> The thing with "diff-channels" is that it requires 2 ints.
>> So,  diff-channels = <0 0>.
>> To use it here, a rule would need to be put in place where  "reg ==
>> diff-channels[0] && reg == diff-channels[1]".
>> That also works from my side.
>> Part of the reason for this patchset, was to also get some feedback
>> (if this is the correct direction).
>>
> So I 'think' this is a datasheet matching thing.
> In many cases, even for strictly differential devices, the pin
> naming allows for a clear A - B channel description. Here
> in the non differential modes, the negative pins are effectively
> not used (from a really quick look at the datasheet)
> 
> So we 'could' introduce magic channels (give them high numbers) for
> the negative ends. I think we may want to do that for the
> userspace ABI (0-0 on the few times it has come up has been a
> calibration / self check mode not what you have here - it
> wires the actual inputs together).  Alternative is just present
> them as a simple voltage and don't worry about the differential aspect
> as it's not hugely different to bipolar (where the zero level is
> effectively the negative input of a differential ADC.
> 
> For the binding I'm fine with the binding using A, A as you suggest
> with an update to adc.yaml to cover this corner.

Yep, let's add it to adc.yaml.

> 
> We never (I think) have bindings for the self check case where the input
> is wired to both sides. It's just a mode that is applied to
> any inputs that are wired. 
> 
Best regards,
Krzysztof

Re: [PATCH 6/7] dt-bindings: iio: adc: add adi,ad7606c-{16,18} compatible strings

[Alexandru Ardelean]

On Fri, Aug 23, 2024 at 12:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 21/08/2024 22:26, Jonathan Cameron wrote:
> >
> >>>> +    type: object
> >>>> +    $ref: adc.yaml
> >>>> +    unevaluatedProperties: false
> >>>> +
> >>>> +    properties:
> >>>> +      reg:
> >>>> +        description: The channel number.
> >>>> +        minimum: 0
> >>>> +        maximum: 7
> >>>> +
> >>>> +      diff-channel:
> >>>> +        description: Channel is bipolar differential.
> >>>
> >>> There is diff-channels property, why do we need one more?
> >>
> >> Yeah, I wanted to use that.
> >> Maybe I will try another spin at that.
> >> The thing with "diff-channels" is that it requires 2 ints.
> >> So,  diff-channels = <0 0>.
> >> To use it here, a rule would need to be put in place where  "reg ==
> >> diff-channels[0] && reg == diff-channels[1]".
> >> That also works from my side.
> >> Part of the reason for this patchset, was to also get some feedback
> >> (if this is the correct direction).
> >>
> > So I 'think' this is a datasheet matching thing.
> > In many cases, even for strictly differential devices, the pin
> > naming allows for a clear A - B channel description. Here
> > in the non differential modes, the negative pins are effectively
> > not used (from a really quick look at the datasheet)
> >
> > So we 'could' introduce magic channels (give them high numbers) for
> > the negative ends. I think we may want to do that for the
> > userspace ABI (0-0 on the few times it has come up has been a
> > calibration / self check mode not what you have here - it
> > wires the actual inputs together).  Alternative is just present
> > them as a simple voltage and don't worry about the differential aspect
> > as it's not hugely different to bipolar (where the zero level is
> > effectively the negative input of a differential ADC.
> >
> > For the binding I'm fine with the binding using A, A as you suggest
> > with an update to adc.yaml to cover this corner.

The main difference the "diff-channels" property brings is a change to
the available scales.
They differ a bit between differential, and single-ended (unipolar and bipolar).

I'll update the adc.yaml file then.

>
> Yep, let's add it to adc.yaml.
>
> >
> > We never (I think) have bindings for the self check case where the input
> > is wired to both sides. It's just a mode that is applied to
> > any inputs that are wired.
> >
> Best regards,
> Krzysztof
>

[PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

[Alexandru Ardelean]

The AD7606C-16 and AD7606C-18 are pretty similar with the AD7606B.
The main difference between AD7606C-16 & AD7606C-18 is the precision in
bits (16 vs 18).
Because of that, some scales need to be defined for the 18-bit variants, as
they need to be computed against 2**18 (vs 2**16 for the 16 bit-variants).

Because the AD7606C-16,18 also supports bipolar & differential channels,
for SW-mode, the default range of 10 V or ±10V should be set at probe.
On reset, the default range (in the registers) is set to value 0x3 which
corresponds to '±10 V single-ended range', regardless of bipolar or
differential configuration.

Aside from the scale/ranges, the AD7606C-16 is similar to the AD7606B.

And the AD7606C-18 variant offers 18-bit precision. The unfortunate effect
of this 18-bit sample size, is that there is no simple/neat way to get the
samples into a 32-bit array without having to do a home-brewed bit-buffer.
The ADC must read all samples (from all 8 channels) in order to get the
N-th sample (this could be reworked to do up-to-N-th sample for scan-direct).
There doesn't seem to be any quick-trick to be usable to pad the samples
up to at least 24 bits.
Even the optional status-header is 8-bits, which would mean 26-bits of data
per sample.
That means that when using a simple SPI controller (which can usually read
8 bit multiples) a simple bit-buffer trick is required.

Datasheet links:
  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf
  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf

Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
---
 drivers/iio/adc/ad7606.c     | 274 ++++++++++++++++++++++++++++++++---
 drivers/iio/adc/ad7606.h     |  17 ++-
 drivers/iio/adc/ad7606_spi.c |  94 ++++++++++++
 3 files changed, 357 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 7533aab4b7c8..55faab321092 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -28,14 +28,44 @@
 
 #include "ad7606.h"
 
+typedef void (*ad7606c_chan_setup_cb_t)(struct ad7606_state *st, int ch,
+					bool bipolar, bool differential);
+
 /*
  * Scales are computed as 5000/32768 and 10000/32768 respectively,
  * so that when applied to the raw values they provide mV values
  */
-static const unsigned int ad7606_scale_avail[2] = {
+static const unsigned int ad7606_16bit_hw_scale_avail[2] = {
 	152588, 305176
 };
 
+static const unsigned int ad7606_18bit_hw_scale_avail[2] = {
+	38147, 76294
+};
+
+static const unsigned int ad7606c_16_scale_single_ended_unipolar_avail[3] = {
+	76294, 152588, 190735,
+};
+
+static const unsigned int ad7606c_16_scale_single_ended_bipolar_avail[5] = {
+	76294, 152588, 190735, 305176, 381470
+};
+
+static const unsigned int ad7606c_16_scale_differential_bipolar_avail[4] = {
+	152588, 305176, 381470, 610352
+};
+
+static const unsigned int ad7606c_18_scale_single_ended_unipolar_avail[3] = {
+	19073, 38147, 47684
+};
+
+static const unsigned int ad7606c_18_scale_single_ended_bipolar_avail[5] = {
+	19073, 38147, 47684, 76294, 95367
+};
+
+static const unsigned int ad7606c_18_scale_differential_bipolar_avail[4] = {
+	38147, 76294, 95367, 152588
+};
 
 static const unsigned int ad7616_sw_scale_avail[3] = {
 	76293, 152588, 305176
@@ -84,10 +114,18 @@ static int ad7606_reg_access(struct iio_dev *indio_dev,
 
 static int ad7606_read_samples(struct ad7606_state *st)
 {
+	unsigned int storagebits = st->chip_info->channels[1].scan_type.storagebits;
 	unsigned int num = st->chip_info->num_channels - 1;
-	u16 *data = st->data;
+	u32 *data32 = st->data.d32;
+	u16 *data16 = st->data.d16;
+	void *data;
 	int ret;
 
+	if (storagebits > 16)
+		data = data32;
+	else
+		data = data16;
+
 	/*
 	 * The frstdata signal is set to high while and after reading the sample
 	 * of the first channel and low for all other channels. This can be used
@@ -108,7 +146,10 @@ static int ad7606_read_samples(struct ad7606_state *st)
 			return -EIO;
 		}
 
-		data++;
+		if (storagebits > 16)
+			data32++;
+		else
+			data16++;
 		num--;
 	}
 
@@ -128,7 +169,7 @@ static irqreturn_t ad7606_trigger_handler(int irq, void *p)
 	if (ret)
 		goto error_ret;
 
-	iio_push_to_buffers_with_timestamp(indio_dev, st->data,
+	iio_push_to_buffers_with_timestamp(indio_dev, st->data.d16,
 					   iio_get_time_ns(indio_dev));
 error_ret:
 	iio_trigger_notify_done(indio_dev->trig);
@@ -142,6 +183,7 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch,
 			      int *val)
 {
 	struct ad7606_state *st = iio_priv(indio_dev);
+	unsigned int storagebits = st->chip_info->channels[1].scan_type.storagebits;
 	int ret;
 
 	gpiod_set_value(st->gpio_convst, 1);
@@ -153,8 +195,12 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch,
 	}
 
 	ret = ad7606_read_samples(st);
-	if (ret == 0)
-		*val = sign_extend32(st->data[ch], 15);
+	if (ret == 0) {
+		if (storagebits > 16)
+			*val = sign_extend32(st->data.d32[ch], 17);
+		else
+			*val = sign_extend32(st->data.d16[ch], 15);
+	}
 
 error_ret:
 	gpiod_set_value(st->gpio_convst, 0);
@@ -267,7 +313,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 			ch = chan->address;
 		cs = &st->chan_scales[ch];
 		i = find_closest(val2, cs->scale_avail, cs->num_scales);
-		ret = st->write_scale(indio_dev, ch, i);
+		ret = st->write_scale(indio_dev, ch, i + cs->reg_offset);
 		if (ret < 0)
 			return ret;
 		cs->range = i;
@@ -350,6 +396,18 @@ static const struct iio_chan_spec ad7606_channels_16bit[] = {
 	AD7606_CHANNEL(7, 16),
 };
 
+static const struct iio_chan_spec ad7606_channels_18bit[] = {
+	IIO_CHAN_SOFT_TIMESTAMP(8),
+	AD7606_CHANNEL(0, 18),
+	AD7606_CHANNEL(1, 18),
+	AD7606_CHANNEL(2, 18),
+	AD7606_CHANNEL(3, 18),
+	AD7606_CHANNEL(4, 18),
+	AD7606_CHANNEL(5, 18),
+	AD7606_CHANNEL(6, 18),
+	AD7606_CHANNEL(7, 18),
+};
+
 /*
  * The current assumption that this driver makes for AD7616, is that it's
  * working in Hardware Mode with Serial, Burst and Sequencer modes activated.
@@ -410,6 +468,18 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
 		.oversampling_avail = ad7606_oversampling_avail,
 		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
 	},
+	[ID_AD7606C_16] = {
+		.channels = ad7606_channels_16bit,
+		.num_channels = 9,
+		.oversampling_avail = ad7606_oversampling_avail,
+		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
+	},
+	[ID_AD7606C_18] = {
+		.channels = ad7606_channels_18bit,
+		.num_channels = 9,
+		.oversampling_avail = ad7606_oversampling_avail,
+		.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
+	},
 	[ID_AD7616] = {
 		.channels = ad7616_channels,
 		.num_channels = 17,
@@ -563,6 +633,14 @@ static const struct iio_info ad7606_info_sw_mode = {
 	.validate_trigger = &ad7606_validate_trigger,
 };
 
+static const struct iio_info ad7606c_info_sw_mode = {
+	.read_raw = &ad7606_read_raw,
+	.write_raw = &ad7606_write_raw,
+	.read_avail = &ad7606_read_avail,
+	.debugfs_reg_access = &ad7606_reg_access,
+	.validate_trigger = &ad7606_validate_trigger,
+};
+
 static const struct iio_info ad7606_info_os = {
 	.read_raw = &ad7606_read_raw,
 	.write_raw = &ad7606_write_raw,
@@ -581,7 +659,136 @@ static const struct iio_trigger_ops ad7606_trigger_ops = {
 	.validate_device = iio_trigger_validate_own_device,
 };
 
-static int ad7606_sw_mode_setup(struct iio_dev *indio_dev)
+static void ad7606c_18_chan_setup(struct ad7606_state *st, int ch,
+				  bool bipolar, bool differential)
+{
+	struct ad7606_chan_scale *cs = &st->chan_scales[ch];
+
+	if (differential) {
+		cs->scale_avail =
+			ad7606c_18_scale_differential_bipolar_avail;
+		cs->num_scales =
+			ARRAY_SIZE(ad7606c_18_scale_differential_bipolar_avail);
+		/* Bipolar differential ranges start at 8 (b1000) */
+		cs->reg_offset = 8;
+		cs->range = 1;
+	} else if (bipolar) {
+		cs->scale_avail =
+			ad7606c_18_scale_single_ended_bipolar_avail;
+		cs->num_scales =
+			ARRAY_SIZE(ad7606c_18_scale_single_ended_bipolar_avail);
+		cs->range = 3;
+	} else {
+		cs->scale_avail =
+			ad7606c_18_scale_single_ended_unipolar_avail;
+		cs->num_scales =
+			ARRAY_SIZE(ad7606c_18_scale_single_ended_unipolar_avail);
+		/* Unipolar single-ended ranges start at 5 (b0101) */
+		cs->reg_offset = 5;
+		cs->range = 1;
+	}
+}
+
+static void ad7606c_16_chan_setup(struct ad7606_state *st, int ch,
+				  bool bipolar, bool differential)
+{
+	struct ad7606_chan_scale *cs = &st->chan_scales[ch];
+
+	if (differential) {
+		cs->scale_avail =
+			ad7606c_16_scale_differential_bipolar_avail;
+		cs->num_scales =
+			ARRAY_SIZE(ad7606c_16_scale_differential_bipolar_avail);
+		/* Bipolar differential ranges start at 8 (b1000) */
+		cs->reg_offset = 8;
+		cs->range = 1;
+	} else if (bipolar) {
+		cs->scale_avail =
+			ad7606c_16_scale_single_ended_bipolar_avail;
+		cs->num_scales =
+			ARRAY_SIZE(ad7606c_16_scale_single_ended_bipolar_avail);
+		cs->range = 3;
+	} else {
+		cs->scale_avail =
+			ad7606c_16_scale_single_ended_unipolar_avail;
+		cs->num_scales =
+			ARRAY_SIZE(ad7606c_16_scale_single_ended_unipolar_avail);
+		/* Unipolar single-ended ranges start at 5 (b0101) */
+		cs->reg_offset = 5;
+		cs->range = 1;
+	}
+}
+
+static int ad7606c_sw_mode_setup_channels(struct iio_dev *indio_dev,
+					  ad7606c_chan_setup_cb_t chan_setup_cb)
+{
+	unsigned int num_channels = indio_dev->num_channels - 1;
+	struct ad7606_state *st = iio_priv(indio_dev);
+	bool chan_configured[AD760X_MAX_CHANNELS];
+	struct device *dev = st->dev;
+	int ret;
+	u32 ch;
+
+	/* We need to hook this first */
+	ret = st->bops->sw_mode_config(indio_dev);
+	if (ret)
+		return ret;
+
+	indio_dev->info = &ad7606c_info_sw_mode;
+
+	memset(chan_configured, 0, sizeof(chan_configured));
+
+	device_for_each_child_node_scoped(dev, child) {
+		bool bipolar, differential;
+
+		ret = fwnode_property_read_u32(child, "reg", &ch);
+		if (ret)
+			continue;
+
+		if (ch >= num_channels) {
+			dev_warn(st->dev,
+				 "Invalid channel number (ignoring): %d\n", ch);
+			continue;
+		}
+
+		bipolar = fwnode_property_present(child, "bipolar");
+		differential = fwnode_property_present(child, "diff-channel");
+
+		chan_setup_cb(st, ch, bipolar, differential);
+		chan_configured[ch] = true;
+	}
+
+	/* Apply default configuration to unconfigured (via DT) channels */
+	for (ch = 0; ch < num_channels; ch++) {
+		struct ad7606_chan_scale *cs;
+		unsigned int *scale_avail_show;
+		int i;
+
+		if (!chan_configured[ch])
+			chan_setup_cb(st, ch, false, false);
+
+		/* AD7606C supports different scales per channel */
+		cs = &st->chan_scales[ch];
+
+		scale_avail_show = devm_kcalloc(st->dev, cs->num_scales * 2,
+						sizeof(*scale_avail_show),
+						GFP_KERNEL);
+		if (!scale_avail_show)
+			return -ENOMEM;
+
+		/* Generate a scale_avail list for showing to userspace */
+		for (i = 0; i < cs->num_scales; i++) {
+			scale_avail_show[i * 2] = 0;
+			scale_avail_show[i * 2 + 1] = cs->scale_avail[i];
+		}
+
+		cs->scale_avail_show = scale_avail_show;
+	}
+
+	return 0;
+}
+
+static int ad7606_sw_mode_setup(struct iio_dev *indio_dev, unsigned int id)
 {
 	unsigned int num_channels = indio_dev->num_channels - 1;
 	struct ad7606_state *st = iio_priv(indio_dev);
@@ -597,21 +804,33 @@ static int ad7606_sw_mode_setup(struct iio_dev *indio_dev)
 
 	indio_dev->info = &ad7606_info_sw_mode;
 
-	/* Scale of 0.076293 is only available in sw mode */
-	/* After reset, in software mode, ±10 V is set by default */
-	for (ch = 0; ch < num_channels; ch++) {
-		struct ad7606_chan_scale *cs = &st->chan_scales[ch];
+	switch (id) {
+	case ID_AD7606C_18:
+		num_scales_avail_show = num_channels;
+		ret = ad7606c_sw_mode_setup_channels(indio_dev,
+						     ad7606c_18_chan_setup);
+		break;
+	case ID_AD7606C_16:
+		num_scales_avail_show = num_channels;
+		ret = ad7606c_sw_mode_setup_channels(indio_dev,
+						     ad7606c_16_chan_setup);
+		break;
+	default:
+		num_scales_avail_show = 1;
 
-		cs->scale_avail = ad7616_sw_scale_avail;
-		cs->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail);
-		cs->range = 2;
-	}
+		/* Scale of 0.076293 is only available in sw mode */
+		/* After reset, in software mode, ±10 V is set by default */
+		for (ch = 0; ch < num_channels; ch++) {
+			struct ad7606_chan_scale *cs = &st->chan_scales[ch];
 
-	ret = st->bops->sw_mode_config(indio_dev);
-	if (ret)
-		return ret;
+			cs->scale_avail = ad7616_sw_scale_avail;
+			cs->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail);
+			cs->range = 2;
+		}
 
-	num_scales_avail_show = 1;
+		ret = st->bops->sw_mode_config(indio_dev);
+		break;
+	}
 
 	for (ch = 0; ch < num_channels; ch++) {
 		struct ad7606_chan_scale *cs;
@@ -667,9 +886,16 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 	st->oversampling = 1;
 
 	cs = &st->chan_scales[0];
-	cs->range = 0;
-	cs->scale_avail = ad7606_scale_avail;
-	cs->num_scales = ARRAY_SIZE(ad7606_scale_avail);
+	switch (id) {
+	case ID_AD7606C_18:
+		cs->scale_avail = ad7606_18bit_hw_scale_avail;
+		cs->num_scales = ARRAY_SIZE(ad7606_18bit_hw_scale_avail);
+		break;
+	default:
+		cs->scale_avail = ad7606_16bit_hw_scale_avail;
+		cs->num_scales = ARRAY_SIZE(ad7606_16bit_hw_scale_avail);
+		break;
+	}
 
 	ret = devm_regulator_get_enable(dev, "avcc");
 	if (ret)
@@ -718,7 +944,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 	st->write_scale = ad7606_write_scale_hw;
 	st->write_os = ad7606_write_os_hw;
 
-	ret = ad7606_sw_mode_setup(indio_dev);
+	ret = ad7606_sw_mode_setup(indio_dev, id);
 	if (ret)
 		return ret;
 
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index d71a843a5de5..fa9305923a72 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -22,7 +22,7 @@
 		.scan_type = {					\
 			.sign = 's',				\
 			.realbits = (bits),			\
-			.storagebits = (bits),			\
+			.storagebits = (bits) > 16 ? 32 : 16,	\
 			.endianness = IIO_CPU,			\
 		},						\
 }
@@ -45,7 +45,7 @@
 		.scan_type = {					\
 			.sign = 's',				\
 			.realbits = (bits),			\
-			.storagebits = (bits),			\
+			.storagebits = (bits) > 16 ? 32 : 16,	\
 			.endianness = IIO_CPU,			\
 		},						\
 }
@@ -88,12 +88,15 @@ struct ad7606_chip_info {
  *			such that it can be read via the 'read_avail' hook
  * @num_scales		number of elements stored in the scale_avail array
  * @range		voltage range selection, selects which scale to apply
+ * @reg_offset		offset for the register value, to be applied when
+ *			writing the value of 'range' to the register value
  */
 struct ad7606_chan_scale {
 	const unsigned int		*scale_avail;
 	const unsigned int		*scale_avail_show;
 	unsigned int			num_scales;
 	unsigned int			range;
+	unsigned int			reg_offset;
 };
 
 /**
@@ -150,9 +153,13 @@ struct ad7606_state {
 	/*
 	 * DMA (thus cache coherency maintenance) may require the
 	 * transfer buffers to live in their own cache lines.
-	 * 16 * 16-bit samples + 64-bit timestamp
+	 * 16 * 16-bit samples + 64-bit timestamp - for AD7616
+	 * 8 * 32-bit samples + 64-bit timestamp - for AD7616C-18 (and similar)
 	 */
-	unsigned short			data[20] __aligned(IIO_DMA_MINALIGN);
+	union {
+		unsigned short d16[20];
+		unsigned int d32[10];
+	} data __aligned(IIO_DMA_MINALIGN);
 	__be16				d16[2];
 };
 
@@ -191,6 +198,8 @@ enum ad7606_supported_device_ids {
 	ID_AD7606_6,
 	ID_AD7606_4,
 	ID_AD7606B,
+	ID_AD7606C_16,
+	ID_AD7606C_18,
 	ID_AD7616,
 };
 
diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
index dd0075c97c24..73a7b0007bf8 100644
--- a/drivers/iio/adc/ad7606_spi.c
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -45,6 +45,8 @@
 #define AD7606_RANGE_CH_ADDR(ch)	(0x03 + ((ch) >> 1))
 #define AD7606_OS_MODE			0x08
 
+#define AD7606C_18_SAMPLE_MASK		GENMASK(17, 0)
+
 static const struct iio_chan_spec ad7616_sw_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(16),
 	AD7616_CHANNEL(0),
@@ -77,6 +79,18 @@ static const struct iio_chan_spec ad7606b_sw_channels[] = {
 	AD7606_SW_CHANNEL(7, 16),
 };
 
+static const struct iio_chan_spec ad7606c_18_sw_channels[] = {
+	IIO_CHAN_SOFT_TIMESTAMP(8),
+	AD7606_SW_CHANNEL(0, 18),
+	AD7606_SW_CHANNEL(1, 18),
+	AD7606_SW_CHANNEL(2, 18),
+	AD7606_SW_CHANNEL(3, 18),
+	AD7606_SW_CHANNEL(4, 18),
+	AD7606_SW_CHANNEL(5, 18),
+	AD7606_SW_CHANNEL(6, 18),
+	AD7606_SW_CHANNEL(7, 18),
+};
+
 static const unsigned int ad7606B_oversampling_avail[9] = {
 	1, 2, 4, 8, 16, 32, 64, 128, 256
 };
@@ -120,6 +134,56 @@ static int ad7606_spi_read_block(struct device *dev,
 	return 0;
 }
 
+static int ad7606_spi_read_block18to32(struct device *dev,
+				       int count, void *buf)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	u32 i, bit_buffer, buf_size, bit_buf_size;
+	u32 *data = buf;
+	u8 *bdata = buf;
+	int j, ret;
+
+	/**
+	 * With the 18 bit ADC variants (here) is that we can't assume that all
+	 * SPI controllers will pad 18-bit sequences into 32-bit arrays,
+	 * so we need to do a bit of buffer magic here.
+	 * Alternatively, we can have a variant of this function that works
+	 * for SPI controllers that can pad 18-bit samples into 32-bit arrays.
+	 */
+
+	/* Write 'count' bytes to the right, to not overwrite samples */
+	bdata += count;
+
+	/* Read 24 bits only, as we'll only get samples of 18 bits each */
+	buf_size = count * 3;
+	ret = spi_read(spi, bdata, buf_size);
+	if (ret < 0) {
+		dev_err(&spi->dev, "SPI read error\n");
+		return ret;
+	}
+
+	bit_buffer = 0;
+	bit_buf_size = 0;
+	for (j = 0, i = 0; i < buf_size; i++) {
+		u32 sample;
+
+		bit_buffer = (bit_buffer << 8) | bdata[i];
+		bit_buf_size += 8;
+
+		if (bit_buf_size < 18)
+			continue;
+
+		bit_buf_size -= 18;
+		sample = (bit_buffer >> bit_buf_size) & AD7606C_18_SAMPLE_MASK;
+		data[j++] = sign_extend32(sample, 17);
+
+		if (j == count)
+			break;
+	}
+
+	return 0;
+}
+
 static int ad7606_spi_reg_read(struct ad7606_state *st, unsigned int addr)
 {
 	struct spi_device *spi = to_spi_device(st->dev);
@@ -283,6 +347,19 @@ static int ad7606B_sw_mode_config(struct iio_dev *indio_dev)
 	return 0;
 }
 
+static int ad7606c_18_sw_mode_config(struct iio_dev *indio_dev)
+{
+	int ret;
+
+	ret = ad7606B_sw_mode_config(indio_dev);
+	if (ret)
+		return ret;
+
+	indio_dev->channels = ad7606c_18_sw_channels;
+
+	return 0;
+}
+
 static const struct ad7606_bus_ops ad7606_spi_bops = {
 	.read_block = ad7606_spi_read_block,
 };
@@ -305,6 +382,15 @@ static const struct ad7606_bus_ops ad7606B_spi_bops = {
 	.sw_mode_config = ad7606B_sw_mode_config,
 };
 
+static const struct ad7606_bus_ops ad7606c_18_spi_bops = {
+	.read_block = ad7606_spi_read_block18to32,
+	.reg_read = ad7606_spi_reg_read,
+	.reg_write = ad7606_spi_reg_write,
+	.write_mask = ad7606_spi_write_mask,
+	.rd_wr_cmd = ad7606B_spi_rd_wr_cmd,
+	.sw_mode_config = ad7606c_18_sw_mode_config,
+};
+
 static int ad7606_spi_probe(struct spi_device *spi)
 {
 	const struct spi_device_id *id = spi_get_device_id(spi);
@@ -315,8 +401,12 @@ static int ad7606_spi_probe(struct spi_device *spi)
 		bops = &ad7616_spi_bops;
 		break;
 	case ID_AD7606B:
+	case ID_AD7606C_16:
 		bops = &ad7606B_spi_bops;
 		break;
+	case ID_AD7606C_18:
+		bops = &ad7606c_18_spi_bops;
+		break;
 	default:
 		bops = &ad7606_spi_bops;
 		break;
@@ -333,6 +423,8 @@ static const struct spi_device_id ad7606_id_table[] = {
 	{ "ad7606-6", ID_AD7606_6 },
 	{ "ad7606-8", ID_AD7606_8 },
 	{ "ad7606b",  ID_AD7606B },
+	{ "ad7606c-16",  ID_AD7606C_16 },
+	{ "ad7606c-18",  ID_AD7606C_18 },
 	{ "ad7616",   ID_AD7616 },
 	{}
 };
@@ -344,6 +436,8 @@ static const struct of_device_id ad7606_of_match[] = {
 	{ .compatible = "adi,ad7606-6" },
 	{ .compatible = "adi,ad7606-8" },
 	{ .compatible = "adi,ad7606b" },
+	{ .compatible = "adi,ad7606c-16" },
+	{ .compatible = "adi,ad7606c-18" },
 	{ .compatible = "adi,ad7616" },
 	{ },
 };
-- 
2.46.0

Re: [PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

[kernel test robot]

Hi Alexandru,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[cannot apply to linus/master v6.11-rc4 next-20240819]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexandru-Ardelean/iio-adc-ad7606-add-bits-parameter-to-channels-macros/20240819-145028
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20240819064721.91494-8-aardelean%40baylibre.com
patch subject: [PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts
config: i386-buildonly-randconfig-001-20240819 (https://download.01.org/0day-ci/archive/20240819/202408192209.IrTzVL49-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240819/202408192209.IrTzVL49-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408192209.IrTzVL49-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/iio/adc/ad7606.c:796:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
     796 |         int ret, ch;
         |             ^
   1 warning generated.


vim +/ret +796 drivers/iio/adc/ad7606.c

94168a5789874a Alexandru Ardelean 2024-08-19  790  
94168a5789874a Alexandru Ardelean 2024-08-19  791  static int ad7606_sw_mode_setup(struct iio_dev *indio_dev, unsigned int id)
b5d2c422286d62 Alexandru Ardelean 2024-08-19  792  {
36b63bb57295f7 Alexandru Ardelean 2024-08-19  793  	unsigned int num_channels = indio_dev->num_channels - 1;
b5d2c422286d62 Alexandru Ardelean 2024-08-19  794  	struct ad7606_state *st = iio_priv(indio_dev);
09d11fa081ef17 Alexandru Ardelean 2024-08-19  795  	unsigned int *scale_avail_show, num_scales_avail_show;
09d11fa081ef17 Alexandru Ardelean 2024-08-19 @796  	int ret, ch;
b5d2c422286d62 Alexandru Ardelean 2024-08-19  797  
b5d2c422286d62 Alexandru Ardelean 2024-08-19  798  	if (!st->bops->sw_mode_config)
b5d2c422286d62 Alexandru Ardelean 2024-08-19  799  		return 0;
b5d2c422286d62 Alexandru Ardelean 2024-08-19  800  
b5d2c422286d62 Alexandru Ardelean 2024-08-19  801  	st->sw_mode_en = device_property_present(st->dev, "adi,sw-mode");
b5d2c422286d62 Alexandru Ardelean 2024-08-19  802  	if (!st->sw_mode_en)
b5d2c422286d62 Alexandru Ardelean 2024-08-19  803  		return 0;
b5d2c422286d62 Alexandru Ardelean 2024-08-19  804  
09d11fa081ef17 Alexandru Ardelean 2024-08-19  805  	indio_dev->info = &ad7606_info_sw_mode;
b5d2c422286d62 Alexandru Ardelean 2024-08-19  806  
94168a5789874a Alexandru Ardelean 2024-08-19  807  	switch (id) {
94168a5789874a Alexandru Ardelean 2024-08-19  808  	case ID_AD7606C_18:
94168a5789874a Alexandru Ardelean 2024-08-19  809  		num_scales_avail_show = num_channels;
94168a5789874a Alexandru Ardelean 2024-08-19  810  		ret = ad7606c_sw_mode_setup_channels(indio_dev,
94168a5789874a Alexandru Ardelean 2024-08-19  811  						     ad7606c_18_chan_setup);
94168a5789874a Alexandru Ardelean 2024-08-19  812  		break;
94168a5789874a Alexandru Ardelean 2024-08-19  813  	case ID_AD7606C_16:
94168a5789874a Alexandru Ardelean 2024-08-19  814  		num_scales_avail_show = num_channels;
94168a5789874a Alexandru Ardelean 2024-08-19  815  		ret = ad7606c_sw_mode_setup_channels(indio_dev,
94168a5789874a Alexandru Ardelean 2024-08-19  816  						     ad7606c_16_chan_setup);
94168a5789874a Alexandru Ardelean 2024-08-19  817  		break;
94168a5789874a Alexandru Ardelean 2024-08-19  818  	default:
94168a5789874a Alexandru Ardelean 2024-08-19  819  		num_scales_avail_show = 1;
94168a5789874a Alexandru Ardelean 2024-08-19  820  
b5d2c422286d62 Alexandru Ardelean 2024-08-19  821  		/* Scale of 0.076293 is only available in sw mode */
b5d2c422286d62 Alexandru Ardelean 2024-08-19  822  		/* After reset, in software mode, ±10 V is set by default */
36b63bb57295f7 Alexandru Ardelean 2024-08-19  823  		for (ch = 0; ch < num_channels; ch++) {
36b63bb57295f7 Alexandru Ardelean 2024-08-19  824  			struct ad7606_chan_scale *cs = &st->chan_scales[ch];
36b63bb57295f7 Alexandru Ardelean 2024-08-19  825  
36b63bb57295f7 Alexandru Ardelean 2024-08-19  826  			cs->scale_avail = ad7616_sw_scale_avail;
36b63bb57295f7 Alexandru Ardelean 2024-08-19  827  			cs->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail);
36b63bb57295f7 Alexandru Ardelean 2024-08-19  828  			cs->range = 2;
36b63bb57295f7 Alexandru Ardelean 2024-08-19  829  		}
b5d2c422286d62 Alexandru Ardelean 2024-08-19  830  
09d11fa081ef17 Alexandru Ardelean 2024-08-19  831  		ret = st->bops->sw_mode_config(indio_dev);
94168a5789874a Alexandru Ardelean 2024-08-19  832  		break;
94168a5789874a Alexandru Ardelean 2024-08-19  833  	}
09d11fa081ef17 Alexandru Ardelean 2024-08-19  834  
09d11fa081ef17 Alexandru Ardelean 2024-08-19  835  	for (ch = 0; ch < num_channels; ch++) {
09d11fa081ef17 Alexandru Ardelean 2024-08-19  836  		struct ad7606_chan_scale *cs;
09d11fa081ef17 Alexandru Ardelean 2024-08-19  837  		int i;
09d11fa081ef17 Alexandru Ardelean 2024-08-19  838  
09d11fa081ef17 Alexandru Ardelean 2024-08-19  839  		/* AD7606C supports different scales per channel */
09d11fa081ef17 Alexandru Ardelean 2024-08-19  840  		cs = &st->chan_scales[ch];
09d11fa081ef17 Alexandru Ardelean 2024-08-19  841  
09d11fa081ef17 Alexandru Ardelean 2024-08-19  842  		if (num_scales_avail_show == 1 && ch > 0) {
09d11fa081ef17 Alexandru Ardelean 2024-08-19  843  			cs->scale_avail_show = scale_avail_show;
09d11fa081ef17 Alexandru Ardelean 2024-08-19  844  			continue;
09d11fa081ef17 Alexandru Ardelean 2024-08-19  845  		}
09d11fa081ef17 Alexandru Ardelean 2024-08-19  846  
09d11fa081ef17 Alexandru Ardelean 2024-08-19  847  		scale_avail_show = devm_kcalloc(st->dev, cs->num_scales * 2,
09d11fa081ef17 Alexandru Ardelean 2024-08-19  848  						sizeof(*scale_avail_show),
09d11fa081ef17 Alexandru Ardelean 2024-08-19  849  						GFP_KERNEL);
09d11fa081ef17 Alexandru Ardelean 2024-08-19  850  		if (!scale_avail_show)
09d11fa081ef17 Alexandru Ardelean 2024-08-19  851  			return -ENOMEM;
09d11fa081ef17 Alexandru Ardelean 2024-08-19  852  
09d11fa081ef17 Alexandru Ardelean 2024-08-19  853  		/* Generate a scale_avail list for showing to userspace */
09d11fa081ef17 Alexandru Ardelean 2024-08-19  854  		for (i = 0; i < cs->num_scales; i++) {
09d11fa081ef17 Alexandru Ardelean 2024-08-19  855  			scale_avail_show[i * 2] = 0;
09d11fa081ef17 Alexandru Ardelean 2024-08-19  856  			scale_avail_show[i * 2 + 1] = cs->scale_avail[i];
09d11fa081ef17 Alexandru Ardelean 2024-08-19  857  		}
09d11fa081ef17 Alexandru Ardelean 2024-08-19  858  
09d11fa081ef17 Alexandru Ardelean 2024-08-19  859  		cs->scale_avail_show = scale_avail_show;
09d11fa081ef17 Alexandru Ardelean 2024-08-19  860  	}
09d11fa081ef17 Alexandru Ardelean 2024-08-19  861  
09d11fa081ef17 Alexandru Ardelean 2024-08-19  862  	return 0;
b5d2c422286d62 Alexandru Ardelean 2024-08-19  863  }
b5d2c422286d62 Alexandru Ardelean 2024-08-19  864  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

[David Lechner]

On 8/19/24 1:47 AM, Alexandru Ardelean wrote:
> The AD7606C-16 and AD7606C-18 are pretty similar with the AD7606B.
> The main difference between AD7606C-16 & AD7606C-18 is the precision in
> bits (16 vs 18).
> Because of that, some scales need to be defined for the 18-bit variants, as
> they need to be computed against 2**18 (vs 2**16 for the 16 bit-variants).
> 
> Because the AD7606C-16,18 also supports bipolar & differential channels,
> for SW-mode, the default range of 10 V or ±10V should be set at probe.
> On reset, the default range (in the registers) is set to value 0x3 which
> corresponds to '±10 V single-ended range', regardless of bipolar or
> differential configuration.
> 
> Aside from the scale/ranges, the AD7606C-16 is similar to the AD7606B.
> 
> And the AD7606C-18 variant offers 18-bit precision. The unfortunate effect
> of this 18-bit sample size, is that there is no simple/neat way to get the
> samples into a 32-bit array without having to do a home-brewed bit-buffer.
> The ADC must read all samples (from all 8 channels) in order to get the
> N-th sample (this could be reworked to do up-to-N-th sample for scan-direct).
> There doesn't seem to be any quick-trick to be usable to pad the samples
> up to at least 24 bits.
> Even the optional status-header is 8-bits, which would mean 26-bits of data
> per sample.
> That means that when using a simple SPI controller (which can usually read
> 8 bit multiples) a simple bit-buffer trick is required.
> 
Maybe it would be better to just use .bits_per_word = 18 for the 18-bit
ADC and not worry about "simple" SPI controller support for that one?

Re: [PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

[Alexandru Ardelean]

On Mon, Aug 19, 2024 at 6:33 PM David Lechner <dlechner@baylibre.com> wrote:
>
> On 8/19/24 1:47 AM, Alexandru Ardelean wrote:
> > The AD7606C-16 and AD7606C-18 are pretty similar with the AD7606B.
> > The main difference between AD7606C-16 & AD7606C-18 is the precision in
> > bits (16 vs 18).
> > Because of that, some scales need to be defined for the 18-bit variants, as
> > they need to be computed against 2**18 (vs 2**16 for the 16 bit-variants).
> >
> > Because the AD7606C-16,18 also supports bipolar & differential channels,
> > for SW-mode, the default range of 10 V or ±10V should be set at probe.
> > On reset, the default range (in the registers) is set to value 0x3 which
> > corresponds to '±10 V single-ended range', regardless of bipolar or
> > differential configuration.
> >
> > Aside from the scale/ranges, the AD7606C-16 is similar to the AD7606B.
> >
> > And the AD7606C-18 variant offers 18-bit precision. The unfortunate effect
> > of this 18-bit sample size, is that there is no simple/neat way to get the
> > samples into a 32-bit array without having to do a home-brewed bit-buffer.
> > The ADC must read all samples (from all 8 channels) in order to get the
> > N-th sample (this could be reworked to do up-to-N-th sample for scan-direct).
> > There doesn't seem to be any quick-trick to be usable to pad the samples
> > up to at least 24 bits.
> > Even the optional status-header is 8-bits, which would mean 26-bits of data
> > per sample.
> > That means that when using a simple SPI controller (which can usually read
> > 8 bit multiples) a simple bit-buffer trick is required.
> >
> Maybe it would be better to just use .bits_per_word = 18 for the 18-bit
> ADC and not worry about "simple" SPI controller support for that one?
>

+cc Mark Brown for some input on the SPI stuff

I'm generally fine with choosing to not support SPI controllers that
can't do padding to 16/32 bit arrays

But, at the same time: would it be an interesting topic to implement
(in the SPI framework) some SW implementation for padding a series of
18-bit samples to 32-bit arrays?
(Similarly, this could work for 10-15 bit samples into 16 bit arrays).

Apologies if this is already implemented and I missed it.

But if there isn't such a functionality (padding done in SW inside the
SPI framework), then I could probably spin-up a proposal.
I think that the functionality could be spun-up in a separate
patch-set/discussion; and this patchset would just go with
"bits_per_word = 18".

It could be done as a new field in the "struct spi_transfer", or
something else like "spi_pad_rx_to_nbits(struct spi_device *)"
Or other suggestions welcome

Thanks
Alex

Re: [PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

[David Lechner]

On 8/23/24 10:54 AM, Alexandru Ardelean wrote:
> On Mon, Aug 19, 2024 at 6:33 PM David Lechner <dlechner@baylibre.com> wrote:
>>
>> On 8/19/24 1:47 AM, Alexandru Ardelean wrote:
>>> The AD7606C-16 and AD7606C-18 are pretty similar with the AD7606B.
>>> The main difference between AD7606C-16 & AD7606C-18 is the precision in
>>> bits (16 vs 18).
>>> Because of that, some scales need to be defined for the 18-bit variants, as
>>> they need to be computed against 2**18 (vs 2**16 for the 16 bit-variants).
>>>
>>> Because the AD7606C-16,18 also supports bipolar & differential channels,
>>> for SW-mode, the default range of 10 V or ±10V should be set at probe.
>>> On reset, the default range (in the registers) is set to value 0x3 which
>>> corresponds to '±10 V single-ended range', regardless of bipolar or
>>> differential configuration.
>>>
>>> Aside from the scale/ranges, the AD7606C-16 is similar to the AD7606B.
>>>
>>> And the AD7606C-18 variant offers 18-bit precision. The unfortunate effect
>>> of this 18-bit sample size, is that there is no simple/neat way to get the
>>> samples into a 32-bit array without having to do a home-brewed bit-buffer.
>>> The ADC must read all samples (from all 8 channels) in order to get the
>>> N-th sample (this could be reworked to do up-to-N-th sample for scan-direct).
>>> There doesn't seem to be any quick-trick to be usable to pad the samples
>>> up to at least 24 bits.
>>> Even the optional status-header is 8-bits, which would mean 26-bits of data
>>> per sample.
>>> That means that when using a simple SPI controller (which can usually read
>>> 8 bit multiples) a simple bit-buffer trick is required.
>>>
>> Maybe it would be better to just use .bits_per_word = 18 for the 18-bit
>> ADC and not worry about "simple" SPI controller support for that one?
>>
> 
> +cc Mark Brown for some input on the SPI stuff
> 
> I'm generally fine with choosing to not support SPI controllers that
> can't do padding to 16/32 bit arrays
> 
> But, at the same time: would it be an interesting topic to implement
> (in the SPI framework) some SW implementation for padding a series of
> 18-bit samples to 32-bit arrays?
> (Similarly, this could work for 10-15 bit samples into 16 bit arrays).
> 
> Apologies if this is already implemented and I missed it.
> 
> But if there isn't such a functionality (padding done in SW inside the
> SPI framework), then I could probably spin-up a proposal.
> I think that the functionality could be spun-up in a separate
> patch-set/discussion; and this patchset would just go with
> "bits_per_word = 18".
> 
> It could be done as a new field in the "struct spi_transfer", or
> something else like "spi_pad_rx_to_nbits(struct spi_device *)"
> Or other suggestions welcome
> 
> Thanks
> Alex

Seems like it would be tricky to do something in the core code to
emulate "odd" sized words in general since what is permissible
likely depends on how the individual peripheral works. For example,

total_bits = xfer->bits_per_word * (xfer->len  /
	roundup_pow_of_two(BITS_TO_BYTES(xfer->bits_per_word)))

If total_bits % 8 != 0, then there will be extra trailing
clock cycles that could be problematic on some peripherals
but not others.

And there are other incompatibilities to consider, like this
could not be used with a peripheral that have the CS_WORD flag
set (highly unlikely, but still something to consider if we
are integrating this into the core).

But if you want to look into it more, another use case for this
could be SPI TFT displays. There are a number of these that use
9-bit data words. Right now emulation is handled in the peripheral
driver code. For example, see mipi_dbi_spi1e_transfer() and
fbtft_write_reg8_bus9().

Re: [PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

[Jonathan Cameron]

On Mon, 19 Aug 2024 09:47:17 +0300
Alexandru Ardelean <aardelean@baylibre.com> wrote:

> The AD7606C-16 and AD7606C-18 are pretty similar with the AD7606B.
> The main difference between AD7606C-16 & AD7606C-18 is the precision in
> bits (16 vs 18).
> Because of that, some scales need to be defined for the 18-bit variants, as
> they need to be computed against 2**18 (vs 2**16 for the 16 bit-variants).
> 
> Because the AD7606C-16,18 also supports bipolar & differential channels,
> for SW-mode, the default range of 10 V or ±10V should be set at probe.
> On reset, the default range (in the registers) is set to value 0x3 which
> corresponds to '±10 V single-ended range', regardless of bipolar or
> differential configuration.
> 
> Aside from the scale/ranges, the AD7606C-16 is similar to the AD7606B.
> 
> And the AD7606C-18 variant offers 18-bit precision. The unfortunate effect
> of this 18-bit sample size, is that there is no simple/neat way to get the
> samples into a 32-bit array without having to do a home-brewed bit-buffer.
> The ADC must read all samples (from all 8 channels) in order to get the
> N-th sample (this could be reworked to do up-to-N-th sample for scan-direct).
> There doesn't seem to be any quick-trick to be usable to pad the samples
> up to at least 24 bits.
> Even the optional status-header is 8-bits, which would mean 26-bits of data
> per sample.
> That means that when using a simple SPI controller (which can usually read
> 8 bit multiples) a simple bit-buffer trick is required.
> 
> Datasheet links:
>   https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf
>   https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf
> 
> Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>

A few minor things. If we can just start with 18 bit word spi controllers only
maybe that's worth doing to make things simpler.

> +static int ad7606c_sw_mode_setup_channels(struct iio_dev *indio_dev,
> +					  ad7606c_chan_setup_cb_t chan_setup_cb)
> +{
> +	unsigned int num_channels = indio_dev->num_channels - 1;
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +	bool chan_configured[AD760X_MAX_CHANNELS];
= {};
and drop the memset.

> +	struct device *dev = st->dev;
> +	int ret;
> +	u32 ch;
> +
> +	/* We need to hook this first */
> +	ret = st->bops->sw_mode_config(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->info = &ad7606c_info_sw_mode;
> +
> +	memset(chan_configured, 0, sizeof(chan_configured));
> +
> +	device_for_each_child_node_scoped(dev, child) {
> +		bool bipolar, differential;
> +
> +		ret = fwnode_property_read_u32(child, "reg", &ch);
> +		if (ret)
> +			continue;
> +
> +		if (ch >= num_channels) {
> +			dev_warn(st->dev,
> +				 "Invalid channel number (ignoring): %d\n", ch);
> +			continue;
> +		}
> +
> +		bipolar = fwnode_property_present(child, "bipolar");
> +		differential = fwnode_property_present(child, "diff-channel");
> +
> +		chan_setup_cb(st, ch, bipolar, differential);
> +		chan_configured[ch] = true;
> +	}
> +
> +	/* Apply default configuration to unconfigured (via DT) channels */
> +	for (ch = 0; ch < num_channels; ch++) {
> +		struct ad7606_chan_scale *cs;
> +		unsigned int *scale_avail_show;
> +		int i;
> +
> +		if (!chan_configured[ch])
> +			chan_setup_cb(st, ch, false, false);
> +
> +		/* AD7606C supports different scales per channel */
> +		cs = &st->chan_scales[ch];
> +
> +		scale_avail_show = devm_kcalloc(st->dev, cs->num_scales * 2,
> +						sizeof(*scale_avail_show),
> +						GFP_KERNEL);

Maybe just make it big enough for worst case and stick it in st always?
How big can it get?



> +		if (!scale_avail_show)
> +			return -ENOMEM;
> +
> +		/* Generate a scale_avail list for showing to userspace */
> +		for (i = 0; i < cs->num_scales; i++) {
> +			scale_avail_show[i * 2] = 0;
> +			scale_avail_show[i * 2 + 1] = cs->scale_avail[i];
> +		}
> +
> +		cs->scale_avail_show = scale_avail_show;
> +	}
> +
> +	return 0;
> +}
>

> diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
> index dd0075c97c24..73a7b0007bf8 100644
> --- a/drivers/iio/adc/ad7606_spi.c
> +++ b/drivers/iio/adc/ad7606_spi.c
> @@ -45,6 +45,8 @@

>  
> +static int ad7606_spi_read_block18to32(struct device *dev,
> +				       int count, void *buf)
> +{
> +	struct spi_device *spi = to_spi_device(dev);
> +	u32 i, bit_buffer, buf_size, bit_buf_size;
> +	u32 *data = buf;
> +	u8 *bdata = buf;
> +	int j, ret;
> +
> +	/**
Not kernel doc.  /*
> +	 * With the 18 bit ADC variants (here) is that we can't assume that all
> +	 * SPI controllers will pad 18-bit sequences into 32-bit arrays,
> +	 * so we need to do a bit of buffer magic here.
> +	 * Alternatively, we can have a variant of this function that works
> +	 * for SPI controllers that can pad 18-bit samples into 32-bit arrays.
> +	 */
> +
> +	/* Write 'count' bytes to the right, to not overwrite samples */
> +	bdata += count;
> +
> +	/* Read 24 bits only, as we'll only get samples of 18 bits each */
> +	buf_size = count * 3;
> +	ret = spi_read(spi, bdata, buf_size);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "SPI read error\n");
> +		return ret;
> +	}
> +
> +	bit_buffer = 0;
> +	bit_buf_size = 0;
> +	for (j = 0, i = 0; i < buf_size; i++) {
> +		u32 sample;
> +
> +		bit_buffer = (bit_buffer << 8) | bdata[i];
> +		bit_buf_size += 8;
> +
> +		if (bit_buf_size < 18)
> +			continue;
> +
> +		bit_buf_size -= 18;
> +		sample = (bit_buffer >> bit_buf_size) & AD7606C_18_SAMPLE_MASK;
> +		data[j++] = sign_extend32(sample, 17);
> +
> +		if (j == count)
> +			break;
> +	}
> +
> +	return 0;
> +}

Re: [PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

[Alexandru Ardelean]

On Fri, Aug 23, 2024 at 10:19 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 19 Aug 2024 09:47:17 +0300
> Alexandru Ardelean <aardelean@baylibre.com> wrote:
>
> > The AD7606C-16 and AD7606C-18 are pretty similar with the AD7606B.
> > The main difference between AD7606C-16 & AD7606C-18 is the precision in
> > bits (16 vs 18).
> > Because of that, some scales need to be defined for the 18-bit variants, as
> > they need to be computed against 2**18 (vs 2**16 for the 16 bit-variants).
> >
> > Because the AD7606C-16,18 also supports bipolar & differential channels,
> > for SW-mode, the default range of 10 V or ±10V should be set at probe.
> > On reset, the default range (in the registers) is set to value 0x3 which
> > corresponds to '±10 V single-ended range', regardless of bipolar or
> > differential configuration.
> >
> > Aside from the scale/ranges, the AD7606C-16 is similar to the AD7606B.
> >
> > And the AD7606C-18 variant offers 18-bit precision. The unfortunate effect
> > of this 18-bit sample size, is that there is no simple/neat way to get the
> > samples into a 32-bit array without having to do a home-brewed bit-buffer.
> > The ADC must read all samples (from all 8 channels) in order to get the
> > N-th sample (this could be reworked to do up-to-N-th sample for scan-direct).
> > There doesn't seem to be any quick-trick to be usable to pad the samples
> > up to at least 24 bits.
> > Even the optional status-header is 8-bits, which would mean 26-bits of data
> > per sample.
> > That means that when using a simple SPI controller (which can usually read
> > 8 bit multiples) a simple bit-buffer trick is required.
> >
> > Datasheet links:
> >   https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf
> >   https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf
> >
> > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com>
>
> A few minor things. If we can just start with 18 bit word spi controllers only
> maybe that's worth doing to make things simpler.

Will go for 18 bit word SPI controllers-only for now.

>
> > +static int ad7606c_sw_mode_setup_channels(struct iio_dev *indio_dev,
> > +                                       ad7606c_chan_setup_cb_t chan_setup_cb)
> > +{
> > +     unsigned int num_channels = indio_dev->num_channels - 1;
> > +     struct ad7606_state *st = iio_priv(indio_dev);
> > +     bool chan_configured[AD760X_MAX_CHANNELS];
> = {};
> and drop the memset.

ack

>
> > +     struct device *dev = st->dev;
> > +     int ret;
> > +     u32 ch;
> > +
> > +     /* We need to hook this first */
> > +     ret = st->bops->sw_mode_config(indio_dev);
> > +     if (ret)
> > +             return ret;
> > +
> > +     indio_dev->info = &ad7606c_info_sw_mode;
> > +
> > +     memset(chan_configured, 0, sizeof(chan_configured));
> > +
> > +     device_for_each_child_node_scoped(dev, child) {
> > +             bool bipolar, differential;
> > +
> > +             ret = fwnode_property_read_u32(child, "reg", &ch);
> > +             if (ret)
> > +                     continue;
> > +
> > +             if (ch >= num_channels) {
> > +                     dev_warn(st->dev,
> > +                              "Invalid channel number (ignoring): %d\n", ch);
> > +                     continue;
> > +             }
> > +
> > +             bipolar = fwnode_property_present(child, "bipolar");
> > +             differential = fwnode_property_present(child, "diff-channel");
> > +
> > +             chan_setup_cb(st, ch, bipolar, differential);
> > +             chan_configured[ch] = true;
> > +     }
> > +
> > +     /* Apply default configuration to unconfigured (via DT) channels */
> > +     for (ch = 0; ch < num_channels; ch++) {
> > +             struct ad7606_chan_scale *cs;
> > +             unsigned int *scale_avail_show;
> > +             int i;
> > +
> > +             if (!chan_configured[ch])
> > +                     chan_setup_cb(st, ch, false, false);
> > +
> > +             /* AD7606C supports different scales per channel */
> > +             cs = &st->chan_scales[ch];
> > +
> > +             scale_avail_show = devm_kcalloc(st->dev, cs->num_scales * 2,
> > +                                             sizeof(*scale_avail_show),
> > +                                             GFP_KERNEL);
>
> Maybe just make it big enough for worst case and stick it in st always?
> How big can it get?

So, that would be 16 channels x 8 bytes-per-scale x 5 = 640 bytes.
Not too bad.

>
>
>
> > +             if (!scale_avail_show)
> > +                     return -ENOMEM;
> > +
> > +             /* Generate a scale_avail list for showing to userspace */
> > +             for (i = 0; i < cs->num_scales; i++) {
> > +                     scale_avail_show[i * 2] = 0;
> > +                     scale_avail_show[i * 2 + 1] = cs->scale_avail[i];
> > +             }
> > +
> > +             cs->scale_avail_show = scale_avail_show;
> > +     }
> > +
> > +     return 0;
> > +}
> >
>
> > diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
> > index dd0075c97c24..73a7b0007bf8 100644
> > --- a/drivers/iio/adc/ad7606_spi.c
> > +++ b/drivers/iio/adc/ad7606_spi.c
> > @@ -45,6 +45,8 @@
>
> >
> > +static int ad7606_spi_read_block18to32(struct device *dev,
> > +                                    int count, void *buf)
> > +{
> > +     struct spi_device *spi = to_spi_device(dev);
> > +     u32 i, bit_buffer, buf_size, bit_buf_size;
> > +     u32 *data = buf;
> > +     u8 *bdata = buf;
> > +     int j, ret;
> > +
> > +     /**
> Not kernel doc.  /*
> > +      * With the 18 bit ADC variants (here) is that we can't assume that all
> > +      * SPI controllers will pad 18-bit sequences into 32-bit arrays,
> > +      * so we need to do a bit of buffer magic here.
> > +      * Alternatively, we can have a variant of this function that works
> > +      * for SPI controllers that can pad 18-bit samples into 32-bit arrays.
> > +      */
> > +
> > +     /* Write 'count' bytes to the right, to not overwrite samples */
> > +     bdata += count;
> > +
> > +     /* Read 24 bits only, as we'll only get samples of 18 bits each */
> > +     buf_size = count * 3;
> > +     ret = spi_read(spi, bdata, buf_size);
> > +     if (ret < 0) {
> > +             dev_err(&spi->dev, "SPI read error\n");
> > +             return ret;
> > +     }
> > +
> > +     bit_buffer = 0;
> > +     bit_buf_size = 0;
> > +     for (j = 0, i = 0; i < buf_size; i++) {
> > +             u32 sample;
> > +
> > +             bit_buffer = (bit_buffer << 8) | bdata[i];
> > +             bit_buf_size += 8;
> > +
> > +             if (bit_buf_size < 18)
> > +                     continue;
> > +
> > +             bit_buf_size -= 18;
> > +             sample = (bit_buffer >> bit_buf_size) & AD7606C_18_SAMPLE_MASK;
> > +             data[j++] = sign_extend32(sample, 17);
> > +
> > +             if (j == count)
> > +                     break;
> > +     }
> > +
> > +     return 0;
> > +}
>