| Message ID | 1442850433-5903-13-git-send-email-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Sudeep Holla <sudeep.holla@arm.com> [150921 08:52]: > 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 > enable_irq_wake instead. Applying into omap-for-v4.4/cleanup thanks. Tony
* Tony Lindgren <tony@atomide.com> [151012 13:27]: > * Sudeep Holla <sudeep.holla@arm.com> [150921 08:52]: > > 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 > > enable_irq_wake instead. > > Applying into omap-for-v4.4/cleanup thanks. Actually I don't think this does the right thing. The interrupts in the $subject patch are in the always on powerdomain, and we really want them to be excluded from the suspend. So not applying without further explanations. Regards, Tony
On 12/10/15 21:28, Tony Lindgren wrote: > * Tony Lindgren <tony@atomide.com> [151012 13:27]: >> * Sudeep Holla <sudeep.holla@arm.com> [150921 08:52]: >>> 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 >>> enable_irq_wake instead. >> >> Applying into omap-for-v4.4/cleanup thanks. > > Actually I don't think this does the right thing. The interrupts > in the $subject patch are in the always on powerdomain, and we really Agreed > want them to be excluded from the suspend. > OK but what's wrong with this patch. At-least the name suggest it's a wakeup interrupt. And using IRQF_NO_SUSPEND for the wakeup interrupt is simply wrong. > So not applying without further explanations. > But I don't understand the real need for IRQF_NO_SUSPEND over wakeup APIs ?
* Sudeep Holla <sudeep.holla@arm.com> [151013 03:46]: > > > On 12/10/15 21:28, Tony Lindgren wrote: > >* Tony Lindgren <tony@atomide.com> [151012 13:27]: > >>* Sudeep Holla <sudeep.holla@arm.com> [150921 08:52]: > >>>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 > >>>enable_irq_wake instead. > >> > >>Applying into omap-for-v4.4/cleanup thanks. > > > >Actually I don't think this does the right thing. The interrupts > >in the $subject patch are in the always on powerdomain, and we really > > Agreed > > >want them to be excluded from the suspend. > > > > OK but what's wrong with this patch. At-least the name suggest it's a > wakeup interrupt. And using IRQF_NO_SUSPEND for the wakeup interrupt is > simply wrong. Hmm so if we have a separate always on irq controller for the wake-up events and we want to keep it always on and exclude it from any suspend related things.. Why would we not use IRQF_NO_SUSPEND on it? Above you say "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..." and that's exactly what we want to do here :) For the dedicated wake-up interrupts, we have separate registers to enable and disable them. The $subject irq is the shared interrupt that allows making use of the pin specific wake-up interrupts, and for those yes we are using enable_irq_wake(). > >So not applying without further explanations. > > > > But I don't understand the real need for IRQF_NO_SUSPEND over wakeup APIs ? Because in the $subject case we just want to always keep it on and never suspend it. It's unrelated to the wakeup APIs at least for the omap related SoCs. Regards, Tony
On 13/10/15 15:53, Tony Lindgren wrote: > * Sudeep Holla <sudeep.holla@arm.com> [151013 03:46]: >> >> >> On 12/10/15 21:28, Tony Lindgren wrote: >>> * Tony Lindgren <tony@atomide.com> [151012 13:27]: >>>> * Sudeep Holla <sudeep.holla@arm.com> [150921 08:52]: >>>>> 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 >>>>> enable_irq_wake instead. >>>> >>>> Applying into omap-for-v4.4/cleanup thanks. >>> >>> Actually I don't think this does the right thing. The interrupts >>> in the $subject patch are in the always on powerdomain, and we really >> >> Agreed >> >>> want them to be excluded from the suspend. >>> >> >> OK but what's wrong with this patch. At-least the name suggest it's a >> wakeup interrupt. And using IRQF_NO_SUSPEND for the wakeup interrupt is >> simply wrong. > > Hmm so if we have a separate always on irq controller for the wake-up events > and we want to keep it always on and exclude it from any suspend related > things.. Why would we not use IRQF_NO_SUSPEND on it? > > Above you say "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..." and that's exactly what we want to do here :) > OK if these interrupts meet that criteria to use IRQF_NO_SUSPEND, then it should be fine, my earlier argument was based on the assumption that it's just another wakeup interrupt. > For the dedicated wake-up interrupts, we have separate registers to enable > and disable them. The $subject irq is the shared interrupt that allows > making use of the pin specific wake-up interrupts, and for those yes we > are using enable_irq_wake(). > If it's already take care, then fine. I am just hunting all the misuse of IRQF_NO_SUSPEND flag especially as wakeup source and fixing them >>> So not applying without further explanations. >>> >> >> But I don't understand the real need for IRQF_NO_SUSPEND over wakeup APIs ? > > Because in the $subject case we just want to always keep it on and > never suspend it. It's unrelated to the wakeup APIs at least for the > omap related SoCs. > OK, understood now. Thanks -- Regards, Sudeep
diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c index 176eef6ef338..12012bef8e63 100644 --- a/arch/arm/mach-omap2/mux.c +++ b/arch/arm/mach-omap2/mux.c @@ -810,13 +810,13 @@ int __init omap_mux_late_init(void) return 0; ret = request_irq(omap_prcm_event_to_irq("io"), - omap_hwmod_mux_handle_irq, IRQF_SHARED | IRQF_NO_SUSPEND, + omap_hwmod_mux_handle_irq, IRQF_SHARED, "hwmod_io", omap_mux_late_init); if (ret) pr_warn("mux: Failed to setup hwmod io irq %d\n", ret); - return 0; + return enable_irq_wake(omap_prcm_event_to_irq("io")); } static void __init omap_mux_package_fixup(struct omap_mux *p, diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c index 87b98bf92366..4b7ac7cd633a 100644 --- a/arch/arm/mach-omap2/pm34xx.c +++ b/arch/arm/mach-omap2/pm34xx.c @@ -472,23 +472,22 @@ int __init omap3_pm_init(void) prcm_setup_regs(); ret = request_irq(omap_prcm_event_to_irq("wkup"), - _prcm_int_handle_wakeup, IRQF_NO_SUSPEND, "pm_wkup", NULL); + _prcm_int_handle_wakeup, 0, "pm_wkup", NULL); if (ret) { pr_err("pm: Failed to request pm_wkup irq\n"); goto err1; } + enable_irq_wake(omap_prcm_event_to_irq("wkup")); /* IO interrupt is shared with mux code */ ret = request_irq(omap_prcm_event_to_irq("io"), - _prcm_int_handle_io, IRQF_SHARED | IRQF_NO_SUSPEND, "pm_io", - omap3_pm_init); - enable_irq(omap_prcm_event_to_irq("io")); - + _prcm_int_handle_io, IRQF_SHARED, "pm_io", omap3_pm_init); if (ret) { pr_err("pm: Failed to request pm_io irq\n"); goto err2; } + enable_irq_wake(omap_prcm_event_to_irq("io")); ret = pwrdm_for_each(pwrdms_setup, NULL); if (ret) {
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 enable_irq_wake instead. Cc: Tony Lindgren <tony@atomide.com> Cc: Kevin Hilman <khilman@deeprootsystems.com> Cc: linux-omap@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- arch/arm/mach-omap2/mux.c | 4 ++-- arch/arm/mach-omap2/pm34xx.c | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-)