Message ID | 20210127085818.23742-1-biwen.li@oss.nxp.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] irqchip: ls-extirq: add flag IRQCHIP_SKIP_SET_WAKE to remove call trace | expand |
On 2021-01-27 08:58, Biwen Li wrote: > From: Biwen Li <biwen.li@nxp.com> > > Add flag IRQCHIP_SKIP_SET_WAKE to remove call trace as follow, > ... > [ 45.605239] Unbalanced IRQ 120 wake disable > [ 45.609445] WARNING: CPU: 0 PID: 1124 at kernel/irq/manage.c:800 > irq_set_irq_wake+0x154/0x1a0 > ... > [ 45.645141] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--) > [ 45.651144] pc : irq_set_irq_wake+0x154/0x1a0 > [ 45.655497] lr : irq_set_irq_wake+0x154/0x1a0 > ... > [ 45.742825] Call trace: > [ 45.745268] irq_set_irq_wake+0x154/0x1a0 > [ 45.749278] ds3232_resume+0x38/0x50 > > On ls2088ardb: > In suspend progress(# echo mem > /sys/power/state), > pm_suspend()->suspend_devices_and_enter()->dpm_suspend()->device_suspend() > ->ds3232_suspend()->enable_irq_wake()->irq_set_irq_wake() > ->set_irq_wake_real(), return -ENXIO, there get > "Cannot set wakeup source" in ds3232_suspend(). > > In resume progress(wakeup by flextimer) > dpm_resume_end()->dpm_resume() > ->device_resume()->ds3232_resume() > ->disable_irq_wake()->irq_set_irq_wake() > ->set_irq_wake_real(), there get > kernel call trace(Unbalanced IRQ 120 wake > disable) This is again paraphrasing the stack trace instead of explaining the problem it fixes. How about: "The ls-extirq driver doesn't implement the irq_set_wake() callback, while being wake-up capable. This results in ugly behaviours across suspend/resume cycles. Advertise this by adding IRQCHIP_SKIP_SET_WAKE to the irqchip flags" The subject line should be fixed along the same lines, and a Fixes: tag added. M.
> -----Original Message----- > From: Marc Zyngier <maz@kernel.org> > Sent: 2021年1月27日 19:38 > To: Biwen Li (OSS) <biwen.li@oss.nxp.com> > Cc: mark.rutland@arm.com; Leo Li <leoyang.li@nxp.com>; tglx@linutronix.de; > jason@lakedaemon.net; linux-kernel@vger.kernel.org; Jiafei Pan > <jiafei.pan@nxp.com>; linux-arm-kernel@lists.infradead.org; Ran Wang > <ran.wang_1@nxp.com>; Biwen Li <biwen.li@nxp.com> > Subject: Re: [v2] irqchip: ls-extirq: add flag IRQCHIP_SKIP_SET_WAKE to > remove call trace > > On 2021-01-27 08:58, Biwen Li wrote: > > From: Biwen Li <biwen.li@nxp.com> > > > > Add flag IRQCHIP_SKIP_SET_WAKE to remove call trace as follow, ... > > [ 45.605239] Unbalanced IRQ 120 wake disable > > [ 45.609445] WARNING: CPU: 0 PID: 1124 at kernel/irq/manage.c:800 > > irq_set_irq_wake+0x154/0x1a0 > > ... > > [ 45.645141] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--) > > [ 45.651144] pc : irq_set_irq_wake+0x154/0x1a0 > > [ 45.655497] lr : irq_set_irq_wake+0x154/0x1a0 > > ... > > [ 45.742825] Call trace: > > [ 45.745268] irq_set_irq_wake+0x154/0x1a0 > > [ 45.749278] ds3232_resume+0x38/0x50 > > > > On ls2088ardb: > > In suspend progress(# echo mem > /sys/power/state), > > > pm_suspend()->suspend_devices_and_enter()->dpm_suspend()->device_suspe > > nd() > > ->ds3232_suspend()->enable_irq_wake()->irq_set_irq_wake() > > ->set_irq_wake_real(), return -ENXIO, there get > > "Cannot set wakeup source" in ds3232_suspend(). > > > > In resume progress(wakeup by flextimer) > > dpm_resume_end()->dpm_resume() > > ->device_resume()->ds3232_resume() > > ->disable_irq_wake()->irq_set_irq_wake() > > ->set_irq_wake_real(), there get > > kernel call trace(Unbalanced IRQ 120 wake > > disable) > > This is again paraphrasing the stack trace instead of explaining the problem it > fixes. How about: > > "The ls-extirq driver doesn't implement the irq_set_wake() > callback, while being wake-up capable. This results in > ugly behaviours across suspend/resume cycles. > > Advertise this by adding IRQCHIP_SKIP_SET_WAKE to the irqchip > flags" > > The subject line should be fixed along the same lines, and a Fixes: tag added. Okay, got it. Thanks. Will update in v3. > > M. > -- > Jazz is not dead. It just smells funny...
On 2021-01-28 02:37, Biwen Li (OSS) wrote: >> -----Original Message----- >> From: Marc Zyngier <maz@kernel.org> >> Sent: 2021年1月27日 19:38 >> To: Biwen Li (OSS) <biwen.li@oss.nxp.com> >> Cc: mark.rutland@arm.com; Leo Li <leoyang.li@nxp.com>; >> tglx@linutronix.de; >> jason@lakedaemon.net; linux-kernel@vger.kernel.org; Jiafei Pan >> <jiafei.pan@nxp.com>; linux-arm-kernel@lists.infradead.org; Ran Wang >> <ran.wang_1@nxp.com>; Biwen Li <biwen.li@nxp.com> >> Subject: Re: [v2] irqchip: ls-extirq: add flag IRQCHIP_SKIP_SET_WAKE >> to >> remove call trace >> >> On 2021-01-27 08:58, Biwen Li wrote: >> > From: Biwen Li <biwen.li@nxp.com> >> > >> > Add flag IRQCHIP_SKIP_SET_WAKE to remove call trace as follow, ... >> > [ 45.605239] Unbalanced IRQ 120 wake disable >> > [ 45.609445] WARNING: CPU: 0 PID: 1124 at kernel/irq/manage.c:800 >> > irq_set_irq_wake+0x154/0x1a0 >> > ... >> > [ 45.645141] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--) >> > [ 45.651144] pc : irq_set_irq_wake+0x154/0x1a0 >> > [ 45.655497] lr : irq_set_irq_wake+0x154/0x1a0 >> > ... >> > [ 45.742825] Call trace: >> > [ 45.745268] irq_set_irq_wake+0x154/0x1a0 >> > [ 45.749278] ds3232_resume+0x38/0x50 >> > >> > On ls2088ardb: >> > In suspend progress(# echo mem > /sys/power/state), >> > >> pm_suspend()->suspend_devices_and_enter()->dpm_suspend()->device_suspe >> > nd() >> > ->ds3232_suspend()->enable_irq_wake()->irq_set_irq_wake() >> > ->set_irq_wake_real(), return -ENXIO, there get >> > "Cannot set wakeup source" in ds3232_suspend(). >> > >> > In resume progress(wakeup by flextimer) >> > dpm_resume_end()->dpm_resume() >> > ->device_resume()->ds3232_resume() >> > ->disable_irq_wake()->irq_set_irq_wake() >> > ->set_irq_wake_real(), there get >> > kernel call trace(Unbalanced IRQ 120 wake >> > disable) >> >> This is again paraphrasing the stack trace instead of explaining the >> problem it >> fixes. How about: >> >> "The ls-extirq driver doesn't implement the irq_set_wake() >> callback, while being wake-up capable. This results in >> ugly behaviours across suspend/resume cycles. >> >> Advertise this by adding IRQCHIP_SKIP_SET_WAKE to the irqchip >> flags" >> >> The subject line should be fixed along the same lines, and a Fixes: >> tag added. > Okay, got it. Thanks. Will update in v3. ... and v3 still doesn't have a Fixes: tag. Frankly, if you can't be bothered to do this, why should I worry about your platform being broken? M.
> -----Original Message----- > From: Marc Zyngier <maz@kernel.org> > Sent: 2021年1月28日 17:02 > To: Biwen Li (OSS) <biwen.li@oss.nxp.com> > Cc: mark.rutland@arm.com; Leo Li <leoyang.li@nxp.com>; tglx@linutronix.de; > linux-kernel@vger.kernel.org; Jiafei Pan <jiafei.pan@nxp.com>; > linux-arm-kernel@lists.infradead.org; Ran Wang <ran.wang_1@nxp.com> > Subject: Re: [v2] irqchip: ls-extirq: add flag IRQCHIP_SKIP_SET_WAKE to > remove call trace > > On 2021-01-28 02:37, Biwen Li (OSS) wrote: > >> -----Original Message----- > >> From: Marc Zyngier <maz@kernel.org> > >> Sent: 2021年1月27日 19:38 > >> To: Biwen Li (OSS) <biwen.li@oss.nxp.com> > >> Cc: mark.rutland@arm.com; Leo Li <leoyang.li@nxp.com>; > >> tglx@linutronix.de; jason@lakedaemon.net; > >> linux-kernel@vger.kernel.org; Jiafei Pan <jiafei.pan@nxp.com>; > >> linux-arm-kernel@lists.infradead.org; Ran Wang <ran.wang_1@nxp.com>; > >> Biwen Li <biwen.li@nxp.com> > >> Subject: Re: [v2] irqchip: ls-extirq: add flag IRQCHIP_SKIP_SET_WAKE > >> to remove call trace > >> > >> On 2021-01-27 08:58, Biwen Li wrote: > >> > From: Biwen Li <biwen.li@nxp.com> > >> > > >> > Add flag IRQCHIP_SKIP_SET_WAKE to remove call trace as follow, ... > >> > [ 45.605239] Unbalanced IRQ 120 wake disable > >> > [ 45.609445] WARNING: CPU: 0 PID: 1124 at kernel/irq/manage.c:800 > >> > irq_set_irq_wake+0x154/0x1a0 > >> > ... > >> > [ 45.645141] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--) > >> > [ 45.651144] pc : irq_set_irq_wake+0x154/0x1a0 > >> > [ 45.655497] lr : irq_set_irq_wake+0x154/0x1a0 > >> > ... > >> > [ 45.742825] Call trace: > >> > [ 45.745268] irq_set_irq_wake+0x154/0x1a0 > >> > [ 45.749278] ds3232_resume+0x38/0x50 > >> > > >> > On ls2088ardb: > >> > In suspend progress(# echo mem > /sys/power/state), > >> > > >> > pm_suspend()->suspend_devices_and_enter()->dpm_suspend()->device_susp > >> e > >> > nd() > >> > ->ds3232_suspend()->enable_irq_wake()->irq_set_irq_wake() > >> > ->set_irq_wake_real(), return -ENXIO, there get > >> > "Cannot set wakeup source" in ds3232_suspend(). > >> > > >> > In resume progress(wakeup by flextimer) > >> > dpm_resume_end()->dpm_resume() > >> > ->device_resume()->ds3232_resume() > >> > ->disable_irq_wake()->irq_set_irq_wake() > >> > ->set_irq_wake_real(), there get > >> > kernel call trace(Unbalanced IRQ 120 wake > >> > disable) > >> > >> This is again paraphrasing the stack trace instead of explaining the > >> problem it fixes. How about: > >> > >> "The ls-extirq driver doesn't implement the irq_set_wake() > >> callback, while being wake-up capable. This results in > >> ugly behaviours across suspend/resume cycles. > >> > >> Advertise this by adding IRQCHIP_SKIP_SET_WAKE to the irqchip > >> flags" > >> > >> The subject line should be fixed along the same lines, and a Fixes: > >> tag added. > > Okay, got it. Thanks. Will update in v3. > > ... and v3 still doesn't have a Fixes: tag. > > Frankly, if you can't be bothered to do this, why should I worry about your > platform being broken? Oh, sorry. Don't know how to add a fixes? Any suggestions? Thanks. > > M. > -- > Jazz is not dead. It just smells funny...
On 2021-01-29 02:55, Biwen Li (OSS) wrote: >> -----Original Message----- >> From: Marc Zyngier <maz@kernel.org> >> Sent: 2021年1月28日 17:02 >> To: Biwen Li (OSS) <biwen.li@oss.nxp.com> >> Cc: mark.rutland@arm.com; Leo Li <leoyang.li@nxp.com>; >> tglx@linutronix.de; >> linux-kernel@vger.kernel.org; Jiafei Pan <jiafei.pan@nxp.com>; >> linux-arm-kernel@lists.infradead.org; Ran Wang <ran.wang_1@nxp.com> >> Subject: Re: [v2] irqchip: ls-extirq: add flag IRQCHIP_SKIP_SET_WAKE >> to >> remove call trace >> >> On 2021-01-28 02:37, Biwen Li (OSS) wrote: >> >> -----Original Message----- >> >> From: Marc Zyngier <maz@kernel.org> >> >> Sent: 2021年1月27日 19:38 >> >> To: Biwen Li (OSS) <biwen.li@oss.nxp.com> >> >> Cc: mark.rutland@arm.com; Leo Li <leoyang.li@nxp.com>; >> >> tglx@linutronix.de; jason@lakedaemon.net; >> >> linux-kernel@vger.kernel.org; Jiafei Pan <jiafei.pan@nxp.com>; >> >> linux-arm-kernel@lists.infradead.org; Ran Wang <ran.wang_1@nxp.com>; >> >> Biwen Li <biwen.li@nxp.com> >> >> Subject: Re: [v2] irqchip: ls-extirq: add flag IRQCHIP_SKIP_SET_WAKE >> >> to remove call trace >> >> >> >> On 2021-01-27 08:58, Biwen Li wrote: >> >> > From: Biwen Li <biwen.li@nxp.com> >> >> > >> >> > Add flag IRQCHIP_SKIP_SET_WAKE to remove call trace as follow, ... >> >> > [ 45.605239] Unbalanced IRQ 120 wake disable >> >> > [ 45.609445] WARNING: CPU: 0 PID: 1124 at kernel/irq/manage.c:800 >> >> > irq_set_irq_wake+0x154/0x1a0 >> >> > ... >> >> > [ 45.645141] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--) >> >> > [ 45.651144] pc : irq_set_irq_wake+0x154/0x1a0 >> >> > [ 45.655497] lr : irq_set_irq_wake+0x154/0x1a0 >> >> > ... >> >> > [ 45.742825] Call trace: >> >> > [ 45.745268] irq_set_irq_wake+0x154/0x1a0 >> >> > [ 45.749278] ds3232_resume+0x38/0x50 >> >> > >> >> > On ls2088ardb: >> >> > In suspend progress(# echo mem > /sys/power/state), >> >> > >> >> >> pm_suspend()->suspend_devices_and_enter()->dpm_suspend()->device_susp >> >> e >> >> > nd() >> >> > ->ds3232_suspend()->enable_irq_wake()->irq_set_irq_wake() >> >> > ->set_irq_wake_real(), return -ENXIO, there get >> >> > "Cannot set wakeup source" in ds3232_suspend(). >> >> > >> >> > In resume progress(wakeup by flextimer) >> >> > dpm_resume_end()->dpm_resume() >> >> > ->device_resume()->ds3232_resume() >> >> > ->disable_irq_wake()->irq_set_irq_wake() >> >> > ->set_irq_wake_real(), there get >> >> > kernel call trace(Unbalanced IRQ 120 wake >> >> > disable) >> >> >> >> This is again paraphrasing the stack trace instead of explaining the >> >> problem it fixes. How about: >> >> >> >> "The ls-extirq driver doesn't implement the irq_set_wake() >> >> callback, while being wake-up capable. This results in >> >> ugly behaviours across suspend/resume cycles. >> >> >> >> Advertise this by adding IRQCHIP_SKIP_SET_WAKE to the irqchip >> >> flags" >> >> >> >> The subject line should be fixed along the same lines, and a Fixes: >> >> tag added. >> > Okay, got it. Thanks. Will update in v3. >> >> ... and v3 still doesn't have a Fixes: tag. >> >> Frankly, if you can't be bothered to do this, why should I worry about >> your >> platform being broken? > Oh, sorry. Don't know how to add a fixes? Any suggestions? Thanks. Please read Documentation/process/submitting-patches.rst, and specially the section titled "Describe your changes". In your cases, there are only two commits for this driver, so picking the one that had a bug shouldn't be too hard. Thanks, M.
> -----Original Message----- > From: Marc Zyngier <maz@kernel.org> > Sent: 2021年1月29日 16:55 > To: Biwen Li (OSS) <biwen.li@oss.nxp.com> > Cc: mark.rutland@arm.com; Leo Li <leoyang.li@nxp.com>; tglx@linutronix.de; > linux-kernel@vger.kernel.org; Jiafei Pan <jiafei.pan@nxp.com>; > linux-arm-kernel@lists.infradead.org; Ran Wang <ran.wang_1@nxp.com> > Subject: Re: [v2] irqchip: ls-extirq: add flag IRQCHIP_SKIP_SET_WAKE to > remove call trace > > On 2021-01-29 02:55, Biwen Li (OSS) wrote: > >> -----Original Message----- > >> From: Marc Zyngier <maz@kernel.org> > >> Sent: 2021年1月28日 17:02 > >> To: Biwen Li (OSS) <biwen.li@oss.nxp.com> > >> Cc: mark.rutland@arm.com; Leo Li <leoyang.li@nxp.com>; > >> tglx@linutronix.de; linux-kernel@vger.kernel.org; Jiafei Pan > >> <jiafei.pan@nxp.com>; linux-arm-kernel@lists.infradead.org; Ran Wang > >> <ran.wang_1@nxp.com> > >> Subject: Re: [v2] irqchip: ls-extirq: add flag IRQCHIP_SKIP_SET_WAKE > >> to remove call trace > >> > >> On 2021-01-28 02:37, Biwen Li (OSS) wrote: > >> >> -----Original Message----- > >> >> From: Marc Zyngier <maz@kernel.org> > >> >> Sent: 2021年1月27日 19:38 > >> >> To: Biwen Li (OSS) <biwen.li@oss.nxp.com> > >> >> Cc: mark.rutland@arm.com; Leo Li <leoyang.li@nxp.com>; > >> >> tglx@linutronix.de; jason@lakedaemon.net; > >> >> linux-kernel@vger.kernel.org; Jiafei Pan <jiafei.pan@nxp.com>; > >> >> linux-arm-kernel@lists.infradead.org; Ran Wang > >> >> <ran.wang_1@nxp.com>; Biwen Li <biwen.li@nxp.com> > >> >> Subject: Re: [v2] irqchip: ls-extirq: add flag > >> >> IRQCHIP_SKIP_SET_WAKE to remove call trace > >> >> > >> >> On 2021-01-27 08:58, Biwen Li wrote: > >> >> > From: Biwen Li <biwen.li@nxp.com> > >> >> > > >> >> > Add flag IRQCHIP_SKIP_SET_WAKE to remove call trace as follow, ... > >> >> > [ 45.605239] Unbalanced IRQ 120 wake disable > >> >> > [ 45.609445] WARNING: CPU: 0 PID: 1124 at > kernel/irq/manage.c:800 > >> >> > irq_set_irq_wake+0x154/0x1a0 > >> >> > ... > >> >> > [ 45.645141] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--) > >> >> > [ 45.651144] pc : irq_set_irq_wake+0x154/0x1a0 > >> >> > [ 45.655497] lr : irq_set_irq_wake+0x154/0x1a0 > >> >> > ... > >> >> > [ 45.742825] Call trace: > >> >> > [ 45.745268] irq_set_irq_wake+0x154/0x1a0 > >> >> > [ 45.749278] ds3232_resume+0x38/0x50 > >> >> > > >> >> > On ls2088ardb: > >> >> > In suspend progress(# echo mem > /sys/power/state), > >> >> > > >> >> > >> > pm_suspend()->suspend_devices_and_enter()->dpm_suspend()->device_susp > >> >> e > >> >> > nd() > >> >> > ->ds3232_suspend()->enable_irq_wake()->irq_set_irq_wake() > >> >> > ->set_irq_wake_real(), return -ENXIO, there get > >> >> > "Cannot set wakeup source" in ds3232_suspend(). > >> >> > > >> >> > In resume progress(wakeup by flextimer) > >> >> > dpm_resume_end()->dpm_resume() > >> >> > ->device_resume()->ds3232_resume() > >> >> > ->disable_irq_wake()->irq_set_irq_wake() > >> >> > ->set_irq_wake_real(), there get > >> >> > kernel call trace(Unbalanced IRQ 120 wake > >> >> > disable) > >> >> > >> >> This is again paraphrasing the stack trace instead of explaining > >> >> the problem it fixes. How about: > >> >> > >> >> "The ls-extirq driver doesn't implement the irq_set_wake() > >> >> callback, while being wake-up capable. This results in > >> >> ugly behaviours across suspend/resume cycles. > >> >> > >> >> Advertise this by adding IRQCHIP_SKIP_SET_WAKE to the irqchip > >> >> flags" > >> >> > >> >> The subject line should be fixed along the same lines, and a Fixes: > >> >> tag added. > >> > Okay, got it. Thanks. Will update in v3. > >> > >> ... and v3 still doesn't have a Fixes: tag. > >> > >> Frankly, if you can't be bothered to do this, why should I worry > >> about your platform being broken? > > Oh, sorry. Don't know how to add a fixes? Any suggestions? Thanks. > > Please read Documentation/process/submitting-patches.rst, and specially the > section titled "Describe your changes". > > In your cases, there are only two commits for this driver, so picking the one > that had a bug shouldn't be too hard. Okay, got it. Thanks. > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
diff --git a/drivers/irqchip/irq-ls-extirq.c b/drivers/irqchip/irq-ls-extirq.c index 564e6de0bd8e..3c6ed7b4744d 100644 --- a/drivers/irqchip/irq-ls-extirq.c +++ b/drivers/irqchip/irq-ls-extirq.c @@ -65,7 +65,7 @@ static struct irq_chip ls_extirq_chip = { .irq_set_type = ls_extirq_set_type, .irq_retrigger = irq_chip_retrigger_hierarchy, .irq_set_affinity = irq_chip_set_affinity_parent, - .flags = IRQCHIP_SET_TYPE_MASKED, + .flags = IRQCHIP_SET_TYPE_MASKED | IRQCHIP_SKIP_SET_WAKE, }; static int