Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Peng Fan <peng.fan@nxp.com>
To:     Mathieu Poirier <mathieu.poirier@linaro.org>
CC:     "ohad@wizery.com" <ohad@wizery.com>,
        "bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>,
        "o.rempel@pengutronix.de" <o.rempel@pengutronix.de>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
        "kernel@pengutronix.de" <kernel@pengutronix.de>,
        "festevam@gmail.com" <festevam@gmail.com>,
        dl-linux-imx <linux-imx@nxp.com>,
        "linux-remoteproc@vger.kernel.org" <linux-remoteproc@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "paul@crapouillou.net" <paul@crapouillou.net>,
        "matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
        "agross@kernel.org" <agross@kernel.org>,
        "patrice.chotard@st.com" <patrice.chotard@st.com>
Subject: RE: [PATCH V5 7/8] remoteproc: imx_rproc: ignore mapping vdev regions
Thread-Topic: [PATCH V5 7/8] remoteproc: imx_rproc: ignore mapping vdev
 regions
Thread-Index: AQHW3Y8IMsfD4HQjaUqOo7Zl/WNCSKojDGiAgADFokCAAJlRgIAAfjgw
Date:   Wed, 13 Jan 2021 02:19:32 +0000
Message-ID: <DB6PR0402MB27602E812FD657F7EC81854F88A90@DB6PR0402MB2760.eurprd04.prod.outlook.com>
References: <20201229033019.25899-1-peng.fan@nxp.com>
 <20201229033019.25899-8-peng.fan@nxp.com> <20210111215023.GJ144935@xps15>
 <DB6PR0402MB2760F6F982C32B6557467C9188AA0@DB6PR0402MB2760.eurprd04.prod.outlook.com>
 <20210112184629.GA186830@xps15>
In-Reply-To: <20210112184629.GA186830@xps15>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 54d80d28-ef39-478e-87dd-08d8b769a9e8
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jan 2021 02:19:32.1293
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Fr4GIsKIJ1Yl+7PpaBZ5dPDpu5tkXpeqBbv2HnieMqqXAEfdu4v/be9RQ08SRMlSkLeWK44sL7DOmWvDYziJtA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR04MB3127
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> Subject: Re: [PATCH V5 7/8] remoteproc: imx_rproc: ignore mapping vdev
> regions
>=20
> On Tue, Jan 12, 2021 at 09:41:12AM +0000, Peng Fan wrote:
> > > 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@nxp.com wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > 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@nxp.com>
> > > > ---
> > > >  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 =3D 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 =3D <0x550ff000 0x1000>;
> > +    };
> > +
> > +    vdev0vring0: vdev0vring0@55000000 {
> > +      no-map;
> > +      reg =3D <0x55000000 0x8000>;
> > +    };
> > +
> > +    vdev0vring1: vdev0vring1@55008000 {
> > +      reg =3D <0x55008000 0x8000>;
> > +      no-map;
> > +    };
> > +
> > +    vdevbuffer: vdevbuffer@55400000 {
> > +      compatible =3D "shared-dma-pool";
> > +      reg =3D <0x55400000 0x100000>;
> > +      no-map;
> > +    };
> > +
> > +    imx8mm-cm4 {
> > +      compatible =3D "fsl,imx8mm-cm4";
> > +      clocks =3D <&clk IMX8MM_CLK_M4_DIV>;
> > +      mbox-names =3D "tx", "rx", "rxdb";
> > +      mboxes =3D <&mu 0 1
> > +                &mu 1 1
> > +                &mu 3 1>;
> > +      memory-region =3D <&vdevbuffer>, <&vdev0vring0>, <&vdev0vring1>,
> <&rsc_table>;
> > +      syscon =3D <&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.
>=20
> They _will_ but for now they don't and as such there a discrepancy betwee=
n
> the bindings and the code that was published in V6.  At this point you ca=
n
> either drop the vrings in the DT or send another revision with the carveo=
uts
> allocated.  I would definitely prefer the latter because it wouldn't invo=
lve yet
> another modification of the bindings.

You mean I drop patch v5 7/8 and send v7, right?

Or are there other changes that I need to do?

Thanks,
Peng.

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