[PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Support i.MX8MN/P MMIO

[PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Support i.MX8MN/P MMIO

[Marek Vasut]

The MX8M CM7 boot via SMC call is problematic, since not all versions
of ATF support this interface. Document MMIO support used to boot the
CM7 on MX8MN/MP instead and discern MMIO interface using DT compatible
string. Document GPR register syscon phandle which is required by the
MMIO interface too.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Note that the MMIO being discerned using compatible string is similar
approach to "st,stm32mp1-rcc" vs "st,stm32mp1-rcc-secure".
---
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-remoteproc@vger.kernel.org
---
 .../bindings/remoteproc/fsl,imx-rproc.yaml    | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
index c940199ce89df..78bb976f738ee 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -20,7 +20,9 @@ properties:
       - fsl,imx7ulp-cm4
       - fsl,imx8mm-cm4
       - fsl,imx8mn-cm7
+      - fsl,imx8mn-cm7-mmio
       - fsl,imx8mp-cm7
+      - fsl,imx8mp-cm7-mmio
       - fsl,imx8mq-cm4
       - fsl,imx8qm-cm4
       - fsl,imx8qxp-cm4
@@ -30,6 +32,11 @@ properties:
   clocks:
     maxItems: 1
 
+  gpr:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to IOMUXC GPR block which provide access to CM7 CPUWAIT bit.
+
   syscon:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
@@ -84,6 +91,19 @@ properties:
 required:
   - compatible
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          not:
+            contains:
+              enum:
+                - fsl,imx8mn-cm7-mmio
+                - fsl,imx8mp-cm7-mmio
+    then:
+      properties:
+        gpr: false
+
 additionalProperties: false
 
 examples:
-- 
2.40.1

[PATCH 2/2] remoteproc: imx_rproc: Switch iMX8MN/MP from SMCCC to MMIO

[Marek Vasut]

The MX8M CM7 boot via SMC call is problematic, since not all versions
of ATF support this interface. Extend the MMIO support so it can boot
the CM7 on MX8MN/MP instead and discern the two alternatives using DT
compatible strings.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-remoteproc@vger.kernel.org
---
 drivers/remoteproc/imx_rproc.c | 53 ++++++++++++++++++++++++++++++++--
 drivers/remoteproc/imx_rproc.h |  2 ++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index d0eb96d6a4fe1..09589f664a2be 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -41,6 +41,12 @@
 #define IMX7D_M4_STOP			(IMX7D_ENABLE_M4 | IMX7D_SW_M4C_RST | \
 					 IMX7D_SW_M4C_NON_SCLR_RST)
 
+#define IMX8M_M7_STOP			(IMX7D_ENABLE_M4 | IMX7D_SW_M4C_RST)
+#define IMX8M_M7_POLL			IMX7D_ENABLE_M4
+
+#define IMX8M_GPR22			0x58
+#define IMX8M_GPR22_CM7_CPUWAIT		BIT(0)
+
 /* Address: 0x020D8000 */
 #define IMX6SX_SRC_SCR			0x00
 #define IMX6SX_ENABLE_M4		BIT(22)
@@ -92,6 +98,7 @@ static int imx_rproc_detach_pd(struct rproc *rproc);
 struct imx_rproc {
 	struct device			*dev;
 	struct regmap			*regmap;
+	struct regmap			*gpr;
 	struct rproc			*rproc;
 	const struct imx_rproc_dcfg	*dcfg;
 	struct imx_rproc_mem		mem[IMX_RPROC_MEM_MAX];
@@ -287,6 +294,18 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
 	{ 0x80000000, 0x80000000, 0x60000000, 0 },
 };
 
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
+	.src_reg	= IMX7D_SRC_SCR,
+	.src_mask	= IMX7D_M4_RST_MASK,
+	.src_start	= IMX7D_M4_START,
+	.src_stop	= IMX8M_M7_STOP,
+	.gpr_reg	= IMX8M_GPR22,
+	.gpr_wait	= IMX8M_GPR22_CM7_CPUWAIT,
+	.att		= imx_rproc_att_imx8mn,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
+	.method		= IMX_RPROC_MMIO,
+};
+
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
 	.att		= imx_rproc_att_imx8mn,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
@@ -367,8 +386,14 @@ static int imx_rproc_start(struct rproc *rproc)
 
 	switch (dcfg->method) {
 	case IMX_RPROC_MMIO:
-		ret = regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask,
-					 dcfg->src_start);
+		if (priv->gpr) {
+			ret = regmap_clear_bits(priv->gpr, dcfg->gpr_reg,
+						dcfg->gpr_wait);
+		} else {
+			ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
+						 dcfg->src_mask,
+						 dcfg->src_start);
+		}
 		break;
 	case IMX_RPROC_SMC:
 		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
@@ -400,6 +425,16 @@ static int imx_rproc_stop(struct rproc *rproc)
 
 	switch (dcfg->method) {
 	case IMX_RPROC_MMIO:
+		if (priv->gpr) {
+			ret = regmap_set_bits(priv->gpr, dcfg->gpr_reg,
+					      dcfg->gpr_wait);
+			if (ret) {
+				dev_err(priv->dev,
+					"Failed to quiescence M4 platform!\n");
+				return ret;
+			}
+		}
+
 		ret = regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask,
 					 dcfg->src_stop);
 		break;
@@ -988,6 +1023,10 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
 		break;
 	}
 
+	priv->gpr = syscon_regmap_lookup_by_phandle(dev->of_node, "gpr");
+	if (IS_ERR(priv->gpr))
+		priv->gpr = NULL;
+
 	regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
 	if (IS_ERR(regmap)) {
 		dev_err(dev, "failed to find syscon\n");
@@ -997,6 +1036,14 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
 	priv->regmap = regmap;
 	regmap_attach_dev(dev, regmap, &config);
 
+	if (priv->gpr) {
+		ret = regmap_read(priv->gpr, dcfg->gpr_reg, &val);
+		if (val & dcfg->gpr_wait) {
+			imx_rproc_stop(priv->rproc);
+			return 0;
+		}
+	}
+
 	ret = regmap_read(regmap, dcfg->src_reg, &val);
 	if (ret) {
 		dev_err(dev, "Failed to read src\n");
@@ -1142,6 +1189,8 @@ static const struct of_device_id imx_rproc_of_match[] = {
 	{ .compatible = "fsl,imx8mm-cm4", .data = &imx_rproc_cfg_imx8mq },
 	{ .compatible = "fsl,imx8mn-cm7", .data = &imx_rproc_cfg_imx8mn },
 	{ .compatible = "fsl,imx8mp-cm7", .data = &imx_rproc_cfg_imx8mn },
+	{ .compatible = "fsl,imx8mn-cm7-mmio", .data = &imx_rproc_cfg_imx8mn_mmio },
+	{ .compatible = "fsl,imx8mp-cm7-mmio", .data = &imx_rproc_cfg_imx8mn_mmio },
 	{ .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 },
diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
index 1c7e2127c7584..79a1b8956d142 100644
--- a/drivers/remoteproc/imx_rproc.h
+++ b/drivers/remoteproc/imx_rproc.h
@@ -31,6 +31,8 @@ struct imx_rproc_dcfg {
 	u32				src_mask;
 	u32				src_start;
 	u32				src_stop;
+	u32				gpr_reg;
+	u32				gpr_wait;
 	const struct imx_rproc_att	*att;
 	size_t				att_size;
 	enum imx_rproc_method		method;
-- 
2.40.1

RE: [PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Support i.MX8MN/P MMIO

[Peng Fan]

> Subject: [PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Support
> i.MX8MN/P MMIO
> 
> The MX8M CM7 boot via SMC call is problematic, since not all versions of
> ATF support this interface. Document MMIO support used to boot the
> CM7 on MX8MN/MP instead and discern MMIO interface using DT
> compatible string. Document GPR register syscon phandle which is required
> by the MMIO interface too.
> 

How about introducing a Boolean property fsl,rproc-mmio and only
valid for i.MX8MN/P?

Regards,
Peng.
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Note that the MMIO being discerned using compatible string is similar
> approach to "st,stm32mp1-rcc" vs "st,stm32mp1-rcc-secure".
> ---
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-remoteproc@vger.kernel.org
> ---
>  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-
> rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-
> rproc.yaml
> index c940199ce89df..78bb976f738ee 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -20,7 +20,9 @@ properties:
>        - fsl,imx7ulp-cm4
>        - fsl,imx8mm-cm4
>        - fsl,imx8mn-cm7
> +      - fsl,imx8mn-cm7-mmio
>        - fsl,imx8mp-cm7
> +      - fsl,imx8mp-cm7-mmio
>        - fsl,imx8mq-cm4
>        - fsl,imx8qm-cm4
>        - fsl,imx8qxp-cm4
> @@ -30,6 +32,11 @@ properties:
>    clocks:
>      maxItems: 1
> 
> +  gpr:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to IOMUXC GPR block which provide access to CM7 CPUWAIT
> bit.
> +
>    syscon:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
> @@ -84,6 +91,19 @@ properties:
>  required:
>    - compatible
> 
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          not:
> +            contains:
> +              enum:
> +                - fsl,imx8mn-cm7-mmio
> +                - fsl,imx8mp-cm7-mmio
> +    then:
> +      properties:
> +        gpr: false
> +
>  additionalProperties: false
> 
>  examples:
> --
> 2.40.1

Re: [PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Support i.MX8MN/P MMIO

[Krzysztof Kozlowski]

On 08/07/2023 01:26, Marek Vasut wrote:
> The MX8M CM7 boot via SMC call is problematic, since not all versions
> of ATF support this interface. Document MMIO support used to boot the
> CM7 on MX8MN/MP instead and discern MMIO interface using DT compatible
> string. Document GPR register syscon phandle which is required by the
> MMIO interface too.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Note that the MMIO being discerned using compatible string is similar
> approach to "st,stm32mp1-rcc" vs "st,stm32mp1-rcc-secure".
> ---

Same hardware should not have two compatibles, depending on the bus it is.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Support i.MX8MN/P MMIO

[Marek Vasut]

On 7/10/23 10:13, Krzysztof Kozlowski wrote:
> On 08/07/2023 01:26, Marek Vasut wrote:
>> The MX8M CM7 boot via SMC call is problematic, since not all versions
>> of ATF support this interface. Document MMIO support used to boot the
>> CM7 on MX8MN/MP instead and discern MMIO interface using DT compatible
>> string. Document GPR register syscon phandle which is required by the
>> MMIO interface too.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Note that the MMIO being discerned using compatible string is similar
>> approach to "st,stm32mp1-rcc" vs "st,stm32mp1-rcc-secure".
>> ---
> 
> Same hardware should not have two compatibles, depending on the bus it is.

Yes, I know, but for whatever reason the ST does, so I picked the same 
approach here. What else would you suggest, boolean properly like Peng 
wrote ?

The compatible is also easier to handle on driver level, at least in 
Linux, see 2/2 .

Re: [PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Support i.MX8MN/P MMIO

[Rob Herring]

On Mon, Jul 10, 2023 at 11:10:48AM +0200, Marek Vasut wrote:
> On 7/10/23 10:13, Krzysztof Kozlowski wrote:
> > On 08/07/2023 01:26, Marek Vasut wrote:
> > > The MX8M CM7 boot via SMC call is problematic, since not all versions
> > > of ATF support this interface. Document MMIO support used to boot the
> > > CM7 on MX8MN/MP instead and discern MMIO interface using DT compatible
> > > string. Document GPR register syscon phandle which is required by the
> > > MMIO interface too.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > ---
> > > Note that the MMIO being discerned using compatible string is similar
> > > approach to "st,stm32mp1-rcc" vs "st,stm32mp1-rcc-secure".
> > > ---
> > 
> > Same hardware should not have two compatibles, depending on the bus it is.
> 
> Yes, I know, but for whatever reason the ST does, so I picked the same
> approach here. What else would you suggest, boolean properly like Peng wrote
> ?
> 
> The compatible is also easier to handle on driver level, at least in Linux,
> see 2/2 .

IIRC, in the ST case, the programming model presented to the OS is 
different, so different compatibles. Seems like this case is similar.

However, can't you try and SMC call and if it fails, use MMIO?

Rob

Re: [PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Support i.MX8MN/P MMIO

[Rob Herring]

On Sat, Jul 08, 2023 at 01:26:25AM +0200, Marek Vasut wrote:
> The MX8M CM7 boot via SMC call is problematic, since not all versions
> of ATF support this interface. Document MMIO support used to boot the
> CM7 on MX8MN/MP instead and discern MMIO interface using DT compatible
> string. Document GPR register syscon phandle which is required by the
> MMIO interface too.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Note that the MMIO being discerned using compatible string is similar
> approach to "st,stm32mp1-rcc" vs "st,stm32mp1-rcc-secure".
> ---
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-remoteproc@vger.kernel.org
> ---
>  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index c940199ce89df..78bb976f738ee 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -20,7 +20,9 @@ properties:
>        - fsl,imx7ulp-cm4
>        - fsl,imx8mm-cm4
>        - fsl,imx8mn-cm7
> +      - fsl,imx8mn-cm7-mmio
>        - fsl,imx8mp-cm7
> +      - fsl,imx8mp-cm7-mmio
>        - fsl,imx8mq-cm4
>        - fsl,imx8qm-cm4
>        - fsl,imx8qxp-cm4
> @@ -30,6 +32,11 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  gpr:

fsl,gpr or fsl,iomuxc-gpr

> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to IOMUXC GPR block which provide access to CM7 CPUWAIT bit.
> +
>    syscon:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
> @@ -84,6 +91,19 @@ properties:
>  required:
>    - compatible
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          not:
> +            contains:
> +              enum:
> +                - fsl,imx8mn-cm7-mmio
> +                - fsl,imx8mp-cm7-mmio
> +    then:
> +      properties:
> +        gpr: false
> +
>  additionalProperties: false
>  
>  examples:
> -- 
> 2.40.1
> 

Re: [PATCH 1/2] dt-bindings: remoteproc: imx_rproc: Support i.MX8MN/P MMIO

[Marek Vasut]

On 7/10/23 17:09, Rob Herring wrote:
> On Mon, Jul 10, 2023 at 11:10:48AM +0200, Marek Vasut wrote:
>> On 7/10/23 10:13, Krzysztof Kozlowski wrote:
>>> On 08/07/2023 01:26, Marek Vasut wrote:
>>>> The MX8M CM7 boot via SMC call is problematic, since not all versions
>>>> of ATF support this interface. Document MMIO support used to boot the
>>>> CM7 on MX8MN/MP instead and discern MMIO interface using DT compatible
>>>> string. Document GPR register syscon phandle which is required by the
>>>> MMIO interface too.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> ---
>>>> Note that the MMIO being discerned using compatible string is similar
>>>> approach to "st,stm32mp1-rcc" vs "st,stm32mp1-rcc-secure".
>>>> ---
>>>
>>> Same hardware should not have two compatibles, depending on the bus it is.
>>
>> Yes, I know, but for whatever reason the ST does, so I picked the same
>> approach here. What else would you suggest, boolean properly like Peng wrote
>> ?
>>
>> The compatible is also easier to handle on driver level, at least in Linux,
>> see 2/2 .
> 
> IIRC, in the ST case, the programming model presented to the OS is
> different, so different compatibles. Seems like this case is similar.
> 
> However, can't you try and SMC call and if it fails, use MMIO?

They already do that to determine the CM4 CPU state, and if the SMC 
errors out, then they indicate the CPU is detached. So no, not to my 
knowledge, which indeed would be my preferred solution though.

Re: [PATCH 2/2] remoteproc: imx_rproc: Switch iMX8MN/MP from SMCCC to MMIO

[Mathieu Poirier]

Good morning,

On Sat, Jul 08, 2023 at 01:26:26AM +0200, Marek Vasut wrote:
> The MX8M CM7 boot via SMC call is problematic, since not all versions
> of ATF support this interface. Extend the MMIO support so it can boot
> the CM7 on MX8MN/MP instead and discern the two alternatives using DT
> compatible strings.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-remoteproc@vger.kernel.org
> ---
>  drivers/remoteproc/imx_rproc.c | 53 ++++++++++++++++++++++++++++++++--
>  drivers/remoteproc/imx_rproc.h |  2 ++
>  2 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index d0eb96d6a4fe1..09589f664a2be 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -41,6 +41,12 @@
>  #define IMX7D_M4_STOP			(IMX7D_ENABLE_M4 | IMX7D_SW_M4C_RST | \
>  					 IMX7D_SW_M4C_NON_SCLR_RST)
>  
> +#define IMX8M_M7_STOP			(IMX7D_ENABLE_M4 | IMX7D_SW_M4C_RST)
> +#define IMX8M_M7_POLL			IMX7D_ENABLE_M4
> +
> +#define IMX8M_GPR22			0x58
> +#define IMX8M_GPR22_CM7_CPUWAIT		BIT(0)
> +
>  /* Address: 0x020D8000 */
>  #define IMX6SX_SRC_SCR			0x00
>  #define IMX6SX_ENABLE_M4		BIT(22)
> @@ -92,6 +98,7 @@ static int imx_rproc_detach_pd(struct rproc *rproc);
>  struct imx_rproc {
>  	struct device			*dev;
>  	struct regmap			*regmap;
> +	struct regmap			*gpr;
>  	struct rproc			*rproc;
>  	const struct imx_rproc_dcfg	*dcfg;
>  	struct imx_rproc_mem		mem[IMX_RPROC_MEM_MAX];
> @@ -287,6 +294,18 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
>  	{ 0x80000000, 0x80000000, 0x60000000, 0 },
>  };
>  
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
> +	.src_reg	= IMX7D_SRC_SCR,
> +	.src_mask	= IMX7D_M4_RST_MASK,
> +	.src_start	= IMX7D_M4_START,
> +	.src_stop	= IMX8M_M7_STOP,
> +	.gpr_reg	= IMX8M_GPR22,
> +	.gpr_wait	= IMX8M_GPR22_CM7_CPUWAIT,
> +	.att		= imx_rproc_att_imx8mn,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
> +	.method		= IMX_RPROC_MMIO,
> +};
> +
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
>  	.att		= imx_rproc_att_imx8mn,
>  	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
> @@ -367,8 +386,14 @@ static int imx_rproc_start(struct rproc *rproc)
>  
>  	switch (dcfg->method) {
>  	case IMX_RPROC_MMIO:
> -		ret = regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask,
> -					 dcfg->src_start);
> +		if (priv->gpr) {
> +			ret = regmap_clear_bits(priv->gpr, dcfg->gpr_reg,
> +						dcfg->gpr_wait);
> +		} else {
> +			ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
> +						 dcfg->src_mask,
> +						 dcfg->src_start);
> +		}
>  		break;
>  	case IMX_RPROC_SMC:
>  		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
> @@ -400,6 +425,16 @@ static int imx_rproc_stop(struct rproc *rproc)
>  
>  	switch (dcfg->method) {
>  	case IMX_RPROC_MMIO:
> +		if (priv->gpr) {
> +			ret = regmap_set_bits(priv->gpr, dcfg->gpr_reg,
> +					      dcfg->gpr_wait);
> +			if (ret) {
> +				dev_err(priv->dev,
> +					"Failed to quiescence M4 platform!\n");
> +				return ret;
> +			}
> +		}
> +
>  		ret = regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask,
>  					 dcfg->src_stop);
>  		break;
> @@ -988,6 +1023,10 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  		break;
>  	}
>  
> +	priv->gpr = syscon_regmap_lookup_by_phandle(dev->of_node, "gpr");
> +	if (IS_ERR(priv->gpr))
> +		priv->gpr = NULL;
> +
>  	regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
>  	if (IS_ERR(regmap)) {
>  		dev_err(dev, "failed to find syscon\n");
> @@ -997,6 +1036,14 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  	priv->regmap = regmap;
>  	regmap_attach_dev(dev, regmap, &config);
>  
> +	if (priv->gpr) {
> +		ret = regmap_read(priv->gpr, dcfg->gpr_reg, &val);
> +		if (val & dcfg->gpr_wait) {
> +			imx_rproc_stop(priv->rproc);
> +			return 0;
> +		}
> +	}

Can you provide a comment to explain what is happening above?  In what kind of
scenario would a GPR be set with the wait bit on startup?

Thanks,
Mathieu

> +
>  	ret = regmap_read(regmap, dcfg->src_reg, &val);
>  	if (ret) {
>  		dev_err(dev, "Failed to read src\n");
> @@ -1142,6 +1189,8 @@ static const struct of_device_id imx_rproc_of_match[] = {
>  	{ .compatible = "fsl,imx8mm-cm4", .data = &imx_rproc_cfg_imx8mq },
>  	{ .compatible = "fsl,imx8mn-cm7", .data = &imx_rproc_cfg_imx8mn },
>  	{ .compatible = "fsl,imx8mp-cm7", .data = &imx_rproc_cfg_imx8mn },
> +	{ .compatible = "fsl,imx8mn-cm7-mmio", .data = &imx_rproc_cfg_imx8mn_mmio },
> +	{ .compatible = "fsl,imx8mp-cm7-mmio", .data = &imx_rproc_cfg_imx8mn_mmio },
>  	{ .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 },
> diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
> index 1c7e2127c7584..79a1b8956d142 100644
> --- a/drivers/remoteproc/imx_rproc.h
> +++ b/drivers/remoteproc/imx_rproc.h
> @@ -31,6 +31,8 @@ struct imx_rproc_dcfg {
>  	u32				src_mask;
>  	u32				src_start;
>  	u32				src_stop;
> +	u32				gpr_reg;
> +	u32				gpr_wait;
>  	const struct imx_rproc_att	*att;
>  	size_t				att_size;
>  	enum imx_rproc_method		method;
> -- 
> 2.40.1
> 

Re: [PATCH 2/2] remoteproc: imx_rproc: Switch iMX8MN/MP from SMCCC to MMIO

[Marek Vasut]

On 7/17/23 19:10, Mathieu Poirier wrote:
> Good morning,
> 
> On Sat, Jul 08, 2023 at 01:26:26AM +0200, Marek Vasut wrote:
>> The MX8M CM7 boot via SMC call is problematic, since not all versions
>> of ATF support this interface. Extend the MMIO support so it can boot
>> the CM7 on MX8MN/MP instead and discern the two alternatives using DT
>> compatible strings.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Bjorn Andersson <andersson@kernel.org>
>> Cc: Conor Dooley <conor+dt@kernel.org>
>> Cc: Fabio Estevam <festevam@gmail.com>
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: NXP Linux Team <linux-imx@nxp.com>
>> Cc: Peng Fan <peng.fan@nxp.com>
>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-remoteproc@vger.kernel.org
>> ---
>>   drivers/remoteproc/imx_rproc.c | 53 ++++++++++++++++++++++++++++++++--
>>   drivers/remoteproc/imx_rproc.h |  2 ++
>>   2 files changed, 53 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
>> index d0eb96d6a4fe1..09589f664a2be 100644
>> --- a/drivers/remoteproc/imx_rproc.c
>> +++ b/drivers/remoteproc/imx_rproc.c
>> @@ -41,6 +41,12 @@
>>   #define IMX7D_M4_STOP			(IMX7D_ENABLE_M4 | IMX7D_SW_M4C_RST | \
>>   					 IMX7D_SW_M4C_NON_SCLR_RST)
>>   
>> +#define IMX8M_M7_STOP			(IMX7D_ENABLE_M4 | IMX7D_SW_M4C_RST)
>> +#define IMX8M_M7_POLL			IMX7D_ENABLE_M4
>> +
>> +#define IMX8M_GPR22			0x58
>> +#define IMX8M_GPR22_CM7_CPUWAIT		BIT(0)
>> +
>>   /* Address: 0x020D8000 */
>>   #define IMX6SX_SRC_SCR			0x00
>>   #define IMX6SX_ENABLE_M4		BIT(22)
>> @@ -92,6 +98,7 @@ static int imx_rproc_detach_pd(struct rproc *rproc);
>>   struct imx_rproc {
>>   	struct device			*dev;
>>   	struct regmap			*regmap;
>> +	struct regmap			*gpr;
>>   	struct rproc			*rproc;
>>   	const struct imx_rproc_dcfg	*dcfg;
>>   	struct imx_rproc_mem		mem[IMX_RPROC_MEM_MAX];
>> @@ -287,6 +294,18 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
>>   	{ 0x80000000, 0x80000000, 0x60000000, 0 },
>>   };
>>   
>> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
>> +	.src_reg	= IMX7D_SRC_SCR,
>> +	.src_mask	= IMX7D_M4_RST_MASK,
>> +	.src_start	= IMX7D_M4_START,
>> +	.src_stop	= IMX8M_M7_STOP,
>> +	.gpr_reg	= IMX8M_GPR22,
>> +	.gpr_wait	= IMX8M_GPR22_CM7_CPUWAIT,
>> +	.att		= imx_rproc_att_imx8mn,
>> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
>> +	.method		= IMX_RPROC_MMIO,
>> +};
>> +
>>   static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
>>   	.att		= imx_rproc_att_imx8mn,
>>   	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
>> @@ -367,8 +386,14 @@ static int imx_rproc_start(struct rproc *rproc)
>>   
>>   	switch (dcfg->method) {
>>   	case IMX_RPROC_MMIO:
>> -		ret = regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask,
>> -					 dcfg->src_start);
>> +		if (priv->gpr) {
>> +			ret = regmap_clear_bits(priv->gpr, dcfg->gpr_reg,
>> +						dcfg->gpr_wait);
>> +		} else {
>> +			ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
>> +						 dcfg->src_mask,
>> +						 dcfg->src_start);
>> +		}
>>   		break;
>>   	case IMX_RPROC_SMC:
>>   		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
>> @@ -400,6 +425,16 @@ static int imx_rproc_stop(struct rproc *rproc)
>>   
>>   	switch (dcfg->method) {
>>   	case IMX_RPROC_MMIO:
>> +		if (priv->gpr) {
>> +			ret = regmap_set_bits(priv->gpr, dcfg->gpr_reg,
>> +					      dcfg->gpr_wait);
>> +			if (ret) {
>> +				dev_err(priv->dev,
>> +					"Failed to quiescence M4 platform!\n");
>> +				return ret;
>> +			}
>> +		}
>> +
>>   		ret = regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask,
>>   					 dcfg->src_stop);
>>   		break;
>> @@ -988,6 +1023,10 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>>   		break;
>>   	}
>>   
>> +	priv->gpr = syscon_regmap_lookup_by_phandle(dev->of_node, "gpr");
>> +	if (IS_ERR(priv->gpr))
>> +		priv->gpr = NULL;
>> +
>>   	regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
>>   	if (IS_ERR(regmap)) {
>>   		dev_err(dev, "failed to find syscon\n");
>> @@ -997,6 +1036,14 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>>   	priv->regmap = regmap;
>>   	regmap_attach_dev(dev, regmap, &config);
>>   
>> +	if (priv->gpr) {
>> +		ret = regmap_read(priv->gpr, dcfg->gpr_reg, &val);
>> +		if (val & dcfg->gpr_wait) {
>> +			imx_rproc_stop(priv->rproc);
>> +			return 0;
>> +		}
>> +	}
> 
> Can you provide a comment to explain what is happening above?  In what kind of
> scenario would a GPR be set with the wait bit on startup?

This happens right after power-on reset, the core indicates its in wait 
state in the GPR, but its not fully stopped, so we stop it and then the 
firmware can be loaded into it.