mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets IRQF_NO_SUSPEND
flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
as it doesn't guarantee wakeup for that interrupt.
This patch removes the redundant mpic_irq_set_wake and sets the
IRQCHIP_SKIP_SET_WAKE for only FSL MPIC.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Scott Wood <redacted>
Cc: Hongtao Jia <redacted>
Cc: Marc Zyngier <redacted>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Sudeep Holla <redacted>
---
arch/powerpc/sysdev/mpic.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
From: Scott Wood <hidden> Date: 2015-09-22 23:50:23
On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
quoted hunk
mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets IRQF_NO_SUSPEND
flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
as it doesn't guarantee wakeup for that interrupt.
This patch removes the redundant mpic_irq_set_wake and sets the
IRQCHIP_SKIP_SET_WAKE for only FSL MPIC.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Scott Wood <redacted>
Cc: Hongtao Jia <redacted>
Cc: Marc Zyngier <redacted>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Sudeep Holla <redacted>
---
arch/powerpc/sysdev/mpic.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
What difference does IRQCHIP_SKIP_SET_WAKE make in the absence of an
.irq_set_wake() callback?
This is basically repealing commit 5ff04b7287d87c1db7 ("powerpc/mpic: add
irq_set_wake support"). Wang Dongsheng, can you explain why that patch was
needed?
-Scott
From: Thomas Gleixner <hidden> Date: 2015-09-23 03:49:53
On Wed, 23 Sep 2015, Wang Dongsheng wrote:
quoted
On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
quoted
mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets IRQF_NO_SUSPEND
flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
as it doesn't guarantee wakeup for that interrupt.
Non-freescale return -ENXIO, is there any issue? If non-freescale
platform does not support it, but IPs still use
enable/disable_irq_wake, we should return a error number.
You can just set IRQCHIP_SKIP_SET_WAKE for FSL chips and not for the
others.
@Scott:
If set this flag we cannot keep a irq as a wakeup source when system going to
SUSPEND or MEM.
irq_set_wake() means we can set this irq as a wake source.
IRQCHIP_SKIP_SET_WAKE is ignore irq_set_wake() feature.
Nonsense. IRQCHIP_SKIP_SET_WAKE merily tells the core not to bail on
!chip->irq_set_wake(), but its still marking the interrupt as wakeup
source and therefor not masking it on suspend.
IRQF_NO_SUSPEND is the wrong tool. End of story.
Thanks,
tglx
From: Scott Wood <hidden> Date: 2015-09-23 04:06:24
On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets IRQF_NO_SUSPEND
flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
as it doesn't guarantee wakeup for that interrupt.
This patch removes the redundant mpic_irq_set_wake and sets the
IRQCHIP_SKIP_SET_WAKE for only FSL MPIC.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Scott Wood <redacted>
Cc: Hongtao Jia <redacted>
Cc: Marc Zyngier <redacted>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Sudeep Holla <redacted>
---
arch/powerpc/sysdev/mpic.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
From: Wang Dongsheng <hidden> Date: 2015-09-23 05:31:09
-----Original Message-----
From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
owner@vger.kernel.org] On Behalf Of Thomas Gleixner
Sent: Wednesday, September 23, 2015 11:49 AM
To: Wang Dongsheng-B40534
Cc: Wood Scott-B07421; Sudeep Holla; linux-pm@vger.kernel.org; linux-
kernel@vger.kernel.org; Rafael J. Wysocki; Benjamin Herrenschmidt; Paul
Mackerras; Michael Ellerman; Jia Hongtao-B38951; Marc Zyngier; linuxppc-
dev@lists.ozlabs.org
Subject: RE: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE inste=
ad of
redundant mpic_irq_set_wake
=20
On Wed, 23 Sep 2015, Wang Dongsheng wrote:
quoted
quoted
On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
quoted
mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets IRQF_NO_S=
USPEND
quoted
quoted
quoted
flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_se=
t_wak
quoted
quoted
quoted
is not implemented. Also there's no need to set the IRQF_NO_SUSPEND=
flag
quoted
quoted
quoted
as it doesn't guarantee wakeup for that interrupt.
Non-freescale return -ENXIO, is there any issue? If non-freescale
platform does not support it, but IPs still use
enable/disable_irq_wake, we should return a error number.
=20
You can just set IRQCHIP_SKIP_SET_WAKE for FSL chips and not for the
others.
=20
quoted
@Scott:
If set this flag we cannot keep a irq as a wakeup source when system go=
ing to
quoted
SUSPEND or MEM.
irq_set_wake() means we can set this irq as a wake source.
IRQCHIP_SKIP_SET_WAKE is ignore irq_set_wake() feature.
=20
Nonsense. IRQCHIP_SKIP_SET_WAKE merily tells the core not to bail on
!chip->irq_set_wake(), but its still marking the interrupt as wakeup
source and therefor not masking it on suspend.
=20
Sorry, I just check irq_set_irq_wake() code, right, IRQCHIP_SKIP_SET_WAKE a=
lso can
going to irqd_set to mask IRQD_WAKEUP_STATE.
Yes, this flag just skip the irq_set_wake() not this feature.
Regards,
-Dongsheng
From: Thomas Gleixner <hidden> Date: 2015-09-23 08:01:10
On Wed, 23 Sep 2015, Wang Dongsheng wrote:
quoted
-----Original Message-----
From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
owner@vger.kernel.org] On Behalf Of Thomas Gleixner
Sent: Wednesday, September 23, 2015 11:49 AM
To: Wang Dongsheng-B40534
Cc: Wood Scott-B07421; Sudeep Holla; linux-pm@vger.kernel.org; linux-
kernel@vger.kernel.org; Rafael J. Wysocki; Benjamin Herrenschmidt; Paul
Mackerras; Michael Ellerman; Jia Hongtao-B38951; Marc Zyngier; linuxppc-
dev@lists.ozlabs.org
Subject: RE: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE instead of
redundant mpic_irq_set_wake
Can you please fix you mail client to get rid of that silly copy of
the mail header?
quoted
On Wed, 23 Sep 2015, Wang Dongsheng wrote:
quoted
quoted
On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
quoted
mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets IRQF_NO_SUSPEND
flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
as it doesn't guarantee wakeup for that interrupt.
Non-freescale return -ENXIO, is there any issue? If non-freescale
platform does not support it, but IPs still use
enable/disable_irq_wake, we should return a error number.
You can just set IRQCHIP_SKIP_SET_WAKE for FSL chips and not for the
others.
quoted
@Scott:
If set this flag we cannot keep a irq as a wakeup source when system going to
SUSPEND or MEM.
irq_set_wake() means we can set this irq as a wake source.
IRQCHIP_SKIP_SET_WAKE is ignore irq_set_wake() feature.
Nonsense. IRQCHIP_SKIP_SET_WAKE merily tells the core not to bail on
!chip->irq_set_wake(), but its still marking the interrupt as wakeup
source and therefor not masking it on suspend.
Sorry, I just check irq_set_irq_wake() code, right, IRQCHIP_SKIP_SET_WAKE also can
going to irqd_set to mask IRQD_WAKEUP_STATE.
Yes, this flag just skip the irq_set_wake() not this feature.
And just for completeness. That commit 5ff04b7287d87c 'powerpc/mpic:
add irq_set_wake support' is another example of trainwreck engineering.
desc->action->flags |= IRQF_NO_SUSPEND;
This is not only horribly avoiding any of the existing APIs, it's also
broken as hell. desc->action can be NULL when that is called.
It seems fleascale is hell bent to fiddle with the guts of the core
code mindlessly. See commit c866cda47f2c
Yours grumpy
tglx
On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
quoted
mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets IRQF_NO_SUSPEND
flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
as it doesn't guarantee wakeup for that interrupt.
This patch removes the redundant mpic_irq_set_wake and sets the
IRQCHIP_SKIP_SET_WAKE for only FSL MPIC.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Scott Wood <redacted>
Cc: Hongtao Jia <redacted>
Cc: Marc Zyngier <redacted>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Sudeep Holla <redacted>
---
arch/powerpc/sysdev/mpic.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
Acked-by: Scott Wood <redacted>
Can you pick this up via your tree ?
--
Regards,
Sudeep
From: Scott Wood <hidden> Date: 2015-10-19 17:46:24
On Mon, 2015-10-19 at 18:35 +0100, Sudeep Holla wrote:
Hi Ben,
On 23/09/15 05:06, Scott Wood wrote:
quoted
On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
quoted
mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets
IRQF_NO_SUSPEND
flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
as it doesn't guarantee wakeup for that interrupt.
This patch removes the redundant mpic_irq_set_wake and sets the
IRQCHIP_SKIP_SET_WAKE for only FSL MPIC.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Scott Wood <redacted>
Cc: Hongtao Jia <redacted>
Cc: Marc Zyngier <redacted>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Sudeep Holla <redacted>
---
arch/powerpc/sysdev/mpic.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)