Received: by 10.223.148.5 with SMTP id 5csp6851806wrq; Wed, 17 Jan 2018 20:31:07 -0800 (PST) X-Google-Smtp-Source: ACJfBou0jyr6yEmIm/QZrZKnYEjS8XsHXTFmvrA5wCBX0FVwlYOeVYA+qVjytk5IbdhZrxKqm2KL X-Received: by 10.98.71.74 with SMTP id u71mr19943390pfa.45.1516249867666; Wed, 17 Jan 2018 20:31:07 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1516249867; cv=none; d=google.com; s=arc-20160816; b=JIvO41kjhC6Bd/Z5dcMoIR983+7e64nEe73nuCYbNsEKhP6GtY6rt1yvxgdcZaRyqh JmYOIf1+Hs8CyqTF2vuPWOGmGXQCh65rp7pmkXzAXYhwU4xZHNodNaiG/Ymh7GUnABxP Xd9FEFbunKG0hPxalYZfmGlENq4A5TZfai6rMszNa47pHsZXXTPQGUKDaQjrCOxcVbbz GnR+kOdZHX7arLUko3r3BL4LAKwqfn6fxzx/D93nLn1Fcni0JDzrbQrVMLl8hD11+e8b kUXFjLT5VmtNl049PmH5X1+EHhwNDtdj3Wa61nd21BC1+mDqTNzABizda9rXkhbrZxay CPig== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:content-transfer-encoding :references:in-reply-to:date:cc:to:from:subject:message-id :arc-authentication-results; bh=aGhXBvmzGncVYPkQ+B3OhUoVz4K8tpLWiBsiUmWGvFk=; b=BGpTAClwECsI/kOVGAF8Q7whkfYCY83tC8r0t+G01CdwnQDouz7uoXZ6Pz8D4bODb2 2+gYDDqVZYO7/4FiyKMPGXEd6WvtgrJrnh6/milQh+K1O++30qd3EYNLDIablSWge6PB 6qXyUGgqss7lxUPehV0Mf4LlZYbRzeHt5bsSriUM9tWtDT/XvmEkDzzAmUYiiZah9xpc 6FFufgElH8KJsb9MkF9DcX+jZ18xq7Fitc5zY1JmR2qL6YWP+ihSd5P/m1gxgDba5HR3 BiNnJJ06zAXoY3WrXY8Jb6jeS4JGhMEJ5W63i+4mttKQdrlECPz2MbO9svI45qHyPbAL TEKg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id l26si5754594pfj.401.2018.01.17.20.30.53; Wed, 17 Jan 2018 20:31:07 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753730AbeAREa2 convert rfc822-to-8bit (ORCPT + 99 others); Wed, 17 Jan 2018 23:30:28 -0500 Received: from mailgw02.mediatek.com ([1.203.163.81]:46019 "EHLO mailgw02.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1752184AbeAREa1 (ORCPT ); Wed, 17 Jan 2018 23:30:27 -0500 X-UUID: 3dba6f5d536946ca8b4f3d821e10844c-20180118 Received: from mtkcas35.mediatek.inc [(172.27.4.250)] by mailgw02.mediatek.com (envelope-from ) (mailgw01.mediatek.com ESMTP with TLS) with ESMTP id 1405480124; Thu, 18 Jan 2018 12:30:23 +0800 Received: from MTKCAS36.mediatek.inc (172.27.4.186) by MTKMBS31N1.mediatek.inc (172.27.4.69) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Thu, 18 Jan 2018 12:30:21 +0800 Received: from [10.17.3.153] (10.17.3.153) by MTKCAS36.mediatek.inc (172.27.4.170) with Microsoft SMTP Server id 15.0.1210.3 via Frontend Transport; Thu, 18 Jan 2018 12:30:20 +0800 Message-ID: <1516249820.13759.18.camel@mhfsdcap03> Subject: Re: FW: [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver From: houlong wei To: Jassi Brar CC: Rob Herring , "; Matthias Brugger" , "; Daniel Kurtz" , "; Sascha Hauer" , "; Devicetree List" , "; Linux Kernel Mailing List" , "; linux-arm-kernel@lists.infradead.org; linux-mediatek@lists.infradead.org; srv_heupstream" , "; Sascha Hauer" , "; Philipp Zabel" , "; Nicolas Boichat" , "; CK Hu =?UTF-8?Q?=28=E8=83=A1=E4=BF=8A=E5=85=89=29?=" , "; Cawa Cheng =?UTF-8?Q?=28=E9=84=AD=E6=9B=84=E7=A6=A7=29?=" , "; Bibby Hsieh =?UTF-8?Q?=28=E8=AC=9D=E6=BF=9F=E9=81=A0=29?=" , "; YT Shen =?UTF-8?Q?=28=E6=B2=88=E5=B2=B3=E9=9C=86=29?=" , "; Daoyuan Huang =?UTF-8?Q?=28=E9=BB=83=E9=81=93=E5=8E=9F=29?=" , "; Damon Chu =?UTF-8?Q?=28=E6=9C=B1=E5=B3=BB=E8=B3=A2=29?=" , "; Josh-YC Liu =?UTF-8?Q?=28=E5=8A=89=E8=82=B2=E8=AA=A0=29?=" , "; Glory Hung =?UTF-8?Q?=28=E6=B4=AA=E6=99=BA=E7=91=8B=29?=" , "; Jiaguang Zhang =?UTF-8?Q?=28=E5=BC=A0=E5=8A=A0=E5=B9=BF=29?=" , "; Dennis-YC Hsieh =?UTF-8?Q?=28=E8=AC=9D=E5=AE=87=E5=93=B2=29?=" , "; Monica Wang =?UTF-8?Q?=28=E7=8E=8B=E5=AD=9F=E5=A9=B7=29?=" , "; Houlong Wei =?UTF-8?Q?=28=E9=AD=8F=E5=8E=9A=E9=BE=99=29?=" , "; Hs Liao =?UTF-8?Q?=28=E5=BB=96=E5=AE=8F=E7=A5=A5=29?=" Date: Thu, 18 Jan 2018 12:30:20 +0800 In-Reply-To: <1515400735.21044.35.camel@mhfsdcap03> References: <1483499169-16329-1-git-send-email-hs.liao@mediatek.com> <1483499169-16329-3-git-send-email-hs.liao@mediatek.com> <1485419833.990.1.camel@mtksdaap41> <1486359476.11424.33.camel@mtksdaap41> <1487733150.15869.11.camel@mtksdaap41> <1487854104.17813.1.camel@mtksdaap41> <497f8e4ef7ae4c8a9b7b4ab259801306@mtkmbs01n1.mediatek.inc> <1515400735.21044.35.camel@mhfsdcap03> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-MTK: N Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Sorry to send the mail again because I missed some mail lists by mistake. Hi Jassi, We prefer to use method (1) to move dma_map_single() outside of spin_lock. Do you have any comment about this? Thanks, Houlong On Mon, 2018-01-08 at 16:38 +0800, houlong wei wrote: > 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 > > Cc: Rob Herring ; Matthias Brugger ; Daniel Kurtz ; Sascha Hauer ; Devicetree List ; Linux Kernel Mailing List ; linux-arm-kernel@lists.infradead.org; linux-mediatek@lists.infradead.org; srv_heupstream ; Sascha Hauer ; Philipp Zabel ; Nicolas Boichat ; CK Hu (胡俊光) ; Cawa Cheng (鄭曄禧) ; Bibby Hsieh (謝濟遠) ; YT Shen (沈岳霆) ; Daoyuan Huang (黃道原) ; Damon Chu (朱峻賢) ; Josh-YC Liu (劉育誠) ; Glory Hung (洪智瑋) ; Jiaguang Zhang (张加广) ; Dennis-YC Hsieh (謝宇哲) ; Monica Wang (王孟婷) ; Houlong Wei (魏厚龙) ; Hs Liao (廖宏祥) > > 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 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 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 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 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 > > > > > > >