V2:
Rebased on linux-next
Dropped early boot feature to make patchset simple.
Drop rsc-da
V1:
https://patchwork.kernel.org/cover/11682461/
This patchset is to support i.MX8MQ/M coproc.
The early boot feature was dropped to make the patchset small in V2.
Since i.MX specific TCM memory requirement, add elf platform hook.
Several patches have got reviewed by Oleksij and Mathieu in v1.
Peng Fan (7):
remoteproc: elf: support platform specific memory hook
remoteproc: imx_rproc: add elf memory hooks
remoteproc: imx_rproc: correct err message
remoteproc: imx_rproc: use devm_ioremap
remoteproc: imx_rproc: add i.MX specific parse fw hook
remoteproc: imx_rproc: support i.MX8MQ/M
remoteproc: imx_proc: enable virtio/mailbox
drivers/remoteproc/imx_rproc.c | 273 ++++++++++++++++++++-
drivers/remoteproc/remoteproc_elf_loader.c | 20 +-
include/linux/remoteproc.h | 2 +
3 files changed, 287 insertions(+), 8 deletions(-)
--
2.28.0
To arm64, "dc zva, dst" is used in memset.
Per ARM DDI 0487A.j, chapter C5.3.8 DC ZVA, Data Cache Zero by VA,
"If the memory region being zeroed is any type of Device memory,
this instruction can give an alignment fault which is prioritized
in the same way as other alignment faults that are determined
by the memory type."
On i.MX platforms, when elf is loaded to onchip TCM area, the region
is ioremapped, so "dc zva, dst" will trigger abort. And ioremap_wc()
on i.MX not able to write correct data to TCM area.
So we need to use io helpers, and extend the elf loader to support
platform specific memory functions.
Acked-by: Richard Zhu <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/remoteproc/remoteproc_elf_loader.c | 20 ++++++++++++++++++--
include/linux/remoteproc.h | 2 ++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index df68d87752e4..6cb71fe47261 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -129,6 +129,22 @@ u64 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
}
EXPORT_SYMBOL(rproc_elf_get_boot_addr);
+static void rproc_elf_memcpy(struct rproc *rproc, void *dest, const void *src, size_t count)
+{
+ if (!rproc->ops->elf_memcpy)
+ memcpy(dest, src, count);
+
+ rproc->ops->elf_memcpy(rproc, dest, src, count);
+}
+
+static void rproc_elf_memset(struct rproc *rproc, void *s, int c, size_t count)
+{
+ if (!rproc->ops->elf_memset)
+ memset(s, c, count);
+
+ rproc->ops->elf_memset(rproc, s, c, count);
+}
+
/**
* rproc_elf_load_segments() - load firmware segments to memory
* @rproc: remote processor which will be booted using these fw segments
@@ -214,7 +230,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
/* put the segment where the remote processor expects it */
if (filesz)
- memcpy(ptr, elf_data + offset, filesz);
+ rproc_elf_memcpy(rproc, ptr, elf_data + offset, filesz);
/*
* Zero out remaining memory for this segment.
@@ -224,7 +240,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
* this.
*/
if (memsz > filesz)
- memset(ptr + filesz, 0, memsz - filesz);
+ rproc_elf_memset(rproc, ptr + filesz, 0, memsz - filesz);
}
return ret;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 2fa68bf5aa4f..1f5fa2c772df 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -392,6 +392,8 @@ struct rproc_ops {
int (*load)(struct rproc *rproc, const struct firmware *fw);
int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
u64 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
+ void (*elf_memcpy)(struct rproc *rproc, void *dest, const void *src, size_t count);
+ void (*elf_memset)(struct rproc *rproc, void *s, int c, size_t count);
unsigned long (*panic)(struct rproc *rproc);
};
--
2.28.0
Add elf memory hooks according to elf_mem_hook setting in the platform
configuration dcfg.
Acked-by: Richard Zhu <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/remoteproc/imx_rproc.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 8957ed271d20..d1abb253b499 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -6,6 +6,7 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
@@ -76,6 +77,7 @@ struct imx_rproc_dcfg {
u32 src_stop;
const struct imx_rproc_att *att;
size_t att_size;
+ bool elf_mem_hook;
};
struct imx_rproc {
@@ -310,6 +312,16 @@ static int imx_rproc_addr_init(struct imx_rproc *priv,
return 0;
}
+static void imx_rproc_memcpy(struct rproc *rproc, void *dest, const void *src, size_t count)
+{
+ memcpy_toio((void * __iomem)dest, src, count);
+}
+
+static void imx_rproc_memset(struct rproc *rproc, void *s, int c, size_t count)
+{
+ memset_io((void * __iomem)s, c, count);
+}
+
static int imx_rproc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -340,6 +352,11 @@ static int imx_rproc_probe(struct platform_device *pdev)
goto err_put_rproc;
}
+ if (dcfg->elf_mem_hook) {
+ rproc->ops->elf_memcpy = imx_rproc_memcpy;
+ rproc->ops->elf_memset = imx_rproc_memset;
+ }
+
priv = rproc->priv;
priv->rproc = rproc;
priv->regmap = regmap;
--
2.28.0
It is using devm_ioremap, so not devm_ioremap_resource. Correct
the error message and print out sa/size.
Acked-by: Richard Zhu <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/remoteproc/imx_rproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index d1abb253b499..aa5fbd0c7768 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -270,7 +270,7 @@ static int imx_rproc_addr_init(struct imx_rproc *priv,
priv->mem[b].cpu_addr = devm_ioremap(&pdev->dev,
att->sa, att->size);
if (!priv->mem[b].cpu_addr) {
- dev_err(dev, "devm_ioremap_resource failed\n");
+ dev_err(dev, "devm_ioremap failed\n");
return -ENOMEM;
}
priv->mem[b].sys_addr = att->sa;
--
2.28.0
Add i.MX8MQ dev/sys addr map and configuration data structure
i.MX8MM share i.MX8MQ settings.
Reviewed-by: Richard Zhu <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/remoteproc/imx_rproc.c | 40 ++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index bd3a42892b22..0f69f3f745ab 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -90,6 +90,34 @@ struct imx_rproc {
struct clk *clk;
};
+static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
+ /* dev addr , sys addr , size , flags */
+ /* TCML - alias */
+ { 0x00000000, 0x007e0000, 0x00020000, 0 },
+ /* OCRAM_S */
+ { 0x00180000, 0x00180000, 0x00008000, 0 },
+ /* OCRAM */
+ { 0x00900000, 0x00900000, 0x00020000, 0 },
+ /* OCRAM */
+ { 0x00920000, 0x00920000, 0x00020000, 0 },
+ /* QSPI Code - alias */
+ { 0x08000000, 0x08000000, 0x08000000, 0 },
+ /* DDR (Code) - alias */
+ { 0x10000000, 0x80000000, 0x0FFE0000, 0 },
+ /* TCML */
+ { 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN },
+ /* TCMU */
+ { 0x20000000, 0x00800000, 0x00020000, ATT_OWN },
+ /* OCRAM_S */
+ { 0x20180000, 0x00180000, 0x00008000, ATT_OWN },
+ /* OCRAM */
+ { 0x20200000, 0x00900000, 0x00020000, ATT_OWN },
+ /* OCRAM */
+ { 0x20220000, 0x00920000, 0x00020000, ATT_OWN },
+ /* DDR (Data) */
+ { 0x40000000, 0x40000000, 0x80000000, 0 },
+};
+
static const struct imx_rproc_att imx_rproc_att_imx7d[] = {
/* dev addr , sys addr , size , flags */
/* OCRAM_S (M4 Boot code) - alias */
@@ -140,6 +168,16 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
{ 0x80000000, 0x80000000, 0x60000000, 0 },
};
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
+ .src_reg = IMX7D_SRC_SCR,
+ .src_mask = IMX7D_M4_RST_MASK,
+ .src_start = IMX7D_M4_START,
+ .src_stop = IMX7D_M4_STOP,
+ .att = imx_rproc_att_imx8mq,
+ .att_size = ARRAY_SIZE(imx_rproc_att_imx8mq),
+ .elf_mem_hook = true,
+};
+
static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
.src_reg = IMX7D_SRC_SCR,
.src_mask = IMX7D_M4_RST_MASK,
@@ -517,6 +555,8 @@ static int imx_rproc_remove(struct platform_device *pdev)
static const struct of_device_id imx_rproc_of_match[] = {
{ .compatible = "fsl,imx7d-cm4", .data = &imx_rproc_cfg_imx7d },
{ .compatible = "fsl,imx6sx-cm4", .data = &imx_rproc_cfg_imx6sx },
+ { .compatible = "fsl,imx8mq-cm4", .data = &imx_rproc_cfg_imx8mq },
+ { .compatible = "fsl,imx8mm-cm4", .data = &imx_rproc_cfg_imx8mq },
{},
};
MODULE_DEVICE_TABLE(of, imx_rproc_of_match);
--
2.28.0
We might need to map an region multiple times, becaue the region might
be shared between remote processors, such i.MX8QM with dual M4 cores.
So use devm_ioremap, not devm_ioremap_resource.
Reviewed-by: Oleksij Rempel <[email protected]>
Reviewed-by: Richard Zhu <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/remoteproc/imx_rproc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index aa5fbd0c7768..48ce09e857ee 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -298,9 +298,10 @@ static int imx_rproc_addr_init(struct imx_rproc *priv,
if (b >= IMX7D_RPROC_MEM_MAX)
break;
- priv->mem[b].cpu_addr = devm_ioremap_resource(&pdev->dev, &res);
+ /* Not use resource version, because we might share region*/
+ priv->mem[b].cpu_addr = devm_ioremap(&pdev->dev, res.start, resource_size(&res));
if (IS_ERR(priv->mem[b].cpu_addr)) {
- dev_err(dev, "devm_ioremap_resource failed\n");
+ dev_err(dev, "devm_ioremap %pR failed\n", &res);
err = PTR_ERR(priv->mem[b].cpu_addr);
return err;
}
--
2.28.0
The hook is used to parse memory-regions and load resource table
from the address the remote processor published.
Reviewed-by: Richard Zhu <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/remoteproc/imx_rproc.c | 97 ++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 48ce09e857ee..bd3a42892b22 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -11,6 +11,7 @@
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of_address.h>
+#include <linux/of_reserved_mem.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -243,10 +244,106 @@ static void *imx_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
return va;
}
+static int imx_rproc_mem_alloc(struct rproc *rproc,
+ struct rproc_mem_entry *mem)
+{
+ struct device *dev = rproc->dev.parent;
+ void *va;
+
+ dev_dbg(dev, "map memory: %p+%zx\n", &mem->dma, mem->len);
+ va = ioremap_wc(mem->dma, mem->len);
+ if (IS_ERR_OR_NULL(va)) {
+ dev_err(dev, "Unable to map memory region: %p+%zx\n",
+ &mem->dma, mem->len);
+ return -ENOMEM;
+ }
+
+ /* Update memory entry va */
+ mem->va = va;
+
+ return 0;
+}
+
+static int imx_rproc_mem_release(struct rproc *rproc,
+ struct rproc_mem_entry *mem)
+{
+ dev_dbg(rproc->dev.parent, "unmap memory: %pa\n", &mem->dma);
+ iounmap(mem->va);
+
+ return 0;
+}
+
+static int imx_rproc_parse_memory_regions(struct rproc *rproc)
+{
+ struct imx_rproc *priv = rproc->priv;
+ struct device_node *np = priv->dev->of_node;
+ struct of_phandle_iterator it;
+ struct rproc_mem_entry *mem;
+ struct reserved_mem *rmem;
+ int index = 0;
+ u32 da;
+
+ /* Register associated reserved memory regions */
+ of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
+ while (of_phandle_iterator_next(&it) == 0) {
+ /*
+ * Ignore the first memory region which will be used vdev buffer.
+ * No need to do extra handlings, rproc_add_virtio_dev will handle it.
+ */
+ if (!index && !strcmp(it.node->name, "vdevbuffer")) {
+ index ++;
+ continue;
+ }
+
+ rmem = of_reserved_mem_lookup(it.node);
+ if (!rmem) {
+ dev_err(priv->dev, "unable to acquire memory-region\n");
+ return -EINVAL;
+ }
+
+ /* No need to translate pa to da, i.MX use same map */
+ da = rmem->base;
+
+ /* Register memory region */
+ mem = rproc_mem_entry_init(priv->dev, NULL, (dma_addr_t)rmem->base, rmem->size, da,
+ imx_rproc_mem_alloc, imx_rproc_mem_release,
+ it.node->name);
+
+ if (mem)
+ rproc_coredump_add_segment(rproc, da, rmem->size);
+ else
+ return -ENOMEM;
+
+ rproc_add_carveout(rproc, mem);
+ index++;
+ }
+
+ return 0;
+}
+
+static int imx_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
+{
+ int ret = imx_rproc_parse_memory_regions(rproc);
+
+ if (ret)
+ return ret;
+
+ ret = rproc_elf_load_rsc_table(rproc, fw);
+ if (ret)
+ dev_info(&rproc->dev, "No resource table in elf\n");
+
+ return 0;
+}
+
static const struct rproc_ops imx_rproc_ops = {
.start = imx_rproc_start,
.stop = imx_rproc_stop,
.da_to_va = imx_rproc_da_to_va,
+ .load = rproc_elf_load_segments,
+ .parse_fw = imx_rproc_parse_fw,
+ .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+ .sanity_check = rproc_elf_sanity_check,
+ .get_boot_addr = rproc_elf_get_boot_addr,
};
static int imx_rproc_addr_init(struct imx_rproc *priv,
--
2.28.0
Use virtio/mailbox to build connection between Remote Proccessors
and Linux. Add delayed work to handle incoming messages.
Reviewed-by: Richard Zhu <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/remoteproc/imx_rproc.c | 112 ++++++++++++++++++++++++++++++++-
1 file changed, 109 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 0f69f3f745ab..c514d7ca7c81 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -8,6 +8,7 @@
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/kernel.h>
+#include <linux/mailbox_client.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of_address.h>
@@ -17,6 +18,8 @@
#include <linux/regmap.h>
#include <linux/remoteproc.h>
+#include "remoteproc_internal.h"
+
#define IMX7D_SRC_SCR 0x0C
#define IMX7D_ENABLE_M4 BIT(3)
#define IMX7D_SW_M4P_RST BIT(2)
@@ -88,6 +91,10 @@ struct imx_rproc {
const struct imx_rproc_dcfg *dcfg;
struct imx_rproc_mem mem[IMX7D_RPROC_MEM_MAX];
struct clk *clk;
+ struct mbox_client cl;
+ struct mbox_chan *tx_ch;
+ struct mbox_chan *rx_ch;
+ struct delayed_work rproc_work;
};
static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
@@ -373,9 +380,30 @@ static int imx_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
return 0;
}
+static void imx_rproc_kick(struct rproc *rproc, int vqid)
+{
+ struct imx_rproc *priv = rproc->priv;
+ int err;
+ __u32 mmsg;
+
+ if (!priv->tx_ch) {
+ dev_err(priv->dev, "No initialized mbox tx channel\n");
+ return;
+ }
+
+ mmsg = vqid << 16;
+
+ priv->cl.tx_tout = 100;
+ err = mbox_send_message(priv->tx_ch, (void *)&mmsg);
+ if (err < 0)
+ dev_err(priv->dev, "%s: failed (%d, err:%d)\n",
+ __func__, vqid, err);
+}
+
static const struct rproc_ops imx_rproc_ops = {
.start = imx_rproc_start,
.stop = imx_rproc_stop,
+ .kick = imx_rproc_kick,
.da_to_va = imx_rproc_da_to_va,
.load = rproc_elf_load_segments,
.parse_fw = imx_rproc_parse_fw,
@@ -458,6 +486,70 @@ static void imx_rproc_memset(struct rproc *rproc, void *s, int c, size_t count)
memset_io((void * __iomem)s, c, count);
}
+static void imx_rproc_vq_work(struct work_struct *work)
+{
+ struct delayed_work *dwork = to_delayed_work(work);
+ struct imx_rproc *priv = container_of(dwork, struct imx_rproc,
+ rproc_work);
+
+ rproc_vq_interrupt(priv->rproc, 0);
+ rproc_vq_interrupt(priv->rproc, 1);
+}
+
+static void imx_rproc_rx_callback(struct mbox_client *cl, void *msg)
+{
+ struct rproc *rproc = dev_get_drvdata(cl->dev);
+ struct imx_rproc *priv = rproc->priv;
+
+ schedule_delayed_work(&(priv->rproc_work), 0);
+}
+
+static int imx_rproc_xtr_mbox_init(struct rproc *rproc)
+{
+ struct imx_rproc *priv = rproc->priv;
+ struct device *dev = priv->dev;
+ struct mbox_client *cl;
+ int ret = 0;
+
+ if (!of_get_property(dev->of_node, "mbox-names", NULL))
+ return 0;
+
+ cl = &priv->cl;
+ cl->dev = dev;
+ cl->tx_block = true;
+ cl->tx_tout = 20;
+ cl->knows_txdone = false;
+ cl->rx_callback = imx_rproc_rx_callback;
+
+ priv->tx_ch = mbox_request_channel_byname(cl, "tx");
+ if (IS_ERR(priv->tx_ch)) {
+ if (PTR_ERR(priv->tx_ch) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ ret = PTR_ERR(priv->tx_ch);
+ dev_dbg(cl->dev, "failed to request mbox tx chan, ret %d\n",
+ ret);
+ goto err_out;
+ }
+
+ priv->rx_ch = mbox_request_channel_byname(cl, "rx");
+ if (IS_ERR(priv->rx_ch)) {
+ ret = PTR_ERR(priv->rx_ch);
+ dev_dbg(cl->dev, "failed to request mbox rx chan, ret %d\n",
+ ret);
+ goto err_out;
+ }
+
+ return ret;
+
+err_out:
+ if (!IS_ERR(priv->tx_ch))
+ mbox_free_channel(priv->tx_ch);
+ if (!IS_ERR(priv->rx_ch))
+ mbox_free_channel(priv->rx_ch);
+
+ return ret;
+}
+
static int imx_rproc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -501,17 +593,24 @@ static int imx_rproc_probe(struct platform_device *pdev)
dev_set_drvdata(dev, rproc);
+ ret = imx_rproc_xtr_mbox_init(rproc);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
+ goto err_put_rproc;
+ /* mbox is optional, so not fail here */
+ }
+
ret = imx_rproc_addr_init(priv, pdev);
if (ret) {
dev_err(dev, "failed on imx_rproc_addr_init\n");
- goto err_put_rproc;
+ goto err_put_mbox;
}
priv->clk = devm_clk_get(dev, NULL);
if (IS_ERR(priv->clk)) {
dev_err(dev, "Failed to get clock\n");
ret = PTR_ERR(priv->clk);
- goto err_put_rproc;
+ goto err_put_mbox;
}
/*
@@ -521,9 +620,11 @@ static int imx_rproc_probe(struct platform_device *pdev)
ret = clk_prepare_enable(priv->clk);
if (ret) {
dev_err(&rproc->dev, "Failed to enable clock\n");
- goto err_put_rproc;
+ goto err_put_mbox;
}
+ INIT_DELAYED_WORK(&(priv->rproc_work), imx_rproc_vq_work);
+
ret = rproc_add(rproc);
if (ret) {
dev_err(dev, "rproc_add failed\n");
@@ -534,6 +635,11 @@ static int imx_rproc_probe(struct platform_device *pdev)
err_put_clk:
clk_disable_unprepare(priv->clk);
+err_put_mbox:
+ if (!IS_ERR(priv->tx_ch))
+ mbox_free_channel(priv->tx_ch);
+ if (!IS_ERR(priv->rx_ch))
+ mbox_free_channel(priv->rx_ch);
err_put_rproc:
rproc_free(rproc);
--
2.28.0
Mathieu, Oleksij
> Subject: [PATCH V2 0/7] remoteproc: imx_rproc: support iMX8MQ/M
Do you have time to give a look at this patchset?
Thanks,
Peng.
>
> V2:
> Rebased on linux-next
> Dropped early boot feature to make patchset simple.
> Drop rsc-da
>
> V1:
> https://patchwork.kernel.org/cover/11682461/
>
> This patchset is to support i.MX8MQ/M coproc.
> The early boot feature was dropped to make the patchset small in V2.
>
> Since i.MX specific TCM memory requirement, add elf platform hook.
> Several patches have got reviewed by Oleksij and Mathieu in v1.
>
> Peng Fan (7):
> remoteproc: elf: support platform specific memory hook
> remoteproc: imx_rproc: add elf memory hooks
> remoteproc: imx_rproc: correct err message
> remoteproc: imx_rproc: use devm_ioremap
> remoteproc: imx_rproc: add i.MX specific parse fw hook
> remoteproc: imx_rproc: support i.MX8MQ/M
> remoteproc: imx_proc: enable virtio/mailbox
>
> drivers/remoteproc/imx_rproc.c | 273
> ++++++++++++++++++++-
> drivers/remoteproc/remoteproc_elf_loader.c | 20 +-
> include/linux/remoteproc.h | 2 +
> 3 files changed, 287 insertions(+), 8 deletions(-)
>
> --
> 2.28.0
On Wed, 7 Oct 2020 at 18:52, Peng Fan <[email protected]> wrote:
>
> Mathieu, Oleksij
>
> > Subject: [PATCH V2 0/7] remoteproc: imx_rproc: support iMX8MQ/M
>
> Do you have time to give a look at this patchset?
I will review your patchset after you have reviewed mine[1].
[1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=339079
>
> Thanks,
> Peng.
>
> >
> > V2:
> > Rebased on linux-next
> > Dropped early boot feature to make patchset simple.
> > Drop rsc-da
> >
> > V1:
> > https://patchwork.kernel.org/cover/11682461/
> >
> > This patchset is to support i.MX8MQ/M coproc.
> > The early boot feature was dropped to make the patchset small in V2.
> >
> > Since i.MX specific TCM memory requirement, add elf platform hook.
> > Several patches have got reviewed by Oleksij and Mathieu in v1.
> >
> > Peng Fan (7):
> > remoteproc: elf: support platform specific memory hook
> > remoteproc: imx_rproc: add elf memory hooks
> > remoteproc: imx_rproc: correct err message
> > remoteproc: imx_rproc: use devm_ioremap
> > remoteproc: imx_rproc: add i.MX specific parse fw hook
> > remoteproc: imx_rproc: support i.MX8MQ/M
> > remoteproc: imx_proc: enable virtio/mailbox
> >
> > drivers/remoteproc/imx_rproc.c | 273
> > ++++++++++++++++++++-
> > drivers/remoteproc/remoteproc_elf_loader.c | 20 +-
> > include/linux/remoteproc.h | 2 +
> > 3 files changed, 287 insertions(+), 8 deletions(-)
> >
> > --
> > 2.28.0
>
> Subject: Re: [PATCH V2 0/7] remoteproc: imx_rproc: support iMX8MQ/M
>
> On Wed, 7 Oct 2020 at 18:52, Peng Fan <[email protected]> wrote:
> >
> > Mathieu, Oleksij
> >
> > > Subject: [PATCH V2 0/7] remoteproc: imx_rproc: support iMX8MQ/M
> >
> > Do you have time to give a look at this patchset?
>
> I will review your patchset after you have reviewed mine[1].
Fair enough:)
I not follow the mailing list tightly, will read your patchset soon.
Thanks,
Peng.
>
> [1].
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.kernel.org%2Fproject%2Flinux-remoteproc%2Flist%2F%3Fseries%3D33
> 9079&data=02%7C01%7Cpeng.fan%40nxp.com%7C7ed3771fc6254908c
> 7e308d86b981d7b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
> 637377648356263771&sdata=Po67f7yqLuoTSZ6lcALKqoxt0Wpwxk6MW
> qNiPx%2B%2BVX8%3D&reserved=0
>
> >
> > Thanks,
> > Peng.
> >
> > >
> > > V2:
> > > Rebased on linux-next
> > > Dropped early boot feature to make patchset simple.
> > > Drop rsc-da
> > >
> > > V1:
> > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.kernel.org%2Fcover%2F11682461%2F&data=02%7C01%7Cpeng.fa
> n%40nxp.com%7C7ed3771fc6254908c7e308d86b981d7b%7C686ea1d3bc2b
> 4c6fa92cd99c5c301635%7C0%7C0%7C637377648356263771&sdata=AI
> XXzRj%2F8mv%2BLLOIB1yIQHbDJ7myt%2BN3bJYrunPALms%3D&reserv
> ed=0
> > >
> > > This patchset is to support i.MX8MQ/M coproc.
> > > The early boot feature was dropped to make the patchset small in V2.
> > >
> > > Since i.MX specific TCM memory requirement, add elf platform hook.
> > > Several patches have got reviewed by Oleksij and Mathieu in v1.
> > >
> > > Peng Fan (7):
> > > remoteproc: elf: support platform specific memory hook
> > > remoteproc: imx_rproc: add elf memory hooks
> > > remoteproc: imx_rproc: correct err message
> > > remoteproc: imx_rproc: use devm_ioremap
> > > remoteproc: imx_rproc: add i.MX specific parse fw hook
> > > remoteproc: imx_rproc: support i.MX8MQ/M
> > > remoteproc: imx_proc: enable virtio/mailbox
> > >
> > > drivers/remoteproc/imx_rproc.c | 273
> > > ++++++++++++++++++++-
> > > drivers/remoteproc/remoteproc_elf_loader.c | 20 +-
> > > include/linux/remoteproc.h | 2 +
> > > 3 files changed, 287 insertions(+), 8 deletions(-)
> > >
> > > --
> > > 2.28.0
> >
Hi Peng,
On Sun, Sep 27, 2020 at 02:41:25PM +0800, Peng Fan wrote:
> To arm64, "dc zva, dst" is used in memset.
> Per ARM DDI 0487A.j, chapter C5.3.8 DC ZVA, Data Cache Zero by VA,
>
> "If the memory region being zeroed is any type of Device memory,
> this instruction can give an alignment fault which is prioritized
> in the same way as other alignment faults that are determined
> by the memory type."
>
> On i.MX platforms, when elf is loaded to onchip TCM area, the region
> is ioremapped, so "dc zva, dst" will trigger abort. And ioremap_wc()
> on i.MX not able to write correct data to TCM area.
>
> So we need to use io helpers, and extend the elf loader to support
> platform specific memory functions.
>
> Acked-by: Richard Zhu <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/remoteproc/remoteproc_elf_loader.c | 20 ++++++++++++++++++--
> include/linux/remoteproc.h | 2 ++
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
> index df68d87752e4..6cb71fe47261 100644
> --- a/drivers/remoteproc/remoteproc_elf_loader.c
> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> @@ -129,6 +129,22 @@ u64 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
> }
> EXPORT_SYMBOL(rproc_elf_get_boot_addr);
>
> +static void rproc_elf_memcpy(struct rproc *rproc, void *dest, const void *src, size_t count)
> +{
> + if (!rproc->ops->elf_memcpy)
> + memcpy(dest, src, count);
> +
> + rproc->ops->elf_memcpy(rproc, dest, src, count);
> +}
> +
> +static void rproc_elf_memset(struct rproc *rproc, void *s, int c, size_t count)
> +{
> + if (!rproc->ops->elf_memset)
> + memset(s, c, count);
> +
> + rproc->ops->elf_memset(rproc, s, c, count);
> +}
> +
> /**
> * rproc_elf_load_segments() - load firmware segments to memory
> * @rproc: remote processor which will be booted using these fw segments
> @@ -214,7 +230,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
>
> /* put the segment where the remote processor expects it */
> if (filesz)
> - memcpy(ptr, elf_data + offset, filesz);
> + rproc_elf_memcpy(rproc, ptr, elf_data + offset, filesz);
>
> /*
> * Zero out remaining memory for this segment.
> @@ -224,7 +240,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
> * this.
> */
> if (memsz > filesz)
> - memset(ptr + filesz, 0, memsz - filesz);
> + rproc_elf_memset(rproc, ptr + filesz, 0, memsz - filesz);
> }
>
> return ret;
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 2fa68bf5aa4f..1f5fa2c772df 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -392,6 +392,8 @@ struct rproc_ops {
> int (*load)(struct rproc *rproc, const struct firmware *fw);
> int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
> u64 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
> + void (*elf_memcpy)(struct rproc *rproc, void *dest, const void *src, size_t count);
> + void (*elf_memset)(struct rproc *rproc, void *s, int c, size_t count);
As with every other operations, the above two addition should be documented.
With that:
Reviewed-by: Mathieu Poirier <[email protected]>
> unsigned long (*panic)(struct rproc *rproc);
> };
>
> --
> 2.28.0
>
On Sun, Sep 27, 2020 at 02:41:29PM +0800, Peng Fan wrote:
> The hook is used to parse memory-regions and load resource table
> from the address the remote processor published.
>
> Reviewed-by: Richard Zhu <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/remoteproc/imx_rproc.c | 97 ++++++++++++++++++++++++++++++++++
> 1 file changed, 97 insertions(+)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 48ce09e857ee..bd3a42892b22 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -11,6 +11,7 @@
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of_address.h>
> +#include <linux/of_reserved_mem.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> @@ -243,10 +244,106 @@ static void *imx_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
> return va;
> }
>
> +static int imx_rproc_mem_alloc(struct rproc *rproc,
> + struct rproc_mem_entry *mem)
> +{
> + struct device *dev = rproc->dev.parent;
> + void *va;
> +
> + dev_dbg(dev, "map memory: %p+%zx\n", &mem->dma, mem->len);
> + va = ioremap_wc(mem->dma, mem->len);
> + if (IS_ERR_OR_NULL(va)) {
> + dev_err(dev, "Unable to map memory region: %p+%zx\n",
> + &mem->dma, mem->len);
> + return -ENOMEM;
> + }
> +
> + /* Update memory entry va */
> + mem->va = va;
> +
> + return 0;
> +}
> +
> +static int imx_rproc_mem_release(struct rproc *rproc,
> + struct rproc_mem_entry *mem)
> +{
> + dev_dbg(rproc->dev.parent, "unmap memory: %pa\n", &mem->dma);
> + iounmap(mem->va);
> +
> + return 0;
> +}
> +
> +static int imx_rproc_parse_memory_regions(struct rproc *rproc)
> +{
> + struct imx_rproc *priv = rproc->priv;
> + struct device_node *np = priv->dev->of_node;
> + struct of_phandle_iterator it;
> + struct rproc_mem_entry *mem;
> + struct reserved_mem *rmem;
> + int index = 0;
> + u32 da;
> +
> + /* Register associated reserved memory regions */
> + of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
> + while (of_phandle_iterator_next(&it) == 0) {
> + /*
> + * Ignore the first memory region which will be used vdev buffer.
> + * No need to do extra handlings, rproc_add_virtio_dev will handle it.
> + */
> + if (!index && !strcmp(it.node->name, "vdevbuffer")) {
> + index ++;
How many vdevs is there in your scenario? Since most of this code is taken from
stm32 anyway I would suggest to use "vdev0buffer" and get rid of the "index ++". It
adds needless complexity and doesn't pass the checkpatch test.
With the above:
Reviewed-by: Mathieu Poirier <[email protected]>
> + continue;
> + }
> +
> + rmem = of_reserved_mem_lookup(it.node);
> + if (!rmem) {
> + dev_err(priv->dev, "unable to acquire memory-region\n");
> + return -EINVAL;
> + }
> +
> + /* No need to translate pa to da, i.MX use same map */
> + da = rmem->base;
> +
> + /* Register memory region */
> + mem = rproc_mem_entry_init(priv->dev, NULL, (dma_addr_t)rmem->base, rmem->size, da,
> + imx_rproc_mem_alloc, imx_rproc_mem_release,
> + it.node->name);
> +
> + if (mem)
> + rproc_coredump_add_segment(rproc, da, rmem->size);
> + else
> + return -ENOMEM;
> +
> + rproc_add_carveout(rproc, mem);
> + index++;
> + }
> +
> + return 0;
> +}
> +
> +static int imx_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
> +{
> + int ret = imx_rproc_parse_memory_regions(rproc);
> +
> + if (ret)
> + return ret;
> +
> + ret = rproc_elf_load_rsc_table(rproc, fw);
> + if (ret)
> + dev_info(&rproc->dev, "No resource table in elf\n");
> +
> + return 0;
> +}
> +
> static const struct rproc_ops imx_rproc_ops = {
> .start = imx_rproc_start,
> .stop = imx_rproc_stop,
> .da_to_va = imx_rproc_da_to_va,
> + .load = rproc_elf_load_segments,
> + .parse_fw = imx_rproc_parse_fw,
> + .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> + .sanity_check = rproc_elf_sanity_check,
> + .get_boot_addr = rproc_elf_get_boot_addr,
> };
>
> static int imx_rproc_addr_init(struct imx_rproc *priv,
> --
> 2.28.0
>
On Sun, Sep 27, 2020 at 02:41:30PM +0800, Peng Fan wrote:
> Add i.MX8MQ dev/sys addr map and configuration data structure
> i.MX8MM share i.MX8MQ settings.
>
> Reviewed-by: Richard Zhu <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/remoteproc/imx_rproc.c | 40 ++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index bd3a42892b22..0f69f3f745ab 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -90,6 +90,34 @@ struct imx_rproc {
> struct clk *clk;
> };
>
> +static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
> + /* dev addr , sys addr , size , flags */
> + /* TCML - alias */
> + { 0x00000000, 0x007e0000, 0x00020000, 0 },
> + /* OCRAM_S */
> + { 0x00180000, 0x00180000, 0x00008000, 0 },
> + /* OCRAM */
> + { 0x00900000, 0x00900000, 0x00020000, 0 },
> + /* OCRAM */
> + { 0x00920000, 0x00920000, 0x00020000, 0 },
> + /* QSPI Code - alias */
> + { 0x08000000, 0x08000000, 0x08000000, 0 },
> + /* DDR (Code) - alias */
> + { 0x10000000, 0x80000000, 0x0FFE0000, 0 },
> + /* TCML */
> + { 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN },
> + /* TCMU */
> + { 0x20000000, 0x00800000, 0x00020000, ATT_OWN },
> + /* OCRAM_S */
> + { 0x20180000, 0x00180000, 0x00008000, ATT_OWN },
> + /* OCRAM */
> + { 0x20200000, 0x00900000, 0x00020000, ATT_OWN },
> + /* OCRAM */
> + { 0x20220000, 0x00920000, 0x00020000, ATT_OWN },
> + /* DDR (Data) */
> + { 0x40000000, 0x40000000, 0x80000000, 0 },
> +};
> +
> static const struct imx_rproc_att imx_rproc_att_imx7d[] = {
> /* dev addr , sys addr , size , flags */
> /* OCRAM_S (M4 Boot code) - alias */
> @@ -140,6 +168,16 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
> { 0x80000000, 0x80000000, 0x60000000, 0 },
> };
>
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
> + .src_reg = IMX7D_SRC_SCR,
> + .src_mask = IMX7D_M4_RST_MASK,
> + .src_start = IMX7D_M4_START,
> + .src_stop = IMX7D_M4_STOP,
> + .att = imx_rproc_att_imx8mq,
> + .att_size = ARRAY_SIZE(imx_rproc_att_imx8mq),
> + .elf_mem_hook = true,
> +};
> +
> static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
> .src_reg = IMX7D_SRC_SCR,
> .src_mask = IMX7D_M4_RST_MASK,
> @@ -517,6 +555,8 @@ static int imx_rproc_remove(struct platform_device *pdev)
> static const struct of_device_id imx_rproc_of_match[] = {
> { .compatible = "fsl,imx7d-cm4", .data = &imx_rproc_cfg_imx7d },
> { .compatible = "fsl,imx6sx-cm4", .data = &imx_rproc_cfg_imx6sx },
> + { .compatible = "fsl,imx8mq-cm4", .data = &imx_rproc_cfg_imx8mq },
> + { .compatible = "fsl,imx8mm-cm4", .data = &imx_rproc_cfg_imx8mq },
Reviewed-by: Mathieu Poirier <[email protected]>
> {},
> };
> MODULE_DEVICE_TABLE(of, imx_rproc_of_match);
> --
> 2.28.0
>
On Sun, Sep 27, 2020 at 02:41:28PM +0800, Peng Fan wrote:
> We might need to map an region multiple times, becaue the region might
> be shared between remote processors, such i.MX8QM with dual M4 cores.
> So use devm_ioremap, not devm_ioremap_resource.
>
> Reviewed-by: Oleksij Rempel <[email protected]>
> Reviewed-by: Richard Zhu <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/remoteproc/imx_rproc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index aa5fbd0c7768..48ce09e857ee 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -298,9 +298,10 @@ static int imx_rproc_addr_init(struct imx_rproc *priv,
> if (b >= IMX7D_RPROC_MEM_MAX)
> break;
>
> - priv->mem[b].cpu_addr = devm_ioremap_resource(&pdev->dev, &res);
> + /* Not use resource version, because we might share region*/
s/"region*"/"region *"
> + priv->mem[b].cpu_addr = devm_ioremap(&pdev->dev, res.start, resource_size(&res));
> if (IS_ERR(priv->mem[b].cpu_addr)) {
> - dev_err(dev, "devm_ioremap_resource failed\n");
> + dev_err(dev, "devm_ioremap %pR failed\n", &res);
> err = PTR_ERR(priv->mem[b].cpu_addr);
> return err;
> }
> --
> 2.28.0
>
On Sun, Sep 27, 2020 at 02:41:31PM +0800, Peng Fan wrote:
> Use virtio/mailbox to build connection between Remote Proccessors
> and Linux. Add delayed work to handle incoming messages.
>
> Reviewed-by: Richard Zhu <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/remoteproc/imx_rproc.c | 112 ++++++++++++++++++++++++++++++++-
> 1 file changed, 109 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 0f69f3f745ab..c514d7ca7c81 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -8,6 +8,7 @@
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/kernel.h>
> +#include <linux/mailbox_client.h>
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of_address.h>
> @@ -17,6 +18,8 @@
> #include <linux/regmap.h>
> #include <linux/remoteproc.h>
>
> +#include "remoteproc_internal.h"
> +
> #define IMX7D_SRC_SCR 0x0C
> #define IMX7D_ENABLE_M4 BIT(3)
> #define IMX7D_SW_M4P_RST BIT(2)
> @@ -88,6 +91,10 @@ struct imx_rproc {
> const struct imx_rproc_dcfg *dcfg;
> struct imx_rproc_mem mem[IMX7D_RPROC_MEM_MAX];
> struct clk *clk;
> + struct mbox_client cl;
> + struct mbox_chan *tx_ch;
> + struct mbox_chan *rx_ch;
> + struct delayed_work rproc_work;
> };
>
> static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
> @@ -373,9 +380,30 @@ static int imx_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
> return 0;
> }
>
> +static void imx_rproc_kick(struct rproc *rproc, int vqid)
> +{
> + struct imx_rproc *priv = rproc->priv;
> + int err;
> + __u32 mmsg;
> +
> + if (!priv->tx_ch) {
> + dev_err(priv->dev, "No initialized mbox tx channel\n");
> + return;
> + }
> +
> + mmsg = vqid << 16;
Please add a comment that explains the reason for the shift left.
> +
> + priv->cl.tx_tout = 100;
See my comment below on this.
> + err = mbox_send_message(priv->tx_ch, (void *)&mmsg);
> + if (err < 0)
> + dev_err(priv->dev, "%s: failed (%d, err:%d)\n",
> + __func__, vqid, err);
> +}
> +
> static const struct rproc_ops imx_rproc_ops = {
> .start = imx_rproc_start,
> .stop = imx_rproc_stop,
> + .kick = imx_rproc_kick,
> .da_to_va = imx_rproc_da_to_va,
> .load = rproc_elf_load_segments,
> .parse_fw = imx_rproc_parse_fw,
> @@ -458,6 +486,70 @@ static void imx_rproc_memset(struct rproc *rproc, void *s, int c, size_t count)
> memset_io((void * __iomem)s, c, count);
> }
>
> +static void imx_rproc_vq_work(struct work_struct *work)
> +{
> + struct delayed_work *dwork = to_delayed_work(work);
> + struct imx_rproc *priv = container_of(dwork, struct imx_rproc,
> + rproc_work);
> +
> + rproc_vq_interrupt(priv->rproc, 0);
> + rproc_vq_interrupt(priv->rproc, 1);
> +}
> +
> +static void imx_rproc_rx_callback(struct mbox_client *cl, void *msg)
> +{
> + struct rproc *rproc = dev_get_drvdata(cl->dev);
> + struct imx_rproc *priv = rproc->priv;
> +
> + schedule_delayed_work(&(priv->rproc_work), 0);
What is the advantage of using a struct delayed_work if there is no delay? And
since schedule_delayed_work() is using the system_wq, you could simply declare a
struct work_struct and call queue_work() on it.
> +}
> +
> +static int imx_rproc_xtr_mbox_init(struct rproc *rproc)
> +{
> + struct imx_rproc *priv = rproc->priv;
> + struct device *dev = priv->dev;
> + struct mbox_client *cl;
> + int ret = 0;
> +
> + if (!of_get_property(dev->of_node, "mbox-names", NULL))
> + return 0;
> +
> + cl = &priv->cl;
> + cl->dev = dev;
> + cl->tx_block = true;
> + cl->tx_tout = 20;
What is the point of setting this here when it is set again in imx_rproc_kick()
to 100?
> + cl->knows_txdone = false;
> + cl->rx_callback = imx_rproc_rx_callback;
> +
> + priv->tx_ch = mbox_request_channel_byname(cl, "tx");
> + if (IS_ERR(priv->tx_ch)) {
> + if (PTR_ERR(priv->tx_ch) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + ret = PTR_ERR(priv->tx_ch);
> + dev_dbg(cl->dev, "failed to request mbox tx chan, ret %d\n",
> + ret);
> + goto err_out;
> + }
> +
> + priv->rx_ch = mbox_request_channel_byname(cl, "rx");
> + if (IS_ERR(priv->rx_ch)) {
> + ret = PTR_ERR(priv->rx_ch);
> + dev_dbg(cl->dev, "failed to request mbox rx chan, ret %d\n",
> + ret);
> + goto err_out;
> + }
Where and when are the channels freed?
Thanks,
Mathieu
> +
> + return ret;
> +
> +err_out:
> + if (!IS_ERR(priv->tx_ch))
> + mbox_free_channel(priv->tx_ch);
> + if (!IS_ERR(priv->rx_ch))
> + mbox_free_channel(priv->rx_ch);
> +
> + return ret;
> +}
> +
> static int imx_rproc_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -501,17 +593,24 @@ static int imx_rproc_probe(struct platform_device *pdev)
>
> dev_set_drvdata(dev, rproc);
>
> + ret = imx_rproc_xtr_mbox_init(rproc);
> + if (ret) {
> + if (ret == -EPROBE_DEFER)
> + goto err_put_rproc;
> + /* mbox is optional, so not fail here */
> + }
> +
> ret = imx_rproc_addr_init(priv, pdev);
> if (ret) {
> dev_err(dev, "failed on imx_rproc_addr_init\n");
> - goto err_put_rproc;
> + goto err_put_mbox;
> }
>
> priv->clk = devm_clk_get(dev, NULL);
> if (IS_ERR(priv->clk)) {
> dev_err(dev, "Failed to get clock\n");
> ret = PTR_ERR(priv->clk);
> - goto err_put_rproc;
> + goto err_put_mbox;
> }
>
> /*
> @@ -521,9 +620,11 @@ static int imx_rproc_probe(struct platform_device *pdev)
> ret = clk_prepare_enable(priv->clk);
> if (ret) {
> dev_err(&rproc->dev, "Failed to enable clock\n");
> - goto err_put_rproc;
> + goto err_put_mbox;
> }
>
> + INIT_DELAYED_WORK(&(priv->rproc_work), imx_rproc_vq_work);
> +
> ret = rproc_add(rproc);
> if (ret) {
> dev_err(dev, "rproc_add failed\n");
> @@ -534,6 +635,11 @@ static int imx_rproc_probe(struct platform_device *pdev)
>
> err_put_clk:
> clk_disable_unprepare(priv->clk);
> +err_put_mbox:
> + if (!IS_ERR(priv->tx_ch))
> + mbox_free_channel(priv->tx_ch);
> + if (!IS_ERR(priv->rx_ch))
> + mbox_free_channel(priv->rx_ch);
> err_put_rproc:
> rproc_free(rproc);
>
> --
> 2.28.0
>