Received: by 2002:ac0:aa62:0:0:0:0:0 with SMTP id w31-v6csp83227ima; Tue, 23 Oct 2018 20:02:27 -0700 (PDT) X-Google-Smtp-Source: AJdET5czhbTYd81x4fkX5aMu09JAiLL9QrWg8P46Xu7h3alaL4d6IEYivdAIO39vwFjcMWkWYPpn X-Received: by 2002:a17:902:ba95:: with SMTP id k21-v6mr863593pls.38.1540350147319; Tue, 23 Oct 2018 20:02:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1540350147; cv=none; d=google.com; s=arc-20160816; b=zt6UwDkWJG9kXZvJHs75rbzzAGKJFIw14pKapar0xOm8b2G1djB7+Tm+TWzVa/gALo QJ/aMfq78r51VwlhuzyB0u7hT3As44Lmnhk/nqYtdlpw30KFnQrECZO0FIZFt7aQKT4F Lx/kXvmyS7WoXMDKsBIiPnEWSZSjAItIor/LdQFR91mU7JeuScOnrM5Eb0E/8nAR714c KjEPHX3wTeKXCs3OViVyj4vmeH9CT4MQkvRK5eqCEcvxZW8SIG4QejUncz0H0KxubEsP R9Bg5k7zemfTVzJ2EvTlrlPd947pJmVh5UD9ivy7wcEgoGSKfDDZk75VrM+uGWuf9pog /GWA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:dkim-signature; bh=yWy4rYUCVtW62lFMlqP9Z1ryLNVq6/FIMM9i3lPVJoU=; b=psPfXtwgkIqiAaJP0D33xKrL0hCdANr1o+aPyhXDMK3NZALoMyKNY3iqX4ksZtC2Pw MDwYMBmzov5wR+uBTnqz+VYZ0GSBg9+IPD+1yTi6gS3gE5Cd+SaZYy+BYmqOU3JiGfjI XNND0sN/vGHhRxUQ6pPsZvythcIH8815mVgQKUiUgL8NJKbzKYdQLA2xi+GKFxeJH+Os /sRbrhNyzp33ewjtPQJQ0Tfo5iu+0WIDVWrdPFQPSjA6O+vZ5FN+vow9X53l6+fNaLfz 0r+OC42AS9dnA2BFQBuq+0mSLvtGWUSTyz6zYxql4c+TBgUlap1qREIwNqIqjD1dWDIJ plig== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b="fH/+991v"; 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; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j5-v6si3337186pgk.85.2018.10.23.20.02.10; Tue, 23 Oct 2018 20:02:27 -0700 (PDT) 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; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b="fH/+991v"; 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; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726407AbeJXL1u (ORCPT + 99 others); Wed, 24 Oct 2018 07:27:50 -0400 Received: from fllv0016.ext.ti.com ([198.47.19.142]:40038 "EHLO fllv0016.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725979AbeJXL1u (ORCPT ); Wed, 24 Oct 2018 07:27:50 -0400 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by fllv0016.ext.ti.com (8.15.2/8.15.2) with ESMTP id w9O31hMF048805; Tue, 23 Oct 2018 22:01:44 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1540350104; bh=yWy4rYUCVtW62lFMlqP9Z1ryLNVq6/FIMM9i3lPVJoU=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=fH/+991vyvDNNWUwUhNOBT9JCZMk4zAazLrxZYpmqnzH/pTjm/g5cSfwHgOm3jeyV bqBvci3MTT4iP6Gjquyj3lOWJUOWSk3aSQK+VTmv2tx/CSdiZmsZvQBZHYz9/F9a/+ ENy/4/dT56BwlpxTMN0GemOjwYYbRlnpiC1kDglY= Received: from DFLE110.ent.ti.com (dfle110.ent.ti.com [10.64.6.31]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id w9O31hsR005971; Tue, 23 Oct 2018 22:01:43 -0500 Received: from DFLE112.ent.ti.com (10.64.6.33) by DFLE110.ent.ti.com (10.64.6.31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Tue, 23 Oct 2018 22:01:42 -0500 Received: from dlep33.itg.ti.com (157.170.170.75) by DFLE112.ent.ti.com (10.64.6.33) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.1.1466.3 via Frontend Transport; Tue, 23 Oct 2018 22:01:42 -0500 Received: from [128.247.58.153] (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep33.itg.ti.com (8.14.3/8.13.8) with ESMTP id w9O31g8t026230; Tue, 23 Oct 2018 22:01:42 -0500 Subject: Re: [PATCH v4 16/17] remoteproc: st: add reserved memory support To: Loic Pallardy , , CC: , , , References: <1532697292-14272-1-git-send-email-loic.pallardy@st.com> <1532697292-14272-17-git-send-email-loic.pallardy@st.com> From: Suman Anna Message-ID: <369c47bb-6ef1-d694-cf14-39bce87dfd0c@ti.com> Date: Tue, 23 Oct 2018 22:01:42 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <1532697292-14272-17-git-send-email-loic.pallardy@st.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Loic, On 7/27/18 8:14 AM, Loic Pallardy wrote: > ST remote processor needs some specified memory regions for > firmware and IPC. > Memory regions are defined as reserved memory and should > be registered in remoteproc core thanks to rproc_add_carveout > function before rproc_start. For this, st rproc driver implements > prepare ops. > > Signed-off-by: Loic Pallardy > --- > drivers/remoteproc/st_remoteproc.c | 96 +++++++++++++++++++++++++++++++++----- > 1 file changed, 85 insertions(+), 11 deletions(-) > > diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c > index aacef0e..45958d5 100644 > --- a/drivers/remoteproc/st_remoteproc.c > +++ b/drivers/remoteproc/st_remoteproc.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -91,6 +92,82 @@ static void st_rproc_kick(struct rproc *rproc, int vqid) > dev_err(dev, "failed to send message via mbox: %d\n", ret); > } > > +static int st_rproc_mem_alloc(struct rproc *rproc, > + struct rproc_mem_entry *mem) > +{ > + struct device *dev = rproc->dev.parent; > + void *va; > + > + va = ioremap_wc(mem->dma, mem->len); > + if (!va) { > + dev_err(dev, "Unable to map memory region: %pa+%zx\n", > + &mem->dma, mem->len); > + return -ENOMEM; > + } > + > + /* Update memory entry va */ > + mem->va = va; > + > + return 0; > +} > + > +static int st_rproc_mem_release(struct rproc *rproc, > + struct rproc_mem_entry *mem) > +{ > + iounmap(mem->va); > + > + return 0; > +} > + > +static int st_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw) > +{ > + struct device *dev = rproc->dev.parent; > + struct device_node *np = dev->of_node; > + struct rproc_mem_entry *mem; > + void *va; > + struct reserved_mem *rmem; > + struct of_phandle_iterator it; > + int err, index = 0; > + > + of_phandle_iterator_init(&it, np, "memory-region", NULL, 0); > + while ((err = of_phandle_iterator_next(&it)) == 0) { > + va = NULL; > + rmem = of_reserved_mem_lookup(it.node); > + > + /* No need to map vdev buffer */ > + if (strcmp(it.node->name, "vdev0buffer")) { > + va = devm_ioremap_wc(dev, rmem->base, rmem->size); > + if (!va) { > + dev_err(dev, "Unable to map memory region: %pa+%zx\n", > + &rmem->base, rmem->size); > + return -ENOMEM; > + } > + > + /* Register memory region */ > + mem = rproc_mem_entry_init(dev, va, > + (dma_addr_t)rmem->base, > + rmem->size, rmem->base, > + st_rproc_mem_alloc, > + st_rproc_mem_release, You seem to be double-mapping, first just above to supply the va to mem_entry_init and then through the alloc function. This is DT parsing and is fixed irrespective of firmware and only needs to be done once really in the platform driver probe. I think this whole logic is unnecessarily complicated in the name of letting the remoteproc core handle the alloc and free for fixed memory carveouts during every boot and shutdown, and having to redo the mapping everytime during the firmware parse. I am not a big fan of overloading the parse_fw to do this runtime remapping everytime for something that should have been done once. I think you have done this in your v2, and somewhere along in v3 got modified into this. regards Suman > + it.node->name); > + } else { > + /* Register reserved memory for vdev buffer allocation */ > + mem = rproc_of_resm_mem_entry_init(dev, index, > + rmem->size, > + rmem->base, > + it.node->name); > + } > + > + if (!mem) > + return -ENOMEM; > + > + rproc_add_carveout(rproc, mem); > + index++; > + } > + > + return rproc_elf_load_rsc_table(rproc, fw); > +} > + > static int st_rproc_start(struct rproc *rproc) > { > struct st_rproc *ddata = rproc->priv; > @@ -158,9 +235,14 @@ static int st_rproc_stop(struct rproc *rproc) > } > > static const struct rproc_ops st_rproc_ops = { > - .kick = st_rproc_kick, > - .start = st_rproc_start, > - .stop = st_rproc_stop, > + .kick = st_rproc_kick, > + .start = st_rproc_start, > + .stop = st_rproc_stop, > + .parse_fw = st_rproc_parse_fw, > + .load = rproc_elf_load_segments, > + .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table, > + .sanity_check = rproc_elf_sanity_check, > + .get_boot_addr = rproc_elf_get_boot_addr, > }; > > /* > @@ -254,12 +336,6 @@ static int st_rproc_parse_dt(struct platform_device *pdev) > return -EINVAL; > } > > - err = of_reserved_mem_device_init(dev); > - if (err) { > - dev_err(dev, "Failed to obtain shared memory\n"); > - return err; > - } > - > err = clk_prepare(ddata->clk); > if (err) > dev_err(dev, "failed to get clock\n"); > @@ -387,8 +463,6 @@ static int st_rproc_remove(struct platform_device *pdev) > > clk_disable_unprepare(ddata->clk); > > - of_reserved_mem_device_release(&pdev->dev); > - > for (i = 0; i < ST_RPROC_MAX_VRING * MBOX_MAX; i++) > mbox_free_channel(ddata->mbox_chan[i]); > >