| Message ID | 20220517064937.4033441-6-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | remoteproc: imx_rproc: support i.MX8QM/QXP | expand |
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. 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 >
> 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 > >
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; + } + 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 }, {},