On 12/04/2015 05:44 PM, Sudeep Holla wrote:
On 04/12/15 15:40, Tony Lindgren wrote:
* Tony Lindgren <tony@xxxxxxxxxxx> [151203 13:41]:
* Sudeep Holla <sudeep.holla@xxxxxxx> [151203 11:00]:
I have added irq_set_irq_wake(pcs_soc->irq, state) in pcs_irq_set_wake
which ensures it's marked for wakeup.
Hmm well see the error I pasted in this thread, maybe that provides
more clues.
The irq_set_irq_wake(pcs_soc->irq, state) in pcs_irq_set_wake does not
look right to me as pcs_irq_set_wake toggles the irq_wake for each pin
separately, not for the whole controller.
OK, my understanding was that this driver supports multiple single
pinmux with one main irq `pcs_soc->irq`. Hence I added the wakeup on
that irq. I now think that understand is wrong.
With this change, PCS parent IRQ will be marked as wake up source as many
times as many pins were requested as wake up IRQs (protected by counter).
Most of all GPIO IRQ chips work this way.
Of course, if we will look on pinctrl-single.c from only OMAP point of view
then Prent IRQ can be marked as wake up source from probe only once.
But, since this driver expected to be generic - this patch is more correct,
because other HW may require to perform some real HW re-configuration to
enable/disable wake up capabilities for Parent IRQ in Parent IRQ controller.
Any way, in my opinion, it's right and more safe to manage all wakeup IRQs
through IRQ PM core and Device wakeirq framework. And this patch should just
go together with platform changes and not alone.