From: Mathieu Poirier <mathieu.poirier@linaro.org> To: "Peng Fan (OSS)" <peng.fan@oss.nxp.com> Cc: bjorn.andersson@linaro.org, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com, linux-remoteproc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Peng Fan <peng.fan@nxp.com> Subject: Re: [PATCH 8/9] remoteproc: imx_rproc: support i.MX8QM Date: Thu, 20 Jan 2022 10:23:43 -0700 [thread overview] Message-ID: <20220120172343.GA1338735@p14s> (raw) In-Reply-To: <20220111033333.403448-11-peng.fan@oss.nxp.com> On Tue, Jan 11, 2022 at 11:33:32AM +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: > Use the lower 16 bits specifying core, higher 16 bits for flags. > The 2nd core has different start address from SCFW view Are the cores running independently or in lockstep? This is relevant information that should be in the changelog. The above is an implementation detail that should be added as comments in the code. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/remoteproc/imx_rproc.c | 55 +++++++++++++++++++++++++++++++--- > 1 file changed, 51 insertions(+), 4 deletions(-) > > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c > index 09d2a06e5ed6..7bc274fbce9f 100644 > --- a/drivers/remoteproc/imx_rproc.c > +++ b/drivers/remoteproc/imx_rproc.c > @@ -77,8 +77,11 @@ struct imx_rproc_mem { > > /* att 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) ATT_OWN was defined in 2017 and has had the same value since. ATT_IOMEM was introduced by this commit [1] (that you signed off on), which as supposed to be a fix for another commit. Now, overnight, both bitfields are changed without any explanation other than a cryptic comments. What about all the other platforms that previously used those bitfields - was this change tested on those as well? I will stop here with this patchset - it needs to much work for me to continue reviewing it. Thanks, Mathieu [1]. 91bb26637353 remoteproc: imx_rproc: Fix TCM io memory type > +/* I = [0:7] */ > +#define ATT_CORE_MASK 0xffff > +#define ATT_CORE(I) BIT((I)) > > struct imx_rproc { > struct device *dev; > @@ -98,11 +101,25 @@ struct imx_rproc { > struct notifier_block proc_nb; > u32 rproc_pt; > u32 rsrc; > + u32 reg; > int num_pd; > struct device **pd_dev; > struct device_link **pd_dev_link; > }; > > +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_CORE(0)}, > + { 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_CORE(1)}, > + /* TCMU */ > + { 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_CORE(0)}, > + { 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_CORE(1)}, > + /* DDR (Data) */ > + { 0x80000000, 0x80000000, 0x60000000, 0 }, > +}; > + > static const struct imx_rproc_att imx_rproc_att_imx8qxp[] = { > /* dev addr , sys addr , size , flags */ > { 0x08000000, 0x08000000, 0x10000000, 0}, > @@ -260,6 +277,12 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = { > .method = IMX_RPROC_NONE, > }; > > +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), > @@ -310,7 +333,10 @@ static int imx_rproc_start(struct rproc *rproc) > ret = res.a0; > break; > case IMX_RPROC_SCU_API: > - ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, true, 0x34fe0000); > + if (priv->reg) > + ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, true, 0x38fe0000); > + else > + ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, true, 0x34fe0000); > break; > default: > return -EOPNOTSUPP; > @@ -342,7 +368,10 @@ static int imx_rproc_stop(struct rproc *rproc) > dev_info(dev, "Not in wfi, force stopped\n"); > break; > case IMX_RPROC_SCU_API: > - ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, false, 0x34fe0000); > + if (priv->reg) > + ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, false, 0x38fe0000); > + else > + ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, false, 0x34fe0000); > break; > default: > return -EOPNOTSUPP; > @@ -364,6 +393,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->reg)) & (att->flags & ATT_CORE_MASK))) > + continue; > + } > + > if (da >= att->da && da + len < att->da + att->size) { > unsigned int offset = da - att->da; > > @@ -594,6 +628,11 @@ static int imx_rproc_addr_init(struct imx_rproc *priv, > if (!(att->flags & ATT_OWN)) > continue; > > + if (att->flags & ATT_CORE_MASK) { > + if (!((BIT(priv->reg)) & (att->flags & ATT_CORE_MASK))) > + continue; > + } > + > if (b >= IMX_RPROC_MEM_MAX) > break; > > @@ -809,6 +848,13 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv) > return ret; > } > > + priv->reg = of_get_cpu_hwid(dev->of_node, 0); > + if (priv->reg == ~0U) > + priv->reg = 0; > + > + if (priv->reg > 1) > + return -EINVAL; > + > priv->has_clk = false; > /* > * If Mcore resource is not owned by Acore partition, It is kicked by ROM, > @@ -1005,6 +1051,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 }, > {}, > }; > -- > 2.25.1 >
WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Poirier <mathieu.poirier@linaro.org> To: "Peng Fan (OSS)" <peng.fan@oss.nxp.com> Cc: bjorn.andersson@linaro.org, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com, linux-remoteproc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Peng Fan <peng.fan@nxp.com> Subject: Re: [PATCH 8/9] remoteproc: imx_rproc: support i.MX8QM Date: Thu, 20 Jan 2022 10:23:43 -0700 [thread overview] Message-ID: <20220120172343.GA1338735@p14s> (raw) In-Reply-To: <20220111033333.403448-11-peng.fan@oss.nxp.com> On Tue, Jan 11, 2022 at 11:33:32AM +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: > Use the lower 16 bits specifying core, higher 16 bits for flags. > The 2nd core has different start address from SCFW view Are the cores running independently or in lockstep? This is relevant information that should be in the changelog. The above is an implementation detail that should be added as comments in the code. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/remoteproc/imx_rproc.c | 55 +++++++++++++++++++++++++++++++--- > 1 file changed, 51 insertions(+), 4 deletions(-) > > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c > index 09d2a06e5ed6..7bc274fbce9f 100644 > --- a/drivers/remoteproc/imx_rproc.c > +++ b/drivers/remoteproc/imx_rproc.c > @@ -77,8 +77,11 @@ struct imx_rproc_mem { > > /* att 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) ATT_OWN was defined in 2017 and has had the same value since. ATT_IOMEM was introduced by this commit [1] (that you signed off on), which as supposed to be a fix for another commit. Now, overnight, both bitfields are changed without any explanation other than a cryptic comments. What about all the other platforms that previously used those bitfields - was this change tested on those as well? I will stop here with this patchset - it needs to much work for me to continue reviewing it. Thanks, Mathieu [1]. 91bb26637353 remoteproc: imx_rproc: Fix TCM io memory type > +/* I = [0:7] */ > +#define ATT_CORE_MASK 0xffff > +#define ATT_CORE(I) BIT((I)) > > struct imx_rproc { > struct device *dev; > @@ -98,11 +101,25 @@ struct imx_rproc { > struct notifier_block proc_nb; > u32 rproc_pt; > u32 rsrc; > + u32 reg; > int num_pd; > struct device **pd_dev; > struct device_link **pd_dev_link; > }; > > +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_CORE(0)}, > + { 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_CORE(1)}, > + /* TCMU */ > + { 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_CORE(0)}, > + { 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_CORE(1)}, > + /* DDR (Data) */ > + { 0x80000000, 0x80000000, 0x60000000, 0 }, > +}; > + > static const struct imx_rproc_att imx_rproc_att_imx8qxp[] = { > /* dev addr , sys addr , size , flags */ > { 0x08000000, 0x08000000, 0x10000000, 0}, > @@ -260,6 +277,12 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = { > .method = IMX_RPROC_NONE, > }; > > +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), > @@ -310,7 +333,10 @@ static int imx_rproc_start(struct rproc *rproc) > ret = res.a0; > break; > case IMX_RPROC_SCU_API: > - ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, true, 0x34fe0000); > + if (priv->reg) > + ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, true, 0x38fe0000); > + else > + ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, true, 0x34fe0000); > break; > default: > return -EOPNOTSUPP; > @@ -342,7 +368,10 @@ static int imx_rproc_stop(struct rproc *rproc) > dev_info(dev, "Not in wfi, force stopped\n"); > break; > case IMX_RPROC_SCU_API: > - ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, false, 0x34fe0000); > + if (priv->reg) > + ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, false, 0x38fe0000); > + else > + ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc, false, 0x34fe0000); > break; > default: > return -EOPNOTSUPP; > @@ -364,6 +393,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->reg)) & (att->flags & ATT_CORE_MASK))) > + continue; > + } > + > if (da >= att->da && da + len < att->da + att->size) { > unsigned int offset = da - att->da; > > @@ -594,6 +628,11 @@ static int imx_rproc_addr_init(struct imx_rproc *priv, > if (!(att->flags & ATT_OWN)) > continue; > > + if (att->flags & ATT_CORE_MASK) { > + if (!((BIT(priv->reg)) & (att->flags & ATT_CORE_MASK))) > + continue; > + } > + > if (b >= IMX_RPROC_MEM_MAX) > break; > > @@ -809,6 +848,13 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv) > return ret; > } > > + priv->reg = of_get_cpu_hwid(dev->of_node, 0); > + if (priv->reg == ~0U) > + priv->reg = 0; > + > + if (priv->reg > 1) > + return -EINVAL; > + > priv->has_clk = false; > /* > * If Mcore resource is not owned by Acore partition, It is kicked by ROM, > @@ -1005,6 +1051,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 }, > {}, > }; > -- > 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-01-20 17:23 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-11 3:33 [PATCH V2] remoteproc: imx_rproc: use imx specific hook for find_loaded_rsc_table Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-11 3:33 ` [PATCH] remoteproc: imx_rproc: validate resource table Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-17 18:25 ` Mathieu Poirier 2022-01-17 18:25 ` Mathieu Poirier 2022-01-18 1:24 ` Peng Fan 2022-01-18 1:24 ` Peng Fan 2022-01-11 3:33 ` [PATCH 0/9] remoteproc: imx_rproc: support i.MX8QXP/QM and self recovery Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-11 3:33 ` [PATCH 1/9] dt-bindings: remoteproc: imx_rproc: support i.MX8QXP Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-11 6:44 ` Peng Fan 2022-01-11 6:44 ` Peng Fan 2022-01-18 18:41 ` Mathieu Poirier 2022-01-18 18:41 ` Mathieu Poirier 2022-01-19 2:20 ` Peng Fan 2022-01-19 2:20 ` Peng Fan 2022-01-11 3:33 ` [PATCH 2/9] dt-bindings: remoteproc: imx_rproc: support i.MX8QM Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-11 6:45 ` Peng Fan 2022-01-11 6:45 ` Peng Fan 2022-01-11 3:33 ` [PATCH 3/9] remoteproc: support self recovery after rproc crash Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-18 18:44 ` Mathieu Poirier 2022-01-18 18:44 ` Mathieu Poirier 2022-01-19 2:21 ` Peng Fan 2022-01-19 2:21 ` Peng Fan 2022-01-11 3:33 ` [PATCH 4/9] remoteproc: imx_rproc: ignore create mem entry for resource table Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-18 18:47 ` Mathieu Poirier 2022-01-18 18:47 ` Mathieu Poirier 2022-01-19 2:23 ` Peng Fan 2022-01-19 2:23 ` Peng Fan 2022-01-11 3:33 ` [PATCH 5/9] remoteproc: imx_rproc: make clk optional Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-18 18:50 ` Mathieu Poirier 2022-01-18 18:50 ` Mathieu Poirier 2022-01-19 2:25 ` Peng Fan 2022-01-19 2:25 ` Peng Fan 2022-01-19 17:49 ` Mathieu Poirier 2022-01-19 17:49 ` Mathieu Poirier 2022-01-11 3:33 ` [PATCH 6/9] remoteproc: imx_rproc: support attaching to i.MX8QXP M4 Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-18 18:57 ` Mathieu Poirier 2022-01-18 18:57 ` Mathieu Poirier 2022-01-19 2:28 ` Peng Fan 2022-01-19 2:28 ` Peng Fan 2022-01-19 18:04 ` Mathieu Poirier 2022-01-19 18:04 ` Mathieu Poirier 2022-01-19 18:31 ` Mathieu Poirier 2022-01-19 18:31 ` Mathieu Poirier 2022-01-11 3:33 ` [PATCH 7/9] remoteproc: imx_rproc: support kicking Mcore from Linux for i.MX8QXP Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-19 18:58 ` Mathieu Poirier 2022-01-19 18:58 ` Mathieu Poirier 2022-01-11 3:33 ` [PATCH 8/9] remoteproc: imx_rproc: support i.MX8QM Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-01-20 17:23 ` Mathieu Poirier [this message] 2022-01-20 17:23 ` Mathieu Poirier 2022-01-21 1:00 ` Peng Fan 2022-01-21 1:00 ` Peng Fan 2022-01-11 3:33 ` [PATCH 9/9] remoteproc: imx_rproc: request mbox channel later Peng Fan (OSS) 2022-01-11 3:33 ` Peng Fan (OSS) 2022-12-21 10:55 ` [PATCH V2] remoteproc: imx_rproc: use imx specific hook for find_loaded_rsc_table Marco Felsch 2022-12-21 10:55 ` Marco Felsch 2023-01-02 22:46 ` Mathieu Poirier 2023-01-02 22:46 ` Mathieu Poirier
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=20220120172343.GA1338735@p14s \ --to=mathieu.poirier@linaro.org \ --cc=bjorn.andersson@linaro.org \ --cc=festevam@gmail.com \ --cc=kernel@pengutronix.de \ --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=peng.fan@nxp.com \ --cc=peng.fan@oss.nxp.com \ --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.