Re: [PATCH RFC v2 3/4] iio: adc: ad7380: add alert support

[David Lechner <dlechner@baylibre.com>]

On 12/28/24 8:24 AM, Jonathan Cameron wrote:
> On Tue, 24 Dec 2024 10:34:32 +0100
> Julien Stephan <jstephan@baylibre.com> wrote:
> 
>> The alert functionality is an out of range indicator and can be used as an
>> early indicator of an out of bounds conversion result.
>>
>> ALERT_LOW_THRESHOLD and ALERT_HIGH_THRESHOLD registers are common to all
>> channels.
>>
>> When using 1 SDO line (only mode supported by the driver right now), i.e
>> data outputs only on SDOA, SDOB (or SDOD for 4 channels variants) is
>> used as an alert pin. The alert pin is updated at the end of the
>> conversion (set to low if an alert occurs) and is cleared on a falling
>> edge of CS.
>>
>> The ALERT register contains information about the exact alert status:
>> channel and direction. Unfortunately we can't read this register because
>> in buffered read we cannot claim for direct mode.
>>
>> User can set high/low thresholds and enable event detection using the
>> regular iio events:
>>
>>   events/in_thresh_falling_value
>>   events/in_thresh_rising_value
>>   events/thresh_either_en
>>
>> If the interrupt properties is present in the device tree, an IIO event
>> will be generated for each interrupt received.
>> Because we cannot read ALERT register, we can't determine the exact
>> channel that triggers the alert, neither the direction (hight/low
>> threshold violation), so we send and IIO_EV_DIR_EITHER event for all
>> channels.
>>
>> In buffered reads, if input stays out of thresholds limit, an interrupt
>> will be generated for each sample read, because the alert pin is cleared
>> on a falling edge of CS (i.e when starting a new conversion). To avoid
>> generating to much interrupt, we introduce a reset_timeout that can be
>> used to disable interrupt for a given time (in ms)
>>
>>   events/thresh_either_reset_timeout
>>
>> When an interrupt is received, interrupts are disabled and re-enabled
>> after thresh_either_reset_timeout ms. If the reset timeout is set to 0,
>> interrupt are re-enabled directly.
>> Note: interrupts are always disabled at least during the handling of the
>> previous interrupt, because each read triggers 2 transactions, that can
>> lead to 2 interrupts for a single user read. IRQF_ONESHOT is not enough,
>> because, it postpones the 2nd irq after the handling of the first one,
>> which can still trigger 2 interrupts for a single user read.
> 
> After some of our recent discussions around interrupt handling and
> the guarantees (that aren't) made, even disabling the interrupt doesn't
> prevent some irq chips queuing up future interrupts.
> 
> https://lore.kernel.org/all/io53lznz3qp3jd5rohqsjhosnmdzd6d44sdbwu5jcfrs3rz2a2@orquwgflrtyc/
> 
> I'm not sure this alert can actually work as a result :(
> I am struggling to come up with a scheme that will work.
> 
Would it work if we change it to a level-triggered interrupt instead of edge
triggered?

Since the main purpose of this is to trigger a hardware shutdown, perhaps we
could just omit the interrupt/emitting the event and keep the threshold and
enable attributes if we can't come up with a reasonable way to handle the
interrupts?