[PATCH v3 0/4] Add support for the ad7908/ad7918/ad7928

[PATCH v3 0/4] Add support for the ad7908/ad7918/ad7928

[Daniel Junho]

This patch adds support to AD7928 family on the implementation of the
AD7923.

The ad7928 is software compatible with the ad7923.
The ad7908 and ad7918 are the 8 and 10-bit versions of the ad7928.

Change log v1 -> v2
- Split the v1 into 4 commits.
- Removes a buggy line committed unintentionally.

Change log v2 -> v3
- Added a comment about the last channel of this driver.
- Changed some macros that are used only on AD7908/AD7918/AD7928 to AD7908_*
- Simplified the MODULE_DESCRIPTION

Daniel Junho (4):
  iio: adc: ad7923: Remove the unused defines
  iio: adc: ad7923: Fix checkpatch warning
  iio: adc: ad7923: Add of_device_id table
  iio: adc: ad7923: Add support for the ad7908/ad7918/ad7928

 drivers/iio/adc/ad7923.c | 73 +++++++++++++++++++++++++++++++++-------
 1 file changed, 61 insertions(+), 12 deletions(-)

-- 
2.24.1

[PATCH v3 1/4] iio: adc: ad7923: Remove the unused defines

[Daniel Junho]

Removes the unused define AD7923_CHANNEL_x from the code.

Signed-off-by: Daniel Junho <djunho@gmail.com>
---
 drivers/iio/adc/ad7923.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index 3212eb4c0f25..969c06b7d2b7 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -29,10 +29,6 @@
 #define AD7923_PM_MODE_AS	(1)		/* auto shutdown */
 #define AD7923_PM_MODE_FS	(2)		/* full shutdown */
 #define AD7923_PM_MODE_OPS	(3)		/* normal operation */
-#define AD7923_CHANNEL_0	(0)		/* analog input 0 */
-#define AD7923_CHANNEL_1	(1)		/* analog input 1 */
-#define AD7923_CHANNEL_2	(2)		/* analog input 2 */
-#define AD7923_CHANNEL_3	(3)		/* analog input 3 */
 #define AD7923_SEQUENCE_OFF	(0)		/* no sequence fonction */
 #define AD7923_SEQUENCE_PROTECT	(2)		/* no interrupt write cycle */
 #define AD7923_SEQUENCE_ON	(3)		/* continuous sequence */
-- 
2.24.1

[PATCH v3 2/4] iio: adc: ad7923: Fix checkpatch warning

[Daniel Junho]

Fix checkpatch warning:
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
+static int ad7923_scan_direct(struct ad7923_state *st, unsigned ch)

Signed-off-by: Daniel Junho <djunho@gmail.com>
---
 drivers/iio/adc/ad7923.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index 969c06b7d2b7..e535cec9fc02 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -184,7 +184,7 @@ static irqreturn_t ad7923_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
-static int ad7923_scan_direct(struct ad7923_state *st, unsigned ch)
+static int ad7923_scan_direct(struct ad7923_state *st, unsigned int ch)
 {
 	int ret, cmd;
 
-- 
2.24.1

[PATCH v3 3/4] iio: adc: ad7923: Add of_device_id table

[Daniel Junho]

Accomplish device tree compatibility to driver AD7923
by adding of_device_id table and making a subsequent call to
MODULE_DEVICE_TABLE.

Signed-off-by: Daniel Junho <djunho@gmail.com>
---
 drivers/iio/adc/ad7923.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index e535cec9fc02..6d56fa0b9e30 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -348,9 +348,19 @@ static const struct spi_device_id ad7923_id[] = {
 };
 MODULE_DEVICE_TABLE(spi, ad7923_id);
 
+static const struct of_device_id ad7923_of_match[] = {
+	{ .compatible = "adi,ad7904", },
+	{ .compatible = "adi,ad7914", },
+	{ .compatible = "adi,ad7923", },
+	{ .compatible = "adi,ad7924", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ad7923_of_match);
+
 static struct spi_driver ad7923_driver = {
 	.driver = {
 		.name	= "ad7923",
+		.of_match_table = ad7923_of_match,
 	},
 	.probe		= ad7923_probe,
 	.remove		= ad7923_remove,
-- 
2.24.1

[PATCH v3 4/4] iio: adc: ad7923: Add support for the ad7908/ad7918/ad7928

[Daniel Junho]

The ad7928 is software compatible with the ad7923.
The ad7908 and ad7918 are the 8 and 10-bit versions of the ad7928.

Signed-off-by: Daniel Junho <djunho@gmail.com>
---
 drivers/iio/adc/ad7923.c | 57 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index 6d56fa0b9e30..1077036d1ed8 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * AD7904/AD7914/AD7923/AD7924 SPI ADC driver
+ * AD7904/AD7914/AD7923/AD7924/AD7908/AD7918/AD7928 SPI ADC driver
  *
  * Copyright 2011 Analog Devices Inc (from AD7923 Driver)
  * Copyright 2012 CS Systemes d'Information
@@ -29,11 +29,13 @@
 #define AD7923_PM_MODE_AS	(1)		/* auto shutdown */
 #define AD7923_PM_MODE_FS	(2)		/* full shutdown */
 #define AD7923_PM_MODE_OPS	(3)		/* normal operation */
-#define AD7923_SEQUENCE_OFF	(0)		/* no sequence fonction */
-#define AD7923_SEQUENCE_PROTECT	(2)		/* no interrupt write cycle */
-#define AD7923_SEQUENCE_ON	(3)		/* continuous sequence */
 
-#define AD7923_MAX_CHAN		4
+#define AD7923_SEQUENCE_OFF	(0x00)		/* no sequence function */
+#define AD7908_SEQUENCE_SHADOW	(0x01)		/* use the shadow register
+						 * for programming
+						 */
+#define AD7923_SEQUENCE_PROTECT	(0x10)		/* no interrupt write cycle */
+#define AD7923_SEQUENCE_ON	(0x11)		/* continuous sequence */
 
 #define AD7923_PM_MODE_WRITE(mode)	((mode) << 4)	 /* write mode */
 #define AD7923_CHANNEL_WRITE(channel)	((channel) << 6) /* write channel */
@@ -74,6 +76,9 @@ enum ad7923_id {
 	AD7904,
 	AD7914,
 	AD7924,
+	AD7908,
+	AD7918,
+	AD7928
 };
 
 #define AD7923_V_CHAN(index, bits)					\
@@ -102,9 +107,25 @@ const struct iio_chan_spec name ## _channels[] = { \
 	IIO_CHAN_SOFT_TIMESTAMP(4), \
 }
 
+#define DECLARE_AD7908_CHANNELS(name, bits) \
+const struct iio_chan_spec name ## _channels[] = { \
+	AD7923_V_CHAN(0, bits), \
+	AD7923_V_CHAN(1, bits), \
+	AD7923_V_CHAN(2, bits), \
+	AD7923_V_CHAN(3, bits), \
+	AD7923_V_CHAN(4, bits), \
+	AD7923_V_CHAN(5, bits), \
+	AD7923_V_CHAN(6, bits), \
+	AD7923_V_CHAN(7, bits), \
+	IIO_CHAN_SOFT_TIMESTAMP(8), \
+}
+
 static DECLARE_AD7923_CHANNELS(ad7904, 8);
 static DECLARE_AD7923_CHANNELS(ad7914, 10);
 static DECLARE_AD7923_CHANNELS(ad7924, 12);
+static DECLARE_AD7908_CHANNELS(ad7908, 8);
+static DECLARE_AD7908_CHANNELS(ad7918, 10);
+static DECLARE_AD7908_CHANNELS(ad7928, 12);
 
 static const struct ad7923_chip_info ad7923_chip_info[] = {
 	[AD7904] = {
@@ -119,6 +140,18 @@ static const struct ad7923_chip_info ad7923_chip_info[] = {
 		.channels = ad7924_channels,
 		.num_channels = ARRAY_SIZE(ad7924_channels),
 	},
+	[AD7908] = {
+		.channels = ad7908_channels,
+		.num_channels = ARRAY_SIZE(ad7908_channels),
+	},
+	[AD7918] = {
+		.channels = ad7918_channels,
+		.num_channels = ARRAY_SIZE(ad7918_channels),
+	},
+	[AD7928] = {
+		.channels = ad7928_channels,
+		.num_channels = ARRAY_SIZE(ad7928_channels),
+	},
 };
 
 /**
@@ -131,7 +164,11 @@ static int ad7923_update_scan_mode(struct iio_dev *indio_dev,
 	int i, cmd, len;
 
 	len = 0;
-	for_each_set_bit(i, active_scan_mask, AD7923_MAX_CHAN) {
+	/*
+	 * For this driver the last channel is always the software timestamp so
+	 * skip that one.
+	 */
+	for_each_set_bit(i, active_scan_mask, indio_dev->num_channels - 1) {
 		cmd = AD7923_WRITE_CR | AD7923_CHANNEL_WRITE(i) |
 			AD7923_SEQUENCE_WRITE(AD7923_SEQUENCE_OFF) |
 			st->settings;
@@ -344,6 +381,9 @@ static const struct spi_device_id ad7923_id[] = {
 	{"ad7914", AD7914},
 	{"ad7923", AD7924},
 	{"ad7924", AD7924},
+	{"ad7908", AD7908},
+	{"ad7918", AD7918},
+	{"ad7928", AD7928},
 	{}
 };
 MODULE_DEVICE_TABLE(spi, ad7923_id);
@@ -353,6 +393,9 @@ static const struct of_device_id ad7923_of_match[] = {
 	{ .compatible = "adi,ad7914", },
 	{ .compatible = "adi,ad7923", },
 	{ .compatible = "adi,ad7924", },
+	{ .compatible = "adi,ad7908", },
+	{ .compatible = "adi,ad7918", },
+	{ .compatible = "adi,ad7928", },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, ad7923_of_match);
@@ -370,5 +413,5 @@ module_spi_driver(ad7923_driver);
 
 MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
 MODULE_AUTHOR("Patrick Vasseur <patrick.vasseur@c-s.fr>");
-MODULE_DESCRIPTION("Analog Devices AD7904/AD7914/AD7923/AD7924 ADC");
+MODULE_DESCRIPTION("Analog Devices AD7923 and similar ADC");
 MODULE_LICENSE("GPL v2");
-- 
2.24.1

Re: [PATCH v3 1/4] iio: adc: ad7923: Remove the unused defines

[Ardelean, Alexandru]

On Tue, 2019-12-17 at 08:11 -0300, Daniel Junho wrote:
> Removes the unused define AD7923_CHANNEL_x from the code.
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Daniel Junho <djunho@gmail.com>
> ---
>  drivers/iio/adc/ad7923.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index 3212eb4c0f25..969c06b7d2b7 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -29,10 +29,6 @@
>  #define AD7923_PM_MODE_AS	(1)		/* auto shutdown */
>  #define AD7923_PM_MODE_FS	(2)		/* full shutdown */
>  #define AD7923_PM_MODE_OPS	(3)		/* normal operation */
> -#define AD7923_CHANNEL_0	(0)		/* analog input 0 */
> -#define AD7923_CHANNEL_1	(1)		/* analog input 1 */
> -#define AD7923_CHANNEL_2	(2)		/* analog input 2 */
> -#define AD7923_CHANNEL_3	(3)		/* analog input 3 */
>  #define AD7923_SEQUENCE_OFF	(0)		/* no sequence fonction
> */
>  #define AD7923_SEQUENCE_PROTECT	(2)		/* no interrupt
> write cycle */
>  #define AD7923_SEQUENCE_ON	(3)		/* continuous sequence */

Re: [PATCH v3 2/4] iio: adc: ad7923: Fix checkpatch warning

[Ardelean, Alexandru]

On Tue, 2019-12-17 at 08:11 -0300, Daniel Junho wrote:
> Fix checkpatch warning:
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> +static int ad7923_scan_direct(struct ad7923_state *st, unsigned ch)
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Daniel Junho <djunho@gmail.com>
> ---
>  drivers/iio/adc/ad7923.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index 969c06b7d2b7..e535cec9fc02 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -184,7 +184,7 @@ static irqreturn_t ad7923_trigger_handler(int irq,
> void *p)
>  	return IRQ_HANDLED;
>  }
>  
> -static int ad7923_scan_direct(struct ad7923_state *st, unsigned ch)
> +static int ad7923_scan_direct(struct ad7923_state *st, unsigned int ch)
>  {
>  	int ret, cmd;
>  

Re: [PATCH v3 3/4] iio: adc: ad7923: Add of_device_id table

[Ardelean, Alexandru]

On Tue, 2019-12-17 at 08:11 -0300, Daniel Junho wrote:
> Accomplish device tree compatibility to driver AD7923
> by adding of_device_id table and making a subsequent call to
> MODULE_DEVICE_TABLE.
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Daniel Junho <djunho@gmail.com>
> ---
>  drivers/iio/adc/ad7923.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index e535cec9fc02..6d56fa0b9e30 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -348,9 +348,19 @@ static const struct spi_device_id ad7923_id[] = {
>  };
>  MODULE_DEVICE_TABLE(spi, ad7923_id);
>  
> +static const struct of_device_id ad7923_of_match[] = {
> +	{ .compatible = "adi,ad7904", },
> +	{ .compatible = "adi,ad7914", },
> +	{ .compatible = "adi,ad7923", },
> +	{ .compatible = "adi,ad7924", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, ad7923_of_match);
> +
>  static struct spi_driver ad7923_driver = {
>  	.driver = {
>  		.name	= "ad7923",
> +		.of_match_table = ad7923_of_match,
>  	},
>  	.probe		= ad7923_probe,
>  	.remove		= ad7923_remove,

Re: [PATCH v3 4/4] iio: adc: ad7923: Add support for the ad7908/ad7918/ad7928

[Ardelean, Alexandru]

On Tue, 2019-12-17 at 08:11 -0300, Daniel Junho wrote:
> The ad7928 is software compatible with the ad7923.
> The ad7908 and ad7918 are the 8 and 10-bit versions of the ad7928.
> 

One minor nit.
Fine to leave it like this.

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Daniel Junho <djunho@gmail.com>
> ---
>  drivers/iio/adc/ad7923.c | 57 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index 6d56fa0b9e30..1077036d1ed8 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * AD7904/AD7914/AD7923/AD7924 SPI ADC driver
> + * AD7904/AD7914/AD7923/AD7924/AD7908/AD7918/AD7928 SPI ADC driver
>   *
>   * Copyright 2011 Analog Devices Inc (from AD7923 Driver)
>   * Copyright 2012 CS Systemes d'Information
> @@ -29,11 +29,13 @@
>  #define AD7923_PM_MODE_AS	(1)		/* auto shutdown */
>  #define AD7923_PM_MODE_FS	(2)		/* full shutdown */
>  #define AD7923_PM_MODE_OPS	(3)		/* normal operation */
> -#define AD7923_SEQUENCE_OFF	(0)		/* no sequence fonction
> */
> -#define AD7923_SEQUENCE_PROTECT	(2)		/* no interrupt
> write cycle */
> -#define AD7923_SEQUENCE_ON	(3)		/* continuous sequence */
>  
> -#define AD7923_MAX_CHAN		4
> +#define AD7923_SEQUENCE_OFF	(0x00)		/* no sequence function
> */
> +#define AD7908_SEQUENCE_SHADOW	(0x01)		/* use the shadow
> register
> +						 * for programming
> +						 */
> +#define AD7923_SEQUENCE_PROTECT	(0x10)		/* no interrupt 

These register changes seem extra.
They don't seem to do anything.
I don't mind the change though.


> write cycle */
> +#define AD7923_SEQUENCE_ON	(0x11)		/* continuous sequence */
>  
>  #define AD7923_PM_MODE_WRITE(mode)	((mode) << 4)	 /* write mode */
>  #define AD7923_CHANNEL_WRITE(channel)	((channel) << 6) /* write
> channel */
> @@ -74,6 +76,9 @@ enum ad7923_id {
>  	AD7904,
>  	AD7914,
>  	AD7924,
> +	AD7908,
> +	AD7918,
> +	AD7928
>  };
>  
>  #define AD7923_V_CHAN(index, bits)					\
> @@ -102,9 +107,25 @@ const struct iio_chan_spec name ## _channels[] = { \
>  	IIO_CHAN_SOFT_TIMESTAMP(4), \
>  }
>  
> +#define DECLARE_AD7908_CHANNELS(name, bits) \
> +const struct iio_chan_spec name ## _channels[] = { \
> +	AD7923_V_CHAN(0, bits), \
> +	AD7923_V_CHAN(1, bits), \
> +	AD7923_V_CHAN(2, bits), \
> +	AD7923_V_CHAN(3, bits), \
> +	AD7923_V_CHAN(4, bits), \
> +	AD7923_V_CHAN(5, bits), \
> +	AD7923_V_CHAN(6, bits), \
> +	AD7923_V_CHAN(7, bits), \
> +	IIO_CHAN_SOFT_TIMESTAMP(8), \
> +}
> +
>  static DECLARE_AD7923_CHANNELS(ad7904, 8);
>  static DECLARE_AD7923_CHANNELS(ad7914, 10);
>  static DECLARE_AD7923_CHANNELS(ad7924, 12);
> +static DECLARE_AD7908_CHANNELS(ad7908, 8);
> +static DECLARE_AD7908_CHANNELS(ad7918, 10);
> +static DECLARE_AD7908_CHANNELS(ad7928, 12);
>  
>  static const struct ad7923_chip_info ad7923_chip_info[] = {
>  	[AD7904] = {
> @@ -119,6 +140,18 @@ static const struct ad7923_chip_info
> ad7923_chip_info[] = {
>  		.channels = ad7924_channels,
>  		.num_channels = ARRAY_SIZE(ad7924_channels),
>  	},
> +	[AD7908] = {
> +		.channels = ad7908_channels,
> +		.num_channels = ARRAY_SIZE(ad7908_channels),
> +	},
> +	[AD7918] = {
> +		.channels = ad7918_channels,
> +		.num_channels = ARRAY_SIZE(ad7918_channels),
> +	},
> +	[AD7928] = {
> +		.channels = ad7928_channels,
> +		.num_channels = ARRAY_SIZE(ad7928_channels),
> +	},
>  };
>  
>  /**
> @@ -131,7 +164,11 @@ static int ad7923_update_scan_mode(struct iio_dev
> *indio_dev,
>  	int i, cmd, len;
>  
>  	len = 0;
> -	for_each_set_bit(i, active_scan_mask, AD7923_MAX_CHAN) {
> +	/*
> +	 * For this driver the last channel is always the software
> timestamp so
> +	 * skip that one.
> +	 */
> +	for_each_set_bit(i, active_scan_mask, indio_dev->num_channels - 1)
> {
>  		cmd = AD7923_WRITE_CR | AD7923_CHANNEL_WRITE(i) |
>  			AD7923_SEQUENCE_WRITE(AD7923_SEQUENCE_OFF) |
>  			st->settings;
> @@ -344,6 +381,9 @@ static const struct spi_device_id ad7923_id[] = {
>  	{"ad7914", AD7914},
>  	{"ad7923", AD7924},
>  	{"ad7924", AD7924},
> +	{"ad7908", AD7908},
> +	{"ad7918", AD7918},
> +	{"ad7928", AD7928},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(spi, ad7923_id);
> @@ -353,6 +393,9 @@ static const struct of_device_id ad7923_of_match[] =
> {
>  	{ .compatible = "adi,ad7914", },
>  	{ .compatible = "adi,ad7923", },
>  	{ .compatible = "adi,ad7924", },
> +	{ .compatible = "adi,ad7908", },
> +	{ .compatible = "adi,ad7918", },
> +	{ .compatible = "adi,ad7928", },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, ad7923_of_match);
> @@ -370,5 +413,5 @@ module_spi_driver(ad7923_driver);
>  
>  MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
>  MODULE_AUTHOR("Patrick Vasseur <patrick.vasseur@c-s.fr>");
> -MODULE_DESCRIPTION("Analog Devices AD7904/AD7914/AD7923/AD7924 ADC");
> +MODULE_DESCRIPTION("Analog Devices AD7923 and similar ADC");
>  MODULE_LICENSE("GPL v2");

Re: [PATCH v3 4/4] iio: adc: ad7923: Add support for the ad7908/ad7918/ad7928

[Jonathan Cameron]

On Tue, 17 Dec 2019 11:35:49 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Tue, 2019-12-17 at 08:11 -0300, Daniel Junho wrote:
> > The ad7928 is software compatible with the ad7923.
> > The ad7908 and ad7918 are the 8 and 10-bit versions of the ad7928.
> >   
> 
> One minor nit.
> Fine to leave it like this.
I disagree on that.  The change as it stands makes no sense.

It's a left over of the change to sequence write that was in the
v1 patch that I commented on and Daniel remove.
That change from using the first two bits then mapping those to
bit 3 and IIRC bit 9 to mapping bits 0 and 8 to those places.

These defines had to change to make that work.  Now we have
the changes to the defines but not the sequence function.

However, we get away with it here as the only value ever passed
in is 0.  Hence nothing happens whichever bit is in use.

Given the fix is just to drop that define block, I've done it whilst
applying.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan


> 
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
> > Signed-off-by: Daniel Junho <djunho@gmail.com>
> > ---
> >  drivers/iio/adc/ad7923.c | 57 +++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 50 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> > index 6d56fa0b9e30..1077036d1ed8 100644
> > --- a/drivers/iio/adc/ad7923.c
> > +++ b/drivers/iio/adc/ad7923.c
> > @@ -1,6 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0-only
> >  /*
> > - * AD7904/AD7914/AD7923/AD7924 SPI ADC driver
> > + * AD7904/AD7914/AD7923/AD7924/AD7908/AD7918/AD7928 SPI ADC driver
> >   *
> >   * Copyright 2011 Analog Devices Inc (from AD7923 Driver)
> >   * Copyright 2012 CS Systemes d'Information
> > @@ -29,11 +29,13 @@
> >  #define AD7923_PM_MODE_AS	(1)		/* auto shutdown */
> >  #define AD7923_PM_MODE_FS	(2)		/* full shutdown */
> >  #define AD7923_PM_MODE_OPS	(3)		/* normal operation */
> > -#define AD7923_SEQUENCE_OFF	(0)		/* no sequence fonction
> > */
> > -#define AD7923_SEQUENCE_PROTECT	(2)		/* no interrupt
> > write cycle */
> > -#define AD7923_SEQUENCE_ON	(3)		/* continuous sequence */
> >  
> > -#define AD7923_MAX_CHAN		4
> > +#define AD7923_SEQUENCE_OFF	(0x00)		/* no sequence function
> > */
> > +#define AD7908_SEQUENCE_SHADOW	(0x01)		/* use the shadow
> > register
> > +						 * for programming
> > +						 */
> > +#define AD7923_SEQUENCE_PROTECT	(0x10)		/* no interrupt   
> 
> These register changes seem extra.
> They don't seem to do anything.
> I don't mind the change though.
> 
> 
> > write cycle */
> > +#define AD7923_SEQUENCE_ON	(0x11)		/* continuous sequence */
> >  
> >  #define AD7923_PM_MODE_WRITE(mode)	((mode) << 4)	 /* write mode */
> >  #define AD7923_CHANNEL_WRITE(channel)	((channel) << 6) /* write
> > channel */
> > @@ -74,6 +76,9 @@ enum ad7923_id {
> >  	AD7904,
> >  	AD7914,
> >  	AD7924,
> > +	AD7908,
> > +	AD7918,
> > +	AD7928
> >  };
> >  
> >  #define AD7923_V_CHAN(index, bits)					\
> > @@ -102,9 +107,25 @@ const struct iio_chan_spec name ## _channels[] = { \
> >  	IIO_CHAN_SOFT_TIMESTAMP(4), \
> >  }
> >  
> > +#define DECLARE_AD7908_CHANNELS(name, bits) \
> > +const struct iio_chan_spec name ## _channels[] = { \
> > +	AD7923_V_CHAN(0, bits), \
> > +	AD7923_V_CHAN(1, bits), \
> > +	AD7923_V_CHAN(2, bits), \
> > +	AD7923_V_CHAN(3, bits), \
> > +	AD7923_V_CHAN(4, bits), \
> > +	AD7923_V_CHAN(5, bits), \
> > +	AD7923_V_CHAN(6, bits), \
> > +	AD7923_V_CHAN(7, bits), \
> > +	IIO_CHAN_SOFT_TIMESTAMP(8), \
> > +}
> > +
> >  static DECLARE_AD7923_CHANNELS(ad7904, 8);
> >  static DECLARE_AD7923_CHANNELS(ad7914, 10);
> >  static DECLARE_AD7923_CHANNELS(ad7924, 12);
> > +static DECLARE_AD7908_CHANNELS(ad7908, 8);
> > +static DECLARE_AD7908_CHANNELS(ad7918, 10);
> > +static DECLARE_AD7908_CHANNELS(ad7928, 12);
> >  
> >  static const struct ad7923_chip_info ad7923_chip_info[] = {
> >  	[AD7904] = {
> > @@ -119,6 +140,18 @@ static const struct ad7923_chip_info
> > ad7923_chip_info[] = {
> >  		.channels = ad7924_channels,
> >  		.num_channels = ARRAY_SIZE(ad7924_channels),
> >  	},
> > +	[AD7908] = {
> > +		.channels = ad7908_channels,
> > +		.num_channels = ARRAY_SIZE(ad7908_channels),
> > +	},
> > +	[AD7918] = {
> > +		.channels = ad7918_channels,
> > +		.num_channels = ARRAY_SIZE(ad7918_channels),
> > +	},
> > +	[AD7928] = {
> > +		.channels = ad7928_channels,
> > +		.num_channels = ARRAY_SIZE(ad7928_channels),
> > +	},
> >  };
> >  
> >  /**
> > @@ -131,7 +164,11 @@ static int ad7923_update_scan_mode(struct iio_dev
> > *indio_dev,
> >  	int i, cmd, len;
> >  
> >  	len = 0;
> > -	for_each_set_bit(i, active_scan_mask, AD7923_MAX_CHAN) {
> > +	/*
> > +	 * For this driver the last channel is always the software
> > timestamp so
> > +	 * skip that one.
> > +	 */
> > +	for_each_set_bit(i, active_scan_mask, indio_dev->num_channels - 1)
> > {
> >  		cmd = AD7923_WRITE_CR | AD7923_CHANNEL_WRITE(i) |
> >  			AD7923_SEQUENCE_WRITE(AD7923_SEQUENCE_OFF) |
> >  			st->settings;
> > @@ -344,6 +381,9 @@ static const struct spi_device_id ad7923_id[] = {
> >  	{"ad7914", AD7914},
> >  	{"ad7923", AD7924},
> >  	{"ad7924", AD7924},
> > +	{"ad7908", AD7908},
> > +	{"ad7918", AD7918},
> > +	{"ad7928", AD7928},
> >  	{}
> >  };
> >  MODULE_DEVICE_TABLE(spi, ad7923_id);
> > @@ -353,6 +393,9 @@ static const struct of_device_id ad7923_of_match[] =
> > {
> >  	{ .compatible = "adi,ad7914", },
> >  	{ .compatible = "adi,ad7923", },
> >  	{ .compatible = "adi,ad7924", },
> > +	{ .compatible = "adi,ad7908", },
> > +	{ .compatible = "adi,ad7918", },
> > +	{ .compatible = "adi,ad7928", },
> >  	{ },
> >  };
> >  MODULE_DEVICE_TABLE(of, ad7923_of_match);
> > @@ -370,5 +413,5 @@ module_spi_driver(ad7923_driver);
> >  
> >  MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
> >  MODULE_AUTHOR("Patrick Vasseur <patrick.vasseur@c-s.fr>");
> > -MODULE_DESCRIPTION("Analog Devices AD7904/AD7914/AD7923/AD7924 ADC");
> > +MODULE_DESCRIPTION("Analog Devices AD7923 and similar ADC");
> >  MODULE_LICENSE("GPL v2");  

Re: [PATCH v3 1/4] iio: adc: ad7923: Remove the unused defines

[Jonathan Cameron]

On Tue, 17 Dec 2019 11:29:57 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Tue, 2019-12-17 at 08:11 -0300, Daniel Junho wrote:
> > Removes the unused define AD7923_CHANNEL_x from the code.
> >   
> 
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied.

Thanks,

Jonathan

> 
> > Signed-off-by: Daniel Junho <djunho@gmail.com>
> > ---
> >  drivers/iio/adc/ad7923.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> > index 3212eb4c0f25..969c06b7d2b7 100644
> > --- a/drivers/iio/adc/ad7923.c
> > +++ b/drivers/iio/adc/ad7923.c
> > @@ -29,10 +29,6 @@
> >  #define AD7923_PM_MODE_AS	(1)		/* auto shutdown */
> >  #define AD7923_PM_MODE_FS	(2)		/* full shutdown */
> >  #define AD7923_PM_MODE_OPS	(3)		/* normal operation */
> > -#define AD7923_CHANNEL_0	(0)		/* analog input 0 */
> > -#define AD7923_CHANNEL_1	(1)		/* analog input 1 */
> > -#define AD7923_CHANNEL_2	(2)		/* analog input 2 */
> > -#define AD7923_CHANNEL_3	(3)		/* analog input 3 */
> >  #define AD7923_SEQUENCE_OFF	(0)		/* no sequence fonction
> > */
> >  #define AD7923_SEQUENCE_PROTECT	(2)		/* no interrupt
> > write cycle */
> >  #define AD7923_SEQUENCE_ON	(3)		/* continuous sequence */  

Re: [PATCH v3 2/4] iio: adc: ad7923: Fix checkpatch warning

[Jonathan Cameron]

On Tue, 17 Dec 2019 11:30:09 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Tue, 2019-12-17 at 08:11 -0300, Daniel Junho wrote:
> > Fix checkpatch warning:
> > WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> > +static int ad7923_scan_direct(struct ad7923_state *st, unsigned ch)
> >   
> 
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Applied,

Thanks,

Jonathan

> 
> > Signed-off-by: Daniel Junho <djunho@gmail.com>
> > ---
> >  drivers/iio/adc/ad7923.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> > index 969c06b7d2b7..e535cec9fc02 100644
> > --- a/drivers/iio/adc/ad7923.c
> > +++ b/drivers/iio/adc/ad7923.c
> > @@ -184,7 +184,7 @@ static irqreturn_t ad7923_trigger_handler(int irq,
> > void *p)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static int ad7923_scan_direct(struct ad7923_state *st, unsigned ch)
> > +static int ad7923_scan_direct(struct ad7923_state *st, unsigned int ch)
> >  {
> >  	int ret, cmd;
> >    

Re: [PATCH v3 3/4] iio: adc: ad7923: Add of_device_id table

[Jonathan Cameron]

On Tue, 17 Dec 2019 11:30:56 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Tue, 2019-12-17 at 08:11 -0300, Daniel Junho wrote:
> > Accomplish device tree compatibility to driver AD7923
> > by adding of_device_id table and making a subsequent call to
> > MODULE_DEVICE_TABLE.
> >   
> 
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> 
> > Signed-off-by: Daniel Junho <djunho@gmail.com>
> > ---
> >  drivers/iio/adc/ad7923.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> > index e535cec9fc02..6d56fa0b9e30 100644
> > --- a/drivers/iio/adc/ad7923.c
> > +++ b/drivers/iio/adc/ad7923.c
> > @@ -348,9 +348,19 @@ static const struct spi_device_id ad7923_id[] = {
> >  };
> >  MODULE_DEVICE_TABLE(spi, ad7923_id);
> >  
> > +static const struct of_device_id ad7923_of_match[] = {
> > +	{ .compatible = "adi,ad7904", },
> > +	{ .compatible = "adi,ad7914", },
> > +	{ .compatible = "adi,ad7923", },
> > +	{ .compatible = "adi,ad7924", },
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(of, ad7923_of_match);
> > +
> >  static struct spi_driver ad7923_driver = {
> >  	.driver = {
> >  		.name	= "ad7923",
> > +		.of_match_table = ad7923_of_match,
> >  	},
> >  	.probe		= ad7923_probe,
> >  	.remove		= ad7923_remove,