Re: [PATCH v2 1/2] iio: adc: Add AD7768-1 ADC basic support

2019-01-21 Thread Jonathan Cameron
On Mon, 21 Jan 2019 12:33:16 +
"Popa, Stefan Serban"  wrote:

> On Sb, 2019-01-19 at 17:55 +, Jonathan Cameron wrote:
> > Use caution when clicking links or attachments as this email originated
> > outside of Analog Devices.
> > 
> > 
> > On Tue, 15 Jan 2019 14:33:11 +0200
> > Stefan Popa  wrote:
> >   
> > > 
> > > The ad7768-1 is a single channel, precision 24-bit analog to digital
> > > converter (ADC).
> > > 
> > > This basic patch configures the device in fast mode, with 32 kSPS and
> > > leaves the default sinc5 filter.
> > > 
> > > Two data conversion modes are made available. When data is retrieved by
> > > using the read_raw attribute, one shot single conversion mode is set.
> > > The continuous conversion mode is enabled when the triggered buffer
> > > mechanism is used. To assure correct data retrieval, the driver waits
> > > for the interrupt triggered by the low to high transition of the DRDY
> > > pin.
> > > 
> > > Datasheets:
> > > Link: https://www.analog.com/media/en/technical-documentation/data-shee
> > > ts/ad7768-1.pdf
> > > 
> > > Signed-off-by: Stefan Popa   
> > Hi Stefan,
> > 
> > Driver is fine, but the licensing needs clarification.
> > "GPL" in module license means GPL v2 or later, but the SPDX is GPL v2
> > only.
> > 
> > I had a question out to Michael for some of his older drivers on
> > exactly this.  We need to fix this one and tidy those up to
> > be clear one way or the other.
> > If you just let me know which one is right, I'll fix it up rather than
> > you having
> > to send a v3.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> Hi Jonathan,
> 
> Thank you for your review. Michael informed me that our drivers are
> typically GPL v2 only.
> 
> I will check the other drivers that we've sent upstream and provide a patch
> to fix the wrong licenses. 

Great, thanks!  It's some of the really old ones in staging that we noticed this
in IIRC.

Jonathan

> 
> -Stefan
> 
> > > 
> > > ---
> > > Changes in v2:
> > >   - Added values to all the elements of ad7768_pwrmode enum.
> > >   - Removed the ad7768_ids enum, as the driver supports only one
> > > device.
> > >   - Added a new data union which is part of the ad7768_state
> > > struct. This
> > > union, now includes a d8 field.
> > >   - Used spi_write_then_read() in ad7768_spi_reg_read().
> > >   - Called spi_read() instead of spi_sync_transfer() in
> > > ad7768_trigger_handler().
> > >   - Used the devm_request_irq() instead of
> > > devm_request_threaded_irq(); called
> > > iio_trigger_poll() instead of iio_trigger_poll_chained().
> > > 
> > >  MAINTAINERS|   7 +
> > >  drivers/iio/adc/Kconfig|  13 ++
> > >  drivers/iio/adc/Makefile   |   1 +
> > >  drivers/iio/adc/ad7768-1.c | 459
> > > +
> > >  4 files changed, 480 insertions(+)
> > >  create mode 100644 drivers/iio/adc/ad7768-1.c
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index d039f66..3ba3811 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -862,6 +862,13 @@ S:   Supported
> > >  F:   drivers/iio/adc/ad7606.c
> > >  F:   Documentation/devicetree/bindings/iio/adc/ad7606.txt
> > > 
> > > +ANALOG DEVICES INC AD7768-1 DRIVER
> > > +M:   Stefan Popa 
> > > +L:   linux-...@vger.kernel.org
> > > +W:   http://ez.analog.com/community/linux-device-drivers
> > > +S:   Supported
> > > +F:   drivers/iio/adc/ad7768-1.c
> > > +
> > >  ANALOG DEVICES INC AD9389B DRIVER
> > >  M:   Hans Verkuil 
> > >  L:   linux-me...@vger.kernel.org
> > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > > index f3cc7a3..6c19dfe 100644
> > > --- a/drivers/iio/adc/Kconfig
> > > +++ b/drivers/iio/adc/Kconfig
> > > @@ -108,6 +108,19 @@ config AD7766
> > > To compile this driver as a module, choose M here: the module
> > > will be
> > > called ad7766.
> > > 
> > > +config AD7768_1
> > > + tristate "Analog Devices AD7768-1 ADC driver"
> > > + depends on SPI
> > > + select IIO_BUFFER
> > > + select IIO_TRIGGER
> > > + select IIO_TRIGGERED_BUFFER
> > > + help
> > > +   Say yes here to build support for Analog Devices AD7768-1 SPI
> > > +   simultaneously sampling sigma-delta analog to digital converter
> > > (ADC).
> > > +
> > > +   To compile this driver as a module, choose M here: the module
> > > will be
> > > +   called ad7768-1.
> > > +
> > >  config AD7791
> > >   tristate "Analog Devices AD7791 ADC driver"
> > >   depends on SPI
> > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > > index ea50313..9d50f7b 100644
> > > --- a/drivers/iio/adc/Makefile
> > > +++ b/drivers/iio/adc/Makefile
> > > @@ -15,6 +15,7 @@ obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
> > >  obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
> > >  obj-$(CONFIG_AD7606) += ad7606.o
> > >  obj-$(CONFIG_AD7766) += ad7766.o
> > > +obj-$(CONFIG_AD7768_1) += ad7768-1.o
> > >  obj-$(CONFIG_AD7791) 

Re: [PATCH v2 1/2] iio: adc: Add AD7768-1 ADC basic support

2019-01-21 Thread Popa, Stefan Serban
On Sb, 2019-01-19 at 17:55 +, Jonathan Cameron wrote:
> Use caution when clicking links or attachments as this email originated
> outside of Analog Devices.
> 
> 
> On Tue, 15 Jan 2019 14:33:11 +0200
> Stefan Popa  wrote:
> 
> > 
> > The ad7768-1 is a single channel, precision 24-bit analog to digital
> > converter (ADC).
> > 
> > This basic patch configures the device in fast mode, with 32 kSPS and
> > leaves the default sinc5 filter.
> > 
> > Two data conversion modes are made available. When data is retrieved by
> > using the read_raw attribute, one shot single conversion mode is set.
> > The continuous conversion mode is enabled when the triggered buffer
> > mechanism is used. To assure correct data retrieval, the driver waits
> > for the interrupt triggered by the low to high transition of the DRDY
> > pin.
> > 
> > Datasheets:
> > Link: https://www.analog.com/media/en/technical-documentation/data-shee
> > ts/ad7768-1.pdf
> > 
> > Signed-off-by: Stefan Popa 
> Hi Stefan,
> 
> Driver is fine, but the licensing needs clarification.
> "GPL" in module license means GPL v2 or later, but the SPDX is GPL v2
> only.
> 
> I had a question out to Michael for some of his older drivers on
> exactly this.  We need to fix this one and tidy those up to
> be clear one way or the other.
> If you just let me know which one is right, I'll fix it up rather than
> you having
> to send a v3.
> 
> Thanks,
> 
> Jonathan
> 
Hi Jonathan,

Thank you for your review. Michael informed me that our drivers are
typically GPL v2 only.

I will check the other drivers that we've sent upstream and provide a patch
to fix the wrong licenses. 

-Stefan

> > 
> > ---
> > Changes in v2:
> >   - Added values to all the elements of ad7768_pwrmode enum.
> >   - Removed the ad7768_ids enum, as the driver supports only one
> > device.
> >   - Added a new data union which is part of the ad7768_state
> > struct. This
> > union, now includes a d8 field.
> >   - Used spi_write_then_read() in ad7768_spi_reg_read().
> >   - Called spi_read() instead of spi_sync_transfer() in
> > ad7768_trigger_handler().
> >   - Used the devm_request_irq() instead of
> > devm_request_threaded_irq(); called
> > iio_trigger_poll() instead of iio_trigger_poll_chained().
> > 
> >  MAINTAINERS|   7 +
> >  drivers/iio/adc/Kconfig|  13 ++
> >  drivers/iio/adc/Makefile   |   1 +
> >  drivers/iio/adc/ad7768-1.c | 459
> > +
> >  4 files changed, 480 insertions(+)
> >  create mode 100644 drivers/iio/adc/ad7768-1.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d039f66..3ba3811 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -862,6 +862,13 @@ S:   Supported
> >  F:   drivers/iio/adc/ad7606.c
> >  F:   Documentation/devicetree/bindings/iio/adc/ad7606.txt
> > 
> > +ANALOG DEVICES INC AD7768-1 DRIVER
> > +M:   Stefan Popa 
> > +L:   linux-...@vger.kernel.org
> > +W:   http://ez.analog.com/community/linux-device-drivers
> > +S:   Supported
> > +F:   drivers/iio/adc/ad7768-1.c
> > +
> >  ANALOG DEVICES INC AD9389B DRIVER
> >  M:   Hans Verkuil 
> >  L:   linux-me...@vger.kernel.org
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index f3cc7a3..6c19dfe 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -108,6 +108,19 @@ config AD7766
> > To compile this driver as a module, choose M here: the module
> > will be
> > called ad7766.
> > 
> > +config AD7768_1
> > + tristate "Analog Devices AD7768-1 ADC driver"
> > + depends on SPI
> > + select IIO_BUFFER
> > + select IIO_TRIGGER
> > + select IIO_TRIGGERED_BUFFER
> > + help
> > +   Say yes here to build support for Analog Devices AD7768-1 SPI
> > +   simultaneously sampling sigma-delta analog to digital converter
> > (ADC).
> > +
> > +   To compile this driver as a module, choose M here: the module
> > will be
> > +   called ad7768-1.
> > +
> >  config AD7791
> >   tristate "Analog Devices AD7791 ADC driver"
> >   depends on SPI
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index ea50313..9d50f7b 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -15,6 +15,7 @@ obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
> >  obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
> >  obj-$(CONFIG_AD7606) += ad7606.o
> >  obj-$(CONFIG_AD7766) += ad7766.o
> > +obj-$(CONFIG_AD7768_1) += ad7768-1.o
> >  obj-$(CONFIG_AD7791) += ad7791.o
> >  obj-$(CONFIG_AD7793) += ad7793.o
> >  obj-$(CONFIG_AD7887) += ad7887.o
> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > new file mode 100644
> > index 000..fdcb966
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -0,0 +1,459 @@
> > +// SPDX-License-Identifier: GPL-2.0
> This is GPl v2 only.  The MODULE_LICENSE is GPL v2 or later.
> 
> > 
> > +/*
> > + * Analog Devices 

Re: [PATCH v2 1/2] iio: adc: Add AD7768-1 ADC basic support

2019-01-19 Thread Jonathan Cameron
On Tue, 15 Jan 2019 14:33:11 +0200
Stefan Popa  wrote:

> The ad7768-1 is a single channel, precision 24-bit analog to digital
> converter (ADC).
> 
> This basic patch configures the device in fast mode, with 32 kSPS and
> leaves the default sinc5 filter.
> 
> Two data conversion modes are made available. When data is retrieved by
> using the read_raw attribute, one shot single conversion mode is set.
> The continuous conversion mode is enabled when the triggered buffer
> mechanism is used. To assure correct data retrieval, the driver waits
> for the interrupt triggered by the low to high transition of the DRDY
> pin.
> 
> Datasheets:
> Link: 
> https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf
> 
> Signed-off-by: Stefan Popa 
Hi Stefan,

Driver is fine, but the licensing needs clarification.
"GPL" in module license means GPL v2 or later, but the SPDX is GPL v2 only.

I had a question out to Michael for some of his older drivers on
exactly this.  We need to fix this one and tidy those up to
be clear one way or the other.

If you just let me know which one is right, I'll fix it up rather than you 
having
to send a v3.

Thanks,

Jonathan

> ---
> Changes in v2:
>   - Added values to all the elements of ad7768_pwrmode enum.
>   - Removed the ad7768_ids enum, as the driver supports only one device.
>   - Added a new data union which is part of the ad7768_state struct. This 
> union, now includes a d8 field.
>   - Used spi_write_then_read() in ad7768_spi_reg_read().
>   - Called spi_read() instead of spi_sync_transfer() in 
> ad7768_trigger_handler().
>   - Used the devm_request_irq() instead of devm_request_threaded_irq(); 
> called
> iio_trigger_poll() instead of iio_trigger_poll_chained().
> 
>  MAINTAINERS|   7 +
>  drivers/iio/adc/Kconfig|  13 ++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/ad7768-1.c | 459 
> +
>  4 files changed, 480 insertions(+)
>  create mode 100644 drivers/iio/adc/ad7768-1.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d039f66..3ba3811 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -862,6 +862,13 @@ S:   Supported
>  F:   drivers/iio/adc/ad7606.c
>  F:   Documentation/devicetree/bindings/iio/adc/ad7606.txt
>  
> +ANALOG DEVICES INC AD7768-1 DRIVER
> +M:   Stefan Popa 
> +L:   linux-...@vger.kernel.org
> +W:   http://ez.analog.com/community/linux-device-drivers
> +S:   Supported
> +F:   drivers/iio/adc/ad7768-1.c
> +
>  ANALOG DEVICES INC AD9389B DRIVER
>  M:   Hans Verkuil 
>  L:   linux-me...@vger.kernel.org
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f3cc7a3..6c19dfe 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -108,6 +108,19 @@ config AD7766
> To compile this driver as a module, choose M here: the module will be
> called ad7766.
>  
> +config AD7768_1
> + tristate "Analog Devices AD7768-1 ADC driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGER
> + select IIO_TRIGGERED_BUFFER
> + help
> +   Say yes here to build support for Analog Devices AD7768-1 SPI
> +   simultaneously sampling sigma-delta analog to digital converter (ADC).
> +
> +   To compile this driver as a module, choose M here: the module will be
> +   called ad7768-1.
> +
>  config AD7791
>   tristate "Analog Devices AD7791 ADC driver"
>   depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ea50313..9d50f7b 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
>  obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
>  obj-$(CONFIG_AD7606) += ad7606.o
>  obj-$(CONFIG_AD7766) += ad7766.o
> +obj-$(CONFIG_AD7768_1) += ad7768-1.o
>  obj-$(CONFIG_AD7791) += ad7791.o
>  obj-$(CONFIG_AD7793) += ad7793.o
>  obj-$(CONFIG_AD7887) += ad7887.o
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> new file mode 100644
> index 000..fdcb966
> --- /dev/null
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -0,0 +1,459 @@
> +// SPDX-License-Identifier: GPL-2.0

This is GPl v2 only.  The MODULE_LICENSE is GPL v2 or later.

> +/*
> + * Analog Devices AD7768-1 SPI ADC driver
> + *
> + * Copyright 2017 Analog Devices Inc.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* AD7768 registers definition */
> +#define AD7768_REG_CHIP_TYPE 0x3
> +#define AD7768_REG_PROD_ID_L 0x4
> +#define AD7768_REG_PROD_ID_H 0x5
> +#define AD7768_REG_CHIP_GRADE0x6
> +#define AD7768_REG_SCRATCH_PAD   0x0A
> +#define AD7768_REG_VENDOR_L  0x0C
> +#define AD7768_REG_VENDOR_H  0x0D
> +#define 

[PATCH v2 1/2] iio: adc: Add AD7768-1 ADC basic support

2019-01-15 Thread Stefan Popa
The ad7768-1 is a single channel, precision 24-bit analog to digital
converter (ADC).

This basic patch configures the device in fast mode, with 32 kSPS and
leaves the default sinc5 filter.

Two data conversion modes are made available. When data is retrieved by
using the read_raw attribute, one shot single conversion mode is set.
The continuous conversion mode is enabled when the triggered buffer
mechanism is used. To assure correct data retrieval, the driver waits
for the interrupt triggered by the low to high transition of the DRDY
pin.

Datasheets:
Link: 
https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf

Signed-off-by: Stefan Popa 
---
Changes in v2:
- Added values to all the elements of ad7768_pwrmode enum.
- Removed the ad7768_ids enum, as the driver supports only one device.
- Added a new data union which is part of the ad7768_state struct. This 
  union, now includes a d8 field.
- Used spi_write_then_read() in ad7768_spi_reg_read().
- Called spi_read() instead of spi_sync_transfer() in 
ad7768_trigger_handler().
- Used the devm_request_irq() instead of devm_request_threaded_irq(); 
called
  iio_trigger_poll() instead of iio_trigger_poll_chained().

 MAINTAINERS|   7 +
 drivers/iio/adc/Kconfig|  13 ++
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/ad7768-1.c | 459 +
 4 files changed, 480 insertions(+)
 create mode 100644 drivers/iio/adc/ad7768-1.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d039f66..3ba3811 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -862,6 +862,13 @@ S: Supported
 F: drivers/iio/adc/ad7606.c
 F: Documentation/devicetree/bindings/iio/adc/ad7606.txt
 
+ANALOG DEVICES INC AD7768-1 DRIVER
+M: Stefan Popa 
+L: linux-...@vger.kernel.org
+W: http://ez.analog.com/community/linux-device-drivers
+S: Supported
+F: drivers/iio/adc/ad7768-1.c
+
 ANALOG DEVICES INC AD9389B DRIVER
 M: Hans Verkuil 
 L: linux-me...@vger.kernel.org
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f3cc7a3..6c19dfe 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -108,6 +108,19 @@ config AD7766
  To compile this driver as a module, choose M here: the module will be
  called ad7766.
 
+config AD7768_1
+   tristate "Analog Devices AD7768-1 ADC driver"
+   depends on SPI
+   select IIO_BUFFER
+   select IIO_TRIGGER
+   select IIO_TRIGGERED_BUFFER
+   help
+ Say yes here to build support for Analog Devices AD7768-1 SPI
+ simultaneously sampling sigma-delta analog to digital converter (ADC).
+
+ To compile this driver as a module, choose M here: the module will be
+ called ad7768-1.
+
 config AD7791
tristate "Analog Devices AD7791 ADC driver"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ea50313..9d50f7b 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
 obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
 obj-$(CONFIG_AD7606) += ad7606.o
 obj-$(CONFIG_AD7766) += ad7766.o
+obj-$(CONFIG_AD7768_1) += ad7768-1.o
 obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7793) += ad7793.o
 obj-$(CONFIG_AD7887) += ad7887.o
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
new file mode 100644
index 000..fdcb966
--- /dev/null
+++ b/drivers/iio/adc/ad7768-1.c
@@ -0,0 +1,459 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Analog Devices AD7768-1 SPI ADC driver
+ *
+ * Copyright 2017 Analog Devices Inc.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* AD7768 registers definition */
+#define AD7768_REG_CHIP_TYPE   0x3
+#define AD7768_REG_PROD_ID_L   0x4
+#define AD7768_REG_PROD_ID_H   0x5
+#define AD7768_REG_CHIP_GRADE  0x6
+#define AD7768_REG_SCRATCH_PAD 0x0A
+#define AD7768_REG_VENDOR_L0x0C
+#define AD7768_REG_VENDOR_H0x0D
+#define AD7768_REG_INTERFACE_FORMAT0x14
+#define AD7768_REG_POWER_CLOCK 0x15
+#define AD7768_REG_ANALOG  0x16
+#define AD7768_REG_ANALOG2 0x17
+#define AD7768_REG_CONVERSION  0x18
+#define AD7768_REG_DIGITAL_FILTER  0x19
+#define AD7768_REG_SINC3_DEC_RATE_MSB  0x1A
+#define AD7768_REG_SINC3_DEC_RATE_LSB  0x1B
+#define AD7768_REG_DUTY_CYCLE_RATIO0x1C
+#define AD7768_REG_SYNC_RESET  0x1D
+#define AD7768_REG_GPIO_CONTROL0x1E
+#define AD7768_REG_GPIO_WRITE  0x1F
+#define AD7768_REG_GPIO_READ   0x20
+#define AD7768_REG_OFFSET_HI   0x21
+#define AD7768_REG_OFFSET_MID  0x22
+#define AD7768_REG_OFFSET_LO   0x23
+#define AD7768_REG_GAIN_HI