Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1229489 > unrolled thread

[PATCH 13/17] net: gianfar: remove misuse of IRQF_NO_SUSPEND flag

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 13/17] net: gianfar: remove misuse of IRQF_NO_SUSPEND flag Sudeep Holla <sudeep.holla@arm.com> - 2015-09-21 17:50 +0200
    Re: [PATCH 13/17] net: gianfar: remove misuse of IRQF_NO_SUSPEND flag Sudeep Holla <sudeep.holla@arm.com> - 2015-09-21 18:40 +0200
    RE: [PATCH 13/17] net: gianfar: remove misuse of IRQF_NO_SUSPEND  flag Thomas Gleixner <tglx@linutronix.de> - 2015-09-21 19:00 +0200
    RE: [PATCH 13/17] net: gianfar: remove misuse of IRQF_NO_SUSPEND flag Manoil Claudiu <claudiu.manoil@freescale.com> - 2015-09-21 19:00 +0200

#1229489 — [PATCH 13/17] net: gianfar: remove misuse of IRQF_NO_SUSPEND flag

The device is set as wakeup capable using proper wakeup API but the
driver misuses IRQF_NO_SUSPEND to set the interrupt as wakeup source
which is incorrect.

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

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Claudiu Manoil <claudiu.manoil@freescale.com>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 4b69d061d90f..803ed4c93503 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -1970,8 +1970,7 @@ static int register_grp_irqs(struct gfar_priv_grp *grp)
 		/* Install our interrupt handlers for Error,
 		 * Transmit, and Receive
 		 */
-		err = request_irq(gfar_irq(grp, ER)->irq, gfar_error,
-				  IRQF_NO_SUSPEND,
+		err = request_irq(gfar_irq(grp, ER)->irq, gfar_error, 0,
 				  gfar_irq(grp, ER)->name, grp);
 		if (err < 0) {
 			netif_err(priv, intr, dev, "Can't get IRQ %d\n",
@@ -1979,6 +1978,8 @@ static int register_grp_irqs(struct gfar_priv_grp *grp)
 
 			goto err_irq_fail;
 		}
+		enable_irq_wake(gfar_irq(grp, ER)->irq);
+
 		err = request_irq(gfar_irq(grp, TX)->irq, gfar_transmit, 0,
 				  gfar_irq(grp, TX)->name, grp);
 		if (err < 0) {
@@ -1994,14 +1995,14 @@ static int register_grp_irqs(struct gfar_priv_grp *grp)
 			goto rx_irq_fail;
 		}
 	} else {
-		err = request_irq(gfar_irq(grp, TX)->irq, gfar_interrupt,
-				  IRQF_NO_SUSPEND,
+		err = request_irq(gfar_irq(grp, TX)->irq, gfar_interrupt, 0,
 				  gfar_irq(grp, TX)->name, grp);
 		if (err < 0) {
 			netif_err(priv, intr, dev, "Can't get IRQ %d\n",
 				  gfar_irq(grp, TX)->irq);
 			goto err_irq_fail;
 		}
+		enable_irq_wake(gfar_irq(grp, TX)->irq);
 	}
 
 	return 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1229546

On 21/09/15 17:24, Manoil Claudiu wrote:
>> -----Original Message-----
>> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
>> Sent: Monday, September 21, 2015 6:47 PM
>> To: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
>> Cc: Sudeep Holla <sudeep.holla@arm.com>; Thomas Gleixner
>> <tglx@linutronix.de>; Rafael J. Wysocki <rjw@rjwysocki.net>; David S. Miller
>> <davem@davemloft.net>; Manoil Claudiu-B08782
>> <claudiu.manoil@freescale.com>; Kevin Hao <haokexin@gmail.com>;
>> netdev@vger.kernel.org
>> Subject: [PATCH 13/17] net: gianfar: remove misuse of IRQF_NO_SUSPEND
>> flag
>>
>> The device is set as wakeup capable using proper wakeup API but the
>> driver misuses IRQF_NO_SUSPEND to set the interrupt as wakeup source
>> which is incorrect.
>>
>> This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
>> enable_irq_wake instead.
>>
>
> What would be the purpose of IRQF_NO_SUSPEND flag then?

If you look at the section "IRQF_NO_SUSPEND Flag" in
Documentation/power/suspend-and-interrupts.txt, it states it doesn't
guarantee that the interrupt will wake the system from a suspended state
-- for such cases it is necessary to use enable_irq_wake().

> The flag is a friendlier API compared to calling enable_irq_wake().

In what sense ?

> For older kernels, on PPC architectures, the flag did the job.

Agreed, it does the job even now, just that it's not designed for that.
One can change that anytime internally as the APIs for configuring
wakeups as in place now. I am just making use of right APIs.

> When did this change? Since when using IRQF_NO_SUSPEND is a "misuse"?
>

Sorry if the term "misuse" is not appropriate here, since the intention
of the flag usage doesn't meet the requirements, I mentioned it as misuse.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229568 — RE: [PATCH 13/17] net: gianfar: remove misuse of IRQF_NO_SUSPEND flag

On Mon, 21 Sep 2015, Manoil Claudiu wrote:
> >The device is set as wakeup capable using proper wakeup API but the
> >driver misuses IRQF_NO_SUSPEND to set the interrupt as wakeup source
> >which is incorrect.
> >
> >This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
> >enable_irq_wake instead.
> >
> 
> What would be the purpose of IRQF_NO_SUSPEND flag then?  The flag is a
> friendlier API compared to calling enable_irq_wake().  For older kernels,

It's not an API, it's just a bandaid for lazy programmers.

> on PPC architectures, the flag did the job.  When did this change? Since
> when using IRQF_NO_SUSPEND is a "misuse"?

It always was. Simply because IRQF_NO_SUSPEND has absolutely nothing
to do with wakeup interrupt sources. It's a flag which excludes the
interrupt from the suspend mechanism, but it does not flag it a wakeup
source.

Thanks,

	tglx


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229570

>-----Original Message-----
>From: Sudeep Holla [mailto:sudeep.holla@arm.com]
>Sent: Monday, September 21, 2015 6:47 PM
>To: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
>Cc: Sudeep Holla <sudeep.holla@arm.com>; Thomas Gleixner
><tglx@linutronix.de>; Rafael J. Wysocki <rjw@rjwysocki.net>; David S. Miller
><davem@davemloft.net>; Manoil Claudiu-B08782
><claudiu.manoil@freescale.com>; Kevin Hao <haokexin@gmail.com>;
>netdev@vger.kernel.org
>Subject: [PATCH 13/17] net: gianfar: remove misuse of IRQF_NO_SUSPEND
>flag
>
>The device is set as wakeup capable using proper wakeup API but the
>driver misuses IRQF_NO_SUSPEND to set the interrupt as wakeup source
>which is incorrect.
>
>This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
>enable_irq_wake instead.
>

What would be the purpose of IRQF_NO_SUSPEND flag then?  The flag is a
friendlier API compared to calling enable_irq_wake().  For older kernels,
on PPC architectures, the flag did the job.  When did this change? Since
when using IRQF_NO_SUSPEND is a "misuse"?

Thanks,
Claudiu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web