[PATCH] iio: ad_sigma_delta: Add custom irq flags

[PATCH] iio: ad_sigma_delta: Add custom irq flags

[Alexandru Tachici]

The data-sheet of AD7124, from the Sigma-Delta ADC family,
recommends that the falling edge of the DOUT line should be used for
an interrupt.

The ad_sigma_delta implementation hardcodes the irq trigger type
to low, assuming that all Sigma-Delta ADCs have the same interrupt-type.
This causes unwanted behaviour. If DOUT line is already low, the
interrupt will fire once, when enabled and the irq handler will send a
read request to the device. At this time the device has not yet finished
the previous conversion and will give a bad reading.

This patch allows drivers using the ad_sigma_delta layer to set the
irq trigger type to the one specified in the corresponding data-sheet.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/iio/adc/ad_sigma_delta.c       | 9 ++++++++-
 include/linux/iio/adc/ad_sigma_delta.h | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index d10bd0c97233..0007df8d50fb 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -454,7 +454,7 @@ static int ad_sd_probe_trigger(struct iio_dev *indio_dev)
 
 	ret = request_irq(sigma_delta->spi->irq,
 			  ad_sd_data_rdy_trig_poll,
-			  IRQF_TRIGGER_LOW,
+			  sigma_delta->irq_flags,
 			  indio_dev->name,
 			  sigma_delta);
 	if (ret)
@@ -540,8 +540,15 @@ EXPORT_SYMBOL_GPL(ad_sd_cleanup_buffer_and_trigger);
 int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
 	struct spi_device *spi, const struct ad_sigma_delta_info *info)
 {
+	unsigned long set_trigger_flags;
+
 	sigma_delta->spi = spi;
 	sigma_delta->info = info;
+
+	set_trigger_flags = sigma_delta->irq_flags & IRQF_TRIGGER_MASK;
+	if (set_trigger_flags == IRQF_TRIGGER_NONE)
+		sigma_delta->irq_flags |= IRQF_TRIGGER_LOW;
+
 	iio_device_set_drvdata(indio_dev, sigma_delta);
 
 	return 0;
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 5ba430cc9a87..94a91731c8e8 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -53,6 +53,7 @@ struct ad_sigma_delta_info {
  * struct ad_sigma_delta - Sigma Delta device struct
  * @spi: The spi device associated with the Sigma Delta device.
  * @trig: The IIO trigger associated with the Sigma Delta device.
+ * @irq_flags: flags for the interrupt used by the triggered buffer
  *
  * Most of the fields are private to the sigma delta library code and should not
  * be accessed by individual drivers.
@@ -60,6 +61,7 @@ struct ad_sigma_delta_info {
 struct ad_sigma_delta {
 	struct spi_device	*spi;
 	struct iio_trigger	*trig;
+	unsigned long		irq_flags;
 
 /* private: */
 	struct completion	completion;
-- 
2.20.1

Re: [PATCH] iio: ad_sigma_delta: Add custom irq flags

[Tachici, Alexandru]

On Mon, 2020-01-06 at 12:57 +0200, Alexandru Tachici wrote:
> The data-sheet of AD7124, from the Sigma-Delta ADC family,
> recommends that the falling edge of the DOUT line should be used for
> an interrupt.
> 
> The ad_sigma_delta implementation hardcodes the irq trigger type
> to low, assuming that all Sigma-Delta ADCs have the same interrupt-
> type.
> This causes unwanted behaviour. If DOUT line is already low, the
> interrupt will fire once, when enabled and the irq handler will send
> a
> read request to the device. At this time the device has not yet
> finished
> the previous conversion and will give a bad reading.
> 
> This patch allows drivers using the ad_sigma_delta layer to set the
> irq trigger type to the one specified in the corresponding data-
> sheet.
> 

Please disregard this will send a propper V2.

> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
> ---
>  drivers/iio/adc/ad_sigma_delta.c       | 9 ++++++++-
>  include/linux/iio/adc/ad_sigma_delta.h | 2 ++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad_sigma_delta.c
> b/drivers/iio/adc/ad_sigma_delta.c
> index d10bd0c97233..0007df8d50fb 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -454,7 +454,7 @@ static int ad_sd_probe_trigger(struct iio_dev
> *indio_dev)
>  
>  	ret = request_irq(sigma_delta->spi->irq,
>  			  ad_sd_data_rdy_trig_poll,
> -			  IRQF_TRIGGER_LOW,
> +			  sigma_delta->irq_flags,
>  			  indio_dev->name,
>  			  sigma_delta);
>  	if (ret)
> @@ -540,8 +540,15 @@
> EXPORT_SYMBOL_GPL(ad_sd_cleanup_buffer_and_trigger);
>  int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev
> *indio_dev,
>  	struct spi_device *spi, const struct ad_sigma_delta_info *info)
>  {
> +	unsigned long set_trigger_flags;
> +
>  	sigma_delta->spi = spi;
>  	sigma_delta->info = info;
> +
> +	set_trigger_flags = sigma_delta->irq_flags & IRQF_TRIGGER_MASK;
> +	if (set_trigger_flags == IRQF_TRIGGER_NONE)
> +		sigma_delta->irq_flags |= IRQF_TRIGGER_LOW;
> +
>  	iio_device_set_drvdata(indio_dev, sigma_delta);
>  
>  	return 0;
> diff --git a/include/linux/iio/adc/ad_sigma_delta.h
> b/include/linux/iio/adc/ad_sigma_delta.h
> index 5ba430cc9a87..94a91731c8e8 100644
> --- a/include/linux/iio/adc/ad_sigma_delta.h
> +++ b/include/linux/iio/adc/ad_sigma_delta.h
> @@ -53,6 +53,7 @@ struct ad_sigma_delta_info {
>   * struct ad_sigma_delta - Sigma Delta device struct
>   * @spi: The spi device associated with the Sigma Delta device.
>   * @trig: The IIO trigger associated with the Sigma Delta device.
> + * @irq_flags: flags for the interrupt used by the triggered buffer
>   *
>   * Most of the fields are private to the sigma delta library code
> and should not
>   * be accessed by individual drivers.
> @@ -60,6 +61,7 @@ struct ad_sigma_delta_info {
>  struct ad_sigma_delta {
>  	struct spi_device	*spi;
>  	struct iio_trigger	*trig;
> +	unsigned long		irq_flags;
>  
>  /* private: */
>  	struct completion	completion;

[PATCH V2] iio: ad_sigma_delta: Add custom irq flags

[Alexandru Tachici]

The data-sheet of AD7124, from the Sigma-Delta ADC family,
recommends that the falling edge of the DOUT line should be used for
an interrupt.

The ad_sigma_delta implementation hardcodes the irq trigger type
to low, assuming that all Sigma-Delta ADCs have the same interrupt-type.
This causes unwanted behaviour. If DOUT line is already low, the
interrupt will fire once, when enabled and the irq handler will send a
read request to the device. At this time the device has not yet finished
the previous conversion and will give a bad reading.

This patch allows drivers using the ad_sigma_delta layer to set the
irq trigger type to the one specified in the corresponding data-sheet.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
Changelog V1-V2:
 - added a V2 tag

 drivers/iio/adc/ad_sigma_delta.c       | 9 ++++++++-
 include/linux/iio/adc/ad_sigma_delta.h | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index d10bd0c97233..0007df8d50fb 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -454,7 +454,7 @@ static int ad_sd_probe_trigger(struct iio_dev *indio_dev)
 
 	ret = request_irq(sigma_delta->spi->irq,
 			  ad_sd_data_rdy_trig_poll,
-			  IRQF_TRIGGER_LOW,
+			  sigma_delta->irq_flags,
 			  indio_dev->name,
 			  sigma_delta);
 	if (ret)
@@ -540,8 +540,15 @@ EXPORT_SYMBOL_GPL(ad_sd_cleanup_buffer_and_trigger);
 int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
 	struct spi_device *spi, const struct ad_sigma_delta_info *info)
 {
+	unsigned long set_trigger_flags;
+
 	sigma_delta->spi = spi;
 	sigma_delta->info = info;
+
+	set_trigger_flags = sigma_delta->irq_flags & IRQF_TRIGGER_MASK;
+	if (set_trigger_flags == IRQF_TRIGGER_NONE)
+		sigma_delta->irq_flags |= IRQF_TRIGGER_LOW;
+
 	iio_device_set_drvdata(indio_dev, sigma_delta);
 
 	return 0;
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 5ba430cc9a87..94a91731c8e8 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -53,6 +53,7 @@ struct ad_sigma_delta_info {
  * struct ad_sigma_delta - Sigma Delta device struct
  * @spi: The spi device associated with the Sigma Delta device.
  * @trig: The IIO trigger associated with the Sigma Delta device.
+ * @irq_flags: flags for the interrupt used by the triggered buffer
  *
  * Most of the fields are private to the sigma delta library code and should not
  * be accessed by individual drivers.
@@ -60,6 +61,7 @@ struct ad_sigma_delta_info {
 struct ad_sigma_delta {
 	struct spi_device	*spi;
 	struct iio_trigger	*trig;
+	unsigned long		irq_flags;
 
 /* private: */
 	struct completion	completion;
-- 
2.20.1

Re: [PATCH V2] iio: ad_sigma_delta: Add custom irq flags

[Jonathan Cameron]

On Tue, 7 Jan 2020 13:06:36 +0200
Alexandru Tachici <alexandru.tachici@analog.com> wrote:

> The data-sheet of AD7124, from the Sigma-Delta ADC family,
> recommends that the falling edge of the DOUT line should be used for
> an interrupt.
> 
> The ad_sigma_delta implementation hardcodes the irq trigger type
> to low, assuming that all Sigma-Delta ADCs have the same interrupt-type.
> This causes unwanted behaviour. If DOUT line is already low, the
> interrupt will fire once, when enabled and the irq handler will send a
> read request to the device. At this time the device has not yet finished
> the previous conversion and will give a bad reading.
> 
> This patch allows drivers using the ad_sigma_delta layer to set the
> irq trigger type to the one specified in the corresponding data-sheet.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>

Hi Alexandru.

Patch is fine, but I'd expect to see it in a series with the change
for the ad7124 driver.  I'm never keen to merge new features without
a user.  So send a v3, with the change to that driver (which I'm guessing
is pretty trivial!)

Thanks,

Jonathan

> ---
> Changelog V1-V2:
>  - added a V2 tag
> 
>  drivers/iio/adc/ad_sigma_delta.c       | 9 ++++++++-
>  include/linux/iio/adc/ad_sigma_delta.h | 2 ++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
> index d10bd0c97233..0007df8d50fb 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -454,7 +454,7 @@ static int ad_sd_probe_trigger(struct iio_dev *indio_dev)
>  
>  	ret = request_irq(sigma_delta->spi->irq,
>  			  ad_sd_data_rdy_trig_poll,
> -			  IRQF_TRIGGER_LOW,
> +			  sigma_delta->irq_flags,
>  			  indio_dev->name,
>  			  sigma_delta);
>  	if (ret)
> @@ -540,8 +540,15 @@ EXPORT_SYMBOL_GPL(ad_sd_cleanup_buffer_and_trigger);
>  int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
>  	struct spi_device *spi, const struct ad_sigma_delta_info *info)
>  {
> +	unsigned long set_trigger_flags;
> +
>  	sigma_delta->spi = spi;
>  	sigma_delta->info = info;
> +
> +	set_trigger_flags = sigma_delta->irq_flags & IRQF_TRIGGER_MASK;
> +	if (set_trigger_flags == IRQF_TRIGGER_NONE)
> +		sigma_delta->irq_flags |= IRQF_TRIGGER_LOW;
> +
>  	iio_device_set_drvdata(indio_dev, sigma_delta);
>  
>  	return 0;
> diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
> index 5ba430cc9a87..94a91731c8e8 100644
> --- a/include/linux/iio/adc/ad_sigma_delta.h
> +++ b/include/linux/iio/adc/ad_sigma_delta.h
> @@ -53,6 +53,7 @@ struct ad_sigma_delta_info {
>   * struct ad_sigma_delta - Sigma Delta device struct
>   * @spi: The spi device associated with the Sigma Delta device.
>   * @trig: The IIO trigger associated with the Sigma Delta device.
> + * @irq_flags: flags for the interrupt used by the triggered buffer
>   *
>   * Most of the fields are private to the sigma delta library code and should not
>   * be accessed by individual drivers.
> @@ -60,6 +61,7 @@ struct ad_sigma_delta_info {
>  struct ad_sigma_delta {
>  	struct spi_device	*spi;
>  	struct iio_trigger	*trig;
> +	unsigned long		irq_flags;
>  
>  /* private: */
>  	struct completion	completion;

[PATCH 0/2 V3] iio: adc: ad7124 fix wrong irq flag

[Alexandru Tachici]

Before these patches the ad7124 driver used a wrong irq flag
because it is using the ad-sigma-delta layer which hardcoded
the used irq flag. This caused an early read of the
data register when ad7124 was set on continous mode giving
a bad conversion.

This V3 series fixes the above explained unwanted behaviour.

1. Allow drivers to set their own irq flag for the iio
triggered buffer.

2. Set the right irq flag in the ad_sigma_delta_info
in ad7124 driver.

Alexandru Tachici (2):
  iio: adc: ad-sigma-delta: Allow custom IRQ flags
  iio: adc: ad7124: Set IRQ type to falling

 drivers/iio/adc/ad7124.c               | 2 ++
 drivers/iio/adc/ad7780.c               | 1 +
 drivers/iio/adc/ad7791.c               | 1 +
 drivers/iio/adc/ad7793.c               | 1 +
 drivers/iio/adc/ad_sigma_delta.c       | 2 +-
 include/linux/iio/adc/ad_sigma_delta.h | 2 ++
 6 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.20.1

[PATCH 1/2 V3] iio: adc: ad-sigma-delta: Allow custom IRQ flags

[Alexandru Tachici]

Before this patch the ad_sigma_delta implementation hardcoded
the irq trigger type to low, assuming that all Sigma-Delta ADCs
have the same interrupt-type.

This patch allows all drivers using the ad_sigma_delta layer to set the
irq trigger type to the one specified in the datasheet.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/iio/adc/ad7124.c               | 2 ++
 drivers/iio/adc/ad7780.c               | 1 +
 drivers/iio/adc/ad7791.c               | 1 +
 drivers/iio/adc/ad7793.c               | 1 +
 drivers/iio/adc/ad_sigma_delta.c       | 2 +-
 include/linux/iio/adc/ad_sigma_delta.h | 2 ++
 6 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index edc6f1cc90b2..9531d8a6cb27 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -9,6 +9,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/regulator/consumer.h>
@@ -222,6 +223,7 @@ static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
 	.addr_shift = 0,
 	.read_mask = BIT(6),
 	.data_reg = AD7124_DATA,
+	.irq_flags = IRQF_TRIGGER_LOW,
 };
 
 static int ad7124_set_channel_odr(struct ad7124_state *st,
diff --git a/drivers/iio/adc/ad7780.c b/drivers/iio/adc/ad7780.c
index 217a5a5c3c6d..291c1a898129 100644
--- a/drivers/iio/adc/ad7780.c
+++ b/drivers/iio/adc/ad7780.c
@@ -203,6 +203,7 @@ static const struct ad_sigma_delta_info ad7780_sigma_delta_info = {
 	.set_mode = ad7780_set_mode,
 	.postprocess_sample = ad7780_postprocess_sample,
 	.has_registers = false,
+	.irq_flags = IRQF_TRIGGER_LOW,
 };
 
 #define AD7780_CHANNEL(bits, wordsize) \
diff --git a/drivers/iio/adc/ad7791.c b/drivers/iio/adc/ad7791.c
index 54025ea10239..abb239392631 100644
--- a/drivers/iio/adc/ad7791.c
+++ b/drivers/iio/adc/ad7791.c
@@ -205,6 +205,7 @@ static const struct ad_sigma_delta_info ad7791_sigma_delta_info = {
 	.has_registers = true,
 	.addr_shift = 4,
 	.read_mask = BIT(3),
+	.irq_flags = IRQF_TRIGGER_LOW,
 };
 
 static int ad7791_read_raw(struct iio_dev *indio_dev,
diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c
index bbc41ecf0d2f..b747db97f78a 100644
--- a/drivers/iio/adc/ad7793.c
+++ b/drivers/iio/adc/ad7793.c
@@ -206,6 +206,7 @@ static const struct ad_sigma_delta_info ad7793_sigma_delta_info = {
 	.has_registers = true,
 	.addr_shift = 3,
 	.read_mask = BIT(6),
+	.irq_flags = IRQF_TRIGGER_LOW,
 };
 
 static const struct ad_sd_calib_data ad7793_calib_arr[6] = {
diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 8ba90486c787..8115b6de1d6c 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -500,7 +500,7 @@ static int ad_sd_probe_trigger(struct iio_dev *indio_dev)
 
 	ret = request_irq(sigma_delta->spi->irq,
 			  ad_sd_data_rdy_trig_poll,
-			  IRQF_TRIGGER_LOW,
+			  sigma_delta->info->irq_flags,
 			  indio_dev->name,
 			  sigma_delta);
 	if (ret)
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 8a4e25a7080c..5a127c0ed200 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -40,6 +40,7 @@ struct iio_dev;
  * @read_mask: Mask for the communications register having the read bit set.
  * @data_reg: Address of the data register, if 0 the default address of 0x3 will
  *   be used.
+ * @irq_flags: flags for the interrupt used by the triggered buffer
  */
 struct ad_sigma_delta_info {
 	int (*set_channel)(struct ad_sigma_delta *, unsigned int channel);
@@ -49,6 +50,7 @@ struct ad_sigma_delta_info {
 	unsigned int addr_shift;
 	unsigned int read_mask;
 	unsigned int data_reg;
+	unsigned long irq_flags;
 };
 
 /**
-- 
2.20.1

Re: [PATCH 1/2 V3] iio: adc: ad-sigma-delta: Allow custom IRQ flags

[Jonathan Cameron]

On Mon, 13 Jan 2020 12:26:52 +0200
Alexandru Tachici <alexandru.tachici@analog.com> wrote:

> Before this patch the ad_sigma_delta implementation hardcoded
> the irq trigger type to low, assuming that all Sigma-Delta ADCs
> have the same interrupt-type.
> 
> This patch allows all drivers using the ad_sigma_delta layer to set the
> irq trigger type to the one specified in the datasheet.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>

Now, as neither of these has a fixes tag, I've taken them in the
togreg branch of iio.git.  If you want them backported to stable,
one they are in Linus's tree you can send a specific message to
request they are applied.

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

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ad7124.c               | 2 ++
>  drivers/iio/adc/ad7780.c               | 1 +
>  drivers/iio/adc/ad7791.c               | 1 +
>  drivers/iio/adc/ad7793.c               | 1 +
>  drivers/iio/adc/ad_sigma_delta.c       | 2 +-
>  include/linux/iio/adc/ad_sigma_delta.h | 2 ++
>  6 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index edc6f1cc90b2..9531d8a6cb27 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -9,6 +9,7 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> +#include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/regulator/consumer.h>
> @@ -222,6 +223,7 @@ static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
>  	.addr_shift = 0,
>  	.read_mask = BIT(6),
>  	.data_reg = AD7124_DATA,
> +	.irq_flags = IRQF_TRIGGER_LOW,
>  };
>  
>  static int ad7124_set_channel_odr(struct ad7124_state *st,
> diff --git a/drivers/iio/adc/ad7780.c b/drivers/iio/adc/ad7780.c
> index 217a5a5c3c6d..291c1a898129 100644
> --- a/drivers/iio/adc/ad7780.c
> +++ b/drivers/iio/adc/ad7780.c
> @@ -203,6 +203,7 @@ static const struct ad_sigma_delta_info ad7780_sigma_delta_info = {
>  	.set_mode = ad7780_set_mode,
>  	.postprocess_sample = ad7780_postprocess_sample,
>  	.has_registers = false,
> +	.irq_flags = IRQF_TRIGGER_LOW,
>  };
>  
>  #define AD7780_CHANNEL(bits, wordsize) \
> diff --git a/drivers/iio/adc/ad7791.c b/drivers/iio/adc/ad7791.c
> index 54025ea10239..abb239392631 100644
> --- a/drivers/iio/adc/ad7791.c
> +++ b/drivers/iio/adc/ad7791.c
> @@ -205,6 +205,7 @@ static const struct ad_sigma_delta_info ad7791_sigma_delta_info = {
>  	.has_registers = true,
>  	.addr_shift = 4,
>  	.read_mask = BIT(3),
> +	.irq_flags = IRQF_TRIGGER_LOW,
>  };
>  
>  static int ad7791_read_raw(struct iio_dev *indio_dev,
> diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c
> index bbc41ecf0d2f..b747db97f78a 100644
> --- a/drivers/iio/adc/ad7793.c
> +++ b/drivers/iio/adc/ad7793.c
> @@ -206,6 +206,7 @@ static const struct ad_sigma_delta_info ad7793_sigma_delta_info = {
>  	.has_registers = true,
>  	.addr_shift = 3,
>  	.read_mask = BIT(6),
> +	.irq_flags = IRQF_TRIGGER_LOW,
>  };
>  
>  static const struct ad_sd_calib_data ad7793_calib_arr[6] = {
> diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
> index 8ba90486c787..8115b6de1d6c 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -500,7 +500,7 @@ static int ad_sd_probe_trigger(struct iio_dev *indio_dev)
>  
>  	ret = request_irq(sigma_delta->spi->irq,
>  			  ad_sd_data_rdy_trig_poll,
> -			  IRQF_TRIGGER_LOW,
> +			  sigma_delta->info->irq_flags,
>  			  indio_dev->name,
>  			  sigma_delta);
>  	if (ret)
> diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
> index 8a4e25a7080c..5a127c0ed200 100644
> --- a/include/linux/iio/adc/ad_sigma_delta.h
> +++ b/include/linux/iio/adc/ad_sigma_delta.h
> @@ -40,6 +40,7 @@ struct iio_dev;
>   * @read_mask: Mask for the communications register having the read bit set.
>   * @data_reg: Address of the data register, if 0 the default address of 0x3 will
>   *   be used.
> + * @irq_flags: flags for the interrupt used by the triggered buffer
>   */
>  struct ad_sigma_delta_info {
>  	int (*set_channel)(struct ad_sigma_delta *, unsigned int channel);
> @@ -49,6 +50,7 @@ struct ad_sigma_delta_info {
>  	unsigned int addr_shift;
>  	unsigned int read_mask;
>  	unsigned int data_reg;
> +	unsigned long irq_flags;
>  };
>  
>  /**

[PATCH 2/2 V3] iio: adc: ad7124: Set IRQ type to falling

[Alexandru Tachici]

Ad7124 data-sheet specifies that the falling edge
of the DOUT line should be used for an interrupt.
The current irq flag (IRQF_TRIGGER_LOW) used will
cause unwanted behaviour. When enabling the interrupt
it will fire once because the DOUT line is already low.
This will make the driver to read an unfinished conversion
from the chip.

This patch sets the irq type to the one specified in
the data-sheet.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/iio/adc/ad7124.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index 9531d8a6cb27..9113f6d36ad4 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -223,7 +223,7 @@ static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
 	.addr_shift = 0,
 	.read_mask = BIT(6),
 	.data_reg = AD7124_DATA,
-	.irq_flags = IRQF_TRIGGER_LOW,
+	.irq_flags = IRQF_TRIGGER_FALLING,
 };
 
 static int ad7124_set_channel_odr(struct ad7124_state *st,
-- 
2.20.1

Re: [PATCH 2/2 V3] iio: adc: ad7124: Set IRQ type to falling

[Jonathan Cameron]

On Mon, 13 Jan 2020 12:26:53 +0200
Alexandru Tachici <alexandru.tachici@analog.com> wrote:

> Ad7124 data-sheet specifies that the falling edge
> of the DOUT line should be used for an interrupt.
> The current irq flag (IRQF_TRIGGER_LOW) used will
> cause unwanted behaviour. When enabling the interrupt
> it will fire once because the DOUT line is already low.
> This will make the driver to read an unfinished conversion
> from the chip.
> 
> This patch sets the irq type to the one specified in
> the data-sheet.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ad7124.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index 9531d8a6cb27..9113f6d36ad4 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -223,7 +223,7 @@ static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
>  	.addr_shift = 0,
>  	.read_mask = BIT(6),
>  	.data_reg = AD7124_DATA,
> -	.irq_flags = IRQF_TRIGGER_LOW,
> +	.irq_flags = IRQF_TRIGGER_FALLING,
>  };
>  
>  static int ad7124_set_channel_odr(struct ad7124_state *st,

[PATCH] iio: ad_sigma_delta: Add custom irq flags

[Alexandru Tachici]

The data-sheet of AD7124, from the Sigma-Delta ADC family,
recommends that the falling edge of the DOUT line should be used for
an interrupt.

The ad_sigma_delta implementation hardcodes the irq trigger type
to low, assuming that all Sigma-Delta ADCs have the same interrupt-type.
This causes unwanted behaviour. If DOUT line is already low, the
interrupt will fire once, when enabled and the irq handler will send a
read request to the device. At this time the device has not yet finished
the previous conversion and will give a bad reading.

This patch allows drivers using the ad_sigma_delta layer to set the
irq trigger type to the one specified in the corresponding data-sheet.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/iio/adc/ad_sigma_delta.c       | 11 ++++++++++-
 include/linux/iio/adc/ad_sigma_delta.h |  5 +++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 8ba90486c787..ef8c356b11ee 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -500,7 +500,7 @@ static int ad_sd_probe_trigger(struct iio_dev *indio_dev)
 
 	ret = request_irq(sigma_delta->spi->irq,
 			  ad_sd_data_rdy_trig_poll,
-			  IRQF_TRIGGER_LOW,
+			  sigma_delta->irq_flags,
 			  indio_dev->name,
 			  sigma_delta);
 	if (ret)
@@ -586,8 +586,17 @@ EXPORT_SYMBOL_GPL(ad_sd_cleanup_buffer_and_trigger);
 int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
 	struct spi_device *spi, const struct ad_sigma_delta_info *info)
 {
+	unsigned long set_trigger_flags;
+
 	sigma_delta->spi = spi;
 	sigma_delta->info = info;
+	sigma_delta->num_slots = 1;
+	sigma_delta->active_slots = 1;
+
+	set_trigger_flags = sigma_delta->irq_flags & IRQF_TRIGGER_MASK;
+	if (set_trigger_flags == IRQF_TRIGGER_NONE)
+		sigma_delta->irq_flags |= IRQF_TRIGGER_LOW;
+
 	iio_device_set_drvdata(indio_dev, sigma_delta);
 
 	return 0;
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 8a4e25a7080c..0d3fa1e16f5e 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -55,6 +55,8 @@ struct ad_sigma_delta_info {
  * struct ad_sigma_delta - Sigma Delta device struct
  * @spi: The spi device associated with the Sigma Delta device.
  * @trig: The IIO trigger associated with the Sigma Delta device.
+ * @num_slots: Number of sequencer slots
+ * @irq_flags: flags for the interrupt used by the triggered buffer
  *
  * Most of the fields are private to the sigma delta library code and should not
  * be accessed by individual drivers.
@@ -63,6 +65,9 @@ struct ad_sigma_delta {
 	struct spi_device	*spi;
 	struct iio_trigger	*trig;
 
+	unsigned int		num_slots;
+	unsigned long		irq_flags;
+
 /* private: */
 	struct completion	completion;
 	bool			irq_dis;
-- 
2.20.1

Re: [PATCH] iio: ad_sigma_delta: Add custom irq flags

[Ardelean, Alexandru]

On Mon, 2020-01-06 at 11:38 +0200, Alexandru Tachici wrote:
> [External]
> 
> The data-sheet of AD7124, from the Sigma-Delta ADC family,
> recommends that the falling edge of the DOUT line should be used for
> an interrupt.
> 
> The ad_sigma_delta implementation hardcodes the irq trigger type
> to low, assuming that all Sigma-Delta ADCs have the same interrupt-type.
> This causes unwanted behaviour. If DOUT line is already low, the
> interrupt will fire once, when enabled and the irq handler will send a
> read request to the device. At this time the device has not yet finished
> the previous conversion and will give a bad reading.
> 
> This patch allows drivers using the ad_sigma_delta layer to set the
> irq trigger type to the one specified in the corresponding data-sheet.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
> ---
>  drivers/iio/adc/ad_sigma_delta.c       | 11 ++++++++++-
>  include/linux/iio/adc/ad_sigma_delta.h |  5 +++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad_sigma_delta.c
> b/drivers/iio/adc/ad_sigma_delta.c
> index 8ba90486c787..ef8c356b11ee 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -500,7 +500,7 @@ static int ad_sd_probe_trigger(struct iio_dev
> *indio_dev)
>  
>  	ret = request_irq(sigma_delta->spi->irq,
>  			  ad_sd_data_rdy_trig_poll,
> -			  IRQF_TRIGGER_LOW,
> +			  sigma_delta->irq_flags,
>  			  indio_dev->name,
>  			  sigma_delta);
>  	if (ret)
> @@ -586,8 +586,17 @@ EXPORT_SYMBOL_GPL(ad_sd_cleanup_buffer_and_trigger);
>  int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev
> *indio_dev,
>  	struct spi_device *spi, const struct ad_sigma_delta_info *info)
>  {
> +	unsigned long set_trigger_flags;
> +
>  	sigma_delta->spi = spi;
>  	sigma_delta->info = info;
> +	sigma_delta->num_slots = 1;
> +	sigma_delta->active_slots = 1;

The slots patch seems to have slipped some cherry-picks and conflicts.

> +
> +	set_trigger_flags = sigma_delta->irq_flags & IRQF_TRIGGER_MASK;
> +	if (set_trigger_flags == IRQF_TRIGGER_NONE)
> +		sigma_delta->irq_flags |= IRQF_TRIGGER_LOW;
> +
>  	iio_device_set_drvdata(indio_dev, sigma_delta);
>  
>  	return 0;
> diff --git a/include/linux/iio/adc/ad_sigma_delta.h
> b/include/linux/iio/adc/ad_sigma_delta.h
> index 8a4e25a7080c..0d3fa1e16f5e 100644
> --- a/include/linux/iio/adc/ad_sigma_delta.h
> +++ b/include/linux/iio/adc/ad_sigma_delta.h
> @@ -55,6 +55,8 @@ struct ad_sigma_delta_info {
>   * struct ad_sigma_delta - Sigma Delta device struct
>   * @spi: The spi device associated with the Sigma Delta device.
>   * @trig: The IIO trigger associated with the Sigma Delta device.
> + * @num_slots: Number of sequencer slots
> + * @irq_flags: flags for the interrupt used by the triggered buffer
>   *
>   * Most of the fields are private to the sigma delta library code and
> should not
>   * be accessed by individual drivers.
> @@ -63,6 +65,9 @@ struct ad_sigma_delta {
>  	struct spi_device	*spi;
>  	struct iio_trigger	*trig;
>  
> +	unsigned int		num_slots;
> +	unsigned long		irq_flags;
> +
>  /* private: */
>  	struct completion	completion;
>  	bool			irq_dis;

Re: [PATCH] iio: ad_sigma_delta: Add custom irq flags

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2603 bytes --]

Hi Alexandru,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on iio/togreg]
[also build test ERROR on linux/master linus/master v5.5-rc5 next-20200106]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Alexandru-Tachici/iio-ad_sigma_delta-Add-custom-irq-flags/20200106-185454
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: i386-randconfig-h001-20200106 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/iio/adc/ad_sigma_delta.c: In function 'ad_sd_init':
>> drivers/iio/adc/ad_sigma_delta.c:594:15: error: 'struct ad_sigma_delta' has no member named 'active_slots'; did you mean 'num_slots'?
     sigma_delta->active_slots = 1;
                  ^~~~~~~~~~~~
                  num_slots

vim +594 drivers/iio/adc/ad_sigma_delta.c

   575	
   576	/**
   577	 * ad_sd_init() - Initializes a ad_sigma_delta struct
   578	 * @sigma_delta: The ad_sigma_delta device
   579	 * @indio_dev: The IIO device which the Sigma Delta device is used for
   580	 * @spi: The SPI device for the ad_sigma_delta device
   581	 * @info: Device specific callbacks and options
   582	 *
   583	 * This function needs to be called before any other operations are performed on
   584	 * the ad_sigma_delta struct.
   585	 */
   586	int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
   587		struct spi_device *spi, const struct ad_sigma_delta_info *info)
   588	{
   589		unsigned long set_trigger_flags;
   590	
   591		sigma_delta->spi = spi;
   592		sigma_delta->info = info;
   593		sigma_delta->num_slots = 1;
 > 594		sigma_delta->active_slots = 1;
   595	
   596		set_trigger_flags = sigma_delta->irq_flags & IRQF_TRIGGER_MASK;
   597		if (set_trigger_flags == IRQF_TRIGGER_NONE)
   598			sigma_delta->irq_flags |= IRQF_TRIGGER_LOW;
   599	
   600		iio_device_set_drvdata(indio_dev, sigma_delta);
   601	
   602		return 0;
   603	}
   604	EXPORT_SYMBOL_GPL(ad_sd_init);
   605	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32106 bytes --]

Re: [PATCH] iio: ad_sigma_delta: Add custom irq flags

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2671 bytes --]

Hi Alexandru,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on iio/togreg]
[also build test ERROR on linux/master linus/master v5.5-rc5 next-20200106]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Alexandru-Tachici/iio-ad_sigma_delta-Add-custom-irq-flags/20200106-185454
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: i386-randconfig-h001-20200106 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/iio/adc/ad_sigma_delta.c: In function 'ad_sd_init':
>> drivers/iio/adc/ad_sigma_delta.c:594:15: error: 'struct ad_sigma_delta' has no member named 'active_slots'; did you mean 'num_slots'?
     sigma_delta->active_slots = 1;
                  ^~~~~~~~~~~~
                  num_slots

vim +594 drivers/iio/adc/ad_sigma_delta.c

   575	
   576	/**
   577	 * ad_sd_init() - Initializes a ad_sigma_delta struct
   578	 * @sigma_delta: The ad_sigma_delta device
   579	 * @indio_dev: The IIO device which the Sigma Delta device is used for
   580	 * @spi: The SPI device for the ad_sigma_delta device
   581	 * @info: Device specific callbacks and options
   582	 *
   583	 * This function needs to be called before any other operations are performed on
   584	 * the ad_sigma_delta struct.
   585	 */
   586	int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
   587		struct spi_device *spi, const struct ad_sigma_delta_info *info)
   588	{
   589		unsigned long set_trigger_flags;
   590	
   591		sigma_delta->spi = spi;
   592		sigma_delta->info = info;
   593		sigma_delta->num_slots = 1;
 > 594		sigma_delta->active_slots = 1;
   595	
   596		set_trigger_flags = sigma_delta->irq_flags & IRQF_TRIGGER_MASK;
   597		if (set_trigger_flags == IRQF_TRIGGER_NONE)
   598			sigma_delta->irq_flags |= IRQF_TRIGGER_LOW;
   599	
   600		iio_device_set_drvdata(indio_dev, sigma_delta);
   601	
   602		return 0;
   603	}
   604	EXPORT_SYMBOL_GPL(ad_sd_init);
   605	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32106 bytes --]