Re: [PATCH v2 2/2] hwmon: add generic GPIO brownout support

[Trent Piepho <tpiepho@impinj.com>]

On Thu, 2018-11-01 at 08:14 -0700, Guenter Roeck wrote:
> On Thu, Nov 01, 2018 at 03:53:12PM +0100, Marco Felsch wrote:
> > 
> > > 
> > > Isn't that configurable with devicetree flags ? I don't think a driver
> > > should get involved in deciding the active edge.
> > 
> > No, AFAIK we can only specify the active level types for gpios. This
> > made sense to me, because I saw no gpio-controller which support
> > 'edge-level' reporting (however it will be called) currently.

Interrupts types are specific to each interrupt controller, but there
is a standard set of flags that, AFAIK, every Linux controller uses. 
These include IRQ_TYPE_EDGE_BOTH, IRQ_TYPE_EDGE_RISING,
IRQ_TYPE_LEVEL_HIGH, and so on.

So you can support hardware that is inherently edge or level triggered.
 

I have used edge triggered interrupts on GPIO pins on many designs.

From what I remember, most hwmon chips use level triggered signals. 
The general process in the driver:

Level goes to asserted, IRQ handler invoked
Ack interrupt in hwmon chip's alarm register, which usually de-asserts
the alarm line
Set bit in driver state to indicate the alarm attribute should be set
sysfs_notify anything polling the attribute
If alarm line did not de-assert on ack, leave IRQ masked
Sysfs attribute stays set until userspace process acks it (by reading)

The important part here is that the alarm is latched in the driver.  We
don't just report the current alarm status in the attribute.  Otherwise
an alarm could come and go without anyone noticing if they didn't read
the attribute at just the right time.

Put another way, the hwmon alarm attribute means an alarm occurred
since the last time the attribute was reset.  It does not mean the
alarm is currently active.

This also means the driver does not need to continuously track the
alarm status.  Once we detect the first alarm, we don't care what it
does until the alarm status is reset and we need to watch for alarms
again.

If one has something like an op-amp voltage comparator, I think the
driver would register a level interrupt.  It be left masked after the
irq handler notes the alarm, to prevent immediately re-asserting.  This
is normal for level interrupts that can not be de-asserted by an action
of the irq handler.  It would be unmasked on the ack/read of the alarm
attribute.  That would trigger another interrupt if the alarm signal is
still asserted.

If instead, you tried registering for IRQs on both edges, then it's not
reliable.  It's possible for the edges to come in too fast, before the
irq controller or the kernel is ready for them, and then you get out of
sync.