This patchset is to support i.MX8MQ/M coproc booted before linux.
Since i.MX8MQ/M was not supported, several patches are needed
to first support the platform, then support early boot case.
I intended to included i.MX8QM/QXP, but that would introduce a large
patchset, so not included. But the clk/syscon optional patch for
i.MX8QM/QXP was still kept here to avoid rebase error.
Peng Fan (10):
dt-bindings: remoteproc: imx_rproc: add i.MX8MQ/M
remoteproc: imx_rproc: correct err message
remoteproc: imx: use devm_ioremap
remoteproc: imx_rproc: make syscon optional
remoteproc: imx_rproc: make clk optional
remoteproc: imx_rproc: add load hook
remoteproc: imx_rproc: add i.MX specific parse fw hook
remoteproc: imx_rproc: support i.MX8MQ/M
remoteproc: imx_proc: enable virtio/mailbox
remoteproc: imx_rproc: support coproc booting before Linux
.../devicetree/bindings/remoteproc/imx-rproc.txt | 3 +
drivers/remoteproc/imx_rproc.c | 409 ++++++++++++++++++++-
2 files changed, 401 insertions(+), 11 deletions(-)
--
2.16.4
Detect Coproc booted or not and Parse resource table
Set remoteproc state to RPROC_DETACHED when M4 is booted early
Add attach hook
Signed-off-by: Peng Fan <[email protected]>
---
drivers/remoteproc/imx_rproc.c | 75 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 73 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index a8ce97c04e1e..0863b3162777 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -91,6 +91,7 @@ struct imx_rproc {
const struct imx_rproc_dcfg *dcfg;
struct imx_rproc_mem mem[IMX7D_RPROC_MEM_MAX];
struct clk *clk;
+ bool early_boot;
void *rsc_va;
struct mbox_client cl;
struct mbox_chan *tx_ch;
@@ -235,6 +236,8 @@ static int imx_rproc_stop(struct rproc *rproc)
dcfg->src_mask, dcfg->src_stop);
if (ret)
dev_err(dev, "Failed to stop M4!\n");
+ else
+ priv->early_boot = false;
return ret;
}
@@ -390,6 +393,32 @@ static int imx_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
return 0;
}
+static int imx_rproc_get_loaded_rsc_table(struct device *dev,
+ struct rproc *rproc)
+{
+ struct imx_rproc *priv = rproc->priv;
+ struct device_node *np = dev->of_node;
+ u32 da;
+ int ret;
+
+ ret = of_property_read_u32(np, "rsc-da", &da);
+ if (!ret)
+ priv->rsc_va = rproc_da_to_va(rproc, (u64)da, SZ_1K);
+ else
+ return 0;
+
+ if (!priv->rsc_va) {
+ dev_err(priv->dev, "no map for rsc-da: %x\n", da);
+ return PTR_ERR(priv->rsc_va);
+ }
+
+ rproc->table_ptr = (struct resource_table *)priv->rsc_va;
+ rproc->table_sz = SZ_1K;
+ rproc->cached_table = NULL;
+
+ return 0;
+}
+
static int imx_rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
{
struct device *dev = &rproc->dev;
@@ -472,9 +501,15 @@ static void imx_rproc_kick(struct rproc *rproc, int vqid)
__func__, vqid, err);
}
+static int imx_rproc_attach(struct rproc *rproc)
+{
+ return 0;
+}
+
static const struct rproc_ops imx_rproc_ops = {
.start = imx_rproc_start,
.stop = imx_rproc_stop,
+ .attach = imx_rproc_attach,
.kick = imx_rproc_kick,
.da_to_va = imx_rproc_da_to_va,
.load = imx_rproc_elf_load_segments,
@@ -609,6 +644,36 @@ static int imx_rproc_xtr_mbox_init(struct rproc *rproc)
return ret;
}
+static int imx_rproc_detect_mode(struct imx_rproc *priv)
+{
+ const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+ struct device *dev = priv->dev;
+ int ret;
+ u32 val;
+
+ ret = regmap_read(priv->regmap, dcfg->src_reg, &val);
+ if (ret) {
+ dev_err(dev, "Failed to read src\n");
+ return ret;
+ }
+
+ priv->early_boot = !(val & dcfg->src_stop);
+
+ if (priv->early_boot) {
+ priv->rproc->state = RPROC_DETACHED;
+
+ ret = imx_rproc_parse_memory_regions(priv->rproc);
+ if (ret)
+ return ret;
+
+ ret = imx_rproc_get_loaded_rsc_table(dev, priv->rproc);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int imx_rproc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -661,6 +726,10 @@ static int imx_rproc_probe(struct platform_device *pdev)
goto err_put_mbox;
}
+ ret = imx_rproc_detect_mode(priv);
+ if (ret)
+ goto err_put_mbox;
+
priv->clk = devm_clk_get_optional(dev, NULL);
if (IS_ERR(priv->clk)) {
dev_err(dev, "Failed to get clock\n");
@@ -689,7 +758,8 @@ static int imx_rproc_probe(struct platform_device *pdev)
return 0;
err_put_clk:
- clk_disable_unprepare(priv->clk);
+ if (!priv->early_boot)
+ clk_disable_unprepare(priv->clk);
err_put_mbox:
if (!IS_ERR(priv->tx_ch))
mbox_free_channel(priv->tx_ch);
@@ -706,7 +776,8 @@ static int imx_rproc_remove(struct platform_device *pdev)
struct rproc *rproc = platform_get_drvdata(pdev);
struct imx_rproc *priv = rproc->priv;
- clk_disable_unprepare(priv->clk);
+ if (!priv->early_boot)
+ clk_disable_unprepare(priv->clk);
rproc_del(rproc);
rproc_free(rproc);
--
2.16.4
Hi,
On Fri, Jul 24, 2020 at 04:08:03PM +0800, Peng Fan wrote:
> This patchset is to support i.MX8MQ/M coproc booted before linux.
> Since i.MX8MQ/M was not supported, several patches are needed
> to first support the platform, then support early boot case.
>
> I intended to included i.MX8QM/QXP, but that would introduce a large
> patchset, so not included. But the clk/syscon optional patch for
> i.MX8QM/QXP was still kept here to avoid rebase error.
Thank you for your work.
Can you please provide more information about big picture of this work.
If I see it correctly, we have here support for i.MX8MM, which seems to
be able to fully control Cortex M4 (enable CPU core, etc...).
And other case, where remoteproc is running on application processor and
can't or should not touch M4 (i.MX7ULP, i.MX8QM/QXP..). Since M4
provides some functionality, you are trying to reuse remoteproc
framework to get resource table present in ELF header and to dynamically
load things. For some reasons this header provides more information then
needed, so you are changing the ELF parser in the kernel to workaround
it.
Correct?
> Peng Fan (10):
> dt-bindings: remoteproc: imx_rproc: add i.MX8MQ/M
> remoteproc: imx_rproc: correct err message
> remoteproc: imx: use devm_ioremap
> remoteproc: imx_rproc: make syscon optional
> remoteproc: imx_rproc: make clk optional
> remoteproc: imx_rproc: add load hook
> remoteproc: imx_rproc: add i.MX specific parse fw hook
> remoteproc: imx_rproc: support i.MX8MQ/M
> remoteproc: imx_proc: enable virtio/mailbox
> remoteproc: imx_rproc: support coproc booting before Linux
>
> .../devicetree/bindings/remoteproc/imx-rproc.txt | 3 +
> drivers/remoteproc/imx_rproc.c | 409 ++++++++++++++++++++-
> 2 files changed, 401 insertions(+), 11 deletions(-)
>
> --
> 2.16.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Oleksij,
> Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and early
> boot
>
> Hi,
>
> On Fri, Jul 24, 2020 at 04:08:03PM +0800, Peng Fan wrote:
> > This patchset is to support i.MX8MQ/M coproc booted before linux.
> > Since i.MX8MQ/M was not supported, several patches are needed to first
> > support the platform, then support early boot case.
> >
> > I intended to included i.MX8QM/QXP, but that would introduce a large
> > patchset, so not included. But the clk/syscon optional patch for
> > i.MX8QM/QXP was still kept here to avoid rebase error.
>
> Thank you for your work.
>
> Can you please provide more information about big picture of this work.
>
> If I see it correctly, we have here support for i.MX8MM, which seems to be
> able to fully control Cortex M4 (enable CPU core, etc...).
Yes.
>
> And other case, where remoteproc is running on application processor and
> can't or should not touch M4 (i.MX7ULP, i.MX8QM/QXP..). Since M4 provides
> some functionality, you are trying to reuse remoteproc framework to get
> resource table present in ELF header and to dynamically load things. For some
> reasons this header provides more information then needed, so you are
> changing the ELF parser in the kernel to workaround it.
Not exactly.
For i.MX8MM, we support two cases. M4 kicked by U-Boot, M4 kicked by Linux remoteproc.
For i.MX8QM/QXP, the typical usecase is M4 kicked by SCFW, but we will also add M4 kicked
by Linux remoteproc.
For i.MX7ULP, I would only support M4 dual boot case, M4 control everything.
The reason the change the elf parser is that when M4 elf is loaded by Linux remoteproc,
It use memset to clear area. However we use ioremap, memset on ARM64 will report
crash to device nGnRE memory. And we could not use ioremap_wc to TCM area, since
it could have data correctly written into TCM.
Maintainer not wanna to drop memset in common code, and TI guys suggest
add i.MX specific elf stuff. So I add elf handler in i.MX code.
Thanks,
Peng.
>
> Correct?
>
> > Peng Fan (10):
> > dt-bindings: remoteproc: imx_rproc: add i.MX8MQ/M
> > remoteproc: imx_rproc: correct err message
> > remoteproc: imx: use devm_ioremap
> > remoteproc: imx_rproc: make syscon optional
> > remoteproc: imx_rproc: make clk optional
> > remoteproc: imx_rproc: add load hook
> > remoteproc: imx_rproc: add i.MX specific parse fw hook
> > remoteproc: imx_rproc: support i.MX8MQ/M
> > remoteproc: imx_proc: enable virtio/mailbox
> > remoteproc: imx_rproc: support coproc booting before Linux
> >
> > .../devicetree/bindings/remoteproc/imx-rproc.txt | 3 +
> > drivers/remoteproc/imx_rproc.c | 409
> ++++++++++++++++++++-
> > 2 files changed, 401 insertions(+), 11 deletions(-)
> >
> > --
> > 2.16.4
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
>
> --
> Pengutronix e.K. |
> |
> Steuerwalder Str. 21 |
> http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone:
> +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |
On Mon, Jul 27, 2020 at 06:44:32AM +0000, Peng Fan wrote:
> Hi Oleksij,
>
> > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and early
> > boot
> >
> > Hi,
> >
> > On Fri, Jul 24, 2020 at 04:08:03PM +0800, Peng Fan wrote:
> > > This patchset is to support i.MX8MQ/M coproc booted before linux.
> > > Since i.MX8MQ/M was not supported, several patches are needed to first
> > > support the platform, then support early boot case.
> > >
> > > I intended to included i.MX8QM/QXP, but that would introduce a large
> > > patchset, so not included. But the clk/syscon optional patch for
> > > i.MX8QM/QXP was still kept here to avoid rebase error.
> >
> > Thank you for your work.
> >
> > Can you please provide more information about big picture of this work.
> >
> > If I see it correctly, we have here support for i.MX8MM, which seems to be
> > able to fully control Cortex M4 (enable CPU core, etc...).
>
> Yes.
In this case, I would recommend to mainline the i.MX8MM part
first/separately.
> >
> > And other case, where remoteproc is running on application processor and
> > can't or should not touch M4 (i.MX7ULP, i.MX8QM/QXP..). Since M4 provides
> > some functionality, you are trying to reuse remoteproc framework to get
> > resource table present in ELF header and to dynamically load things. For some
> > reasons this header provides more information then needed, so you are
> > changing the ELF parser in the kernel to workaround it.
>
> Not exactly.
>
> For i.MX8MM, we support two cases. M4 kicked by U-Boot, M4 kicked by Linux remoteproc.
> For i.MX8QM/QXP, the typical usecase is M4 kicked by SCFW, but we will also add M4 kicked
> by Linux remoteproc.
> For i.MX7ULP, I would only support M4 dual boot case, M4 control everything.
From current state of discussion, i'm not sure what role plays
remoteproc in the scenario where M4 is started before linux. Especially
if we are not using resource table.
> The reason the change the elf parser is that when M4 elf is loaded by Linux remoteproc,
> It use memset to clear area.
The use of memset, depends on ELF format. Fix/change the linker script
on your firmware and memset will be never called.
> However we use ioremap, memset on ARM64 will report
> crash to device nGnRE memory. And we could not use ioremap_wc to TCM area, since
> it could have data correctly written into TCM.
I have strong feeling, that we are talking about badly or not properly
formatted ELF binary. I would prefer to double check it, before we will
apply fixes on wrong place.
> Maintainer not wanna to drop memset in common code, and TI guys suggest
> add i.MX specific elf stuff. So I add elf handler in i.MX code.
I think, removing memset may damage current users of imx_rproc driver.
Since, like I said: the use of memset depends on ELF format.
> Thanks,
> Peng.
>
> >
> > Correct?
> >
> > > Peng Fan (10):
> > > dt-bindings: remoteproc: imx_rproc: add i.MX8MQ/M
> > > remoteproc: imx_rproc: correct err message
> > > remoteproc: imx: use devm_ioremap
> > > remoteproc: imx_rproc: make syscon optional
> > > remoteproc: imx_rproc: make clk optional
> > > remoteproc: imx_rproc: add load hook
> > > remoteproc: imx_rproc: add i.MX specific parse fw hook
> > > remoteproc: imx_rproc: support i.MX8MQ/M
> > > remoteproc: imx_proc: enable virtio/mailbox
> > > remoteproc: imx_rproc: support coproc booting before Linux
> > >
> > > .../devicetree/bindings/remoteproc/imx-rproc.txt | 3 +
> > > drivers/remoteproc/imx_rproc.c | 409
> > ++++++++++++++++++++-
> > > 2 files changed, 401 insertions(+), 11 deletions(-)
> > >
> > > --
> > > 2.16.4
> > >
> > >
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > [email protected]
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > >
> >
> > --
> > Pengutronix e.K. |
> > |
> > Steuerwalder Str. 21 |
> > http://www.pengutronix.de/ |
> > 31137 Hildesheim, Germany | Phone:
> > +49-5121-206917-0 |
> > Amtsgericht Hildesheim, HRA 2686 | Fax:
> > +49-5121-206917-5555 |
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
> Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and early
> boot
>
> On Mon, Jul 27, 2020 at 06:44:32AM +0000, Peng Fan wrote:
> > Hi Oleksij,
> >
> > > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and
> > > early boot
> > >
> > > Hi,
> > >
> > > On Fri, Jul 24, 2020 at 04:08:03PM +0800, Peng Fan wrote:
> > > > This patchset is to support i.MX8MQ/M coproc booted before linux.
> > > > Since i.MX8MQ/M was not supported, several patches are needed to
> > > > first support the platform, then support early boot case.
> > > >
> > > > I intended to included i.MX8QM/QXP, but that would introduce a
> > > > large patchset, so not included. But the clk/syscon optional patch
> > > > for i.MX8QM/QXP was still kept here to avoid rebase error.
> > >
> > > Thank you for your work.
> > >
> > > Can you please provide more information about big picture of this work.
> > >
> > > If I see it correctly, we have here support for i.MX8MM, which seems
> > > to be able to fully control Cortex M4 (enable CPU core, etc...).
> >
> > Yes.
>
> In this case, I would recommend to mainline the i.MX8MM part
> first/separately.
Only the last patch is to support earlyboot, all others is imx8mm part.
>
> > >
> > > And other case, where remoteproc is running on application processor
> > > and can't or should not touch M4 (i.MX7ULP, i.MX8QM/QXP..). Since M4
> > > provides some functionality, you are trying to reuse remoteproc
> > > framework to get resource table present in ELF header and to
> > > dynamically load things. For some reasons this header provides more
> > > information then needed, so you are changing the ELF parser in the kernel
> to workaround it.
> >
> > Not exactly.
> >
> > For i.MX8MM, we support two cases. M4 kicked by U-Boot, M4 kicked by
> Linux remoteproc.
> > For i.MX8QM/QXP, the typical usecase is M4 kicked by SCFW, but we will
> > also add M4 kicked by Linux remoteproc.
> > For i.MX7ULP, I would only support M4 dual boot case, M4 control
> everything.
>
> From current state of discussion, i'm not sure what role plays remoteproc in
> the scenario where M4 is started before linux. Especially if we are not using
> resource table.
We are using resource table from an address, not in elf file.
This is the new feature in Linux-next to support coproc booted early.
>
> > The reason the change the elf parser is that when M4 elf is loaded by
> > Linux remoteproc, It use memset to clear area.
>
> The use of memset, depends on ELF format. Fix/change the linker script on
> your firmware and memset will be never called.
>
> > However we use ioremap, memset on ARM64 will report crash to device
> > nGnRE memory. And we could not use ioremap_wc to TCM area, since it
> > could have data correctly written into TCM.
>
> I have strong feeling, that we are talking about badly or not properly
> formatted ELF binary. I would prefer to double check it, before we will apply
> fixes on wrong place.
>
> > Maintainer not wanna to drop memset in common code, and TI guys
> > suggest add i.MX specific elf stuff. So I add elf handler in i.MX code.
>
> I think, removing memset may damage current users of imx_rproc driver.
> Since, like I said: the use of memset depends on ELF format.
In my elf file, the last PT_LOAD contains data/bss/heap/stack. I'll check
with our MCU guys, we only need the specific data loaded.
Elf file type is EXEC (Executable file)
Entry point 0x1ffe0355
There are 3 program headers, starting at offset 52
Program Headers:
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
LOAD 0x010000 0x1ffe0000 0x1ffe0000 0x00240 0x00240 R 0x10000
LOAD 0x010240 0x1ffe0240 0x1ffe0240 0x03e90 0x03e90 RWE 0x10000
LOAD 0x020000 0x20000000 0x1ffe40d0 0x00068 0x0ad00 RW 0x10000
Section to Segment mapping:
Segment Sections...
00 .interrupts
01 .resource_table .text .ARM .init_array .fini_array
02 .data .bss .heap .stack
Thanks,
Peng.
>
> > Thanks,
> > Peng.
> >
> > >
> > > Correct?
> > >
> > > > Peng Fan (10):
> > > > dt-bindings: remoteproc: imx_rproc: add i.MX8MQ/M
> > > > remoteproc: imx_rproc: correct err message
> > > > remoteproc: imx: use devm_ioremap
> > > > remoteproc: imx_rproc: make syscon optional
> > > > remoteproc: imx_rproc: make clk optional
> > > > remoteproc: imx_rproc: add load hook
> > > > remoteproc: imx_rproc: add i.MX specific parse fw hook
> > > > remoteproc: imx_rproc: support i.MX8MQ/M
> > > > remoteproc: imx_proc: enable virtio/mailbox
> > > > remoteproc: imx_rproc: support coproc booting before Linux
> > > >
> > > > .../devicetree/bindings/remoteproc/imx-rproc.txt | 3 +
> > > > drivers/remoteproc/imx_rproc.c | 409
> > > ++++++++++++++++++++-
> > > > 2 files changed, 401 insertions(+), 11 deletions(-)
> > > >
> > > > --
> > > > 2.16.4
> > > >
> > > >
> > > > _______________________________________________
> > > > linux-arm-kernel mailing list
> > > > [email protected]
> > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > > >
> > >
> > > --
> > > Pengutronix e.K. |
> > > |
> > > Steuerwalder Str. 21 |
> > > http://www.pengutronix.de/ |
> > > 31137 Hildesheim, Germany | Phone:
> > > +49-5121-206917-0 |
> > > Amtsgericht Hildesheim, HRA 2686 | Fax:
> > > +49-5121-206917-5555 |
>
> --
> Pengutronix e.K. |
> |
> Steuerwalder Str. 21 |
> http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone:
> +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |
On Mon, Jul 27, 2020 at 09:18:31AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and early
> > boot
> >
> > On Mon, Jul 27, 2020 at 06:44:32AM +0000, Peng Fan wrote:
> > > Hi Oleksij,
> > >
> > > > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and
> > > > early boot
> > > >
> > > > Hi,
> > > >
> > > > On Fri, Jul 24, 2020 at 04:08:03PM +0800, Peng Fan wrote:
> > > > > This patchset is to support i.MX8MQ/M coproc booted before linux.
> > > > > Since i.MX8MQ/M was not supported, several patches are needed to
> > > > > first support the platform, then support early boot case.
> > > > >
> > > > > I intended to included i.MX8QM/QXP, but that would introduce a
> > > > > large patchset, so not included. But the clk/syscon optional patch
> > > > > for i.MX8QM/QXP was still kept here to avoid rebase error.
> > > >
> > > > Thank you for your work.
> > > >
> > > > Can you please provide more information about big picture of this work.
> > > >
> > > > If I see it correctly, we have here support for i.MX8MM, which seems
> > > > to be able to fully control Cortex M4 (enable CPU core, etc...).
> > >
> > > Yes.
> >
> > In this case, I would recommend to mainline the i.MX8MM part
> > first/separately.
>
> Only the last patch is to support earlyboot, all others is imx8mm part.
ok
> >
> > > >
> > > > And other case, where remoteproc is running on application processor
> > > > and can't or should not touch M4 (i.MX7ULP, i.MX8QM/QXP..). Since M4
> > > > provides some functionality, you are trying to reuse remoteproc
> > > > framework to get resource table present in ELF header and to
> > > > dynamically load things. For some reasons this header provides more
> > > > information then needed, so you are changing the ELF parser in the kernel
> > to workaround it.
> > >
> > > Not exactly.
> > >
> > > For i.MX8MM, we support two cases. M4 kicked by U-Boot, M4 kicked by
> > Linux remoteproc.
> > > For i.MX8QM/QXP, the typical usecase is M4 kicked by SCFW, but we will
> > > also add M4 kicked by Linux remoteproc.
> > > For i.MX7ULP, I would only support M4 dual boot case, M4 control
> > everything.
> >
> > From current state of discussion, i'm not sure what role plays remoteproc in
> > the scenario where M4 is started before linux. Especially if we are not using
> > resource table.
>
> We are using resource table from an address, not in elf file.
> This is the new feature in Linux-next to support coproc booted early.
>
> >
> > > The reason the change the elf parser is that when M4 elf is loaded by
> > > Linux remoteproc, It use memset to clear area.
> >
> > The use of memset, depends on ELF format. Fix/change the linker script on
> > your firmware and memset will be never called.
> >
> > > However we use ioremap, memset on ARM64 will report crash to device
> > > nGnRE memory. And we could not use ioremap_wc to TCM area, since it
> > > could have data correctly written into TCM.
> >
> > I have strong feeling, that we are talking about badly or not properly
> > formatted ELF binary. I would prefer to double check it, before we will apply
> > fixes on wrong place.
> >
> > > Maintainer not wanna to drop memset in common code, and TI guys
> > > suggest add i.MX specific elf stuff. So I add elf handler in i.MX code.
> >
> > I think, removing memset may damage current users of imx_rproc driver.
> > Since, like I said: the use of memset depends on ELF format.
>
> In my elf file, the last PT_LOAD contains data/bss/heap/stack. I'll check
> with our MCU guys, we only need the specific data loaded.
>
> Elf file type is EXEC (Executable file)
> Entry point 0x1ffe0355
> There are 3 program headers, starting at offset 52
>
> Program Headers:
> Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
> LOAD 0x010000 0x1ffe0000 0x1ffe0000 0x00240 0x00240 R 0x10000
> LOAD 0x010240 0x1ffe0240 0x1ffe0240 0x03e90 0x03e90 RWE 0x10000
> LOAD 0x020000 0x20000000 0x1ffe40d0 0x00068 0x0ad00 RW 0x10000
>
> Section to Segment mapping:
> Segment Sections...
> 00 .interrupts
> 01 .resource_table .text .ARM .init_array .fini_array
> 02 .data .bss .heap .stack
Here is an example of formatting ELF for remoteproc:
https://git.pengutronix.de/cgit/ore/OSELAS.BSP-Pengutronix-DualKit/tree/local_src/remoteproc-elf/linker.ld
https://git.pengutronix.de/cgit/ore/OSELAS.BSP-Pengutronix-DualKit/tree/local_src/remoteproc-elf/imx7m4.S
In this example I pack linux in to remoteproc elf image and start linux
on imx7d-m4 part.
Will be interesting if you can do the same on imx8* SoCs ;)
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
> Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and early
> boot
>
> On Mon, Jul 27, 2020 at 09:18:31AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and
> > > early boot
> > >
> > > On Mon, Jul 27, 2020 at 06:44:32AM +0000, Peng Fan wrote:
> > > > Hi Oleksij,
> > > >
> > > > > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M
> > > > > and early boot
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Fri, Jul 24, 2020 at 04:08:03PM +0800, Peng Fan wrote:
> > > > > > This patchset is to support i.MX8MQ/M coproc booted before linux.
> > > > > > Since i.MX8MQ/M was not supported, several patches are needed
> > > > > > to first support the platform, then support early boot case.
> > > > > >
> > > > > > I intended to included i.MX8QM/QXP, but that would introduce a
> > > > > > large patchset, so not included. But the clk/syscon optional
> > > > > > patch for i.MX8QM/QXP was still kept here to avoid rebase error.
> > > > >
> > > > > Thank you for your work.
> > > > >
> > > > > Can you please provide more information about big picture of this
> work.
> > > > >
> > > > > If I see it correctly, we have here support for i.MX8MM, which
> > > > > seems to be able to fully control Cortex M4 (enable CPU core, etc...).
> > > >
> > > > Yes.
> > >
> > > In this case, I would recommend to mainline the i.MX8MM part
> > > first/separately.
> >
> > Only the last patch is to support earlyboot, all others is imx8mm part.
>
> ok
>
> > >
> > > > >
> > > > > And other case, where remoteproc is running on application
> > > > > processor and can't or should not touch M4 (i.MX7ULP,
> > > > > i.MX8QM/QXP..). Since M4 provides some functionality, you are
> > > > > trying to reuse remoteproc framework to get resource table
> > > > > present in ELF header and to dynamically load things. For some
> > > > > reasons this header provides more information then needed, so
> > > > > you are changing the ELF parser in the kernel
> > > to workaround it.
> > > >
> > > > Not exactly.
> > > >
> > > > For i.MX8MM, we support two cases. M4 kicked by U-Boot, M4 kicked
> > > > by
> > > Linux remoteproc.
> > > > For i.MX8QM/QXP, the typical usecase is M4 kicked by SCFW, but we
> > > > will also add M4 kicked by Linux remoteproc.
> > > > For i.MX7ULP, I would only support M4 dual boot case, M4 control
> > > everything.
> > >
> > > From current state of discussion, i'm not sure what role plays
> > > remoteproc in the scenario where M4 is started before linux.
> > > Especially if we are not using resource table.
> >
> > We are using resource table from an address, not in elf file.
> > This is the new feature in Linux-next to support coproc booted early.
> >
> > >
> > > > The reason the change the elf parser is that when M4 elf is loaded
> > > > by Linux remoteproc, It use memset to clear area.
> > >
> > > The use of memset, depends on ELF format. Fix/change the linker
> > > script on your firmware and memset will be never called.
> > >
> > > > However we use ioremap, memset on ARM64 will report crash to
> > > > device nGnRE memory. And we could not use ioremap_wc to TCM area,
> > > > since it could have data correctly written into TCM.
> > >
> > > I have strong feeling, that we are talking about badly or not
> > > properly formatted ELF binary. I would prefer to double check it,
> > > before we will apply fixes on wrong place.
> > >
> > > > Maintainer not wanna to drop memset in common code, and TI guys
> > > > suggest add i.MX specific elf stuff. So I add elf handler in i.MX code.
> > >
> > > I think, removing memset may damage current users of imx_rproc driver.
> > > Since, like I said: the use of memset depends on ELF format.
> >
> > In my elf file, the last PT_LOAD contains data/bss/heap/stack. I'll
> > check with our MCU guys, we only need the specific data loaded.
> >
> > Elf file type is EXEC (Executable file) Entry point 0x1ffe0355 There
> > are 3 program headers, starting at offset 52
> >
> > Program Headers:
> > Type Offset VirtAddr PhysAddr FileSiz MemSiz
> Flg Align
> > LOAD 0x010000 0x1ffe0000 0x1ffe0000 0x00240 0x00240
> R 0x10000
> > LOAD 0x010240 0x1ffe0240 0x1ffe0240 0x03e90 0x03e90
> RWE 0x10000
> > LOAD 0x020000 0x20000000 0x1ffe40d0 0x00068 0x0ad00
> RW 0x10000
> >
> > Section to Segment mapping:
> > Segment Sections...
> > 00 .interrupts
> > 01 .resource_table .text .ARM .init_array .fini_array
> > 02 .data .bss .heap .stack
>
> Here is an example of formatting ELF for remoteproc:
> https://git.pengutronix.de/cgit/ore/OSELAS.BSP-Pengutronix-DualKit/tree/loc
> al_src/remoteproc-elf/linker.ld
> https://git.pengutronix.de/cgit/ore/OSELAS.BSP-Pengutronix-DualKit/tree/loc
> al_src/remoteproc-elf/imx7m4.S
>
> In this example I pack linux in to remoteproc elf image and start linux on
> imx7d-m4 part.
> Will be interesting if you can do the same on imx8* SoCs ;)
In NXP release, the m4 elf files have data/bss/heap/stack in the same
data area, so the linker merged them into one segment and cause
memsz > filesz.
I think I need to propose platform specific elf memset/memcpy,
such as rproc_elf_memcpy, rproc_elf_memset,
To i.MX, need use memset_io and memcpy_toio, taking TCM
as device memory.
Note: memset without io will cause abort when memsz>filesz.
So use memset_io is safe.
Thanks,
Peng.
>
> Regards,
> Oleksij
> --
> Pengutronix e.K. |
> |
> Steuerwalder Str. 21 |
> http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone:
> +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |
On Tue, Jul 28, 2020 at 07:50:04AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and early
> > boot
> >
> > On Mon, Jul 27, 2020 at 09:18:31AM +0000, Peng Fan wrote:
> > > > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and
> > > > early boot
> > > >
> > > > On Mon, Jul 27, 2020 at 06:44:32AM +0000, Peng Fan wrote:
> > > > > Hi Oleksij,
> > > > >
> > > > > > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M
> > > > > > and early boot
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Fri, Jul 24, 2020 at 04:08:03PM +0800, Peng Fan wrote:
> > > > > > > This patchset is to support i.MX8MQ/M coproc booted before linux.
> > > > > > > Since i.MX8MQ/M was not supported, several patches are needed
> > > > > > > to first support the platform, then support early boot case.
> > > > > > >
> > > > > > > I intended to included i.MX8QM/QXP, but that would introduce a
> > > > > > > large patchset, so not included. But the clk/syscon optional
> > > > > > > patch for i.MX8QM/QXP was still kept here to avoid rebase error.
> > > > > >
> > > > > > Thank you for your work.
> > > > > >
> > > > > > Can you please provide more information about big picture of this
> > work.
> > > > > >
> > > > > > If I see it correctly, we have here support for i.MX8MM, which
> > > > > > seems to be able to fully control Cortex M4 (enable CPU core, etc...).
> > > > >
> > > > > Yes.
> > > >
> > > > In this case, I would recommend to mainline the i.MX8MM part
> > > > first/separately.
> > >
> > > Only the last patch is to support earlyboot, all others is imx8mm part.
> >
> > ok
> >
> > > >
> > > > > >
> > > > > > And other case, where remoteproc is running on application
> > > > > > processor and can't or should not touch M4 (i.MX7ULP,
> > > > > > i.MX8QM/QXP..). Since M4 provides some functionality, you are
> > > > > > trying to reuse remoteproc framework to get resource table
> > > > > > present in ELF header and to dynamically load things. For some
> > > > > > reasons this header provides more information then needed, so
> > > > > > you are changing the ELF parser in the kernel
> > > > to workaround it.
> > > > >
> > > > > Not exactly.
> > > > >
> > > > > For i.MX8MM, we support two cases. M4 kicked by U-Boot, M4 kicked
> > > > > by
> > > > Linux remoteproc.
> > > > > For i.MX8QM/QXP, the typical usecase is M4 kicked by SCFW, but we
> > > > > will also add M4 kicked by Linux remoteproc.
> > > > > For i.MX7ULP, I would only support M4 dual boot case, M4 control
> > > > everything.
> > > >
> > > > From current state of discussion, i'm not sure what role plays
> > > > remoteproc in the scenario where M4 is started before linux.
> > > > Especially if we are not using resource table.
> > >
> > > We are using resource table from an address, not in elf file.
> > > This is the new feature in Linux-next to support coproc booted early.
> > >
> > > >
> > > > > The reason the change the elf parser is that when M4 elf is loaded
> > > > > by Linux remoteproc, It use memset to clear area.
> > > >
> > > > The use of memset, depends on ELF format. Fix/change the linker
> > > > script on your firmware and memset will be never called.
> > > >
> > > > > However we use ioremap, memset on ARM64 will report crash to
> > > > > device nGnRE memory. And we could not use ioremap_wc to TCM area,
> > > > > since it could have data correctly written into TCM.
> > > >
> > > > I have strong feeling, that we are talking about badly or not
> > > > properly formatted ELF binary. I would prefer to double check it,
> > > > before we will apply fixes on wrong place.
> > > >
> > > > > Maintainer not wanna to drop memset in common code, and TI guys
> > > > > suggest add i.MX specific elf stuff. So I add elf handler in i.MX code.
> > > >
> > > > I think, removing memset may damage current users of imx_rproc driver.
> > > > Since, like I said: the use of memset depends on ELF format.
> > >
> > > In my elf file, the last PT_LOAD contains data/bss/heap/stack. I'll
> > > check with our MCU guys, we only need the specific data loaded.
> > >
> > > Elf file type is EXEC (Executable file) Entry point 0x1ffe0355 There
> > > are 3 program headers, starting at offset 52
> > >
> > > Program Headers:
> > > Type Offset VirtAddr PhysAddr FileSiz MemSiz
> > Flg Align
> > > LOAD 0x010000 0x1ffe0000 0x1ffe0000 0x00240 0x00240
> > R 0x10000
> > > LOAD 0x010240 0x1ffe0240 0x1ffe0240 0x03e90 0x03e90
> > RWE 0x10000
> > > LOAD 0x020000 0x20000000 0x1ffe40d0 0x00068 0x0ad00
> > RW 0x10000
> > >
> > > Section to Segment mapping:
> > > Segment Sections...
> > > 00 .interrupts
> > > 01 .resource_table .text .ARM .init_array .fini_array
> > > 02 .data .bss .heap .stack
> >
> > Here is an example of formatting ELF for remoteproc:
> > https://git.pengutronix.de/cgit/ore/OSELAS.BSP-Pengutronix-DualKit/tree/loc
> > al_src/remoteproc-elf/linker.ld
> > https://git.pengutronix.de/cgit/ore/OSELAS.BSP-Pengutronix-DualKit/tree/loc
> > al_src/remoteproc-elf/imx7m4.S
> >
> > In this example I pack linux in to remoteproc elf image and start linux on
> > imx7d-m4 part.
> > Will be interesting if you can do the same on imx8* SoCs ;)
>
> In NXP release, the m4 elf files have data/bss/heap/stack in the same
> data area, so the linker merged them into one segment and cause
> memsz > filesz.
>
> I think I need to propose platform specific elf memset/memcpy,
> such as rproc_elf_memcpy, rproc_elf_memset,
>
> To i.MX, need use memset_io and memcpy_toio, taking TCM
> as device memory.
>
> Note: memset without io will cause abort when memsz>filesz.
> So use memset_io is safe.
Sounds good, i would prefer this way.
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
> Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and early
> boot
>
> On Tue, Jul 28, 2020 at 07:50:04AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M and
> > > early boot
> > >
> > > On Mon, Jul 27, 2020 at 09:18:31AM +0000, Peng Fan wrote:
> > > > > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support iMX8M
> > > > > and early boot
> > > > >
> > > > > On Mon, Jul 27, 2020 at 06:44:32AM +0000, Peng Fan wrote:
> > > > > > Hi Oleksij,
> > > > > >
> > > > > > > Subject: Re: [PATCH 00/10] remoteproc: imx_rproc: support
> > > > > > > iMX8M and early boot
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Fri, Jul 24, 2020 at 04:08:03PM +0800, Peng Fan wrote:
> > > > > > > > This patchset is to support i.MX8MQ/M coproc booted before
> linux.
> > > > > > > > Since i.MX8MQ/M was not supported, several patches are
> > > > > > > > needed to first support the platform, then support early boot
> case.
> > > > > > > >
> > > > > > > > I intended to included i.MX8QM/QXP, but that would
> > > > > > > > introduce a large patchset, so not included. But the
> > > > > > > > clk/syscon optional patch for i.MX8QM/QXP was still kept here to
> avoid rebase error.
> > > > > > >
> > > > > > > Thank you for your work.
> > > > > > >
> > > > > > > Can you please provide more information about big picture of
> > > > > > > this
> > > work.
> > > > > > >
> > > > > > > If I see it correctly, we have here support for i.MX8MM,
> > > > > > > which seems to be able to fully control Cortex M4 (enable CPU
> core, etc...).
> > > > > >
> > > > > > Yes.
> > > > >
> > > > > In this case, I would recommend to mainline the i.MX8MM part
> > > > > first/separately.
> > > >
> > > > Only the last patch is to support earlyboot, all others is imx8mm part.
> > >
> > > ok
> > >
> > > > >
> > > > > > >
> > > > > > > And other case, where remoteproc is running on application
> > > > > > > processor and can't or should not touch M4 (i.MX7ULP,
> > > > > > > i.MX8QM/QXP..). Since M4 provides some functionality, you
> > > > > > > are trying to reuse remoteproc framework to get resource
> > > > > > > table present in ELF header and to dynamically load things.
> > > > > > > For some reasons this header provides more information then
> > > > > > > needed, so you are changing the ELF parser in the kernel
> > > > > to workaround it.
> > > > > >
> > > > > > Not exactly.
> > > > > >
> > > > > > For i.MX8MM, we support two cases. M4 kicked by U-Boot, M4
> > > > > > kicked by
> > > > > Linux remoteproc.
> > > > > > For i.MX8QM/QXP, the typical usecase is M4 kicked by SCFW, but
> > > > > > we will also add M4 kicked by Linux remoteproc.
> > > > > > For i.MX7ULP, I would only support M4 dual boot case, M4
> > > > > > control
> > > > > everything.
> > > > >
> > > > > From current state of discussion, i'm not sure what role plays
> > > > > remoteproc in the scenario where M4 is started before linux.
> > > > > Especially if we are not using resource table.
> > > >
> > > > We are using resource table from an address, not in elf file.
> > > > This is the new feature in Linux-next to support coproc booted early.
> > > >
> > > > >
> > > > > > The reason the change the elf parser is that when M4 elf is
> > > > > > loaded by Linux remoteproc, It use memset to clear area.
> > > > >
> > > > > The use of memset, depends on ELF format. Fix/change the linker
> > > > > script on your firmware and memset will be never called.
> > > > >
> > > > > > However we use ioremap, memset on ARM64 will report crash to
> > > > > > device nGnRE memory. And we could not use ioremap_wc to TCM
> > > > > > area, since it could have data correctly written into TCM.
> > > > >
> > > > > I have strong feeling, that we are talking about badly or not
> > > > > properly formatted ELF binary. I would prefer to double check
> > > > > it, before we will apply fixes on wrong place.
> > > > >
> > > > > > Maintainer not wanna to drop memset in common code, and TI
> > > > > > guys suggest add i.MX specific elf stuff. So I add elf handler in i.MX
> code.
> > > > >
> > > > > I think, removing memset may damage current users of imx_rproc
> driver.
> > > > > Since, like I said: the use of memset depends on ELF format.
> > > >
> > > > In my elf file, the last PT_LOAD contains data/bss/heap/stack.
> > > > I'll check with our MCU guys, we only need the specific data loaded.
> > > >
> > > > Elf file type is EXEC (Executable file) Entry point 0x1ffe0355
> > > > There are 3 program headers, starting at offset 52
> > > >
> > > > Program Headers:
> > > > Type Offset VirtAddr PhysAddr FileSiz MemSiz
> > > Flg Align
> > > > LOAD 0x010000 0x1ffe0000 0x1ffe0000 0x00240
> 0x00240
> > > R 0x10000
> > > > LOAD 0x010240 0x1ffe0240 0x1ffe0240 0x03e90
> 0x03e90
> > > RWE 0x10000
> > > > LOAD 0x020000 0x20000000 0x1ffe40d0 0x00068
> 0x0ad00
> > > RW 0x10000
> > > >
> > > > Section to Segment mapping:
> > > > Segment Sections...
> > > > 00 .interrupts
> > > > 01 .resource_table .text .ARM .init_array .fini_array
> > > > 02 .data .bss .heap .stack
> > >
> > > Here is an example of formatting ELF for remoteproc:
> > > https://git.pengutronix.de/cgit/ore/OSELAS.BSP-Pengutronix-DualKit/t
> > > ree/loc
> > > al_src/remoteproc-elf/linker.ld
> > > https://git.pengutronix.de/cgit/ore/OSELAS.BSP-Pengutronix-DualKit/t
> > > ree/loc
> > > al_src/remoteproc-elf/imx7m4.S
> > >
> > > In this example I pack linux in to remoteproc elf image and start
> > > linux on
> > > imx7d-m4 part.
> > > Will be interesting if you can do the same on imx8* SoCs ;)
> >
> > In NXP release, the m4 elf files have data/bss/heap/stack in the same
> > data area, so the linker merged them into one segment and cause memsz
> > > filesz.
> >
> > I think I need to propose platform specific elf memset/memcpy, such as
> > rproc_elf_memcpy, rproc_elf_memset,
> >
> > To i.MX, need use memset_io and memcpy_toio, taking TCM as device
> > memory.
> >
> > Note: memset without io will cause abort when memsz>filesz.
> > So use memset_io is safe.
>
> Sounds good, i would prefer this way.
Just sent out, please help review there.
https://patchwork.kernel.org/patch/11688751/
https://patchwork.kernel.org/patch/11688753/
Thanks,
Peng.
>
> --
> Pengutronix e.K. |
> |
> Steuerwalder Str. 21 |
> http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone:
> +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |
On Fri, Jul 24, 2020 at 04:08:13PM +0800, Peng Fan wrote:
> Detect Coproc booted or not and Parse resource table
> Set remoteproc state to RPROC_DETACHED when M4 is booted early
> Add attach hook
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/remoteproc/imx_rproc.c | 75 ++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index a8ce97c04e1e..0863b3162777 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -91,6 +91,7 @@ struct imx_rproc {
> const struct imx_rproc_dcfg *dcfg;
> struct imx_rproc_mem mem[IMX7D_RPROC_MEM_MAX];
> struct clk *clk;
> + bool early_boot;
> void *rsc_va;
> struct mbox_client cl;
> struct mbox_chan *tx_ch;
> @@ -235,6 +236,8 @@ static int imx_rproc_stop(struct rproc *rproc)
> dcfg->src_mask, dcfg->src_stop);
> if (ret)
> dev_err(dev, "Failed to stop M4!\n");
> + else
> + priv->early_boot = false;
>
> return ret;
> }
> @@ -390,6 +393,32 @@ static int imx_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
> return 0;
> }
>
> +static int imx_rproc_get_loaded_rsc_table(struct device *dev,
> + struct rproc *rproc)
> +{
> + struct imx_rproc *priv = rproc->priv;
> + struct device_node *np = dev->of_node;
> + u32 da;
> + int ret;
> +
> + ret = of_property_read_u32(np, "rsc-da", &da);
As Rob pointed out I don't think there is a need to invent a new bindings for
this. It could simply be a memory region that is looked up with a name.
> + if (!ret)
> + priv->rsc_va = rproc_da_to_va(rproc, (u64)da, SZ_1K);
> + else
> + return 0;
> +
> + if (!priv->rsc_va) {
> + dev_err(priv->dev, "no map for rsc-da: %x\n", da);
> + return PTR_ERR(priv->rsc_va);
> + }
> +
> + rproc->table_ptr = (struct resource_table *)priv->rsc_va;
> + rproc->table_sz = SZ_1K;
> + rproc->cached_table = NULL;
> +
> + return 0;
> +}
> +
> static int imx_rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
> {
> struct device *dev = &rproc->dev;
> @@ -472,9 +501,15 @@ static void imx_rproc_kick(struct rproc *rproc, int vqid)
> __func__, vqid, err);
> }
>
> +static int imx_rproc_attach(struct rproc *rproc)
> +{
> + return 0;
> +}
> +
> static const struct rproc_ops imx_rproc_ops = {
> .start = imx_rproc_start,
> .stop = imx_rproc_stop,
> + .attach = imx_rproc_attach,
> .kick = imx_rproc_kick,
> .da_to_va = imx_rproc_da_to_va,
> .load = imx_rproc_elf_load_segments,
> @@ -609,6 +644,36 @@ static int imx_rproc_xtr_mbox_init(struct rproc *rproc)
> return ret;
> }
>
> +static int imx_rproc_detect_mode(struct imx_rproc *priv)
> +{
> + const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> + struct device *dev = priv->dev;
> + int ret;
> + u32 val;
> +
> + ret = regmap_read(priv->regmap, dcfg->src_reg, &val);
Patch 04 made it possible for priv->regmap to be NULL and as far as I can see
there is no further check on the value of ->regmap before we get to this
function.
> + if (ret) {
> + dev_err(dev, "Failed to read src\n");
> + return ret;
> + }
> +
> + priv->early_boot = !(val & dcfg->src_stop);
Please add a comment that describe the condition. As much as I try guessing
the relation between ->src_stop and an already booted co-processor I come out
short.
> +
> + if (priv->early_boot) {
I don't see a need for ->early_boot, please re-arrange the code in
imx_rproc_probe() to avoid needing yet an extra variable.
> + priv->rproc->state = RPROC_DETACHED;
> +
> + ret = imx_rproc_parse_memory_regions(priv->rproc);
> + if (ret)
> + return ret;
> +
> + ret = imx_rproc_get_loaded_rsc_table(dev, priv->rproc);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int imx_rproc_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -661,6 +726,10 @@ static int imx_rproc_probe(struct platform_device *pdev)
> goto err_put_mbox;
> }
>
> + ret = imx_rproc_detect_mode(priv);
> + if (ret)
> + goto err_put_mbox;
> +
> priv->clk = devm_clk_get_optional(dev, NULL);
> if (IS_ERR(priv->clk)) {
> dev_err(dev, "Failed to get clock\n");
> @@ -689,7 +758,8 @@ static int imx_rproc_probe(struct platform_device *pdev)
> return 0;
>
> err_put_clk:
> - clk_disable_unprepare(priv->clk);
> + if (!priv->early_boot)
> + clk_disable_unprepare(priv->clk);
> err_put_mbox:
> if (!IS_ERR(priv->tx_ch))
> mbox_free_channel(priv->tx_ch);
> @@ -706,7 +776,8 @@ static int imx_rproc_remove(struct platform_device *pdev)
> struct rproc *rproc = platform_get_drvdata(pdev);
> struct imx_rproc *priv = rproc->priv;
>
> - clk_disable_unprepare(priv->clk);
> + if (!priv->early_boot)
> + clk_disable_unprepare(priv->clk);
> rproc_del(rproc);
> rproc_free(rproc);
>
> --
> 2.16.4
>