[PATCH v3 0/4] remoteproc: st: add virtio_rpmsg support

[PATCH v3 0/4] remoteproc: st: add virtio_rpmsg support

[Loic Pallardy]

Goal of this series is:
- to add vring based communication link (virtio_rpmsg)
- to add rproc_da_to_va translation function to allow firmware loading in
  pre-reserved carveout memory region

V2:
Only changes in patch 1 to:
- fix typos
- fix mailbox allocation per coprocessor

V3:
- Integrate Bjorn's comments about mailbox support simplification
- Add missing Signed-of-by
- Move st_rproc_probe error management correction in a dedicated patch
- Fix warnings reported by checkpatch --strict

Loic Pallardy (4):
  remoteproc: st: correct probe error management
  remoteproc: st: add virtio communication support
  remoteproc: st: add da to va support
  remoteproc: core: don't allocate carveout if pa or da are defined

 drivers/remoteproc/Kconfig           |   3 +
 drivers/remoteproc/remoteproc_core.c |   5 ++
 drivers/remoteproc/st_remoteproc.c   | 158 +++++++++++++++++++++++++++++++++--
 3 files changed, 158 insertions(+), 8 deletions(-)

-- 
1.9.1

[PATCH v3 1/4] remoteproc: st: correct probe error management

[Loic Pallardy]

Associated clock is prepared in st_rproc_parse_dt function.
it should be unprepared in case of error during probing.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
Changes from V2:
New patch fixing st_rproc_probe error management

 drivers/remoteproc/st_remoteproc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
index da4e152..bdfab49 100644
--- a/drivers/remoteproc/st_remoteproc.c
+++ b/drivers/remoteproc/st_remoteproc.c
@@ -247,7 +247,7 @@ static int st_rproc_probe(struct platform_device *pdev)
 	enabled = st_rproc_state(pdev);
 	if (enabled < 0) {
 		ret = enabled;
-		goto free_rproc;
+		goto free_clk;
 	}
 
 	if (enabled) {
@@ -259,10 +259,12 @@ static int st_rproc_probe(struct platform_device *pdev)
 
 	ret = rproc_add(rproc);
 	if (ret)
-		goto free_rproc;
+		goto free_clk;
 
 	return 0;
 
+free_clk:
+	clk_unprepare(ddata->clk);
 free_rproc:
 	rproc_free(rproc);
 	return ret;
-- 
1.9.1

Re: [PATCH v3 1/4] remoteproc: st: correct probe error management

[Bjorn Andersson]

On Tue 31 Jan 04:35 PST 2017, Loic Pallardy wrote:

> Associated clock is prepared in st_rproc_parse_dt function.
> it should be unprepared in case of error during probing.
> 
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

Applied patch 1 and 2.

Regards,
Bjorn

RE: [PATCH v3 1/4] remoteproc: st: correct probe error management

[Loic PALLARDY]

> -----Original Message-----
> From: Bjorn Andersson [mailto:bjorn.andersson@linaro.org]
> Sent: Monday, February 06, 2017 10:20 PM
> To: Loic PALLARDY <loic.pallardy@st.com>
> Cc: ohad@wizery.com; lee.jones@linaro.org; linux-
> remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org;
> kernel@stlinux.com; Patrice CHOTARD <patrice.chotard@st.com>; Hugues
> FRUCHET <hugues.fruchet@st.com>; peter.griffin@linaro.org
> Subject: Re: [PATCH v3 1/4] remoteproc: st: correct probe error management
> 
> On Tue 31 Jan 04:35 PST 2017, Loic Pallardy wrote:
> 
> > Associated clock is prepared in st_rproc_parse_dt function.
> > it should be unprepared in case of error during probing.
> >
> > Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> 
> Applied patch 1 and 2.
Thanks Bjorn.
Did you have time to review patch 3 and 4?
Regards,
Loic
> 
> Regards,
> Bjorn

[PATCH v3 2/4] remoteproc: st: add virtio communication support

[Loic Pallardy]

This patch provides virtio communication support based on mailbox
for ST co-processors.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
Changes from V2:
- Flatten mailbox channel to simplify mailbox management
- Flip test condition to simplify code
- Move clk_unprepare in a dedicated fix patch
- Add missing Signed-off-by

Changes from V1:
- fix typos
- fix mailbox allocation per co-processor
---
 drivers/remoteproc/Kconfig         |   3 +
 drivers/remoteproc/st_remoteproc.c | 113 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 8f9cf0b..108add8 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -111,6 +111,9 @@ config ST_REMOTEPROC
 	tristate "ST remoteproc support"
 	depends on ARCH_STI
 	depends on REMOTEPROC
+	select MAILBOX
+	select STI_MBOX
+	select RPMSG_VIRTIO
 	help
 	  Say y here to support ST's adjunct processors via the remote
 	  processor framework.
diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
index bdfab49..f1e9339 100644
--- a/drivers/remoteproc/st_remoteproc.c
+++ b/drivers/remoteproc/st_remoteproc.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
+#include <linux/mailbox_client.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -25,6 +26,16 @@
 #include <linux/remoteproc.h>
 #include <linux/reset.h>
 
+#include "remoteproc_internal.h"
+
+#define ST_RPROC_VQ0		0
+#define ST_RPROC_VQ1		1
+#define ST_RPROC_MAX_VRING	2
+
+#define MBOX_RX			0
+#define MBOX_TX			1
+#define MBOX_MAX		2
+
 struct st_rproc_config {
 	bool			sw_reset;
 	bool			pwr_reset;
@@ -39,8 +50,47 @@ struct st_rproc {
 	u32			clk_rate;
 	struct regmap		*boot_base;
 	u32			boot_offset;
+	struct mbox_chan	*mbox_chan[ST_RPROC_MAX_VRING * MBOX_MAX];
+	struct mbox_client mbox_client_vq0;
+	struct mbox_client mbox_client_vq1;
 };
 
+static void st_rproc_mbox_callback(struct device *dev, u32 msg)
+{
+	struct rproc *rproc = dev_get_drvdata(dev);
+
+	if (rproc_vq_interrupt(rproc, msg) == IRQ_NONE)
+		dev_dbg(dev, "no message was found in vqid %d\n", msg);
+}
+
+static
+void st_rproc_mbox_callback_vq0(struct mbox_client *mbox_client, void *data)
+{
+	st_rproc_mbox_callback(mbox_client->dev, 0);
+}
+
+static
+void st_rproc_mbox_callback_vq1(struct mbox_client *mbox_client, void *data)
+{
+	st_rproc_mbox_callback(mbox_client->dev, 1);
+}
+
+static void st_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct st_rproc *ddata = rproc->priv;
+	struct device *dev = rproc->dev.parent;
+	int ret;
+
+	/* send the index of the triggered virtqueue in the mailbox payload */
+	if (WARN_ON(vqid >= ST_RPROC_MAX_VRING))
+		return;
+
+	ret = mbox_send_message(ddata->mbox_chan[vqid * MBOX_MAX + MBOX_TX],
+				(void *)&vqid);
+	if (ret < 0)
+		dev_err(dev, "failed to send message via mbox: %d\n", ret);
+}
+
 static int st_rproc_start(struct rproc *rproc)
 {
 	struct st_rproc *ddata = rproc->priv;
@@ -108,6 +158,7 @@ static int st_rproc_stop(struct rproc *rproc)
 }
 
 static struct rproc_ops st_rproc_ops = {
+	.kick		= st_rproc_kick,
 	.start		= st_rproc_start,
 	.stop		= st_rproc_stop,
 };
@@ -221,8 +272,9 @@ static int st_rproc_probe(struct platform_device *pdev)
 	struct st_rproc *ddata;
 	struct device_node *np = dev->of_node;
 	struct rproc *rproc;
+	struct mbox_chan *chan;
 	int enabled;
-	int ret;
+	int ret, i;
 
 	match = of_match_device(st_rproc_match, dev);
 	if (!match || !match->data) {
@@ -257,12 +309,65 @@ static int st_rproc_probe(struct platform_device *pdev)
 		clk_set_rate(ddata->clk, ddata->clk_rate);
 	}
 
+	if (of_get_property(np, "mbox-names", NULL)) {
+		ddata->mbox_client_vq0.dev		= dev;
+		ddata->mbox_client_vq0.tx_done		= NULL;
+		ddata->mbox_client_vq0.tx_block	= false;
+		ddata->mbox_client_vq0.knows_txdone	= false;
+		ddata->mbox_client_vq0.rx_callback	= st_rproc_mbox_callback_vq0;
+
+		ddata->mbox_client_vq1.dev		= dev;
+		ddata->mbox_client_vq1.tx_done		= NULL;
+		ddata->mbox_client_vq1.tx_block	= false;
+		ddata->mbox_client_vq1.knows_txdone	= false;
+		ddata->mbox_client_vq1.rx_callback	= st_rproc_mbox_callback_vq1;
+
+		/*
+		 * To control a co-processor without IPC mechanism.
+		 * This driver can be used without mbox and rpmsg.
+		 */
+		chan = mbox_request_channel_byname(&ddata->mbox_client_vq0, "vq0_rx");
+		if (IS_ERR(chan)) {
+			dev_err(&rproc->dev, "failed to request mbox chan 0\n");
+			ret = PTR_ERR(chan);
+			goto free_clk;
+		}
+		ddata->mbox_chan[ST_RPROC_VQ0 * MBOX_MAX + MBOX_RX] = chan;
+
+		chan = mbox_request_channel_byname(&ddata->mbox_client_vq0, "vq0_tx");
+		if (IS_ERR(chan)) {
+			dev_err(&rproc->dev, "failed to request mbox chan 0\n");
+			ret = PTR_ERR(chan);
+			goto free_mbox;
+		}
+		ddata->mbox_chan[ST_RPROC_VQ0 * MBOX_MAX + MBOX_TX] = chan;
+
+		chan = mbox_request_channel_byname(&ddata->mbox_client_vq1, "vq1_rx");
+		if (IS_ERR(chan)) {
+			dev_err(&rproc->dev, "failed to request mbox chan 1\n");
+			ret = PTR_ERR(chan);
+			goto free_mbox;
+		}
+		ddata->mbox_chan[ST_RPROC_VQ1 * MBOX_MAX + MBOX_RX] = chan;
+
+		chan = mbox_request_channel_byname(&ddata->mbox_client_vq1, "vq1_tx");
+		if (IS_ERR(chan)) {
+			dev_err(&rproc->dev, "failed to request mbox chan 1\n");
+			ret = PTR_ERR(chan);
+			goto free_mbox;
+		}
+		ddata->mbox_chan[ST_RPROC_VQ1 * MBOX_MAX + MBOX_TX] = chan;
+	}
+
 	ret = rproc_add(rproc);
 	if (ret)
-		goto free_clk;
+		goto free_mbox;
 
 	return 0;
 
+free_mbox:
+	for (i = 0; i < ST_RPROC_MAX_VRING * MBOX_MAX; i++)
+		mbox_free_channel(ddata->mbox_chan[i]);
 free_clk:
 	clk_unprepare(ddata->clk);
 free_rproc:
@@ -274,6 +379,7 @@ static int st_rproc_remove(struct platform_device *pdev)
 {
 	struct rproc *rproc = platform_get_drvdata(pdev);
 	struct st_rproc *ddata = rproc->priv;
+	int i;
 
 	rproc_del(rproc);
 
@@ -281,6 +387,9 @@ static int st_rproc_remove(struct platform_device *pdev)
 
 	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]);
+
 	rproc_free(rproc);
 
 	return 0;
-- 
1.9.1

[PATCH v3 3/4] remoteproc: st: add da to va support

[Loic Pallardy]

ST remoteproc driver needs to provide information about
carveout memory region to allow remoteproc core to load
firmware and access trace buffer.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
Changes since V2:
- fix checkpatch --strict warning
---
 drivers/remoteproc/st_remoteproc.c | 43 ++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
index f1e9339..80f8118 100644
--- a/drivers/remoteproc/st_remoteproc.c
+++ b/drivers/remoteproc/st_remoteproc.c
@@ -21,6 +21,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_reserved_mem.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/remoteproc.h>
@@ -53,6 +54,10 @@ struct st_rproc {
 	struct mbox_chan	*mbox_chan[ST_RPROC_MAX_VRING * MBOX_MAX];
 	struct mbox_client mbox_client_vq0;
 	struct mbox_client mbox_client_vq1;
+	phys_addr_t mem_phys;
+	phys_addr_t mem_reloc;
+	void *mem_region;
+	size_t mem_size;
 };
 
 static void st_rproc_mbox_callback(struct device *dev, u32 msg)
@@ -157,10 +162,23 @@ static int st_rproc_stop(struct rproc *rproc)
 	return sw_err ?: pwr_err;
 }
 
+static void *st_proc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct st_rproc *ddata = rproc->priv;
+	int offset;
+
+	offset = da - ddata->mem_reloc;
+	if (offset < 0 || offset + len > ddata->mem_size)
+		return NULL;
+
+	return ddata->mem_region + offset;
+}
+
 static struct rproc_ops st_rproc_ops = {
 	.kick		= st_rproc_kick,
 	.start		= st_rproc_start,
 	.stop		= st_rproc_stop,
+	.da_to_va	= st_proc_da_to_va,
 };
 
 /*
@@ -208,7 +226,8 @@ static int st_rproc_parse_dt(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct rproc *rproc = platform_get_drvdata(pdev);
 	struct st_rproc *ddata = rproc->priv;
-	struct device_node *np = dev->of_node;
+	struct device_node *np = dev->of_node, *node;
+	struct resource res;
 	int err;
 
 	if (ddata->config->sw_reset) {
@@ -252,10 +271,24 @@ 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");
+	node = of_parse_phandle(np, "memory-region", 0);
+	if (!node) {
+		dev_err(dev, "No memory-region specified\n");
+		return -EINVAL;
+	}
+
+	err = of_address_to_resource(node, 0, &res);
+	if (err)
 		return err;
+
+	ddata->mem_phys = res.start;
+	ddata->mem_reloc = res.start;
+	ddata->mem_size = resource_size(&res);
+	ddata->mem_region = devm_ioremap_wc(dev, ddata->mem_phys, ddata->mem_size);
+	if (!ddata->mem_region) {
+		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
+			&res.start, ddata->mem_size);
+		return -EBUSY;
 	}
 
 	err = clk_prepare(ddata->clk);
@@ -385,8 +418,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]);
 
-- 
1.9.1

Re: [v3,3/4] remoteproc: st: add da to va support

[Hugues FRUCHET]

On 01/31/2017 01:35 PM, Loic PALLARDY wrote:
> ST remoteproc driver needs to provide information about
> carveout memory region to allow remoteproc core to load
> firmware and access trace buffer.
>
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

Acked-and-tested-by: Hugues Fruchet <hugues.fruchet@st.com>

Tested on B2260 ST Platform with st-delta video decoder V4L2 kernel 
driver 
(http://www.mail-archive.com/linux-media@vger.kernel.org/msg107644.html).

> ---
> Changes since V2:
> - fix checkpatch --strict warning
> ---
>  drivers/remoteproc/st_remoteproc.c | 43 ++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
> index f1e9339..80f8118 100644
> --- a/drivers/remoteproc/st_remoteproc.c
> +++ b/drivers/remoteproc/st_remoteproc.c
> @@ -21,6 +21,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/of_reserved_mem.h>
> +#include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/remoteproc.h>
> @@ -53,6 +54,10 @@ struct st_rproc {
>  	struct mbox_chan	*mbox_chan[ST_RPROC_MAX_VRING * MBOX_MAX];
>  	struct mbox_client mbox_client_vq0;
>  	struct mbox_client mbox_client_vq1;
> +	phys_addr_t mem_phys;
> +	phys_addr_t mem_reloc;
> +	void *mem_region;
> +	size_t mem_size;
>  };
>
>  static void st_rproc_mbox_callback(struct device *dev, u32 msg)
> @@ -157,10 +162,23 @@ static int st_rproc_stop(struct rproc *rproc)
>  	return sw_err ?: pwr_err;
>  }
>
> +static void *st_proc_da_to_va(struct rproc *rproc, u64 da, int len)
> +{
> +	struct st_rproc *ddata = rproc->priv;
> +	int offset;
> +
> +	offset = da - ddata->mem_reloc;
> +	if (offset < 0 || offset + len > ddata->mem_size)
> +		return NULL;
> +
> +	return ddata->mem_region + offset;
> +}
> +
>  static struct rproc_ops st_rproc_ops = {
>  	.kick		= st_rproc_kick,
>  	.start		= st_rproc_start,
>  	.stop		= st_rproc_stop,
> +	.da_to_va	= st_proc_da_to_va,
>  };
>
>  /*
> @@ -208,7 +226,8 @@ static int st_rproc_parse_dt(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct rproc *rproc = platform_get_drvdata(pdev);
>  	struct st_rproc *ddata = rproc->priv;
> -	struct device_node *np = dev->of_node;
> +	struct device_node *np = dev->of_node, *node;
> +	struct resource res;
>  	int err;
>
>  	if (ddata->config->sw_reset) {
> @@ -252,10 +271,24 @@ 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");
> +	node = of_parse_phandle(np, "memory-region", 0);
> +	if (!node) {
> +		dev_err(dev, "No memory-region specified\n");
> +		return -EINVAL;
> +	}
> +
> +	err = of_address_to_resource(node, 0, &res);
> +	if (err)
>  		return err;
> +
> +	ddata->mem_phys = res.start;
> +	ddata->mem_reloc = res.start;
> +	ddata->mem_size = resource_size(&res);
> +	ddata->mem_region = devm_ioremap_wc(dev, ddata->mem_phys, ddata->mem_size);
> +	if (!ddata->mem_region) {
> +		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
> +			&res.start, ddata->mem_size);
> +		return -EBUSY;
>  	}
>
>  	err = clk_prepare(ddata->clk);
> @@ -385,8 +418,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]);
>
>

[PATCH v3 4/4] remoteproc: core: don't allocate carveout if pa or da are defined

[Loic Pallardy]

Remoteproc doesn't check if firmware requests fixed
addresses for carveout regions.
Current assumption is that platform specific driver is in
charge of coprocessor specific memory region allocation and
remoteproc core doesn't have to handle them.
If a da or a pa is specified in firmware resource table, remoteproc
core doesn't have to perform any allocation.
Access to carveout will be done thanks to rproc_da_to_pa function,
which will provide virtual address on carveout region allocated
by platform specific driver.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
No change since V1

 drivers/remoteproc/remoteproc_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 90b05c7..dd63ceed 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -622,6 +622,11 @@ static int rproc_handle_carveout(struct rproc *rproc,
 	dev_dbg(dev, "carveout rsc: name: %s, da 0x%x, pa 0x%x, len 0x%x, flags 0x%x\n",
 		rsc->name, rsc->da, rsc->pa, rsc->len, rsc->flags);
 
+	if (rsc->pa != FW_RSC_ADDR_ANY || rsc->da != FW_RSC_ADDR_ANY) {
+		dev_dbg(dev, "carveout already allocated by low level driver\n");
+		return 0;
+	}
+
 	carveout = kzalloc(sizeof(*carveout), GFP_KERNEL);
 	if (!carveout)
 		return -ENOMEM;
-- 
1.9.1

Re: [v3, 4/4] remoteproc: core: don't allocate carveout if pa or da are defined

[Hugues FRUCHET]

On 01/31/2017 01:35 PM, Loic PALLARDY wrote:
> Remoteproc doesn't check if firmware requests fixed
> addresses for carveout regions.
> Current assumption is that platform specific driver is in
> charge of coprocessor specific memory region allocation and
> remoteproc core doesn't have to handle them.
> If a da or a pa is specified in firmware resource table, remoteproc
> core doesn't have to perform any allocation.
> Access to carveout will be done thanks to rproc_da_to_pa function,
> which will provide virtual address on carveout region allocated
> by platform specific driver.
>
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

Acked-and-tested-by: Hugues Fruchet <hugues.fruchet@st.com>

Tested on B2260 ST Platform with st-delta video decoder V4L2 kernel 
driver 
(http://www.mail-archive.com/linux-media@vger.kernel.org/msg107644.html).

> ---
> No change since V1
>
>  drivers/remoteproc/remoteproc_core.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 90b05c7..dd63ceed 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -622,6 +622,11 @@ static int rproc_handle_carveout(struct rproc *rproc,
>  	dev_dbg(dev, "carveout rsc: name: %s, da 0x%x, pa 0x%x, len 0x%x, flags 0x%x\n",
>  		rsc->name, rsc->da, rsc->pa, rsc->len, rsc->flags);
>
> +	if (rsc->pa != FW_RSC_ADDR_ANY || rsc->da != FW_RSC_ADDR_ANY) {
> +		dev_dbg(dev, "carveout already allocated by low level driver\n");
> +		return 0;
> +	}
> +
>  	carveout = kzalloc(sizeof(*carveout), GFP_KERNEL);
>  	if (!carveout)
>  		return -ENOMEM;
>
>

Re: [PATCH v3 4/4] remoteproc: core: don't allocate carveout if pa or da are defined

[Bjorn Andersson]

On Tue 31 Jan 04:35 PST 2017, Loic Pallardy wrote:

> Remoteproc doesn't check if firmware requests fixed
> addresses for carveout regions.
> Current assumption is that platform specific driver is in
> charge of coprocessor specific memory region allocation and
> remoteproc core doesn't have to handle them.
> If a da or a pa is specified in firmware resource table, remoteproc
> core doesn't have to perform any allocation.
> Access to carveout will be done thanks to rproc_da_to_pa function,
> which will provide virtual address on carveout region allocated
> by platform specific driver.
> 
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> ---
> No change since V1
> 
>  drivers/remoteproc/remoteproc_core.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 90b05c7..dd63ceed 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -622,6 +622,11 @@ static int rproc_handle_carveout(struct rproc *rproc,
>  	dev_dbg(dev, "carveout rsc: name: %s, da 0x%x, pa 0x%x, len 0x%x, flags 0x%x\n",
>  		rsc->name, rsc->da, rsc->pa, rsc->len, rsc->flags);
>  
> +	if (rsc->pa != FW_RSC_ADDR_ANY || rsc->da != FW_RSC_ADDR_ANY) {

For devices with an IOMMU it's valid to specify "da" and have the
allocated region mapped there. So this is not correct.

Regards,
Bjorn

RE: [PATCH v3 4/4] remoteproc: core: don't allocate carveout if pa or da are defined

[Loic PALLARDY]

> -----Original Message-----
> From: Bjorn Andersson [mailto:bjorn.andersson@linaro.org]
> Sent: Thursday, February 09, 2017 2:55 AM
> To: Loic PALLARDY <loic.pallardy@st.com>
> Cc: ohad@wizery.com; lee.jones@linaro.org; linux-
> remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org;
> kernel@stlinux.com; Patrice CHOTARD <patrice.chotard@st.com>; Hugues
> FRUCHET <hugues.fruchet@st.com>; peter.griffin@linaro.org
> Subject: Re: [PATCH v3 4/4] remoteproc: core: don't allocate carveout if pa or
> da are defined
> 
> On Tue 31 Jan 04:35 PST 2017, Loic Pallardy wrote:
> 
> > Remoteproc doesn't check if firmware requests fixed
> > addresses for carveout regions.
> > Current assumption is that platform specific driver is in
> > charge of coprocessor specific memory region allocation and
> > remoteproc core doesn't have to handle them.
> > If a da or a pa is specified in firmware resource table, remoteproc
> > core doesn't have to perform any allocation.
> > Access to carveout will be done thanks to rproc_da_to_pa function,
> > which will provide virtual address on carveout region allocated
> > by platform specific driver.
> >
> > Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> > ---
> > No change since V1
> >
> >  drivers/remoteproc/remoteproc_core.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> > index 90b05c7..dd63ceed 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -622,6 +622,11 @@ static int rproc_handle_carveout(struct rproc
> *rproc,
> >  	dev_dbg(dev, "carveout rsc: name: %s, da 0x%x, pa 0x%x, len 0x%x,
> flags 0x%x\n",
> >  		rsc->name, rsc->da, rsc->pa, rsc->len, rsc->flags);
> >
> > +	if (rsc->pa != FW_RSC_ADDR_ANY || rsc->da !=
> FW_RSC_ADDR_ANY) {
> 
> For devices with an IOMMU it's valid to specify "da" and have the
> allocated region mapped there. So this is not correct.

You're right, moreover patch series touch only to pa, so even if da behavior should be changed, should be done in a dedicated patch.
I'll send a V4.

Regards,
Loic
> 
> Regards,
> Bjorn