From: Peng Fan <peng.fan@nxp.com> To: Mathieu Poirier <mathieu.poirier@linaro.org>, "Peng Fan (OSS)" <peng.fan@oss.nxp.com> Cc: "bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>, "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>, "robh+dt@kernel.org" <robh+dt@kernel.org>, "krzysztof.kozlowski+dt@linaro.org" <krzysztof.kozlowski+dt@linaro.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org> Subject: RE: [PATCH V3 5/6] remoteproc: imx_rproc: support i.MX8QM Date: Wed, 29 Jun 2022 01:19:36 +0000 [thread overview] Message-ID: <DU0PR04MB94172F057A00F55C2E53046088BB9@DU0PR04MB9417.eurprd04.prod.outlook.com> (raw) In-Reply-To: <20220628180257.GD1942439@p14s> > Subject: Re: [PATCH V3 5/6] remoteproc: imx_rproc: support i.MX8QM > > On Tue, May 17, 2022 at 02:49:36PM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Most logic are same as i.MX8QXP, but i.MX8QM has two general purpose > > M4 cores, the two cores runs independently and they has different > > resource id, different start address from SCFW view. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/remoteproc/imx_rproc.c | 41 > > +++++++++++++++++++++++++++++++--- > > 1 file changed, 38 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/remoteproc/imx_rproc.c > > b/drivers/remoteproc/imx_rproc.c index 49cce9dd55c7..8326193c13d6 > > 100644 > > --- a/drivers/remoteproc/imx_rproc.c > > +++ b/drivers/remoteproc/imx_rproc.c > > @@ -3,6 +3,7 @@ > > * Copyright (c) 2017 Pengutronix, Oleksij Rempel > <kernel@pengutronix.de> > > */ > > > > +#include <dt-bindings/firmware/imx/rsrc.h> > > #include <linux/arm-smccc.h> > > #include <linux/clk.h> > > #include <linux/err.h> > > @@ -75,10 +76,13 @@ struct imx_rproc_mem { > > size_t size; > > }; > > > > -/* att flags */ > > +/* att flags: lower 16 bits specifying core, higher 16 bits for flags > > +*/ > > /* M4 own area. Can be mapped at probe */ > > -#define ATT_OWN BIT(1) > > -#define ATT_IOMEM BIT(2) > > +#define ATT_OWN BIT(31) > > +#define ATT_IOMEM BIT(30) > > + > > +#define ATT_CORE_MASK 0xffff > > +#define ATT_CORE(I) BIT((I)) > > > > struct imx_rproc { > > struct device *dev; > > @@ -99,6 +103,7 @@ struct imx_rproc { > > u32 rsrc_id; /* resource id */ > > u32 entry; /* cpu start address */ > > int num_pd; > > + u32 core_index; > > struct device **pd_dev; > > struct device_link **pd_dev_link; > > }; > > @@ -129,6 +134,19 @@ static const struct imx_rproc_att > imx_rproc_att_imx93[] = { > > { 0xD0000000, 0xa0000000, 0x10000000, 0 }, }; > > > > +static const struct imx_rproc_att imx_rproc_att_imx8qm[] = { > > + /* dev addr , sys addr , size , flags */ > > + { 0x08000000, 0x08000000, 0x10000000, 0}, > > + /* TCML */ > > + { 0x1FFE0000, 0x34FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | > ATT_CORE(0)}, > > + { 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | > ATT_CORE(1)}, > > + /* TCMU */ > > + { 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_IOMEM | > ATT_CORE(0)}, > > + { 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_IOMEM | > ATT_CORE(1)}, > > + /* DDR (Data) */ > > + { 0x80000000, 0x80000000, 0x60000000, 0 }, }; > > + > > static const struct imx_rproc_att imx_rproc_att_imx8qxp[] = { > > { 0x08000000, 0x08000000, 0x10000000, 0 }, > > /* TCML/U */ > > @@ -279,6 +297,12 @@ static const struct imx_rproc_dcfg > imx_rproc_cfg_imx8mq = { > > .method = IMX_RPROC_MMIO, > > }; > > > > +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = { > > + .att = imx_rproc_att_imx8qm, > > + .att_size = ARRAY_SIZE(imx_rproc_att_imx8qm), > > + .method = IMX_RPROC_SCU_API, > > +}; > > + > > static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = { > > .att = imx_rproc_att_imx8qxp, > > .att_size = ARRAY_SIZE(imx_rproc_att_imx8qxp), > > @@ -395,6 +419,11 @@ static int imx_rproc_da_to_sys(struct imx_rproc > *priv, u64 da, > > for (i = 0; i < dcfg->att_size; i++) { > > const struct imx_rproc_att *att = &dcfg->att[i]; > > > > + if (att->flags & ATT_CORE_MASK) { > > + if (!((BIT(priv->core_index)) & (att->flags & ATT_CORE_MASK))) > > + continue; > > + } > > This is very cryptic - I just spent 20 minutes looking at it and I'm still not sure I > got the full meaning. Please add enough comments to make things obvious > on first read. There are two generic M4 cores in i.MX8QM, so core_index is 0 for M4_0, and 1 for M4_1. In the memory mapping array: { 0x1FFE0000, 0x34FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)}, { 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)}, /* TCMU */ { 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)}, { 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)}, ATT_CORE(0) means it is for M4_0, ATT_CORE(1) for M4_1. Back to this piece code: if (att->flags & ATT_CORE_MASK) { if (!((BIT(priv->core_index)) & (att->flags & ATT_CORE_MASK))) continue; } Taking M4_1 for example, priv->core_index is 1. So when it need translate an address with ATT_CORE(X) flag, it should ignore ATT_CORE(0) entries. Hope this is clear. For adding comments, how do you think: /* Bypass the entries that not belong to the current remote core */ Thanks, Peng. > > I am done reviewing this patchset. > > Thanks, > Mathieu > > > > + > > if (da >= att->da && da + len < att->da + att->size) { > > unsigned int offset = da - att->da; > > > > @@ -815,6 +844,11 @@ static int imx_rproc_detect_mode(struct > imx_rproc *priv) > > return ret; > > } > > > > + if (priv->rsrc_id == IMX_SC_R_M4_1_PID0) > > + priv->core_index = 1; > > + else > > + priv->core_index = 0; > > + > > /* > > * If Mcore resource is not owned by Acore partition, It is kicked by > ROM, > > * and Linux could only do IPC with Mcore and nothing else. > > @@ -1008,6 +1042,7 @@ static const struct of_device_id > imx_rproc_of_match[] = { > > { .compatible = "fsl,imx8mn-cm7", .data = &imx_rproc_cfg_imx8mn }, > > { .compatible = "fsl,imx8mp-cm7", .data = &imx_rproc_cfg_imx8mn }, > > { .compatible = "fsl,imx8qxp-cm4", .data = &imx_rproc_cfg_imx8qxp }, > > + { .compatible = "fsl,imx8qm-cm4", .data = &imx_rproc_cfg_imx8qm }, > > { .compatible = "fsl,imx8ulp-cm33", .data = &imx_rproc_cfg_imx8ulp }, > > { .compatible = "fsl,imx93-cm33", .data = &imx_rproc_cfg_imx93 }, > > {}, > > -- > > 2.25.1 > >
WARNING: multiple messages have this Message-ID (diff)
From: Peng Fan <peng.fan@nxp.com> To: Mathieu Poirier <mathieu.poirier@linaro.org>, "Peng Fan (OSS)" <peng.fan@oss.nxp.com> Cc: "bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>, "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>, "robh+dt@kernel.org" <robh+dt@kernel.org>, "krzysztof.kozlowski+dt@linaro.org" <krzysztof.kozlowski+dt@linaro.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org> Subject: RE: [PATCH V3 5/6] remoteproc: imx_rproc: support i.MX8QM Date: Wed, 29 Jun 2022 01:19:36 +0000 [thread overview] Message-ID: <DU0PR04MB94172F057A00F55C2E53046088BB9@DU0PR04MB9417.eurprd04.prod.outlook.com> (raw) In-Reply-To: <20220628180257.GD1942439@p14s> > Subject: Re: [PATCH V3 5/6] remoteproc: imx_rproc: support i.MX8QM > > On Tue, May 17, 2022 at 02:49:36PM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Most logic are same as i.MX8QXP, but i.MX8QM has two general purpose > > M4 cores, the two cores runs independently and they has different > > resource id, different start address from SCFW view. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/remoteproc/imx_rproc.c | 41 > > +++++++++++++++++++++++++++++++--- > > 1 file changed, 38 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/remoteproc/imx_rproc.c > > b/drivers/remoteproc/imx_rproc.c index 49cce9dd55c7..8326193c13d6 > > 100644 > > --- a/drivers/remoteproc/imx_rproc.c > > +++ b/drivers/remoteproc/imx_rproc.c > > @@ -3,6 +3,7 @@ > > * Copyright (c) 2017 Pengutronix, Oleksij Rempel > <kernel@pengutronix.de> > > */ > > > > +#include <dt-bindings/firmware/imx/rsrc.h> > > #include <linux/arm-smccc.h> > > #include <linux/clk.h> > > #include <linux/err.h> > > @@ -75,10 +76,13 @@ struct imx_rproc_mem { > > size_t size; > > }; > > > > -/* att flags */ > > +/* att flags: lower 16 bits specifying core, higher 16 bits for flags > > +*/ > > /* M4 own area. Can be mapped at probe */ > > -#define ATT_OWN BIT(1) > > -#define ATT_IOMEM BIT(2) > > +#define ATT_OWN BIT(31) > > +#define ATT_IOMEM BIT(30) > > + > > +#define ATT_CORE_MASK 0xffff > > +#define ATT_CORE(I) BIT((I)) > > > > struct imx_rproc { > > struct device *dev; > > @@ -99,6 +103,7 @@ struct imx_rproc { > > u32 rsrc_id; /* resource id */ > > u32 entry; /* cpu start address */ > > int num_pd; > > + u32 core_index; > > struct device **pd_dev; > > struct device_link **pd_dev_link; > > }; > > @@ -129,6 +134,19 @@ static const struct imx_rproc_att > imx_rproc_att_imx93[] = { > > { 0xD0000000, 0xa0000000, 0x10000000, 0 }, }; > > > > +static const struct imx_rproc_att imx_rproc_att_imx8qm[] = { > > + /* dev addr , sys addr , size , flags */ > > + { 0x08000000, 0x08000000, 0x10000000, 0}, > > + /* TCML */ > > + { 0x1FFE0000, 0x34FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | > ATT_CORE(0)}, > > + { 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | > ATT_CORE(1)}, > > + /* TCMU */ > > + { 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_IOMEM | > ATT_CORE(0)}, > > + { 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_IOMEM | > ATT_CORE(1)}, > > + /* DDR (Data) */ > > + { 0x80000000, 0x80000000, 0x60000000, 0 }, }; > > + > > static const struct imx_rproc_att imx_rproc_att_imx8qxp[] = { > > { 0x08000000, 0x08000000, 0x10000000, 0 }, > > /* TCML/U */ > > @@ -279,6 +297,12 @@ static const struct imx_rproc_dcfg > imx_rproc_cfg_imx8mq = { > > .method = IMX_RPROC_MMIO, > > }; > > > > +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = { > > + .att = imx_rproc_att_imx8qm, > > + .att_size = ARRAY_SIZE(imx_rproc_att_imx8qm), > > + .method = IMX_RPROC_SCU_API, > > +}; > > + > > static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = { > > .att = imx_rproc_att_imx8qxp, > > .att_size = ARRAY_SIZE(imx_rproc_att_imx8qxp), > > @@ -395,6 +419,11 @@ static int imx_rproc_da_to_sys(struct imx_rproc > *priv, u64 da, > > for (i = 0; i < dcfg->att_size; i++) { > > const struct imx_rproc_att *att = &dcfg->att[i]; > > > > + if (att->flags & ATT_CORE_MASK) { > > + if (!((BIT(priv->core_index)) & (att->flags & ATT_CORE_MASK))) > > + continue; > > + } > > This is very cryptic - I just spent 20 minutes looking at it and I'm still not sure I > got the full meaning. Please add enough comments to make things obvious > on first read. There are two generic M4 cores in i.MX8QM, so core_index is 0 for M4_0, and 1 for M4_1. In the memory mapping array: { 0x1FFE0000, 0x34FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)}, { 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)}, /* TCMU */ { 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)}, { 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)}, ATT_CORE(0) means it is for M4_0, ATT_CORE(1) for M4_1. Back to this piece code: if (att->flags & ATT_CORE_MASK) { if (!((BIT(priv->core_index)) & (att->flags & ATT_CORE_MASK))) continue; } Taking M4_1 for example, priv->core_index is 1. So when it need translate an address with ATT_CORE(X) flag, it should ignore ATT_CORE(0) entries. Hope this is clear. For adding comments, how do you think: /* Bypass the entries that not belong to the current remote core */ Thanks, Peng. > > I am done reviewing this patchset. > > Thanks, > Mathieu > > > > + > > if (da >= att->da && da + len < att->da + att->size) { > > unsigned int offset = da - att->da; > > > > @@ -815,6 +844,11 @@ static int imx_rproc_detect_mode(struct > imx_rproc *priv) > > return ret; > > } > > > > + if (priv->rsrc_id == IMX_SC_R_M4_1_PID0) > > + priv->core_index = 1; > > + else > > + priv->core_index = 0; > > + > > /* > > * If Mcore resource is not owned by Acore partition, It is kicked by > ROM, > > * and Linux could only do IPC with Mcore and nothing else. > > @@ -1008,6 +1042,7 @@ static const struct of_device_id > imx_rproc_of_match[] = { > > { .compatible = "fsl,imx8mn-cm7", .data = &imx_rproc_cfg_imx8mn }, > > { .compatible = "fsl,imx8mp-cm7", .data = &imx_rproc_cfg_imx8mn }, > > { .compatible = "fsl,imx8qxp-cm4", .data = &imx_rproc_cfg_imx8qxp }, > > + { .compatible = "fsl,imx8qm-cm4", .data = &imx_rproc_cfg_imx8qm }, > > { .compatible = "fsl,imx8ulp-cm33", .data = &imx_rproc_cfg_imx8ulp }, > > { .compatible = "fsl,imx93-cm33", .data = &imx_rproc_cfg_imx93 }, > > {}, > > -- > > 2.25.1 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-06-29 1:19 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-17 6:49 [PATCH V3 0/6] remoteproc: imx_rproc: support i.MX8QM/QXP Peng Fan (OSS) 2022-05-17 6:49 ` Peng Fan (OSS) 2022-05-17 6:49 ` [PATCH V3 1/6] dt-bindings: remoteproc: imx_rproc: support i.MX8QXP Peng Fan (OSS) 2022-05-17 6:49 ` Peng Fan (OSS) 2022-05-17 15:19 ` Rob Herring 2022-05-17 15:19 ` Rob Herring 2022-05-17 6:49 ` [PATCH V3 2/6] dt-bindings: remoteproc: imx_rproc: support i.MX8QM Peng Fan (OSS) 2022-05-17 6:49 ` Peng Fan (OSS) 2022-05-17 15:19 ` Rob Herring 2022-05-17 15:19 ` Rob Herring 2022-05-17 6:49 ` [PATCH V3 3/6] remoteproc: imx_rproc: support attaching to i.MX8QXP M4 Peng Fan (OSS) 2022-05-17 6:49 ` Peng Fan (OSS) 2022-06-28 17:27 ` Mathieu Poirier 2022-06-28 17:27 ` Mathieu Poirier 2022-06-29 1:01 ` Peng Fan 2022-06-29 1:01 ` Peng Fan 2022-05-17 6:49 ` [PATCH V3 4/6] remoteproc: imx_rproc: support kicking Mcore from Linux for i.MX8QXP Peng Fan (OSS) 2022-05-17 6:49 ` Peng Fan (OSS) 2022-06-28 17:43 ` Mathieu Poirier 2022-06-28 17:43 ` Mathieu Poirier 2022-06-29 1:08 ` Peng Fan 2022-06-29 1:08 ` Peng Fan 2022-05-17 6:49 ` [PATCH V3 5/6] remoteproc: imx_rproc: support i.MX8QM Peng Fan (OSS) 2022-05-17 6:49 ` Peng Fan (OSS) 2022-06-28 18:02 ` Mathieu Poirier 2022-06-28 18:02 ` Mathieu Poirier 2022-06-29 1:19 ` Peng Fan [this message] 2022-06-29 1:19 ` Peng Fan 2022-05-17 6:49 ` [PATCH V3 6/6] remoteproc: imx_rproc: request mbox channel later Peng Fan (OSS) 2022-05-17 6:49 ` Peng Fan (OSS) 2022-06-15 3:24 ` [PATCH V3 0/6] remoteproc: imx_rproc: support i.MX8QM/QXP Peng Fan 2022-06-15 3:24 ` Peng Fan 2022-06-15 20:44 ` Mathieu Poirier 2022-06-15 20:44 ` Mathieu Poirier 2022-06-16 3:08 ` Peng Fan 2022-06-16 3:08 ` Peng Fan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=DU0PR04MB94172F057A00F55C2E53046088BB9@DU0PR04MB9417.eurprd04.prod.outlook.com \ --to=peng.fan@nxp.com \ --cc=bjorn.andersson@linaro.org \ --cc=devicetree@vger.kernel.org \ --cc=festevam@gmail.com \ --cc=kernel@pengutronix.de \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-imx@nxp.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-remoteproc@vger.kernel.org \ --cc=mathieu.poirier@linaro.org \ --cc=peng.fan@oss.nxp.com \ --cc=robh+dt@kernel.org \ --cc=s.hauer@pengutronix.de \ --cc=shawnguo@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.