From:   Peng Fan <peng.fan@nxp.com>
To:     Mathieu Poirier <mathieu.poirier@linaro.org>,
        "Peng Fan (OSS)" <peng.fan@oss.nxp.com>
CC:     "andersson@kernel.org" <andersson@kernel.org>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
        "arnaud.pouliquen@foss.st.com" <arnaud.pouliquen@foss.st.com>,
        "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>
Subject: RE: [PATCH V2 5/6] remoteproc: imx_rproc: set Cortex-M stack/pc to
 TCML
Thread-Topic: [PATCH V2 5/6] remoteproc: imx_rproc: set Cortex-M stack/pc to
 TCML
Thread-Index: AQHZMjDJzgt7cVgC9UWv0kJgugWwrq68PgaAgAAiy7A=
Date:   Fri, 3 Feb 2023 00:04:27 +0000
Message-ID: <DU0PR04MB9417620B668E43118933821388D79@DU0PR04MB9417.eurprd04.prod.outlook.com>
References: <20230127092246.1470865-1-peng.fan@oss.nxp.com>
 <20230127092246.1470865-6-peng.fan@oss.nxp.com>
 <20230202215629.GC1147631@p14s>
In-Reply-To: <20230202215629.GC1147631@p14s>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?lWm63t75sN0usCiFsX68FWID7uSaBM9OWQMl7IMUaA1x1RAJWtM0bnoHK7Xi?=
 =?us-ascii?Q?2VhoGYFOIJlGl7Z9MegjULxXn8OwVid1M7eG5wvBkc51oMAUxzLMVN8Mdmfv?=
 =?us-ascii?Q?8pEyMQryuOlGmTKtyEP8YSH+Xxk2R/V5WoyTg5g+5itemURJZWq8iWA4bN/2?=
 =?us-ascii?Q?scC386ZacY8tzkiSAQ7vvv1PHAZEFjIGXeclKL1uDMefhBrjAZf61FZMF0Ql?=
 =?us-ascii?Q?fNMIwAK1UXKOFtINYsn/TUaps7czu1Vu2ZmoO2WdiOSpAFPNJBhVhjRNX3By?=
 =?us-ascii?Q?8FV/9PlaVccX7Oi3VbkZaEh8KwyoTIC8KPa3dDArNL82HJ8m2TEwQVopioo3?=
 =?us-ascii?Q?6ZQgMDhDbrDkJ1xhXioBn0P/IUOj8XNZL/d5LQkVnVexw/2nxN/o4EfN14FO?=
 =?us-ascii?Q?0ItCO+9C6EmuwuzyAbxRvFRC6txfTJf9TIMb4ns/Oq6Myneai5KFmcPG049g?=
 =?us-ascii?Q?cywHWXLqKkzMol0RxIe7pfS3AnhxnnkONb2s3ZZ7BVtBJESsa/ESZa7po/ng?=
 =?us-ascii?Q?Mod6wnyjyRVEvu6GefqfFyNBhDBFtar1MyGCEgkqinvUCWoXOJ8mjQ4XfuLP?=
 =?us-ascii?Q?U4EUf+9TFCVLfEA7iYw+KClBdEvUBPeRJsbclEDa+wTri70aH9WOK8Wih8pE?=
 =?us-ascii?Q?6zyPLdqR9Gu74t2ocPAAp28Zq5MoGK4tWgqd4/taInhkYJlqzKtQFYrrRTmV?=
 =?us-ascii?Q?ZaAmRLG2bg7ONpZ4p8kXXdOSMEkSrPiVzRRm+r2xEdbLqz/rYlMBI/+0h8XB?=
 =?us-ascii?Q?oaLrR+4jhZmV6lEa66hFg66blrVP5/lPRCrQVyFfdx8pptpb0KQnS/UnCxf5?=
 =?us-ascii?Q?/fteoMt5q0y+okY8mftBq+RqALo+fnLtOKdP+0iY5pKrOSDzyNBkhj1rjN1O?=
 =?us-ascii?Q?KU1V+GuXs57XC52Ze+Rv4V1LoiWAXl58XiK7pFNdQY9fO77eujk6nd8BYZNM?=
 =?us-ascii?Q?PDEKWWVQxOKA6OwJDx6AAOzpe0oP2/10hTosClnzcKcZrwaOKZmjcYEIrvc7?=
 =?us-ascii?Q?0kSqCsi84cJQ23Cvxyd2pcIq6uxX8u7ynVe6DvCkC6o0iX5iXQpq8zhCLx5l?=
 =?us-ascii?Q?y+o8B0D6qR6c1SXLWzO0nUvmdneukc8pjazjXtOzYpA+nP1gWtXABEFWJmPO?=
 =?us-ascii?Q?2RvwI9Xd6G7qaN3dA5Np3h19MYX+NhTg45Q6EUUKTTLaWQ8pCtnQ1nRk+4Ik?=
 =?us-ascii?Q?erpB8wtqkpZaCT4APE5VasxjzcEIxsQYSEvj86OJcb2DoC6Wm3jhjvWi/14o?=
 =?us-ascii?Q?cVY+2zCCq5s7Z2HHEIYGmffL5SEN5ZEC7Ojhp9vmP6/2Ln6vZny+Od8WlM5h?=
 =?us-ascii?Q?IQR5hquJMoT4IUlEFrKViBN37rIgoy92wUb6914HRXXH8as5bNk48ks9e4v/?=
 =?us-ascii?Q?o30ofi8+kv4YQXkUoUa53YCPL7fwSzOAZrKEZ2p3FLKCcLj6cw24hJOJo3MC?=
 =?us-ascii?Q?ibcUSAVduXcJo8+bN9l0nTgomU6zGiRMsChjggcF0lvKp1vcYMUJZ6haWub/?=
 =?us-ascii?Q?NBgjtU7hTfjU+kUfUMjA6m2WkURTNF17yFUcq8DA/La4mubLwwHuIa+Mm5IV?=
 =?us-ascii?Q?bkB1HK+xXhi5gtXRblw=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DU0PR04MB9417.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e0018c0c-bfdf-4456-816b-08db057a3767
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Feb 2023 00:04:27.6580
 (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: o/oobiCrnqqHWfOissi/jpaCCg0JT9gnHNbAjL0ONuZ92UyIdYB2swGP2ghQAxlL4OZIrIMYj8i78Su7MUa3yw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB8176
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> Subject: Re: [PATCH V2 5/6] remoteproc: imx_rproc: set Cortex-M stack/pc
> to TCML
>=20
> On Fri, Jan 27, 2023 at 05:22:45PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The i.MX8M Cortex-M core not has ROM. It has a requirement is the
> > stack, pc value should be set in address 0 and 4 from the view of
> > itself. From Cortex-A core view, the region is at TCML start address.
> >
> > The stack and pc value are the first two words stored in section
> > ".interrupts" of the firmware, and the section is the first section in
> > the firmware.
> >
> > When the firmware is built to run in TCML, there is no issue, because
> > when copying elf segments, the first two words are copied to TCML also.
> >
> > However when the firmware is built ro run in DDR, the first two words
> > are not copied to TCML start address.
> >
> > This patch is to find the ".interrupts" section, read out the first
> > two words and write to TCML start address at offset 0 and 4.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/remoteproc/imx_rproc.c | 37
> > +++++++++++++++++++++++++++++++++-
> >  1 file changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c
> > b/drivers/remoteproc/imx_rproc.c index 295e0e0e869a..f5ee0c9bb09d
> > 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -7,6 +7,7 @@
> >  #include <linux/arm-smccc.h>
> >  #include <linux/clk.h>
> >  #include <linux/err.h>
> > +#include <linux/firmware.h>
> >  #include <linux/firmware/imx/sci.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/kernel.h>
> > @@ -23,6 +24,7 @@
> >  #include <linux/workqueue.h>
> >
> >  #include "imx_rproc.h"
> > +#include "remoteproc_elf_helpers.h"
> >  #include "remoteproc_internal.h"
> >
> >  #define IMX7D_SRC_SCR			0x0C
> > @@ -634,6 +636,39 @@ static struct resource_table
> *imx_rproc_get_loaded_rsc_table(struct rproc *rproc
> >  	return (struct resource_table __force *)priv->rsc_table;  }
> >
> > +static u64 imx_rproc_get_boot_addr(struct rproc *rproc, const struct
> > +firmware *fw) {
> > +	struct imx_rproc *priv =3D rproc->priv;
> > +	const u8 *elf_data =3D (void *)fw->data;
> > +	u8 class =3D fw_elf_get_class(fw);
> > +	u64 bootaddr =3D rproc_elf_get_boot_addr(rproc, fw);
> > +	const void *shdr;
> > +	void __iomem *va;
> > +	u64 sh_addr, offset;
> > +
> > +	if (priv->dcfg->devtype =3D=3D IMX_RPROC_IMX8M) {
> > +		/*
> > +		 * i.MX8M Cortex-M requires [stack, pc] be put in address
> > +		 * [0, 4], so the da address is 0, size is 8 words.
> > +		 */
> > +		va =3D (__force void __iomem *)rproc_da_to_va(rproc, 0, 8,
> NULL);
> > +		shdr =3D rproc_elf_find_shdr(rproc, fw, ".interrupts");
> > +		if (!shdr || !va)
> > +			return bootaddr;
> > +		sh_addr =3D elf_shdr_get_sh_addr(class, shdr);
>=20
> This isn't used - why is it still there?

will drop it.

>=20
> > +		offset =3D elf_shdr_get_sh_offset(class, shdr);
> > +
> > +		/*
> > +		 * Write stack, pc to TCML start address. The TCML region
> > +		 * is marked with ATT_IOMEM, so use writel.
> > +		 */
> > +		writel(*(u32 *)(elf_data + offset), va);
> > +		writel(*(u32 *)(elf_data + offset + 4), va + 4);
>=20
> Here you are writing 2 words at address 0x0 and 2 words at address 0x4.
> Why are you saying the size is 8 words in the comment above?

Typo. I should mean 8 bytes.

Thanks,
Peng.

>=20
> > +	}
> > +
> > +	return bootaddr;
> > +}
> > +
> >  static const struct rproc_ops imx_rproc_ops =3D {
> >  	.prepare	=3D imx_rproc_prepare,
> >  	.attach		=3D imx_rproc_attach,
> > @@ -647,7 +682,7 @@ static const struct rproc_ops imx_rproc_ops =3D {
> >  	.find_loaded_rsc_table =3D rproc_elf_find_loaded_rsc_table,
> >  	.get_loaded_rsc_table =3D imx_rproc_get_loaded_rsc_table,
> >  	.sanity_check	=3D rproc_elf_sanity_check,
> > -	.get_boot_addr	=3D rproc_elf_get_boot_addr,
> > +	.get_boot_addr	=3D imx_rproc_get_boot_addr,
> >  };
> >
> >  static int imx_rproc_addr_init(struct imx_rproc *priv,
> > --
> > 2.37.1
> >