On 11/29/2011 08:24 PM, Stephen Warren wrote: > Rob Herring wrote at Monday, November 28, 2011 4:24 PM: >> 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.... > ... >>> 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. > > Taking a look at the code, I think it can work like this: > > * Except for SPARC, irq_of_parse_and_map() calls irq_create_of_mapping(), > which calls the IRQ chip's dt_translate() function which can return the > type from the interrupt specifier in DT, and if it did, the type is > passed to irq_set_irq_type(). > > * SPARC's custom irq_of_parse_and_map() doesn't call irq_set_irq_type() > as far as I can tell. I think we can just ignore this for now; if SPARC > wants to participate in this scheme, its custom irq_of_parse_and_map() > could be enhanced appropriately. > > * I assert that all IRQ chips that want to participate must define an > interrupt specifier DT binding that defines the trigger type; e.g. > that implemented by using irq_domain_add_simple() and #interrupt-cells > greater than 1. > > * I assert that all IRQ chips that want to participate must call > irqd_set_trigger_type() from their irq_set_type op. This will cache the > type for later access. This call is missing from Tegra's GPIO interrupt > controller, but can easily be added. > > * Create a new function that drivers can call on their Linux interrupt ID, > which internally calls irqd_get_trigger_type(). This will return the > configured type, and they can adjust their interrupt output pin > appropriately, or fail to probe if they can't support the selected type. > > * We can probably remove any platform data fields that configure a device's > interrupt output type (e.g. WM8903 has an irq_active_low field in pdata) > and replace it with this scheme. If that won't work, we can have drivers > call the new API only when instantiated from DT, so they continue to > work unchanged in the non-DT case. > > Does that sound like a reasonable solution? If so, I can start hooking this > together.
Sounds good to me. Rob _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
