* [PATCH] mailbox: imx: Fix Tx doorbell shutdown path
@ 2019-07-31 20:55 Daniel Baluta
2019-08-01 6:10 ` Oleksij Rempel
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Baluta @ 2019-07-31 20:55 UTC (permalink / raw)
To: shawnguo, jassisinghbrar
Cc: s.hauer, kernel, festevam, linux-imx, linux-kernel,
linux-arm-kernel, o.rempel, Daniel Baluta
Tx doorbell is handled by txdb_tasklet and doesn't
have an associated IRQ.
Anyhow, imx_mu_shutdown ignores this and tries to
free an IRQ that wasn't requested for Tx DB resulting
in the following warning:
[ 1.967644] Trying to free already-free IRQ 26
[ 1.972108] WARNING: CPU: 2 PID: 157 at kernel/irq/manage.c:1708 __free_irq+0xc0/0x358
[ 1.980024] Modules linked in:
[ 1.983088] CPU: 2 PID: 157 Comm: kworker/2:1 Tainted: G
[ 1.993524] Hardware name: Freescale i.MX8QXP MEK (DT)
[ 1.998668] Workqueue: events deferred_probe_work_func
[ 2.003812] pstate: 60000085 (nZCv daIf -PAN -UAO)
[ 2.008607] pc : __free_irq+0xc0/0x358
[ 2.012364] lr : __free_irq+0xc0/0x358
[ 2.016111] sp : ffff00001179b7e0
[ 2.019422] x29: ffff00001179b7e0 x28: 0000000000000018
[ 2.024736] x27: ffff000011233000 x26: 0000000000000004
[ 2.030053] x25: 000000000000001a x24: ffff80083bec74d4
[ 2.035369] x23: 0000000000000000 x22: ffff80083bec7588
[ 2.040686] x21: ffff80083b1fe8d8 x20: ffff80083bec7400
[ 2.046003] x19: 0000000000000000 x18: ffffffffffffffff
[ 2.051320] x17: 0000000000000000 x16: 0000000000000000
[ 2.056637] x15: ffff0000111296c8 x14: ffff00009179b517
[ 2.061953] x13: ffff00001179b525 x12: ffff000011142000
[ 2.067270] x11: ffff000011129f20 x10: ffff0000105da970
[ 2.072587] x9 : 00000000ffffffd0 x8 : 0000000000000194
[ 2.077903] x7 : 612065657266206f x6 : ffff0000111e7b09
[ 2.083220] x5 : 0000000000000003 x4 : 0000000000000000
[ 2.088537] x3 : 0000000000000000 x2 : 00000000ffffffff
[ 2.093854] x1 : 28b70f0a2b60a500 x0 : 0000000000000000
[ 2.099173] Call trace:
[ 2.101618] __free_irq+0xc0/0x358
[ 2.105021] free_irq+0x38/0x98
[ 2.108170] imx_mu_shutdown+0x90/0xb0
[ 2.111921] mbox_free_channel.part.2+0x24/0xb8
[ 2.116453] mbox_free_channel+0x18/0x28
This bug is present from the beginning of times.
Cc: Oleksij Rempel <o.rempel@pengutronix.de>
Fixes: 2bb7005696e2246 ("mailbox: Add support for i.MX messaging unit")
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
drivers/mailbox/imx-mailbox.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 9f74dee1a58c..957c10c4e674 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -214,8 +214,10 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
struct imx_mu_con_priv *cp = chan->con_priv;
- if (cp->type == IMX_MU_TYPE_TXDB)
+ if (cp->type == IMX_MU_TYPE_TXDB) {
tasklet_kill(&cp->txdb_tasklet);
+ return;
+ }
imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_TIEn(cp->idx) |
IMX_MU_xCR_RIEn(cp->idx) | IMX_MU_xCR_GIEn(cp->idx));
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mailbox: imx: Fix Tx doorbell shutdown path
2019-07-31 20:55 [PATCH] mailbox: imx: Fix Tx doorbell shutdown path Daniel Baluta
@ 2019-08-01 6:10 ` Oleksij Rempel
2019-08-01 14:53 ` Daniel Baluta
0 siblings, 1 reply; 3+ messages in thread
From: Oleksij Rempel @ 2019-08-01 6:10 UTC (permalink / raw)
To: Daniel Baluta
Cc: shawnguo, jassisinghbrar, s.hauer, linux-kernel, linux-imx,
kernel, festevam, linux-arm-kernel, hongxing.zhu, aisheng.dong
Hi Daniel,
your patch is in conflicht with Richard's Zhu <hongxing.zhu@nxp.com>
patch "[PATCH v3] mailbox: imx: add support for imx v1 mu".
Please sync your works.
On Wed, Jul 31, 2019 at 11:55:39PM +0300, Daniel Baluta wrote:
> Tx doorbell is handled by txdb_tasklet and doesn't
> have an associated IRQ.
>
> Anyhow, imx_mu_shutdown ignores this and tries to
> free an IRQ that wasn't requested for Tx DB resulting
> in the following warning:
>
> [ 1.967644] Trying to free already-free IRQ 26
> [ 1.972108] WARNING: CPU: 2 PID: 157 at kernel/irq/manage.c:1708 __free_irq+0xc0/0x358
> [ 1.980024] Modules linked in:
> [ 1.983088] CPU: 2 PID: 157 Comm: kworker/2:1 Tainted: G
> [ 1.993524] Hardware name: Freescale i.MX8QXP MEK (DT)
> [ 1.998668] Workqueue: events deferred_probe_work_func
> [ 2.003812] pstate: 60000085 (nZCv daIf -PAN -UAO)
> [ 2.008607] pc : __free_irq+0xc0/0x358
> [ 2.012364] lr : __free_irq+0xc0/0x358
> [ 2.016111] sp : ffff00001179b7e0
> [ 2.019422] x29: ffff00001179b7e0 x28: 0000000000000018
> [ 2.024736] x27: ffff000011233000 x26: 0000000000000004
> [ 2.030053] x25: 000000000000001a x24: ffff80083bec74d4
> [ 2.035369] x23: 0000000000000000 x22: ffff80083bec7588
> [ 2.040686] x21: ffff80083b1fe8d8 x20: ffff80083bec7400
> [ 2.046003] x19: 0000000000000000 x18: ffffffffffffffff
> [ 2.051320] x17: 0000000000000000 x16: 0000000000000000
> [ 2.056637] x15: ffff0000111296c8 x14: ffff00009179b517
> [ 2.061953] x13: ffff00001179b525 x12: ffff000011142000
> [ 2.067270] x11: ffff000011129f20 x10: ffff0000105da970
> [ 2.072587] x9 : 00000000ffffffd0 x8 : 0000000000000194
> [ 2.077903] x7 : 612065657266206f x6 : ffff0000111e7b09
> [ 2.083220] x5 : 0000000000000003 x4 : 0000000000000000
> [ 2.088537] x3 : 0000000000000000 x2 : 00000000ffffffff
> [ 2.093854] x1 : 28b70f0a2b60a500 x0 : 0000000000000000
> [ 2.099173] Call trace:
> [ 2.101618] __free_irq+0xc0/0x358
> [ 2.105021] free_irq+0x38/0x98
> [ 2.108170] imx_mu_shutdown+0x90/0xb0
> [ 2.111921] mbox_free_channel.part.2+0x24/0xb8
> [ 2.116453] mbox_free_channel+0x18/0x28
>
> This bug is present from the beginning of times.
>
> Cc: Oleksij Rempel <o.rempel@pengutronix.de>
> Fixes: 2bb7005696e2246 ("mailbox: Add support for i.MX messaging unit")
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
> drivers/mailbox/imx-mailbox.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> index 9f74dee1a58c..957c10c4e674 100644
> --- a/drivers/mailbox/imx-mailbox.c
> +++ b/drivers/mailbox/imx-mailbox.c
> @@ -214,8 +214,10 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
> struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> struct imx_mu_con_priv *cp = chan->con_priv;
>
> - if (cp->type == IMX_MU_TYPE_TXDB)
> + if (cp->type == IMX_MU_TYPE_TXDB) {
> tasklet_kill(&cp->txdb_tasklet);
> + return;
> + }
>
> imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_TIEn(cp->idx) |
> IMX_MU_xCR_RIEn(cp->idx) | IMX_MU_xCR_GIEn(cp->idx));
Looks like here is one more bug "from the beginning of times" :)
The imx_mu_xcr_rmw() should disable only one channel depending on the
type of channel.
It should be:
switch (cp->type) {
case IMX_MU_TYPE_TX:
imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_TIEn(cp->idx));
break;
case IMX_MU_TYPE_RX:
imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_RIEn(cp->idx));
break;
case IMX_MU_TYPE_RXDB:
imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_GIEn(cp->idx));
break;
default:
break;
}
Right now I'm on vocation and have nothing against, if you'll provide
this fix.
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mailbox: imx: Fix Tx doorbell shutdown path
2019-08-01 6:10 ` Oleksij Rempel
@ 2019-08-01 14:53 ` Daniel Baluta
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Baluta @ 2019-08-01 14:53 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Daniel Baluta, Shawn Guo, jassisinghbrar, Sascha Hauer,
Linux Kernel Mailing List, dl-linux-imx, Pengutronix Kernel Team,
Fabio Estevam, linux-arm-kernel, Richard Zhu, Aisheng Dong
Hi Oleksij,
Thanks for review
<snip>
>
> your patch is in conflicht with Richard's Zhu <hongxing.zhu@nxp.com>
> patch "[PATCH v3] mailbox: imx: add support for imx v1 mu".
> Please sync your works.
Sent an email to Richard. Hopefully he can rebase his change on my patches.
<snip>
>
> Looks like here is one more bug "from the beginning of times" :)
> The imx_mu_xcr_rmw() should disable only one channel depending on the
> type of channel.
>
> It should be:
> switch (cp->type) {
> case IMX_MU_TYPE_TX:
> imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_TIEn(cp->idx));
> break;
> case IMX_MU_TYPE_RX:
> imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_RIEn(cp->idx));
> break;
> case IMX_MU_TYPE_RXDB:
> imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_GIEn(cp->idx));
> break;
> default:
> break;
Honestly, this is not really a bug. The registers are expected to
already be cleared.
Also, please mind that we shouldn't clear TIE here because it should be cleared
in the mu_isr.
Anyhow, I have sent the patches with your exact suggestion to Richard.
> }
>
> Right now I'm on vocation and have nothing against, if you'll provide
> this fix.
Have a nice vacation and thanks for the review.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-01 14:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-31 20:55 [PATCH] mailbox: imx: Fix Tx doorbell shutdown path Daniel Baluta
2019-08-01 6:10 ` Oleksij Rempel
2019-08-01 14:53 ` Daniel Baluta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).