[2/2] pinctrl: single: remove misuse of IRQF_NO_SUSPEND flag
diff mbox

Message ID 1448644860-29323-2-git-send-email-sudeep.holla@arm.com
State New
Headers show

Commit Message

Sudeep Holla Nov. 27, 2015, 5:21 p.m. UTC
From: Sudeep Holla <Sudeep.Holla@arm.com>

The IRQF_NO_SUSPEND flag is used to identify the interrupts that should
be left enabled so as to allow them to work as expected during the
suspend-resume cycle, but doesn't guarantee that it will wake the system
from a suspended state, enable_irq_wake is recommended to be used for
the wakeup.

This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
irq_set_irq_wake instead.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-gpio@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/pinctrl/pinctrl-single.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Linus Walleij Dec. 1, 2015, 2:06 p.m. UTC | #1
On Fri, Nov 27, 2015 at 6:21 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:

> From: Sudeep Holla <Sudeep.Holla@arm.com>
>
> The IRQF_NO_SUSPEND flag is used to identify the interrupts that should
> be left enabled so as to allow them to work as expected during the
> suspend-resume cycle, but doesn't guarantee that it will wake the system
> from a suspended state, enable_irq_wake is recommended to be used for
> the wakeup.
>
> This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
> irq_set_irq_wake instead.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-gpio@vger.kernel.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

I need Tony's ACK on this as well.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Dec. 3, 2015, 6:13 p.m. UTC | #2
* Linus Walleij <linus.walleij@linaro.org> [151201 06:07]:
> On Fri, Nov 27, 2015 at 6:21 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> > From: Sudeep Holla <Sudeep.Holla@arm.com>
> >
> > The IRQF_NO_SUSPEND flag is used to identify the interrupts that should
> > be left enabled so as to allow them to work as expected during the
> > suspend-resume cycle, but doesn't guarantee that it will wake the system
> > from a suspended state, enable_irq_wake is recommended to be used for
> > the wakeup.
> >
> > This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
> > irq_set_irq_wake instead.
> >
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: linux-gpio@vger.kernel.org
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> I need Tony's ACK on this as well.

At least on omaps, this controller is always powered and we never want to
suspend it as it handles wake-up events for all the IO pins. And that
usecase sounds exactly like what you're describing above.

I don't quite follow what your suggested alternative for an interrupt
controller is?

At least we need to have the alternative patched in with this chage before
just removing IRQF_NO_SUSPEND.

The enable_irq_wake is naturally used for the consumer drivers of this
interrupt controller and actually mostly done automatically now with the
dev_pm_set_dedicated_wake_irq.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grygorii Strashko Dec. 3, 2015, 6:35 p.m. UTC | #3
On 12/03/2015 08:13 PM, Tony Lindgren wrote:
> * Linus Walleij <linus.walleij@linaro.org> [151201 06:07]:
>> On Fri, Nov 27, 2015 at 6:21 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>> From: Sudeep Holla <Sudeep.Holla@arm.com>
>>>
>>> The IRQF_NO_SUSPEND flag is used to identify the interrupts that should
>>> be left enabled so as to allow them to work as expected during the
>>> suspend-resume cycle, but doesn't guarantee that it will wake the system
>>> from a suspended state, enable_irq_wake is recommended to be used for
>>> the wakeup.
>>>
>>> This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
>>> irq_set_irq_wake instead.
>>>
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>> Cc: linux-gpio@vger.kernel.org
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>
>> I need Tony's ACK on this as well.
> 
> At least on omaps, this controller is always powered and we never want to
> suspend it as it handles wake-up events for all the IO pins. And that
> usecase sounds exactly like what you're describing above.
> 
> I don't quite follow what your suggested alternative for an interrupt
> controller is?
> 
> At least we need to have the alternative patched in with this chage before
> just removing IRQF_NO_SUSPEND.
> 
> The enable_irq_wake is naturally used for the consumer drivers of this
> interrupt controller and actually mostly done automatically now with the
> dev_pm_set_dedicated_wake_irq.
> 

I think, this patch should not break our wake-up functionality.
It will just change the moment when pcs_irq_handler() will be called:

before this change:
- suspend_enter()
  ....
  - arch_suspend_enable_irqs();
    - ^ right here

after this change:
- suspend_enter()
  ....
  dpm_resume_noirq()
  - resume_device_irqs()
    ^ here

Correct? And as for me this is more safe.
Sudeep Holla Dec. 3, 2015, 6:59 p.m. UTC | #4
On 03/12/15 18:13, Tony Lindgren wrote:
> * Linus Walleij <linus.walleij@linaro.org> [151201 06:07]:
>> On Fri, Nov 27, 2015 at 6:21 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>> From: Sudeep Holla <Sudeep.Holla@arm.com>
>>>
>>> The IRQF_NO_SUSPEND flag is used to identify the interrupts that should
>>> be left enabled so as to allow them to work as expected during the
>>> suspend-resume cycle, but doesn't guarantee that it will wake the system
>>> from a suspended state, enable_irq_wake is recommended to be used for
>>> the wakeup.
>>>
>>> This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
>>> irq_set_irq_wake instead.
>>>
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>> Cc: linux-gpio@vger.kernel.org
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>
>> I need Tony's ACK on this as well.
>
> At least on omaps, this controller is always powered and we never want to
> suspend it as it handles wake-up events for all the IO pins. And that
> usecase sounds exactly like what you're describing above.
>

Understood, but I assume this is a generic driver that can be used by
any pinmux.

> I don't quite follow what your suggested alternative for an interrupt
> controller is?
>

Why can't we use enable_irq_wake even for parent/interrupt controller as
they can be considered as parent wakeup irq. I agree the interrupt
controller may not be powered down, but still it's part of wakeup and
the irq core needs to identify that. By just marking IRQF_NO_SUSPEND,
you are saying that you can handle interrupt in the suspend path but not
informing that it's a wakeup interrupt.

With this change, the wakeup handler (including the parent handler) is
called when it's safe as the irq core maintains the state machine.

> At least we need to have the alternative patched in with this chage before
> just removing IRQF_NO_SUSPEND.
>

I have added irq_set_irq_wake(pcs_soc->irq, state) in pcs_irq_set_wake
which ensures it's marked for wakeup.

> The enable_irq_wake is naturally used for the consumer drivers of this
> interrupt controller and actually mostly done automatically now with the
> dev_pm_set_dedicated_wake_irq.
>

Agreed, no doubt on that.
Tony Lindgren Dec. 3, 2015, 9:37 p.m. UTC | #5
* Grygorii Strashko <grygorii.strashko@ti.com> [151203 10:36]:
> 
> I think, this patch should not break our wake-up functionality.
> It will just change the moment when pcs_irq_handler() will be called:
> 
> before this change:
> - suspend_enter()
>   ....
>   - arch_suspend_enable_irqs();
>     - ^ right here
> 
> after this change:
> - suspend_enter()
>   ....
>   dpm_resume_noirq()
>   - resume_device_irqs()
>     ^ here
> 
> Correct? And as for me this is more safe.

I think there's more to it though. With both applied, it produces this on
coming back up from suspend:

PM: noirq resume of devices complete after 18.127 msecs
------------[ cut here ]------------
WARNING: CPU: 0 PID: 123 at kernel/irq/manage.c:605 irq_set_irq_wake+0xbc/0xfc()
Unbalanced IRQ 375 wake disable
Modules linked in: ledtrig_default_on leds_gpio led_class rtc_twl twl4030_wdt
CPU: 0 PID: 123 Comm: bash Tainted: G        W       4.4.0-rc3-dirty #2682
Hardware name: Generic OMAP36xx (Flattened Device Tree)
[<c0017df0>] (unwind_backtrace) from [<c0014084>] (show_stack+0x10/0x14)
<c0014084>] (show_stack) from [<c03492d0>] (dump_stack+0x84/0x9c)
[<c03492d0>] (dump_stack) from [<c003ca2c>] (warn_slowpath_common+0x7c/0xb8)
[<c003ca2c>] (warn_slowpath_common) from [<c003ca98>] (warn_slowpath_fmt+0x30/0x40)
[<c003ca98>] (warn_slowpath_fmt) from [<c009b66c>] (irq_set_irq_wake+0xbc/0xfc)
[<c009b66c>] (irq_set_irq_wake) from [<c03f0f1c>] (device_wakeup_disarm_wake_irqs+0x70/0x12c)
[<c03f0f1c>] (device_wakeup_disarm_wake_irqs) from [<c03ee4ac>] (dpm_resume_noirq+0x20c/0x2e4)
[<c03ee4ac>] (dpm_resume_noirq) from [<c0095e94>] (suspend_devices_and_enter+0x1e4/0x6bc)
[<c0095e94>] (suspend_devices_and_enter) from [<c00966c4>] (pm_suspend+0x358/0x4b8)
[<c00966c4>] (pm_suspend) from [<c0094fdc>] (state_store+0x64/0xb8)
[<c0094fdc>] (state_store) from [<c034b46c>] (kobj_attr_store+0x14/0x20)
[<c034b46c>] (kobj_attr_store) from [<c01ea4d8>] (sysfs_kf_write+0x4c/0x50)
[<c01ea4d8>] (sysfs_kf_write) from [<c01e9afc>] (kernfs_fop_write+0xbc/0x1cc)
[<c01e9afc>] (kernfs_fop_write) from [<c0171c7c>] (__vfs_write+0x24/0xd8)
[<c0171c7c>] (__vfs_write) from [<c0172520>] (vfs_write+0x94/0x154)
[<c0172520>] (vfs_write) from [<c0172d1c>] (SyS_write+0x40/0x94)
[<c0172d1c>] (SyS_write) from [<c000f760>] (ret_fast_syscall+0x0/0x1c)
---[ end trace 321b51565e161bee ]---

And these both need to be applied together when we have a fix for the above
as otherwise we'll get the lock recursion Sudeep mentioned in patch 2/2.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Dec. 3, 2015, 9:40 p.m. UTC | #6
* Sudeep Holla <sudeep.holla@arm.com> [151203 11:00]:
> On 03/12/15 18:13, Tony Lindgren wrote:
> >At least on omaps, this controller is always powered and we never want to
> >suspend it as it handles wake-up events for all the IO pins. And that
> >usecase sounds exactly like what you're describing above.
> >
> 
> Understood, but I assume this is a generic driver that can be used by
> any pinmux.

Right no question about that, but we need to keep things working. I just
pasted the output to this thread what happens coming back up from suspend.

> >I don't quite follow what your suggested alternative for an interrupt
> >controller is?
> 
> Why can't we use enable_irq_wake even for parent/interrupt controller as
> they can be considered as parent wakeup irq. I agree the interrupt
> controller may not be powered down, but still it's part of wakeup and
> the irq core needs to identify that. By just marking IRQF_NO_SUSPEND,
> you are saying that you can handle interrupt in the suspend path but not
> informing that it's a wakeup interrupt.
> 
> With this change, the wakeup handler (including the parent handler) is
> called when it's safe as the irq core maintains the state machine.

Maybe paste a suggested patch and I can try it. I guess you mean call
that from pinctrl-single.c.

> >At least we need to have the alternative patched in with this chage before
> >just removing IRQF_NO_SUSPEND.
> >
> 
> 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.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Dec. 4, 2015, 3:40 p.m. UTC | #7
* Tony Lindgren <tony@atomide.com> [151203 13:41]:
> * Sudeep Holla <sudeep.holla@arm.com> [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.

I think all that can be left out with the snipped from Grygorii, and maybe
also the lock_class_key changes.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sudeep Holla Dec. 4, 2015, 3:44 p.m. UTC | #8
On 04/12/15 15:40, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [151203 13:41]:
>> * Sudeep Holla <sudeep.holla@arm.com> [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.

> I think all that can be left out with the snipped from Grygorii, and maybe
> also the lock_class_key changes.

OK
Sudeep Holla Dec. 4, 2015, 4:15 p.m. UTC | #9
Hi Tony,

On 04/12/15 15:40, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [151203 13:41]:
>> * Sudeep Holla <sudeep.holla@arm.com> [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.
>

After thinking more about it we need some way to tell IRQ core that
pcs_soc->irq is wakeup capable. Is that going to happen automatically
via dev_pm_set_dedicated_wake_irq as you mentioned earlier ?

> I think all that can be left out with the snipped from Grygorii, and maybe
> also the lock_class_key changes.
>

If we not calling irq_set_irq_wake(pcs_soc->irq) in pcs_irq_set_wake, do
you see possibility of lockdep recursion in any other paths.

Otherwise we don't need this if we remove irq_set_irq_wake(pcs_soc->irq)
from pcs_irq_set_wake
Grygorii Strashko Dec. 4, 2015, 4:19 p.m. UTC | #10
On 12/04/2015 05:44 PM, Sudeep Holla wrote:
> 
> 
> On 04/12/15 15:40, Tony Lindgren wrote:
>> * Tony Lindgren <tony@atomide.com> [151203 13:41]:
>>> * Sudeep Holla <sudeep.holla@arm.com> [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.
Sudeep Holla Dec. 4, 2015, 4:26 p.m. UTC | #11
On 04/12/15 16:19, Grygorii Strashko wrote:
> On 12/04/2015 05:44 PM, Sudeep Holla wrote:
>>
>>
>> On 04/12/15 15:40, Tony Lindgren wrote:
>>> * Tony Lindgren <tony@atomide.com> [151203 13:41]:
>>>> * Sudeep Holla <sudeep.holla@arm.com> [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.
>

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

> 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.
>

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.
Tony Lindgren Dec. 4, 2015, 5:06 p.m. UTC | #12
* Sudeep Holla <sudeep.holla@arm.com> [151204 08:27]:
> 
> 
> On 04/12/15 16:19, Grygorii Strashko wrote:
> >On 12/04/2015 05:44 PM, Sudeep Holla wrote:
> >>
> >>
> >>On 04/12/15 15:40, Tony Lindgren wrote:
> >>>* Tony Lindgren <tony@atomide.com> [151203 13:41]:
> >>>>* Sudeep Holla <sudeep.holla@arm.com> [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.
> >
> 
> Thanks for the detailed explanation. I was bit confused if my
> understanding is correct or not.
> 
> >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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Dec. 4, 2015, 5:10 p.m. UTC | #13
* Sudeep Holla <sudeep.holla@arm.com> [151204 08:16]:
> Hi Tony,
> 
> On 04/12/15 15:40, Tony Lindgren wrote:
> >* Tony Lindgren <tony@atomide.com> [151203 13:41]:
> >>* Sudeep Holla <sudeep.holla@arm.com> [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.
> >
> 
> After thinking more about it we need some way to tell IRQ core that
> pcs_soc->irq is wakeup capable. Is that going to happen automatically
> via dev_pm_set_dedicated_wake_irq as you mentioned earlier ?
> 
> >I think all that can be left out with the snipped from Grygorii, and maybe
> >also the lock_class_key changes.
> >
> 
> If we not calling irq_set_irq_wake(pcs_soc->irq) in pcs_irq_set_wake, do
> you see possibility of lockdep recursion in any other paths.
> 
> Otherwise we don't need this if we remove irq_set_irq_wake(pcs_soc->irq)
> from pcs_irq_set_wake

I think Grygorii is right here and this is correct as it's a counter
once the other issues are sorted out and we have figured out what all
needs to be patched together.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 945a7d0f0704..a1e32c6b554a 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1622,12 +1622,14 @@  static void pcs_irq_unmask(struct irq_data *d)
  */
 static int pcs_irq_set_wake(struct irq_data *d, unsigned int state)
 {
+	struct pcs_soc_data *pcs_soc = irq_data_get_irq_chip_data(d);
+
 	if (state)
 		pcs_irq_unmask(d);
 	else
 		pcs_irq_mask(d);
 
-	return 0;
+	return irq_set_irq_wake(pcs_soc->irq, state);
 }
 
 /**
@@ -1763,8 +1765,7 @@  static int pcs_irq_init_chained_handler(struct pcs_device *pcs,
 		int res;
 
 		res = request_irq(pcs_soc->irq, pcs_irq_handler,
-				  IRQF_SHARED | IRQF_NO_SUSPEND |
-				  IRQF_NO_THREAD,
+				  IRQF_SHARED | IRQF_NO_THREAD,
 				  name, pcs_soc);
 		if (res) {
 			pcs_soc->irq = -1;