Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
From:   Fabrizio Lamarque <fl.scratchpad@gmail.com>
Date:   Wed, 5 Apr 2023 11:53:10 +0200
Message-ID: <CAPJMGm4GaSjD6bdqMwCr2EVZGenWzT-nCCf3BMRaD1TSfAabpA@mail.gmail.com>
Subject: iio: ad7192: Pending IRQ on SDO/RDY causing retrigger of interrupt
 handler and missing every sample
To:     alexandru.tachici@analog.com, linux-kernel@vger.kernel.org
Cc:     Jonathan Cameron <jic23@kernel.org>,
        =?UTF-8?B?TnVubyBTw6E=?= <noname.nuno@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Link: https://lore.kernel.org/all/20210906065630.16325-3-alexandru.tachici@analog.com/

This commit broke the driver functionality, i.e. cat in_voltage1_raw
triggers a correct sampling sequence only the first time, then it
always returns the first sampled value.

Following the sequence of ad_sigma_delta_single_conversion() within
ad_sigma_delta.c, this is due to:

- IRQ resend mechanism is always enabled for ARM cores
(CONFIG_HARDIRQS_SW_RESEND)
- Edge IRQs are always made pending when a corresponding event
happens, even after disable_irq(). This is intentional and designed to
prevent missing signal edges.
- Level IRQs are not impacted by IRQ resend (i.e. IRQ_PENDING is
always cleared).
- SPI communication causes the IRQ to be set pending (even if
corresponding interrupt is disabled)
- The second time ad_sigma_delta_single_conversion() is called,
enable_irq() will trigger the interrupt immediately, even if RDY line
is high.
- In turn, this causes the call
wait_for_completion_interruptible_timeout() to exit immediately, with
RDY line still high.
- Right after the SPI register read, the MODE register is written with
AD_SD_MODE_IDLE, and pending conversion is stopped. Hence DATA
register is never updated.

I would suggest to revert this commit or set the flag with
IRQF_TRIGGER_LOW instead of IRQF_TRIGGER_FALLING, but I am not sure
about the problem solved by this commit and how to replicate it.
Another option would be to keep IRQ flags within bindings only.

As a side note, AD7192 datasheet says that the falling edge on SDO
line _can_ be used as an interrupt to processor, but it also states
that the _level_ on SDO/RDY line indicates the sample is ready. What
happens on SDO/RDY interrupt line before the ADC conversion is
triggered should be ignored.

This bug should be easy to reproduce on ADI demo boards with impacted
kernel versions, just by manually reading any input channel from
sysfs.

Fabrizio Lamarque