[PATCH 0/6] remoteproc: imx_rproc: various patches for misc

[PATCH 0/6] remoteproc: imx_rproc: various patches for misc

[Peng Fan (OSS)]

This patchset is to upstream a few patches that in NXP downstream for
quite sometime. For patches directly cherry-picked from NXP downstream,
I keep the R-b tags.

Patch 1 is a minor fix to DDR alias.
Patch 2 was sent out before,
https://patchwork.kernel.org/project/linux-remoteproc/patch/20220111033333.403448-1-peng.fan@oss.nxp.com/#25144792
this is just a resend
Patch 3 is to avoid mu interrupt trigger earlier.
Patch 4 is merge small area to support elf that has large section
Patch 5 and Patch 6 is i.MX7ULP requires a message before linux
reboot/poweroff, so need use non block mu to send message.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (6):
      remoteproc: imx_rproc: correct ddr alias for i.MX8M
      remoteproc: imx_rproc: use imx specific hook for find_loaded_rsc_table
      remoteproc: imx_rproc: initialize workqueue earlier
      remoteproc: imx_rproc: merge TCML/U
      remoteproc: imx_rproc: allow tx_block to be set
      remoteproc: imx_rproc: handle system off for i.MX7ULP

 drivers/remoteproc/imx_rproc.c | 85 +++++++++++++++++++++++++++++++-----------
 1 file changed, 63 insertions(+), 22 deletions(-)
---
base-commit: f477dd6eede3ecedc8963478571d99ec3bf3f762
change-id: 20240712-imx_rproc-25f3ab753c58

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH 1/6] remoteproc: imx_rproc: correct ddr alias for i.MX8M

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

The DDR Alias address should be 0x40000000 according to RM, so correct
it.

Fixes: 4ab8f9607aad ("remoteproc: imx_rproc: support i.MX8MQ/M")
Reported-by: Terry Lv <terry.lv@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 144c8e9a642e..3c8b64db8823 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -210,7 +210,7 @@ static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
 	/* QSPI Code - alias */
 	{ 0x08000000, 0x08000000, 0x08000000, 0 },
 	/* DDR (Code) - alias */
-	{ 0x10000000, 0x80000000, 0x0FFE0000, 0 },
+	{ 0x10000000, 0x40000000, 0x0FFE0000, 0 },
 	/* TCML */
 	{ 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN  | ATT_IOMEM},
 	/* TCMU */

-- 
2.37.1

[PATCH 2/6] remoteproc: imx_rproc: use imx specific hook for find_loaded_rsc_table

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

If there is a resource table device tree node, use the address as
the resource table address, otherwise use the address(where
.resource_table section loaded) inside the Cortex-M elf file.

And there is an update in NXP SDK that Resource Domain Control(RDC)
enabled to protect TCM, linux not able to write the TCM space when
updating resource table status and cause kernel dump. So use the address
from device tree could avoid kernel dump.

Note: NXP M4 SDK not check resource table update, so it does not matter
use whether resource table address specified in elf file or in device
tree. But to reflect the fact that if people specific resource table
address in device tree, it means people are aware and going to use it,
not the address specified in elf file.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 3c8b64db8823..48c48b53a3aa 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -666,6 +666,17 @@ static struct resource_table *imx_rproc_get_loaded_rsc_table(struct rproc *rproc
 	return (struct resource_table *)priv->rsc_table;
 }
 
+static struct resource_table *
+imx_rproc_elf_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
+{
+	struct imx_rproc *priv = rproc->priv;
+
+	if (priv->rsc_table)
+		return (struct resource_table *)priv->rsc_table;
+
+	return rproc_elf_find_loaded_rsc_table(rproc, fw);
+}
+
 static const struct rproc_ops imx_rproc_ops = {
 	.prepare	= imx_rproc_prepare,
 	.attach		= imx_rproc_attach,
@@ -676,7 +687,7 @@ static const struct rproc_ops imx_rproc_ops = {
 	.da_to_va       = imx_rproc_da_to_va,
 	.load		= rproc_elf_load_segments,
 	.parse_fw	= imx_rproc_parse_fw,
-	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+	.find_loaded_rsc_table = imx_rproc_elf_find_loaded_rsc_table,
 	.get_loaded_rsc_table = imx_rproc_get_loaded_rsc_table,
 	.sanity_check	= rproc_elf_sanity_check,
 	.get_boot_addr	= rproc_elf_get_boot_addr,

-- 
2.37.1

[PATCH 3/6] remoteproc: imx_rproc: initialize workqueue earlier

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Initialize workqueue before requesting mailbox channel, otherwise if
mailbox interrupt comes before workqueue ready, the imx_rproc_rx_callback
will trigger issue.

Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 48c48b53a3aa..9e99bb27c033 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -1087,6 +1087,8 @@ static int imx_rproc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	INIT_WORK(&priv->rproc_work, imx_rproc_vq_work);
+
 	ret = imx_rproc_xtr_mbox_init(rproc);
 	if (ret)
 		goto err_put_wkq;
@@ -1105,8 +1107,6 @@ static int imx_rproc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_put_scu;
 
-	INIT_WORK(&priv->rproc_work, imx_rproc_vq_work);
-
 	if (rproc->state != RPROC_DETACHED)
 		rproc->auto_boot = of_property_read_bool(np, "fsl,auto-boot");
 

-- 
2.37.1

[PATCH 4/6] remoteproc: imx_rproc: merge TCML/U

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Merge contiguous TCML/U regions into one to avoid load elf files which
has large sections failure.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 9e99bb27c033..552fccebf7e2 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -119,20 +119,16 @@ struct imx_rproc {
 static const struct imx_rproc_att imx_rproc_att_imx93[] = {
 	/* dev addr , sys addr  , size	    , flags */
 	/* TCM CODE NON-SECURE */
-	{ 0x0FFC0000, 0x201C0000, 0x00020000, ATT_OWN | ATT_IOMEM },
-	{ 0x0FFE0000, 0x201E0000, 0x00020000, ATT_OWN | ATT_IOMEM },
+	{ 0x0FFC0000, 0x201C0000, 0x00040000, ATT_OWN | ATT_IOMEM },
 
 	/* TCM CODE SECURE */
-	{ 0x1FFC0000, 0x201C0000, 0x00020000, ATT_OWN | ATT_IOMEM },
-	{ 0x1FFE0000, 0x201E0000, 0x00020000, ATT_OWN | ATT_IOMEM },
+	{ 0x1FFC0000, 0x201C0000, 0x00040000, ATT_OWN | ATT_IOMEM },
 
 	/* TCM SYS NON-SECURE*/
-	{ 0x20000000, 0x20200000, 0x00020000, ATT_OWN | ATT_IOMEM },
-	{ 0x20020000, 0x20220000, 0x00020000, ATT_OWN | ATT_IOMEM },
+	{ 0x20000000, 0x20200000, 0x00040000, ATT_OWN | ATT_IOMEM },
 
 	/* TCM SYS SECURE*/
-	{ 0x30000000, 0x20200000, 0x00020000, ATT_OWN | ATT_IOMEM },
-	{ 0x30020000, 0x20220000, 0x00020000, ATT_OWN | ATT_IOMEM },
+	{ 0x30000000, 0x20200000, 0x00040000, ATT_OWN | ATT_IOMEM },
 
 	/* DDR */
 	{ 0x80000000, 0x80000000, 0x10000000, 0 },
@@ -211,10 +207,8 @@ static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
 	{ 0x08000000, 0x08000000, 0x08000000, 0 },
 	/* DDR (Code) - alias */
 	{ 0x10000000, 0x40000000, 0x0FFE0000, 0 },
-	/* TCML */
-	{ 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN  | ATT_IOMEM},
-	/* TCMU */
-	{ 0x20000000, 0x00800000, 0x00020000, ATT_OWN  | ATT_IOMEM},
+	/* TCML/U */
+	{ 0x1FFE0000, 0x007E0000, 0x00040000, ATT_OWN  | ATT_IOMEM},
 	/* OCRAM_S */
 	{ 0x20180000, 0x00180000, 0x00008000, ATT_OWN },
 	/* OCRAM */

-- 
2.37.1

[PATCH 5/6] remoteproc: imx_rproc: allow tx_block to be set

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Current tx_block is set to true, but there is case that no need to wait
response. Linux just needs to send data to remote processor, so let's
allow tx_block could be set to false.

Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 552fccebf7e2..01cf1dfb2e87 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -90,7 +90,7 @@ struct imx_rproc_mem {
 #define ATT_CORE_MASK   0xffff
 #define ATT_CORE(I)     BIT((I))
 
-static int imx_rproc_xtr_mbox_init(struct rproc *rproc);
+static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block);
 static void imx_rproc_free_mbox(struct rproc *rproc);
 
 struct imx_rproc {
@@ -369,7 +369,7 @@ static int imx_rproc_start(struct rproc *rproc)
 	struct arm_smccc_res res;
 	int ret;
 
-	ret = imx_rproc_xtr_mbox_init(rproc);
+	ret = imx_rproc_xtr_mbox_init(rproc, true);
 	if (ret)
 		return ret;
 
@@ -629,7 +629,7 @@ static void imx_rproc_kick(struct rproc *rproc, int vqid)
 
 static int imx_rproc_attach(struct rproc *rproc)
 {
-	return imx_rproc_xtr_mbox_init(rproc);
+	return imx_rproc_xtr_mbox_init(rproc, true);
 }
 
 static int imx_rproc_detach(struct rproc *rproc)
@@ -794,7 +794,7 @@ static void imx_rproc_rx_callback(struct mbox_client *cl, void *msg)
 	queue_work(priv->workqueue, &priv->rproc_work);
 }
 
-static int imx_rproc_xtr_mbox_init(struct rproc *rproc)
+static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)
 {
 	struct imx_rproc *priv = rproc->priv;
 	struct device *dev = priv->dev;
@@ -817,7 +817,7 @@ static int imx_rproc_xtr_mbox_init(struct rproc *rproc)
 
 	cl = &priv->cl;
 	cl->dev = dev;
-	cl->tx_block = true;
+	cl->tx_block = tx_block;
 	cl->tx_tout = 100;
 	cl->knows_txdone = false;
 	cl->rx_callback = imx_rproc_rx_callback;
@@ -1083,7 +1083,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
 
 	INIT_WORK(&priv->rproc_work, imx_rproc_vq_work);
 
-	ret = imx_rproc_xtr_mbox_init(rproc);
+	ret = imx_rproc_xtr_mbox_init(rproc, true);
 	if (ret)
 		goto err_put_wkq;
 

-- 
2.37.1

[PATCH 6/6] remoteproc: imx_rproc: handle system off for i.MX7ULP

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

The i.MX7ULP Cortex-A7 is under control of Cortex-M4. The i.MX7ULP Linux
poweroff and restart rely on rpmsg driver to send a message to Cortex-M4
firmware. Then Cortex-A7 could poweroff or restart by Cortex-M4 to
configure the i.MX7ULP power controller properly.

However the reboot and restart kernel common code use atomic notifier,
so with blocking tx mailbox will trigger kernel dump, because of
blocking mailbox will use wait_for_completion_timeout. In such case,
linux no need to wait for completion.

Current patch is to use non-blocking tx mailbox channel when system
is going to poweroff or restart.

Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 01cf1dfb2e87..e1abf110abc9 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -18,6 +18,7 @@
 #include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
+#include <linux/reboot.h>
 #include <linux/regmap.h>
 #include <linux/remoteproc.h>
 #include <linux/workqueue.h>
@@ -114,6 +115,7 @@ struct imx_rproc {
 	u32				entry;		/* cpu start address */
 	u32				core_index;
 	struct dev_pm_domain_list	*pd_list;
+	struct sys_off_data		data;
 };
 
 static const struct imx_rproc_att imx_rproc_att_imx93[] = {
@@ -1050,6 +1052,22 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv)
 	return 0;
 }
 
+static int imx_rproc_sys_off_handler(struct sys_off_data *data)
+{
+	struct rproc *rproc = data->cb_data;
+	int ret;
+
+	imx_rproc_free_mbox(rproc);
+
+	ret = imx_rproc_xtr_mbox_init(rproc, false);
+	if (ret) {
+		dev_err(&rproc->dev, "Failed to request non-blocking mbox\n");
+		return NOTIFY_BAD;
+	}
+
+	return NOTIFY_DONE;
+}
+
 static int imx_rproc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1104,6 +1122,24 @@ static int imx_rproc_probe(struct platform_device *pdev)
 	if (rproc->state != RPROC_DETACHED)
 		rproc->auto_boot = of_property_read_bool(np, "fsl,auto-boot");
 
+	if (of_device_is_compatible(dev->of_node, "fsl,imx7ulp-cm4")) {
+		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_POWER_OFF_PREPARE,
+						    SYS_OFF_PRIO_DEFAULT,
+						    imx_rproc_sys_off_handler, rproc);
+		if (ret) {
+			dev_err(dev, "register power off handler failure\n");
+			goto err_put_clk;
+		}
+
+		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_RESTART_PREPARE,
+						    SYS_OFF_PRIO_DEFAULT,
+						    imx_rproc_sys_off_handler, rproc);
+		if (ret) {
+			dev_err(dev, "register restart handler failure\n");
+			goto err_put_clk;
+		}
+	}
+
 	ret = rproc_add(rproc);
 	if (ret) {
 		dev_err(dev, "rproc_add failed\n");

-- 
2.37.1

Re: [PATCH 1/6] remoteproc: imx_rproc: correct ddr alias for i.MX8M

[Mathieu Poirier]

Good morning,

On Fri, Jul 12, 2024 at 04:34:54PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The DDR Alias address should be 0x40000000 according to RM, so correct
> it.
> 
> Fixes: 4ab8f9607aad ("remoteproc: imx_rproc: support i.MX8MQ/M")

This commit was merged more than 3 years ago...  I don't see how such a blatant
mistake could have survived this long without causing problems or being noticed.
On top of things checkpatch gives me a warning.

> Reported-by: Terry Lv <terry.lv@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 144c8e9a642e..3c8b64db8823 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -210,7 +210,7 @@ static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
>  	/* QSPI Code - alias */
>  	{ 0x08000000, 0x08000000, 0x08000000, 0 },
>  	/* DDR (Code) - alias */
> -	{ 0x10000000, 0x80000000, 0x0FFE0000, 0 },
> +	{ 0x10000000, 0x40000000, 0x0FFE0000, 0 },

Without access to HW or the documentation there is no way for me to assess the
validity of this patch.  Marek, Iuliana and Daniel - please review and test this
patch.

Thanks,
Mathieu

>  	/* TCML */
>  	{ 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN  | ATT_IOMEM},
>  	/* TCMU */
> 
> -- 
> 2.37.1
> 

Re: [PATCH 3/6] remoteproc: imx_rproc: initialize workqueue earlier

[Mathieu Poirier]

On Fri, Jul 12, 2024 at 04:34:56PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Initialize workqueue before requesting mailbox channel, otherwise if
> mailbox interrupt comes before workqueue ready, the imx_rproc_rx_callback
> will trigger issue.
> 
> Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>

All reviews should be done publicly - please remove here and for all the other
patches.

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 48c48b53a3aa..9e99bb27c033 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -1087,6 +1087,8 @@ static int imx_rproc_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> +	INIT_WORK(&priv->rproc_work, imx_rproc_vq_work);
> +

There should be a "Fixes:" tag on this patch.

>  	ret = imx_rproc_xtr_mbox_init(rproc);
>  	if (ret)
>  		goto err_put_wkq;
> @@ -1105,8 +1107,6 @@ static int imx_rproc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_put_scu;
>  
> -	INIT_WORK(&priv->rproc_work, imx_rproc_vq_work);
> -
>  	if (rproc->state != RPROC_DETACHED)
>  		rproc->auto_boot = of_property_read_bool(np, "fsl,auto-boot");
>  
> 
> -- 
> 2.37.1
> 

Re: [PATCH 4/6] remoteproc: imx_rproc: merge TCML/U

[Mathieu Poirier]

On Fri, Jul 12, 2024 at 04:34:57PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Merge contiguous TCML/U regions into one to avoid load elf files which
> has large sections failure.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 9e99bb27c033..552fccebf7e2 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -119,20 +119,16 @@ struct imx_rproc {
>  static const struct imx_rproc_att imx_rproc_att_imx93[] = {
>  	/* dev addr , sys addr  , size	    , flags */
>  	/* TCM CODE NON-SECURE */
> -	{ 0x0FFC0000, 0x201C0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> -	{ 0x0FFE0000, 0x201E0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> +	{ 0x0FFC0000, 0x201C0000, 0x00040000, ATT_OWN | ATT_IOMEM },
>  
>  	/* TCM CODE SECURE */
> -	{ 0x1FFC0000, 0x201C0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> -	{ 0x1FFE0000, 0x201E0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> +	{ 0x1FFC0000, 0x201C0000, 0x00040000, ATT_OWN | ATT_IOMEM },
>  
>  	/* TCM SYS NON-SECURE*/
> -	{ 0x20000000, 0x20200000, 0x00020000, ATT_OWN | ATT_IOMEM },
> -	{ 0x20020000, 0x20220000, 0x00020000, ATT_OWN | ATT_IOMEM },
> +	{ 0x20000000, 0x20200000, 0x00040000, ATT_OWN | ATT_IOMEM },
>  
>  	/* TCM SYS SECURE*/
> -	{ 0x30000000, 0x20200000, 0x00020000, ATT_OWN | ATT_IOMEM },
> -	{ 0x30020000, 0x20220000, 0x00020000, ATT_OWN | ATT_IOMEM },
> +	{ 0x30000000, 0x20200000, 0x00040000, ATT_OWN | ATT_IOMEM },
>  
>  	/* DDR */
>  	{ 0x80000000, 0x80000000, 0x10000000, 0 },
> @@ -211,10 +207,8 @@ static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
>  	{ 0x08000000, 0x08000000, 0x08000000, 0 },
>  	/* DDR (Code) - alias */
>  	{ 0x10000000, 0x40000000, 0x0FFE0000, 0 },
> -	/* TCML */
> -	{ 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN  | ATT_IOMEM},
> -	/* TCMU */
> -	{ 0x20000000, 0x00800000, 0x00020000, ATT_OWN  | ATT_IOMEM},
> +	/* TCML/U */
> +	{ 0x1FFE0000, 0x007E0000, 0x00040000, ATT_OWN  | ATT_IOMEM},

Here too I will need an RB tag by Marek, Iuliana or Daniel.

>  	/* OCRAM_S */
>  	{ 0x20180000, 0x00180000, 0x00008000, ATT_OWN },
>  	/* OCRAM */
> 
> -- 
> 2.37.1
> 

Re: [PATCH 5/6] remoteproc: imx_rproc: allow tx_block to be set

[Mathieu Poirier]

On Fri, Jul 12, 2024 at 04:34:58PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Current tx_block is set to true, but there is case that no need to wait
> response. Linux just needs to send data to remote processor, so let's
> allow tx_block could be set to false.

Ok, maybe - but this patch doesn't use the new functionality, i.e nothing
changes.  

> 
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 552fccebf7e2..01cf1dfb2e87 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -90,7 +90,7 @@ struct imx_rproc_mem {
>  #define ATT_CORE_MASK   0xffff
>  #define ATT_CORE(I)     BIT((I))
>  
> -static int imx_rproc_xtr_mbox_init(struct rproc *rproc);
> +static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block);
>  static void imx_rproc_free_mbox(struct rproc *rproc);
>  
>  struct imx_rproc {
> @@ -369,7 +369,7 @@ static int imx_rproc_start(struct rproc *rproc)
>  	struct arm_smccc_res res;
>  	int ret;
>  
> -	ret = imx_rproc_xtr_mbox_init(rproc);
> +	ret = imx_rproc_xtr_mbox_init(rproc, true);
>  	if (ret)
>  		return ret;
>  
> @@ -629,7 +629,7 @@ static void imx_rproc_kick(struct rproc *rproc, int vqid)
>  
>  static int imx_rproc_attach(struct rproc *rproc)
>  {
> -	return imx_rproc_xtr_mbox_init(rproc);
> +	return imx_rproc_xtr_mbox_init(rproc, true);
>  }
>  
>  static int imx_rproc_detach(struct rproc *rproc)
> @@ -794,7 +794,7 @@ static void imx_rproc_rx_callback(struct mbox_client *cl, void *msg)
>  	queue_work(priv->workqueue, &priv->rproc_work);
>  }
>  
> -static int imx_rproc_xtr_mbox_init(struct rproc *rproc)
> +static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)
>  {
>  	struct imx_rproc *priv = rproc->priv;
>  	struct device *dev = priv->dev;
> @@ -817,7 +817,7 @@ static int imx_rproc_xtr_mbox_init(struct rproc *rproc)
>  
>  	cl = &priv->cl;
>  	cl->dev = dev;
> -	cl->tx_block = true;
> +	cl->tx_block = tx_block;
>  	cl->tx_tout = 100;
>  	cl->knows_txdone = false;
>  	cl->rx_callback = imx_rproc_rx_callback;
> @@ -1083,7 +1083,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
>  
>  	INIT_WORK(&priv->rproc_work, imx_rproc_vq_work);
>  
> -	ret = imx_rproc_xtr_mbox_init(rproc);
> +	ret = imx_rproc_xtr_mbox_init(rproc, true);
>  	if (ret)
>  		goto err_put_wkq;
>  
> 
> -- 
> 2.37.1
> 

Re: [PATCH 6/6] remoteproc: imx_rproc: handle system off for i.MX7ULP

[Mathieu Poirier]

On Fri, Jul 12, 2024 at 04:34:59PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The i.MX7ULP Cortex-A7 is under control of Cortex-M4. The i.MX7ULP Linux
> poweroff and restart rely on rpmsg driver to send a message to Cortex-M4
> firmware. Then Cortex-A7 could poweroff or restart by Cortex-M4 to
> configure the i.MX7ULP power controller properly.
> 
> However the reboot and restart kernel common code use atomic notifier,
> so with blocking tx mailbox will trigger kernel dump, because of
> blocking mailbox will use wait_for_completion_timeout. In such case,
> linux no need to wait for completion.
> 
> Current patch is to use non-blocking tx mailbox channel when system
> is going to poweroff or restart.
> 
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 01cf1dfb2e87..e1abf110abc9 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -18,6 +18,7 @@
>  #include <linux/of_reserved_mem.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
> +#include <linux/reboot.h>
>  #include <linux/regmap.h>
>  #include <linux/remoteproc.h>
>  #include <linux/workqueue.h>
> @@ -114,6 +115,7 @@ struct imx_rproc {
>  	u32				entry;		/* cpu start address */
>  	u32				core_index;
>  	struct dev_pm_domain_list	*pd_list;
> +	struct sys_off_data		data;
>  };
>  
>  static const struct imx_rproc_att imx_rproc_att_imx93[] = {
> @@ -1050,6 +1052,22 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv)
>  	return 0;
>  }
>  
> +static int imx_rproc_sys_off_handler(struct sys_off_data *data)
> +{
> +	struct rproc *rproc = data->cb_data;
> +	int ret;
> +
> +	imx_rproc_free_mbox(rproc);
> +
> +	ret = imx_rproc_xtr_mbox_init(rproc, false);

This is new functionality and has nothing to do with the rest of this set.
Please introduce patches 5/6 and 6/6 as part of a new patchset.

Thanks,
Mathieu

> +	if (ret) {
> +		dev_err(&rproc->dev, "Failed to request non-blocking mbox\n");
> +		return NOTIFY_BAD;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
>  static int imx_rproc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -1104,6 +1122,24 @@ static int imx_rproc_probe(struct platform_device *pdev)
>  	if (rproc->state != RPROC_DETACHED)
>  		rproc->auto_boot = of_property_read_bool(np, "fsl,auto-boot");
>  
> +	if (of_device_is_compatible(dev->of_node, "fsl,imx7ulp-cm4")) {
> +		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_POWER_OFF_PREPARE,
> +						    SYS_OFF_PRIO_DEFAULT,
> +						    imx_rproc_sys_off_handler, rproc);
> +		if (ret) {
> +			dev_err(dev, "register power off handler failure\n");
> +			goto err_put_clk;
> +		}
> +
> +		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_RESTART_PREPARE,
> +						    SYS_OFF_PRIO_DEFAULT,
> +						    imx_rproc_sys_off_handler, rproc);
> +		if (ret) {
> +			dev_err(dev, "register restart handler failure\n");
> +			goto err_put_clk;
> +		}
> +	}
> +
>  	ret = rproc_add(rproc);
>  	if (ret) {
>  		dev_err(dev, "rproc_add failed\n");
> 
> -- 
> 2.37.1
> 

Re: [PATCH 1/6] remoteproc: imx_rproc: correct ddr alias for i.MX8M

[Mathieu Poirier]

On Tue, Jul 16, 2024 at 06:49:39PM +0300, Iuliana Prodan wrote:
> Hi Mathieu,
> 
> On 7/16/2024 6:16 PM, Mathieu Poirier wrote:
> > Good morning,
> > 
> > On Fri, Jul 12, 2024 at 04:34:54PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan<peng.fan@nxp.com>
> > > 
> > > The DDR Alias address should be 0x40000000 according to RM, so correct
> > > it.
> > > 
> > > Fixes: 4ab8f9607aad ("remoteproc: imx_rproc: support i.MX8MQ/M")
> > This commit was merged more than 3 years ago...  I don't see how such a blatant
> > mistake could have survived this long without causing problems or being noticed.
> 
> Most probably whoever used imx_rproc and ran into this issue checked NXP
> tree where this is fixed - see here https://github.com/nxp-imx/linux-imx/blob/lf-6.6.y/drivers/remoteproc/imx_rproc.c#L232
> > On top of things checkpatch gives me a warning.
> > 
> > > Reported-by: Terry Lv<terry.lv@nxp.com>
> > > Signed-off-by: Peng Fan<peng.fan@nxp.com>
> > > ---
> > >   drivers/remoteproc/imx_rproc.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> > > index 144c8e9a642e..3c8b64db8823 100644
> > > --- a/drivers/remoteproc/imx_rproc.c
> > > +++ b/drivers/remoteproc/imx_rproc.c
> > > @@ -210,7 +210,7 @@ static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
> > >   	/* QSPI Code - alias */
> > >   	{ 0x08000000, 0x08000000, 0x08000000, 0 },
> > >   	/* DDR (Code) - alias */
> > > -	{ 0x10000000, 0x80000000, 0x0FFE0000, 0 },
> > > +	{ 0x10000000, 0x40000000, 0x0FFE0000, 0 },
> > Without access to HW or the documentation there is no way for me to assess the
> > validity of this patch.  Marek, Iuliana and Daniel - please review and test this
> > patch.
> 
> HW documentation can be downloaded from
> https://www.nxp.com/webapp/Download?colCode=IMX8MDQLQRM (one needs to create
> an account)
> So,
> Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
>

The very quick reply is much appreciated. I will merge this at the beginning of
the 6.11 cycle.

> Thanks,
> Iulia
> 
> > Thanks,
> > Mathieu
> > 
> > >   	/* TCML */
> > >   	{ 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN  | ATT_IOMEM},
> > >   	/* TCMU */
> > > 
> > > -- 
> > > 2.37.1
> > > 

RE: [PATCH 1/6] remoteproc: imx_rproc: correct ddr alias for i.MX8M

[Peng Fan]

> Subject: Re: [PATCH 1/6] remoteproc: imx_rproc: correct ddr alias for
> i.MX8M
> 
> Good morning,
> 
> On Fri, Jul 12, 2024 at 04:34:54PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The DDR Alias address should be 0x40000000 according to RM, so
> correct
> > it.
> >
> > Fixes: 4ab8f9607aad ("remoteproc: imx_rproc: support i.MX8MQ/M")
> 
> This commit was merged more than 3 years ago...  I don't see how such
> a blatant mistake could have survived this long without causing
> problems or being noticed.

I got this reported quite sometime ago inside NXP. For community
users, not sure people encounter issue or not.

I am trying to upstream downstream patches, so post this out.

> On top of things checkpatch gives me a warning.

The warning could ignored.
There is no public url for reporting this issue.

> 
> > Reported-by: Terry Lv <terry.lv@nxp.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/remoteproc/imx_rproc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c
> > b/drivers/remoteproc/imx_rproc.c index
> 144c8e9a642e..3c8b64db8823
> > 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -210,7 +210,7 @@ static const struct imx_rproc_att
> imx_rproc_att_imx8mq[] = {
> >  	/* QSPI Code - alias */
> >  	{ 0x08000000, 0x08000000, 0x08000000, 0 },
> >  	/* DDR (Code) - alias */
> > -	{ 0x10000000, 0x80000000, 0x0FFE0000, 0 },
> > +	{ 0x10000000, 0x40000000, 0x0FFE0000, 0 },
> 
> Without access to HW or the documentation there is no way for me to
> assess the validity of this patch.  Marek, Iuliana and Daniel - please
> review and test this patch.

I see Iuliana gaves R-b and doc link.

Thanks,
Peng.

> 
> Thanks,
> Mathieu
> 
> >  	/* TCML */
> >  	{ 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN  |
> ATT_IOMEM},
> >  	/* TCMU */
> >
> > --
> > 2.37.1
> >

RE: [PATCH 3/6] remoteproc: imx_rproc: initialize workqueue earlier

[Peng Fan]

> Subject: Re: [PATCH 3/6] remoteproc: imx_rproc: initialize workqueue
> earlier
> 
> On Fri, Jul 12, 2024 at 04:34:56PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Initialize workqueue before requesting mailbox channel, otherwise if
> > mailbox interrupt comes before workqueue ready, the
> > imx_rproc_rx_callback will trigger issue.
> >
> > Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
> 
> All reviews should be done publicly - please remove here and for all the
> other patches.

Sure.

> 
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/remoteproc/imx_rproc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c
> > b/drivers/remoteproc/imx_rproc.c index
> 48c48b53a3aa..9e99bb27c033
> > 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -1087,6 +1087,8 @@ static int imx_rproc_probe(struct
> platform_device *pdev)
> >  		return -ENOMEM;
> >  	}
> >
> > +	INIT_WORK(&priv->rproc_work, imx_rproc_vq_work);
> > +
> 
> There should be a "Fixes:" tag on this patch.

Add in v2.

Thanks,
Peng.

RE: [PATCH 4/6] remoteproc: imx_rproc: merge TCML/U

[Peng Fan]

> Subject: Re: [PATCH 4/6] remoteproc: imx_rproc: merge TCML/U
> 
> On Fri, Jul 12, 2024 at 04:34:57PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Merge contiguous TCML/U regions into one to avoid load elf files
> which
> > has large sections failure.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/remoteproc/imx_rproc.c | 18 ++++++------------
> >  1 file changed, 6 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c
> > b/drivers/remoteproc/imx_rproc.c index
> 9e99bb27c033..552fccebf7e2
> > 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -119,20 +119,16 @@ struct imx_rproc {  static const struct
> > imx_rproc_att imx_rproc_att_imx93[] = {
> >  	/* dev addr , sys addr  , size	    , flags */
> >  	/* TCM CODE NON-SECURE */
> > -	{ 0x0FFC0000, 0x201C0000, 0x00020000, ATT_OWN |
> ATT_IOMEM },
> > -	{ 0x0FFE0000, 0x201E0000, 0x00020000, ATT_OWN |
> ATT_IOMEM },
> > +	{ 0x0FFC0000, 0x201C0000, 0x00040000, ATT_OWN |
> ATT_IOMEM },
> >
> >  	/* TCM CODE SECURE */
> > -	{ 0x1FFC0000, 0x201C0000, 0x00020000, ATT_OWN |
> ATT_IOMEM },
> > -	{ 0x1FFE0000, 0x201E0000, 0x00020000, ATT_OWN |
> ATT_IOMEM },
> > +	{ 0x1FFC0000, 0x201C0000, 0x00040000, ATT_OWN |
> ATT_IOMEM },
> >
> >  	/* TCM SYS NON-SECURE*/
> > -	{ 0x20000000, 0x20200000, 0x00020000, ATT_OWN |
> ATT_IOMEM },
> > -	{ 0x20020000, 0x20220000, 0x00020000, ATT_OWN |
> ATT_IOMEM },
> > +	{ 0x20000000, 0x20200000, 0x00040000, ATT_OWN |
> ATT_IOMEM },
> >
> >  	/* TCM SYS SECURE*/
> > -	{ 0x30000000, 0x20200000, 0x00020000, ATT_OWN |
> ATT_IOMEM },
> > -	{ 0x30020000, 0x20220000, 0x00020000, ATT_OWN |
> ATT_IOMEM },
> > +	{ 0x30000000, 0x20200000, 0x00040000, ATT_OWN |
> ATT_IOMEM },
> >
> >  	/* DDR */
> >  	{ 0x80000000, 0x80000000, 0x10000000, 0 }, @@ -211,10
> +207,8 @@
> > static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
> >  	{ 0x08000000, 0x08000000, 0x08000000, 0 },
> >  	/* DDR (Code) - alias */
> >  	{ 0x10000000, 0x40000000, 0x0FFE0000, 0 },
> > -	/* TCML */
> > -	{ 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN  |
> ATT_IOMEM},
> > -	/* TCMU */
> > -	{ 0x20000000, 0x00800000, 0x00020000, ATT_OWN  |
> ATT_IOMEM},
> > +	/* TCML/U */
> > +	{ 0x1FFE0000, 0x007E0000, 0x00040000, ATT_OWN  |
> ATT_IOMEM},
> 
> Here too I will need an RB tag by Marek, Iuliana or Daniel.

BTW, downstream release code:
https://github.com/nxp-imx/linux-imx/blob/lf-6.6.y/drivers/remoteproc/imx_rproc.c#L142

Thanks,
Peng.

> 
> >  	/* OCRAM_S */
> >  	{ 0x20180000, 0x00180000, 0x00008000, ATT_OWN },
> >  	/* OCRAM */
> >
> > --
> > 2.37.1
> >

RE: [PATCH 5/6] remoteproc: imx_rproc: allow tx_block to be set

[Peng Fan]

> Subject: Re: [PATCH 5/6] remoteproc: imx_rproc: allow tx_block to be
> set
> 
> On Fri, Jul 12, 2024 at 04:34:58PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Current tx_block is set to true, but there is case that no need to
> > wait response. Linux just needs to send data to remote processor, so
> > let's allow tx_block could be set to false.
> 
> Ok, maybe - but this patch doesn't use the new functionality, i.e
> nothing changes.
> 
Will update commit log to add:
"No functional changes"

Thanks,
Peng.

Re: [PATCH 2/6] remoteproc: imx_rproc: use imx specific hook for find_loaded_rsc_table

[Iuliana Prodan]

On 7/12/2024 11:34 AM, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> If there is a resource table device tree node, use the address as
> the resource table address, otherwise use the address(where
> .resource_table section loaded) inside the Cortex-M elf file.
>
> And there is an update in NXP SDK that Resource Domain Control(RDC)
> enabled to protect TCM, linux not able to write the TCM space when
> updating resource table status and cause kernel dump. So use the address
> from device tree could avoid kernel dump.
>
> Note: NXP M4 SDK not check resource table update, so it does not matter
> use whether resource table address specified in elf file or in device
> tree. But to reflect the fact that if people specific resource table
> address in device tree, it means people are aware and going to use it,
> not the address specified in elf file.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>

Thanks,
Iulia


> ---
>   drivers/remoteproc/imx_rproc.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 3c8b64db8823..48c48b53a3aa 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -666,6 +666,17 @@ static struct resource_table *imx_rproc_get_loaded_rsc_table(struct rproc *rproc
>   	return (struct resource_table *)priv->rsc_table;
>   }
>   
> +static struct resource_table *
> +imx_rproc_elf_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
> +{
> +	struct imx_rproc *priv = rproc->priv;
> +
> +	if (priv->rsc_table)
> +		return (struct resource_table *)priv->rsc_table;
> +
> +	return rproc_elf_find_loaded_rsc_table(rproc, fw);
> +}
> +
>   static const struct rproc_ops imx_rproc_ops = {
>   	.prepare	= imx_rproc_prepare,
>   	.attach		= imx_rproc_attach,
> @@ -676,7 +687,7 @@ static const struct rproc_ops imx_rproc_ops = {
>   	.da_to_va       = imx_rproc_da_to_va,
>   	.load		= rproc_elf_load_segments,
>   	.parse_fw	= imx_rproc_parse_fw,
> -	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> +	.find_loaded_rsc_table = imx_rproc_elf_find_loaded_rsc_table,
>   	.get_loaded_rsc_table = imx_rproc_get_loaded_rsc_table,
>   	.sanity_check	= rproc_elf_sanity_check,
>   	.get_boot_addr	= rproc_elf_get_boot_addr,
>

Re: [PATCH 4/6] remoteproc: imx_rproc: merge TCML/U

[Iuliana Prodan]

On 7/12/2024 11:34 AM, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Merge contiguous TCML/U regions into one to avoid load elf files which
> has large sections failure.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>

Thanks,
Iulia

>   drivers/remoteproc/imx_rproc.c | 18 ++++++------------
>   1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 9e99bb27c033..552fccebf7e2 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -119,20 +119,16 @@ struct imx_rproc {
>   static const struct imx_rproc_att imx_rproc_att_imx93[] = {
>   	/* dev addr , sys addr  , size	    , flags */
>   	/* TCM CODE NON-SECURE */
> -	{ 0x0FFC0000, 0x201C0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> -	{ 0x0FFE0000, 0x201E0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> +	{ 0x0FFC0000, 0x201C0000, 0x00040000, ATT_OWN | ATT_IOMEM },
>   
>   	/* TCM CODE SECURE */
> -	{ 0x1FFC0000, 0x201C0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> -	{ 0x1FFE0000, 0x201E0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> +	{ 0x1FFC0000, 0x201C0000, 0x00040000, ATT_OWN | ATT_IOMEM },
>   
>   	/* TCM SYS NON-SECURE*/
> -	{ 0x20000000, 0x20200000, 0x00020000, ATT_OWN | ATT_IOMEM },
> -	{ 0x20020000, 0x20220000, 0x00020000, ATT_OWN | ATT_IOMEM },
> +	{ 0x20000000, 0x20200000, 0x00040000, ATT_OWN | ATT_IOMEM },
>   
>   	/* TCM SYS SECURE*/
> -	{ 0x30000000, 0x20200000, 0x00020000, ATT_OWN | ATT_IOMEM },
> -	{ 0x30020000, 0x20220000, 0x00020000, ATT_OWN | ATT_IOMEM },
> +	{ 0x30000000, 0x20200000, 0x00040000, ATT_OWN | ATT_IOMEM },
>   
>   	/* DDR */
>   	{ 0x80000000, 0x80000000, 0x10000000, 0 },
> @@ -211,10 +207,8 @@ static const struct imx_rproc_att imx_rproc_att_imx8mq[] = {
>   	{ 0x08000000, 0x08000000, 0x08000000, 0 },
>   	/* DDR (Code) - alias */
>   	{ 0x10000000, 0x40000000, 0x0FFE0000, 0 },
> -	/* TCML */
> -	{ 0x1FFE0000, 0x007E0000, 0x00020000, ATT_OWN  | ATT_IOMEM},
> -	/* TCMU */
> -	{ 0x20000000, 0x00800000, 0x00020000, ATT_OWN  | ATT_IOMEM},
> +	/* TCML/U */
> +	{ 0x1FFE0000, 0x007E0000, 0x00040000, ATT_OWN  | ATT_IOMEM},
>   	/* OCRAM_S */
>   	{ 0x20180000, 0x00180000, 0x00008000, ATT_OWN },
>   	/* OCRAM */
>

Re: [PATCH 6/6] remoteproc: imx_rproc: handle system off for i.MX7ULP

[Frank Li]

On Fri, Jul 12, 2024 at 04:34:59PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> The i.MX7ULP Cortex-A7 is under control of Cortex-M4. The i.MX7ULP Linux
> poweroff and restart rely on rpmsg driver to send a message to Cortex-M4
> firmware. Then Cortex-A7 could poweroff or restart by Cortex-M4 to
> configure the i.MX7ULP power controller properly.
>
> However the reboot and restart kernel common code use atomic notifier,
> so with blocking tx mailbox will trigger kernel dump, because of
> blocking mailbox will use wait_for_completion_timeout. In such case,
> linux no need to wait for completion.
>
> Current patch is to use non-blocking tx mailbox channel when system
> is going to poweroff or restart.
>
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 01cf1dfb2e87..e1abf110abc9 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -18,6 +18,7 @@
>  #include <linux/of_reserved_mem.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
> +#include <linux/reboot.h>
>  #include <linux/regmap.h>
>  #include <linux/remoteproc.h>
>  #include <linux/workqueue.h>
> @@ -114,6 +115,7 @@ struct imx_rproc {
>  	u32				entry;		/* cpu start address */
>  	u32				core_index;
>  	struct dev_pm_domain_list	*pd_list;
> +	struct sys_off_data		data;
>  };
>
>  static const struct imx_rproc_att imx_rproc_att_imx93[] = {
> @@ -1050,6 +1052,22 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv)
>  	return 0;
>  }
>
> +static int imx_rproc_sys_off_handler(struct sys_off_data *data)
> +{
> +	struct rproc *rproc = data->cb_data;
> +	int ret;
> +
> +	imx_rproc_free_mbox(rproc);
> +
> +	ret = imx_rproc_xtr_mbox_init(rproc, false);
> +	if (ret) {
> +		dev_err(&rproc->dev, "Failed to request non-blocking mbox\n");
> +		return NOTIFY_BAD;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
>  static int imx_rproc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -1104,6 +1122,24 @@ static int imx_rproc_probe(struct platform_device *pdev)
>  	if (rproc->state != RPROC_DETACHED)
>  		rproc->auto_boot = of_property_read_bool(np, "fsl,auto-boot");
>
> +	if (of_device_is_compatible(dev->of_node, "fsl,imx7ulp-cm4")) {

I don't suggest check compatible string. It'd better add a field  in
imx_rproc_dcfg, such as need_sys_off

	if (dcfg->need_sys_off) {
		...
	}

If there are new compatible string added, just need set need_sys_off to
true in driver data.

Frank

> +		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_POWER_OFF_PREPARE,
> +						    SYS_OFF_PRIO_DEFAULT,
> +						    imx_rproc_sys_off_handler, rproc);
> +		if (ret) {
> +			dev_err(dev, "register power off handler failure\n");
> +			goto err_put_clk;
> +		}
> +
> +		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_RESTART_PREPARE,
> +						    SYS_OFF_PRIO_DEFAULT,
> +						    imx_rproc_sys_off_handler, rproc);
> +		if (ret) {
> +			dev_err(dev, "register restart handler failure\n");
> +			goto err_put_clk;
> +		}
> +	}
> +
>  	ret = rproc_add(rproc);
>  	if (ret) {
>  		dev_err(dev, "rproc_add failed\n");
>
> --
> 2.37.1
>

RE: [PATCH 6/6] remoteproc: imx_rproc: handle system off for i.MX7ULP

[Peng Fan]

> Subject: Re: [PATCH 6/6] remoteproc: imx_rproc: handle system off for
> i.MX7ULP
> 
> On Fri, Jul 12, 2024 at 04:34:59PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The i.MX7ULP Cortex-A7 is under control of Cortex-M4. The
> i.MX7ULP
> > Linux poweroff and restart rely on rpmsg driver to send a message to
> > Cortex-M4 firmware. Then Cortex-A7 could poweroff or restart by
> > Cortex-M4 to configure the i.MX7ULP power controller properly.
> >
> > However the reboot and restart kernel common code use atomic
> notifier,
> > so with blocking tx mailbox will trigger kernel dump, because of
> > blocking mailbox will use wait_for_completion_timeout. In such case,
> > linux no need to wait for completion.
> >
> > Current patch is to use non-blocking tx mailbox channel when system
> is
> > going to poweroff or restart.
> >
> > Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/remoteproc/imx_rproc.c | 36
> > ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c
> > b/drivers/remoteproc/imx_rproc.c index
> 01cf1dfb2e87..e1abf110abc9
> > 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/of_reserved_mem.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_domain.h>
> > +#include <linux/reboot.h>
> >  #include <linux/regmap.h>
> >  #include <linux/remoteproc.h>
> >  #include <linux/workqueue.h>
> > @@ -114,6 +115,7 @@ struct imx_rproc {
> >  	u32				entry;		/* cpu start
> address */
> >  	u32				core_index;
> >  	struct dev_pm_domain_list	*pd_list;
> > +	struct sys_off_data		data;
> >  };
> >
> >  static const struct imx_rproc_att imx_rproc_att_imx93[] = { @@
> > -1050,6 +1052,22 @@ static int imx_rproc_clk_enable(struct
> imx_rproc *priv)
> >  	return 0;
> >  }
> >
> > +static int imx_rproc_sys_off_handler(struct sys_off_data *data) {
> > +	struct rproc *rproc = data->cb_data;
> > +	int ret;
> > +
> > +	imx_rproc_free_mbox(rproc);
> > +
> > +	ret = imx_rproc_xtr_mbox_init(rproc, false);
> > +	if (ret) {
> > +		dev_err(&rproc->dev, "Failed to request non-blocking
> mbox\n");
> > +		return NOTIFY_BAD;
> > +	}
> > +
> > +	return NOTIFY_DONE;
> > +}
> > +
> >  static int imx_rproc_probe(struct platform_device *pdev)  {
> >  	struct device *dev = &pdev->dev;
> > @@ -1104,6 +1122,24 @@ static int imx_rproc_probe(struct
> platform_device *pdev)
> >  	if (rproc->state != RPROC_DETACHED)
> >  		rproc->auto_boot = of_property_read_bool(np,
> "fsl,auto-boot");
> >
> > +	if (of_device_is_compatible(dev->of_node, "fsl,imx7ulp-cm4"))
> {
> 
> I don't suggest check compatible string. It'd better add a field  in
> imx_rproc_dcfg, such as need_sys_off
> 
> 	if (dcfg->need_sys_off) {
> 		...
> 	}
> 
> If there are new compatible string added, just need set need_sys_off to
> true in driver data.

Could we delay the change when there is really new chips need this?
The downstream commit time is " Date:   Tue Dec 6 17:10:14 2022",
In the past days, I not see other platforms require this.

Mathieu, which do you prefer? add need_sys_off or keep current
approach?

Thanks,
Peng.

> 
> Frank
> 
> > +		ret = devm_register_sys_off_handler(dev,
> SYS_OFF_MODE_POWER_OFF_PREPARE,
> > +
> SYS_OFF_PRIO_DEFAULT,
> > +
> imx_rproc_sys_off_handler, rproc);
> > +		if (ret) {
> > +			dev_err(dev, "register power off handler
> failure\n");
> > +			goto err_put_clk;
> > +		}
> > +
> > +		ret = devm_register_sys_off_handler(dev,
> SYS_OFF_MODE_RESTART_PREPARE,
> > +
> SYS_OFF_PRIO_DEFAULT,
> > +
> imx_rproc_sys_off_handler, rproc);
> > +		if (ret) {
> > +			dev_err(dev, "register restart handler
> failure\n");
> > +			goto err_put_clk;
> > +		}
> > +	}
> > +
> >  	ret = rproc_add(rproc);
> >  	if (ret) {
> >  		dev_err(dev, "rproc_add failed\n");
> >
> > --
> > 2.37.1
> >

Re: [PATCH 6/6] remoteproc: imx_rproc: handle system off for i.MX7ULP

[Mathieu Poirier]

On Thu, Aug 08, 2024 at 02:56:20AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH 6/6] remoteproc: imx_rproc: handle system off for
> > i.MX7ULP
> > 
> > On Fri, Jul 12, 2024 at 04:34:59PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > The i.MX7ULP Cortex-A7 is under control of Cortex-M4. The
> > i.MX7ULP
> > > Linux poweroff and restart rely on rpmsg driver to send a message to
> > > Cortex-M4 firmware. Then Cortex-A7 could poweroff or restart by
> > > Cortex-M4 to configure the i.MX7ULP power controller properly.
> > >
> > > However the reboot and restart kernel common code use atomic
> > notifier,
> > > so with blocking tx mailbox will trigger kernel dump, because of
> > > blocking mailbox will use wait_for_completion_timeout. In such case,
> > > linux no need to wait for completion.
> > >
> > > Current patch is to use non-blocking tx mailbox channel when system
> > is
> > > going to poweroff or restart.
> > >
> > > Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  drivers/remoteproc/imx_rproc.c | 36
> > > ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 36 insertions(+)
> > >
> > > diff --git a/drivers/remoteproc/imx_rproc.c
> > > b/drivers/remoteproc/imx_rproc.c index
> > 01cf1dfb2e87..e1abf110abc9
> > > 100644
> > > --- a/drivers/remoteproc/imx_rproc.c
> > > +++ b/drivers/remoteproc/imx_rproc.c
> > > @@ -18,6 +18,7 @@
> > >  #include <linux/of_reserved_mem.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/pm_domain.h>
> > > +#include <linux/reboot.h>
> > >  #include <linux/regmap.h>
> > >  #include <linux/remoteproc.h>
> > >  #include <linux/workqueue.h>
> > > @@ -114,6 +115,7 @@ struct imx_rproc {
> > >  	u32				entry;		/* cpu start
> > address */
> > >  	u32				core_index;
> > >  	struct dev_pm_domain_list	*pd_list;
> > > +	struct sys_off_data		data;
> > >  };
> > >
> > >  static const struct imx_rproc_att imx_rproc_att_imx93[] = { @@
> > > -1050,6 +1052,22 @@ static int imx_rproc_clk_enable(struct
> > imx_rproc *priv)
> > >  	return 0;
> > >  }
> > >
> > > +static int imx_rproc_sys_off_handler(struct sys_off_data *data) {
> > > +	struct rproc *rproc = data->cb_data;
> > > +	int ret;
> > > +
> > > +	imx_rproc_free_mbox(rproc);
> > > +
> > > +	ret = imx_rproc_xtr_mbox_init(rproc, false);
> > > +	if (ret) {
> > > +		dev_err(&rproc->dev, "Failed to request non-blocking
> > mbox\n");
> > > +		return NOTIFY_BAD;
> > > +	}
> > > +
> > > +	return NOTIFY_DONE;
> > > +}
> > > +
> > >  static int imx_rproc_probe(struct platform_device *pdev)  {
> > >  	struct device *dev = &pdev->dev;
> > > @@ -1104,6 +1122,24 @@ static int imx_rproc_probe(struct
> > platform_device *pdev)
> > >  	if (rproc->state != RPROC_DETACHED)
> > >  		rproc->auto_boot = of_property_read_bool(np,
> > "fsl,auto-boot");
> > >
> > > +	if (of_device_is_compatible(dev->of_node, "fsl,imx7ulp-cm4"))
> > {
> > 
> > I don't suggest check compatible string. It'd better add a field  in
> > imx_rproc_dcfg, such as need_sys_off
> > 
> > 	if (dcfg->need_sys_off) {
> > 		...
> > 	}
> > 
> > If there are new compatible string added, just need set need_sys_off to
> > true in driver data.
> 
> Could we delay the change when there is really new chips need this?
> The downstream commit time is " Date:   Tue Dec 6 17:10:14 2022",
> In the past days, I not see other platforms require this.
> 
> Mathieu, which do you prefer? add need_sys_off or keep current
> approach?
>

This driver is already making extensive use of device data and as such suggest
to continue with that method.

> Thanks,
> Peng.
> 
> > 
> > Frank
> > 
> > > +		ret = devm_register_sys_off_handler(dev,
> > SYS_OFF_MODE_POWER_OFF_PREPARE,
> > > +
> > SYS_OFF_PRIO_DEFAULT,
> > > +
> > imx_rproc_sys_off_handler, rproc);
> > > +		if (ret) {
> > > +			dev_err(dev, "register power off handler
> > failure\n");
> > > +			goto err_put_clk;
> > > +		}
> > > +
> > > +		ret = devm_register_sys_off_handler(dev,
> > SYS_OFF_MODE_RESTART_PREPARE,
> > > +
> > SYS_OFF_PRIO_DEFAULT,
> > > +
> > imx_rproc_sys_off_handler, rproc);
> > > +		if (ret) {
> > > +			dev_err(dev, "register restart handler
> > failure\n");
> > > +			goto err_put_clk;
> > > +		}
> > > +	}
> > > +
> > >  	ret = rproc_add(rproc);
> > >  	if (ret) {
> > >  		dev_err(dev, "rproc_add failed\n");
> > >
> > > --
> > > 2.37.1
> > >