[PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Arnaud Pouliquen]

1) Update from V7 [1]:

- rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc: imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
  The updates take into account the integration of the
  commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after rproc_shutdown")
- add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> according to reviews on V7


[1] https://lkml.org/lkml/2022/7/13/663
[2] https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/log/?h=for-next

2) Patchset description:

This series is a part of the work initiated a long time ago in 
the series "remoteproc: Decorelate virtio from core"[3]

Objective of the work:
- Update the remoteproc VirtIO device creation (use platform device)
- Allow to declare remoteproc VirtIO device in DT
    - declare resources associated to a remote proc VirtIO
    - declare a list of VirtIO supported by the platform.
- Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
  For instance be able to declare a I2C device in a virtio-i2C node.
- Keep the legacy working!
- Try to improve the picture about concerns reported by Christoph Hellwing [4][5]

[3] https://lkml.org/lkml/2020/4/16/1817
[4] https://lkml.org/lkml/2021/6/23/607
[5] https://patchwork.kernel.org/project/linux-remoteproc/patch/AOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E@cp7-web-042.plabs.ch/

In term of device tree this would result in such hierarchy (stm32mp1 example with 2 virtio RPMSG):

	m4_rproc: m4@10000000 {
		compatible = "st,stm32mp1-m4";
		reg = <0x10000000 0x40000>,
		      <0x30000000 0x40000>,
		      <0x38000000 0x10000>;
        memory-region = <&retram>, <&mcuram>,<&mcuram2>;
        mboxes = <&ipcc 2>, <&ipcc 3>;
        mbox-names = "shutdown", "detach";
        status = "okay";

        #address-cells = <1>;
        #size-cells = <0>;
        
        vdev@0 {
		compatible = "rproc-virtio";
		reg = <0>;
		virtio,id = <7>;  /* RPMSG */
		memory-region = <&vdev0vring0>, <&vdev0vring1>, <&vdev0buffer>;
		mboxes = <&ipcc 0>, <&ipcc 1>;
		mbox-names = "vq0", "vq1";
		status = "okay";
        };

        vdev@1 {
		compatible = "rproc-virtio";
		reg = <1>;
		virtio,id = <7>;  /*RPMSG */
		memory-region = <&vdev1vring0>, <&vdev1vring1>, <&vdev1buffer>;
		mboxes = <&ipcc 4>, <&ipcc 5>;
		mbox-names = "vq0", "vq1";
		status = "okay";
        };
};

I have divided the work in 4 steps to simplify the review, This series implements only
the step 1:
step 1: Redefine the remoteproc VirtIO device as a platform device
  - migrate rvdev management in remoteproc virtio.c,
  - create a remotproc virtio config ( can be disabled for platform that not use VirtIO IPC.
step 2: Add possibility to declare and probe a VirtIO sub node
  - VirtIO bindings declaration,
  - multi DT VirtIO devices support,
  - introduction of a remote proc virtio bind device mechanism ,
=> https://github.com/arnopo/linux/commits/step2-virtio-in-DT
step 3: Add memory declaration in VirtIO subnode
=> https://github.com/arnopo/linux/commits/step3-virtio-memories
step 4: Add mailbox declaration in VirtIO subnode
=> https://github.com/arnopo/linux/commits/step4-virtio-mailboxes

Arnaud Pouliquen (4):
  remoteproc: core: Introduce rproc_rvdev_add_device function
  remoteproc: core: Introduce rproc_add_rvdev function
  remoteproc: Move rproc_vdev management to remoteproc_virtio.c
  remoteproc: virtio: Create platform device for the remoteproc_virtio

 drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
 drivers/remoteproc/remoteproc_internal.h |  23 ++-
 drivers/remoteproc/remoteproc_virtio.c   | 189 ++++++++++++++++++++---
 include/linux/remoteproc.h               |   6 +-
 4 files changed, 210 insertions(+), 162 deletions(-)

-- 
2.24.3

[PATCH v8 1/4] remoteproc: core: Introduce rproc_rvdev_add_device function

[Arnaud Pouliquen]

In preparation of the migration of the management of rvdev in
remoteproc_virtio.c, this patch spins off a new function to manage the
remoteproc virtio device creation.

The rproc_rvdev_add_device will be moved to remoteproc_virtio.c.

The rproc_vdev_data structure is introduced to provide information for
the rvdev creation. This structure allows to manage the rvdev and vrings
allocation in the rproc_rvdev_add_device function.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c     | 145 +++++++++++++----------
 drivers/remoteproc/remoteproc_internal.h |  15 +++
 2 files changed, 97 insertions(+), 63 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index e5279ed9a8d7..c977e6c0bdb7 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -486,74 +486,23 @@ static int copy_dma_range_map(struct device *to, struct device *from)
 	return 0;
 }
 
-/**
- * rproc_handle_vdev() - handle a vdev fw resource
- * @rproc: the remote processor
- * @ptr: the vring resource descriptor
- * @offset: offset of the resource entry
- * @avail: size of available data (for sanity checking the image)
- *
- * This resource entry requests the host to statically register a virtio
- * device (vdev), and setup everything needed to support it. It contains
- * everything needed to make it possible: the virtio device id, virtio
- * device features, vrings information, virtio config space, etc...
- *
- * Before registering the vdev, the vrings are allocated from non-cacheable
- * physically contiguous memory. Currently we only support two vrings per
- * remote processor (temporary limitation). We might also want to consider
- * doing the vring allocation only later when ->find_vqs() is invoked, and
- * then release them upon ->del_vqs().
- *
- * Note: @da is currently not really handled correctly: we dynamically
- * allocate it using the DMA API, ignoring requested hard coded addresses,
- * and we don't take care of any required IOMMU programming. This is all
- * going to be taken care of when the generic iommu-based DMA API will be
- * merged. Meanwhile, statically-addressed iommu-based firmware images should
- * use RSC_DEVMEM resource entries to map their required @da to the physical
- * address of their base CMA region (ouch, hacky!).
- *
- * Return: 0 on success, or an appropriate error code otherwise
- */
-static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
-			     int offset, int avail)
+static struct rproc_vdev *
+rproc_rvdev_add_device(struct rproc *rproc, struct rproc_vdev_data *rvdev_data)
 {
-	struct fw_rsc_vdev *rsc = ptr;
-	struct device *dev = &rproc->dev;
 	struct rproc_vdev *rvdev;
-	int i, ret;
+	struct fw_rsc_vdev *rsc = rvdev_data->rsc;
 	char name[16];
-
-	/* make sure resource isn't truncated */
-	if (struct_size(rsc, vring, rsc->num_of_vrings) + rsc->config_len >
-			avail) {
-		dev_err(dev, "vdev rsc is truncated\n");
-		return -EINVAL;
-	}
-
-	/* make sure reserved bytes are zeroes */
-	if (rsc->reserved[0] || rsc->reserved[1]) {
-		dev_err(dev, "vdev rsc has non zero reserved bytes\n");
-		return -EINVAL;
-	}
-
-	dev_dbg(dev, "vdev rsc: id %d, dfeatures 0x%x, cfg len %d, %d vrings\n",
-		rsc->id, rsc->dfeatures, rsc->config_len, rsc->num_of_vrings);
-
-	/* we currently support only two vrings per rvdev */
-	if (rsc->num_of_vrings > ARRAY_SIZE(rvdev->vring)) {
-		dev_err(dev, "too many vrings: %d\n", rsc->num_of_vrings);
-		return -EINVAL;
-	}
+	int i, ret;
 
 	rvdev = kzalloc(sizeof(*rvdev), GFP_KERNEL);
 	if (!rvdev)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	kref_init(&rvdev->refcount);
 
-	rvdev->id = rsc->id;
+	rvdev->id = rvdev_data->id;
 	rvdev->rproc = rproc;
-	rvdev->index = rproc->nb_vdev++;
+	rvdev->index = rvdev_data->index;
 
 	/* Initialise vdev subdevice */
 	snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
@@ -565,7 +514,7 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 	ret = device_register(&rvdev->dev);
 	if (ret) {
 		put_device(&rvdev->dev);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
 	ret = copy_dma_range_map(&rvdev->dev, rproc->dev.parent);
@@ -578,7 +527,7 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 	ret = dma_coerce_mask_and_coherent(&rvdev->dev,
 					   dma_get_mask(rproc->dev.parent));
 	if (ret) {
-		dev_warn(dev,
+		dev_warn(&rvdev->dev,
 			 "Failed to set DMA mask %llx. Trying to continue... (%pe)\n",
 			 dma_get_mask(rproc->dev.parent), ERR_PTR(ret));
 	}
@@ -591,7 +540,7 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 	}
 
 	/* remember the resource offset*/
-	rvdev->rsc_offset = offset;
+	rvdev->rsc_offset = rvdev_data->rsc_offset;
 
 	/* allocate the vring resources */
 	for (i = 0; i < rsc->num_of_vrings; i++) {
@@ -607,14 +556,14 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 
 	rproc_add_subdev(rproc, &rvdev->subdev);
 
-	return 0;
+	return rvdev;
 
 unwind_vring_allocations:
 	for (i--; i >= 0; i--)
 		rproc_free_vring(&rvdev->vring[i]);
 free_rvdev:
 	device_unregister(&rvdev->dev);
-	return ret;
+	return ERR_PTR(ret);
 }
 
 void rproc_vdev_release(struct kref *ref)
@@ -634,6 +583,76 @@ void rproc_vdev_release(struct kref *ref)
 	device_unregister(&rvdev->dev);
 }
 
+/**
+ * rproc_handle_vdev() - handle a vdev fw resource
+ * @rproc: the remote processor
+ * @ptr: the vring resource descriptor
+ * @offset: offset of the resource entry
+ * @avail: size of available data (for sanity checking the image)
+ *
+ * This resource entry requests the host to statically register a virtio
+ * device (vdev), and setup everything needed to support it. It contains
+ * everything needed to make it possible: the virtio device id, virtio
+ * device features, vrings information, virtio config space, etc...
+ *
+ * Before registering the vdev, the vrings are allocated from non-cacheable
+ * physically contiguous memory. Currently we only support two vrings per
+ * remote processor (temporary limitation). We might also want to consider
+ * doing the vring allocation only later when ->find_vqs() is invoked, and
+ * then release them upon ->del_vqs().
+ *
+ * Note: @da is currently not really handled correctly: we dynamically
+ * allocate it using the DMA API, ignoring requested hard coded addresses,
+ * and we don't take care of any required IOMMU programming. This is all
+ * going to be taken care of when the generic iommu-based DMA API will be
+ * merged. Meanwhile, statically-addressed iommu-based firmware images should
+ * use RSC_DEVMEM resource entries to map their required @da to the physical
+ * address of their base CMA region (ouch, hacky!).
+ *
+ * Return: 0 on success, or an appropriate error code otherwise
+ */
+static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
+			     int offset, int avail)
+{
+	struct fw_rsc_vdev *rsc = ptr;
+	struct device *dev = &rproc->dev;
+	struct rproc_vdev *rvdev;
+	struct rproc_vdev_data rvdev_data;
+
+	/* make sure resource isn't truncated */
+	if (struct_size(rsc, vring, rsc->num_of_vrings) + rsc->config_len >
+			avail) {
+		dev_err(dev, "vdev rsc is truncated\n");
+		return -EINVAL;
+	}
+
+	/* make sure reserved bytes are zeroes */
+	if (rsc->reserved[0] || rsc->reserved[1]) {
+		dev_err(dev, "vdev rsc has non zero reserved bytes\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "vdev rsc: id %d, dfeatures 0x%x, cfg len %d, %d vrings\n",
+		rsc->id, rsc->dfeatures, rsc->config_len, rsc->num_of_vrings);
+
+	/* we currently support only two vrings per rvdev */
+	if (rsc->num_of_vrings > ARRAY_SIZE(rvdev->vring)) {
+		dev_err(dev, "too many vrings: %d\n", rsc->num_of_vrings);
+		return -EINVAL;
+	}
+
+	rvdev_data.id = rsc->id;
+	rvdev_data.index = rproc->nb_vdev++;
+	rvdev_data.rsc_offset = offset;
+	rvdev_data.rsc = rsc;
+
+	rvdev = rproc_rvdev_add_device(rproc, &rvdev_data);
+	if (IS_ERR(rvdev))
+		return PTR_ERR(rvdev);
+
+	return 0;
+}
+
 /**
  * rproc_handle_trace() - handle a shared trace buffer resource
  * @rproc: the remote processor
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 72d4d3d7d94d..07c503de0f95 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -24,6 +24,21 @@ struct rproc_debug_trace {
 	struct rproc_mem_entry trace_mem;
 };
 
+/**
+ * struct rproc_vdev_data - remoteproc virtio device data
+ * @rsc_offset: offset of the vdev's resource entry
+ * @id: virtio device id (as in virtio_ids.h)
+ * @index: vdev position versus other vdev declared in resource table
+ * @rsc: pointer to the vdev resource entry. Valid only during vdev init as
+ *       the resource can be cached by rproc.
+ */
+struct rproc_vdev_data {
+	u32 rsc_offset;
+	unsigned int id;
+	u32 index;
+	struct fw_rsc_vdev *rsc;
+};
+
 /* from remoteproc_core.c */
 void rproc_release(struct kref *kref);
 irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
-- 
2.24.3

[PATCH v8 2/4] remoteproc: core: Introduce rproc_add_rvdev function

[Arnaud Pouliquen]

The rproc structure contains a list of registered rproc_vdev structure.
To be able to move the management of the rproc_vdev structure in
remoteproc_virtio.c (i.e rproc_rvdev_add_device function),
introduce the rproc_add_rvdev and rproc_remove_rvdev functions.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index c977e6c0bdb7..756b84811022 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -486,6 +486,18 @@ static int copy_dma_range_map(struct device *to, struct device *from)
 	return 0;
 }
 
+static void rproc_add_rvdev(struct rproc *rproc, struct rproc_vdev *rvdev)
+{
+	if (rvdev && rproc)
+		list_add_tail(&rvdev->node, &rproc->rvdevs);
+}
+
+static void rproc_remove_rvdev(struct rproc_vdev *rvdev)
+{
+	if (rvdev)
+		list_del(&rvdev->node);
+}
+
 static struct rproc_vdev *
 rproc_rvdev_add_device(struct rproc *rproc, struct rproc_vdev_data *rvdev_data)
 {
@@ -549,7 +561,7 @@ rproc_rvdev_add_device(struct rproc *rproc, struct rproc_vdev_data *rvdev_data)
 			goto unwind_vring_allocations;
 	}
 
-	list_add_tail(&rvdev->node, &rproc->rvdevs);
+	rproc_add_rvdev(rproc, rvdev);
 
 	rvdev->subdev.start = rproc_vdev_do_start;
 	rvdev->subdev.stop = rproc_vdev_do_stop;
@@ -579,7 +591,7 @@ void rproc_vdev_release(struct kref *ref)
 	}
 
 	rproc_remove_subdev(rproc, &rvdev->subdev);
-	list_del(&rvdev->node);
+	rproc_remove_rvdev(rvdev);
 	device_unregister(&rvdev->dev);
 }
 
-- 
2.24.3

[PATCH v8 3/4] remoteproc: Move rproc_vdev management to remoteproc_virtio.c

[Arnaud Pouliquen]

Move functions related to the management of the rproc_vdev
structure in the remoteproc_virtio.c.
The aim is to decorrelate as possible the virtio management from
the core part.

Due to the strong correlation between the vrings and the resource table
the rproc_alloc/parse/free_vring functions are kept in the remoteproc core.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
Update vs previous revision:
 - Take into account rproc_rvdev_release update according to
   1404acbb7f68 ("remoteproc: Fix dma_mem leak after rproc_shutdown")
   integration.
---
 drivers/remoteproc/remoteproc_core.c     | 156 +----------------------
 drivers/remoteproc/remoteproc_core.c     | 157 +----------------------
 drivers/remoteproc/remoteproc_internal.h |  10 +-
 drivers/remoteproc/remoteproc_virtio.c   | 154 +++++++++++++++++++++-
 3 files changed, 161 insertions(+), 160 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 756b84811022..a556504a4851 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -23,9 +23,7 @@
 #include <linux/panic_notifier.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
-#include <linux/dma-map-ops.h>
 #include <linux/dma-mapping.h>
-#include <linux/dma-direct.h> /* XXX: pokes into bus_dma_range */
 #include <linux/firmware.h>
 #include <linux/string.h>
 #include <linux/debugfs.h>
@@ -384,7 +382,7 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
 	return 0;
 }
 
-static int
+int
 rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)
 {
 	struct rproc *rproc = rvdev->rproc;
@@ -435,166 +433,17 @@ void rproc_free_vring(struct rproc_vring *rvring)
 	}
 }
 
-static int rproc_vdev_do_start(struct rproc_subdev *subdev)
-{
-	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
-
-	return rproc_add_virtio_dev(rvdev, rvdev->id);
-}
-
-static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
-{
-	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
-	int ret;
-
-	ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
-	if (ret)
-		dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n", ret);
-}
-
-/**
- * rproc_rvdev_release() - release the existence of a rvdev
- *
- * @dev: the subdevice's dev
- */
-static void rproc_rvdev_release(struct device *dev)
-{
-	struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
-
-	of_reserved_mem_device_release(dev);
-	dma_release_coherent_memory(dev);
-
-	kfree(rvdev);
-}
-
-static int copy_dma_range_map(struct device *to, struct device *from)
-{
-	const struct bus_dma_region *map = from->dma_range_map, *new_map, *r;
-	int num_ranges = 0;
-
-	if (!map)
-		return 0;
-
-	for (r = map; r->size; r++)
-		num_ranges++;
-
-	new_map = kmemdup(map, array_size(num_ranges + 1, sizeof(*map)),
-			  GFP_KERNEL);
-	if (!new_map)
-		return -ENOMEM;
-	to->dma_range_map = new_map;
-	return 0;
-}
-
-static void rproc_add_rvdev(struct rproc *rproc, struct rproc_vdev *rvdev)
+void rproc_add_rvdev(struct rproc *rproc, struct rproc_vdev *rvdev)
 {
 	if (rvdev && rproc)
 		list_add_tail(&rvdev->node, &rproc->rvdevs);
 }
 
-static void rproc_remove_rvdev(struct rproc_vdev *rvdev)
+void rproc_remove_rvdev(struct rproc_vdev *rvdev)
 {
 	if (rvdev)
 		list_del(&rvdev->node);
 }
-
-static struct rproc_vdev *
-rproc_rvdev_add_device(struct rproc *rproc, struct rproc_vdev_data *rvdev_data)
-{
-	struct rproc_vdev *rvdev;
-	struct fw_rsc_vdev *rsc = rvdev_data->rsc;
-	char name[16];
-	int i, ret;
-
-	rvdev = kzalloc(sizeof(*rvdev), GFP_KERNEL);
-	if (!rvdev)
-		return ERR_PTR(-ENOMEM);
-
-	kref_init(&rvdev->refcount);
-
-	rvdev->id = rvdev_data->id;
-	rvdev->rproc = rproc;
-	rvdev->index = rvdev_data->index;
-
-	/* Initialise vdev subdevice */
-	snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
-	rvdev->dev.parent = &rproc->dev;
-	rvdev->dev.release = rproc_rvdev_release;
-	dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
-	dev_set_drvdata(&rvdev->dev, rvdev);
-
-	ret = device_register(&rvdev->dev);
-	if (ret) {
-		put_device(&rvdev->dev);
-		return ERR_PTR(ret);
-	}
-
-	ret = copy_dma_range_map(&rvdev->dev, rproc->dev.parent);
-	if (ret)
-		goto free_rvdev;
-
-	/* Make device dma capable by inheriting from parent's capabilities */
-	set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
-
-	ret = dma_coerce_mask_and_coherent(&rvdev->dev,
-					   dma_get_mask(rproc->dev.parent));
-	if (ret) {
-		dev_warn(&rvdev->dev,
-			 "Failed to set DMA mask %llx. Trying to continue... (%pe)\n",
-			 dma_get_mask(rproc->dev.parent), ERR_PTR(ret));
-	}
-
-	/* parse the vrings */
-	for (i = 0; i < rsc->num_of_vrings; i++) {
-		ret = rproc_parse_vring(rvdev, rsc, i);
-		if (ret)
-			goto free_rvdev;
-	}
-
-	/* remember the resource offset*/
-	rvdev->rsc_offset = rvdev_data->rsc_offset;
-
-	/* allocate the vring resources */
-	for (i = 0; i < rsc->num_of_vrings; i++) {
-		ret = rproc_alloc_vring(rvdev, i);
-		if (ret)
-			goto unwind_vring_allocations;
-	}
-
-	rproc_add_rvdev(rproc, rvdev);
-
-	rvdev->subdev.start = rproc_vdev_do_start;
-	rvdev->subdev.stop = rproc_vdev_do_stop;
-
-	rproc_add_subdev(rproc, &rvdev->subdev);
-
-	return rvdev;
-
-unwind_vring_allocations:
-	for (i--; i >= 0; i--)
-		rproc_free_vring(&rvdev->vring[i]);
-free_rvdev:
-	device_unregister(&rvdev->dev);
-	return ERR_PTR(ret);
-}
-
-void rproc_vdev_release(struct kref *ref)
-{
-	struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
-	struct rproc_vring *rvring;
-	struct rproc *rproc = rvdev->rproc;
-	int id;
-
-	for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
-		rvring = &rvdev->vring[id];
-		rproc_free_vring(rvring);
-	}
-
-	rproc_remove_subdev(rproc, &rvdev->subdev);
-	rproc_remove_rvdev(rvdev);
-	device_unregister(&rvdev->dev);
-}
-
 /**
  * rproc_handle_vdev() - handle a vdev fw resource
  * @rproc: the remote processor
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 07c503de0f95..711b0e1f2118 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -41,14 +41,13 @@ struct rproc_vdev_data {
 
 /* from remoteproc_core.c */
 void rproc_release(struct kref *kref);
-irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
-void rproc_vdev_release(struct kref *ref);
 int rproc_of_parse_firmware(struct device *dev, int index,
 			    const char **fw_name);
 
 /* from remoteproc_virtio.c */
-int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id);
-int rproc_remove_virtio_dev(struct device *dev, void *data);
+struct rproc_vdev *rproc_rvdev_add_device(struct rproc *rproc, struct rproc_vdev_data *rvdev_data);
+irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
+void rproc_vdev_release(struct kref *ref);
 
 /* from remoteproc_debugfs.c */
 void rproc_remove_trace_file(struct dentry *tfile);
@@ -98,6 +97,7 @@ static inline void  rproc_char_device_remove(struct rproc *rproc)
 
 void rproc_free_vring(struct rproc_vring *rvring);
 int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
+int rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i);
 
 phys_addr_t rproc_va_to_pa(void *cpu_addr);
 int rproc_trigger_recovery(struct rproc *rproc);
@@ -110,6 +110,8 @@ struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
 						       const struct firmware *fw);
 struct rproc_mem_entry *
 rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...);
+void rproc_add_rvdev(struct rproc *rproc, struct rproc_vdev *rvdev);
+void rproc_remove_rvdev(struct rproc_vdev *rvdev);
 
 static inline int rproc_prepare_device(struct rproc *rproc)
 {
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 0f7706e23eb9..0aaa70d91aa8 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -9,7 +9,9 @@
  * Brian Swetland <swetland@google.com>
  */
 
+#include <linux/dma-direct.h>
 #include <linux/dma-map-ops.h>
+#include <linux/dma-mapping.h>
 #include <linux/export.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/remoteproc.h>
@@ -23,6 +25,25 @@
 
 #include "remoteproc_internal.h"
 
+static int copy_dma_range_map(struct device *to, struct device *from)
+{
+	const struct bus_dma_region *map = from->dma_range_map, *new_map, *r;
+	int num_ranges = 0;
+
+	if (!map)
+		return 0;
+
+	for (r = map; r->size; r++)
+		num_ranges++;
+
+	new_map = kmemdup(map, array_size(num_ranges + 1, sizeof(*map)),
+			  GFP_KERNEL);
+	if (!new_map)
+		return -ENOMEM;
+	to->dma_range_map = new_map;
+	return 0;
+}
+
 static struct rproc_vdev *vdev_to_rvdev(struct virtio_device *vdev)
 {
 	return container_of(vdev->dev.parent, struct rproc_vdev, dev);
@@ -341,7 +362,7 @@ static void rproc_virtio_dev_release(struct device *dev)
  *
  * Return: 0 on success or an appropriate error value otherwise
  */
-int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
+static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 {
 	struct rproc *rproc = rvdev->rproc;
 	struct device *dev = &rvdev->dev;
@@ -449,10 +470,139 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
  *
  * Return: 0
  */
-int rproc_remove_virtio_dev(struct device *dev, void *data)
+static int rproc_remove_virtio_dev(struct device *dev, void *data)
 {
 	struct virtio_device *vdev = dev_to_virtio(dev);
 
 	unregister_virtio_device(vdev);
 	return 0;
 }
+
+static int rproc_vdev_do_start(struct rproc_subdev *subdev)
+{
+	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
+
+	return rproc_add_virtio_dev(rvdev, rvdev->id);
+}
+
+static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
+{
+	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
+	int ret;
+
+	ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
+	if (ret)
+		dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n", ret);
+}
+
+/**
+ * rproc_rvdev_release() - release the existence of a rvdev
+ *
+ * @dev: the subdevice's dev
+ */
+static void rproc_rvdev_release(struct device *dev)
+{
+	struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
+
+	of_reserved_mem_device_release(dev);
+	dma_release_coherent_memory(dev);
+
+	kfree(rvdev);
+}
+
+struct rproc_vdev *
+rproc_rvdev_add_device(struct rproc *rproc, struct rproc_vdev_data *rvdev_data)
+{
+	struct rproc_vdev *rvdev;
+	struct fw_rsc_vdev *rsc = rvdev_data->rsc;
+	char name[16];
+	int i, ret;
+
+	rvdev = kzalloc(sizeof(*rvdev), GFP_KERNEL);
+	if (!rvdev)
+		return ERR_PTR(-ENOMEM);
+
+	kref_init(&rvdev->refcount);
+
+	rvdev->id = rvdev_data->id;
+	rvdev->rproc = rproc;
+	rvdev->index = rvdev_data->index;
+
+	/* Initialise vdev subdevice */
+	snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
+	rvdev->dev.parent = &rproc->dev;
+	rvdev->dev.release = rproc_rvdev_release;
+	dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
+	dev_set_drvdata(&rvdev->dev, rvdev);
+
+	ret = device_register(&rvdev->dev);
+	if (ret) {
+		put_device(&rvdev->dev);
+		return ERR_PTR(ret);
+	}
+
+	ret = copy_dma_range_map(&rvdev->dev, rproc->dev.parent);
+	if (ret)
+		goto free_rvdev;
+
+	/* Make device dma capable by inheriting from parent's capabilities */
+	set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
+
+	ret = dma_coerce_mask_and_coherent(&rvdev->dev,
+					   dma_get_mask(rproc->dev.parent));
+	if (ret) {
+		dev_warn(&rvdev->dev,
+			 "Failed to set DMA mask %llx. Trying to continue... (%pe)\n",
+			 dma_get_mask(rproc->dev.parent), ERR_PTR(ret));
+	}
+
+	/* parse the vrings */
+	for (i = 0; i < rsc->num_of_vrings; i++) {
+		ret = rproc_parse_vring(rvdev, rsc, i);
+		if (ret)
+			goto free_rvdev;
+	}
+
+	/* remember the resource offset*/
+	rvdev->rsc_offset = rvdev_data->rsc_offset;
+
+	/* allocate the vring resources */
+	for (i = 0; i < rsc->num_of_vrings; i++) {
+		ret = rproc_alloc_vring(rvdev, i);
+		if (ret)
+			goto unwind_vring_allocations;
+	}
+
+	rproc_add_rvdev(rproc, rvdev);
+
+	rvdev->subdev.start = rproc_vdev_do_start;
+	rvdev->subdev.stop = rproc_vdev_do_stop;
+
+	rproc_add_subdev(rproc, &rvdev->subdev);
+
+	return rvdev;
+
+unwind_vring_allocations:
+	for (i--; i >= 0; i--)
+		rproc_free_vring(&rvdev->vring[i]);
+free_rvdev:
+	device_unregister(&rvdev->dev);
+	return ERR_PTR(ret);
+}
+
+void rproc_vdev_release(struct kref *ref)
+{
+	struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
+	struct rproc_vring *rvring;
+	struct rproc *rproc = rvdev->rproc;
+	int id;
+
+	for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
+		rvring = &rvdev->vring[id];
+		rproc_free_vring(rvring);
+	}
+
+	rproc_remove_subdev(rproc, &rvdev->subdev);
+	rproc_remove_rvdev(rvdev);
+	device_unregister(&rvdev->dev);
+}
-- 
2.24.3

[PATCH v8 4/4] remoteproc: virtio: Create platform device for the remoteproc_virtio

[Arnaud Pouliquen]

Define a platform driver to manage the remoteproc virtio device as
a platform devices.

The platform device allows to pass rproc_vdev_data platform data to
specify properties that are stored in the rproc_vdev structure.

Such approach will allow to preserve legacy remoteproc virtio device
creation but also to probe the device using device tree mechanism.

remoteproc_virtio.c update:
  - Add rproc_virtio_driver platform driver. The probe ops replaces
    the rproc_rvdev_add_device function.
  - All reference to the rvdev->dev has been updated to rvdev-pdev->dev.
  - rproc_rvdev_release is removed as associated to the rvdev device.
  - The use of rvdev->kref counter is replaced by get/put_device on the
    remoteproc virtio platform device.
  - The vdev device no longer increments rproc device counter.
    increment/decrement is done in rproc_virtio_probe/rproc_virtio_remove
    function in charge of the vrings allocation/free.

remoteproc_core.c update:
  Migrate from the rvdev device to the rvdev platform device.
  From this patch, when a vdev resource is found in the resource table
  the remoteproc core register a platform device.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
Update vs previous revision:
 - add dma_release_coherent_memory call in rproc_virtio_remove

---
 drivers/remoteproc/remoteproc_core.c     |  12 +-
 drivers/remoteproc/remoteproc_internal.h |   2 -
 drivers/remoteproc/remoteproc_virtio.c   | 143 ++++++++++++-----------
 include/linux/remoteproc.h               |   6 +-
 4 files changed, 82 insertions(+), 81 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index a556504a4851..52e724b274c9 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -479,6 +479,7 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 	struct device *dev = &rproc->dev;
 	struct rproc_vdev *rvdev;
 	struct rproc_vdev_data rvdev_data;
+	struct platform_device *pdev;
 
 	/* make sure resource isn't truncated */
 	if (struct_size(rsc, vring, rsc->num_of_vrings) + rsc->config_len >
@@ -507,9 +508,12 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 	rvdev_data.rsc_offset = offset;
 	rvdev_data.rsc = rsc;
 
-	rvdev = rproc_rvdev_add_device(rproc, &rvdev_data);
-	if (IS_ERR(rvdev))
-		return PTR_ERR(rvdev);
+	pdev = platform_device_register_data(dev, "rproc-virtio", rvdev_data.index, &rvdev_data,
+					     sizeof(rvdev_data));
+	if (IS_ERR(pdev)) {
+		dev_err(dev, "failed to create rproc-virtio device\n");
+		return PTR_ERR(pdev);
+	}
 
 	return 0;
 }
@@ -1245,7 +1249,7 @@ void rproc_resource_cleanup(struct rproc *rproc)
 
 	/* clean up remote vdev entries */
 	list_for_each_entry_safe(rvdev, rvtmp, &rproc->rvdevs, node)
-		kref_put(&rvdev->refcount, rproc_vdev_release);
+		platform_device_unregister(rvdev->pdev);
 
 	rproc_coredump_cleanup(rproc);
 }
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 711b0e1f2118..bf1fb7bba1a3 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -45,9 +45,7 @@ int rproc_of_parse_firmware(struct device *dev, int index,
 			    const char **fw_name);
 
 /* from remoteproc_virtio.c */
-struct rproc_vdev *rproc_rvdev_add_device(struct rproc *rproc, struct rproc_vdev_data *rvdev_data);
 irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
-void rproc_vdev_release(struct kref *ref);
 
 /* from remoteproc_debugfs.c */
 void rproc_remove_trace_file(struct dentry *tfile);
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 0aaa70d91aa8..a29e3b8ff69c 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -13,6 +13,7 @@
 #include <linux/dma-map-ops.h>
 #include <linux/dma-mapping.h>
 #include <linux/export.h>
+#include <linux/of_platform.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/remoteproc.h>
 #include <linux/virtio.h>
@@ -46,7 +47,11 @@ static int copy_dma_range_map(struct device *to, struct device *from)
 
 static struct rproc_vdev *vdev_to_rvdev(struct virtio_device *vdev)
 {
-	return container_of(vdev->dev.parent, struct rproc_vdev, dev);
+	struct platform_device *pdev;
+
+	pdev = container_of(vdev->dev.parent, struct platform_device, dev);
+
+	return platform_get_drvdata(pdev);
 }
 
 static  struct rproc *vdev_to_rproc(struct virtio_device *vdev)
@@ -343,13 +348,10 @@ static void rproc_virtio_dev_release(struct device *dev)
 {
 	struct virtio_device *vdev = dev_to_virtio(dev);
 	struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
-	struct rproc *rproc = vdev_to_rproc(vdev);
 
 	kfree(vdev);
 
-	kref_put(&rvdev->refcount, rproc_vdev_release);
-
-	put_device(&rproc->dev);
+	put_device(&rvdev->pdev->dev);
 }
 
 /**
@@ -365,7 +367,7 @@ static void rproc_virtio_dev_release(struct device *dev)
 static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 {
 	struct rproc *rproc = rvdev->rproc;
-	struct device *dev = &rvdev->dev;
+	struct device *dev = &rvdev->pdev->dev;
 	struct virtio_device *vdev;
 	struct rproc_mem_entry *mem;
 	int ret;
@@ -435,18 +437,8 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 	vdev->dev.parent = dev;
 	vdev->dev.release = rproc_virtio_dev_release;
 
-	/*
-	 * We're indirectly making a non-temporary copy of the rproc pointer
-	 * here, because drivers probed with this vdev will indirectly
-	 * access the wrapping rproc.
-	 *
-	 * Therefore we must increment the rproc refcount here, and decrement
-	 * it _only_ when the vdev is released.
-	 */
-	get_device(&rproc->dev);
-
 	/* Reference the vdev and vring allocations */
-	kref_get(&rvdev->refcount);
+	get_device(dev);
 
 	ret = register_virtio_device(vdev);
 	if (ret) {
@@ -488,79 +480,57 @@ static int rproc_vdev_do_start(struct rproc_subdev *subdev)
 static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
 {
 	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
+	struct device *dev = &rvdev->pdev->dev;
 	int ret;
 
-	ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
+	ret = device_for_each_child(dev, NULL, rproc_remove_virtio_dev);
 	if (ret)
-		dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n", ret);
+		dev_warn(dev, "can't remove vdev child device: %d\n", ret);
 }
 
-/**
- * rproc_rvdev_release() - release the existence of a rvdev
- *
- * @dev: the subdevice's dev
- */
-static void rproc_rvdev_release(struct device *dev)
-{
-	struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
-
-	of_reserved_mem_device_release(dev);
-	dma_release_coherent_memory(dev);
-
-	kfree(rvdev);
-}
-
-struct rproc_vdev *
-rproc_rvdev_add_device(struct rproc *rproc, struct rproc_vdev_data *rvdev_data)
+static int rproc_virtio_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
+	struct rproc_vdev_data *rvdev_data = dev->platform_data;
 	struct rproc_vdev *rvdev;
-	struct fw_rsc_vdev *rsc = rvdev_data->rsc;
-	char name[16];
+	struct rproc *rproc = container_of(dev->parent, struct rproc, dev);
+	struct fw_rsc_vdev *rsc;
 	int i, ret;
 
-	rvdev = kzalloc(sizeof(*rvdev), GFP_KERNEL);
-	if (!rvdev)
-		return ERR_PTR(-ENOMEM);
+	if (!rvdev_data)
+		return -EINVAL;
 
-	kref_init(&rvdev->refcount);
+	rvdev = devm_kzalloc(dev, sizeof(*rvdev), GFP_KERNEL);
+	if (!rvdev)
+		return -ENOMEM;
 
 	rvdev->id = rvdev_data->id;
 	rvdev->rproc = rproc;
 	rvdev->index = rvdev_data->index;
 
-	/* Initialise vdev subdevice */
-	snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
-	rvdev->dev.parent = &rproc->dev;
-	rvdev->dev.release = rproc_rvdev_release;
-	dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
-	dev_set_drvdata(&rvdev->dev, rvdev);
-
-	ret = device_register(&rvdev->dev);
-	if (ret) {
-		put_device(&rvdev->dev);
-		return ERR_PTR(ret);
-	}
-
-	ret = copy_dma_range_map(&rvdev->dev, rproc->dev.parent);
+	ret = copy_dma_range_map(dev, rproc->dev.parent);
 	if (ret)
-		goto free_rvdev;
+		return ret;
 
 	/* Make device dma capable by inheriting from parent's capabilities */
-	set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
+	set_dma_ops(dev, get_dma_ops(rproc->dev.parent));
 
-	ret = dma_coerce_mask_and_coherent(&rvdev->dev,
-					   dma_get_mask(rproc->dev.parent));
+	ret = dma_coerce_mask_and_coherent(dev, dma_get_mask(rproc->dev.parent));
 	if (ret) {
-		dev_warn(&rvdev->dev,
-			 "Failed to set DMA mask %llx. Trying to continue... (%pe)\n",
+		dev_warn(dev, "Failed to set DMA mask %llx. Trying to continue... (%pe)\n",
 			 dma_get_mask(rproc->dev.parent), ERR_PTR(ret));
 	}
 
+	platform_set_drvdata(pdev, rvdev);
+	rvdev->pdev = pdev;
+
+	rsc = rvdev_data->rsc;
+
 	/* parse the vrings */
 	for (i = 0; i < rsc->num_of_vrings; i++) {
 		ret = rproc_parse_vring(rvdev, rsc, i);
 		if (ret)
-			goto free_rvdev;
+			return ret;
 	}
 
 	/* remember the resource offset*/
@@ -580,21 +550,30 @@ rproc_rvdev_add_device(struct rproc *rproc, struct rproc_vdev_data *rvdev_data)
 
 	rproc_add_subdev(rproc, &rvdev->subdev);
 
-	return rvdev;
+	/*
+	 * We're indirectly making a non-temporary copy of the rproc pointer
+	 * here, because the platform device or the vdev device will indirectly
+	 * access the wrapping rproc.
+	 *
+	 * Therefore we must increment the rproc refcount here, and decrement
+	 * it _only_ on platform remove.
+	 */
+	get_device(&rproc->dev);
+
+	return 0;
 
 unwind_vring_allocations:
 	for (i--; i >= 0; i--)
 		rproc_free_vring(&rvdev->vring[i]);
-free_rvdev:
-	device_unregister(&rvdev->dev);
-	return ERR_PTR(ret);
+
+	return ret;
 }
 
-void rproc_vdev_release(struct kref *ref)
+static int rproc_virtio_remove(struct platform_device *pdev)
 {
-	struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
-	struct rproc_vring *rvring;
+	struct rproc_vdev *rvdev = dev_get_drvdata(&pdev->dev);
 	struct rproc *rproc = rvdev->rproc;
+	struct rproc_vring *rvring;
 	int id;
 
 	for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
@@ -604,5 +583,27 @@ void rproc_vdev_release(struct kref *ref)
 
 	rproc_remove_subdev(rproc, &rvdev->subdev);
 	rproc_remove_rvdev(rvdev);
-	device_unregister(&rvdev->dev);
+
+	of_reserved_mem_device_release(&pdev->dev);
+	dma_release_coherent_memory(&pdev->dev);
+
+	put_device(&rproc->dev);
+
+	return 0;
 }
+
+/* Platform driver */
+static const struct of_device_id rproc_virtio_match[] = {
+	{ .compatible = "virtio,rproc" },
+	{},
+};
+
+static struct platform_driver rproc_virtio_driver = {
+	.probe		= rproc_virtio_probe,
+	.remove		= rproc_virtio_remove,
+	.driver		= {
+		.name	= "rproc-virtio",
+		.of_match_table	= rproc_virtio_match,
+	},
+};
+builtin_platform_driver(rproc_virtio_driver);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index aea79c77db0f..1abf56ad02da 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -616,9 +616,8 @@ struct rproc_vring {
 
 /**
  * struct rproc_vdev - remoteproc state for a supported virtio device
- * @refcount: reference counter for the vdev and vring allocations
  * @subdev: handle for registering the vdev as a rproc subdevice
- * @dev: device struct used for reference count semantics
+ * @pdev: remoteproc virtio platform device
  * @id: virtio device id (as in virtio_ids.h)
  * @node: list node
  * @rproc: the rproc handle
@@ -627,10 +626,9 @@ struct rproc_vring {
  * @index: vdev position versus other vdev declared in resource table
  */
 struct rproc_vdev {
-	struct kref refcount;
 
 	struct rproc_subdev subdev;
-	struct device dev;
+	struct platform_device *pdev;
 
 	unsigned int id;
 	struct list_head node;
-- 
2.24.3

Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Mathieu Poirier]

Hi,

On Fri, Aug 26, 2022 at 01:52:28PM +0200, Arnaud Pouliquen wrote:
> 1) Update from V7 [1]:
> 
> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc: imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
>   The updates take into account the integration of the
>   commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after rproc_shutdown")
> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> according to reviews on V7
> 
> 
> [1] https://lkml.org/lkml/2022/7/13/663
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/log/?h=for-next
> 
> 2) Patchset description:
> 
> This series is a part of the work initiated a long time ago in 
> the series "remoteproc: Decorelate virtio from core"[3]
> 
> Objective of the work:
> - Update the remoteproc VirtIO device creation (use platform device)
> - Allow to declare remoteproc VirtIO device in DT
>     - declare resources associated to a remote proc VirtIO
>     - declare a list of VirtIO supported by the platform.
> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
>   For instance be able to declare a I2C device in a virtio-i2C node.
> - Keep the legacy working!
> - Try to improve the picture about concerns reported by Christoph Hellwing [4][5]
> 
> [3] https://lkml.org/lkml/2020/4/16/1817
> [4] https://lkml.org/lkml/2021/6/23/607
> [5] https://patchwork.kernel.org/project/linux-remoteproc/patch/AOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E@cp7-web-042.plabs.ch/
> 
> In term of device tree this would result in such hierarchy (stm32mp1 example with 2 virtio RPMSG):
> 
> 	m4_rproc: m4@10000000 {
> 		compatible = "st,stm32mp1-m4";
> 		reg = <0x10000000 0x40000>,
> 		      <0x30000000 0x40000>,
> 		      <0x38000000 0x10000>;
>         memory-region = <&retram>, <&mcuram>,<&mcuram2>;
>         mboxes = <&ipcc 2>, <&ipcc 3>;
>         mbox-names = "shutdown", "detach";
>         status = "okay";
> 
>         #address-cells = <1>;
>         #size-cells = <0>;
>         
>         vdev@0 {
> 		compatible = "rproc-virtio";
> 		reg = <0>;
> 		virtio,id = <7>;  /* RPMSG */
> 		memory-region = <&vdev0vring0>, <&vdev0vring1>, <&vdev0buffer>;
> 		mboxes = <&ipcc 0>, <&ipcc 1>;
> 		mbox-names = "vq0", "vq1";
> 		status = "okay";
>         };
> 
>         vdev@1 {
> 		compatible = "rproc-virtio";
> 		reg = <1>;
> 		virtio,id = <7>;  /*RPMSG */
> 		memory-region = <&vdev1vring0>, <&vdev1vring1>, <&vdev1buffer>;
> 		mboxes = <&ipcc 4>, <&ipcc 5>;
> 		mbox-names = "vq0", "vq1";
> 		status = "okay";
>         };
> };

I was in the process of applying this set when the last patch gave me a
checkpatch warning about "virtio,rproc" not being documented.

I suggest to introduce a new "virtio-rproc.yaml" based on this work[1], with the
above in the example sections.

Thanks,
Mathieu

[1]. https://elixir.bootlin.com/linux/v6.0-rc6/source/Documentation/devicetree/bindings/virtio/virtio-device.yaml


> 
> I have divided the work in 4 steps to simplify the review, This series implements only
> the step 1:
> step 1: Redefine the remoteproc VirtIO device as a platform device
>   - migrate rvdev management in remoteproc virtio.c,
>   - create a remotproc virtio config ( can be disabled for platform that not use VirtIO IPC.
> step 2: Add possibility to declare and probe a VirtIO sub node
>   - VirtIO bindings declaration,
>   - multi DT VirtIO devices support,
>   - introduction of a remote proc virtio bind device mechanism ,
> => https://github.com/arnopo/linux/commits/step2-virtio-in-DT
> step 3: Add memory declaration in VirtIO subnode
> => https://github.com/arnopo/linux/commits/step3-virtio-memories
> step 4: Add mailbox declaration in VirtIO subnode
> => https://github.com/arnopo/linux/commits/step4-virtio-mailboxes
> 
> Arnaud Pouliquen (4):
>   remoteproc: core: Introduce rproc_rvdev_add_device function
>   remoteproc: core: Introduce rproc_add_rvdev function
>   remoteproc: Move rproc_vdev management to remoteproc_virtio.c
>   remoteproc: virtio: Create platform device for the remoteproc_virtio
> 
>  drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
>  drivers/remoteproc/remoteproc_internal.h |  23 ++-
>  drivers/remoteproc/remoteproc_virtio.c   | 189 ++++++++++++++++++++---
>  include/linux/remoteproc.h               |   6 +-
>  4 files changed, 210 insertions(+), 162 deletions(-)
> 
> -- 
> 2.24.3
> 

Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Arnaud POULIQUEN]

Hi Mathieu,

On 9/20/22 00:30, Mathieu Poirier wrote:
> Hi,
> 
> On Fri, Aug 26, 2022 at 01:52:28PM +0200, Arnaud Pouliquen wrote:
>> 1) Update from V7 [1]:
>>
>> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc: imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
>>   The updates take into account the integration of the
>>   commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after rproc_shutdown")
>> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> according to reviews on V7
>>
>>
>> [1] https://lkml.org/lkml/2022/7/13/663
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/log/?h=for-next
>>
>> 2) Patchset description:
>>
>> This series is a part of the work initiated a long time ago in 
>> the series "remoteproc: Decorelate virtio from core"[3]
>>
>> Objective of the work:
>> - Update the remoteproc VirtIO device creation (use platform device)
>> - Allow to declare remoteproc VirtIO device in DT
>>     - declare resources associated to a remote proc VirtIO
>>     - declare a list of VirtIO supported by the platform.
>> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
>>   For instance be able to declare a I2C device in a virtio-i2C node.
>> - Keep the legacy working!
>> - Try to improve the picture about concerns reported by Christoph Hellwing [4][5]
>>
>> [3] https://lkml.org/lkml/2020/4/16/1817
>> [4] https://lkml.org/lkml/2021/6/23/607
>> [5] https://patchwork.kernel.org/project/linux-remoteproc/patch/AOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E@cp7-web-042.plabs.ch/
>>
>> In term of device tree this would result in such hierarchy (stm32mp1 example with 2 virtio RPMSG):
>>
>> 	m4_rproc: m4@10000000 {
>> 		compatible = "st,stm32mp1-m4";
>> 		reg = <0x10000000 0x40000>,
>> 		      <0x30000000 0x40000>,
>> 		      <0x38000000 0x10000>;
>>         memory-region = <&retram>, <&mcuram>,<&mcuram2>;
>>         mboxes = <&ipcc 2>, <&ipcc 3>;
>>         mbox-names = "shutdown", "detach";
>>         status = "okay";
>>
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>>         
>>         vdev@0 {
>> 		compatible = "rproc-virtio";
>> 		reg = <0>;
>> 		virtio,id = <7>;  /* RPMSG */
>> 		memory-region = <&vdev0vring0>, <&vdev0vring1>, <&vdev0buffer>;
>> 		mboxes = <&ipcc 0>, <&ipcc 1>;
>> 		mbox-names = "vq0", "vq1";
>> 		status = "okay";
>>         };
>>
>>         vdev@1 {
>> 		compatible = "rproc-virtio";
>> 		reg = <1>;
>> 		virtio,id = <7>;  /*RPMSG */
>> 		memory-region = <&vdev1vring0>, <&vdev1vring1>, <&vdev1buffer>;
>> 		mboxes = <&ipcc 4>, <&ipcc 5>;
>> 		mbox-names = "vq0", "vq1";
>> 		status = "okay";
>>         };
>> };
> 
> I was in the process of applying this set when the last patch gave me a
> checkpatch warning about "virtio,rproc" not being documented.
> 
> I suggest to introduce a new "virtio-rproc.yaml" based on this work[1], with the
> above in the example sections.

Yes I saw the warning, but for this first series it is not possible to declare
the associated "rproc-virtio" device  in device tree.
So at this step it seems not make senses to create the devicetree bindings file.
More than that I don't know how I could justify the properties in bindings if
there is not driver code associated.

So i would be in favor of not adding the bindings in this series but to define
bindings in the first patch of my "step 2" series; as done on my github:
https://github.com/arnopo/linux/commit/9616d89a4f478cf78865a244efcde108d900f69f

Please let me know your preference.

Regards,
Arnaud


> 
> Thanks,
> Mathieu
> 
> [1]. https://elixir.bootlin.com/linux/v6.0-rc6/source/Documentation/devicetree/bindings/virtio/virtio-device.yaml
> 
> 
>>
>> I have divided the work in 4 steps to simplify the review, This series implements only
>> the step 1:
>> step 1: Redefine the remoteproc VirtIO device as a platform device
>>   - migrate rvdev management in remoteproc virtio.c,
>>   - create a remotproc virtio config ( can be disabled for platform that not use VirtIO IPC.
>> step 2: Add possibility to declare and probe a VirtIO sub node
>>   - VirtIO bindings declaration,
>>   - multi DT VirtIO devices support,
>>   - introduction of a remote proc virtio bind device mechanism ,
>> => https://github.com/arnopo/linux/commits/step2-virtio-in-DT
>> step 3: Add memory declaration in VirtIO subnode
>> => https://github.com/arnopo/linux/commits/step3-virtio-memories
>> step 4: Add mailbox declaration in VirtIO subnode
>> => https://github.com/arnopo/linux/commits/step4-virtio-mailboxes
>>
>> Arnaud Pouliquen (4):
>>   remoteproc: core: Introduce rproc_rvdev_add_device function
>>   remoteproc: core: Introduce rproc_add_rvdev function
>>   remoteproc: Move rproc_vdev management to remoteproc_virtio.c
>>   remoteproc: virtio: Create platform device for the remoteproc_virtio
>>
>>  drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
>>  drivers/remoteproc/remoteproc_internal.h |  23 ++-
>>  drivers/remoteproc/remoteproc_virtio.c   | 189 ++++++++++++++++++++---
>>  include/linux/remoteproc.h               |   6 +-
>>  4 files changed, 210 insertions(+), 162 deletions(-)
>>
>> -- 
>> 2.24.3
>>

Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Mathieu Poirier]

On Tue, Sep 20, 2022 at 03:44:18PM +0200, Arnaud POULIQUEN wrote:
> Hi Mathieu,
> 
> On 9/20/22 00:30, Mathieu Poirier wrote:
> > Hi,
> > 
> > On Fri, Aug 26, 2022 at 01:52:28PM +0200, Arnaud Pouliquen wrote:
> >> 1) Update from V7 [1]:
> >>
> >> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc: imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
> >>   The updates take into account the integration of the
> >>   commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after rproc_shutdown")
> >> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> according to reviews on V7
> >>
> >>
> >> [1] https://lkml.org/lkml/2022/7/13/663
> >> [2] https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/log/?h=for-next
> >>
> >> 2) Patchset description:
> >>
> >> This series is a part of the work initiated a long time ago in 
> >> the series "remoteproc: Decorelate virtio from core"[3]
> >>
> >> Objective of the work:
> >> - Update the remoteproc VirtIO device creation (use platform device)
> >> - Allow to declare remoteproc VirtIO device in DT
> >>     - declare resources associated to a remote proc VirtIO
> >>     - declare a list of VirtIO supported by the platform.
> >> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
> >>   For instance be able to declare a I2C device in a virtio-i2C node.
> >> - Keep the legacy working!
> >> - Try to improve the picture about concerns reported by Christoph Hellwing [4][5]
> >>
> >> [3] https://lkml.org/lkml/2020/4/16/1817
> >> [4] https://lkml.org/lkml/2021/6/23/607
> >> [5] https://patchwork.kernel.org/project/linux-remoteproc/patch/AOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E@cp7-web-042.plabs.ch/
> >>
> >> In term of device tree this would result in such hierarchy (stm32mp1 example with 2 virtio RPMSG):
> >>
> >> 	m4_rproc: m4@10000000 {
> >> 		compatible = "st,stm32mp1-m4";
> >> 		reg = <0x10000000 0x40000>,
> >> 		      <0x30000000 0x40000>,
> >> 		      <0x38000000 0x10000>;
> >>         memory-region = <&retram>, <&mcuram>,<&mcuram2>;
> >>         mboxes = <&ipcc 2>, <&ipcc 3>;
> >>         mbox-names = "shutdown", "detach";
> >>         status = "okay";
> >>
> >>         #address-cells = <1>;
> >>         #size-cells = <0>;
> >>         
> >>         vdev@0 {
> >> 		compatible = "rproc-virtio";
> >> 		reg = <0>;
> >> 		virtio,id = <7>;  /* RPMSG */
> >> 		memory-region = <&vdev0vring0>, <&vdev0vring1>, <&vdev0buffer>;
> >> 		mboxes = <&ipcc 0>, <&ipcc 1>;
> >> 		mbox-names = "vq0", "vq1";
> >> 		status = "okay";
> >>         };
> >>
> >>         vdev@1 {
> >> 		compatible = "rproc-virtio";
> >> 		reg = <1>;
> >> 		virtio,id = <7>;  /*RPMSG */
> >> 		memory-region = <&vdev1vring0>, <&vdev1vring1>, <&vdev1buffer>;
> >> 		mboxes = <&ipcc 4>, <&ipcc 5>;
> >> 		mbox-names = "vq0", "vq1";
> >> 		status = "okay";
> >>         };
> >> };
> > 
> > I was in the process of applying this set when the last patch gave me a
> > checkpatch warning about "virtio,rproc" not being documented.
> > 
> > I suggest to introduce a new "virtio-rproc.yaml" based on this work[1], with the
> > above in the example sections.
> 
> Yes I saw the warning, but for this first series it is not possible to declare
> the associated "rproc-virtio" device  in device tree.

I understand and agree with your position.

I am going ahead and merging this set in order for it to get some exposure in
linux-next.  That said be on the ready to address potential problems it may
cause.

> So at this step it seems not make senses to create the devicetree bindings file.
> More than that I don't know how I could justify the properties in bindings if
> there is not driver code associated.
> 
> So i would be in favor of not adding the bindings in this series but to define
> bindings in the first patch of my "step 2" series; as done on my github:
> https://github.com/arnopo/linux/commit/9616d89a4f478cf78865a244efcde108d900f69f
> 
> Please let me know your preference.
> 
> Regards,
> Arnaud
> 
> 
> > 
> > Thanks,
> > Mathieu
> > 
> > [1]. https://elixir.bootlin.com/linux/v6.0-rc6/source/Documentation/devicetree/bindings/virtio/virtio-device.yaml
> > 
> > 
> >>
> >> I have divided the work in 4 steps to simplify the review, This series implements only
> >> the step 1:
> >> step 1: Redefine the remoteproc VirtIO device as a platform device
> >>   - migrate rvdev management in remoteproc virtio.c,
> >>   - create a remotproc virtio config ( can be disabled for platform that not use VirtIO IPC.
> >> step 2: Add possibility to declare and probe a VirtIO sub node
> >>   - VirtIO bindings declaration,
> >>   - multi DT VirtIO devices support,
> >>   - introduction of a remote proc virtio bind device mechanism ,
> >> => https://github.com/arnopo/linux/commits/step2-virtio-in-DT
> >> step 3: Add memory declaration in VirtIO subnode
> >> => https://github.com/arnopo/linux/commits/step3-virtio-memories
> >> step 4: Add mailbox declaration in VirtIO subnode
> >> => https://github.com/arnopo/linux/commits/step4-virtio-mailboxes
> >>
> >> Arnaud Pouliquen (4):
> >>   remoteproc: core: Introduce rproc_rvdev_add_device function
> >>   remoteproc: core: Introduce rproc_add_rvdev function
> >>   remoteproc: Move rproc_vdev management to remoteproc_virtio.c
> >>   remoteproc: virtio: Create platform device for the remoteproc_virtio
> >>
> >>  drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
> >>  drivers/remoteproc/remoteproc_internal.h |  23 ++-
> >>  drivers/remoteproc/remoteproc_virtio.c   | 189 ++++++++++++++++++++---
> >>  include/linux/remoteproc.h               |   6 +-
> >>  4 files changed, 210 insertions(+), 162 deletions(-)
> >>
> >> -- 
> >> 2.24.3
> >>

Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Mathieu Poirier]

On Tue, Sep 20, 2022 at 02:22:01PM -0600, Mathieu Poirier wrote:
> On Tue, Sep 20, 2022 at 03:44:18PM +0200, Arnaud POULIQUEN wrote:
> > Hi Mathieu,
> > 
> > On 9/20/22 00:30, Mathieu Poirier wrote:
> > > Hi,
> > > 
> > > On Fri, Aug 26, 2022 at 01:52:28PM +0200, Arnaud Pouliquen wrote:
> > >> 1) Update from V7 [1]:
> > >>
> > >> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc: imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
> > >>   The updates take into account the integration of the
> > >>   commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after rproc_shutdown")
> > >> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> according to reviews on V7
> > >>
> > >>
> > >> [1] https://lkml.org/lkml/2022/7/13/663
> > >> [2] https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/log/?h=for-next
> > >>
> > >> 2) Patchset description:
> > >>
> > >> This series is a part of the work initiated a long time ago in 
> > >> the series "remoteproc: Decorelate virtio from core"[3]
> > >>
> > >> Objective of the work:
> > >> - Update the remoteproc VirtIO device creation (use platform device)
> > >> - Allow to declare remoteproc VirtIO device in DT
> > >>     - declare resources associated to a remote proc VirtIO
> > >>     - declare a list of VirtIO supported by the platform.
> > >> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
> > >>   For instance be able to declare a I2C device in a virtio-i2C node.
> > >> - Keep the legacy working!
> > >> - Try to improve the picture about concerns reported by Christoph Hellwing [4][5]
> > >>
> > >> [3] https://lkml.org/lkml/2020/4/16/1817
> > >> [4] https://lkml.org/lkml/2021/6/23/607
> > >> [5] https://patchwork.kernel.org/project/linux-remoteproc/patch/AOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E@cp7-web-042.plabs.ch/
> > >>
> > >> In term of device tree this would result in such hierarchy (stm32mp1 example with 2 virtio RPMSG):
> > >>
> > >> 	m4_rproc: m4@10000000 {
> > >> 		compatible = "st,stm32mp1-m4";
> > >> 		reg = <0x10000000 0x40000>,
> > >> 		      <0x30000000 0x40000>,
> > >> 		      <0x38000000 0x10000>;
> > >>         memory-region = <&retram>, <&mcuram>,<&mcuram2>;
> > >>         mboxes = <&ipcc 2>, <&ipcc 3>;
> > >>         mbox-names = "shutdown", "detach";
> > >>         status = "okay";
> > >>
> > >>         #address-cells = <1>;
> > >>         #size-cells = <0>;
> > >>         
> > >>         vdev@0 {
> > >> 		compatible = "rproc-virtio";
> > >> 		reg = <0>;
> > >> 		virtio,id = <7>;  /* RPMSG */
> > >> 		memory-region = <&vdev0vring0>, <&vdev0vring1>, <&vdev0buffer>;
> > >> 		mboxes = <&ipcc 0>, <&ipcc 1>;
> > >> 		mbox-names = "vq0", "vq1";
> > >> 		status = "okay";
> > >>         };
> > >>
> > >>         vdev@1 {
> > >> 		compatible = "rproc-virtio";
> > >> 		reg = <1>;
> > >> 		virtio,id = <7>;  /*RPMSG */
> > >> 		memory-region = <&vdev1vring0>, <&vdev1vring1>, <&vdev1buffer>;
> > >> 		mboxes = <&ipcc 4>, <&ipcc 5>;
> > >> 		mbox-names = "vq0", "vq1";
> > >> 		status = "okay";
> > >>         };
> > >> };
> > > 
> > > I was in the process of applying this set when the last patch gave me a
> > > checkpatch warning about "virtio,rproc" not being documented.
> > > 
> > > I suggest to introduce a new "virtio-rproc.yaml" based on this work[1], with the
> > > above in the example sections.
> > 
> > Yes I saw the warning, but for this first series it is not possible to declare
> > the associated "rproc-virtio" device  in device tree.
> 
> I understand and agree with your position.
> 
> I am going ahead and merging this set in order for it to get some exposure in
> linux-next.  That said be on the ready to address potential problems it may
> cause.

I am getting conflicts because of the patches previously applied to rproc-next.
Please resent a series that applies to "7d7f8fe4e399" and I'll move forward with
the merge.

> 
> > So at this step it seems not make senses to create the devicetree bindings file.
> > More than that I don't know how I could justify the properties in bindings if
> > there is not driver code associated.
> > 
> > So i would be in favor of not adding the bindings in this series but to define
> > bindings in the first patch of my "step 2" series; as done on my github:
> > https://github.com/arnopo/linux/commit/9616d89a4f478cf78865a244efcde108d900f69f
> > 
> > Please let me know your preference.
> > 
> > Regards,
> > Arnaud
> > 
> > 
> > > 
> > > Thanks,
> > > Mathieu
> > > 
> > > [1]. https://elixir.bootlin.com/linux/v6.0-rc6/source/Documentation/devicetree/bindings/virtio/virtio-device.yaml
> > > 
> > > 
> > >>
> > >> I have divided the work in 4 steps to simplify the review, This series implements only
> > >> the step 1:
> > >> step 1: Redefine the remoteproc VirtIO device as a platform device
> > >>   - migrate rvdev management in remoteproc virtio.c,
> > >>   - create a remotproc virtio config ( can be disabled for platform that not use VirtIO IPC.
> > >> step 2: Add possibility to declare and probe a VirtIO sub node
> > >>   - VirtIO bindings declaration,
> > >>   - multi DT VirtIO devices support,
> > >>   - introduction of a remote proc virtio bind device mechanism ,
> > >> => https://github.com/arnopo/linux/commits/step2-virtio-in-DT
> > >> step 3: Add memory declaration in VirtIO subnode
> > >> => https://github.com/arnopo/linux/commits/step3-virtio-memories
> > >> step 4: Add mailbox declaration in VirtIO subnode
> > >> => https://github.com/arnopo/linux/commits/step4-virtio-mailboxes
> > >>
> > >> Arnaud Pouliquen (4):
> > >>   remoteproc: core: Introduce rproc_rvdev_add_device function
> > >>   remoteproc: core: Introduce rproc_add_rvdev function
> > >>   remoteproc: Move rproc_vdev management to remoteproc_virtio.c
> > >>   remoteproc: virtio: Create platform device for the remoteproc_virtio
> > >>
> > >>  drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
> > >>  drivers/remoteproc/remoteproc_internal.h |  23 ++-
> > >>  drivers/remoteproc/remoteproc_virtio.c   | 189 ++++++++++++++++++++---
> > >>  include/linux/remoteproc.h               |   6 +-
> > >>  4 files changed, 210 insertions(+), 162 deletions(-)
> > >>
> > >> -- 
> > >> 2.24.3
> > >>

RE: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Peng Fan]

Hi Arnaud,

> Subject: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO
> device

Sorry to get in at this late time, just try to catch up.
Not reviewing comments, just have a question,
Does remote core firmware requires changes to use this new feature?
Does your 4 branches listed below still work with linux-6.x?
Could the multiple vdev still share same mbox channel?

I not own i.MX remote core firmware development, so if no need
firmware change, I would like give a try and see how it works.

Thanks,
Peng.

> 
> 1) Update from V7 [1]:
> 
> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc:
> imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
>   The updates take into account the integration of the
>   commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after
> rproc_shutdown")
> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> according
> to reviews on V7
> 
> 
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
> org%2Flkml%2F2022%2F7%2F13%2F663&amp;data=05%7C01%7Cpeng.fan%
> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=77pEuwAI7Lh61hx1%2B
> Hs79Cu0G5KOa6mzQ0PnTC5r8Xk%3D&amp;reserved=0
> [2]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ke
> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fremoteproc%2Flinux.g
> it%2Flog%2F%3Fh%3Dfor-
> next&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e7
> 439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
> C%7C&amp;sdata=iWUSzKkN9BpHwqbO62awIcyVf9PXcftcdt2kytWVR78%3D
> &amp;reserved=0
> 
> 2) Patchset description:
> 
> This series is a part of the work initiated a long time ago in the series
> "remoteproc: Decorelate virtio from core"[3]
> 
> Objective of the work:
> - Update the remoteproc VirtIO device creation (use platform device)
> - Allow to declare remoteproc VirtIO device in DT
>     - declare resources associated to a remote proc VirtIO
>     - declare a list of VirtIO supported by the platform.
> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
>   For instance be able to declare a I2C device in a virtio-i2C node.
> - Keep the legacy working!
> - Try to improve the picture about concerns reported by Christoph Hellwing
> [4][5]
> 
> [3]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
> org%2Flkml%2F2020%2F4%2F16%2F1817&amp;data=05%7C01%7Cpeng.fan
> %40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4
> c6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw
> iLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=oPWSfUweLdhUFK5X9
> 2YcGHem8s%2Bfelcr%2FHx9JAlKG%2BI%3D&amp;reserved=0
> [4]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
> org%2Flkml%2F2021%2F6%2F23%2F607&amp;data=05%7C01%7Cpeng.fan%
> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=HPpnlaykes8R1Kz1dEN
> nirEHkDNr7JvRs%2FcsaDPuLdI%3D&amp;reserved=0
> [5]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> hwork.kernel.org%2Fproject%2Flinux-
> remoteproc%2Fpatch%2FAOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E
> %40cp7-web-
> 042.plabs.ch%2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e520
> 0d739a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> 3000%7C%7C%7C&amp;sdata=GtNruefDreOoogL%2BlntAC7GBfk6E1Goq4j%
> 2BYXt36RdI%3D&amp;reserved=0
> 
> In term of device tree this would result in such hierarchy (stm32mp1
> example with 2 virtio RPMSG):
> 
> 	m4_rproc: m4@10000000 {
> 		compatible = "st,stm32mp1-m4";
> 		reg = <0x10000000 0x40000>,
> 		      <0x30000000 0x40000>,
> 		      <0x38000000 0x10000>;
>         memory-region = <&retram>, <&mcuram>,<&mcuram2>;
>         mboxes = <&ipcc 2>, <&ipcc 3>;
>         mbox-names = "shutdown", "detach";
>         status = "okay";
> 
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         vdev@0 {
> 		compatible = "rproc-virtio";
> 		reg = <0>;
> 		virtio,id = <7>;  /* RPMSG */
> 		memory-region = <&vdev0vring0>, <&vdev0vring1>,
> <&vdev0buffer>;
> 		mboxes = <&ipcc 0>, <&ipcc 1>;
> 		mbox-names = "vq0", "vq1";
> 		status = "okay";
>         };
> 
>         vdev@1 {
> 		compatible = "rproc-virtio";
> 		reg = <1>;
> 		virtio,id = <7>;  /*RPMSG */
> 		memory-region = <&vdev1vring0>, <&vdev1vring1>,
> <&vdev1buffer>;
> 		mboxes = <&ipcc 4>, <&ipcc 5>;
> 		mbox-names = "vq0", "vq1";
> 		status = "okay";
>         };
> };
> 
> I have divided the work in 4 steps to simplify the review, This series
> implements only the step 1:
> step 1: Redefine the remoteproc VirtIO device as a platform device
>   - migrate rvdev management in remoteproc virtio.c,
>   - create a remotproc virtio config ( can be disabled for platform that not
> use VirtIO IPC.
> step 2: Add possibility to declare and probe a VirtIO sub node
>   - VirtIO bindings declaration,
>   - multi DT VirtIO devices support,
>   - introduction of a remote proc virtio bind device mechanism , =>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep2-virtio-in-
> DT&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e74
> 39508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
> 637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
> %7C&amp;sdata=XtF%2FQnml3QXFL7rgqST1Z2FotUzoj%2FD57WfiuAVMnr8
> %3D&amp;reserved=0
> step 3: Add memory declaration in VirtIO subnode =>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep3-virtio-
> memories&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
> 7C%7C%7C&amp;sdata=6gq28c6a1TJ%2FdkvokcEjgy6FKQcKTXSz%2BNAbJPo
> mjac%3D&amp;reserved=0
> step 4: Add mailbox declaration in VirtIO subnode =>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep4-virtio-
> mailboxes&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
> 7C%7C%7C&amp;sdata=wfy2euuOPoMmBMIH3BOsGcsEYGSTWsDaRr7ENN
> QCK70%3D&amp;reserved=0
> 
> Arnaud Pouliquen (4):
>   remoteproc: core: Introduce rproc_rvdev_add_device function
>   remoteproc: core: Introduce rproc_add_rvdev function
>   remoteproc: Move rproc_vdev management to remoteproc_virtio.c
>   remoteproc: virtio: Create platform device for the remoteproc_virtio
> 
>  drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
>  drivers/remoteproc/remoteproc_internal.h |  23 ++-
>  drivers/remoteproc/remoteproc_virtio.c   | 189 ++++++++++++++++++++---
>  include/linux/remoteproc.h               |   6 +-
>  4 files changed, 210 insertions(+), 162 deletions(-)
> 
> --
> 2.24.3

Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Arnaud POULIQUEN]

Hi Mathieu,

On 9/20/22 22:51, Mathieu Poirier wrote:
> On Tue, Sep 20, 2022 at 02:22:01PM -0600, Mathieu Poirier wrote:
>> On Tue, Sep 20, 2022 at 03:44:18PM +0200, Arnaud POULIQUEN wrote:
>>> Hi Mathieu,
>>>
>>> On 9/20/22 00:30, Mathieu Poirier wrote:
>>>> Hi,
>>>>
>>>> On Fri, Aug 26, 2022 at 01:52:28PM +0200, Arnaud Pouliquen wrote:
>>>>> 1) Update from V7 [1]:
>>>>>
>>>>> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc: imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
>>>>>   The updates take into account the integration of the
>>>>>   commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after rproc_shutdown")
>>>>> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> according to reviews on V7
>>>>>
>>>>>
>>>>> [1] https://lkml.org/lkml/2022/7/13/663
>>>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/log/?h=for-next
>>>>>
>>>>> 2) Patchset description:
>>>>>
>>>>> This series is a part of the work initiated a long time ago in 
>>>>> the series "remoteproc: Decorelate virtio from core"[3]
>>>>>
>>>>> Objective of the work:
>>>>> - Update the remoteproc VirtIO device creation (use platform device)
>>>>> - Allow to declare remoteproc VirtIO device in DT
>>>>>     - declare resources associated to a remote proc VirtIO
>>>>>     - declare a list of VirtIO supported by the platform.
>>>>> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
>>>>>   For instance be able to declare a I2C device in a virtio-i2C node.
>>>>> - Keep the legacy working!
>>>>> - Try to improve the picture about concerns reported by Christoph Hellwing [4][5]
>>>>>
>>>>> [3] https://lkml.org/lkml/2020/4/16/1817
>>>>> [4] https://lkml.org/lkml/2021/6/23/607
>>>>> [5] https://patchwork.kernel.org/project/linux-remoteproc/patch/AOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E@cp7-web-042.plabs.ch/
>>>>>
>>>>> In term of device tree this would result in such hierarchy (stm32mp1 example with 2 virtio RPMSG):
>>>>>
>>>>> 	m4_rproc: m4@10000000 {
>>>>> 		compatible = "st,stm32mp1-m4";
>>>>> 		reg = <0x10000000 0x40000>,
>>>>> 		      <0x30000000 0x40000>,
>>>>> 		      <0x38000000 0x10000>;
>>>>>         memory-region = <&retram>, <&mcuram>,<&mcuram2>;
>>>>>         mboxes = <&ipcc 2>, <&ipcc 3>;
>>>>>         mbox-names = "shutdown", "detach";
>>>>>         status = "okay";
>>>>>
>>>>>         #address-cells = <1>;
>>>>>         #size-cells = <0>;
>>>>>         
>>>>>         vdev@0 {
>>>>> 		compatible = "rproc-virtio";
>>>>> 		reg = <0>;
>>>>> 		virtio,id = <7>;  /* RPMSG */
>>>>> 		memory-region = <&vdev0vring0>, <&vdev0vring1>, <&vdev0buffer>;
>>>>> 		mboxes = <&ipcc 0>, <&ipcc 1>;
>>>>> 		mbox-names = "vq0", "vq1";
>>>>> 		status = "okay";
>>>>>         };
>>>>>
>>>>>         vdev@1 {
>>>>> 		compatible = "rproc-virtio";
>>>>> 		reg = <1>;
>>>>> 		virtio,id = <7>;  /*RPMSG */
>>>>> 		memory-region = <&vdev1vring0>, <&vdev1vring1>, <&vdev1buffer>;
>>>>> 		mboxes = <&ipcc 4>, <&ipcc 5>;
>>>>> 		mbox-names = "vq0", "vq1";
>>>>> 		status = "okay";
>>>>>         };
>>>>> };
>>>>
>>>> I was in the process of applying this set when the last patch gave me a
>>>> checkpatch warning about "virtio,rproc" not being documented.
>>>>
>>>> I suggest to introduce a new "virtio-rproc.yaml" based on this work[1], with the
>>>> above in the example sections.
>>>
>>> Yes I saw the warning, but for this first series it is not possible to declare
>>> the associated "rproc-virtio" device  in device tree.
>>
>> I understand and agree with your position.
>>
>> I am going ahead and merging this set in order for it to get some exposure in
>> linux-next.  That said be on the ready to address potential problems it may
>> cause.

Yes sure!

> 
> I am getting conflicts because of the patches previously applied to rproc-next.
> Please resent a series that applies to "7d7f8fe4e399" and I'll move forward with
> the merge.
> 

I just sent the V9 to address the rebase.

Thanks,
Arnaud


>>
>>> So at this step it seems not make senses to create the devicetree bindings file.
>>> More than that I don't know how I could justify the properties in bindings if
>>> there is not driver code associated.
>>>
>>> So i would be in favor of not adding the bindings in this series but to define
>>> bindings in the first patch of my "step 2" series; as done on my github:
>>> https://github.com/arnopo/linux/commit/9616d89a4f478cf78865a244efcde108d900f69f
>>>
>>> Please let me know your preference.
>>>
>>> Regards,
>>> Arnaud
>>>
>>>
>>>>
>>>> Thanks,
>>>> Mathieu
>>>>
>>>> [1]. https://elixir.bootlin.com/linux/v6.0-rc6/source/Documentation/devicetree/bindings/virtio/virtio-device.yaml
>>>>
>>>>
>>>>>
>>>>> I have divided the work in 4 steps to simplify the review, This series implements only
>>>>> the step 1:
>>>>> step 1: Redefine the remoteproc VirtIO device as a platform device
>>>>>   - migrate rvdev management in remoteproc virtio.c,
>>>>>   - create a remotproc virtio config ( can be disabled for platform that not use VirtIO IPC.
>>>>> step 2: Add possibility to declare and probe a VirtIO sub node
>>>>>   - VirtIO bindings declaration,
>>>>>   - multi DT VirtIO devices support,
>>>>>   - introduction of a remote proc virtio bind device mechanism ,
>>>>> => https://github.com/arnopo/linux/commits/step2-virtio-in-DT
>>>>> step 3: Add memory declaration in VirtIO subnode
>>>>> => https://github.com/arnopo/linux/commits/step3-virtio-memories
>>>>> step 4: Add mailbox declaration in VirtIO subnode
>>>>> => https://github.com/arnopo/linux/commits/step4-virtio-mailboxes
>>>>>
>>>>> Arnaud Pouliquen (4):
>>>>>   remoteproc: core: Introduce rproc_rvdev_add_device function
>>>>>   remoteproc: core: Introduce rproc_add_rvdev function
>>>>>   remoteproc: Move rproc_vdev management to remoteproc_virtio.c
>>>>>   remoteproc: virtio: Create platform device for the remoteproc_virtio
>>>>>
>>>>>  drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
>>>>>  drivers/remoteproc/remoteproc_internal.h |  23 ++-
>>>>>  drivers/remoteproc/remoteproc_virtio.c   | 189 ++++++++++++++++++++---
>>>>>  include/linux/remoteproc.h               |   6 +-
>>>>>  4 files changed, 210 insertions(+), 162 deletions(-)
>>>>>
>>>>> -- 
>>>>> 2.24.3
>>>>>

Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Arnaud POULIQUEN]

Hi Peng,

On 9/21/22 10:54, Peng Fan wrote:
> Hi Arnaud,
> 
>> Subject: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO
>> device
> 
> Sorry to get in at this late time, just try to catch up.
> Not reviewing comments, just have a question,
> Does remote core firmware requires changes to use this new feature?

For this series, it is not.
For the whole work, it should not, but it will probably depend on the
evolutions related to the reviews and requirements that will come.

> Does your 4 branches listed below still work with linux-6.x?

I have to rebase them. Today my github branches are based on v5.18.rc1
I plan to do this end of this week or next week.
   
> Could the multiple vdev still share same mbox channel?

Yes I'm trying to keep the legacy support of the mailbox in the
remoteproc platform driver.
If no mailbox is declared in the virtio subnode it calls the rproc->ops->kick

> 
> I not own i.MX remote core firmware development, so if no need
> firmware change, I would like give a try and see how it works.

Great! That would be nice to have your feedback. 
Mailbox management is one point, I'm also ineterested in having feedback on 
the memory regions management
I will ping you when my work will be rebased on 6.0

Thanks,
Arnaud

> 
> Thanks,
> Peng.
> 
>>
>> 1) Update from V7 [1]:
>>
>> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc:
>> imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
>>   The updates take into account the integration of the
>>   commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after
>> rproc_shutdown")
>> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> according
>> to reviews on V7
>>
>>
>> [1]
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
>> org%2Flkml%2F2022%2F7%2F13%2F663&amp;data=05%7C01%7Cpeng.fan%
>> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
>> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
>> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
>> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=77pEuwAI7Lh61hx1%2B
>> Hs79Cu0G5KOa6mzQ0PnTC5r8Xk%3D&amp;reserved=0
>> [2]
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ke
>> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fremoteproc%2Flinux.g
>> it%2Flog%2F%3Fh%3Dfor-
>> next&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e7
>> 439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
>> C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
>> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
>> C%7C&amp;sdata=iWUSzKkN9BpHwqbO62awIcyVf9PXcftcdt2kytWVR78%3D
>> &amp;reserved=0
>>
>> 2) Patchset description:
>>
>> This series is a part of the work initiated a long time ago in the series
>> "remoteproc: Decorelate virtio from core"[3]
>>
>> Objective of the work:
>> - Update the remoteproc VirtIO device creation (use platform device)
>> - Allow to declare remoteproc VirtIO device in DT
>>     - declare resources associated to a remote proc VirtIO
>>     - declare a list of VirtIO supported by the platform.
>> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
>>   For instance be able to declare a I2C device in a virtio-i2C node.
>> - Keep the legacy working!
>> - Try to improve the picture about concerns reported by Christoph Hellwing
>> [4][5]
>>
>> [3]
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
>> org%2Flkml%2F2020%2F4%2F16%2F1817&amp;data=05%7C01%7Cpeng.fan
>> %40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4
>> c6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7
>> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw
>> iLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=oPWSfUweLdhUFK5X9
>> 2YcGHem8s%2Bfelcr%2FHx9JAlKG%2BI%3D&amp;reserved=0
>> [4]
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
>> org%2Flkml%2F2021%2F6%2F23%2F607&amp;data=05%7C01%7Cpeng.fan%
>> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
>> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
>> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
>> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=HPpnlaykes8R1Kz1dEN
>> nirEHkDNr7JvRs%2FcsaDPuLdI%3D&amp;reserved=0
>> [5]
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>> hwork.kernel.org%2Fproject%2Flinux-
>> remoteproc%2Fpatch%2FAOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E
>> %40cp7-web-
>> 042.plabs.ch%2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e520
>> 0d739a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%
>> 7C0%7C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIj
>> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
>> 3000%7C%7C%7C&amp;sdata=GtNruefDreOoogL%2BlntAC7GBfk6E1Goq4j%
>> 2BYXt36RdI%3D&amp;reserved=0
>>
>> In term of device tree this would result in such hierarchy (stm32mp1
>> example with 2 virtio RPMSG):
>>
>> 	m4_rproc: m4@10000000 {
>> 		compatible = "st,stm32mp1-m4";
>> 		reg = <0x10000000 0x40000>,
>> 		      <0x30000000 0x40000>,
>> 		      <0x38000000 0x10000>;
>>         memory-region = <&retram>, <&mcuram>,<&mcuram2>;
>>         mboxes = <&ipcc 2>, <&ipcc 3>;
>>         mbox-names = "shutdown", "detach";
>>         status = "okay";
>>
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>>
>>         vdev@0 {
>> 		compatible = "rproc-virtio";
>> 		reg = <0>;
>> 		virtio,id = <7>;  /* RPMSG */
>> 		memory-region = <&vdev0vring0>, <&vdev0vring1>,
>> <&vdev0buffer>;
>> 		mboxes = <&ipcc 0>, <&ipcc 1>;
>> 		mbox-names = "vq0", "vq1";
>> 		status = "okay";
>>         };
>>
>>         vdev@1 {
>> 		compatible = "rproc-virtio";
>> 		reg = <1>;
>> 		virtio,id = <7>;  /*RPMSG */
>> 		memory-region = <&vdev1vring0>, <&vdev1vring1>,
>> <&vdev1buffer>;
>> 		mboxes = <&ipcc 4>, <&ipcc 5>;
>> 		mbox-names = "vq0", "vq1";
>> 		status = "okay";
>>         };
>> };
>>
>> I have divided the work in 4 steps to simplify the review, This series
>> implements only the step 1:
>> step 1: Redefine the remoteproc VirtIO device as a platform device
>>   - migrate rvdev management in remoteproc virtio.c,
>>   - create a remotproc virtio config ( can be disabled for platform that not
>> use VirtIO IPC.
>> step 2: Add possibility to declare and probe a VirtIO sub node
>>   - VirtIO bindings declaration,
>>   - multi DT VirtIO devices support,
>>   - introduction of a remote proc virtio bind device mechanism , =>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep2-virtio-in-
>> DT&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e74
>> 39508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
>> 637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
>> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
>> %7C&amp;sdata=XtF%2FQnml3QXFL7rgqST1Z2FotUzoj%2FD57WfiuAVMnr8
>> %3D&amp;reserved=0
>> step 3: Add memory declaration in VirtIO subnode =>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep3-virtio-
>> memories&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
>> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
>> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
>> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
>> 7C%7C%7C&amp;sdata=6gq28c6a1TJ%2FdkvokcEjgy6FKQcKTXSz%2BNAbJPo
>> mjac%3D&amp;reserved=0
>> step 4: Add mailbox declaration in VirtIO subnode =>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep4-virtio-
>> mailboxes&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
>> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
>> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
>> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
>> 7C%7C%7C&amp;sdata=wfy2euuOPoMmBMIH3BOsGcsEYGSTWsDaRr7ENN
>> QCK70%3D&amp;reserved=0
>>
>> Arnaud Pouliquen (4):
>>   remoteproc: core: Introduce rproc_rvdev_add_device function
>>   remoteproc: core: Introduce rproc_add_rvdev function
>>   remoteproc: Move rproc_vdev management to remoteproc_virtio.c
>>   remoteproc: virtio: Create platform device for the remoteproc_virtio
>>
>>  drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
>>  drivers/remoteproc/remoteproc_internal.h |  23 ++-
>>  drivers/remoteproc/remoteproc_virtio.c   | 189 ++++++++++++++++++++---
>>  include/linux/remoteproc.h               |   6 +-
>>  4 files changed, 210 insertions(+), 162 deletions(-)
>>
>> --
>> 2.24.3
> 

Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Arnaud POULIQUEN]

Hi Peng,

On 9/21/22 16:07, Arnaud POULIQUEN wrote:
> Hi Peng,
> 
> On 9/21/22 10:54, Peng Fan wrote:
>> Hi Arnaud,
>>
>>> Subject: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO
>>> device
>>
>> Sorry to get in at this late time, just try to catch up.
>> Not reviewing comments, just have a question,
>> Does remote core firmware requires changes to use this new feature?
> 
> For this series, it is not.
> For the whole work, it should not, but it will probably depend on the
> evolutions related to the reviews and requirements that will come.
> 
>> Does your 4 branches listed below still work with linux-6.x?
> 
> I have to rebase them. Today my github branches are based on v5.18.rc1
> I plan to do this end of this week or next week.
>    
>> Could the multiple vdev still share same mbox channel?
> 
> Yes I'm trying to keep the legacy support of the mailbox in the
> remoteproc platform driver.
> If no mailbox is declared in the virtio subnode it calls the rproc->ops->kick
> 
>>
>> I not own i.MX remote core firmware development, so if no need
>> firmware change, I would like give a try and see how it works.
> 
> Great! That would be nice to have your feedback. 
> Mailbox management is one point, I'm also ineterested in having feedback on 
> the memory regions management
> I will ping you when my work will be rebased on 6.0

My github branches have been rebased on top of thre rproc_next(1d7b61c06dc3)

As a first step you should be able to rebase on my step4-virtio-mailboxes[1]
without any update of your driver. If I did my dev well, I kept the
compatibility with the legacy.

[1] https://github.com/arnopo/linux/commits/step4-virtio-mailboxes

Regards,
Arnaud

> 
> Thanks,
> Arnaud
> 
>>
>> Thanks,
>> Peng.
>>
>>>
>>> 1) Update from V7 [1]:
>>>
>>> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc:
>>> imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
>>>   The updates take into account the integration of the
>>>   commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after
>>> rproc_shutdown")
>>> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> according
>>> to reviews on V7
>>>
>>>
>>> [1]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
>>> org%2Flkml%2F2022%2F7%2F13%2F663&amp;data=05%7C01%7Cpeng.fan%
>>> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
>>> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
>>> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
>>> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=77pEuwAI7Lh61hx1%2B
>>> Hs79Cu0G5KOa6mzQ0PnTC5r8Xk%3D&amp;reserved=0
>>> [2]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ke
>>> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fremoteproc%2Flinux.g
>>> it%2Flog%2F%3Fh%3Dfor-
>>> next&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e7
>>> 439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
>>> C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
>>> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
>>> C%7C&amp;sdata=iWUSzKkN9BpHwqbO62awIcyVf9PXcftcdt2kytWVR78%3D
>>> &amp;reserved=0
>>>
>>> 2) Patchset description:
>>>
>>> This series is a part of the work initiated a long time ago in the series
>>> "remoteproc: Decorelate virtio from core"[3]
>>>
>>> Objective of the work:
>>> - Update the remoteproc VirtIO device creation (use platform device)
>>> - Allow to declare remoteproc VirtIO device in DT
>>>     - declare resources associated to a remote proc VirtIO
>>>     - declare a list of VirtIO supported by the platform.
>>> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
>>>   For instance be able to declare a I2C device in a virtio-i2C node.
>>> - Keep the legacy working!
>>> - Try to improve the picture about concerns reported by Christoph Hellwing
>>> [4][5]
>>>
>>> [3]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
>>> org%2Flkml%2F2020%2F4%2F16%2F1817&amp;data=05%7C01%7Cpeng.fan
>>> %40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4
>>> c6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7
>>> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw
>>> iLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=oPWSfUweLdhUFK5X9
>>> 2YcGHem8s%2Bfelcr%2FHx9JAlKG%2BI%3D&amp;reserved=0
>>> [4]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
>>> org%2Flkml%2F2021%2F6%2F23%2F607&amp;data=05%7C01%7Cpeng.fan%
>>> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
>>> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
>>> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
>>> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=HPpnlaykes8R1Kz1dEN
>>> nirEHkDNr7JvRs%2FcsaDPuLdI%3D&amp;reserved=0
>>> [5]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>>> hwork.kernel.org%2Fproject%2Flinux-
>>> remoteproc%2Fpatch%2FAOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E
>>> %40cp7-web-
>>> 042.plabs.ch%2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e520
>>> 0d739a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%
>>> 7C0%7C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIj
>>> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
>>> 3000%7C%7C%7C&amp;sdata=GtNruefDreOoogL%2BlntAC7GBfk6E1Goq4j%
>>> 2BYXt36RdI%3D&amp;reserved=0
>>>
>>> In term of device tree this would result in such hierarchy (stm32mp1
>>> example with 2 virtio RPMSG):
>>>
>>> 	m4_rproc: m4@10000000 {
>>> 		compatible = "st,stm32mp1-m4";
>>> 		reg = <0x10000000 0x40000>,
>>> 		      <0x30000000 0x40000>,
>>> 		      <0x38000000 0x10000>;
>>>         memory-region = <&retram>, <&mcuram>,<&mcuram2>;
>>>         mboxes = <&ipcc 2>, <&ipcc 3>;
>>>         mbox-names = "shutdown", "detach";
>>>         status = "okay";
>>>
>>>         #address-cells = <1>;
>>>         #size-cells = <0>;
>>>
>>>         vdev@0 {
>>> 		compatible = "rproc-virtio";
>>> 		reg = <0>;
>>> 		virtio,id = <7>;  /* RPMSG */
>>> 		memory-region = <&vdev0vring0>, <&vdev0vring1>,
>>> <&vdev0buffer>;
>>> 		mboxes = <&ipcc 0>, <&ipcc 1>;
>>> 		mbox-names = "vq0", "vq1";
>>> 		status = "okay";
>>>         };
>>>
>>>         vdev@1 {
>>> 		compatible = "rproc-virtio";
>>> 		reg = <1>;
>>> 		virtio,id = <7>;  /*RPMSG */
>>> 		memory-region = <&vdev1vring0>, <&vdev1vring1>,
>>> <&vdev1buffer>;
>>> 		mboxes = <&ipcc 4>, <&ipcc 5>;
>>> 		mbox-names = "vq0", "vq1";
>>> 		status = "okay";
>>>         };
>>> };
>>>
>>> I have divided the work in 4 steps to simplify the review, This series
>>> implements only the step 1:
>>> step 1: Redefine the remoteproc VirtIO device as a platform device
>>>   - migrate rvdev management in remoteproc virtio.c,
>>>   - create a remotproc virtio config ( can be disabled for platform that not
>>> use VirtIO IPC.
>>> step 2: Add possibility to declare and probe a VirtIO sub node
>>>   - VirtIO bindings declaration,
>>>   - multi DT VirtIO devices support,
>>>   - introduction of a remote proc virtio bind device mechanism , =>
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep2-virtio-in-
>>> DT&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e74
>>> 39508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
>>> 637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
>>> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
>>> %7C&amp;sdata=XtF%2FQnml3QXFL7rgqST1Z2FotUzoj%2FD57WfiuAVMnr8
>>> %3D&amp;reserved=0
>>> step 3: Add memory declaration in VirtIO subnode =>
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep3-virtio-
>>> memories&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
>>> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
>>> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
>>> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
>>> 7C%7C%7C&amp;sdata=6gq28c6a1TJ%2FdkvokcEjgy6FKQcKTXSz%2BNAbJPo
>>> mjac%3D&amp;reserved=0
>>> step 4: Add mailbox declaration in VirtIO subnode =>
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep4-virtio-
>>> mailboxes&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
>>> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
>>> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
>>> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
>>> 7C%7C%7C&amp;sdata=wfy2euuOPoMmBMIH3BOsGcsEYGSTWsDaRr7ENN
>>> QCK70%3D&amp;reserved=0
>>>
>>> Arnaud Pouliquen (4):
>>>   remoteproc: core: Introduce rproc_rvdev_add_device function
>>>   remoteproc: core: Introduce rproc_add_rvdev function
>>>   remoteproc: Move rproc_vdev management to remoteproc_virtio.c
>>>   remoteproc: virtio: Create platform device for the remoteproc_virtio
>>>
>>>  drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
>>>  drivers/remoteproc/remoteproc_internal.h |  23 ++-
>>>  drivers/remoteproc/remoteproc_virtio.c   | 189 ++++++++++++++++++++---
>>>  include/linux/remoteproc.h               |   6 +-
>>>  4 files changed, 210 insertions(+), 162 deletions(-)
>>>
>>> --
>>> 2.24.3
>>

RE: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device

[Peng Fan]

> Subject: Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO
> device
> 
> Hi Peng,
> 
> On 9/21/22 16:07, Arnaud POULIQUEN wrote:
> > Hi Peng,
> >
> > On 9/21/22 10:54, Peng Fan wrote:
> >> Hi Arnaud,
> >>
> >>> Subject: [PATCH v8 0/4] remoteproc: restructure the remoteproc
> >>> VirtIO device
> >>
> >> Sorry to get in at this late time, just try to catch up.
> >> Not reviewing comments, just have a question, Does remote core
> >> firmware requires changes to use this new feature?
> >
> > For this series, it is not.
> > For the whole work, it should not, but it will probably depend on the
> > evolutions related to the reviews and requirements that will come.
> >
> >> Does your 4 branches listed below still work with linux-6.x?
> >
> > I have to rebase them. Today my github branches are based on v5.18.rc1
> > I plan to do this end of this week or next week.
> >
> >> Could the multiple vdev still share same mbox channel?
> >
> > Yes I'm trying to keep the legacy support of the mailbox in the
> > remoteproc platform driver.
> > If no mailbox is declared in the virtio subnode it calls the
> > rproc->ops->kick
> >
> >>
> >> I not own i.MX remote core firmware development, so if no need
> >> firmware change, I would like give a try and see how it works.
> >
> > Great! That would be nice to have your feedback.
> > Mailbox management is one point, I'm also ineterested in having
> > feedback on the memory regions management I will ping you when my
> work
> > will be rebased on 6.0
> 
> My github branches have been rebased on top of thre
> rproc_next(1d7b61c06dc3)
> 
> As a first step you should be able to rebase on my step4-virtio-mailboxes[1]
> without any update of your driver. If I did my dev well, I kept the
> compatibility with the legacy.

Thanks for the quick action. I will give a try on i.MX, and see how it works,
but I may not respond quick.

Thanks,
Peng.

> 
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep4-virtio-
> mailboxes&data=05%7C01%7Cpeng.fan%40nxp.com%7C50e9c0e75117
> 4c09328808da9d4bab9b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> 0%7C637995245590228209%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
> %7C%7C&sdata=Jbwa0aPM4aoR8H1cyTLFKw4lK9VUcCyHwzkleVsnkRc
> %3D&reserved=0
> 
> Regards,
> Arnaud
> 
> >
> > Thanks,
> > Arnaud
> >
> >>
> >> Thanks,
> >> Peng.
> >>
> >>>
> >>> 1) Update from V7 [1]:
> >>>
> >>> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc:
> >>> imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
> >>>   The updates take into account the integration of the
> >>>   commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after
> >>> rproc_shutdown")
> >>> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >>> according to reviews on V7
> >>>
> >>>
> >>> [1]
> >>>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
> >>>
> org%2Flkml%2F2022%2F7%2F13%2F663&amp;data=05%7C01%7Cpeng.fan%
> >>>
> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
> >>>
> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
> >>>
> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> >>>
> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=77pEuwAI7Lh61hx1%2B
> >>> Hs79Cu0G5KOa6mzQ0PnTC5r8Xk%3D&amp;reserved=0
> >>> [2]
> >>>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> >>>
> t.ke%2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C50e9c0e751174c
> 093288
> >>>
> 08da9d4bab9b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637
> 9952455
> >>>
> 90228209%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjo
> iV2luMzI
> >>>
> iLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=bF
> M3T6bY1c
> >>> p%2FvyRyZo6mdXGeN7%2BTCMfcmeY3OFpksWA%3D&amp;reserved=0
> >>>
> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fremoteproc%2Flinux.g
> >>> it%2Flog%2F%3Fh%3Dfor-
> >>>
> next&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e7
> >>>
> 439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> >>>
> C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> >>>
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
> >>>
> C%7C&amp;sdata=iWUSzKkN9BpHwqbO62awIcyVf9PXcftcdt2kytWVR78%3D
> >>> &amp;reserved=0
> >>>
> >>> 2) Patchset description:
> >>>
> >>> This series is a part of the work initiated a long time ago in the
> >>> series
> >>> "remoteproc: Decorelate virtio from core"[3]
> >>>
> >>> Objective of the work:
> >>> - Update the remoteproc VirtIO device creation (use platform device)
> >>> - Allow to declare remoteproc VirtIO device in DT
> >>>     - declare resources associated to a remote proc VirtIO
> >>>     - declare a list of VirtIO supported by the platform.
> >>> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio,
> video, ...).
> >>>   For instance be able to declare a I2C device in a virtio-i2C node.
> >>> - Keep the legacy working!
> >>> - Try to improve the picture about concerns reported by Christoph
> >>> Hellwing [4][5]
> >>>
> >>> [3]
> >>>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
> >>>
> org%2Flkml%2F2020%2F4%2F16%2F1817&amp;data=05%7C01%7Cpeng.fan
> >>> %40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3b
> c2b4
> >>>
> c6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7
> >>>
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw
> >>>
> iLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=oPWSfUweLdhUFK5X9
> >>> 2YcGHem8s%2Bfelcr%2FHx9JAlKG%2BI%3D&amp;reserved=0
> >>> [4]
> >>>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
> >>>
> org%2Flkml%2F2021%2F6%2F23%2F607&amp;data=05%7C01%7Cpeng.fan%
> >>>
> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
> >>>
> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
> >>>
> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> >>>
> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=HPpnlaykes8R1Kz1dEN
> >>> nirEHkDNr7JvRs%2FcsaDPuLdI%3D&amp;reserved=0
> >>> [5]
> >>>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> >>> tc
> >>> hwork.kernel.org%2Fproject%2Flinux-
> >>>
> remoteproc%2Fpatch%2FAOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E
> >>> %40cp7-web-
> >>>
> 042.plabs.ch%2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e520
> >>>
> 0d739a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> >>>
> 7C0%7C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> >>>
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> >>>
> 3000%7C%7C%7C&amp;sdata=GtNruefDreOoogL%2BlntAC7GBfk6E1Goq4j%
> >>> 2BYXt36RdI%3D&amp;reserved=0
> >>>
> >>> In term of device tree this would result in such hierarchy (stm32mp1
> >>> example with 2 virtio RPMSG):
> >>>
> >>> 	m4_rproc: m4@10000000 {
> >>> 		compatible = "st,stm32mp1-m4";
> >>> 		reg = <0x10000000 0x40000>,
> >>> 		      <0x30000000 0x40000>,
> >>> 		      <0x38000000 0x10000>;
> >>>         memory-region = <&retram>, <&mcuram>,<&mcuram2>;
> >>>         mboxes = <&ipcc 2>, <&ipcc 3>;
> >>>         mbox-names = "shutdown", "detach";
> >>>         status = "okay";
> >>>
> >>>         #address-cells = <1>;
> >>>         #size-cells = <0>;
> >>>
> >>>         vdev@0 {
> >>> 		compatible = "rproc-virtio";
> >>> 		reg = <0>;
> >>> 		virtio,id = <7>;  /* RPMSG */
> >>> 		memory-region = <&vdev0vring0>, <&vdev0vring1>,
> <&vdev0buffer>;
> >>> 		mboxes = <&ipcc 0>, <&ipcc 1>;
> >>> 		mbox-names = "vq0", "vq1";
> >>> 		status = "okay";
> >>>         };
> >>>
> >>>         vdev@1 {
> >>> 		compatible = "rproc-virtio";
> >>> 		reg = <1>;
> >>> 		virtio,id = <7>;  /*RPMSG */
> >>> 		memory-region = <&vdev1vring0>, <&vdev1vring1>,
> <&vdev1buffer>;
> >>> 		mboxes = <&ipcc 4>, <&ipcc 5>;
> >>> 		mbox-names = "vq0", "vq1";
> >>> 		status = "okay";
> >>>         };
> >>> };
> >>>
> >>> I have divided the work in 4 steps to simplify the review, This
> >>> series implements only the step 1:
> >>> step 1: Redefine the remoteproc VirtIO device as a platform device
> >>>   - migrate rvdev management in remoteproc virtio.c,
> >>>   - create a remotproc virtio config ( can be disabled for platform
> >>> that not use VirtIO IPC.
> >>> step 2: Add possibility to declare and probe a VirtIO sub node
> >>>   - VirtIO bindings declaration,
> >>>   - multi DT VirtIO devices support,
> >>>   - introduction of a remote proc virtio bind device mechanism , =>
> >>>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> >>> thu
> >>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep2-virtio-in-
> >>>
> DT&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e74
> >>>
> 39508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
> >>>
> 637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> >>>
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
> >>> %7C&amp;sdata=XtF%2FQnml3QXFL7rgqST1Z2FotUzoj%2FD57WfiuAVM
> nr8
> >>> %3D&amp;reserved=0
> >>> step 3: Add memory declaration in VirtIO subnode =>
> >>>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> >>> thu
> >>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep3-virtio-
> >>>
> memories&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
> >>>
> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> >>>
> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> >>>
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
> >>>
> 7C%7C%7C&amp;sdata=6gq28c6a1TJ%2FdkvokcEjgy6FKQcKTXSz%2BNAbJPo
> >>> mjac%3D&amp;reserved=0
> >>> step 4: Add mailbox declaration in VirtIO subnode =>
> >>>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> >>> thu
> >>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep4-virtio-
> >>>
> mailboxes&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
> >>>
> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> >>>
> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> >>>
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
> >>>
> 7C%7C%7C&amp;sdata=wfy2euuOPoMmBMIH3BOsGcsEYGSTWsDaRr7ENN
> >>> QCK70%3D&amp;reserved=0
> >>>
> >>> Arnaud Pouliquen (4):
> >>>   remoteproc: core: Introduce rproc_rvdev_add_device function
> >>>   remoteproc: core: Introduce rproc_add_rvdev function
> >>>   remoteproc: Move rproc_vdev management to remoteproc_virtio.c
> >>>   remoteproc: virtio: Create platform device for the
> >>> remoteproc_virtio
> >>>
> >>>  drivers/remoteproc/remoteproc_core.c     | 154 +++---------------
> >>>  drivers/remoteproc/remoteproc_internal.h |  23 ++-
> >>>  drivers/remoteproc/remoteproc_virtio.c   | 189
> ++++++++++++++++++++---
> >>>  include/linux/remoteproc.h               |   6 +-
> >>>  4 files changed, 210 insertions(+), 162 deletions(-)
> >>>
> >>> --
> >>> 2.24.3
> >>