Re: Configurable interrupt sources, and DT bindings

[Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 11/28/2011 04:29 PM, Stephen Warren wrote:
> Many interrupt sinks support various of active high/low, rising/falling
> edge. This can already be configured by setting #interrupt-cells
> appropriately, and defining an interrupt-controller-specific binding for
> the additional cells.
> 
> At least some interrupt sources have similar configurability in HW. An
> example is the WM8903 audio codec, which can generate both active high
> and active low interrupt signals.
> 
> One possibility is to describe this directly in the binding for each
> interrupt source. I originally proposed the following for the WM8903:
> 
> * Interrupt output is active high by default.
> * Presence of an "irq-active-low" property selects an active low output
> instead.
> 
> Mark Brown pointed out that we probably want to standardize the binding,
> so that every interrupt source doesn't do something different.
> 
> Perhaps one of:
> 
> irq-active-low;
> irq-active-high;
> irq-edge-falling;
> irq-edge-rising;
> 
> or:
> 
> interrupts-config = <"active-low"> // or "active-high", etc.
> (perhaps with indices in that list matching the interrupts property)
> 
> ... and a helper function in the kernel to parse those
> 
> However, given that interrupt sinks already know which signaling methods
> they support, and may be configured to accept a specific method per
> interrupt input using extra interrupt cells, perhaps the solution isn't
> to create a binding that the interrupt sink parses in isolation, but
> rather to create a new API within the kernel where the interrupt source
> can query the interrupt sink for a list of supported signaling methods,
> and pick one that it also supports, and use it. This list could be
> restricted based on the interrupts property's extra cells.
> 
> What are people's thoughts here; should we go down this
> auto-configuration route, or just explicitly configure interrupt sources
> using a binding other than the interrupts property?
> 
> Related, having this negotiation followed by a request_irq() passing the
> flags back to the sink seems a little redundant, but I suppose if the
> sink is configurable and unconstrained, this is necessary.
> 

I think adding another property is the wrong approach. The information
is already there in the interrupt binding. irq_create_of_mapping almost
does what you need. Perhaps it could be extended to return the type as
part of the irq resource. There are already defined resource bits for this:

/* PnP IRQ specific bits (IORESOURCE_BITS) */
#define IORESOURCE_IRQ_HIGHEDGE		(1<<0)
#define IORESOURCE_IRQ_LOWEDGE		(1<<1)
#define IORESOURCE_IRQ_HIGHLEVEL	(1<<2)
#define IORESOURCE_IRQ_LOWLEVEL		(1<<3)

It may be a bit problematic to change irq_create_of_mapping as Sparc has
a different version.

Rob