[PATCH v3 0/2] remoteproc: imx_rproc: support non-blocking tx for i.MX7ULP

[PATCH v3 0/2] remoteproc: imx_rproc: support non-blocking tx for i.MX7ULP

[Peng Fan (OSS)]

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,
see kernel/reboot.c: do_kernel_power_off, do_kernel_restart.
Linux 'poweroff' cmd will trigger kernel 'do_kernel_power_off'
Linux 'reboot' cmd will trigger kernel 'do_kernel_restart'.

Power off and restart are totally different operations and are not
complementary. So need to make sure the mailbox be changed to non-blocking
mode in SYS_OFF_MODE_POWER_OFF_PREPARE and SYS_OFF_MODE_RESTART_PREPARE
stage. Otherwise with blocking tx mailbox, there will be kernel dump,
because of blocking mailbox will use wait_for_completion_timeout. In the
poweroff or restart case, there is no need to wait, because after M4 got
the message, M4 will put A7 to reboot or poweroff.

patch 1 is to support non-blocking tx mailbox channel
patch 2 is to switch to non-blocking tx for system poweroff or restart.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v3:
- patch 2: Introduce an flags entry for dcfg and set IMX_RPROC_NEED_SYSTEM_OFF
  for i.MX7ULP, drop used "struct sys_off_data data" and add comment(per Frank).
  Drop R-b of patch 2 because of this change.
- Patch 1: commit log rewritten and R-b dropped.
  (Thanks to Daniel).
- Update patch 2 commit log and cover-letter to clarify reboot and
  poweroff are different operations and not complementary 
  (Thanks to Daniel).
- Link to v2: https://lore.kernel.org/r/20240719-imx_rproc-v2-0-cd8549aa3f1f@nxp.com

Changes in v2:
- Separated patch 5,6 from v1
- Update commit patch 1 with adding "No functional changes"
- Link to v1: https://lore.kernel.org/r/20240712-imx_rproc-v1-0-7bcf6732d328@nxp.com

---
Peng Fan (2):
      remoteproc: imx_rproc: Allow setting of the mailbox transmit mode
      remoteproc: imx_rproc: Add support for poweroff and reboot

 drivers/remoteproc/imx_rproc.c | 54 +++++++++++++++++++++++++++++++++++++-----
 drivers/remoteproc/imx_rproc.h |  4 ++++
 2 files changed, 52 insertions(+), 6 deletions(-)
---
base-commit: 33a32de2d79c051f27ed57c4ac90cdb406f91928
change-id: 20240712-imx_rproc-25f3ab753c58

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

[PATCH v3 1/2] remoteproc: imx_rproc: Allow setting of the mailbox transmit mode

[Peng Fan (OSS)]

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

Current mailbox is blocking by default, but there are cases where we don't
need to wait for a response.

Linux just needs to send data to the remote processor, so let's
allow tx_block mode to be set (true/false) depending on usecase.

No functional changes.

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 9744b004bd77..22677f581067 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 v3 1/2] remoteproc: imx_rproc: Allow setting of the mailbox transmit mode

[Daniel Baluta]

On Thu, Aug 22, 2024 at 4:40 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> Current mailbox is blocking by default, but there are cases where we don't
> need to wait for a response.
>
> Linux just needs to send data to the remote processor, so let's
> allow tx_block mode to be set (true/false) depending on usecase.
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>

[PATCH v3 2/2] remoteproc: imx_rproc: Add support for poweroff and reboot

[Peng Fan (OSS)]

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

On some NXP platforms (e.g i.MX7ULP) the poweroff and reboot operations
are done via a separate remote core.

Typically Linux needs to send a message to the remote core and requests
for poweroff or reboot.

By default the communication between Linux core and the remote core is
is done via a blocking mailbox mechanism but Linux doesn't allow blocking
operations in the system off (reboot, power off) handlers.

So, we need to make sure the mailbox message send operations do not block
for this specific operations. Fortunately, Linux allows us to register
handlers that are called in preparation of the system off operations.

Thus, before carrying the power off or reboot preparations, just destroy
the existing mailboxes and create them as non-blocking.

Note that power off and restart are totally different operations and are
not complementary.

We introduce a new flag in the imx remoteproc per device data which tells
us when a device needs this special setup. For now, only imx7ulp needs it.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 drivers/remoteproc/imx_rproc.h |  4 ++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 22677f581067..800015ff7ff9 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>
@@ -333,6 +334,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx7ulp = {
 	.att		= imx_rproc_att_imx7ulp,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7ulp),
 	.method		= IMX_RPROC_NONE,
+	.flags		= IMX_RPROC_NEED_SYSTEM_OFF,
 };
 
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
@@ -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,30 @@ 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 (dcfg->flags & IMX_RPROC_NEED_SYSTEM_OFF) {
+		/*
+		 * setup mailbox to non-blocking mode in
+		 * [SYS_OFF_MODE_POWER_OFF_PREPARE, SYS_OFF_MODE_RESTART_PREPARE]
+		 * phase before invoking [SYS_OFF_MODE_POWER_OFF, SYS_OFF_MODE_RESTART]
+		 * atomic chain, see kernel/reboot.c.
+		 */
+		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");
diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
index 79a1b8956d14..17a7d051c531 100644
--- a/drivers/remoteproc/imx_rproc.h
+++ b/drivers/remoteproc/imx_rproc.h
@@ -26,6 +26,9 @@ enum imx_rproc_method {
 	IMX_RPROC_SCU_API,
 };
 
+/* dcfg flags */
+#define IMX_RPROC_NEED_SYSTEM_OFF	BIT(0)
+
 struct imx_rproc_dcfg {
 	u32				src_reg;
 	u32				src_mask;
@@ -36,6 +39,7 @@ struct imx_rproc_dcfg {
 	const struct imx_rproc_att	*att;
 	size_t				att_size;
 	enum imx_rproc_method		method;
+	u32				flags;
 };
 
 #endif /* _IMX_RPROC_H */

-- 
2.37.1

Re: [PATCH v3 2/2] remoteproc: imx_rproc: Add support for poweroff and reboot

[Daniel Baluta]

On Thu, Aug 22, 2024 at 4:40 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> On some NXP platforms (e.g i.MX7ULP) the poweroff and reboot operations
> are done via a separate remote core.
>
> Typically Linux needs to send a message to the remote core and requests
> for poweroff or reboot.
>
> By default the communication between Linux core and the remote core is
> is done via a blocking mailbox mechanism but Linux doesn't allow blocking
> operations in the system off (reboot, power off) handlers.
>
> So, we need to make sure the mailbox message send operations do not block
> for this specific operations. Fortunately, Linux allows us to register
> handlers that are called in preparation of the system off operations.
>
> Thus, before carrying the power off or reboot preparations, just destroy
> the existing mailboxes and create them as non-blocking.
>
> Note that power off and restart are totally different operations and are
> not complementary.
>
> We introduce a new flag in the imx remoteproc per device data which tells
> us when a device needs this special setup. For now, only imx7ulp needs it.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>

Re: [PATCH v3 0/2] remoteproc: imx_rproc: support non-blocking tx for i.MX7ULP

[Mathieu Poirier]

On Thu, Aug 22, 2024 at 09:48:48PM +0800, Peng Fan (OSS) wrote:
> 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,
> see kernel/reboot.c: do_kernel_power_off, do_kernel_restart.
> Linux 'poweroff' cmd will trigger kernel 'do_kernel_power_off'
> Linux 'reboot' cmd will trigger kernel 'do_kernel_restart'.
> 
> Power off and restart are totally different operations and are not
> complementary. So need to make sure the mailbox be changed to non-blocking
> mode in SYS_OFF_MODE_POWER_OFF_PREPARE and SYS_OFF_MODE_RESTART_PREPARE
> stage. Otherwise with blocking tx mailbox, there will be kernel dump,
> because of blocking mailbox will use wait_for_completion_timeout. In the
> poweroff or restart case, there is no need to wait, because after M4 got
> the message, M4 will put A7 to reboot or poweroff.
> 
> patch 1 is to support non-blocking tx mailbox channel
> patch 2 is to switch to non-blocking tx for system poweroff or restart.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> Changes in v3:
> - patch 2: Introduce an flags entry for dcfg and set IMX_RPROC_NEED_SYSTEM_OFF
>   for i.MX7ULP, drop used "struct sys_off_data data" and add comment(per Frank).
>   Drop R-b of patch 2 because of this change.
> - Patch 1: commit log rewritten and R-b dropped.
>   (Thanks to Daniel).
> - Update patch 2 commit log and cover-letter to clarify reboot and
>   poweroff are different operations and not complementary 
>   (Thanks to Daniel).
> - Link to v2: https://lore.kernel.org/r/20240719-imx_rproc-v2-0-cd8549aa3f1f@nxp.com
> 
> Changes in v2:
> - Separated patch 5,6 from v1
> - Update commit patch 1 with adding "No functional changes"
> - Link to v1: https://lore.kernel.org/r/20240712-imx_rproc-v1-0-7bcf6732d328@nxp.com
> 
> ---
> Peng Fan (2):
>       remoteproc: imx_rproc: Allow setting of the mailbox transmit mode
>       remoteproc: imx_rproc: Add support for poweroff and reboot
> 
>  drivers/remoteproc/imx_rproc.c | 54 +++++++++++++++++++++++++++++++++++++-----
>  drivers/remoteproc/imx_rproc.h |  4 ++++
>  2 files changed, 52 insertions(+), 6 deletions(-)

I have applied this set.

Thanks,
Mathieu

> ---
> base-commit: 33a32de2d79c051f27ed57c4ac90cdb406f91928
> change-id: 20240712-imx_rproc-25f3ab753c58
> 
> Best regards,
> -- 
> Peng Fan <peng.fan@nxp.com>
>