[RFC 0/2] rpmsg: imx: add the initial imx rpmsg support

[RFC 0/2] rpmsg: imx: add the initial imx rpmsg support

[Richard Zhu]

 [RFC 1/2] dt-binding: add imx rpmsg compatibility documentation
 [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

Documentation/devicetree/bindings/soc/fsl/fsl,rpmsg.txt |  75 ++++++++++
drivers/rpmsg/Kconfig                                   |  24 ++++
drivers/rpmsg/Makefile                                  |   2 +
drivers/rpmsg/imx_rpmsg.c                               | 542 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/rpmsg/imx_rpmsg_pingpong.c                      | 100 +++++++++++++
include/linux/imx_rpmsg.h                               |  43 ++++++
6 files changed, 786 insertions(+)

[RFC 1/2] dt-binding: add imx rpmsg compatibility documentation

[Richard Zhu]

Add imx rpmsg compatibility documentation.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 .../devicetree/bindings/soc/fsl/fsl,rpmsg.txt      | 75 ++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/fsl/fsl,rpmsg.txt

diff --git a/Documentation/devicetree/bindings/soc/fsl/fsl,rpmsg.txt b/Documentation/devicetree/bindings/soc/fsl/fsl,rpmsg.txt
new file mode 100644
index 0000000..7267756
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/fsl/fsl,rpmsg.txt
@@ -0,0 +1,75 @@
+i.MX RPMSG platform implementations
+
+Distributed framework is used in IMX RPMSG implementation, refer to the
+following requirements:
+  - The CAN functions contained in M core and RTOS should be ready and
+    complete functional in 50ms after AMP system is turned on.
+  - Partition reset. System wouldn't be stalled by the exceptions (e.x
+    the reset triggered by the system hang) occurred at the other side.
+    And the RPMSG mechanism should be recovered automactilly after the
+    partition reset is completed.
+In this scenario, the M core and RTOS would be kicked off by bootloader
+firstly, then A core and Linux would be loaded later. Both M core/RTOS
+and A core/Linux are running independly.
+One physical memory region used to store the vring is mandatory required
+to pre-reserved and well-knowned by both A core and M core
+
+Required properties:
+- compatible: "fsl,imx8qxp-rpmsg", "fsl,imx8mq-rpmsg".
+- vdev-nums: The number of the remote virtual devices.
+- reg: The reserved phisical DDR memory used to store vring descriptors.
+
+Optional properties:
+- rpmsg_dma_reserved: The reserved per device dma pool, that used to
+  allocate the shared memory buffers from the per device.
+  And it is optional for some platforms, since the system dma pool
+  is used to allocate the shared memory buffers directly on them.
+- mub-partition: The partition ID of muB side, that's optional
+  and used on i.mx8qm/8qxp for partition reset. The default
+  value is 3 in driver without this property.
+
+=====================================================================
+Mailbox used by iMX RPMSG
+
+- mboxes: mailboxes used in the RPMSG transactions.
+- mbox-names: names of the mailboxes used in RPMSG.
+	- "tx":TX channel with 32bit transmit register and IRQ transmit
+	- "rx":RX channel with 32bit receive register and IRQ support
+	- "rxdb":RX doorbell channel.
+
+Example:
+Rpmsg node in board dts file.
+&rpmsg{
+	/*
+	 * 64K for one rpmsg instance:
+	 */
+	vdev-nums = <2>;
+	reg = <0x0 0x90000000 0x0 0x20000>;
+	status = "okay";
+};
+
+SOC level dts node definitions:
+rpmsg_reserved: rpmsg@0x90000000 {
+	no-map;
+	reg = <0 0x90000000 0 0x400000>;
+};
+rpmsg_dma_reserved:rpmsg_dma@0x90400000 {
+	compatible = "shared-dma-pool";
+	no-map;
+	reg = <0 0x90400000 0 0x1C00000>;
+};
+rpmsg: rpmsg{
+	compatible = "fsl,imx8qxp-rpmsg";
+	/* up to now, the following channels are used in imx rpmsg
+	 * - tx1/rx1: messages channel.
+	 * - general interrupt1: remote proc finish re-init rpmsg stack
+	 *   when A core is partition reset.
+	 */
+	mbox-names = "tx", "rx", "rxdb";
+	mboxes = <&lsio_mu5 0 1
+		  &lsio_mu5 1 1
+		  &lsio_mu5 3 1>;
+	mub-partition = <3>;
+	memory-region = <&rpmsg_dma_reserved>;
+	status = "disabled";
+};
-- 
2.7.4

[RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Richard Zhu]

Based on "virtio_rpmsg_bus" driver, This patch-set is used to set up
the communication mechanism between A core and M core on i.MX AMP SOCs.

Add the initial imx rpmsg support glue driver and one pingpong demo,
demonstrated the data transactions between A core and remote M core.
Distributed framework is used in IMX RPMSG implementation, refer to the
following requirements:
  - The CAN functions contained in M core and RTOS should be ready and
    complete functional in 50ms after AMP system is turned on.
  - Partition reset. System wouldn't be stalled by the exceptions (e.x
    the reset triggered by the system hang) occurred at the other side.
    And the RPMSG mechanism should be recovered automactilly after the
    partition reset is completed.
In this scenario, the M core and RTOS would be kicked off by bootloader
firstly, then A core and Linux would be loaded later. Both M core/RTOS
and A core/Linux are running independly.

One physical memory region used to store the vring is mandatory required
to pre-reserved and well-knowned by both A core and M core

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/rpmsg/Kconfig              |  24 ++
 drivers/rpmsg/Makefile             |   2 +
 drivers/rpmsg/imx_rpmsg.c          | 542 +++++++++++++++++++++++++++++++++++++
 drivers/rpmsg/imx_rpmsg_pingpong.c | 100 +++++++
 include/linux/imx_rpmsg.h          |  43 +++
 5 files changed, 711 insertions(+)
 create mode 100644 drivers/rpmsg/imx_rpmsg.c
 create mode 100644 drivers/rpmsg/imx_rpmsg_pingpong.c
 create mode 100644 include/linux/imx_rpmsg.h

diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
index d0322b4..636460e 100644
--- a/drivers/rpmsg/Kconfig
+++ b/drivers/rpmsg/Kconfig
@@ -55,4 +55,28 @@ config RPMSG_VIRTIO
 	select RPMSG
 	select VIRTIO
 
+config HAVE_IMX_RPMSG
+	bool "IMX RPMSG driver on the AMP SOCs"
+	default y
+	depends on IMX_MBOX
+	select RPMSG_VIRTIO
+	help
+	  Say y here to enable support for the iMX Rpmsg Driver	providing
+	  communication channels to remote processors in iMX asymmetric
+	  multiprocessing (AMP) platforms.
+
+	  Especially, it is mandatory required when the partition reset is
+	  required on some iMX AMP platforms.
+
+config IMX_RPMSG_PINGPONG
+	tristate "IMX RPMSG pingpong driver -- loadable modules only"
+	default m
+	depends on HAVE_IMX_RPMSG && m
+	help
+	  One 32bit unsigned int data transactions demoe between the A core
+	  and the remote M core on the iMX AMP platforms.
+
+	  Only the module mode is supported here, the demo would be kicked off
+	  immediately when this module is insmoded.
+
 endmenu
diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile
index 9aa8595..9c6fce5 100644
--- a/drivers/rpmsg/Makefile
+++ b/drivers/rpmsg/Makefile
@@ -6,3 +6,5 @@ obj-$(CONFIG_RPMSG_QCOM_GLINK_NATIVE) += qcom_glink_native.o
 obj-$(CONFIG_RPMSG_QCOM_GLINK_SMEM) += qcom_glink_smem.o
 obj-$(CONFIG_RPMSG_QCOM_SMD)	+= qcom_smd.o
 obj-$(CONFIG_RPMSG_VIRTIO)	+= virtio_rpmsg_bus.o
+obj-$(CONFIG_HAVE_IMX_RPMSG)	+= imx_rpmsg.o
+obj-$(CONFIG_IMX_RPMSG_PINGPONG)	+= imx_rpmsg_pingpong.o
diff --git a/drivers/rpmsg/imx_rpmsg.c b/drivers/rpmsg/imx_rpmsg.c
new file mode 100644
index 0000000..58888d1
--- /dev/null
+++ b/drivers/rpmsg/imx_rpmsg.c
@@ -0,0 +1,542 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 NXP
+ */
+
+#include <linux/circ_buf.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/virtio_config.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_ring.h>
+#include <linux/imx_rpmsg.h>
+#include "rpmsg_internal.h"
+
+enum imx_rpmsg_variants {
+	IMX8MQ,
+	IMX8QXP,
+};
+
+struct imx_virdev {
+	struct virtio_device vdev;
+	unsigned int vring[2];
+	struct virtqueue *vq[2];
+	int base_vq_id;
+	int num_of_vqs;
+	struct imx_rpmsg_vproc *rpdev;
+};
+
+struct imx_rpmsg_vproc {
+	struct mbox_client cl;
+	struct mbox_client cl_rxdb;
+	struct mbox_chan *tx_ch;
+	struct mbox_chan *rx_ch;
+	struct mbox_chan *rxdb_ch;
+	enum imx_rpmsg_variants variant;
+	int vdev_nums;
+	int first_notify;
+	u32 flags;
+#define MAX_VDEV_NUMS  8
+	struct imx_virdev *ivdev[MAX_VDEV_NUMS];
+	struct delayed_work rpmsg_work;
+	struct circ_buf rx_buffer;
+	spinlock_t mu_lock;
+	struct notifier_block proc_nb;
+	struct platform_device *pdev;
+};
+
+#define RPMSG_NUM_BUFS		(512)
+#define RPMSG_BUF_SIZE		(512)
+#define RPMSG_BUFS_SPACE	(RPMSG_NUM_BUFS * RPMSG_BUF_SIZE)
+#define RPMSG_VRING_ALIGN	(4096)
+#define RPMSG_RING_SIZE	((DIV_ROUND_UP(vring_size(RPMSG_NUM_BUFS / 2, \
+				RPMSG_VRING_ALIGN), PAGE_SIZE)) * PAGE_SIZE)
+
+#define to_imx_virdev(vd) container_of(vd, struct imx_virdev, vdev)
+
+/*
+ * 1: indicated that remote processor is ready from re-initialization.
+ * Clear this bit after the RPMSG restore is finished at master side.
+ */
+#define REMOTE_IS_READY			BIT(0)
+/* 1: Use reserved memory region as DMA pool */
+#define SPECIFIC_DMA_POOL		BIT(1)
+
+struct imx_rpmsg_vq_info {
+	__u16 num;	/* number of entries in the virtio_ring */
+	__u16 vq_id;	/* a globaly unique index of this virtqueue */
+	void *addr;	/* address where we mapped the virtio ring */
+	struct imx_rpmsg_vproc *rpdev;
+};
+
+static u64 imx_rpmsg_get_features(struct virtio_device *vdev)
+{
+	/* VIRTIO_RPMSG_F_NS has been made private */
+	return 1 << 0;
+}
+
+static int imx_rpmsg_finalize_features(struct virtio_device *vdev)
+{
+	/* Give virtio_ring a chance to accept features */
+	vring_transport_features(vdev);
+	return 0;
+}
+
+/* kick the remote processor, and let it know which virtqueue to poke at */
+static bool imx_rpmsg_notify(struct virtqueue *vq)
+{
+	int ret;
+	unsigned long flags;
+	unsigned int mu_rpmsg = 0;
+	struct imx_rpmsg_vq_info *rpvq = vq->priv;
+	struct imx_rpmsg_vproc *rpdev = rpvq->rpdev;
+
+	mu_rpmsg = rpvq->vq_id << 16;
+	spin_lock_irqsave(&rpdev->mu_lock, flags);
+	/*
+	 * Send the index of the triggered virtqueue as the mu payload.
+	 * Use the timeout MU send message here.
+	 * Since that M4 core may not be loaded, and the first MSG may
+	 * not be handled by M4 when multi-vdev is enabled.
+	 * To make sure that the message wound't be discarded when M4
+	 * is running normally or in the suspend mode. Only use
+	 * the timeout mechanism by the first notify when the vdev is
+	 * registered.
+	 * ~14ms is required by M4 ready to process the MU message from
+	 * cold boot. Set the wait time 20ms here.
+	 */
+	if (unlikely(rpdev->first_notify > 0)) {
+		rpdev->first_notify--;
+		rpdev->cl.tx_tout = 20;
+		ret = mbox_send_message(rpdev->tx_ch, &mu_rpmsg);
+		if (ret < 0)
+			return false;
+	} else {
+		rpdev->cl.tx_tout = 0;
+		ret = mbox_send_message(rpdev->tx_ch, &mu_rpmsg);
+		if (ret < 0)
+			return false;
+	}
+	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
+
+	return true;
+}
+
+static struct virtqueue *rp_find_vq(struct virtio_device *vdev,
+				    unsigned int index,
+				    void (*callback)(struct virtqueue *vq),
+				    const char *name,
+				    bool ctx)
+{
+	struct imx_virdev *virdev = to_imx_virdev(vdev);
+	struct imx_rpmsg_vproc *rpdev = virdev->rpdev;
+	struct platform_device *pdev = rpdev->pdev;
+	struct device *dev = &pdev->dev;
+	struct imx_rpmsg_vq_info *rpvq;
+	struct virtqueue *vq;
+	int err;
+
+	rpvq = kmalloc(sizeof(*rpvq), GFP_KERNEL);
+	if (!rpvq)
+		return ERR_PTR(-ENOMEM);
+
+	/* ioremap'ing normal memory, so we cast away sparse's complaints */
+	rpvq->addr = (__force void *) ioremap_nocache(virdev->vring[index],
+							RPMSG_RING_SIZE);
+	if (!rpvq->addr) {
+		err = -ENOMEM;
+		goto free_rpvq;
+	}
+
+	memset_io(rpvq->addr, 0, RPMSG_RING_SIZE);
+
+	dev_dbg(dev, "vring%d: phys 0x%x, virt 0x%p\n",
+			index, virdev->vring[index], rpvq->addr);
+
+	vq = vring_new_virtqueue(index, RPMSG_NUM_BUFS / 2, RPMSG_VRING_ALIGN,
+			vdev, true, ctx,
+			rpvq->addr,
+			imx_rpmsg_notify, callback,
+			name);
+	if (!vq) {
+		dev_err(dev, "vring_new_virtqueue failed\n");
+		err = -ENOMEM;
+		goto unmap_vring;
+	}
+
+	virdev->vq[index] = vq;
+	vq->priv = rpvq;
+	/* system-wide unique id for this virtqueue */
+	rpvq->vq_id = virdev->base_vq_id + index;
+	rpvq->rpdev = rpdev;
+
+	return vq;
+
+unmap_vring:
+	/* iounmap normal memory, so make sparse happy */
+	iounmap((__force void __iomem *) rpvq->addr);
+free_rpvq:
+	kfree(rpvq);
+	return ERR_PTR(err);
+}
+
+static void imx_rpmsg_del_vqs(struct virtio_device *vdev)
+{
+	struct virtqueue *vq, *n;
+
+	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
+		struct imx_rpmsg_vq_info *rpvq = vq->priv;
+
+		iounmap(rpvq->addr);
+		vring_del_virtqueue(vq);
+		kfree(rpvq);
+	}
+}
+
+static int imx_rpmsg_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
+		       struct virtqueue *vqs[],
+		       vq_callback_t *callbacks[],
+		       const char * const names[],
+		       const bool *ctx,
+		       struct irq_affinity *desc)
+{
+	struct imx_virdev *virdev = to_imx_virdev(vdev);
+	int i, err;
+
+	/* we maintain two virtqueues per remote processor (for RX and TX) */
+	if (nvqs != 2)
+		return -EINVAL;
+
+	for (i = 0; i < nvqs; ++i) {
+		vqs[i] = rp_find_vq(vdev, i, callbacks[i], names[i],
+				ctx ? ctx[i] : false);
+		if (IS_ERR(vqs[i])) {
+			err = PTR_ERR(vqs[i]);
+			goto error;
+		}
+	}
+
+	virdev->num_of_vqs = nvqs;
+	return 0;
+
+error:
+	imx_rpmsg_del_vqs(vdev);
+	return err;
+}
+
+static void imx_rpmsg_reset(struct virtio_device *vdev)
+{
+	dev_dbg(&vdev->dev, "reset !\n");
+}
+
+static u8 imx_rpmsg_get_status(struct virtio_device *vdev)
+{
+	return 0;
+}
+
+static void imx_rpmsg_set_status(struct virtio_device *vdev, u8 status)
+{
+	dev_dbg(&vdev->dev, "%s new status: %d\n", __func__, status);
+}
+
+static void imx_rpmsg_vproc_release(struct device *dev)
+{
+	/* this handler is provided so driver core doesn't yell at us */
+}
+
+static struct virtio_config_ops imx_rpmsg_config_ops = {
+	.get_features	= imx_rpmsg_get_features,
+	.finalize_features = imx_rpmsg_finalize_features,
+	.find_vqs	= imx_rpmsg_find_vqs,
+	.del_vqs	= imx_rpmsg_del_vqs,
+	.reset		= imx_rpmsg_reset,
+	.set_status	= imx_rpmsg_set_status,
+	.get_status	= imx_rpmsg_get_status,
+};
+
+static const struct of_device_id imx_rpmsg_dt_ids[] = {
+	{ .compatible = "fsl,imx8mq-rpmsg", .data = (void *)IMX8MQ, },
+	{ .compatible = "fsl,imx8qxp-rpmsg", .data = (void *)IMX8QXP, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_rpmsg_dt_ids);
+
+static int set_vring_phy_buf(struct platform_device *pdev,
+		       struct imx_rpmsg_vproc *rpdev, int vdev_nums)
+{
+	struct resource *res;
+	resource_size_t size;
+	unsigned int start, end;
+	int i, ret = 0;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res) {
+		size = resource_size(res);
+		start = res->start;
+		end = res->start + size;
+		for (i = 0; i < vdev_nums; i++) {
+			rpdev->ivdev[i] = kzalloc(sizeof(struct imx_virdev),
+							GFP_KERNEL);
+			if (!rpdev->ivdev[i])
+				return -ENOMEM;
+
+			rpdev->ivdev[i]->vring[0] = start;
+			rpdev->ivdev[i]->vring[1] = start + 0x8000;
+			start += 0x10000;
+			if (start > end) {
+				dev_err(&pdev->dev,
+					"Too small memory size %x!\n",
+					(u32)size);
+				ret = -EINVAL;
+				break;
+			}
+		}
+	} else {
+		return -ENOMEM;
+	}
+
+	return ret;
+}
+
+static void rpmsg_work_handler(struct work_struct *work)
+{
+	u32 message;
+	unsigned long flags;
+	struct imx_virdev *virdev;
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct imx_rpmsg_vproc *rpdev = container_of(dwork,
+			struct imx_rpmsg_vproc, rpmsg_work);
+	struct circ_buf *cb = &rpdev->rx_buffer;
+	struct platform_device *pdev = rpdev->pdev;
+	struct device *dev = &pdev->dev;
+
+	spin_lock_irqsave(&rpdev->mu_lock, flags);
+	/* handle all incoming mu message */
+	while (CIRC_CNT(cb->head, cb->tail, PAGE_SIZE)) {
+		spin_unlock_irqrestore(&rpdev->mu_lock, flags);
+		message = (u32) cb->buf[cb->tail];
+		virdev = rpdev->ivdev[(message >> 16) / 2];
+
+		dev_dbg(dev, "%s msg: 0x%x\n", __func__, message);
+		message = message >> 16;
+		message -= virdev->base_vq_id;
+
+		/*
+		 * Currently both PENDING_MSG and explicit-virtqueue-index
+		 * messaging are supported.
+		 * Whatever approach is taken, at this point message contains
+		 * the index of the vring which was just triggered.
+		 */
+		if (message  < virdev->num_of_vqs)
+			vring_interrupt(message, virdev->vq[message]);
+		spin_lock_irqsave(&rpdev->mu_lock, flags);
+		cb->tail = CIRC_ADD(cb->tail, PAGE_SIZE, 4);
+	}
+	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
+}
+
+static int imx_rpmsg_partition_notify(struct notifier_block *nb,
+				      unsigned long event, void *group)
+{
+	/* Reserved for the partition reset. */
+	return 0;
+}
+
+static void imx_rpmsg_rxdb_callback(struct mbox_client *c, void *msg)
+{
+	unsigned long flags;
+	struct imx_rpmsg_vproc *rpdev = container_of(c,
+			struct imx_rpmsg_vproc, cl);
+
+	spin_lock_irqsave(&rpdev->mu_lock, flags);
+	rpdev->flags |= REMOTE_IS_READY;
+	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
+}
+
+static int imx_rpmsg_rxdb_channel_init(struct imx_rpmsg_vproc *rpdev)
+{
+	struct platform_device *pdev = rpdev->pdev;
+	struct device *dev = &pdev->dev;
+	struct mbox_client *cl;
+	int ret = 0;
+
+	cl = &rpdev->cl_rxdb;
+	cl->dev = dev;
+	cl->rx_callback = imx_rpmsg_rxdb_callback;
+
+	/*
+	 * RX door bell is used to receive the ready signal from remote
+	 * after the partition reset of A core.
+	 */
+	rpdev->rxdb_ch = mbox_request_channel_byname(cl, "rxdb");
+	if (IS_ERR(rpdev->rxdb_ch)) {
+		ret = PTR_ERR(rpdev->rxdb_ch);
+		dev_err(cl->dev, "failed to request mbox chan rxdb, ret %d\n",
+			ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static void imx_rpmsg_rx_callback(struct mbox_client *c, void *msg)
+{
+	int buf_space;
+	unsigned long flags;
+	u32 *data = msg;
+	struct imx_rpmsg_vproc *rpdev = container_of(c,
+			struct imx_rpmsg_vproc, cl);
+	struct circ_buf *cb = &rpdev->rx_buffer;
+
+	spin_lock_irqsave(&rpdev->mu_lock, flags);
+	buf_space = CIRC_SPACE(cb->head, cb->tail, PAGE_SIZE);
+	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
+	if (unlikely(!buf_space)) {
+		dev_err(c->dev, "RPMSG RX overflow!\n");
+		return;
+	}
+	spin_lock_irqsave(&rpdev->mu_lock, flags);
+	cb->buf[cb->head] = *data;
+	cb->head = CIRC_ADD(cb->head, PAGE_SIZE, 4);
+	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
+
+	schedule_delayed_work(&(rpdev->rpmsg_work), 0);
+}
+
+static int imx_rpmsg_probe(struct platform_device *pdev)
+{
+	int j, ret = 0;
+	char *buf;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct imx_rpmsg_vproc *rpdev;
+	struct mbox_client *cl;
+
+	buf = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	rpdev = devm_kzalloc(dev, sizeof(*rpdev), GFP_KERNEL);
+	if (!rpdev)
+		return -ENOMEM;
+
+	rpdev->proc_nb.notifier_call = imx_rpmsg_partition_notify;
+	rpdev->variant = (enum imx_rpmsg_variants)of_device_get_match_data(dev);
+	rpdev->rx_buffer.buf = buf;
+	rpdev->rx_buffer.head = 0;
+	rpdev->rx_buffer.tail = 0;
+
+	cl = &rpdev->cl;
+	cl->dev = dev;
+	cl->tx_block = false;
+	cl->tx_tout = 20;
+	cl->knows_txdone = false;
+	cl->rx_callback = imx_rpmsg_rx_callback;
+
+	rpdev->tx_ch = mbox_request_channel_byname(cl, "tx");
+	if (IS_ERR(rpdev->tx_ch)) {
+		ret = PTR_ERR(rpdev->tx_ch);
+		goto err_chl;
+	}
+	rpdev->rx_ch = mbox_request_channel_byname(cl, "rx");
+	if (IS_ERR(rpdev->rx_ch)) {
+		ret = PTR_ERR(rpdev->rx_ch);
+		goto err_chl;
+	}
+
+	spin_lock_init(&rpdev->mu_lock);
+	INIT_DELAYED_WORK(&(rpdev->rpmsg_work), rpmsg_work_handler);
+	ret = of_property_read_u32(np, "vdev-nums", &rpdev->vdev_nums);
+	if (ret)
+		rpdev->vdev_nums = 1;
+	if (rpdev->vdev_nums > MAX_VDEV_NUMS) {
+		dev_err(dev, "vdev-nums exceed the max %d\n", MAX_VDEV_NUMS);
+		ret = -EINVAL;
+		goto err_chl;
+	}
+	rpdev->first_notify = rpdev->vdev_nums;
+
+	ret = set_vring_phy_buf(pdev, rpdev, rpdev->vdev_nums);
+	if (ret) {
+		dev_err(dev, "No vring buffer.\n");
+		ret = -ENOMEM;
+		goto err_chl;
+	}
+	if (of_reserved_mem_device_init(dev)) {
+		dev_dbg(dev, "dev doesn't have specific DMA pool.\n");
+		rpdev->flags &= (~SPECIFIC_DMA_POOL);
+	} else {
+		rpdev->flags |= SPECIFIC_DMA_POOL;
+	}
+
+	for (j = 0; j < rpdev->vdev_nums; j++) {
+		dev_dbg(dev, "%s rpdev vdev%d: vring0 0x%x, vring1 0x%x\n",
+			 __func__, rpdev->vdev_nums,
+			 rpdev->ivdev[j]->vring[0],
+			 rpdev->ivdev[j]->vring[1]);
+		rpdev->ivdev[j]->vdev.id.device = VIRTIO_ID_RPMSG;
+		rpdev->ivdev[j]->vdev.config = &imx_rpmsg_config_ops;
+		rpdev->pdev = pdev;
+		rpdev->ivdev[j]->vdev.dev.parent = &pdev->dev;
+		rpdev->ivdev[j]->vdev.dev.release = imx_rpmsg_vproc_release;
+		rpdev->ivdev[j]->base_vq_id = j * 2;
+		rpdev->ivdev[j]->rpdev = rpdev;
+
+		ret = register_virtio_device(&rpdev->ivdev[j]->vdev);
+		if (ret) {
+			dev_err(dev, "%s failed to register rpdev: %d\n",
+					__func__, ret);
+			goto err_out;
+		}
+	}
+	/* Initialize the RX doorbell channel. */
+	ret = imx_rpmsg_rxdb_channel_init(rpdev);
+	if (ret)
+		goto err_out;
+
+	return ret;
+
+err_out:
+	if (rpdev->flags & SPECIFIC_DMA_POOL)
+		of_reserved_mem_device_release(dev);
+err_chl:
+	if (!IS_ERR(rpdev->rxdb_ch))
+		mbox_free_channel(rpdev->rxdb_ch);
+	if (!IS_ERR(rpdev->tx_ch))
+		mbox_free_channel(rpdev->tx_ch);
+	if (!IS_ERR(rpdev->rx_ch))
+		mbox_free_channel(rpdev->rx_ch);
+	return ret;
+}
+
+static struct platform_driver imx_rpmsg_driver = {
+	.driver = {
+		   .owner = THIS_MODULE,
+		   .name = "imx-rpmsg",
+		   .of_match_table = imx_rpmsg_dt_ids,
+		   },
+	.probe = imx_rpmsg_probe,
+};
+
+static int __init imx_rpmsg_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&imx_rpmsg_driver);
+	if (ret)
+		pr_err("Unable to initialize rpmsg driver\n");
+	else
+		pr_info("imx rpmsg driver is registered.\n");
+
+	return ret;
+}
+
+MODULE_DESCRIPTION("iMX remote processor messaging virtio device");
+MODULE_LICENSE("GPL v2");
+arch_initcall(imx_rpmsg_init);
diff --git a/drivers/rpmsg/imx_rpmsg_pingpong.c b/drivers/rpmsg/imx_rpmsg_pingpong.c
new file mode 100644
index 0000000..b028914
--- /dev/null
+++ b/drivers/rpmsg/imx_rpmsg_pingpong.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 NXP
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/virtio.h>
+#include <linux/rpmsg.h>
+
+#define MSG		"hello world!"
+static unsigned int rpmsg_pingpong;
+
+static int rpmsg_pingpong_cb(struct rpmsg_device *rpdev, void *data, int len,
+						void *priv, u32 src)
+{
+	int err;
+
+	/* reply */
+	rpmsg_pingpong = *(unsigned int *)data;
+	pr_info("get %d (src: 0x%x)\n", rpmsg_pingpong, src);
+
+	/* pingpongs should not live forever */
+	if (rpmsg_pingpong > 100) {
+		dev_info(&rpdev->dev, "goodbye!\n");
+		return 0;
+	}
+	rpmsg_pingpong++;
+	err = rpmsg_sendto(rpdev->ept, (void *)(&rpmsg_pingpong), 4, src);
+
+	if (err)
+		dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", err);
+
+	return err;
+}
+
+static int rpmsg_pingpong_probe(struct rpmsg_device *rpdev)
+{
+	int err;
+
+	dev_info(&rpdev->dev, "new channel: 0x%x -> 0x%x!\n",
+			rpdev->src, rpdev->dst);
+
+	/*
+	 * send a message to our remote processor, and tell remote
+	 * processor about this channel
+	 */
+	err = rpmsg_send(rpdev->ept, MSG, strlen(MSG));
+	if (err) {
+		dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", err);
+		return err;
+	}
+
+	rpmsg_pingpong = 0;
+	err = rpmsg_sendto(rpdev->ept, (void *)(&rpmsg_pingpong),
+			   4, rpdev->dst);
+	if (err) {
+		dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static void rpmsg_pingpong_remove(struct rpmsg_device *rpdev)
+{
+	dev_info(&rpdev->dev, "rpmsg pingpong driver is removed\n");
+}
+
+static struct rpmsg_device_id rpmsg_driver_pingpong_id_table[] = {
+	{ .name	= "rpmsg-openamp-demo-channel" },
+	{ .name	= "rpmsg-openamp-demo-channel-1" },
+	{ },
+};
+MODULE_DEVICE_TABLE(rpmsg, rpmsg_driver_pingpong_id_table);
+
+static struct rpmsg_driver rpmsg_pingpong_driver = {
+	.drv.name	= KBUILD_MODNAME,
+	.drv.owner	= THIS_MODULE,
+	.id_table	= rpmsg_driver_pingpong_id_table,
+	.probe		= rpmsg_pingpong_probe,
+	.callback	= rpmsg_pingpong_cb,
+	.remove		= rpmsg_pingpong_remove,
+};
+
+static int __init init(void)
+{
+	return register_rpmsg_driver(&rpmsg_pingpong_driver);
+}
+
+static void __exit fini(void)
+{
+	unregister_rpmsg_driver(&rpmsg_pingpong_driver);
+}
+module_init(init);
+module_exit(fini);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc.");
+MODULE_DESCRIPTION("iMX virtio remote processor messaging pingpong driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/imx_rpmsg.h b/include/linux/imx_rpmsg.h
new file mode 100644
index 0000000..e0d5e97
--- /dev/null
+++ b/include/linux/imx_rpmsg.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 NXP.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * @file linux/imx_rpmsg.h
+ *
+ * @brief Global header file for iMX RPMSG
+ *
+ * @ingroup RPMSG
+ */
+#ifndef __LINUX_IMX_RPMSG_H__
+#define __LINUX_IMX_RPMSG_H__
+
+/* Category define */
+#define IMX_RMPSG_LIFECYCLE	1
+#define IMX_RPMSG_PMIC		2
+#define IMX_RPMSG_AUDIO		3
+#define IMX_RPMSG_KEY		4
+#define IMX_RPMSG_GPIO		5
+#define IMX_RPMSG_RTC		6
+#define IMX_RPMSG_SENSOR	7
+/* rpmsg version */
+#define IMX_RMPSG_MAJOR		1
+#define IMX_RMPSG_MINOR		0
+
+#define CIRC_ADD(idx, size, value)	(((idx) + (value)) & ((size) - 1))
+
+struct imx_rpmsg_head {
+	u8 cate;
+	u8 major;
+	u8 minor;
+	u8 type;
+	u8 cmd;
+	u8 reserved[5];
+} __packed;
+
+#endif /* __LINUX_IMX_RPMSG_H__ */
-- 
2.7.4

Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Oleksij Rempel]

Hi Richard,

On 01.07.19 10:34, Richard Zhu wrote:
> Based on "virtio_rpmsg_bus" driver, This patch-set is used to set up
> the communication mechanism between A core and M core on i.MX AMP SOCs.
> 
> Add the initial imx rpmsg support glue driver and one pingpong demo,
> demonstrated the data transactions between A core and remote M core.
> Distributed framework is used in IMX RPMSG implementation, refer to the
> following requirements:
>    - The CAN functions contained in M core and RTOS should be ready and
>      complete functional in 50ms after AMP system is turned on.
>    - Partition reset. System wouldn't be stalled by the exceptions (e.x
>      the reset triggered by the system hang) occurred at the other side.
>      And the RPMSG mechanism should be recovered automactilly after the
>      partition reset is completed.
> In this scenario, the M core and RTOS would be kicked off by bootloader
> firstly, then A core and Linux would be loaded later. Both M core/RTOS
> and A core/Linux are running independly.
> 
> One physical memory region used to store the vring is mandatory required
> to pre-reserved and well-knowned by both A core and M core

I don't see any thing imx specific in this patch. We already have remoteproc which would 
parse firmware header and create needed devices. This driver is only needed for the case 
where firmware was stared by the bootloader.

I personally would prefer to have generic driver or extend the remoteproc framework. So we 
can notify kernel about work already done by bootloader.

In general, some more issues should be solved:
- Handle or not touch idle clocks for different node used by M core and not main system.
- pin control
- regulators

ST devs already tried to solve this issues by creating "remoteproc: add system resource 
manager device" patch. I don't know what is current state of it (/me adding ST devs to CC).

Beside, I ported remoteproc to barebox. So same firmware used for remoteproc on linux
can be used by bootloader (barebox):
http://lists.infradead.org/pipermail/barebox/2019-June/038395.html

> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>   drivers/rpmsg/Kconfig              |  24 ++
>   drivers/rpmsg/Makefile             |   2 +
>   drivers/rpmsg/imx_rpmsg.c          | 542 +++++++++++++++++++++++++++++++++++++
>   drivers/rpmsg/imx_rpmsg_pingpong.c | 100 +++++++
>   include/linux/imx_rpmsg.h          |  43 +++
>   5 files changed, 711 insertions(+)
>   create mode 100644 drivers/rpmsg/imx_rpmsg.c
>   create mode 100644 drivers/rpmsg/imx_rpmsg_pingpong.c
>   create mode 100644 include/linux/imx_rpmsg.h
> 
> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
> index d0322b4..636460e 100644
> --- a/drivers/rpmsg/Kconfig
> +++ b/drivers/rpmsg/Kconfig
> @@ -55,4 +55,28 @@ config RPMSG_VIRTIO
>   	select RPMSG
>   	select VIRTIO
>   
> +config HAVE_IMX_RPMSG
> +	bool "IMX RPMSG driver on the AMP SOCs"
> +	default y
> +	depends on IMX_MBOX
> +	select RPMSG_VIRTIO
> +	help
> +	  Say y here to enable support for the iMX Rpmsg Driver	providing
> +	  communication channels to remote processors in iMX asymmetric
> +	  multiprocessing (AMP) platforms.
> +
> +	  Especially, it is mandatory required when the partition reset is
> +	  required on some iMX AMP platforms.
> +
> +config IMX_RPMSG_PINGPONG
> +	tristate "IMX RPMSG pingpong driver -- loadable modules only"
> +	default m
> +	depends on HAVE_IMX_RPMSG && m
> +	help
> +	  One 32bit unsigned int data transactions demoe between the A core
> +	  and the remote M core on the iMX AMP platforms.
> +
> +	  Only the module mode is supported here, the demo would be kicked off
> +	  immediately when this module is insmoded.
> +
>   endmenu
> diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile
> index 9aa8595..9c6fce5 100644
> --- a/drivers/rpmsg/Makefile
> +++ b/drivers/rpmsg/Makefile
> @@ -6,3 +6,5 @@ obj-$(CONFIG_RPMSG_QCOM_GLINK_NATIVE) += qcom_glink_native.o
>   obj-$(CONFIG_RPMSG_QCOM_GLINK_SMEM) += qcom_glink_smem.o
>   obj-$(CONFIG_RPMSG_QCOM_SMD)	+= qcom_smd.o
>   obj-$(CONFIG_RPMSG_VIRTIO)	+= virtio_rpmsg_bus.o
> +obj-$(CONFIG_HAVE_IMX_RPMSG)	+= imx_rpmsg.o
> +obj-$(CONFIG_IMX_RPMSG_PINGPONG)	+= imx_rpmsg_pingpong.o
> diff --git a/drivers/rpmsg/imx_rpmsg.c b/drivers/rpmsg/imx_rpmsg.c
> new file mode 100644
> index 0000000..58888d1
> --- /dev/null
> +++ b/drivers/rpmsg/imx_rpmsg.c
> @@ -0,0 +1,542 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 NXP
> + */
> +
> +#include <linux/circ_buf.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/of_device.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/virtio_config.h>
> +#include <linux/virtio_ids.h>
> +#include <linux/virtio_ring.h>
> +#include <linux/imx_rpmsg.h>
> +#include "rpmsg_internal.h"
> +
> +enum imx_rpmsg_variants {
> +	IMX8MQ,
> +	IMX8QXP,
> +};
> +
> +struct imx_virdev {
> +	struct virtio_device vdev;
> +	unsigned int vring[2];
> +	struct virtqueue *vq[2];
> +	int base_vq_id;
> +	int num_of_vqs;
> +	struct imx_rpmsg_vproc *rpdev;
> +};
> +
> +struct imx_rpmsg_vproc {
> +	struct mbox_client cl;
> +	struct mbox_client cl_rxdb;
> +	struct mbox_chan *tx_ch;
> +	struct mbox_chan *rx_ch;
> +	struct mbox_chan *rxdb_ch;
> +	enum imx_rpmsg_variants variant;
> +	int vdev_nums;
> +	int first_notify;
> +	u32 flags;
> +#define MAX_VDEV_NUMS  8
> +	struct imx_virdev *ivdev[MAX_VDEV_NUMS];
> +	struct delayed_work rpmsg_work;
> +	struct circ_buf rx_buffer;
> +	spinlock_t mu_lock;
> +	struct notifier_block proc_nb;
> +	struct platform_device *pdev;
> +};
> +
> +#define RPMSG_NUM_BUFS		(512)
> +#define RPMSG_BUF_SIZE		(512)
> +#define RPMSG_BUFS_SPACE	(RPMSG_NUM_BUFS * RPMSG_BUF_SIZE)
> +#define RPMSG_VRING_ALIGN	(4096)
> +#define RPMSG_RING_SIZE	((DIV_ROUND_UP(vring_size(RPMSG_NUM_BUFS / 2, \
> +				RPMSG_VRING_ALIGN), PAGE_SIZE)) * PAGE_SIZE)
> +
> +#define to_imx_virdev(vd) container_of(vd, struct imx_virdev, vdev)
> +
> +/*
> + * 1: indicated that remote processor is ready from re-initialization.
> + * Clear this bit after the RPMSG restore is finished at master side.
> + */
> +#define REMOTE_IS_READY			BIT(0)
> +/* 1: Use reserved memory region as DMA pool */
> +#define SPECIFIC_DMA_POOL		BIT(1)
> +
> +struct imx_rpmsg_vq_info {
> +	__u16 num;	/* number of entries in the virtio_ring */
> +	__u16 vq_id;	/* a globaly unique index of this virtqueue */
> +	void *addr;	/* address where we mapped the virtio ring */
> +	struct imx_rpmsg_vproc *rpdev;
> +};
> +
> +static u64 imx_rpmsg_get_features(struct virtio_device *vdev)
> +{
> +	/* VIRTIO_RPMSG_F_NS has been made private */
> +	return 1 << 0;
> +}
> +
> +static int imx_rpmsg_finalize_features(struct virtio_device *vdev)
> +{
> +	/* Give virtio_ring a chance to accept features */
> +	vring_transport_features(vdev);
> +	return 0;
> +}
> +
> +/* kick the remote processor, and let it know which virtqueue to poke at */
> +static bool imx_rpmsg_notify(struct virtqueue *vq)
> +{
> +	int ret;
> +	unsigned long flags;
> +	unsigned int mu_rpmsg = 0;
> +	struct imx_rpmsg_vq_info *rpvq = vq->priv;
> +	struct imx_rpmsg_vproc *rpdev = rpvq->rpdev;
> +
> +	mu_rpmsg = rpvq->vq_id << 16;
> +	spin_lock_irqsave(&rpdev->mu_lock, flags);
> +	/*
> +	 * Send the index of the triggered virtqueue as the mu payload.
> +	 * Use the timeout MU send message here.
> +	 * Since that M4 core may not be loaded, and the first MSG may
> +	 * not be handled by M4 when multi-vdev is enabled.
> +	 * To make sure that the message wound't be discarded when M4
> +	 * is running normally or in the suspend mode. Only use
> +	 * the timeout mechanism by the first notify when the vdev is
> +	 * registered.
> +	 * ~14ms is required by M4 ready to process the MU message from
> +	 * cold boot. Set the wait time 20ms here.
> +	 */
> +	if (unlikely(rpdev->first_notify > 0)) {
> +		rpdev->first_notify--;
> +		rpdev->cl.tx_tout = 20;
> +		ret = mbox_send_message(rpdev->tx_ch, &mu_rpmsg);
> +		if (ret < 0)
> +			return false;
> +	} else {
> +		rpdev->cl.tx_tout = 0;
> +		ret = mbox_send_message(rpdev->tx_ch, &mu_rpmsg);
> +		if (ret < 0)
> +			return false;
> +	}
> +	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> +
> +	return true;
> +}
> +
> +static struct virtqueue *rp_find_vq(struct virtio_device *vdev,
> +				    unsigned int index,
> +				    void (*callback)(struct virtqueue *vq),
> +				    const char *name,
> +				    bool ctx)
> +{
> +	struct imx_virdev *virdev = to_imx_virdev(vdev);
> +	struct imx_rpmsg_vproc *rpdev = virdev->rpdev;
> +	struct platform_device *pdev = rpdev->pdev;
> +	struct device *dev = &pdev->dev;
> +	struct imx_rpmsg_vq_info *rpvq;
> +	struct virtqueue *vq;
> +	int err;
> +
> +	rpvq = kmalloc(sizeof(*rpvq), GFP_KERNEL);
> +	if (!rpvq)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* ioremap'ing normal memory, so we cast away sparse's complaints */
> +	rpvq->addr = (__force void *) ioremap_nocache(virdev->vring[index],
> +							RPMSG_RING_SIZE);
> +	if (!rpvq->addr) {
> +		err = -ENOMEM;
> +		goto free_rpvq;
> +	}
> +
> +	memset_io(rpvq->addr, 0, RPMSG_RING_SIZE);
> +
> +	dev_dbg(dev, "vring%d: phys 0x%x, virt 0x%p\n",
> +			index, virdev->vring[index], rpvq->addr);
> +
> +	vq = vring_new_virtqueue(index, RPMSG_NUM_BUFS / 2, RPMSG_VRING_ALIGN,
> +			vdev, true, ctx,
> +			rpvq->addr,
> +			imx_rpmsg_notify, callback,
> +			name);
> +	if (!vq) {
> +		dev_err(dev, "vring_new_virtqueue failed\n");
> +		err = -ENOMEM;
> +		goto unmap_vring;
> +	}
> +
> +	virdev->vq[index] = vq;
> +	vq->priv = rpvq;
> +	/* system-wide unique id for this virtqueue */
> +	rpvq->vq_id = virdev->base_vq_id + index;
> +	rpvq->rpdev = rpdev;
> +
> +	return vq;
> +
> +unmap_vring:
> +	/* iounmap normal memory, so make sparse happy */
> +	iounmap((__force void __iomem *) rpvq->addr);
> +free_rpvq:
> +	kfree(rpvq);
> +	return ERR_PTR(err);
> +}
> +
> +static void imx_rpmsg_del_vqs(struct virtio_device *vdev)
> +{
> +	struct virtqueue *vq, *n;
> +
> +	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
> +		struct imx_rpmsg_vq_info *rpvq = vq->priv;
> +
> +		iounmap(rpvq->addr);
> +		vring_del_virtqueue(vq);
> +		kfree(rpvq);
> +	}
> +}
> +
> +static int imx_rpmsg_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> +		       struct virtqueue *vqs[],
> +		       vq_callback_t *callbacks[],
> +		       const char * const names[],
> +		       const bool *ctx,
> +		       struct irq_affinity *desc)
> +{
> +	struct imx_virdev *virdev = to_imx_virdev(vdev);
> +	int i, err;
> +
> +	/* we maintain two virtqueues per remote processor (for RX and TX) */
> +	if (nvqs != 2)
> +		return -EINVAL;
> +
> +	for (i = 0; i < nvqs; ++i) {
> +		vqs[i] = rp_find_vq(vdev, i, callbacks[i], names[i],
> +				ctx ? ctx[i] : false);
> +		if (IS_ERR(vqs[i])) {
> +			err = PTR_ERR(vqs[i]);
> +			goto error;
> +		}
> +	}
> +
> +	virdev->num_of_vqs = nvqs;
> +	return 0;
> +
> +error:
> +	imx_rpmsg_del_vqs(vdev);
> +	return err;
> +}
> +
> +static void imx_rpmsg_reset(struct virtio_device *vdev)
> +{
> +	dev_dbg(&vdev->dev, "reset !\n");
> +}
> +
> +static u8 imx_rpmsg_get_status(struct virtio_device *vdev)
> +{
> +	return 0;
> +}
> +
> +static void imx_rpmsg_set_status(struct virtio_device *vdev, u8 status)
> +{
> +	dev_dbg(&vdev->dev, "%s new status: %d\n", __func__, status);
> +}
> +
> +static void imx_rpmsg_vproc_release(struct device *dev)
> +{
> +	/* this handler is provided so driver core doesn't yell at us */
> +}
> +
> +static struct virtio_config_ops imx_rpmsg_config_ops = {
> +	.get_features	= imx_rpmsg_get_features,
> +	.finalize_features = imx_rpmsg_finalize_features,
> +	.find_vqs	= imx_rpmsg_find_vqs,
> +	.del_vqs	= imx_rpmsg_del_vqs,
> +	.reset		= imx_rpmsg_reset,
> +	.set_status	= imx_rpmsg_set_status,
> +	.get_status	= imx_rpmsg_get_status,
> +};
> +
> +static const struct of_device_id imx_rpmsg_dt_ids[] = {
> +	{ .compatible = "fsl,imx8mq-rpmsg", .data = (void *)IMX8MQ, },
> +	{ .compatible = "fsl,imx8qxp-rpmsg", .data = (void *)IMX8QXP, },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx_rpmsg_dt_ids);
> +
> +static int set_vring_phy_buf(struct platform_device *pdev,
> +		       struct imx_rpmsg_vproc *rpdev, int vdev_nums)
> +{
> +	struct resource *res;
> +	resource_size_t size;
> +	unsigned int start, end;
> +	int i, ret = 0;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (res) {
> +		size = resource_size(res);
> +		start = res->start;
> +		end = res->start + size;
> +		for (i = 0; i < vdev_nums; i++) {
> +			rpdev->ivdev[i] = kzalloc(sizeof(struct imx_virdev),
> +							GFP_KERNEL);
> +			if (!rpdev->ivdev[i])
> +				return -ENOMEM;
> +
> +			rpdev->ivdev[i]->vring[0] = start;
> +			rpdev->ivdev[i]->vring[1] = start + 0x8000;
> +			start += 0x10000;
> +			if (start > end) {
> +				dev_err(&pdev->dev,
> +					"Too small memory size %x!\n",
> +					(u32)size);
> +				ret = -EINVAL;
> +				break;
> +			}
> +		}
> +	} else {
> +		return -ENOMEM;
> +	}
> +
> +	return ret;
> +}
> +
> +static void rpmsg_work_handler(struct work_struct *work)
> +{
> +	u32 message;
> +	unsigned long flags;
> +	struct imx_virdev *virdev;
> +	struct delayed_work *dwork = to_delayed_work(work);
> +	struct imx_rpmsg_vproc *rpdev = container_of(dwork,
> +			struct imx_rpmsg_vproc, rpmsg_work);
> +	struct circ_buf *cb = &rpdev->rx_buffer;
> +	struct platform_device *pdev = rpdev->pdev;
> +	struct device *dev = &pdev->dev;
> +
> +	spin_lock_irqsave(&rpdev->mu_lock, flags);
> +	/* handle all incoming mu message */
> +	while (CIRC_CNT(cb->head, cb->tail, PAGE_SIZE)) {
> +		spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> +		message = (u32) cb->buf[cb->tail];
> +		virdev = rpdev->ivdev[(message >> 16) / 2];
> +
> +		dev_dbg(dev, "%s msg: 0x%x\n", __func__, message);
> +		message = message >> 16;
> +		message -= virdev->base_vq_id;
> +
> +		/*
> +		 * Currently both PENDING_MSG and explicit-virtqueue-index
> +		 * messaging are supported.
> +		 * Whatever approach is taken, at this point message contains
> +		 * the index of the vring which was just triggered.
> +		 */
> +		if (message  < virdev->num_of_vqs)
> +			vring_interrupt(message, virdev->vq[message]);
> +		spin_lock_irqsave(&rpdev->mu_lock, flags);
> +		cb->tail = CIRC_ADD(cb->tail, PAGE_SIZE, 4);
> +	}
> +	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> +}
> +
> +static int imx_rpmsg_partition_notify(struct notifier_block *nb,
> +				      unsigned long event, void *group)
> +{
> +	/* Reserved for the partition reset. */
> +	return 0;
> +}
> +
> +static void imx_rpmsg_rxdb_callback(struct mbox_client *c, void *msg)
> +{
> +	unsigned long flags;
> +	struct imx_rpmsg_vproc *rpdev = container_of(c,
> +			struct imx_rpmsg_vproc, cl);
> +
> +	spin_lock_irqsave(&rpdev->mu_lock, flags);
> +	rpdev->flags |= REMOTE_IS_READY;
> +	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> +}
> +
> +static int imx_rpmsg_rxdb_channel_init(struct imx_rpmsg_vproc *rpdev)
> +{
> +	struct platform_device *pdev = rpdev->pdev;
> +	struct device *dev = &pdev->dev;
> +	struct mbox_client *cl;
> +	int ret = 0;
> +
> +	cl = &rpdev->cl_rxdb;
> +	cl->dev = dev;
> +	cl->rx_callback = imx_rpmsg_rxdb_callback;
> +
> +	/*
> +	 * RX door bell is used to receive the ready signal from remote
> +	 * after the partition reset of A core.
> +	 */
> +	rpdev->rxdb_ch = mbox_request_channel_byname(cl, "rxdb");
> +	if (IS_ERR(rpdev->rxdb_ch)) {
> +		ret = PTR_ERR(rpdev->rxdb_ch);
> +		dev_err(cl->dev, "failed to request mbox chan rxdb, ret %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static void imx_rpmsg_rx_callback(struct mbox_client *c, void *msg)
> +{
> +	int buf_space;
> +	unsigned long flags;
> +	u32 *data = msg;
> +	struct imx_rpmsg_vproc *rpdev = container_of(c,
> +			struct imx_rpmsg_vproc, cl);
> +	struct circ_buf *cb = &rpdev->rx_buffer;
> +
> +	spin_lock_irqsave(&rpdev->mu_lock, flags);
> +	buf_space = CIRC_SPACE(cb->head, cb->tail, PAGE_SIZE);
> +	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> +	if (unlikely(!buf_space)) {
> +		dev_err(c->dev, "RPMSG RX overflow!\n");
> +		return;
> +	}
> +	spin_lock_irqsave(&rpdev->mu_lock, flags);
> +	cb->buf[cb->head] = *data;
> +	cb->head = CIRC_ADD(cb->head, PAGE_SIZE, 4);
> +	spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> +
> +	schedule_delayed_work(&(rpdev->rpmsg_work), 0);
> +}
> +
> +static int imx_rpmsg_probe(struct platform_device *pdev)
> +{
> +	int j, ret = 0;
> +	char *buf;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct imx_rpmsg_vproc *rpdev;
> +	struct mbox_client *cl;
> +
> +	buf = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	rpdev = devm_kzalloc(dev, sizeof(*rpdev), GFP_KERNEL);
> +	if (!rpdev)
> +		return -ENOMEM;
> +
> +	rpdev->proc_nb.notifier_call = imx_rpmsg_partition_notify;
> +	rpdev->variant = (enum imx_rpmsg_variants)of_device_get_match_data(dev);
> +	rpdev->rx_buffer.buf = buf;
> +	rpdev->rx_buffer.head = 0;
> +	rpdev->rx_buffer.tail = 0;
> +
> +	cl = &rpdev->cl;
> +	cl->dev = dev;
> +	cl->tx_block = false;
> +	cl->tx_tout = 20;
> +	cl->knows_txdone = false;
> +	cl->rx_callback = imx_rpmsg_rx_callback;
> +
> +	rpdev->tx_ch = mbox_request_channel_byname(cl, "tx");
> +	if (IS_ERR(rpdev->tx_ch)) {
> +		ret = PTR_ERR(rpdev->tx_ch);
> +		goto err_chl;
> +	}
> +	rpdev->rx_ch = mbox_request_channel_byname(cl, "rx");
> +	if (IS_ERR(rpdev->rx_ch)) {
> +		ret = PTR_ERR(rpdev->rx_ch);
> +		goto err_chl;
> +	}
> +
> +	spin_lock_init(&rpdev->mu_lock);
> +	INIT_DELAYED_WORK(&(rpdev->rpmsg_work), rpmsg_work_handler);
> +	ret = of_property_read_u32(np, "vdev-nums", &rpdev->vdev_nums);
> +	if (ret)
> +		rpdev->vdev_nums = 1;
> +	if (rpdev->vdev_nums > MAX_VDEV_NUMS) {
> +		dev_err(dev, "vdev-nums exceed the max %d\n", MAX_VDEV_NUMS);
> +		ret = -EINVAL;
> +		goto err_chl;
> +	}
> +	rpdev->first_notify = rpdev->vdev_nums;
> +
> +	ret = set_vring_phy_buf(pdev, rpdev, rpdev->vdev_nums);
> +	if (ret) {
> +		dev_err(dev, "No vring buffer.\n");
> +		ret = -ENOMEM;
> +		goto err_chl;
> +	}
> +	if (of_reserved_mem_device_init(dev)) {
> +		dev_dbg(dev, "dev doesn't have specific DMA pool.\n");
> +		rpdev->flags &= (~SPECIFIC_DMA_POOL);
> +	} else {
> +		rpdev->flags |= SPECIFIC_DMA_POOL;
> +	}
> +
> +	for (j = 0; j < rpdev->vdev_nums; j++) {
> +		dev_dbg(dev, "%s rpdev vdev%d: vring0 0x%x, vring1 0x%x\n",
> +			 __func__, rpdev->vdev_nums,
> +			 rpdev->ivdev[j]->vring[0],
> +			 rpdev->ivdev[j]->vring[1]);
> +		rpdev->ivdev[j]->vdev.id.device = VIRTIO_ID_RPMSG;
> +		rpdev->ivdev[j]->vdev.config = &imx_rpmsg_config_ops;
> +		rpdev->pdev = pdev;
> +		rpdev->ivdev[j]->vdev.dev.parent = &pdev->dev;
> +		rpdev->ivdev[j]->vdev.dev.release = imx_rpmsg_vproc_release;
> +		rpdev->ivdev[j]->base_vq_id = j * 2;
> +		rpdev->ivdev[j]->rpdev = rpdev;
> +
> +		ret = register_virtio_device(&rpdev->ivdev[j]->vdev);
> +		if (ret) {
> +			dev_err(dev, "%s failed to register rpdev: %d\n",
> +					__func__, ret);
> +			goto err_out;
> +		}
> +	}
> +	/* Initialize the RX doorbell channel. */
> +	ret = imx_rpmsg_rxdb_channel_init(rpdev);
> +	if (ret)
> +		goto err_out;
> +
> +	return ret;
> +
> +err_out:
> +	if (rpdev->flags & SPECIFIC_DMA_POOL)
> +		of_reserved_mem_device_release(dev);
> +err_chl:
> +	if (!IS_ERR(rpdev->rxdb_ch))
> +		mbox_free_channel(rpdev->rxdb_ch);
> +	if (!IS_ERR(rpdev->tx_ch))
> +		mbox_free_channel(rpdev->tx_ch);
> +	if (!IS_ERR(rpdev->rx_ch))
> +		mbox_free_channel(rpdev->rx_ch);
> +	return ret;
> +}
> +
> +static struct platform_driver imx_rpmsg_driver = {
> +	.driver = {
> +		   .owner = THIS_MODULE,
> +		   .name = "imx-rpmsg",
> +		   .of_match_table = imx_rpmsg_dt_ids,
> +		   },
> +	.probe = imx_rpmsg_probe,
> +};
> +
> +static int __init imx_rpmsg_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&imx_rpmsg_driver);
> +	if (ret)
> +		pr_err("Unable to initialize rpmsg driver\n");
> +	else
> +		pr_info("imx rpmsg driver is registered.\n");
> +
> +	return ret;
> +}
> +
> +MODULE_DESCRIPTION("iMX remote processor messaging virtio device");
> +MODULE_LICENSE("GPL v2");
> +arch_initcall(imx_rpmsg_init);
> diff --git a/drivers/rpmsg/imx_rpmsg_pingpong.c b/drivers/rpmsg/imx_rpmsg_pingpong.c
> new file mode 100644
> index 0000000..b028914
> --- /dev/null
> +++ b/drivers/rpmsg/imx_rpmsg_pingpong.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 NXP
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/virtio.h>
> +#include <linux/rpmsg.h>
> +
> +#define MSG		"hello world!"
> +static unsigned int rpmsg_pingpong;
> +
> +static int rpmsg_pingpong_cb(struct rpmsg_device *rpdev, void *data, int len,
> +						void *priv, u32 src)
> +{
> +	int err;
> +
> +	/* reply */
> +	rpmsg_pingpong = *(unsigned int *)data;
> +	pr_info("get %d (src: 0x%x)\n", rpmsg_pingpong, src);
> +
> +	/* pingpongs should not live forever */
> +	if (rpmsg_pingpong > 100) {
> +		dev_info(&rpdev->dev, "goodbye!\n");
> +		return 0;
> +	}
> +	rpmsg_pingpong++;
> +	err = rpmsg_sendto(rpdev->ept, (void *)(&rpmsg_pingpong), 4, src);
> +
> +	if (err)
> +		dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", err);
> +
> +	return err;
> +}
> +
> +static int rpmsg_pingpong_probe(struct rpmsg_device *rpdev)
> +{
> +	int err;
> +
> +	dev_info(&rpdev->dev, "new channel: 0x%x -> 0x%x!\n",
> +			rpdev->src, rpdev->dst);
> +
> +	/*
> +	 * send a message to our remote processor, and tell remote
> +	 * processor about this channel
> +	 */
> +	err = rpmsg_send(rpdev->ept, MSG, strlen(MSG));
> +	if (err) {
> +		dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", err);
> +		return err;
> +	}
> +
> +	rpmsg_pingpong = 0;
> +	err = rpmsg_sendto(rpdev->ept, (void *)(&rpmsg_pingpong),
> +			   4, rpdev->dst);
> +	if (err) {
> +		dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static void rpmsg_pingpong_remove(struct rpmsg_device *rpdev)
> +{
> +	dev_info(&rpdev->dev, "rpmsg pingpong driver is removed\n");
> +}
> +
> +static struct rpmsg_device_id rpmsg_driver_pingpong_id_table[] = {
> +	{ .name	= "rpmsg-openamp-demo-channel" },
> +	{ .name	= "rpmsg-openamp-demo-channel-1" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(rpmsg, rpmsg_driver_pingpong_id_table);
> +
> +static struct rpmsg_driver rpmsg_pingpong_driver = {
> +	.drv.name	= KBUILD_MODNAME,
> +	.drv.owner	= THIS_MODULE,
> +	.id_table	= rpmsg_driver_pingpong_id_table,
> +	.probe		= rpmsg_pingpong_probe,
> +	.callback	= rpmsg_pingpong_cb,
> +	.remove		= rpmsg_pingpong_remove,
> +};
> +
> +static int __init init(void)
> +{
> +	return register_rpmsg_driver(&rpmsg_pingpong_driver);
> +}
> +
> +static void __exit fini(void)
> +{
> +	unregister_rpmsg_driver(&rpmsg_pingpong_driver);
> +}
> +module_init(init);
> +module_exit(fini);
> +
> +MODULE_AUTHOR("Freescale Semiconductor, Inc.");
> +MODULE_DESCRIPTION("iMX virtio remote processor messaging pingpong driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/imx_rpmsg.h b/include/linux/imx_rpmsg.h
> new file mode 100644
> index 0000000..e0d5e97
> --- /dev/null
> +++ b/include/linux/imx_rpmsg.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019 NXP.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +/*
> + * @file linux/imx_rpmsg.h
> + *
> + * @brief Global header file for iMX RPMSG
> + *
> + * @ingroup RPMSG
> + */
> +#ifndef __LINUX_IMX_RPMSG_H__
> +#define __LINUX_IMX_RPMSG_H__
> +
> +/* Category define */
> +#define IMX_RMPSG_LIFECYCLE	1
> +#define IMX_RPMSG_PMIC		2
> +#define IMX_RPMSG_AUDIO		3
> +#define IMX_RPMSG_KEY		4
> +#define IMX_RPMSG_GPIO		5
> +#define IMX_RPMSG_RTC		6
> +#define IMX_RPMSG_SENSOR	7
> +/* rpmsg version */
> +#define IMX_RMPSG_MAJOR		1
> +#define IMX_RMPSG_MINOR		0
> +
> +#define CIRC_ADD(idx, size, value)	(((idx) + (value)) & ((size) - 1))
> +
> +struct imx_rpmsg_head {
> +	u8 cate;
> +	u8 major;
> +	u8 minor;
> +	u8 type;
> +	u8 cmd;
> +	u8 reserved[5];
> +} __packed;
> +
> +#endif /* __LINUX_IMX_RPMSG_H__ */
> 

Kind regards,
Oleksij Rempel

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

RE: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Richard Zhu]

Hi Oleksij:
Thanks for your comments.


> -----Original Message-----
> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
> Sent: 2019年7月4日 17:36
> To: Richard Zhu <hongxing.zhu@nxp.com>; ohad@wizery.com;
> bjorn.andersson@linaro.org; linux-remoteproc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org; Fabien DESSENNE
> <fabien.dessenne@st.com>; loic.pallardy@st.com; arnaud.pouliquen@st.com;
> s-anna@ti.com; elder@linaro.org
> Subject: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support
> 
> Caution: EXT Email
> 
> Hi Richard,
> 
> On 01.07.19 10:34, Richard Zhu wrote:
> > Based on "virtio_rpmsg_bus" driver, This patch-set is used to set up
> > the communication mechanism between A core and M core on i.MX AMP
> SOCs.
> >
> > Add the initial imx rpmsg support glue driver and one pingpong demo,
> > demonstrated the data transactions between A core and remote M core.
> > Distributed framework is used in IMX RPMSG implementation, refer to
> > the following requirements:
> >    - The CAN functions contained in M core and RTOS should be ready and
> >      complete functional in 50ms after AMP system is turned on.
> >    - Partition reset. System wouldn't be stalled by the exceptions (e.x
> >      the reset triggered by the system hang) occurred at the other side.
> >      And the RPMSG mechanism should be recovered automactilly after
> the
> >      partition reset is completed.
> > In this scenario, the M core and RTOS would be kicked off by
> > bootloader firstly, then A core and Linux would be loaded later. Both
> > M core/RTOS and A core/Linux are running independly.
> >
> > One physical memory region used to store the vring is mandatory
> > required to pre-reserved and well-knowned by both A core and M core
> 
> I don't see any thing imx specific in this patch. We already have remoteproc
> which would parse firmware header and create needed devices. This driver is
> only needed for the case where firmware was stared by the bootloader.
> 
[Richard Zhu] Bootloader starts the firmware is mandatory required in these scenario
refer to the reasons listed in the commit.
Thus, the distributed framework has to be used, and both A core/Linux and remote core/RTOS
works independently.

> I personally would prefer to have generic driver or extend the remoteproc
> framework. So we can notify kernel about work already done by bootloader.
> 
[Richard Zhu] Thanks for your suggestions.
Regarding to my understand, it seems that master/slave mode is used in the remoteproc currently.
A core/Linux acts as master, to controls/manipulates remote core/RTOS.
It isn't applicable for the scenario described by this patch-set.

> In general, some more issues should be solved:
> - Handle or not touch idle clocks for different node used by M core and not
> main system.
> - pin control
> - regulators
> 
> ST devs already tried to solve this issues by creating "remoteproc: add system
> resource manager device" patch. I don't know what is current state of it (/me
> adding ST devs to CC).
> 
[Richard Zhu] Yes, it is. Many contributions have been made by Fabien.
IMHO, there are some different behaviors on iMX8QXP/QM platforms, the
 resources (e.x IP modules) had been assigned and managed by the XRDC.
In the other words, the HW resources would be assigned and managed would
 be transparent to SW.

Thus, both A core/Linux and M core/RTOS can work real independently.
System wouldn't be stalled by the exceptions (e.x the reset triggered by the 
system hang) occurred at the other side. And the RPMSG mechanism should
 be recovered automatically after the partition reset is completed.

> Beside, I ported remoteproc to barebox. So same firmware used for
> remoteproc on linux can be used by bootloader (barebox):
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infr
> adead.org%2Fpipermail%2Fbarebox%2F2019-June%2F038395.html&amp;dat
> a=02%7C01%7Chongxing.zhu%40nxp.com%7Cdd44732d8d084ca7077b08d70
> 06370cc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63697829
> 9412610922&amp;sdata=z%2BV5UG5SI7nvE4V7es6cSST5iYoZ9jpvkExsHS%2F
> jDjU%3D&amp;reserved=0
> 
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >   drivers/rpmsg/Kconfig              |  24 ++
> >   drivers/rpmsg/Makefile             |   2 +
> >   drivers/rpmsg/imx_rpmsg.c          | 542
> +++++++++++++++++++++++++++++++++++++
> >   drivers/rpmsg/imx_rpmsg_pingpong.c | 100 +++++++
> >   include/linux/imx_rpmsg.h          |  43 +++
> >   5 files changed, 711 insertions(+)
> >   create mode 100644 drivers/rpmsg/imx_rpmsg.c
> >   create mode 100644 drivers/rpmsg/imx_rpmsg_pingpong.c
> >   create mode 100644 include/linux/imx_rpmsg.h
> >
> > diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig index
> > d0322b4..636460e 100644
> > --- a/drivers/rpmsg/Kconfig
> > +++ b/drivers/rpmsg/Kconfig
> > @@ -55,4 +55,28 @@ config RPMSG_VIRTIO
> >       select RPMSG
> >       select VIRTIO
> >
> > +config HAVE_IMX_RPMSG
> > +     bool "IMX RPMSG driver on the AMP SOCs"
> > +     default y
> > +     depends on IMX_MBOX
> > +     select RPMSG_VIRTIO
> > +     help
> > +       Say y here to enable support for the iMX Rpmsg Driver providing
> > +       communication channels to remote processors in iMX asymmetric
> > +       multiprocessing (AMP) platforms.
> > +
> > +       Especially, it is mandatory required when the partition reset is
> > +       required on some iMX AMP platforms.
> > +
> > +config IMX_RPMSG_PINGPONG
> > +     tristate "IMX RPMSG pingpong driver -- loadable modules only"
> > +     default m
> > +     depends on HAVE_IMX_RPMSG && m
> > +     help
> > +       One 32bit unsigned int data transactions demoe between the A
> core
> > +       and the remote M core on the iMX AMP platforms.
> > +
> > +       Only the module mode is supported here, the demo would be
> kicked off
> > +       immediately when this module is insmoded.
> > +
> >   endmenu
> > diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile index
> > 9aa8595..9c6fce5 100644
> > --- a/drivers/rpmsg/Makefile
> > +++ b/drivers/rpmsg/Makefile
> > @@ -6,3 +6,5 @@ obj-$(CONFIG_RPMSG_QCOM_GLINK_NATIVE) +=
> qcom_glink_native.o
> >   obj-$(CONFIG_RPMSG_QCOM_GLINK_SMEM) += qcom_glink_smem.o
> >   obj-$(CONFIG_RPMSG_QCOM_SMD)        += qcom_smd.o
> >   obj-$(CONFIG_RPMSG_VIRTIO)  += virtio_rpmsg_bus.o
> > +obj-$(CONFIG_HAVE_IMX_RPMSG) += imx_rpmsg.o
> > +obj-$(CONFIG_IMX_RPMSG_PINGPONG)     += imx_rpmsg_pingpong.o
> > diff --git a/drivers/rpmsg/imx_rpmsg.c b/drivers/rpmsg/imx_rpmsg.c new
> > file mode 100644 index 0000000..58888d1
> > --- /dev/null
> > +++ b/drivers/rpmsg/imx_rpmsg.c
> > @@ -0,0 +1,542 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2019 NXP
> > + */
> > +
> > +#include <linux/circ_buf.h>
> > +#include <linux/err.h>
> > +#include <linux/init.h>
> > +#include <linux/mailbox_client.h>
> > +#include <linux/module.h>
> > +#include <linux/notifier.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_reserved_mem.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/virtio_config.h>
> > +#include <linux/virtio_ids.h>
> > +#include <linux/virtio_ring.h>
> > +#include <linux/imx_rpmsg.h>
> > +#include "rpmsg_internal.h"
> > +
> > +enum imx_rpmsg_variants {
> > +     IMX8MQ,
> > +     IMX8QXP,
> > +};
> > +
> > +struct imx_virdev {
> > +     struct virtio_device vdev;
> > +     unsigned int vring[2];
> > +     struct virtqueue *vq[2];
> > +     int base_vq_id;
> > +     int num_of_vqs;
> > +     struct imx_rpmsg_vproc *rpdev;
> > +};
> > +
> > +struct imx_rpmsg_vproc {
> > +     struct mbox_client cl;
> > +     struct mbox_client cl_rxdb;
> > +     struct mbox_chan *tx_ch;
> > +     struct mbox_chan *rx_ch;
> > +     struct mbox_chan *rxdb_ch;
> > +     enum imx_rpmsg_variants variant;
> > +     int vdev_nums;
> > +     int first_notify;
> > +     u32 flags;
> > +#define MAX_VDEV_NUMS  8
> > +     struct imx_virdev *ivdev[MAX_VDEV_NUMS];
> > +     struct delayed_work rpmsg_work;
> > +     struct circ_buf rx_buffer;
> > +     spinlock_t mu_lock;
> > +     struct notifier_block proc_nb;
> > +     struct platform_device *pdev;
> > +};
> > +
> > +#define RPMSG_NUM_BUFS               (512)
> > +#define RPMSG_BUF_SIZE               (512)
> > +#define RPMSG_BUFS_SPACE     (RPMSG_NUM_BUFS *
> RPMSG_BUF_SIZE)
> > +#define RPMSG_VRING_ALIGN    (4096)
> > +#define RPMSG_RING_SIZE
> ((DIV_ROUND_UP(vring_size(RPMSG_NUM_BUFS / 2, \
> > +                             RPMSG_VRING_ALIGN), PAGE_SIZE)) *
> > +PAGE_SIZE)
> > +
> > +#define to_imx_virdev(vd) container_of(vd, struct imx_virdev, vdev)
> > +
> > +/*
> > + * 1: indicated that remote processor is ready from re-initialization.
> > + * Clear this bit after the RPMSG restore is finished at master side.
> > + */
> > +#define REMOTE_IS_READY                      BIT(0)
> > +/* 1: Use reserved memory region as DMA pool */
> > +#define SPECIFIC_DMA_POOL            BIT(1)
> > +
> > +struct imx_rpmsg_vq_info {
> > +     __u16 num;      /* number of entries in the virtio_ring */
> > +     __u16 vq_id;    /* a globaly unique index of this virtqueue */
> > +     void *addr;     /* address where we mapped the virtio ring */
> > +     struct imx_rpmsg_vproc *rpdev;
> > +};
> > +
> > +static u64 imx_rpmsg_get_features(struct virtio_device *vdev) {
> > +     /* VIRTIO_RPMSG_F_NS has been made private */
> > +     return 1 << 0;
> > +}
> > +
> > +static int imx_rpmsg_finalize_features(struct virtio_device *vdev) {
> > +     /* Give virtio_ring a chance to accept features */
> > +     vring_transport_features(vdev);
> > +     return 0;
> > +}
> > +
> > +/* kick the remote processor, and let it know which virtqueue to poke
> > +at */ static bool imx_rpmsg_notify(struct virtqueue *vq) {
> > +     int ret;
> > +     unsigned long flags;
> > +     unsigned int mu_rpmsg = 0;
> > +     struct imx_rpmsg_vq_info *rpvq = vq->priv;
> > +     struct imx_rpmsg_vproc *rpdev = rpvq->rpdev;
> > +
> > +     mu_rpmsg = rpvq->vq_id << 16;
> > +     spin_lock_irqsave(&rpdev->mu_lock, flags);
> > +     /*
> > +      * Send the index of the triggered virtqueue as the mu payload.
> > +      * Use the timeout MU send message here.
> > +      * Since that M4 core may not be loaded, and the first MSG may
> > +      * not be handled by M4 when multi-vdev is enabled.
> > +      * To make sure that the message wound't be discarded when M4
> > +      * is running normally or in the suspend mode. Only use
> > +      * the timeout mechanism by the first notify when the vdev is
> > +      * registered.
> > +      * ~14ms is required by M4 ready to process the MU message from
> > +      * cold boot. Set the wait time 20ms here.
> > +      */
> > +     if (unlikely(rpdev->first_notify > 0)) {
> > +             rpdev->first_notify--;
> > +             rpdev->cl.tx_tout = 20;
> > +             ret = mbox_send_message(rpdev->tx_ch, &mu_rpmsg);
> > +             if (ret < 0)
> > +                     return false;
> > +     } else {
> > +             rpdev->cl.tx_tout = 0;
> > +             ret = mbox_send_message(rpdev->tx_ch, &mu_rpmsg);
> > +             if (ret < 0)
> > +                     return false;
> > +     }
> > +     spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> > +
> > +     return true;
> > +}
> > +
> > +static struct virtqueue *rp_find_vq(struct virtio_device *vdev,
> > +                                 unsigned int index,
> > +                                 void (*callback)(struct virtqueue
> *vq),
> > +                                 const char *name,
> > +                                 bool ctx) {
> > +     struct imx_virdev *virdev = to_imx_virdev(vdev);
> > +     struct imx_rpmsg_vproc *rpdev = virdev->rpdev;
> > +     struct platform_device *pdev = rpdev->pdev;
> > +     struct device *dev = &pdev->dev;
> > +     struct imx_rpmsg_vq_info *rpvq;
> > +     struct virtqueue *vq;
> > +     int err;
> > +
> > +     rpvq = kmalloc(sizeof(*rpvq), GFP_KERNEL);
> > +     if (!rpvq)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     /* ioremap'ing normal memory, so we cast away sparse's complaints
> */
> > +     rpvq->addr = (__force void *)
> ioremap_nocache(virdev->vring[index],
> > +
> RPMSG_RING_SIZE);
> > +     if (!rpvq->addr) {
> > +             err = -ENOMEM;
> > +             goto free_rpvq;
> > +     }
> > +
> > +     memset_io(rpvq->addr, 0, RPMSG_RING_SIZE);
> > +
> > +     dev_dbg(dev, "vring%d: phys 0x%x, virt 0x%p\n",
> > +                     index, virdev->vring[index], rpvq->addr);
> > +
> > +     vq = vring_new_virtqueue(index, RPMSG_NUM_BUFS / 2,
> RPMSG_VRING_ALIGN,
> > +                     vdev, true, ctx,
> > +                     rpvq->addr,
> > +                     imx_rpmsg_notify, callback,
> > +                     name);
> > +     if (!vq) {
> > +             dev_err(dev, "vring_new_virtqueue failed\n");
> > +             err = -ENOMEM;
> > +             goto unmap_vring;
> > +     }
> > +
> > +     virdev->vq[index] = vq;
> > +     vq->priv = rpvq;
> > +     /* system-wide unique id for this virtqueue */
> > +     rpvq->vq_id = virdev->base_vq_id + index;
> > +     rpvq->rpdev = rpdev;
> > +
> > +     return vq;
> > +
> > +unmap_vring:
> > +     /* iounmap normal memory, so make sparse happy */
> > +     iounmap((__force void __iomem *) rpvq->addr);
> > +free_rpvq:
> > +     kfree(rpvq);
> > +     return ERR_PTR(err);
> > +}
> > +
> > +static void imx_rpmsg_del_vqs(struct virtio_device *vdev) {
> > +     struct virtqueue *vq, *n;
> > +
> > +     list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
> > +             struct imx_rpmsg_vq_info *rpvq = vq->priv;
> > +
> > +             iounmap(rpvq->addr);
> > +             vring_del_virtqueue(vq);
> > +             kfree(rpvq);
> > +     }
> > +}
> > +
> > +static int imx_rpmsg_find_vqs(struct virtio_device *vdev, unsigned int
> nvqs,
> > +                    struct virtqueue *vqs[],
> > +                    vq_callback_t *callbacks[],
> > +                    const char * const names[],
> > +                    const bool *ctx,
> > +                    struct irq_affinity *desc) {
> > +     struct imx_virdev *virdev = to_imx_virdev(vdev);
> > +     int i, err;
> > +
> > +     /* we maintain two virtqueues per remote processor (for RX and TX)
> */
> > +     if (nvqs != 2)
> > +             return -EINVAL;
> > +
> > +     for (i = 0; i < nvqs; ++i) {
> > +             vqs[i] = rp_find_vq(vdev, i, callbacks[i], names[i],
> > +                             ctx ? ctx[i] : false);
> > +             if (IS_ERR(vqs[i])) {
> > +                     err = PTR_ERR(vqs[i]);
> > +                     goto error;
> > +             }
> > +     }
> > +
> > +     virdev->num_of_vqs = nvqs;
> > +     return 0;
> > +
> > +error:
> > +     imx_rpmsg_del_vqs(vdev);
> > +     return err;
> > +}
> > +
> > +static void imx_rpmsg_reset(struct virtio_device *vdev) {
> > +     dev_dbg(&vdev->dev, "reset !\n"); }
> > +
> > +static u8 imx_rpmsg_get_status(struct virtio_device *vdev) {
> > +     return 0;
> > +}
> > +
> > +static void imx_rpmsg_set_status(struct virtio_device *vdev, u8
> > +status) {
> > +     dev_dbg(&vdev->dev, "%s new status: %d\n", __func__, status); }
> > +
> > +static void imx_rpmsg_vproc_release(struct device *dev) {
> > +     /* this handler is provided so driver core doesn't yell at us */
> > +}
> > +
> > +static struct virtio_config_ops imx_rpmsg_config_ops = {
> > +     .get_features   = imx_rpmsg_get_features,
> > +     .finalize_features = imx_rpmsg_finalize_features,
> > +     .find_vqs       = imx_rpmsg_find_vqs,
> > +     .del_vqs        = imx_rpmsg_del_vqs,
> > +     .reset          = imx_rpmsg_reset,
> > +     .set_status     = imx_rpmsg_set_status,
> > +     .get_status     = imx_rpmsg_get_status,
> > +};
> > +
> > +static const struct of_device_id imx_rpmsg_dt_ids[] = {
> > +     { .compatible = "fsl,imx8mq-rpmsg", .data = (void *)IMX8MQ, },
> > +     { .compatible = "fsl,imx8qxp-rpmsg", .data = (void *)IMX8QXP, },
> > +     { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx_rpmsg_dt_ids);
> > +
> > +static int set_vring_phy_buf(struct platform_device *pdev,
> > +                    struct imx_rpmsg_vproc *rpdev, int vdev_nums) {
> > +     struct resource *res;
> > +     resource_size_t size;
> > +     unsigned int start, end;
> > +     int i, ret = 0;
> > +
> > +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +     if (res) {
> > +             size = resource_size(res);
> > +             start = res->start;
> > +             end = res->start + size;
> > +             for (i = 0; i < vdev_nums; i++) {
> > +                     rpdev->ivdev[i] = kzalloc(sizeof(struct
> imx_virdev),
> > +
> GFP_KERNEL);
> > +                     if (!rpdev->ivdev[i])
> > +                             return -ENOMEM;
> > +
> > +                     rpdev->ivdev[i]->vring[0] = start;
> > +                     rpdev->ivdev[i]->vring[1] = start + 0x8000;
> > +                     start += 0x10000;
> > +                     if (start > end) {
> > +                             dev_err(&pdev->dev,
> > +                                     "Too small memory
> size %x!\n",
> > +                                     (u32)size);
> > +                             ret = -EINVAL;
> > +                             break;
> > +                     }
> > +             }
> > +     } else {
> > +             return -ENOMEM;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static void rpmsg_work_handler(struct work_struct *work) {
> > +     u32 message;
> > +     unsigned long flags;
> > +     struct imx_virdev *virdev;
> > +     struct delayed_work *dwork = to_delayed_work(work);
> > +     struct imx_rpmsg_vproc *rpdev = container_of(dwork,
> > +                     struct imx_rpmsg_vproc, rpmsg_work);
> > +     struct circ_buf *cb = &rpdev->rx_buffer;
> > +     struct platform_device *pdev = rpdev->pdev;
> > +     struct device *dev = &pdev->dev;
> > +
> > +     spin_lock_irqsave(&rpdev->mu_lock, flags);
> > +     /* handle all incoming mu message */
> > +     while (CIRC_CNT(cb->head, cb->tail, PAGE_SIZE)) {
> > +             spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> > +             message = (u32) cb->buf[cb->tail];
> > +             virdev = rpdev->ivdev[(message >> 16) / 2];
> > +
> > +             dev_dbg(dev, "%s msg: 0x%x\n", __func__, message);
> > +             message = message >> 16;
> > +             message -= virdev->base_vq_id;
> > +
> > +             /*
> > +              * Currently both PENDING_MSG and
> explicit-virtqueue-index
> > +              * messaging are supported.
> > +              * Whatever approach is taken, at this point message
> contains
> > +              * the index of the vring which was just triggered.
> > +              */
> > +             if (message  < virdev->num_of_vqs)
> > +                     vring_interrupt(message, virdev->vq[message]);
> > +             spin_lock_irqsave(&rpdev->mu_lock, flags);
> > +             cb->tail = CIRC_ADD(cb->tail, PAGE_SIZE, 4);
> > +     }
> > +     spin_unlock_irqrestore(&rpdev->mu_lock, flags); }
> > +
> > +static int imx_rpmsg_partition_notify(struct notifier_block *nb,
> > +                                   unsigned long event, void
> *group)
> > +{
> > +     /* Reserved for the partition reset. */
> > +     return 0;
> > +}
> > +
> > +static void imx_rpmsg_rxdb_callback(struct mbox_client *c, void *msg)
> > +{
> > +     unsigned long flags;
> > +     struct imx_rpmsg_vproc *rpdev = container_of(c,
> > +                     struct imx_rpmsg_vproc, cl);
> > +
> > +     spin_lock_irqsave(&rpdev->mu_lock, flags);
> > +     rpdev->flags |= REMOTE_IS_READY;
> > +     spin_unlock_irqrestore(&rpdev->mu_lock, flags); }
> > +
> > +static int imx_rpmsg_rxdb_channel_init(struct imx_rpmsg_vproc *rpdev)
> > +{
> > +     struct platform_device *pdev = rpdev->pdev;
> > +     struct device *dev = &pdev->dev;
> > +     struct mbox_client *cl;
> > +     int ret = 0;
> > +
> > +     cl = &rpdev->cl_rxdb;
> > +     cl->dev = dev;
> > +     cl->rx_callback = imx_rpmsg_rxdb_callback;
> > +
> > +     /*
> > +      * RX door bell is used to receive the ready signal from remote
> > +      * after the partition reset of A core.
> > +      */
> > +     rpdev->rxdb_ch = mbox_request_channel_byname(cl, "rxdb");
> > +     if (IS_ERR(rpdev->rxdb_ch)) {
> > +             ret = PTR_ERR(rpdev->rxdb_ch);
> > +             dev_err(cl->dev, "failed to request mbox chan rxdb,
> ret %d\n",
> > +                     ret);
> > +             return ret;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static void imx_rpmsg_rx_callback(struct mbox_client *c, void *msg) {
> > +     int buf_space;
> > +     unsigned long flags;
> > +     u32 *data = msg;
> > +     struct imx_rpmsg_vproc *rpdev = container_of(c,
> > +                     struct imx_rpmsg_vproc, cl);
> > +     struct circ_buf *cb = &rpdev->rx_buffer;
> > +
> > +     spin_lock_irqsave(&rpdev->mu_lock, flags);
> > +     buf_space = CIRC_SPACE(cb->head, cb->tail, PAGE_SIZE);
> > +     spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> > +     if (unlikely(!buf_space)) {
> > +             dev_err(c->dev, "RPMSG RX overflow!\n");
> > +             return;
> > +     }
> > +     spin_lock_irqsave(&rpdev->mu_lock, flags);
> > +     cb->buf[cb->head] = *data;
> > +     cb->head = CIRC_ADD(cb->head, PAGE_SIZE, 4);
> > +     spin_unlock_irqrestore(&rpdev->mu_lock, flags);
> > +
> > +     schedule_delayed_work(&(rpdev->rpmsg_work), 0); }
> > +
> > +static int imx_rpmsg_probe(struct platform_device *pdev) {
> > +     int j, ret = 0;
> > +     char *buf;
> > +     struct device *dev = &pdev->dev;
> > +     struct device_node *np = pdev->dev.of_node;
> > +     struct imx_rpmsg_vproc *rpdev;
> > +     struct mbox_client *cl;
> > +
> > +     buf = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL);
> > +     if (!buf)
> > +             return -ENOMEM;
> > +
> > +     rpdev = devm_kzalloc(dev, sizeof(*rpdev), GFP_KERNEL);
> > +     if (!rpdev)
> > +             return -ENOMEM;
> > +
> > +     rpdev->proc_nb.notifier_call = imx_rpmsg_partition_notify;
> > +     rpdev->variant = (enum
> imx_rpmsg_variants)of_device_get_match_data(dev);
> > +     rpdev->rx_buffer.buf = buf;
> > +     rpdev->rx_buffer.head = 0;
> > +     rpdev->rx_buffer.tail = 0;
> > +
> > +     cl = &rpdev->cl;
> > +     cl->dev = dev;
> > +     cl->tx_block = false;
> > +     cl->tx_tout = 20;
> > +     cl->knows_txdone = false;
> > +     cl->rx_callback = imx_rpmsg_rx_callback;
> > +
> > +     rpdev->tx_ch = mbox_request_channel_byname(cl, "tx");
> > +     if (IS_ERR(rpdev->tx_ch)) {
> > +             ret = PTR_ERR(rpdev->tx_ch);
> > +             goto err_chl;
> > +     }
> > +     rpdev->rx_ch = mbox_request_channel_byname(cl, "rx");
> > +     if (IS_ERR(rpdev->rx_ch)) {
> > +             ret = PTR_ERR(rpdev->rx_ch);
> > +             goto err_chl;
> > +     }
> > +
> > +     spin_lock_init(&rpdev->mu_lock);
> > +     INIT_DELAYED_WORK(&(rpdev->rpmsg_work),
> rpmsg_work_handler);
> > +     ret = of_property_read_u32(np, "vdev-nums", &rpdev->vdev_nums);
> > +     if (ret)
> > +             rpdev->vdev_nums = 1;
> > +     if (rpdev->vdev_nums > MAX_VDEV_NUMS) {
> > +             dev_err(dev, "vdev-nums exceed the max %d\n",
> MAX_VDEV_NUMS);
> > +             ret = -EINVAL;
> > +             goto err_chl;
> > +     }
> > +     rpdev->first_notify = rpdev->vdev_nums;
> > +
> > +     ret = set_vring_phy_buf(pdev, rpdev, rpdev->vdev_nums);
> > +     if (ret) {
> > +             dev_err(dev, "No vring buffer.\n");
> > +             ret = -ENOMEM;
> > +             goto err_chl;
> > +     }
> > +     if (of_reserved_mem_device_init(dev)) {
> > +             dev_dbg(dev, "dev doesn't have specific DMA pool.\n");
> > +             rpdev->flags &= (~SPECIFIC_DMA_POOL);
> > +     } else {
> > +             rpdev->flags |= SPECIFIC_DMA_POOL;
> > +     }
> > +
> > +     for (j = 0; j < rpdev->vdev_nums; j++) {
> > +             dev_dbg(dev, "%s rpdev vdev%d: vring0 0x%x, vring1
> 0x%x\n",
> > +                      __func__, rpdev->vdev_nums,
> > +                      rpdev->ivdev[j]->vring[0],
> > +                      rpdev->ivdev[j]->vring[1]);
> > +             rpdev->ivdev[j]->vdev.id.device = VIRTIO_ID_RPMSG;
> > +             rpdev->ivdev[j]->vdev.config = &imx_rpmsg_config_ops;
> > +             rpdev->pdev = pdev;
> > +             rpdev->ivdev[j]->vdev.dev.parent = &pdev->dev;
> > +             rpdev->ivdev[j]->vdev.dev.release =
> imx_rpmsg_vproc_release;
> > +             rpdev->ivdev[j]->base_vq_id = j * 2;
> > +             rpdev->ivdev[j]->rpdev = rpdev;
> > +
> > +             ret = register_virtio_device(&rpdev->ivdev[j]->vdev);
> > +             if (ret) {
> > +                     dev_err(dev, "%s failed to register rpdev: %d\n",
> > +                                     __func__, ret);
> > +                     goto err_out;
> > +             }
> > +     }
> > +     /* Initialize the RX doorbell channel. */
> > +     ret = imx_rpmsg_rxdb_channel_init(rpdev);
> > +     if (ret)
> > +             goto err_out;
> > +
> > +     return ret;
> > +
> > +err_out:
> > +     if (rpdev->flags & SPECIFIC_DMA_POOL)
> > +             of_reserved_mem_device_release(dev);
> > +err_chl:
> > +     if (!IS_ERR(rpdev->rxdb_ch))
> > +             mbox_free_channel(rpdev->rxdb_ch);
> > +     if (!IS_ERR(rpdev->tx_ch))
> > +             mbox_free_channel(rpdev->tx_ch);
> > +     if (!IS_ERR(rpdev->rx_ch))
> > +             mbox_free_channel(rpdev->rx_ch);
> > +     return ret;
> > +}
> > +
> > +static struct platform_driver imx_rpmsg_driver = {
> > +     .driver = {
> > +                .owner = THIS_MODULE,
> > +                .name = "imx-rpmsg",
> > +                .of_match_table = imx_rpmsg_dt_ids,
> > +                },
> > +     .probe = imx_rpmsg_probe,
> > +};
> > +
> > +static int __init imx_rpmsg_init(void) {
> > +     int ret;
> > +
> > +     ret = platform_driver_register(&imx_rpmsg_driver);
> > +     if (ret)
> > +             pr_err("Unable to initialize rpmsg driver\n");
> > +     else
> > +             pr_info("imx rpmsg driver is registered.\n");
> > +
> > +     return ret;
> > +}
> > +
> > +MODULE_DESCRIPTION("iMX remote processor messaging virtio device");
> > +MODULE_LICENSE("GPL v2"); arch_initcall(imx_rpmsg_init);
> > diff --git a/drivers/rpmsg/imx_rpmsg_pingpong.c
> > b/drivers/rpmsg/imx_rpmsg_pingpong.c
> > new file mode 100644
> > index 0000000..b028914
> > --- /dev/null
> > +++ b/drivers/rpmsg/imx_rpmsg_pingpong.c
> > @@ -0,0 +1,100 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2019 NXP
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/virtio.h>
> > +#include <linux/rpmsg.h>
> > +
> > +#define MSG          "hello world!"
> > +static unsigned int rpmsg_pingpong;
> > +
> > +static int rpmsg_pingpong_cb(struct rpmsg_device *rpdev, void *data, int
> len,
> > +                                             void *priv, u32 src)
> {
> > +     int err;
> > +
> > +     /* reply */
> > +     rpmsg_pingpong = *(unsigned int *)data;
> > +     pr_info("get %d (src: 0x%x)\n", rpmsg_pingpong, src);
> > +
> > +     /* pingpongs should not live forever */
> > +     if (rpmsg_pingpong > 100) {
> > +             dev_info(&rpdev->dev, "goodbye!\n");
> > +             return 0;
> > +     }
> > +     rpmsg_pingpong++;
> > +     err = rpmsg_sendto(rpdev->ept, (void *)(&rpmsg_pingpong), 4,
> > + src);
> > +
> > +     if (err)
> > +             dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", err);
> > +
> > +     return err;
> > +}
> > +
> > +static int rpmsg_pingpong_probe(struct rpmsg_device *rpdev) {
> > +     int err;
> > +
> > +     dev_info(&rpdev->dev, "new channel: 0x%x -> 0x%x!\n",
> > +                     rpdev->src, rpdev->dst);
> > +
> > +     /*
> > +      * send a message to our remote processor, and tell remote
> > +      * processor about this channel
> > +      */
> > +     err = rpmsg_send(rpdev->ept, MSG, strlen(MSG));
> > +     if (err) {
> > +             dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", err);
> > +             return err;
> > +     }
> > +
> > +     rpmsg_pingpong = 0;
> > +     err = rpmsg_sendto(rpdev->ept, (void *)(&rpmsg_pingpong),
> > +                        4, rpdev->dst);
> > +     if (err) {
> > +             dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", err);
> > +             return err;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static void rpmsg_pingpong_remove(struct rpmsg_device *rpdev) {
> > +     dev_info(&rpdev->dev, "rpmsg pingpong driver is removed\n"); }
> > +
> > +static struct rpmsg_device_id rpmsg_driver_pingpong_id_table[] = {
> > +     { .name = "rpmsg-openamp-demo-channel" },
> > +     { .name = "rpmsg-openamp-demo-channel-1" },
> > +     { },
> > +};
> > +MODULE_DEVICE_TABLE(rpmsg, rpmsg_driver_pingpong_id_table);
> > +
> > +static struct rpmsg_driver rpmsg_pingpong_driver = {
> > +     .drv.name       = KBUILD_MODNAME,
> > +     .drv.owner      = THIS_MODULE,
> > +     .id_table       = rpmsg_driver_pingpong_id_table,
> > +     .probe          = rpmsg_pingpong_probe,
> > +     .callback       = rpmsg_pingpong_cb,
> > +     .remove         = rpmsg_pingpong_remove,
> > +};
> > +
> > +static int __init init(void)
> > +{
> > +     return register_rpmsg_driver(&rpmsg_pingpong_driver);
> > +}
> > +
> > +static void __exit fini(void)
> > +{
> > +     unregister_rpmsg_driver(&rpmsg_pingpong_driver);
> > +}
> > +module_init(init);
> > +module_exit(fini);
> > +
> > +MODULE_AUTHOR("Freescale Semiconductor, Inc.");
> > +MODULE_DESCRIPTION("iMX virtio remote processor messaging pingpong
> > +driver"); MODULE_LICENSE("GPL v2");
> > diff --git a/include/linux/imx_rpmsg.h b/include/linux/imx_rpmsg.h new
> > file mode 100644 index 0000000..e0d5e97
> > --- /dev/null
> > +++ b/include/linux/imx_rpmsg.h
> > @@ -0,0 +1,43 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2019 NXP.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +/*
> > + * @file linux/imx_rpmsg.h
> > + *
> > + * @brief Global header file for iMX RPMSG
> > + *
> > + * @ingroup RPMSG
> > + */
> > +#ifndef __LINUX_IMX_RPMSG_H__
> > +#define __LINUX_IMX_RPMSG_H__
> > +
> > +/* Category define */
> > +#define IMX_RMPSG_LIFECYCLE  1
> > +#define IMX_RPMSG_PMIC               2
> > +#define IMX_RPMSG_AUDIO              3
> > +#define IMX_RPMSG_KEY                4
> > +#define IMX_RPMSG_GPIO               5
> > +#define IMX_RPMSG_RTC                6
> > +#define IMX_RPMSG_SENSOR     7
> > +/* rpmsg version */
> > +#define IMX_RMPSG_MAJOR              1
> > +#define IMX_RMPSG_MINOR              0
> > +
> > +#define CIRC_ADD(idx, size, value)   (((idx) + (value)) & ((size) - 1))
> > +
> > +struct imx_rpmsg_head {
> > +     u8 cate;
> > +     u8 major;
> > +     u8 minor;
> > +     u8 type;
> > +     u8 cmd;
> > +     u8 reserved[5];
> > +} __packed;
> > +
> > +#endif /* __LINUX_IMX_RPMSG_H__ */
> >
> 
> Kind regards,
> Oleksij Rempel
> 
> --
> Pengutronix e.K.                           |
> |
> Industrial Linux Solutions                 |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.p
> engutronix.de%2F&amp;data=02%7C01%7Chongxing.zhu%40nxp.com%7Cdd
> 44732d8d084ca7077b08d7006370cc%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636978299412610922&amp;sdata=o1ISCctoOdKtD8KYNLV
> zb9DPMWcmQq0XRXd9aKQCYzc%3D&amp;reserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |

Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Oleksij Rempel]

Hi Richard,

On 08.07.19 12:17, Richard Zhu wrote:
> Hi Oleksij:
> Thanks for your comments.
> 
> 
>> -----Original Message-----
>> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
>> Sent: 2019年7月4日 17:36
>> To: Richard Zhu <hongxing.zhu@nxp.com>; ohad@wizery.com;
>> bjorn.andersson@linaro.org; linux-remoteproc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org; Fabien DESSENNE
>> <fabien.dessenne@st.com>; loic.pallardy@st.com; arnaud.pouliquen@st.com;
>> s-anna@ti.com; elder@linaro.org
>> Subject: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support
>>
>> Caution: EXT Email
>>
>> Hi Richard,
>>
>> On 01.07.19 10:34, Richard Zhu wrote:
>>> Based on "virtio_rpmsg_bus" driver, This patch-set is used to set up
>>> the communication mechanism between A core and M core on i.MX AMP
>> SOCs.
>>>
>>> Add the initial imx rpmsg support glue driver and one pingpong demo,
>>> demonstrated the data transactions between A core and remote M core.
>>> Distributed framework is used in IMX RPMSG implementation, refer to
>>> the following requirements:
>>>     - The CAN functions contained in M core and RTOS should be ready and
>>>       complete functional in 50ms after AMP system is turned on.
>>>     - Partition reset. System wouldn't be stalled by the exceptions (e.x
>>>       the reset triggered by the system hang) occurred at the other side.
>>>       And the RPMSG mechanism should be recovered automactilly after
>> the
>>>       partition reset is completed.
>>> In this scenario, the M core and RTOS would be kicked off by
>>> bootloader firstly, then A core and Linux would be loaded later. Both
>>> M core/RTOS and A core/Linux are running independly.
>>>
>>> One physical memory region used to store the vring is mandatory
>>> required to pre-reserved and well-knowned by both A core and M core
>>
>> I don't see any thing imx specific in this patch. We already have remoteproc
>> which would parse firmware header and create needed devices. This driver is
>> only needed for the case where firmware was stared by the bootloader.
>>
> [Richard Zhu] Bootloader starts the firmware is mandatory required in these scenario
> refer to the reasons listed in the commit.
> Thus, the distributed framework has to be used, and both A core/Linux and remote core/RTOS
> works independently.
> 
>> I personally would prefer to have generic driver or extend the remoteproc
>> framework. So we can notify kernel about work already done by bootloader.
>>
> [Richard Zhu] Thanks for your suggestions.
> Regarding to my understand, it seems that master/slave mode is used in the remoteproc currently.
> A core/Linux acts as master, to controls/manipulates remote core/RTOS.
> It isn't applicable for the scenario described by this patch-set.
> 
>> In general, some more issues should be solved:
>> - Handle or not touch idle clocks for different node used by M core and not
>> main system.
>> - pin control
>> - regulators
>>
>> ST devs already tried to solve this issues by creating "remoteproc: add system
>> resource manager device" patch. I don't know what is current state of it (/me
>> adding ST devs to CC).
>>
> [Richard Zhu] Yes, it is. Many contributions have been made by Fabien.
> IMHO, there are some different behaviors on iMX8QXP/QM platforms, the
>   resources (e.x IP modules) had been assigned and managed by the XRDC.
> In the other words, the HW resources would be assigned and managed would
>   be transparent to SW.
> 
> Thus, both A core/Linux and M core/RTOS can work real independently.
> System wouldn't be stalled by the exceptions (e.x the reset triggered by the
> system hang) occurred at the other side. And the RPMSG mechanism should
>   be recovered automatically after the partition reset is completed.

It is exactly the way I did understood it in the firs mail. Any way, i'm ok
with this driver. Just rename imx to some thing generic. This driver can and will be 
reused on other platforms as well.

Kind regards,
Oleksij Rempel

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Arnaud Pouliquen]

Hello Richard,

On 7/8/19 1:02 PM, Oleksij Rempel wrote:
> Hi Richard,
> 
> On 08.07.19 12:17, Richard Zhu wrote:
>> Hi Oleksij:
>> Thanks for your comments.
>>
>>
>>> -----Original Message-----
>>> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
>>> Sent: 2019年7月4日 17:36
>>> To: Richard Zhu <hongxing.zhu@nxp.com>; ohad@wizery.com;
>>> bjorn.andersson@linaro.org; linux-remoteproc@vger.kernel.org
>>> Cc: linux-arm-kernel@lists.infradead.org; Fabien DESSENNE
>>> <fabien.dessenne@st.com>; loic.pallardy@st.com; arnaud.pouliquen@st.com;
>>> s-anna@ti.com; elder@linaro.org
>>> Subject: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg
>>> support
>>>
>>> Caution: EXT Email
>>>
>>> Hi Richard,
>>>
>>> On 01.07.19 10:34, Richard Zhu wrote:
>>>> Based on "virtio_rpmsg_bus" driver, This patch-set is used to set up
>>>> the communication mechanism between A core and M core on i.MX AMP
>>> SOCs.
>>>>
>>>> Add the initial imx rpmsg support glue driver and one pingpong demo,
>>>> demonstrated the data transactions between A core and remote M core.
>>>> Distributed framework is used in IMX RPMSG implementation, refer to
>>>> the following requirements:
>>>>     - The CAN functions contained in M core and RTOS should be ready
>>>> and
>>>>       complete functional in 50ms after AMP system is turned on.
>>>>     - Partition reset. System wouldn't be stalled by the exceptions
>>>> (e.x
>>>>       the reset triggered by the system hang) occurred at the other
>>>> side.
>>>>       And the RPMSG mechanism should be recovered automactilly after
>>> the
>>>>       partition reset is completed.
>>>> In this scenario, the M core and RTOS would be kicked off by
>>>> bootloader firstly, then A core and Linux would be loaded later. Both
>>>> M core/RTOS and A core/Linux are running independly.
>>>>
>>>> One physical memory region used to store the vring is mandatory
>>>> required to pre-reserved and well-knowned by both A core and M core
>>>
>>> I don't see any thing imx specific in this patch. We already have
>>> remoteproc
>>> which would parse firmware header and create needed devices. This
>>> driver is
>>> only needed for the case where firmware was stared by the bootloader.
>>>
>> [Richard Zhu] Bootloader starts the firmware is mandatory required in
>> these scenario
>> refer to the reasons listed in the commit.
>> Thus, the distributed framework has to be used, and both A core/Linux
>> and remote core/RTOS
>> works independently.
>>
>>> I personally would prefer to have generic driver or extend the
>>> remoteproc
>>> framework. So we can notify kernel about work already done by
>>> bootloader.
>>>
>> [Richard Zhu] Thanks for your suggestions.
>> Regarding to my understand, it seems that master/slave mode is used in
>> the remoteproc currently.
>> A core/Linux acts as master, to controls/manipulates remote core/RTOS.
>> It isn't applicable for the scenario described by this patch-set.
>>
>>> In general, some more issues should be solved:
>>> - Handle or not touch idle clocks for different node used by M core
>>> and not
>>> main system.
>>> - pin control
>>> - regulators
>>>
>>> ST devs already tried to solve this issues by creating "remoteproc:
>>> add system
>>> resource manager device" patch. I don't know what is current state of
>>> it (/me
>>> adding ST devs to CC).
The resource manager implementation as been proposed but no real
adhesion of the community on it... Perhaps SCMI should be a candidate...

>>>
>> [Richard Zhu] Yes, it is. Many contributions have been made by Fabien.
>> IMHO, there are some different behaviors on iMX8QXP/QM platforms, the
>>   resources (e.x IP modules) had been assigned and managed by the XRDC.
>> In the other words, the HW resources would be assigned and managed would
>>   be transparent to SW.
>>
>> Thus, both A core/Linux and M core/RTOS can work real independently.
>> System wouldn't be stalled by the exceptions (e.x the reset triggered
>> by the
>> system hang) occurred at the other side. And the RPMSG mechanism should
>>   be recovered automatically after the partition reset is completed.
> 
> It is exactly the way I did understood it in the firs mail. Any way, i'm ok
> with this driver. Just rename imx to some thing generic. This driver can
> and will be reused on other platforms as well.
> 
> Kind regards,
> Oleksij Rempel
> 

I'm trying to figure out what is the interest of these drivers vs
existing ones.
Please find below a list of features i noticed in your driver (don't
hesitate if i missed some of them), with some comments/questions.

1) The coprocessor is started before the one running Linux OS.
Have you taken a look to this set of patches proposed by Loic:
https://lkml.org/lkml/2018/11/30/157
with this patch you should be able to"attach" on the fly on a preloaded
firmware.

2) RPMSG recovery
Agree with you, this feature is important in AMP systems, as cores can
have their own live cycle.

But I can not see related code, could you point out it to me?

Could you explain How do you recover the rpmsg channels that has been
already established?
For instance what happen if your coprocessor crash during the rpmsg
pingpong demo?

3) ping-pong demo sample
Perhaps you could re-use the rpmsg sample available here:
https://elixir.bootlin.com/linux/v5.2/source/samples/rpmsg

4) No use of the resource table
Is there a reason to not use the resource table to declare the the
vrings? Your implementation seems to impose the same definition in both
firmware while resource table allow to share them.
Furthermore the resource table could be updated by the Linux before the
remote proc is started (in case of Linux booting first)

5) slave and master mode support.
Seems that this drivers not fully respect the virtio protocol (for
instance status field). If you use a synchro mechanism (mailbox...) not
sure that you really need to be virtio slave on Linux.

Thanks,
Arnaud

RE: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Richard Zhu]

Hi Arnaud:
Thanks a lot for your kindly guidance and review comments.


> -----Original Message-----
> From: Arnaud Pouliquen [mailto:arnaud.pouliquen@st.com]
> Sent: 2019年7月8日 22:12
> To: Oleksij Rempel <o.rempel@pengutronix.de>; Richard Zhu
> <hongxing.zhu@nxp.com>; ohad@wizery.com; bjorn.andersson@linaro.org;
> linux-remoteproc@vger.kernel.org
> Cc: loic.pallardy@st.com; Fabien DESSENNE <fabien.dessenne@st.com>;
> elder@linaro.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support
> 
> 
> Hello Richard,
> 
> On 7/8/19 1:02 PM, Oleksij Rempel wrote:
> > Hi Richard,
> >
> > On 08.07.19 12:17, Richard Zhu wrote:
> >> Hi Oleksij:
> >> Thanks for your comments.
> >>
> >>
> >>> -----Original Message-----
> >>> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
> >>> Sent: 2019年7月4日 17:36
> >>> To: Richard Zhu <hongxing.zhu@nxp.com>; ohad@wizery.com;
> >>> bjorn.andersson@linaro.org; linux-remoteproc@vger.kernel.org
> >>> Cc: linux-arm-kernel@lists.infradead.org; Fabien DESSENNE
> >>> <fabien.dessenne@st.com>; loic.pallardy@st.com;
> >>> arnaud.pouliquen@st.com; s-anna@ti.com; elder@linaro.org
> >>> Subject: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg
> >>> support
> >>>
> >>>
> >>> Hi Richard,
> >>>
> >>> On 01.07.19 10:34, Richard Zhu wrote:
> >>>> Based on "virtio_rpmsg_bus" driver, This patch-set is used to set
> >>>> up the communication mechanism between A core and M core on i.MX
> >>>> AMP
> >>> SOCs.
> >>>>
> >>>> Add the initial imx rpmsg support glue driver and one pingpong
> >>>> demo, demonstrated the data transactions between A core and remote
> M core.
> >>>> Distributed framework is used in IMX RPMSG implementation, refer to
> >>>> the following requirements:
> >>>>     - The CAN functions contained in M core and RTOS should be
> >>>> ready and
> >>>>       complete functional in 50ms after AMP system is turned on.
> >>>>     - Partition reset. System wouldn't be stalled by the exceptions
> >>>> (e.x
> >>>>       the reset triggered by the system hang) occurred at the other
> >>>> side.
> >>>>       And the RPMSG mechanism should be recovered automactilly
> >>>> after
> >>> the
> >>>>       partition reset is completed.
> >>>> In this scenario, the M core and RTOS would be kicked off by
> >>>> bootloader firstly, then A core and Linux would be loaded later.
> >>>> Both M core/RTOS and A core/Linux are running independly.
> >>>>
> >>>> One physical memory region used to store the vring is mandatory
> >>>> required to pre-reserved and well-knowned by both A core and M core
> >>>
> >>> I don't see any thing imx specific in this patch. We already have
> >>> remoteproc which would parse firmware header and create needed
> >>> devices. This driver is only needed for the case where firmware was
> >>> stared by the bootloader.
> >>>
> >> [Richard Zhu] Bootloader starts the firmware is mandatory required in
> >> these scenario refer to the reasons listed in the commit.
> >> Thus, the distributed framework has to be used, and both A core/Linux
> >> and remote core/RTOS works independently.
> >>
> >>> I personally would prefer to have generic driver or extend the
> >>> remoteproc framework. So we can notify kernel about work already
> >>> done by bootloader.
> >>>
> >> [Richard Zhu] Thanks for your suggestions.
> >> Regarding to my understand, it seems that master/slave mode is used
> >> in the remoteproc currently.
> >> A core/Linux acts as master, to controls/manipulates remote core/RTOS.
> >> It isn't applicable for the scenario described by this patch-set.
> >>
> >>> In general, some more issues should be solved:
> >>> - Handle or not touch idle clocks for different node used by M core
> >>> and not main system.
> >>> - pin control
> >>> - regulators
> >>>
> >>> ST devs already tried to solve this issues by creating "remoteproc:
> >>> add system
> >>> resource manager device" patch. I don't know what is current state
> >>> of it (/me adding ST devs to CC).
> The resource manager implementation as been proposed but no real
> adhesion of the community on it... Perhaps SCMI should be a candidate...
> 
> >>>
> >> [Richard Zhu] Yes, it is. Many contributions have been made by Fabien.
> >> IMHO, there are some different behaviors on iMX8QXP/QM platforms, the
> >>   resources (e.x IP modules) had been assigned and managed by the
> XRDC.
> >> In the other words, the HW resources would be assigned and managed
> would
> >>   be transparent to SW.
> >>
> >> Thus, both A core/Linux and M core/RTOS can work real independently.
> >> System wouldn't be stalled by the exceptions (e.x the reset triggered
> >> by the system hang) occurred at the other side. And the RPMSG
> >> mechanism should
> >>   be recovered automatically after the partition reset is completed.
> >
> > It is exactly the way I did understood it in the firs mail. Any way,
> > i'm ok with this driver. Just rename imx to some thing generic. This
> > driver can and will be reused on other platforms as well.
> >
> > Kind regards,
> > Oleksij Rempel
> >
> 
> I'm trying to figure out what is the interest of these drivers vs existing ones.
> Please find below a list of features i noticed in your driver (don't hesitate if i
> missed some of them), with some comments/questions.
> 
> 1) The coprocessor is started before the one running Linux OS.
> Have you taken a look to this set of patches proposed by Loic:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.or
> g%2Flkml%2F2018%2F11%2F30%2F157&amp;data=02%7C01%7Chongxing.z
> hu%40nxp.com%7C6773999ac6394dfe37d008d703ae3475%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C636981919064186660&amp;sdata=
> OUogIs2S7gLR46%2FNcAU3OqEtB4rK3sW0gRKRRSO6xpk%3D&amp;reserved
> =0
> with this patch you should be able to"attach" on the fly on a preloaded
> firmware.
[Richard Zhu] Yes, this patch-set enable to pre-load the firmware in bootloader.
The most difficulties when I try to use the current master/slave mode are that
 the remote-proc controls/management mode is not applicable to the scenario,
 especially the iMX8QXP/QM partition reset usage.
Both A core/Linux and M core/RTOS are working independently.
HW resources(e.x: some IP modules, DDR memory region, power domains, clocks and so on)
 would be pre-managed and pre-assigned to A core or M core by SCFW through XRDC module
refer to the security reasons or something else I don't know.

M core/RTOS insists to run and manage its resources assigned by XRDC standalone.
All the interactions between A core and M core are transferred on RPMSG channels.
For example, the audio codec configuration and so on.
So, what I do here is just setup the communication RPMSG channels between A core/Linux
and M core/RTOS.

One more concern, I'm afraid that I may mess up the current solid reproc flow and framework if
 I force this implementation into the current reproc drivers. 
So, I summit this patch-set in the end. Pre-reserved vring buffer, register virtio_device, establish
the RPMSG channels lets A core/Linux and M Core/RTOS can communicate with each other.
That's all.
> 
> 2) RPMSG recovery
> Agree with you, this feature is important in AMP systems, as cores can have
> their own live cycle.
> 
> But I can not see related code, could you point out it to me?
> 
[Richard Zhu] This feature had been validated in the local repos.
But these codes are not contained in this patch-set, because this feature is
relied on the SCFW(system control firm ware) used to monitor the status of
 both side, and trigger one irq to the other side, if one side is stall.
Unfortunately, it is not up streamed yet. So, these codes would be updated later
If the SCFW is ready.

> Could you explain How do you recover the rpmsg channels that has been
> already established?
> For instance what happen if your coprocessor crash during the rpmsg
> pingpong demo?
[Richard Zhu] SCFW would inform the other side, if one core/OS is crashed.
Then, the RPMSG stack would be re-initialized itself on the lived core/OS, and clarify that
It's ready to re-establish the channels again.
For example, M4/RTOS is crashed when pingpong demo is running.
1. Pingpong demo is stopped.
2. Lived A core/Linux would receive one irq from SCFW indicated that remote M4/RTOS is
 reset, then all the virtio_device registered in A core/Linux side, would be un-registered,
 and these virtio_devices would be registered again after receive the signal(e.x the mailbox rdb)
 that M4/RTOS RPMSG stack is ready again.
3. Thus RPMS channels can be re-established in this situation.
4. Accordingly, the consumer of the rpmsg glue driver should be re-initialized too.
For example, remove the pingpond demo module, and insmod it again.

> 
> 3) ping-pong demo sample
> Perhaps you could re-use the rpmsg sample available here:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.b
> ootlin.com%2Flinux%2Fv5.2%2Fsource%2Fsamples%2Frpmsg&amp;data=02
> %7C01%7Chongxing.zhu%40nxp.com%7C6773999ac6394dfe37d008d703ae3
> 475%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636981919064
> 186660&amp;sdata=mSV3YsoyhAO%2FROfWX79X0woGQN3jx%2Fv4pL8LRUf
> bHUM%3D&amp;reserved=0
[Richard Zhu] Thanks a lot.
This demo sample can be used. Sorry about that I didn't notice it before.

> 
> 4) No use of the resource table
> Is there a reason to not use the resource table to declare the the vrings? Your
> implementation seems to impose the same definition in both firmware while
> resource table allow to share them.
> Furthermore the resource table could be updated by the Linux before the
> remote proc is started (in case of Linux booting first)
> 
[Richard Zhu] Regarding to the auto industry requirements, the M core/RTOS is always
started firstly, because that the CAN functions should be ready in 50ms after system is
power up.
BTW, resource table is a great idea in the case when Linux is booting firstly.

> 5) slave and master mode support.
> Seems that this drivers not fully respect the virtio protocol (for instance status
> field). If you use a synchro mechanism (mailbox...) not sure that you really
> need to be virtio slave on Linux.
[Richard Zhu] Sorry about that. I used trying to keep this driver compatible with
the current slave-master mode, but I'm failed to achieve that. ☹.
- Partition reset feature is mandatory required.
- M4 side insists that they should run and manage its resources standalone.

Best Regards
Richard Zhu
> 
> Thanks,
> Arnaud

Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Arnaud Pouliquen]

On 7/9/19 9:32 AM, Richard Zhu wrote:
> Hi Arnaud:
> Thanks a lot for your kindly guidance and review comments.
> 
> 
>> -----Original Message-----
>> From: Arnaud Pouliquen [mailto:arnaud.pouliquen@st.com]
>> Sent: 2019年7月8日 22:12
>> To: Oleksij Rempel <o.rempel@pengutronix.de>; Richard Zhu
>> <hongxing.zhu@nxp.com>; ohad@wizery.com; bjorn.andersson@linaro.org;
>> linux-remoteproc@vger.kernel.org
>> Cc: loic.pallardy@st.com; Fabien DESSENNE <fabien.dessenne@st.com>;
>> elder@linaro.org; linux-arm-kernel@lists.infradead.org
>> Subject: Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support
>>
>>
>> Hello Richard,
>>
>> On 7/8/19 1:02 PM, Oleksij Rempel wrote:
>>> Hi Richard,
>>>
>>> On 08.07.19 12:17, Richard Zhu wrote:
>>>> Hi Oleksij:
>>>> Thanks for your comments.
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
>>>>> Sent: 2019年7月4日 17:36
>>>>> To: Richard Zhu <hongxing.zhu@nxp.com>; ohad@wizery.com;
>>>>> bjorn.andersson@linaro.org; linux-remoteproc@vger.kernel.org
>>>>> Cc: linux-arm-kernel@lists.infradead.org; Fabien DESSENNE
>>>>> <fabien.dessenne@st.com>; loic.pallardy@st.com;
>>>>> arnaud.pouliquen@st.com; s-anna@ti.com; elder@linaro.org
>>>>> Subject: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg
>>>>> support
>>>>>
>>>>>
>>>>> Hi Richard,
>>>>>
>>>>> On 01.07.19 10:34, Richard Zhu wrote:
>>>>>> Based on "virtio_rpmsg_bus" driver, This patch-set is used to set
>>>>>> up the communication mechanism between A core and M core on i.MX
>>>>>> AMP
>>>>> SOCs.
>>>>>>
>>>>>> Add the initial imx rpmsg support glue driver and one pingpong
>>>>>> demo, demonstrated the data transactions between A core and remote
>> M core.
>>>>>> Distributed framework is used in IMX RPMSG implementation, refer to
>>>>>> the following requirements:
>>>>>>     - The CAN functions contained in M core and RTOS should be
>>>>>> ready and
>>>>>>       complete functional in 50ms after AMP system is turned on.
>>>>>>     - Partition reset. System wouldn't be stalled by the exceptions
>>>>>> (e.x
>>>>>>       the reset triggered by the system hang) occurred at the other
>>>>>> side.
>>>>>>       And the RPMSG mechanism should be recovered automactilly
>>>>>> after
>>>>> the
>>>>>>       partition reset is completed.
>>>>>> In this scenario, the M core and RTOS would be kicked off by
>>>>>> bootloader firstly, then A core and Linux would be loaded later.
>>>>>> Both M core/RTOS and A core/Linux are running independly.
>>>>>>
>>>>>> One physical memory region used to store the vring is mandatory
>>>>>> required to pre-reserved and well-knowned by both A core and M core
>>>>>
>>>>> I don't see any thing imx specific in this patch. We already have
>>>>> remoteproc which would parse firmware header and create needed
>>>>> devices. This driver is only needed for the case where firmware was
>>>>> stared by the bootloader.
>>>>>
>>>> [Richard Zhu] Bootloader starts the firmware is mandatory required in
>>>> these scenario refer to the reasons listed in the commit.
>>>> Thus, the distributed framework has to be used, and both A core/Linux
>>>> and remote core/RTOS works independently.
>>>>
>>>>> I personally would prefer to have generic driver or extend the
>>>>> remoteproc framework. So we can notify kernel about work already
>>>>> done by bootloader.
>>>>>
>>>> [Richard Zhu] Thanks for your suggestions.
>>>> Regarding to my understand, it seems that master/slave mode is used
>>>> in the remoteproc currently.
>>>> A core/Linux acts as master, to controls/manipulates remote core/RTOS.
>>>> It isn't applicable for the scenario described by this patch-set.
>>>>
>>>>> In general, some more issues should be solved:
>>>>> - Handle or not touch idle clocks for different node used by M core
>>>>> and not main system.
>>>>> - pin control
>>>>> - regulators
>>>>>
>>>>> ST devs already tried to solve this issues by creating "remoteproc:
>>>>> add system
>>>>> resource manager device" patch. I don't know what is current state
>>>>> of it (/me adding ST devs to CC).
>> The resource manager implementation as been proposed but no real
>> adhesion of the community on it... Perhaps SCMI should be a candidate...
>>
>>>>>
>>>> [Richard Zhu] Yes, it is. Many contributions have been made by Fabien.
>>>> IMHO, there are some different behaviors on iMX8QXP/QM platforms, the
>>>>   resources (e.x IP modules) had been assigned and managed by the
>> XRDC.
>>>> In the other words, the HW resources would be assigned and managed
>> would
>>>>   be transparent to SW.
>>>>
>>>> Thus, both A core/Linux and M core/RTOS can work real independently.
>>>> System wouldn't be stalled by the exceptions (e.x the reset triggered
>>>> by the system hang) occurred at the other side. And the RPMSG
>>>> mechanism should
>>>>   be recovered automatically after the partition reset is completed.
>>>
>>> It is exactly the way I did understood it in the firs mail. Any way,
>>> i'm ok with this driver. Just rename imx to some thing generic. This
>>> driver can and will be reused on other platforms as well.
>>>
>>> Kind regards,
>>> Oleksij Rempel
>>>
>>
>> I'm trying to figure out what is the interest of these drivers vs existing ones.
>> Please find below a list of features i noticed in your driver (don't hesitate if i
>> missed some of them), with some comments/questions.
>>
>> 1) The coprocessor is started before the one running Linux OS.
>> Have you taken a look to this set of patches proposed by Loic:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.or
>> g%2Flkml%2F2018%2F11%2F30%2F157&amp;data=02%7C01%7Chongxing.z
>> hu%40nxp.com%7C6773999ac6394dfe37d008d703ae3475%7C686ea1d3bc2
>> b4c6fa92cd99c5c301635%7C0%7C0%7C636981919064186660&amp;sdata=
>> OUogIs2S7gLR46%2FNcAU3OqEtB4rK3sW0gRKRRSO6xpk%3D&amp;reserved
>> =0
>> with this patch you should be able to"attach" on the fly on a preloaded
>> firmware.
> [Richard Zhu] Yes, this patch-set enable to pre-load the firmware in bootloader.
> The most difficulties when I try to use the current master/slave mode are that
>  the remote-proc controls/management mode is not applicable to the scenario,
>  especially the iMX8QXP/QM partition reset usage.
> Both A core/Linux and M core/RTOS are working independently.
> HW resources(e.x: some IP modules, DDR memory region, power domains, clocks and so on)
>  would be pre-managed and pre-assigned to A core or M core by SCFW through XRDC module
> refer to the security reasons or something else I don't know.
If i well understand the xRDC is an IP which allows hardware isolation
of some resources to the A or M core. So it is set by the secure part of
the bootloader, right?
We have an equivalence on the STM32MP1 named ETZPC.
So we also manage this use case on stm32mp1. The bootloader configures
resource isolation, and can load and start the Cortex-M firmware, before
the linux firmware. That why i pointed this patch. In case of preloaded
firmware the remote proc does not load the firmware but just parse the
resource table (address needs to be provided by the rproc_platform
driver). The rpmsg bus is probed according to the resource table entries.
This part of code is not upstreamed for time being (waiting integration
of the mentioned patch). Nevertheless You can take a look on mechanism
we implemented, on ST github (we named it early_boot):
https://github.com/STMicroelectronics/linux/blob/v4.19-stm32mp/drivers/remoteproc/stm32_rproc.c
> 
> M core/RTOS insists to run and manage its resources assigned by XRDC standalone.
> All the interactions between A core and M core are transferred on RPMSG channels.
> For example, the audio codec configuration and so on.
> So, what I do here is just setup the communication RPMSG channels between A core/Linux
> and M core/RTOS.
> 
> One more concern, I'm afraid that I may mess up the current solid reproc flow and framework if
>  I force this implementation into the current reproc drivers. 
> So, I summit this patch-set in the end. Pre-reserved vring buffer, register virtio_device, establish
> the RPMSG channels lets A core/Linux and M Core/RTOS can communicate with each other.
> That's all.
Your concern is valid, and as we have the same requirement, it would be
nice to find a common solution. That's why i propose this alternative,
which would have the advantage of reusing existing rpmsg implementation.

>>
>> 2) RPMSG recovery
>> Agree with you, this feature is important in AMP systems, as cores can have
>> their own live cycle.
>>
>> But I can not see related code, could you point out it to me?
>>
> [Richard Zhu] This feature had been validated in the local repos.
> But these codes are not contained in this patch-set, because this feature is
> relied on the SCFW(system control firm ware) used to monitor the status of
>  both side, and trigger one irq to the other side, if one side is stall.
> Unfortunately, it is not up streamed yet. So, these codes would be updated later
> If the SCFW is ready.
> 
>> Could you explain How do you recover the rpmsg channels that has been
>> already established?
>> For instance what happen if your coprocessor crash during the rpmsg
>> pingpong demo?
> [Richard Zhu] SCFW would inform the other side, if one core/OS is crashed.
> Then, the RPMSG stack would be re-initialized itself on the lived core/OS, and clarify that
> It's ready to re-establish the channels again.
> For example, M4/RTOS is crashed when pingpong demo is running.
> 1. Pingpong demo is stopped.
> 2. Lived A core/Linux would receive one irq from SCFW indicated that remote M4/RTOS is
>  reset, then all the virtio_device registered in A core/Linux side, would be un-registered,
>  and these virtio_devices would be registered again after receive the signal(e.x the mailbox rdb)
>  that M4/RTOS RPMSG stack is ready again.
> 3. Thus RPMS channels can be re-established in this situation.
> 4. Accordingly, the consumer of the rpmsg glue driver should be re-initialized too.
> For example, remove the pingpond demo module, and insmod it again.

Thanks for the clarification, i think this is no so far from the
recovery already implemented in remoteproc. Seems you remote proc driver
handles the recovery:
 -stop rproc on irq reception, restart it ( in preloaded mode) on
mailbox rdb.
On stm32MP1 we have a similar mechanism based on a Watchdog.

> 
>>
>> 3) ping-pong demo sample
>> Perhaps you could re-use the rpmsg sample available here:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.b
>> ootlin.com%2Flinux%2Fv5.2%2Fsource%2Fsamples%2Frpmsg&amp;data=02
>> %7C01%7Chongxing.zhu%40nxp.com%7C6773999ac6394dfe37d008d703ae3
>> 475%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636981919064
>> 186660&amp;sdata=mSV3YsoyhAO%2FROfWX79X0woGQN3jx%2Fv4pL8LRUf
>> bHUM%3D&amp;reserved=0
> [Richard Zhu] Thanks a lot.
> This demo sample can be used. Sorry about that I didn't notice it before.
> 
>>
>> 4) No use of the resource table
>> Is there a reason to not use the resource table to declare the the vrings? Your
>> implementation seems to impose the same definition in both firmware while
>> resource table allow to share them.
>> Furthermore the resource table could be updated by the Linux before the
>> remote proc is started (in case of Linux booting first)
>>
> [Richard Zhu] Regarding to the auto industry requirements, the M core/RTOS is always
> started firstly, because that the CAN functions should be ready in 50ms after system is
> power up.
> BTW, resource table is a great idea in the case when Linux is booting firstly.
As explained before We also use it when cortex-M4 is booted firstly. A
constraint is that the resource table address should be known by the
remoteproc driver: either the resource table address is defined in DT,
or provided by the bootloader which loads the firmware so parses it.

> 
>> 5) slave and master mode support.
>> Seems that this drivers not fully respect the virtio protocol (for instance status
>> field). If you use a synchro mechanism (mailbox...) not sure that you really
>> need to be virtio slave on Linux.
> [Richard Zhu] Sorry about that. I used trying to keep this driver compatible with
> the current slave-master mode, but I'm failed to achieve that. ☹.
> - Partition reset feature is mandatory required.
> - M4 side insists that they should run and manage its resources standalone.
No problem, it is an RFC.
Anyway regarding you requirements and concerns, it seems that we have
the same ones. I don't know if the solution we propose can fit with your
needs, but i would be nice to have a common implementation.

Best Regards,
Arnaud

> 
> Best Regards
> Richard Zhu
>>
>> Thanks,
>> Arnaud

RE: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Richard Zhu]

> -----Original Message-----
> From: Arnaud Pouliquen [mailto:arnaud.pouliquen@st.com]
> Sent: 2019年7月9日 17:57
> To: Richard Zhu <hongxing.zhu@nxp.com>; Oleksij Rempel
> <o.rempel@pengutronix.de>; ohad@wizery.com; bjorn.andersson@linaro.org;
> linux-remoteproc@vger.kernel.org
> Cc: loic.pallardy@st.com; Fabien DESSENNE <fabien.dessenne@st.com>;
> elder@linaro.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support
> 
> On 7/9/19 9:32 AM, Richard Zhu wrote:
> > Hi Arnaud:
> > Thanks a lot for your kindly guidance and review comments.
> >
> >
> >> -----Original Message-----
> >> From: Arnaud Pouliquen [mailto:arnaud.pouliquen@st.com]
> >> Sent: 2019年7月8日 22:12
> >> To: Oleksij Rempel <o.rempel@pengutronix.de>; Richard Zhu
> >> <hongxing.zhu@nxp.com>; ohad@wizery.com;
> bjorn.andersson@linaro.org;
> >> linux-remoteproc@vger.kernel.org
> >> Cc: loic.pallardy@st.com; Fabien DESSENNE <fabien.dessenne@st.com>;
> >> elder@linaro.org; linux-arm-kernel@lists.infradead.org
> >> Subject: Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx
> >> rpmsg support
> >>
> >>
> >> Hello Richard,
> >>
> >> On 7/8/19 1:02 PM, Oleksij Rempel wrote:
> >>> Hi Richard,
> >>>
> >>> On 08.07.19 12:17, Richard Zhu wrote:
> >>>> Hi Oleksij:
> >>>> Thanks for your comments.
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
> >>>>> Sent: 2019年7月4日 17:36
> >>>>> To: Richard Zhu <hongxing.zhu@nxp.com>; ohad@wizery.com;
> >>>>> bjorn.andersson@linaro.org; linux-remoteproc@vger.kernel.org
> >>>>> Cc: linux-arm-kernel@lists.infradead.org; Fabien DESSENNE
> >>>>> <fabien.dessenne@st.com>; loic.pallardy@st.com;
> >>>>> arnaud.pouliquen@st.com; s-anna@ti.com; elder@linaro.org
> >>>>> Subject: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg
> >>>>> support
> >>>>>
> >>>>>
> >>>>> Hi Richard,
> >>>>>
> >>>>> On 01.07.19 10:34, Richard Zhu wrote:
> >>>>>> Based on "virtio_rpmsg_bus" driver, This patch-set is used to set
> >>>>>> up the communication mechanism between A core and M core on
> i.MX
> >>>>>> AMP
> >>>>> SOCs.
> >>>>>>
> >>>>>> Add the initial imx rpmsg support glue driver and one pingpong
> >>>>>> demo, demonstrated the data transactions between A core and
> >>>>>> remote
> >> M core.
> >>>>>> Distributed framework is used in IMX RPMSG implementation, refer
> >>>>>> to the following requirements:
> >>>>>>     - The CAN functions contained in M core and RTOS should be
> >>>>>> ready and
> >>>>>>       complete functional in 50ms after AMP system is turned on.
> >>>>>>     - Partition reset. System wouldn't be stalled by the
> >>>>>> exceptions (e.x
> >>>>>>       the reset triggered by the system hang) occurred at the
> >>>>>> other side.
> >>>>>>       And the RPMSG mechanism should be recovered automactilly
> >>>>>> after
> >>>>> the
> >>>>>>       partition reset is completed.
> >>>>>> In this scenario, the M core and RTOS would be kicked off by
> >>>>>> bootloader firstly, then A core and Linux would be loaded later.
> >>>>>> Both M core/RTOS and A core/Linux are running independly.
> >>>>>>
> >>>>>> One physical memory region used to store the vring is mandatory
> >>>>>> required to pre-reserved and well-knowned by both A core and M
> >>>>>> core
> >>>>>
> >>>>> I don't see any thing imx specific in this patch. We already have
> >>>>> remoteproc which would parse firmware header and create needed
> >>>>> devices. This driver is only needed for the case where firmware
> >>>>> was stared by the bootloader.
> >>>>>
> >>>> [Richard Zhu] Bootloader starts the firmware is mandatory required
> >>>> in these scenario refer to the reasons listed in the commit.
> >>>> Thus, the distributed framework has to be used, and both A
> >>>> core/Linux and remote core/RTOS works independently.
> >>>>
> >>>>> I personally would prefer to have generic driver or extend the
> >>>>> remoteproc framework. So we can notify kernel about work already
> >>>>> done by bootloader.
> >>>>>
> >>>> [Richard Zhu] Thanks for your suggestions.
> >>>> Regarding to my understand, it seems that master/slave mode is used
> >>>> in the remoteproc currently.
> >>>> A core/Linux acts as master, to controls/manipulates remote core/RTOS.
> >>>> It isn't applicable for the scenario described by this patch-set.
> >>>>
> >>>>> In general, some more issues should be solved:
> >>>>> - Handle or not touch idle clocks for different node used by M
> >>>>> core and not main system.
> >>>>> - pin control
> >>>>> - regulators
> >>>>>
> >>>>> ST devs already tried to solve this issues by creating "remoteproc:
> >>>>> add system
> >>>>> resource manager device" patch. I don't know what is current state
> >>>>> of it (/me adding ST devs to CC).
> >> The resource manager implementation as been proposed but no real
> >> adhesion of the community on it... Perhaps SCMI should be a candidate...
> >>
> >>>>>
> >>>> [Richard Zhu] Yes, it is. Many contributions have been made by Fabien.
> >>>> IMHO, there are some different behaviors on iMX8QXP/QM platforms,
> the
> >>>>   resources (e.x IP modules) had been assigned and managed by the
> >> XRDC.
> >>>> In the other words, the HW resources would be assigned and managed
> >> would
> >>>>   be transparent to SW.
> >>>>
> >>>> Thus, both A core/Linux and M core/RTOS can work real independently.
> >>>> System wouldn't be stalled by the exceptions (e.x the reset
> >>>> triggered by the system hang) occurred at the other side. And the
> >>>> RPMSG mechanism should
> >>>>   be recovered automatically after the partition reset is completed.
> >>>
> >>> It is exactly the way I did understood it in the firs mail. Any way,
> >>> i'm ok with this driver. Just rename imx to some thing generic. This
> >>> driver can and will be reused on other platforms as well.
> >>>
> >>> Kind regards,
> >>> Oleksij Rempel
> >>>
> >>
> >> I'm trying to figure out what is the interest of these drivers vs existing ones.
> >> Please find below a list of features i noticed in your driver (don't
> >> hesitate if i missed some of them), with some comments/questions.
> >>
> >> 1) The coprocessor is started before the one running Linux OS.
> >> Have you taken a look to this set of patches proposed by Loic:
> >> https://lkml.or
> >>
> g%2Flkml%2F2018%2F11%2F30%2F157&amp;data=02%7C01%7Chongxing.z
> >>
> hu%40nxp.com%7C6773999ac6394dfe37d008d703ae3475%7C686ea1d3bc2
> >>
> b4c6fa92cd99c5c301635%7C0%7C0%7C636981919064186660&amp;sdata=
> >>
> OUogIs2S7gLR46%2FNcAU3OqEtB4rK3sW0gRKRRSO6xpk%3D&amp;reserved
> >> =0
> >> with this patch you should be able to"attach" on the fly on a
> >> preloaded firmware.
> > [Richard Zhu] Yes, this patch-set enable to pre-load the firmware in
> bootloader.
> > The most difficulties when I try to use the current master/slave mode
> > are that  the remote-proc controls/management mode is not applicable
> > to the scenario,  especially the iMX8QXP/QM partition reset usage.
> > Both A core/Linux and M core/RTOS are working independently.
> > HW resources(e.x: some IP modules, DDR memory region, power domains,
> > clocks and so on)  would be pre-managed and pre-assigned to A core or
> > M core by SCFW through XRDC module refer to the security reasons or
> something else I don't know.
> If i well understand the xRDC is an IP which allows hardware isolation of some
> resources to the A or M core. So it is set by the secure part of the bootloader,
> right?
[Richard Zhu] Yes, you're right.
It would be configured by SCFW.

> We have an equivalence on the STM32MP1 named ETZPC.
> So we also manage this use case on stm32mp1. The bootloader configures
> resource isolation, and can load and start the Cortex-M firmware, before the
> linux firmware. That why i pointed this patch. In case of preloaded firmware
> the remote proc does not load the firmware but just parse the resource table
> (address needs to be provided by the rproc_platform driver). The rpmsg bus is
> probed according to the resource table entries.
> This part of code is not upstreamed for time being (waiting integration of the
> mentioned patch). Nevertheless You can take a look on mechanism we
> implemented, on ST github (we named it early_boot):
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2FSTMicroelectronics%2Flinux%2Fblob%2Fv4.19-stm32mp%2Fdrivers%
> 2Fremoteproc%2Fstm32_rproc.c&amp;data=02%7C01%7Chongxing.zhu%40n
> xp.com%7Ce47bde268f3c4290b91708d70453c995%7C686ea1d3bc2b4c6fa9
> 2cd99c5c301635%7C0%7C0%7C636982630222216880&amp;sdata=1dtIyIVYf
> sg693v6UjM%2BFEk7TYHxgg6RDX611%2FKfjqA%3D&amp;reserved=0
> >
> > M core/RTOS insists to run and manage its resources assigned by XRDC
> standalone.
> > All the interactions between A core and M core are transferred on RPMSG
> channels.
> > For example, the audio codec configuration and so on.
> > So, what I do here is just setup the communication RPMSG channels
> > between A core/Linux and M core/RTOS.
> >
> > One more concern, I'm afraid that I may mess up the current solid
> > reproc flow and framework if  I force this implementation into the current
> reproc drivers.
> > So, I summit this patch-set in the end. Pre-reserved vring buffer,
> > register virtio_device, establish the RPMSG channels lets A core/Linux and
> M Core/RTOS can communicate with each other.
> > That's all.
> Your concern is valid, and as we have the same requirement, it would be nice
> to find a common solution. That's why i propose this alternative, which would
> have the advantage of reusing existing rpmsg implementation.
> 
 [Richard Zhu] I looked through the codes briefly. Correct me if my understand
 is wrong.
It seems that the A core side does a lot of manipulations to the remote M4 core
 on ST32M.
During the start/stop/recovery operations, M4 acted as slave and waiting for the
 control constructions sent from the master A core/Linux side although the
 early_boot is set.

There are some differences in the relationship between A core and M core.
On ST32M: M4/RTOS would started/stopped/recovered by A core/Linux side.

In my purposed implementation, both A core/Linux and M core/RTOS working in the real
 independent mode.
- M4/RTOS complete the start/stop/recovery and son on operations by itself, it wouldn't
 accept any start/stop/reset interactions from A core/Linux side. Same to A core/Linux side.
- SCFW monitors the running status of each side, would notify the other side, if there is a
 system stall at one side.
 when the lived side receives the notification and know the other side is reset,
 It would only recover its own rpmsg stack, wait the rpmsg "ready" signal of the opposite side,
 then re-establish the rpmsg channels again.
 A core/Linux or M core/RTOS wouldn't do the start/stop/recovery operations on the opposite side.
 
Anyway, let me do some more homework, and figure out that whether I can fit these into the existing
remoteproc framework or not.

> >>
> >> 2) RPMSG recovery
> >> Agree with you, this feature is important in AMP systems, as cores
> >> can have their own live cycle.
> >>
> >> But I can not see related code, could you point out it to me?
> >>
> > [Richard Zhu] This feature had been validated in the local repos.
> > But these codes are not contained in this patch-set, because this
> > feature is relied on the SCFW(system control firm ware) used to
> > monitor the status of  both side, and trigger one irq to the other side, if one
> side is stall.
> > Unfortunately, it is not up streamed yet. So, these codes would be
> > updated later If the SCFW is ready.
> >
> >> Could you explain How do you recover the rpmsg channels that has been
> >> already established?
> >> For instance what happen if your coprocessor crash during the rpmsg
> >> pingpong demo?
> > [Richard Zhu] SCFW would inform the other side, if one core/OS is crashed.
> > Then, the RPMSG stack would be re-initialized itself on the lived
> > core/OS, and clarify that It's ready to re-establish the channels again.
> > For example, M4/RTOS is crashed when pingpong demo is running.
> > 1. Pingpong demo is stopped.
> > 2. Lived A core/Linux would receive one irq from SCFW indicated that
> > remote M4/RTOS is  reset, then all the virtio_device registered in A
> > core/Linux side, would be un-registered,  and these virtio_devices
> > would be registered again after receive the signal(e.x the mailbox rdb)
> that M4/RTOS RPMSG stack is ready again.
> > 3. Thus RPMS channels can be re-established in this situation.
> > 4. Accordingly, the consumer of the rpmsg glue driver should be re-initialized
> too.
> > For example, remove the pingpond demo module, and insmod it again.
> 
> Thanks for the clarification, i think this is no so far from the recovery already
> implemented in remoteproc. Seems you remote proc driver handles the
> recovery:
>  -stop rproc on irq reception, restart it ( in preloaded mode) on mailbox rdb.
> On stm32MP1 we have a similar mechanism based on a Watchdog.
> 
[Richard Zhu] Regarding to my understand, STM32MP1 stop/restart the remote
 M core/RTOS although the preloaded mode is used, right?


> >
> >>
> >> 3) ping-pong demo sample
> >> Perhaps you could re-use the rpmsg sample available here:
> >> https://elixir.b
> >>
> ootlin.com%2Flinux%2Fv5.2%2Fsource%2Fsamples%2Frpmsg&amp;data=02
> >> %7C01%7Chongxing.zhu%40nxp.com%7C6773999ac6394dfe37d008d703a
> e3
> >>
> 475%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636981919064
> >>
> 186660&amp;sdata=mSV3YsoyhAO%2FROfWX79X0woGQN3jx%2Fv4pL8LRUf
> >> bHUM%3D&amp;reserved=0
> > [Richard Zhu] Thanks a lot.
> > This demo sample can be used. Sorry about that I didn't notice it before.
> >
> >>
> >> 4) No use of the resource table
> >> Is there a reason to not use the resource table to declare the the
> >> vrings? Your implementation seems to impose the same definition in
> >> both firmware while resource table allow to share them.
> >> Furthermore the resource table could be updated by the Linux before
> >> the remote proc is started (in case of Linux booting first)
> >>
> > [Richard Zhu] Regarding to the auto industry requirements, the M
> > core/RTOS is always started firstly, because that the CAN functions
> > should be ready in 50ms after system is power up.
> > BTW, resource table is a great idea in the case when Linux is booting firstly.
> As explained before We also use it when cortex-M4 is booted firstly. A
> constraint is that the resource table address should be known by the
> remoteproc driver: either the resource table address is defined in DT, or
> provided by the bootloader which loads the firmware so parses it.
> 
[Richard Zhu] Up to now, the pre-defined vring address and the mailbox channels
 are defined in the DT in my local implementation.
FYI. Here are the details. "https://patchwork.kernel.org/patch/11031059/"

> >
> >> 5) slave and master mode support.
> >> Seems that this drivers not fully respect the virtio protocol (for
> >> instance status field). If you use a synchro mechanism (mailbox...)
> >> not sure that you really need to be virtio slave on Linux.
> > [Richard Zhu] Sorry about that. I used trying to keep this driver
> > compatible with the current slave-master mode, but I'm failed to achieve
> that. ☹.
> > - Partition reset feature is mandatory required.
> > - M4 side insists that they should run and manage its resources standalone.
> No problem, it is an RFC.
> Anyway regarding you requirements and concerns, it seems that we have the
> same ones. I don't know if the solution we propose can fit with your needs,
> but i would be nice to have a common implementation.
> 
[Richard Zhu] Agree. It's great if there is a common solution taking the advantage of
 reusing existing rpmsg implementation.

Thanks a lot for your kindly review comments.
Best Regards
Richard

> Best Regards,
> Arnaud
> 
> >
> > Best Regards
> > Richard Zhu
> >>
> >> Thanks,
> >> Arnaud

Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Arnaud Pouliquen]

On 7/10/19 10:13 AM, Richard Zhu wrote:
>> -----Original Message-----
>> From: Arnaud Pouliquen [mailto:arnaud.pouliquen@st.com]
>> Sent: 2019年7月9日 17:57
>> To: Richard Zhu <hongxing.zhu@nxp.com>; Oleksij Rempel
>> <o.rempel@pengutronix.de>; ohad@wizery.com; bjorn.andersson@linaro.org;
>> linux-remoteproc@vger.kernel.org
>> Cc: loic.pallardy@st.com; Fabien DESSENNE <fabien.dessenne@st.com>;
>> elder@linaro.org; linux-arm-kernel@lists.infradead.org
>> Subject: Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support
>>
>> On 7/9/19 9:32 AM, Richard Zhu wrote:
>>> Hi Arnaud:
>>> Thanks a lot for your kindly guidance and review comments.
>>>
>>>
>>>> -----Original Message-----
>>>> From: Arnaud Pouliquen [mailto:arnaud.pouliquen@st.com]
>>>> Sent: 2019年7月8日 22:12
>>>> To: Oleksij Rempel <o.rempel@pengutronix.de>; Richard Zhu
>>>> <hongxing.zhu@nxp.com>; ohad@wizery.com;
>> bjorn.andersson@linaro.org;
>>>> linux-remoteproc@vger.kernel.org
>>>> Cc: loic.pallardy@st.com; Fabien DESSENNE <fabien.dessenne@st.com>;
>>>> elder@linaro.org; linux-arm-kernel@lists.infradead.org
>>>> Subject: Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx
>>>> rpmsg support
>>>>
>>>>
>>>> Hello Richard,
>>>>
>>>> On 7/8/19 1:02 PM, Oleksij Rempel wrote:
>>>>> Hi Richard,
>>>>>
>>>>> On 08.07.19 12:17, Richard Zhu wrote:
>>>>>> Hi Oleksij:
>>>>>> Thanks for your comments.
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
>>>>>>> Sent: 2019年7月4日 17:36
>>>>>>> To: Richard Zhu <hongxing.zhu@nxp.com>; ohad@wizery.com;
>>>>>>> bjorn.andersson@linaro.org; linux-remoteproc@vger.kernel.org
>>>>>>> Cc: linux-arm-kernel@lists.infradead.org; Fabien DESSENNE
>>>>>>> <fabien.dessenne@st.com>; loic.pallardy@st.com;
>>>>>>> arnaud.pouliquen@st.com; s-anna@ti.com; elder@linaro.org
>>>>>>> Subject: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg
>>>>>>> support
>>>>>>>
>>>>>>>
>>>>>>> Hi Richard,
>>>>>>>
>>>>>>> On 01.07.19 10:34, Richard Zhu wrote:
>>>>>>>> Based on "virtio_rpmsg_bus" driver, This patch-set is used to set
>>>>>>>> up the communication mechanism between A core and M core on
>> i.MX
>>>>>>>> AMP
>>>>>>> SOCs.
>>>>>>>>
>>>>>>>> Add the initial imx rpmsg support glue driver and one pingpong
>>>>>>>> demo, demonstrated the data transactions between A core and
>>>>>>>> remote
>>>> M core.
>>>>>>>> Distributed framework is used in IMX RPMSG implementation, refer
>>>>>>>> to the following requirements:
>>>>>>>>      - The CAN functions contained in M core and RTOS should be
>>>>>>>> ready and
>>>>>>>>        complete functional in 50ms after AMP system is turned on.
>>>>>>>>      - Partition reset. System wouldn't be stalled by the
>>>>>>>> exceptions (e.x
>>>>>>>>        the reset triggered by the system hang) occurred at the
>>>>>>>> other side.
>>>>>>>>        And the RPMSG mechanism should be recovered automactilly
>>>>>>>> after
>>>>>>> the
>>>>>>>>        partition reset is completed.
>>>>>>>> In this scenario, the M core and RTOS would be kicked off by
>>>>>>>> bootloader firstly, then A core and Linux would be loaded later.
>>>>>>>> Both M core/RTOS and A core/Linux are running independly.
>>>>>>>>
>>>>>>>> One physical memory region used to store the vring is mandatory
>>>>>>>> required to pre-reserved and well-knowned by both A core and M
>>>>>>>> core
>>>>>>>
>>>>>>> I don't see any thing imx specific in this patch. We already have
>>>>>>> remoteproc which would parse firmware header and create needed
>>>>>>> devices. This driver is only needed for the case where firmware
>>>>>>> was stared by the bootloader.
>>>>>>>
>>>>>> [Richard Zhu] Bootloader starts the firmware is mandatory required
>>>>>> in these scenario refer to the reasons listed in the commit.
>>>>>> Thus, the distributed framework has to be used, and both A
>>>>>> core/Linux and remote core/RTOS works independently.
>>>>>>
>>>>>>> I personally would prefer to have generic driver or extend the
>>>>>>> remoteproc framework. So we can notify kernel about work already
>>>>>>> done by bootloader.
>>>>>>>
>>>>>> [Richard Zhu] Thanks for your suggestions.
>>>>>> Regarding to my understand, it seems that master/slave mode is used
>>>>>> in the remoteproc currently.
>>>>>> A core/Linux acts as master, to controls/manipulates remote core/RTOS.
>>>>>> It isn't applicable for the scenario described by this patch-set.
>>>>>>
>>>>>>> In general, some more issues should be solved:
>>>>>>> - Handle or not touch idle clocks for different node used by M
>>>>>>> core and not main system.
>>>>>>> - pin control
>>>>>>> - regulators
>>>>>>>
>>>>>>> ST devs already tried to solve this issues by creating "remoteproc:
>>>>>>> add system
>>>>>>> resource manager device" patch. I don't know what is current state
>>>>>>> of it (/me adding ST devs to CC).
>>>> The resource manager implementation as been proposed but no real
>>>> adhesion of the community on it... Perhaps SCMI should be a candidate...
>>>>
>>>>>>>
>>>>>> [Richard Zhu] Yes, it is. Many contributions have been made by Fabien.
>>>>>> IMHO, there are some different behaviors on iMX8QXP/QM platforms,
>> the
>>>>>>    resources (e.x IP modules) had been assigned and managed by the
>>>> XRDC.
>>>>>> In the other words, the HW resources would be assigned and managed
>>>> would
>>>>>>    be transparent to SW.
>>>>>>
>>>>>> Thus, both A core/Linux and M core/RTOS can work real independently.
>>>>>> System wouldn't be stalled by the exceptions (e.x the reset
>>>>>> triggered by the system hang) occurred at the other side. And the
>>>>>> RPMSG mechanism should
>>>>>>    be recovered automatically after the partition reset is completed.
>>>>>
>>>>> It is exactly the way I did understood it in the firs mail. Any way,
>>>>> i'm ok with this driver. Just rename imx to some thing generic. This
>>>>> driver can and will be reused on other platforms as well.
>>>>>
>>>>> Kind regards,
>>>>> Oleksij Rempel
>>>>>
>>>>
>>>> I'm trying to figure out what is the interest of these drivers vs existing ones.
>>>> Please find below a list of features i noticed in your driver (don't
>>>> hesitate if i missed some of them), with some comments/questions.
>>>>
>>>> 1) The coprocessor is started before the one running Linux OS.
>>>> Have you taken a look to this set of patches proposed by Loic:
>>>> https://lkml.or
>>>>
>> g%2Flkml%2F2018%2F11%2F30%2F157&amp;data=02%7C01%7Chongxing.z
>>>>
>> hu%40nxp.com%7C6773999ac6394dfe37d008d703ae3475%7C686ea1d3bc2
>>>>
>> b4c6fa92cd99c5c301635%7C0%7C0%7C636981919064186660&amp;sdata=
>>>>
>> OUogIs2S7gLR46%2FNcAU3OqEtB4rK3sW0gRKRRSO6xpk%3D&amp;reserved
>>>> =0
>>>> with this patch you should be able to"attach" on the fly on a
>>>> preloaded firmware.
>>> [Richard Zhu] Yes, this patch-set enable to pre-load the firmware in
>> bootloader.
>>> The most difficulties when I try to use the current master/slave mode
>>> are that  the remote-proc controls/management mode is not applicable
>>> to the scenario,  especially the iMX8QXP/QM partition reset usage.
>>> Both A core/Linux and M core/RTOS are working independently.
>>> HW resources(e.x: some IP modules, DDR memory region, power domains,
>>> clocks and so on)  would be pre-managed and pre-assigned to A core or
>>> M core by SCFW through XRDC module refer to the security reasons or
>> something else I don't know.
>> If i well understand the xRDC is an IP which allows hardware isolation of some
>> resources to the A or M core. So it is set by the secure part of the bootloader,
>> right?
> [Richard Zhu] Yes, you're right.
> It would be configured by SCFW.
> 
>> We have an equivalence on the STM32MP1 named ETZPC.
>> So we also manage this use case on stm32mp1. The bootloader configures
>> resource isolation, and can load and start the Cortex-M firmware, before the
>> linux firmware. That why i pointed this patch. In case of preloaded firmware
>> the remote proc does not load the firmware but just parse the resource table
>> (address needs to be provided by the rproc_platform driver). The rpmsg bus is
>> probed according to the resource table entries.
>> This part of code is not upstreamed for time being (waiting integration of the
>> mentioned patch). Nevertheless You can take a look on mechanism we
>> implemented, on ST github (we named it early_boot):
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
>> com%2FSTMicroelectronics%2Flinux%2Fblob%2Fv4.19-stm32mp%2Fdrivers%
>> 2Fremoteproc%2Fstm32_rproc.c&amp;data=02%7C01%7Chongxing.zhu%40n
>> xp.com%7Ce47bde268f3c4290b91708d70453c995%7C686ea1d3bc2b4c6fa9
>> 2cd99c5c301635%7C0%7C0%7C636982630222216880&amp;sdata=1dtIyIVYf
>> sg693v6UjM%2BFEk7TYHxgg6RDX611%2FKfjqA%3D&amp;reserved=0
>>>
>>> M core/RTOS insists to run and manage its resources assigned by XRDC
>> standalone.
>>> All the interactions between A core and M core are transferred on RPMSG
>> channels.
>>> For example, the audio codec configuration and so on.
>>> So, what I do here is just setup the communication RPMSG channels
>>> between A core/Linux and M core/RTOS.
>>>
>>> One more concern, I'm afraid that I may mess up the current solid
>>> reproc flow and framework if  I force this implementation into the current
>> reproc drivers.
>>> So, I summit this patch-set in the end. Pre-reserved vring buffer,
>>> register virtio_device, establish the RPMSG channels lets A core/Linux and
>> M Core/RTOS can communicate with each other.
>>> That's all.
>> Your concern is valid, and as we have the same requirement, it would be nice
>> to find a common solution. That's why i propose this alternative, which would
>> have the advantage of reusing existing rpmsg implementation.
>>
>   [Richard Zhu] I looked through the codes briefly. Correct me if my understand
>   is wrong.
> It seems that the A core side does a lot of manipulations to the remote M4 core
>   on ST32M.
> During the start/stop/recovery operations, M4 acted as slave and waiting for the
>   control constructions sent from the master A core/Linux side although the
>   early_boot is set.
> 
> There are some differences in the relationship between A core and M core.
> On ST32M: M4/RTOS would started/stopped/recovered by A core/Linux side.
> 
> In my purposed implementation, both A core/Linux and M core/RTOS working in the real
>   independent mode.
> - M4/RTOS complete the start/stop/recovery and son on operations by itself, it wouldn't
>   accept any start/stop/reset interactions from A core/Linux side. Same to A core/Linux side.
> - SCFW monitors the running status of each side, would notify the other side, if there is a
>   system stall at one side.
>   when the lived side receives the notification and know the other side is reset,
>   It would only recover its own rpmsg stack, wait the rpmsg "ready" signal of the opposite side,
>   then re-establish the rpmsg channels again.
>   A core/Linux or M core/RTOS wouldn't do the start/stop/recovery operations on the opposite side.
On STM32MP1 we have not exactly the same strategy but it only a ST 
design choice, implemented in our stm32 remoteproc driver. You should be 
able to implement your expected behavior in your the imx remoteproc driver.

On STM32MP1 we manage the M4 preloaded firmware in this way:
-  On Linux stm32 remoteproc probe:
	We detect that the firmware is preloaded (early-booted filed in DT) and 
set the earl_boot variable.
	we provide the resource table address to the remoteproc core that 
parses it an call the stm32_rproc_start. here we do nothing as M4 
already started we just set the hold boot to freeze the M4 in case of crash

- On M4 crash we have not the same strategy as your one. We consider 
that the M4 firmware can be corrupted and either we try to reload a firmware
which as been provided by application, or we don't let it restarting 
(hold boot set on start).

-We allow userland to stop the preloaded firmware to load and to run a 
new one.

>   
> Anyway, let me do some more homework, and figure out that whether I can fit these into the existing
> remoteproc framework or not.
Sorry to give you homework... but seems (IMHO) possible to integrate 
your constraint in rpmsg/remoteproc current design.

> 
>>>>
>>>> 2) RPMSG recovery
>>>> Agree with you, this feature is important in AMP systems, as cores
>>>> can have their own live cycle.
>>>>
>>>> But I can not see related code, could you point out it to me?
>>>>
>>> [Richard Zhu] This feature had been validated in the local repos.
>>> But these codes are not contained in this patch-set, because this
>>> feature is relied on the SCFW(system control firm ware) used to
>>> monitor the status of  both side, and trigger one irq to the other side, if one
>> side is stall.
>>> Unfortunately, it is not up streamed yet. So, these codes would be
>>> updated later If the SCFW is ready.
>>>
>>>> Could you explain How do you recover the rpmsg channels that has been
>>>> already established?
>>>> For instance what happen if your coprocessor crash during the rpmsg
>>>> pingpong demo?
>>> [Richard Zhu] SCFW would inform the other side, if one core/OS is crashed.
>>> Then, the RPMSG stack would be re-initialized itself on the lived
>>> core/OS, and clarify that It's ready to re-establish the channels again.
>>> For example, M4/RTOS is crashed when pingpong demo is running.
>>> 1. Pingpong demo is stopped.
>>> 2. Lived A core/Linux would receive one irq from SCFW indicated that
>>> remote M4/RTOS is  reset, then all the virtio_device registered in A
>>> core/Linux side, would be un-registered,  and these virtio_devices
>>> would be registered again after receive the signal(e.x the mailbox rdb)
>> that M4/RTOS RPMSG stack is ready again.
>>> 3. Thus RPMS channels can be re-established in this situation.
>>> 4. Accordingly, the consumer of the rpmsg glue driver should be re-initialized
>> too.
>>> For example, remove the pingpond demo module, and insmod it again.
>>
>> Thanks for the clarification, i think this is no so far from the recovery already
>> implemented in remoteproc. Seems you remote proc driver handles the
>> recovery:
>>   -stop rproc on irq reception, restart it ( in preloaded mode) on mailbox rdb.
>> On stm32MP1 we have a similar mechanism based on a Watchdog.
>>
> [Richard Zhu] Regarding to my understand, STM32MP1 stop/restart the remote
>   M core/RTOS although the preloaded mode is used, right?
Yes as explained above we allow to reload a firmware if recovery mode is 
enabled or simply if application when to load a new one.
> 
> 
>>>
>>>>
>>>> 3) ping-pong demo sample
>>>> Perhaps you could re-use the rpmsg sample available here:
>>>> https://elixir.b
>>>>
>> ootlin.com%2Flinux%2Fv5.2%2Fsource%2Fsamples%2Frpmsg&amp;data=02
>>>> %7C01%7Chongxing.zhu%40nxp.com%7C6773999ac6394dfe37d008d703a
>> e3
>>>>
>> 475%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636981919064
>>>>
>> 186660&amp;sdata=mSV3YsoyhAO%2FROfWX79X0woGQN3jx%2Fv4pL8LRUf
>>>> bHUM%3D&amp;reserved=0
>>> [Richard Zhu] Thanks a lot.
>>> This demo sample can be used. Sorry about that I didn't notice it before.
>>>
>>>>
>>>> 4) No use of the resource table
>>>> Is there a reason to not use the resource table to declare the the
>>>> vrings? Your implementation seems to impose the same definition in
>>>> both firmware while resource table allow to share them.
>>>> Furthermore the resource table could be updated by the Linux before
>>>> the remote proc is started (in case of Linux booting first)
>>>>
>>> [Richard Zhu] Regarding to the auto industry requirements, the M
>>> core/RTOS is always started firstly, because that the CAN functions
>>> should be ready in 50ms after system is power up.
>>> BTW, resource table is a great idea in the case when Linux is booting firstly.
>> As explained before We also use it when cortex-M4 is booted firstly. A
>> constraint is that the resource table address should be known by the
>> remoteproc driver: either the resource table address is defined in DT, or
>> provided by the bootloader which loads the firmware so parses it.
>>
> [Richard Zhu] Up to now, the pre-defined vring address and the mailbox channels
>   are defined in the DT in my local implementation.
> FYI. Here are the details. "https://patchwork.kernel.org/patch/11031059/"
> 
Thanks, we have similar use of the reserved memory to declare the vring 
(vdev0vring0, vdev0vring1, vdev0buffer) according to remoteproc core 
requirement:
https://github.com/STMicroelectronics/linux/blob/v4.19-stm32mp/arch/arm/boot/dts/stm32mp157a-dk1.dts

>>>
>>>> 5) slave and master mode support.
>>>> Seems that this drivers not fully respect the virtio protocol (for
>>>> instance status field). If you use a synchro mechanism (mailbox...)
>>>> not sure that you really need to be virtio slave on Linux.
>>> [Richard Zhu] Sorry about that. I used trying to keep this driver
>>> compatible with the current slave-master mode, but I'm failed to achieve
>> that. ☹.
>>> - Partition reset feature is mandatory required.
>>> - M4 side insists that they should run and manage its resources standalone.
>> No problem, it is an RFC.
>> Anyway regarding you requirements and concerns, it seems that we have the
>> same ones. I don't know if the solution we propose can fit with your needs,
>> but i would be nice to have a common implementation.
>>
> [Richard Zhu] Agree. It's great if there is a common solution taking the advantage of
>   reusing existing rpmsg implementation.
> 
> Thanks a lot for your kindly review comments.
Thanks a lot to you for considering an alternative proposal!

Best regards
Arnaud

> Best Regards
> Richard
> 
>> Best Regards,
>> Arnaud
>>
>>>
>>> Best Regards
>>> Richard Zhu
>>>>
>>>> Thanks,
>>>> Arnaud

RE: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support

[Richard Zhu]

Hi Oleksij:

> -----Original Message-----
> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
> Sent: 2019年7月8日 19:03
> To: Richard Zhu <hongxing.zhu@nxp.com>; ohad@wizery.com;
> bjorn.andersson@linaro.org; linux-remoteproc@vger.kernel.org
> Cc: loic.pallardy@st.com; arnaud.pouliquen@st.com; Fabien DESSENNE
> <fabien.dessenne@st.com>; elder@linaro.org;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg support
> 
> Caution: EXT Email
> 
> Hi Richard,
> 
> On 08.07.19 12:17, Richard Zhu wrote:
> > Hi Oleksij:
> > Thanks for your comments.
> >
> >
> >> -----Original Message-----
> >> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
> >> Sent: 2019年7月4日 17:36
> >> To: Richard Zhu <hongxing.zhu@nxp.com>; ohad@wizery.com;
> >> bjorn.andersson@linaro.org; linux-remoteproc@vger.kernel.org
> >> Cc: linux-arm-kernel@lists.infradead.org; Fabien DESSENNE
> >> <fabien.dessenne@st.com>; loic.pallardy@st.com;
> >> arnaud.pouliquen@st.com; s-anna@ti.com; elder@linaro.org
> >> Subject: [EXT] Re: [RFC 2/2] rpmsg: imx: add the initial imx rpmsg
> >> support
> >>
> >> Caution: EXT Email
> >>
> >> Hi Richard,
> >>
> >> On 01.07.19 10:34, Richard Zhu wrote:
> >>> Based on "virtio_rpmsg_bus" driver, This patch-set is used to set up
> >>> the communication mechanism between A core and M core on i.MX AMP
> >> SOCs.
> >>>
> >>> Add the initial imx rpmsg support glue driver and one pingpong demo,
> >>> demonstrated the data transactions between A core and remote M core.
> >>> Distributed framework is used in IMX RPMSG implementation, refer to
> >>> the following requirements:
> >>>     - The CAN functions contained in M core and RTOS should be ready
> and
> >>>       complete functional in 50ms after AMP system is turned on.
> >>>     - Partition reset. System wouldn't be stalled by the exceptions (e.x
> >>>       the reset triggered by the system hang) occurred at the other
> side.
> >>>       And the RPMSG mechanism should be recovered automactilly
> after
> >> the
> >>>       partition reset is completed.
> >>> In this scenario, the M core and RTOS would be kicked off by
> >>> bootloader firstly, then A core and Linux would be loaded later.
> >>> Both M core/RTOS and A core/Linux are running independly.
> >>>
> >>> One physical memory region used to store the vring is mandatory
> >>> required to pre-reserved and well-knowned by both A core and M core
> >>
> >> I don't see any thing imx specific in this patch. We already have
> >> remoteproc which would parse firmware header and create needed
> >> devices. This driver is only needed for the case where firmware was stared
> by the bootloader.
> >>
> > [Richard Zhu] Bootloader starts the firmware is mandatory required in
> > these scenario refer to the reasons listed in the commit.
> > Thus, the distributed framework has to be used, and both A core/Linux
> > and remote core/RTOS works independently.
> >
> >> I personally would prefer to have generic driver or extend the
> >> remoteproc framework. So we can notify kernel about work already done
> by bootloader.
> >>
> > [Richard Zhu] Thanks for your suggestions.
> > Regarding to my understand, it seems that master/slave mode is used in the
> remoteproc currently.
> > A core/Linux acts as master, to controls/manipulates remote core/RTOS.
> > It isn't applicable for the scenario described by this patch-set.
> >
> >> In general, some more issues should be solved:
> >> - Handle or not touch idle clocks for different node used by M core
> >> and not main system.
> >> - pin control
> >> - regulators
> >>
> >> ST devs already tried to solve this issues by creating "remoteproc:
> >> add system resource manager device" patch. I don't know what is
> >> current state of it (/me adding ST devs to CC).
> >>
> > [Richard Zhu] Yes, it is. Many contributions have been made by Fabien.
> > IMHO, there are some different behaviors on iMX8QXP/QM platforms, the
> >   resources (e.x IP modules) had been assigned and managed by the
> XRDC.
> > In the other words, the HW resources would be assigned and managed
> would
> >   be transparent to SW.
> >
> > Thus, both A core/Linux and M core/RTOS can work real independently.
> > System wouldn't be stalled by the exceptions (e.x the reset triggered
> > by the system hang) occurred at the other side. And the RPMSG mechanism
> should
> >   be recovered automatically after the partition reset is completed.
> 
> It is exactly the way I did understood it in the firs mail. Any way, i'm ok with
> this driver. Just rename imx to some thing generic. This driver can and will be
> reused on other platforms as well.
[Richard Zhu] Thanks for your understand. Would change to generic name if this patch-set
 is acceptable. Thank you.

Best Regards
Richard Zhu
> 
> Kind regards,
> Oleksij Rempel
> 
> --
> Pengutronix e.K.                           |
> |
> Industrial Linux Solutions                 |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.p
> engutronix.de%2F&amp;data=02%7C01%7Chongxing.zhu%40nxp.com%7C85f
> 5637c82ce4ce80f0508d70393d48e%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0%7C0%7C636981805773245171&amp;sdata=%2BZrjvkLWF7agguOkvK
> iWiyaK743oI1V9YmpLcyRrgJQ%3D&amp;reserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |