diff mbox series

iio: ad_sigma_delta: Add custom irq flags

Message ID 20200106093852.29702-1-alexandru.tachici@analog.com (mailing list archive)
State New, archived
Headers show
Series iio: ad_sigma_delta: Add custom irq flags | expand

Commit Message

Alexandru Tachici Jan. 6, 2020, 9:38 a.m. UTC
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(-)

Comments

Alexandru Ardelean Jan. 6, 2020, 9:41 a.m. UTC | #1
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;
kernel test robot Jan. 6, 2020, 1:54 p.m. UTC | #2
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
diff mbox series

Patch

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;