From: Jassi Brar <jassisinghbrar@gmail.com>
To: HS Liao <hs.liao@mediatek.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
Daniel Kurtz <djkurtz@chromium.org>,
Sascha Hauer <s.hauer@pengutronix.de>,
Devicetree List <devicetree@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
linux-mediatek@lists.infradead.org, srv_heupstream@mediatek.com,
Sascha Hauer <kernel@pengutronix.de>,
Philipp Zabel <p.zabel@pengutronix.de>,
Nicolas Boichat <drinkcat@chromium.org>,
CK HU <ck.hu@mediatek.com>, cawa cheng <cawa.cheng@mediatek.com>,
Bibby Hsieh <bibby.hsieh@mediatek.com>,
YT Shen <yt.shen@mediatek.com>,
Daoyuan Huang <daoyuan.huang@mediatek.com>,
Damon Chu <damon.chu@mediatek.com>,
Josh-YC Liu <josh-yc.liu@mediatek.com>,
Glory Hung <glory.hung@mediatek.com>,
Jiaguang Zhang <jiaguang.zhang@mediatek.com>,
Dennis-YC Hsieh <dennis-yc.hsieh@mediatek.com>,
Monica Wang <monica.wang@mediatek.com>,
Houlong Wei <houlong.wei@mediatek.com>
Subject: Re: [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
Date: Thu, 26 Jan 2017 10:08:21 +0530 [thread overview]
Message-ID: <CABb+yY23TUB8NpmpiO=cCmVFAVJMqKPfGgX14j+_Yj7tJvMOuQ@mail.gmail.com> (raw)
In-Reply-To: <1483499169-16329-3-git-send-email-hs.liao@mediatek.com>
On Wed, Jan 4, 2017 at 8:36 AM, HS Liao <hs.liao@mediatek.com> wrote:
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> new file mode 100644
> index 0000000..747bcd3
> --- /dev/null
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
...
> +static void cmdq_task_exec(struct cmdq_pkt *pkt, struct cmdq_thread *thread)
> +{
> + struct cmdq *cmdq;
> + struct cmdq_task *task;
> + unsigned long curr_pa, end_pa;
> +
> + cmdq = dev_get_drvdata(thread->chan->mbox->dev);
> +
> + /* Client should not flush new tasks if suspended. */
> + WARN_ON(cmdq->suspended);
> +
> + task = kzalloc(sizeof(*task), GFP_ATOMIC);
> + task->cmdq = cmdq;
> + INIT_LIST_HEAD(&task->list_entry);
> + task->pa_base = dma_map_single(cmdq->mbox.dev, pkt->va_base,
> + pkt->cmd_buf_size, DMA_TO_DEVICE);
>
You seem to parse the requests and responses, that should ideally be
done in client driver.
Also, we are here in atomic context, can you move it in client driver
(before the spin_lock)?
Maybe by adding a new 'pa_base' member as well in 'cmdq_pkt'.
....
> +
> + cmdq->mbox.num_chans = CMDQ_THR_MAX_COUNT;
> + cmdq->mbox.ops = &cmdq_mbox_chan_ops;
> + cmdq->mbox.of_xlate = cmdq_xlate;
> +
> + /* make use of TXDONE_BY_ACK */
> + cmdq->mbox.txdone_irq = false;
> + cmdq->mbox.txdone_poll = false;
> +
> + for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
>
You mean i < CMDQ_THR_MAX_COUNT
> + cmdq->thread[i].base = cmdq->base + CMDQ_THR_BASE +
> + CMDQ_THR_SIZE * i;
> + INIT_LIST_HEAD(&cmdq->thread[i].task_busy_list);
>
You seem the queue mailbox requests in this controller driver? why not
use the mailbox api for that?
> + init_timer(&cmdq->thread[i].timeout);
> + cmdq->thread[i].timeout.function = cmdq_thread_handle_timeout;
> + cmdq->thread[i].timeout.data = (unsigned long)&cmdq->thread[i];
>
Here again... you seem to ignore the polling mechanism provided by the
mailbox api, and implement your own.
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> new file mode 100644
> index 0000000..3433c64
> --- /dev/null
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
....
> +
> +struct cmdq_pkt {
> + void *va_base;
>
maybe add 'pa_base' here so you don't have to do dma_map_single() in send_data
> + size_t cmd_buf_size; /* command occupied size */
> + size_t buf_size; /* real buffer size */
> + struct cmdq_task_cb cb;
> +};
> +
> +#endif /* __MTK_CMDQ_MAILBOX_H__ */
> --
> 1.9.1
>
WARNING: multiple messages have this Message-ID (diff)
From: Jassi Brar <jassisinghbrar@gmail.com>
To: HS Liao <hs.liao@mediatek.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
Daniel Kurtz <djkurtz@chromium.org>,
Sascha Hauer <s.hauer@pengutronix.de>,
Devicetree List <devicetree@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
linux-mediatek@lists.infradead.org, srv_heupstream@mediatek.com,
Sascha Hauer <kernel@pengutronix.de>,
Philipp Zabel <p.zabel@pengutronix.de>,
Nicolas Boichat <drinkcat@chromium.org>,
CK HU <ck.hu@mediatek.com>, cawa cheng <cawa.cheng@mediatek.com>,
Bibby Hsieh <bibby.hsieh@mediatek.com>,
YT Shen <yt.shen@mediatek.com>,
Daoyuan Huang <daoyuan.huang@mediatek.com>,
Damon Chu <damon.chu@mediatek.com>
Subject: Re: [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
Date: Thu, 26 Jan 2017 10:08:21 +0530 [thread overview]
Message-ID: <CABb+yY23TUB8NpmpiO=cCmVFAVJMqKPfGgX14j+_Yj7tJvMOuQ@mail.gmail.com> (raw)
In-Reply-To: <1483499169-16329-3-git-send-email-hs.liao@mediatek.com>
On Wed, Jan 4, 2017 at 8:36 AM, HS Liao <hs.liao@mediatek.com> wrote:
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> new file mode 100644
> index 0000000..747bcd3
> --- /dev/null
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
...
> +static void cmdq_task_exec(struct cmdq_pkt *pkt, struct cmdq_thread *thread)
> +{
> + struct cmdq *cmdq;
> + struct cmdq_task *task;
> + unsigned long curr_pa, end_pa;
> +
> + cmdq = dev_get_drvdata(thread->chan->mbox->dev);
> +
> + /* Client should not flush new tasks if suspended. */
> + WARN_ON(cmdq->suspended);
> +
> + task = kzalloc(sizeof(*task), GFP_ATOMIC);
> + task->cmdq = cmdq;
> + INIT_LIST_HEAD(&task->list_entry);
> + task->pa_base = dma_map_single(cmdq->mbox.dev, pkt->va_base,
> + pkt->cmd_buf_size, DMA_TO_DEVICE);
>
You seem to parse the requests and responses, that should ideally be
done in client driver.
Also, we are here in atomic context, can you move it in client driver
(before the spin_lock)?
Maybe by adding a new 'pa_base' member as well in 'cmdq_pkt'.
....
> +
> + cmdq->mbox.num_chans = CMDQ_THR_MAX_COUNT;
> + cmdq->mbox.ops = &cmdq_mbox_chan_ops;
> + cmdq->mbox.of_xlate = cmdq_xlate;
> +
> + /* make use of TXDONE_BY_ACK */
> + cmdq->mbox.txdone_irq = false;
> + cmdq->mbox.txdone_poll = false;
> +
> + for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
>
You mean i < CMDQ_THR_MAX_COUNT
> + cmdq->thread[i].base = cmdq->base + CMDQ_THR_BASE +
> + CMDQ_THR_SIZE * i;
> + INIT_LIST_HEAD(&cmdq->thread[i].task_busy_list);
>
You seem the queue mailbox requests in this controller driver? why not
use the mailbox api for that?
> + init_timer(&cmdq->thread[i].timeout);
> + cmdq->thread[i].timeout.function = cmdq_thread_handle_timeout;
> + cmdq->thread[i].timeout.data = (unsigned long)&cmdq->thread[i];
>
Here again... you seem to ignore the polling mechanism provided by the
mailbox api, and implement your own.
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> new file mode 100644
> index 0000000..3433c64
> --- /dev/null
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
....
> +
> +struct cmdq_pkt {
> + void *va_base;
>
maybe add 'pa_base' here so you don't have to do dma_map_single() in send_data
> + size_t cmd_buf_size; /* command occupied size */
> + size_t buf_size; /* real buffer size */
> + struct cmdq_task_cb cb;
> +};
> +
> +#endif /* __MTK_CMDQ_MAILBOX_H__ */
> --
> 1.9.1
>
WARNING: multiple messages have this Message-ID (diff)
From: jassisinghbrar@gmail.com (Jassi Brar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
Date: Thu, 26 Jan 2017 10:08:21 +0530 [thread overview]
Message-ID: <CABb+yY23TUB8NpmpiO=cCmVFAVJMqKPfGgX14j+_Yj7tJvMOuQ@mail.gmail.com> (raw)
In-Reply-To: <1483499169-16329-3-git-send-email-hs.liao@mediatek.com>
On Wed, Jan 4, 2017 at 8:36 AM, HS Liao <hs.liao@mediatek.com> wrote:
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> new file mode 100644
> index 0000000..747bcd3
> --- /dev/null
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
...
> +static void cmdq_task_exec(struct cmdq_pkt *pkt, struct cmdq_thread *thread)
> +{
> + struct cmdq *cmdq;
> + struct cmdq_task *task;
> + unsigned long curr_pa, end_pa;
> +
> + cmdq = dev_get_drvdata(thread->chan->mbox->dev);
> +
> + /* Client should not flush new tasks if suspended. */
> + WARN_ON(cmdq->suspended);
> +
> + task = kzalloc(sizeof(*task), GFP_ATOMIC);
> + task->cmdq = cmdq;
> + INIT_LIST_HEAD(&task->list_entry);
> + task->pa_base = dma_map_single(cmdq->mbox.dev, pkt->va_base,
> + pkt->cmd_buf_size, DMA_TO_DEVICE);
>
You seem to parse the requests and responses, that should ideally be
done in client driver.
Also, we are here in atomic context, can you move it in client driver
(before the spin_lock)?
Maybe by adding a new 'pa_base' member as well in 'cmdq_pkt'.
....
> +
> + cmdq->mbox.num_chans = CMDQ_THR_MAX_COUNT;
> + cmdq->mbox.ops = &cmdq_mbox_chan_ops;
> + cmdq->mbox.of_xlate = cmdq_xlate;
> +
> + /* make use of TXDONE_BY_ACK */
> + cmdq->mbox.txdone_irq = false;
> + cmdq->mbox.txdone_poll = false;
> +
> + for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
>
You mean i < CMDQ_THR_MAX_COUNT
> + cmdq->thread[i].base = cmdq->base + CMDQ_THR_BASE +
> + CMDQ_THR_SIZE * i;
> + INIT_LIST_HEAD(&cmdq->thread[i].task_busy_list);
>
You seem the queue mailbox requests in this controller driver? why not
use the mailbox api for that?
> + init_timer(&cmdq->thread[i].timeout);
> + cmdq->thread[i].timeout.function = cmdq_thread_handle_timeout;
> + cmdq->thread[i].timeout.data = (unsigned long)&cmdq->thread[i];
>
Here again... you seem to ignore the polling mechanism provided by the
mailbox api, and implement your own.
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> new file mode 100644
> index 0000000..3433c64
> --- /dev/null
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
....
> +
> +struct cmdq_pkt {
> + void *va_base;
>
maybe add 'pa_base' here so you don't have to do dma_map_single() in send_data
> + size_t cmd_buf_size; /* command occupied size */
> + size_t buf_size; /* real buffer size */
> + struct cmdq_task_cb cb;
> +};
> +
> +#endif /* __MTK_CMDQ_MAILBOX_H__ */
> --
> 1.9.1
>
next prev parent reply other threads:[~2017-01-26 4:38 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-04 3:06 [PATCH v20 0/4] Mediatek MT8173 CMDQ support HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-04 3:06 ` [PATCH v20 1/4] dt-bindings: soc: Add documentation for the MediaTek GCE unit HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-04 3:06 ` [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-26 4:38 ` Jassi Brar [this message]
2017-01-26 4:38 ` Jassi Brar
2017-01-26 4:38 ` Jassi Brar
2017-01-26 8:37 ` Horng-Shyang Liao
2017-01-26 8:37 ` Horng-Shyang Liao
2017-01-26 8:37 ` Horng-Shyang Liao
2017-02-01 5:22 ` Jassi Brar
2017-02-01 5:22 ` Jassi Brar
2017-02-01 5:22 ` Jassi Brar
2017-02-06 5:37 ` Horng-Shyang Liao
2017-02-06 5:37 ` Horng-Shyang Liao
2017-02-06 5:37 ` Horng-Shyang Liao
2017-02-09 12:03 ` Horng-Shyang Liao
2017-02-09 12:03 ` Horng-Shyang Liao
2017-02-09 12:03 ` Horng-Shyang Liao
2017-02-16 15:32 ` Jassi Brar
2017-02-16 15:32 ` Jassi Brar
2017-02-16 15:32 ` Jassi Brar
2017-02-22 3:12 ` Horng-Shyang Liao
2017-02-22 3:12 ` Horng-Shyang Liao
2017-02-22 3:12 ` Horng-Shyang Liao
2017-02-23 4:10 ` Jassi Brar
2017-02-23 4:10 ` Jassi Brar
2017-02-23 4:10 ` Jassi Brar
2017-02-23 12:48 ` Horng-Shyang Liao
2017-02-23 12:48 ` Horng-Shyang Liao
2017-02-23 12:48 ` Horng-Shyang Liao
[not found] ` <497f8e4ef7ae4c8a9b7b4ab259801306@mtkmbs01n1.mediatek.inc>
2018-01-08 8:38 ` FW: " houlong wei
2018-01-08 8:38 ` houlong wei
2018-01-18 3:42 ` houlong wei
2018-01-18 3:42 ` houlong wei
2018-01-18 4:30 ` FW: " houlong wei
2018-01-18 4:30 ` houlong wei
2018-01-18 8:01 ` Jassi Brar
2018-01-18 8:01 ` Jassi Brar
2018-01-18 8:31 ` houlong wei
2018-01-18 8:31 ` houlong wei
2018-01-18 8:31 ` houlong wei
2017-01-04 3:06 ` [PATCH v20 3/4] arm64: dts: mt8173: Add GCE node HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-04 3:06 ` [PATCH v20 4/4] soc: mediatek: Add Mediatek CMDQ helper HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-04 3:06 ` HS Liao
2017-01-13 1:27 ` [PATCH v20 0/4] Mediatek MT8173 CMDQ support Horng-Shyang Liao
2017-01-13 1:27 ` Horng-Shyang Liao
2017-01-13 1:27 ` Horng-Shyang Liao
2017-01-20 3:11 ` Horng-Shyang Liao
2017-01-20 3:11 ` Horng-Shyang Liao
2017-01-20 3:11 ` Horng-Shyang Liao
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='CABb+yY23TUB8NpmpiO=cCmVFAVJMqKPfGgX14j+_Yj7tJvMOuQ@mail.gmail.com' \
--to=jassisinghbrar@gmail.com \
--cc=bibby.hsieh@mediatek.com \
--cc=cawa.cheng@mediatek.com \
--cc=ck.hu@mediatek.com \
--cc=damon.chu@mediatek.com \
--cc=daoyuan.huang@mediatek.com \
--cc=dennis-yc.hsieh@mediatek.com \
--cc=devicetree@vger.kernel.org \
--cc=djkurtz@chromium.org \
--cc=drinkcat@chromium.org \
--cc=glory.hung@mediatek.com \
--cc=houlong.wei@mediatek.com \
--cc=hs.liao@mediatek.com \
--cc=jiaguang.zhang@mediatek.com \
--cc=josh-yc.liu@mediatek.com \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=monica.wang@mediatek.com \
--cc=p.zabel@pengutronix.de \
--cc=robh+dt@kernel.org \
--cc=s.hauer@pengutronix.de \
--cc=srv_heupstream@mediatek.com \
--cc=yt.shen@mediatek.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.