[PATCH v2 0/3] remoteproc: st: add virtio_rpmsg support

[PATCH v2 0/3] 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

Loic Pallardy (3):
  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   | 160 +++++++++++++++++++++++++++++++++--
 3 files changed, 162 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] remoteproc: st: add virtio communication support

[Loic Pallardy]

This patch provides virtio communication support based on mailbox
for ST coprocessors.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/Kconfig         |   3 +
 drivers/remoteproc/st_remoteproc.c | 120 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 122 insertions(+), 1 deletion(-)

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 da4e152..2d99111 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,14 @@
 #include <linux/remoteproc.h>
 #include <linux/reset.h>
 
+#include "remoteproc_internal.h"
+
+#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 +48,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 (vqid < ST_RPROC_MAX_VRING) {
+		ret = mbox_send_message(ddata->mbox_chan[vqid][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 +156,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,6 +270,7 @@ 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;
 
@@ -257,13 +307,76 @@ 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_rproc;
+		} else {
+			ddata->mbox_chan[0][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_one;
+		} else {
+			ddata->mbox_chan[0][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_two;
+		} else {
+			ddata->mbox_chan[1][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_three;
+		} else {
+			ddata->mbox_chan[1][MBOX_TX] = chan;
+		}
+	}
+
 	ret = rproc_add(rproc);
 	if (ret)
-		goto free_rproc;
+		goto free_four;
 
 	return 0;
 
+free_four:
+	mbox_free_channel(ddata->mbox_chan[1][MBOX_TX]);
+free_three:
+	mbox_free_channel(ddata->mbox_chan[1][MBOX_RX]);
+free_two:
+	mbox_free_channel(ddata->mbox_chan[0][MBOX_TX]);
+free_one:
+	mbox_free_channel(ddata->mbox_chan[0][MBOX_RX]);
 free_rproc:
+	clk_unprepare(ddata->clk);
 	rproc_free(rproc);
 	return ret;
 }
@@ -279,6 +392,11 @@ static int st_rproc_remove(struct platform_device *pdev)
 
 	of_reserved_mem_device_release(&pdev->dev);
 
+	mbox_free_channel(ddata->mbox_chan[0][MBOX_RX]);
+	mbox_free_channel(ddata->mbox_chan[1][MBOX_RX]);
+	mbox_free_channel(ddata->mbox_chan[0][MBOX_TX]);
+	mbox_free_channel(ddata->mbox_chan[1][MBOX_TX]);
+
 	rproc_free(rproc);
 
 	return 0;
-- 
1.9.1

[PATCH v2 2/3] 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>
---
 drivers/remoteproc/st_remoteproc.c | 42 ++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
index 2d99111..b6eb716 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>
@@ -51,6 +52,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)
@@ -155,10 +160,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,
 };
 
 /*
@@ -206,7 +224,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) {
@@ -250,10 +269,23 @@ 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 = 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);
@@ -390,8 +422,6 @@ static int st_rproc_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(ddata->clk);
 
-	of_reserved_mem_device_release(&pdev->dev);
-
 	mbox_free_channel(ddata->mbox_chan[0][MBOX_RX]);
 	mbox_free_channel(ddata->mbox_chan[1][MBOX_RX]);
 	mbox_free_channel(ddata->mbox_chan[0][MBOX_TX]);
-- 
1.9.1

[PATCH v2 3/3] 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>
---
 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 9a507e7..4af80cb 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -625,6 +625,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: [PATCH v2 0/3] remoteproc: st: add virtio_rpmsg support

[Patrice Chotard]

On 01/06/2017 06:16 PM, Loic Pallardy wrote:
> 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
> 
> Loic Pallardy (3):
>   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   | 160 +++++++++++++++++++++++++++++++++--
>  3 files changed, 162 insertions(+), 6 deletions(-)
> 

Hi Loic

For the series

Acked-by: Patrice Chotard <patrice.chotard@st.com>

Thanks

Patrice

Re: [PATCH v2 0/3] remoteproc: st: add virtio_rpmsg support

[loic pallardy]

On 01/11/2017 11:19 AM, Patrice Chotard wrote:
> On 01/06/2017 06:16 PM, Loic Pallardy wrote:
>> 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
>>
>> Loic Pallardy (3):
>>   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   | 160 +++++++++++++++++++++++++++++++++--
>>  3 files changed, 162 insertions(+), 6 deletions(-)
>>
>
> Hi Loic
>
> For the series
>
> Acked-by: Patrice Chotard <patrice.chotard@st.com>

Hi Bjorn, Lee,

Any chance to review this series to enter it in v4.11?
For information, the DT counterpart [1] has been submitted to arm-soc 
maintainers by Patrice (you were in CC).

Regards,
Loic

[1]: https://www.spinics.net/lists/arm-kernel/msg553878.html

>
> Thanks
>
> Patrice
>

Re: [PATCH v2 1/3] remoteproc: st: add virtio communication support

[Lee Jones]

On Fri, 06 Jan 2017, Loic Pallardy wrote:

> This patch provides virtio communication support based on mailbox
> for ST coprocessors.

You need to add mine and Ludo's SoB.

> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  drivers/remoteproc/Kconfig         |   3 +
>  drivers/remoteproc/st_remoteproc.c | 120 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 122 insertions(+), 1 deletion(-)

Besides that, still looks good to me:

Acked-by: Lee Jones <lee.jones@linaro.org>

> 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 da4e152..2d99111 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,14 @@
>  #include <linux/remoteproc.h>
>  #include <linux/reset.h>
>  
> +#include "remoteproc_internal.h"
> +
> +#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 +48,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 (vqid < ST_RPROC_MAX_VRING) {
> +		ret = mbox_send_message(ddata->mbox_chan[vqid][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 +156,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,6 +270,7 @@ 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;
>  
> @@ -257,13 +307,76 @@ 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_rproc;
> +		} else {
> +			ddata->mbox_chan[0][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_one;
> +		} else {
> +			ddata->mbox_chan[0][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_two;
> +		} else {
> +			ddata->mbox_chan[1][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_three;
> +		} else {
> +			ddata->mbox_chan[1][MBOX_TX] = chan;
> +		}
> +	}
> +
>  	ret = rproc_add(rproc);
>  	if (ret)
> -		goto free_rproc;
> +		goto free_four;
>  
>  	return 0;
>  
> +free_four:
> +	mbox_free_channel(ddata->mbox_chan[1][MBOX_TX]);
> +free_three:
> +	mbox_free_channel(ddata->mbox_chan[1][MBOX_RX]);
> +free_two:
> +	mbox_free_channel(ddata->mbox_chan[0][MBOX_TX]);
> +free_one:
> +	mbox_free_channel(ddata->mbox_chan[0][MBOX_RX]);
>  free_rproc:
> +	clk_unprepare(ddata->clk);
>  	rproc_free(rproc);
>  	return ret;
>  }
> @@ -279,6 +392,11 @@ static int st_rproc_remove(struct platform_device *pdev)
>  
>  	of_reserved_mem_device_release(&pdev->dev);
>  
> +	mbox_free_channel(ddata->mbox_chan[0][MBOX_RX]);
> +	mbox_free_channel(ddata->mbox_chan[1][MBOX_RX]);
> +	mbox_free_channel(ddata->mbox_chan[0][MBOX_TX]);
> +	mbox_free_channel(ddata->mbox_chan[1][MBOX_TX]);
> +
>  	rproc_free(rproc);
>  
>  	return 0;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog