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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250112114001.6295d2d4@jic23-huawei>
Date: Sun, 12 Jan 2025 11:40:01 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Julien Stephan <jstephan@...libre.com>
Cc: David Lechner <dlechner@...libre.com>, Lars-Peter Clausen
 <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, Nuno
 Sá <nuno.sa@...log.com>, Jonathan Corbet <corbet@....net>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH RFC v2 3/4] iio: adc: ad7380: add alert support

On Sat, 11 Jan 2025 12:51:45 +0000
Jonathan Cameron <jic23@...nel.org> wrote:

> On Tue, 7 Jan 2025 09:51:23 +0100
> Julien Stephan <jstephan@...libre.com> wrote:
> 
> > Le lun. 6 janv. 2025 à 16:29, David Lechner <dlechner@...libre.com> a écrit :  
> > >
> > > On 12/28/24 8:24 AM, Jonathan Cameron wrote:    
> > > > On Tue, 24 Dec 2024 10:34:32 +0100
> > > > Julien Stephan <jstephan@...libre.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?  
> 
> Whilst I'd hope it would I'm not 100% sure.

How much do we care about a potential double report?  Maybe we just let it happen?
Does it create an infinite chain?

Jonathan

> 
> > >
> > > 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?
> > >    
> > 
> > Hi Jonathan, and David,
> > 
> > I think this is getting very complicated for something not that useful
> > in practice.
> > If needed we can go back on this later to find an appropriate solution.
> > I sent a non RFC V3 version, removing the interrupt handling? Does
> > that work for you?  
> 
> Works for me.
> 
> > 
> > Cheers
> > Julein
> >   
> 
>