Re: [PATCH v2 1/1] irqchip: exynos-combiner: Save IRQ enable set on suspend

Tomasz Figa
2015-06-16T12:32:19Z

2015-06-16 0:00 GMT+09:00 Javier Martinez Canillas
<javier.martinez@collabora.co.uk>:
> Hello Sudeep,
>
> On 06/15/2015 11:01 AM, Sudeep Holla wrote:
>>
>>
>> On 15/06/15 08:46, Javier Martinez Canillas wrote:
>> [...]
>>
>>>
>>> Sudeep, so we may need something like $subject after all from Doug's
>>> explanations since the combiner chip state is lost during a S2R. I know
>>> that it adds more duplicated code (others irqchip drivers do the same)
>>> and it may not scale well if a chip has many registers but is the best
>>> solution I could came with.
>>>
>>
>> OK
>>
>>> If you have a suggestion for a better alternative, I can give a try and
>>> write the patch. But I think $subject could also land to fix this issue
>>> since is a very non intrusive change and later can be changed once the
>>> irqchip core supports this use case.
>>>
>>
>> Agreed. But I would suggest also to add MASK_ON_SUSPEND and set_irq_wake
>> also and then you can restore iff it's non-zero as irq core will take
>> care of most of the non-wakeup sources. Because I am planning to push
>
> I've looking at this and a problem I found is that IIUC the set_irq_wake
> is not propagated from the the Exynos pinctrl / GPIO driver which is the
> combiner's external interrupt source so the callback is never called.
> Which means that right now only the state of the wakeup source IRQs can't
> be saved since that information is not present.
>
> The drivers/pinctrl/samsung/pinctrl-exynos.c driver enables and disables
> the combiner interrupts but its .irq_set_wake handler only updates the
> wakeup source mask for the external interrupts but does not call the
> combiner .set_irq_wake so that should be changed as well.
>

As far as I'm aware of, wake-up events from pin controllers don't go
through GIC, but rather directly to PMU, which is a dedicated unit
responsible for power management and not a standalone interrupt
controller (well actually I saw a series making it a cascaded
controller some time ago, but I'm not sure if that went in). Based on
this, I don't think we have to call set_irq_wake on GIC. Correct me if
I'm wrong, though.

Best regards,
Tomasz