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

[Trent Piepho <tpiepho@impinj.com>]

On Fri, 2018-11-02 at 07:38 +0100, Marco Felsch wrote:
> On 18-11-01 18:21, Trent Piepho wrote:
> > 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've been spoken about gpio-controllers and AFAIK there are no edge
> types. Interrupt-Controller are a different story, as you pointed out
> above.

You can use edge triggering with gpios.  Just try writing "rising" or
"falling" into /sys/class/gpio/gpioX/edge

It's level you can't do sysfs.  The irq masking necessary isn't
supported to get it to work in a useful way, i.e. without a live-lock
IRQ loop.

But you can in the kernel.

Normal process is to call gpiod_to_irq() and then use standard IRQF
flags to select level, edge, etc.

> Too fast state changes sounds like a glitch. Anyway, IMHO we should

Linux has no hard real-time guarantee about interrupt latency, so
there's no way you can be sure each interrupt is processed before the
next.

Trying to track level by interrupting on both edges doesn't work well. 
You get out of sync and stuck waiting for something that's already
happened.

> support support both interrupts and gpios. If the users use gpios they
> have to poll the gpio, as Guenter pointed out, else we register a
> irq-handler.

If gpio(d?)_to_irq returns a valid value, why poll?  It usually works
to call this.  Plenty of call sites in the kernel that use it and don't
fallback to polling if it doesn't work.

I think it's fine to call gpiod_to_irq() and fail if that fails, and
let a polling fallback be written if and when the need arises.