genirq: provide dummy set_irq_wake()

PATCH In review Roger Quadros 5 participants 12 messages Mar 30, 2015 – May 11, 2015 IRQ

Share & tools

Roger Quadros

author

2015-03-30 13:15 UTC #0 lore raw

PATCH 1/1 genirq: provide dummy set_irq_wake()

Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
(e.g. drivers/gpio/gpio-pcf857x.c)

http://article.gmane.org/gmane.linux.kernel/1879035

Signed-off-by Roger Quadros <[email protected]>

---
 kernel/irq/dummychip.c | 6 ++++++
 1 file changed, 6 insertions(+)

kernel/irq/dummychip.c +6 -0

index 988dc58..2405d7a 100644
@@ -32,6 +32,11 @@ static unsigned int noop_ret(struct irq_data *data)
 	return 0;
 }
 
+static int noop_int_ret(struct irq_data *data, unsigned int val)
+{
+	return 0;
+}
+
 /*
  * Generic no controller implementation
  */

@@ -57,5 +62,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.irq_set_wake	= noop_int_ret,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);

Signature · 1 line

2.1.0

Felipe Balbi

2015-03-30 18:06 UTC #1 lore raw

Quoted 17 lines · Roger Quadros

Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
(e.g. drivers/gpio/gpio-pcf857x.c)
http://article.gmane.org/gmane.linux.kernel/1879035
Signed-off-by Roger Quadros <[email protected]>
---
 kernel/irq/dummychip.c | 6 ++++++
 1 file changed, 6 insertions(+)
kernel/irq/dummychip.c +0 -0
index 988dc58..2405d7a 100644

kernel/irq/dummychip.c static unsigned int noop_ret(struct irq_data *data) · line 39

Quoted 8 lines

kernel/irq/dummychip.c +5 -0

@@ -32,6 +32,11 @@ static unsigned int noop_ret(struct irq_data *data)
 	return 0;
 }
 
+static int noop_int_ret(struct irq_data *data, unsigned int val)
+{
+	return 0;
+}
+

isn't this the same as noop_ret ?

Signature · 1 line

balbi

Felipe Balbi

2015-03-30 18:06 UTC #2 lore raw

Quoted 21 lines · Felipe Balbi

Quoted 20 lines · Roger Quadros
Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
(e.g. drivers/gpio/gpio-pcf857x.c)
http://article.gmane.org/gmane.linux.kernel/1879035
Signed-off-by Roger Quadros <[email protected]>
---
 kernel/irq/dummychip.c | 6 ++++++
 1 file changed, 6 insertions(+)
kernel/irq/dummychip.c +0 -0
index 988dc58..2405d7a 100644
@@ -32,6 +32,11 @@ static unsigned int noop_ret(struct irq_data *data)
 	return 0;
 }
 

Quoted 7 lines

kernel/irq/dummychip.c static unsigned int noop_ret(struct irq_data *data) · line 39
Quoted 5 lines
kernel/irq/dummychip.c +5 -0
+static int noop_int_ret(struct irq_data *data, unsigned int val)
+{
+	return 0;
+}
+
isn't this the same as noop_ret ?

oh no, unsigned versus signed.

Signature · 1 line

balbi

Roger Quadros

author

2015-04-14 10:14 UTC #3 lore raw

In reply to Roger Quadros

Hi Thomas,

Quoted 8 lines · Roger Quadros

Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
(e.g. drivers/gpio/gpio-pcf857x.c)
http://article.gmane.org/gmane.linux.kernel/1879035
Signed-off-by Roger Quadros <[email protected]>

Any comments on this patch?

cheers,
-roger

Quoted 28 lines

---
 kernel/irq/dummychip.c | 6 ++++++
 1 file changed, 6 insertions(+)

kernel/irq/dummychip.c +6 -0

index 988dc58..2405d7a 100644
@@ -32,6 +32,11 @@ static unsigned int noop_ret(struct irq_data *data)
 	return 0;
 }
 
+static int noop_int_ret(struct irq_data *data, unsigned int val)
+{
+	return 0;
+}
+
 /*
  * Generic no controller implementation
  */

@@ -57,5 +62,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.irq_set_wake	= noop_int_ret,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);

Gregory CLEMENT

2015-04-14 14:02 UTC #4 lore raw

Hi Roger,

Quoted 8 lines · Roger Quadros

Hi Thomas,
Quoted 5 lines · Roger Quadros
Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
(e.g. drivers/gpio/gpio-pcf857x.c)

Quoted 5 lines

Quoted 3 lines
http://article.gmane.org/gmane.linux.kernel/1879035
Signed-off-by Roger Quadros <[email protected]>
Any comments on this patch?

I read the url you pointed and I wonder why, at then end, did you choose to add a dummy set_irq_wake() instead of using IRQCHIP_SKIP_SET_WAKE ?

Thanks,

Gregory

Quoted 38 lines

cheers,
-roger
Quoted 22 lines
---
 kernel/irq/dummychip.c | 6 ++++++
 1 file changed, 6 insertions(+)
kernel/irq/dummychip.c +5 -0
index 988dc58..2405d7a 100644
@@ -32,6 +32,11 @@ static unsigned int noop_ret(struct irq_data *data)
 	return 0;
 }
 
+static int noop_int_ret(struct irq_data *data, unsigned int val)
+{
+	return 0;
+}
+
 /*
  * Generic no controller implementation
  */
kernel/irq/dummychip.c struct irq_chip dummy_irq_chip = { · line 67
Quoted 6 lines
kernel/irq/dummychip.c +1 -0
@@ -57,5 +62,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.irq_set_wake	= noop_int_ret,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Roger Quadros

author

2015-04-15 08:07 UTC #5 lore raw

Hi Gregory,

Quoted 13 lines · Gregory CLEMENT

Hi Roger,
Quoted 10 lines · Roger Quadros
Hi Thomas,
Quoted 7 lines · Roger Quadros
Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
(e.g. drivers/gpio/gpio-pcf857x.c)
http://article.gmane.org/gmane.linux.kernel/1879035

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Signed-off-by Roger Quadros <[email protected]>
Any comments on this patch?
I read the url you pointed and I wonder why, at then end, did you choose to add a dummy set_irq_wake() instead of using IRQCHIP_SKIP_SET_WAKE ?

You mean like this?

kernel/irq/dummychip.c +1 -0

index 988dc58..2feb6fe 100644
@@ -57,5 +57,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.flags		= IRQCHIP_SKIP_SET_WAKE,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);

This works as well and is better. I'll post a v2.

cheers,
-roger

Quoted 37 lines

Quoted 35 lines
Quoted 23 lines
---
 kernel/irq/dummychip.c | 6 ++++++
 1 file changed, 6 insertions(+)
kernel/irq/dummychip.c +5 -0
index 988dc58..2405d7a 100644
@@ -32,6 +32,11 @@ static unsigned int noop_ret(struct irq_data *data)
 	return 0;
 }
 
+static int noop_int_ret(struct irq_data *data, unsigned int val)
+{
+	return 0;
+}
+
 /*
  * Generic no controller implementation
  */
kernel/irq/dummychip.c struct irq_chip dummy_irq_chip = { · line 67
Quoted 6 lines
kernel/irq/dummychip.c +1 -0
@@ -57,5 +62,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.irq_set_wake	= noop_int_ret,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gregory CLEMENT

2015-04-15 08:51 UTC #6 lore raw

Hi Roger,

Quoted 31 lines · Roger Quadros

Hi Gregory,
Quoted 13 lines · Gregory CLEMENT
Hi Roger,
Quoted 10 lines · Roger Quadros
Hi Thomas,
Quoted 7 lines · Roger Quadros
Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
(e.g. drivers/gpio/gpio-pcf857x.c)
http://article.gmane.org/gmane.linux.kernel/1879035
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Roger Quadros <[email protected]>
Any comments on this patch?
I read the url you pointed and I wonder why, at then end, did you choose to add a dummy set_irq_wake() instead of using IRQCHIP_SKIP_SET_WAKE ?
You mean like this?
kernel/irq/dummychip.c +0 -0
index 988dc58..2feb6fe 100644

kernel/irq/dummychip.c struct irq_chip dummy_irq_chip = { · line 62

Quoted 6 lines

kernel/irq/dummychip.c +1 -0

@@ -57,5 +57,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.flags		= IRQCHIP_SKIP_SET_WAKE,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);

Yes exactly.

Quoted 2 lines

This works as well and is better. I'll post a v2.

Thanks,

Gregory

Quoted 42 lines

cheers,
-roger
Quoted 37 lines
Quoted 35 lines
Quoted 23 lines
---
 kernel/irq/dummychip.c | 6 ++++++
 1 file changed, 6 insertions(+)
kernel/irq/dummychip.c +5 -0
index 988dc58..2405d7a 100644
@@ -32,6 +32,11 @@ static unsigned int noop_ret(struct irq_data *data)
 	return 0;
 }
 
+static int noop_int_ret(struct irq_data *data, unsigned int val)
+{
+	return 0;
+}
+
 /*
  * Generic no controller implementation
  */
kernel/irq/dummychip.c struct irq_chip dummy_irq_chip = { · line 67
Quoted 6 lines
kernel/irq/dummychip.c +1 -0
@@ -57,5 +62,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.irq_set_wake	= noop_int_ret,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 8 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Roger Quadros

author

2015-04-15 08:14 UTC #7 lore raw

PATCH v2 1/1 genirq: Set IRQCHIP_SKIP_SET_WAKE flag for dummy_irq_chip

In reply to Roger Quadros

Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
(e.g. drivers/gpio/gpio-pcf857x.c)

http://article.gmane.org/gmane.linux.kernel/1879035

Signed-off-by Roger Quadros <[email protected]>

---
 kernel/irq/dummychip.c | 1 +
 1 file changed, 1 insertion(+)

kernel/irq/dummychip.c +1 -0

index 988dc58..2feb6fe 100644
@@ -57,5 +57,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.flags		= IRQCHIP_SKIP_SET_WAKE,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);

Signature · 1 line

2.1.0

Gregory CLEMENT

Reviewed-by on v2 1/1

2015-04-15 09:29 UTC #8 lore raw

Re: [PATCH v2] genirq: Set IRQCHIP_SKIP_SET_WAKE flag for dummy_irq_chip

Quoted 8 lines · Roger Quadros

Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
(e.g. drivers/gpio/gpio-pcf857x.c)
http://article.gmane.org/gmane.linux.kernel/1879035
Signed-off-by Roger Quadros <[email protected]>

FWIW:

Reviewed-by Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/[email protected]>

Quoted 16 lines

---
 kernel/irq/dummychip.c | 1 +
 1 file changed, 1 insertion(+)

kernel/irq/dummychip.c +1 -0

index 988dc58..2feb6fe 100644
@@ -57,5 +57,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.flags		= IRQCHIP_SKIP_SET_WAKE,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);

Signature · 8 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

tip-bot for Roger Quadros

merge bot

2015-04-24 19:01 UTC #9 lore raw

[tip:irq/urgent] genirq: Set IRQCHIP_SKIP_SET_WAKE flag for dummy_irq_chip

In reply to Roger Quadros

genirq: Set IRQCHIP_SKIP_SET_WAKE flag for dummy_irq_chip

Commit-ID:  10a50f1ab5f06c9a3ee5ece3ec52e607ed53c79f
Gitweb:     http://git.kernel.org/tip/10a50f1ab5f06c9a3ee5ece3ec52e607ed53c79f
Author:     Roger Quadros <[email protected]>
AuthorDate: Wed, 15 Apr 2015 11:14:11 +0300
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 24 Apr 2015 20:57:06 +0200

genirq: Set IRQCHIP_SKIP_SET_WAKE flag for dummy_irq_chip

Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook. (e.g. drivers/gpio/gpio-pcf857x.c)

Signed-off-by Roger Quadros <[email protected]>

Cc <[email protected]>

Cc Gregory Clement <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/irq/dummychip.c | 1 +
 1 file changed, 1 insertion(+)

kernel/irq/dummychip.c +1 -0

index 988dc58..2feb6fe 100644
@@ -57,5 +57,6 @@ struct irq_chip dummy_irq_chip = {
 	.irq_ack	= noop,
 	.irq_mask	= noop,
 	.irq_unmask	= noop,
+	.flags		= IRQCHIP_SKIP_SET_WAKE,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);

Geert Uytterhoeven

2015-05-11 10:50 UTC #10 lore raw

Re: [PATCH v2] genirq: Set IRQCHIP_SKIP_SET_WAKE flag for dummy_irq_chip

In reply to Roger Quadros

Quoted 3 lines · Roger Quadros

Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.

Good to see this was applied. Still, I think no_irq_chip should also be fixed, cfr. my patch "genirq: Set IRQCHIP_SKIP_SET_WAKE for no_irq_chip and dummy_irq_chip" (https://lkml.org/lkml/2015/1/12/506 is down, but
https://patchwork.ozlabs.org/patch/427788/ works)

Quoted 1 line

(e.g. drivers/gpio/gpio-pcf857x.c)

Please note the particular pfc857x.c case was fixed by commit b80eef95beb04760 ("gpio: pcf857x: Propagate wake-up setting to parent irq controller"), as that driver has other problems. Now it no longer uses dummy_irq_chip.

Quoted 19 lines

http://article.gmane.org/gmane.linux.kernel/1879035

Signed-off-by Roger Quadros <[email protected]>

---
 kernel/irq/dummychip.c | 1 +
 1 file changed, 1 insertion(+)

kernel/irq/dummychip.c +1 -1

index 988dc58..2feb6fe 100644
@@ -57,5 +57,6 @@ struct irq_chip dummy_irq_chip = {
        .irq_ack        = noop,
        .irq_mask       = noop,
        .irq_unmask     = noop,
+       .flags          = IRQCHIP_SKIP_SET_WAKE,
 };
 EXPORT_SYMBOL_GPL(dummy_irq_chip);
--

Quoted 1 line

2.1.0

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Roger Quadros

author

2015-05-11 11:15 UTC #11 lore raw

Re: [PATCH v2] genirq: Set IRQCHIP_SKIP_SET_WAKE flag for dummy_irq_chip

Hi Geert,

Quoted 7 lines · Geert Uytterhoeven

Quoted 3 lines · Roger Quadros
Without this system suspend is broken on systems that have drivers calling enable/disable_irq_wake() for interrupts based off the dummy irq hook.
Good to see this was applied. Still, I think no_irq_chip should also be fixed, cfr. my patch "genirq: Set IRQCHIP_SKIP_SET_WAKE for no_irq_chip and

I don't see why not so can you please send a patch to add that? Thanks.

cheers,
-roger

Quoted 42 lines

dummy_irq_chip" (https://lkml.org/lkml/2015/1/12/506 is down, but
https://patchwork.ozlabs.org/patch/427788/ works)
Quoted 1 line
(e.g. drivers/gpio/gpio-pcf857x.c)
Please note the particular pfc857x.c case was fixed by commit b80eef95beb04760 ("gpio: pcf857x: Propagate wake-up setting to parent irq controller"), as that driver has other problems. Now it no longer uses dummy_irq_chip.
Quoted 19 lines
http://article.gmane.org/gmane.linux.kernel/1879035
Signed-off-by Roger Quadros <[email protected]>
---
  kernel/irq/dummychip.c | 1 +
  1 file changed, 1 insertion(+)
kernel/irq/dummychip.c +1 -1
index 988dc58..2feb6fe 100644
@@ -57,5 +57,6 @@ struct irq_chip dummy_irq_chip = {
         .irq_ack        = noop,
         .irq_mask       = noop,
         .irq_unmask     = noop,
+       .flags          = IRQCHIP_SKIP_SET_WAKE,
  };
  EXPORT_SYMBOL_GPL(dummy_irq_chip);
--
Quoted 1 line
2.1.0
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds