RE: [PATCH V5 7/8] remoteproc: imx_rproc: ignore mapping vdev regions

[Peng Fan <peng.fan@xxxxxxx>]

> Subject: Re: [PATCH V5 7/8] remoteproc: imx_rproc: ignore mapping vdev
> regions
> 
> On Tue, Dec 29, 2020 at 11:30:18AM +0800, peng.fan@xxxxxxx wrote:
> > From: Peng Fan <peng.fan@xxxxxxx>
> >
> > vdev regions are vdev0vring0, vdev0vring1, vdevbuffer and similar.
> > They are handled by remoteproc common code, no need to map in imx
> > rproc driver.
> >
> > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > ---
> >  drivers/remoteproc/imx_rproc.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c
> > b/drivers/remoteproc/imx_rproc.c index f80428afb8a7..e62a53ee128e
> > 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -417,6 +417,9 @@ static int imx_rproc_addr_init(struct imx_rproc
> *priv,
> >  		struct resource res;
> >
> >  		node = of_parse_phandle(np, "memory-region", a);
> > +		/* Not map vdev region */
> > +		if (!strcmp(node->name, "vdev"))
> > +			continue;
> 
> I am very confused and because I don't see an example for the DT in the
> bindings document I have to guess what is going on.

V6 will include the DT yaml.

> 
> So I am guessing that you have laid out the memory regions for the vrings and
> the vdev0buffer in the DT "memory-region".

The dts part will be similar as following:

+    #include <dt-bindings/clock/imx8mm-clock.h>
+    rsc_table: rsc_table@550ff000 {
+      no-map;
+      reg = <0x550ff000 0x1000>;
+    };
+
+    vdev0vring0: vdev0vring0@55000000 {
+      no-map;
+      reg = <0x55000000 0x8000>;
+    };
+
+    vdev0vring1: vdev0vring1@55008000 {
+      reg = <0x55008000 0x8000>;
+      no-map;
+    };
+
+    vdevbuffer: vdevbuffer@55400000 {
+      compatible = "shared-dma-pool";
+      reg = <0x55400000 0x100000>;
+      no-map;
+    };
+
+    imx8mm-cm4 {
+      compatible = "fsl,imx8mm-cm4";
+      clocks = <&clk IMX8MM_CLK_M4_DIV>;
+      mbox-names = "tx", "rx", "rxdb";
+      mboxes = <&mu 0 1
+                &mu 1 1
+                &mu 3 1>;
+      memory-region = <&vdevbuffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
+      syscon = <&src>;
+    };

> 
> For the vrings I don't see the allocation of a carveout, which means that you
> will take the memory out of the DMA pool and the reserve memory will be
> wasted.

imx_rproc_parse_memory_regions will alloc carveout.

> 
> For the vdev0buffer, what you have will work *only* if that entry is the first
> one in the list of memory regions, as we agreed here [2].

Yes. I agree and follow this rule from then.

Thanks,
Peng.

> 
> [1].
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.b
> ootlin.com%2Flinux%2Fv5.11-rc3%2Fsource%2Fdrivers%2Fremoteproc%2Fre
> moteproc_core.c%23L321&amp;data=04%7C01%7Cpeng.fan%40nxp.com%7
> C581784529b1646b9d34b08d8b67ae8c7%7C686ea1d3bc2b4c6fa92cd99c5c
> 301635%7C0%7C0%7C637459986311799770%7CUnknown%7CTWFpbGZsb3
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%7C1000&amp;sdata=Qur6YiTWlak0ZRnrUZRzawfoO38EBrAItqZm66b4
> m20%3D&amp;reserved=0
> [2].
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.kernel.org%2Fproject%2Flinux-remoteproc%2Fpatch%2F202007221315
> 43.7024-1-peng.fan%40nxp.com%2F&amp;data=04%7C01%7Cpeng.fan%40n
> xp.com%7C581784529b1646b9d34b08d8b67ae8c7%7C686ea1d3bc2b4c6fa9
> 2cd99c5c301635%7C0%7C0%7C637459986311799770%7CUnknown%7CTW
> FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> VCI6Mn0%3D%7C1000&amp;sdata=b%2F8muWtb3yxKIsnXmKmRGYYV33%2
> FHjwA6a8x58geY7eE%3D&amp;reserved=0
> 
> >  		err = of_address_to_resource(node, 0, &res);
> >  		if (err) {
> >  			dev_err(dev, "unable to resolve memory region\n");
> > --
> > 2.28.0
> >