[PATCH 48/74] GIC: Added dummy handlers for Power Management Suspend Resume

[Rob Herring]

Russell,

On 09/20/2010 10:07 AM, Russell King - ARM Linux wrote:
> On Mon, Sep 20, 2010 at 09:56:10AM -0500, Rob Herring wrote:
>> On 09/20/2010 08:48 AM, Russell King - ARM Linux wrote:
>>> On Mon, Sep 20, 2010 at 07:14:33PM +0530, deepaksi wrote:
>>>> I request you to provide some more inputs on this, so that we can close
>>>> the issue.
>>>
>>> I don't know, especially so without seeing the drivers.
>>
>> Assuming there is no powergating of the GIC, then this patch should be
>> sufficient depending on one question.
>
> The GIC provides no wakeup capabilities.  Providing a dummy 'set_wake'
> function means that driver authors will expect a successful
> enable_irq_wake() to work - when it results in precisely nothing
> being done.

There is no programming distinction between an irq and an wakeup if the 
GIC is powered on in suspend which is platform dependent. It is only a 
s/w distinction of what interrupts you allow to wake you up. If GIC 
remains powered, then it can assert nirq to the core and wake-up. If the 
core is powered down, the external power controller can monitor the NIRQ 
line to wake-up the core. So the powergating of the core has no impact 
on the prvgramming of the GIC for wake-up.

>
> With the situation described, what I envisage is drivers doing something
> like:
>
> 	err = enable_irq_wake(irq);
> 	if (err == 0) {
> 		/* everything is fine, don't need to do anything else */
> 	} else if (err == -ENXIO) {
> 		/* IRQ controller doesn't support wakeups, try something else */
> 	} else {
> 		/* an error occurred trying to enable wakeups */
> 	}
>
> If we go around adding dummy set_irq_wake functions to IRQ controllers
> which don't support wakeup, then there's no way for drivers to know
> this, and there is no method (other than by #ifdeffery) for them to
> sort out this mess.

How about adding a set_wake function ptr as a parameter to 
gic_dist_init? If a platform needs more than the basic operation, they 
can override it.

It doesn't make sense to push the platform dependent behavior of an 
interrupt controller into the drivers. enable_irq_wake is the defined 
way to enable wakeup. Why suggest drivers implement a custom interface 
for that? That will introduce ifdeffery in common drivers like ARM 
Primecell blocks.

Rob