devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: houlong wei <houlong.wei@mediatek.com>
To: Jassi Brar <jassisinghbrar@gmail.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-mediatek@lists.infradead.org; srv_heupstream"
	<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@>
Subject: Re: FW: [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
Date: Mon, 8 Jan 2018 16:38:55 +0800	[thread overview]
Message-ID: <1515400735.21044.35.camel@mhfsdcap03> (raw)
In-Reply-To: <497f8e4ef7ae4c8a9b7b4ab259801306@mtkmbs01n1.mediatek.inc>

Hi Jassi,

Sorry for reply so late.
According to previous discussion, there are two methods to move
dma_map_single() outside of spin_lock.
(1) put in mtk-cmdq-helper.c, as described by HS on 2017-02-09.
  > I think a trade-off solution is to put in mtk-cmdq-helper.c.
  > Although it is a mailbox client, it is not a CMDQ client.
  > We can include mailbox_controller.h in mtk-cmdq-helper.c (instead of
mtk-cmdq.h), and then map dma at cmdq_pkt_flush_async before
mbox_send_message.

  > pkt->pa_base = dma_map_single(client->chan->mbox->dev, pkt->va_base,
  >                              pkt->cmd_buf_size, DMA_TO_DEVICE);
(2) schedule a tasklet in send_data().

After internal discussion with HS and other experts, now we prefer
method (1).
How do you think about it?

Thanks
Houlong


> -----Original Message-----
> From: Horng-Shyang Liao [mailto:hs.liao@mediatek.com] 
> Sent: Thursday, February 23, 2017 8:48 PM
> To: Jassi Brar <jassisinghbrar@gmail.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-mediatek@lists.infradead.org; srv_heupstream <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>; Hs Liao (廖宏祥) <hs.liao@mediatek.com>
> Subject: Re: [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
> 
> On Thu, 2017-02-23 at 09:40 +0530, Jassi Brar wrote:
> > On Wed, Feb 22, 2017 at 8:42 AM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> > > On Thu, 2017-02-16 at 21:02 +0530, Jassi Brar wrote:
> > >> On Mon, Feb 6, 2017 at 11:07 AM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> > >> > Hi Jassi,
> > >> >
> > >> > On Wed, 2017-02-01 at 10:52 +0530, Jassi Brar wrote:
> > >> >> On Thu, Jan 26, 2017 at 2:07 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> > >> >> > Hi Jassi,
> > >> >> >
> > >> >> > On Thu, 2017-01-26 at 10:08 +0530, Jassi Brar wrote:
> > >> >> >> 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'.
> > >> >> >
> > >> >> > will do
> > >> >
> > >> > I agree with moving dma_map_single out from spin_lock.
> > >> >
> > >> > However, mailbox clients cannot map virtual memory to mailbox 
> > >> > controller's device for DMA.
> > >> >
> > >> If DMA is a resource used by MBox to transfer data, then yes the 
> > >> mapping needs to be done in the Mbox controller driver. To map 
> > >> memory outside of spinlock, you could schedule a tasklet in send_data() ?
> > >
> > > Hi Jassi,
> > >
> > > For CMDQ, the order of CMDQ tasks should be guaranteed.
> > > However, it seems tasklet cannot ensure this requirement.
> > >
> > > Quote from Linux Device Drivers 3rd edition ch7.
> > > "void tasklet_schedule(struct tasklet_struct *t);
> > >   Schedule the tasklet for execution. If a tasklet is scheduled 
> > > again before it has a chance to run, it runs only once...."
> > >
> > Not sure what bothers you.
> > If you only add requests to a list, protected by some spinlock, during 
> > send_datam you could always iterate over (submit) requests in the 
> > order you queued them.
> 
> Hi Jassi,
> 
> OK. I will do it.
> 
> Thanks,
> HS
> 
> 
> 

  parent reply	other threads:[~2018-01-08  8:38 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-04  3:06 [PATCH v20 0/4] Mediatek MT8173 CMDQ support HS Liao
     [not found] ` <1483499169-16329-1-git-send-email-hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
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   ` [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver HS Liao
2017-01-26  4:38     ` Jassi Brar
2017-01-26  8:37       ` Horng-Shyang Liao
2017-02-01  5:22         ` Jassi Brar
     [not found]           ` <CABb+yY13VaFpYz6EExvAXLpAqEWDJ4R50WG0DYbXO6iCESbA5A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-06  5:37             ` Horng-Shyang Liao
2017-02-09 12:03               ` Horng-Shyang Liao
2017-02-16 15:32               ` Jassi Brar
     [not found]                 ` <CABb+yY0jCGqnrMLkkTNoEmLX4jDstWh7-UYhDyKjV-gFOnxpsg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-22  3:12                   ` Horng-Shyang Liao
2017-02-23  4:10                     ` Jassi Brar
     [not found]                       ` <CABb+yY2dzcR4fYvrSQK5jemwUSkYVJ8aWEmiE6wstw-Yyqy7bw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-23 12:48                         ` Horng-Shyang Liao
     [not found]                           ` <497f8e4ef7ae4c8a9b7b4ab259801306@mtkmbs01n1.mediatek.inc>
2018-01-08  8:38                             ` houlong wei [this message]
2018-01-18  3:42                               ` houlong wei
2018-01-18  4:30                               ` FW: " houlong wei
2018-01-18  8:01                               ` Jassi Brar
     [not found]                                 ` <CABb+yY02S=+00mY-NTkJUBPiVruYdf0xuovKomehUxLNOQPLuA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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   ` [PATCH v20 4/4] soc: mediatek: Add Mediatek CMDQ helper HS Liao
2017-01-13  1:27 ` [PATCH v20 0/4] Mediatek MT8173 CMDQ support 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=1515400735.21044.35.camel@mhfsdcap03 \
    --to=houlong.wei@mediatek.com \
    --cc=Bibby.Hsieh@mediatek.com \
    --cc=Daoyuan.Huang@mediatek.com \
    --cc=cawa.cheng@mediatek.com \
    --cc=ck.hu@mediatek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=djkurtz@chromium.org \
    --cc=drinkcat@chromium.org \
    --cc=jassisinghbrar@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthias.bgg@gmail.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 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).