* Sudeep Holla [off-list ref] [151204 08:27]:

On 04/12/15 16:19, Grygorii Strashko wrote:

quoted

On 12/04/2015 05:44 PM, Sudeep Holla wrote:

quoted

On 04/12/15 15:40, Tony Lindgren wrote:

quoted

* Tony Lindgren [off-list ref] [151203 13:41]:

quoted

* Sudeep Holla [off-list ref] [151203 11:00]:

quoted

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.

Thanks for the detailed explanation. I was bit confused if my
understanding is correct or not.

quoted

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.

OK yeah if it's a counter then it makes sense to me.

Agreed, since I don't have platform to test, I will leave it you guys to
pick up these patches when ready and with any changes if required.

Yeah probably best that Grygorii tries to sort it out :)

Regards,

Tony