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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 12 Feb 2019 18:17:53 -0800
From:   Robert Eshleman <bobbyeshleman@...il.com>
To:     Sven Van Asbroeck <thesven73@...il.com>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-iio@...r.kernel.org
Subject: Re: [PATCH 1/3] iio: light: Add driver for ap3216c

On Mon, Feb 11, 2019 at 02:29:58PM -0500, Sven Van Asbroeck wrote:
> On Sun, Feb 10, 2019 at 3:39 PM Robert Eshleman <bobbyeshleman@...il.com> wrote:
> >
> > This patch adds support for the ap3216c ambient light and proximity
> > sensor.
> 
> PS
> 
> Why not use the chip in the mode where the interrupt is automatically cleared by
> reading the data? This could work if you read the data in the
> interrupt routine, store
> it in a buffer, then send the event to iio. Then when userspace wants
> to read out the
> value, don't actually touch the hardware, but return the buffered value.
> 
> I don't think you then need any synchronization primitives to
> accomplish this, such as
> mutexes, spin locks, etc. Because the interrupt routine is single-threaded.
> 
> You don't even need a memory barrier for the buffered value,
> because the iio core uses a waitqueue internally, which automatically issues
> an mb(). As far as I know.

Hey Sven,

First, thank you for the feedback.

I had initially went with a similar design, but there is
the case in which the interrupt fires and then before the status
register is read by the handler a user process reads the data and
clears the interrupt.  When the handler continues execution it will
read a zero status and return IRQ_NONE.  My understanding of how
Linux handles IRQ_NONE is pretty poor, but I felt that this behavior
is incorrect even if inconsequential.  This could be avoided by
doing a status register read with every data read, and buffering
that as well, but then we lose the benefit altogether by increasing
I2C reads.

In the approach you describe here, it seems like that would
work if this driver wasn't supporting shared interrupts.  In the
case that a user-space read happens to clear the interrupt before
the handler reads the status register, I think we would end up
falsely returning IRQ_NONE.

Is my understanding of this correct?  It's very possible I'm
misunderstanding IRQ_NONE and shared interrupts.

Again, thank you for the feedback.

-Bobby

Powered by blists - more mailing lists