From: haidong yao <yaohaidong369@gmail.com>
To: jassisinghbrar@gmail.com, natechancellor@gmail.com,
ndesaulniers@google.com, linux-kernel@vger.kernel.org,
clang-built-linux@googlegroups.com, orsonzhai@gmail.com
Cc: zhang.lyra@gmail.com
Subject: Re: [PATCH] add chan->cl check in mbox_chan_received_data()
Date: Thu, 7 Jan 2021 19:53:52 +0800 [thread overview]
Message-ID: <CA+kUaCeebDNXi12-nNpw4i_auZbM+JQUC5uNhEzXxqa7=W4LnQ@mail.gmail.com> (raw)
In-Reply-To: <1608022118-29993-1-git-send-email-yaohaidong369@gmail.com>
Hi Jassi Brar
Thank you very much for your reply.
Look at the function sprd_mbox_outbox_isr .
Chan is !NULL.
chan->cl is NULL when the client driver not loaded, the controller
driver don't know the client driver loaded successfully, so, I do not
use mbox_free_channel.
Here,How do you know chan->cl is ok?
chan = &priv->chan[id];
mbox_chan_received_data(chan, (void *)msg);
static irqreturn_t sprd_mbox_outbox_isr(int irq, void *data)
{
struct sprd_mbox_priv *priv = data;
struct mbox_chan *chan;
u32 fifo_sts, fifo_len, msg[2];
int i, id;
fifo_sts = readl(priv->outbox_base + SPRD_MBOX_FIFO_STS);
fifo_len = sprd_mbox_get_fifo_len(priv, fifo_sts);
if (!fifo_len) {
dev_warn_ratelimited(priv->dev, "spurious outbox interrupt\n");
return IRQ_NONE;
}
for (i = 0; i < fifo_len; i++) {
msg[0] = readl(priv->outbox_base + SPRD_MBOX_MSG_LOW);
msg[1] = readl(priv->outbox_base + SPRD_MBOX_MSG_HIGH);
id = readl(priv->outbox_base + SPRD_MBOX_ID);
chan = &priv->chan[id];
mbox_chan_received_data(chan, (void *)msg);
/* Trigger to update outbox FIFO pointer */
writel(0x1, priv->outbox_base + SPRD_MBOX_TRIGGER);
}
/* Clear irq status after reading all message. */
writel(SPRD_MBOX_IRQ_CLR, priv->outbox_base + SPRD_MBOX_IRQ_STS);
return IRQ_HANDLED;
}
On Tue, Dec 15, 2020 at 2:48 AM Haidong Yao <yaohaidong369@gmail.com> wrote:
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -152,7 +152,7 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer)
> void mbox_chan_received_data(struct mbox_chan *chan, void *mssg)
> {
> /* No buffering the received data */
> - if (chan->cl->rx_callback)
> + if (chan->cl && chan->cl->rx_callback)
> chan->cl->rx_callback(chan->cl, mssg);
> }
The proper fix is in the controller driver. Which should stop tx/rx
when the channel is freed.
thnx.
2020-12-15 16:48 GMT+08:00, Haidong Yao <yaohaidong369@gmail.com>:
> From: Haidong Yao <haidong.yao@unisoc.com>
>
> mailbox outbox irq is coming, but mbox_request_channel
> is not be registered, so cl->rx_callback is NULL.
>
> panic log:
> [ 9.852090]c0 Unable to handle kernel NULL pointer dereference at
> virtual address 0000000000000020
> [ 9.954634]c0 pstate: 60400089 (nZCv daIf +PAN -UAO)
> [ 9.954651]c0 pc : mbox_chan_received_data+0x1c/0x88
> [ 9.954666]c0 lr : sprd_mbox_outbox_isr+0x1d0/0x204 [sprd_mailbox]
> [ 9.967439]c0 sp : ffffffc010003e10
> [ 9.967443]c0 x29: ffffffc010003e20 x28: ffffffc011c2f6c0-
> [ 9.984918]c0 x27: ffffffc010e92e08 x26: 0000000000000001-
> [ 10.140344]c0 x25: 0000000000000378 x24: ffffff80f4064130-
> [ 10.145880]c0 x23: 0000000000000001 x22: ffffffc0091072c7-
> [ 10.151418]c0 x21: ffffffc009107212 x20: 0000000000000005-
> [ 10.156957]c0 x19: ffffff80f4064080 x18: ffffffc010005038-
> [ 10.162494]c0 x17: 0000000000000000 x16: ffffffc010e6f844-
> [ 10.168033]c0 x15: ffffffc0117abac7 x14: 000000000000003f-
> [ 10.173571]c0 x13: ffff0000ffffff00 x12: ffff0a01ffffff10-
> [ 10.179110]c0 x11: 0000000000000001 x10: 00000000ffffffff-
> [ 10.184649]c0 x9 : ffffff80f40644a8 x8 : c366877097809900-
> [ 10.190187]c0 x7 : 207273695f786f62 x6 : ffffffc011d62231-
> [ 10.195726]c0 x5 : 0000000000000034 x4 : 000000000000000c-
> [ 10.201265]c0 x3 : ffffffc010e9842c x2 : 0000000000000001-
> [ 10.206803]c0 x1 : ffffffc010003e40 x0 : 0000000000000000-
> [ 10.212343]c0 Call trace:
> [ 10.215029]c0 mbox_chan_received_data+0x1c/0x88
> [ 10.219705]c0 sprd_mbox_outbox_isr+0x1d0/0x204 [sprd_mailbox]
> [ 10.225590]c0 __handle_irq_event_percpu+0x164/0x358
> [ 10.230604]c0 handle_irq_event+0x60/0xd8
> [ 10.234675]c0 handle_fasteoi_irq+0x128/0x32c
> [ 10.239086]c0 __handle_domain_irq+0xa0/0x100
> [ 10.243502]c0 efi_header_end+0xb8/0x15c
> [ 10.247478]c0 el1_irq+0x104/0x200
> [ 10.250945]c0 cpuidle_enter_state+0x158/0x2d8
> [ 10.255440]c0 cpuidle_enter+0x38/0x50
> [ 10.259253]c0 do_idle.llvm.10091284334483161164+0x1a4/0x294
> [ 10.264963]c0 cpu_startup_entry+0x24/0x28
> [ 10.269120]c0 kernel_init+0x0/0x29c
> [ 10.272752]c0 start_kernel+0x0/0x418
> [ 10.276468]c0 start_kernel+0x3a0/0x418
> [ 10.280371]c0 Code: f90013f3 910043fd aa0003e9 f9400800 (f9401008)-
> [ 10.286684]c0 ---[ end trace b868997a960c667a ]---
>
> Signed-off-by: Haidong Yao <haidong.yao@unisoc.com>
> ---
> drivers/mailbox/mailbox.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index 3e7d4b20ab34..58697298a95f 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -152,7 +152,7 @@ static enum hrtimer_restart txdone_hrtimer(struct
> hrtimer *hrtimer)
> void mbox_chan_received_data(struct mbox_chan *chan, void *mssg)
> {
> /* No buffering the received data */
> - if (chan->cl->rx_callback)
> + if (chan->cl && chan->cl->rx_callback)
> chan->cl->rx_callback(chan->cl, mssg);
> }
> EXPORT_SYMBOL_GPL(mbox_chan_received_data);
> --
> 2.28.0
>
>
next prev parent reply other threads:[~2021-01-07 11:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-15 8:48 [PATCH] add chan->cl check in mbox_chan_received_data() Haidong Yao
2020-12-15 21:20 ` Jassi Brar
2020-12-16 0:52 ` Orson Zhai
2021-01-07 11:53 ` haidong yao [this message]
2021-02-01 6:28 ` Jassi Brar
2021-02-04 7:22 ` Yao Haidong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CA+kUaCeebDNXi12-nNpw4i_auZbM+JQUC5uNhEzXxqa7=W4LnQ@mail.gmail.com' \
--to=yaohaidong369@gmail.com \
--cc=clang-built-linux@googlegroups.com \
--cc=jassisinghbrar@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=natechancellor@gmail.com \
--cc=ndesaulniers@google.com \
--cc=orsonzhai@gmail.com \
--cc=zhang.lyra@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).