On Tue, 7 Jan 2020 13:06:36 +0200
Alexandru Tachici <alexandru.tachici@xxxxxxxxxx> 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@xxxxxxxxxx>
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;
