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 _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
