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. -- regards, -grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
