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@xxxxxxxxx> 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
