[PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Arnaud Pouliquen]

Updates from V8 [1]:
- rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
- updates based on Bjorn Andersson's comments:
  - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
    function.
  - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel

Patchset description:

The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
remote processor.
This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
in the generic rpmsg virtio backend.
This series restructures the rpmsg_char driver to decorrelate the control part from the data part
in order to improve its compatible with the rpmsg virtio backend.

Objective:
- Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
  rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
  rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
  RPMSG_DESTROY_DEV_IOCTL controls).
  An application will be able to create/establish an rpmsg communication channel to communicate
  with the remote processor, and not only wait the remote processor initiative.
  This is interesting for example to establish a temporary communication link for diagnosis,
  calibration, debugging... or instantiate  new data flows on some user actions.
- Add capability to probe the rpmsg_char device at the initiative of the remote processor
 (rpmsg service announcement mechanism).
  This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
  a rpmsg name service announcement.

Subsets:
  - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
  - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
  - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
    devices (patch 11)
    The application can then create or release a channel by specifying:
       - the name service of the device to instantiate.   
       - the source address.
       - the destination address.

This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
rpmsg_char cdev release fixes [2][3]

[1] https://lkml.org/lkml/2021/12/7/186
[2] https://lkml.org/lkml/2022/1/10/1129
[3] https://lkml.org/lkml/2022/1/10/1130

Arnaud Pouliquen (11):
  rpmsg: char: Export eptdev create and destroy functions
  rpmsg: Create the rpmsg class in core instead of in rpmsg char
  rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
  arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
  RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
  arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
  rpmsg: Update rpmsg_chrdev_register_device function
  rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
  rpmsg: char: Add possibility to use default endpoint of the rpmsg
    device
  rpmsg: char: Introduce the "rpmsg-raw" channel
  rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls

 arch/arm/configs/qcom_defconfig   |   1 +
 arch/arm64/configs/defconfig      |   1 +
 arch/riscv/configs/defconfig      |   1 +
 arch/riscv/configs/rv32_defconfig |   1 +
 drivers/rpmsg/Kconfig             |   8 +
 drivers/rpmsg/Makefile            |   1 +
 drivers/rpmsg/qcom_glink_native.c |   2 +-
 drivers/rpmsg/qcom_smd.c          |   2 +-
 drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
 drivers/rpmsg/rpmsg_char.h        |  46 ++++++
 drivers/rpmsg/rpmsg_core.c        |  15 +-
 drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
 drivers/rpmsg/rpmsg_internal.h    |  10 +-
 drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
 include/uapi/linux/rpmsg.h        |  10 ++
 15 files changed, 419 insertions(+), 155 deletions(-)
 create mode 100644 drivers/rpmsg/rpmsg_char.h
 create mode 100644 drivers/rpmsg/rpmsg_ctrl.c

-- 
2.25.1

[PATCH v9 01/11] rpmsg: char: Export eptdev create and destroy functions

[Arnaud Pouliquen]

To prepare the split of the code related to the control (ctrldev)
and the endpoint (eptdev) devices in 2 separate files:

- Rename and export the functions in rpmsg_char.h.

- Suppress the dependency with the rpmsg_ctrldev struct in the
  rpmsg_eptdev_create function.

Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/rpmsg/rpmsg_char.c | 18 +++++++++------
 drivers/rpmsg/rpmsg_char.h | 46 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 7 deletions(-)
 create mode 100644 drivers/rpmsg/rpmsg_char.h

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index 5663cf799c95..3708233cf736 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
+ * Copyright (C) 2022, STMicroelectronics
  * Copyright (c) 2016, Linaro Ltd.
  * Copyright (c) 2012, Michal Simek <monstr@monstr.eu>
  * Copyright (c) 2012, PetaLogix
@@ -25,6 +26,8 @@
 #include <linux/uaccess.h>
 #include <uapi/linux/rpmsg.h>
 
+#include "rpmsg_char.h"
+
 #define RPMSG_DEV_MAX	(MINORMASK + 1)
 
 static dev_t rpmsg_major;
@@ -79,7 +82,7 @@ struct rpmsg_eptdev {
 	wait_queue_head_t readq;
 };
 
-static int rpmsg_eptdev_destroy(struct device *dev, void *data)
+int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data)
 {
 	struct rpmsg_eptdev *eptdev = dev_to_eptdev(dev);
 
@@ -98,6 +101,7 @@ static int rpmsg_eptdev_destroy(struct device *dev, void *data)
 
 	return 0;
 }
+EXPORT_SYMBOL(rpmsg_chrdev_eptdev_destroy);
 
 static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
 			void *priv, u32 addr)
@@ -281,7 +285,7 @@ static long rpmsg_eptdev_ioctl(struct file *fp, unsigned int cmd,
 	if (cmd != RPMSG_DESTROY_EPT_IOCTL)
 		return -EINVAL;
 
-	return rpmsg_eptdev_destroy(&eptdev->dev, NULL);
+	return rpmsg_chrdev_eptdev_destroy(&eptdev->dev, NULL);
 }
 
 static const struct file_operations rpmsg_eptdev_fops = {
@@ -339,10 +343,9 @@ static void rpmsg_eptdev_release_device(struct device *dev)
 	kfree(eptdev);
 }
 
-static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
+int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
 			       struct rpmsg_channel_info chinfo)
 {
-	struct rpmsg_device *rpdev = ctrldev->rpdev;
 	struct rpmsg_eptdev *eptdev;
 	struct device *dev;
 	int ret;
@@ -362,7 +365,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
 
 	device_initialize(dev);
 	dev->class = rpmsg_class;
-	dev->parent = &ctrldev->dev;
+	dev->parent = parent;
 	dev->groups = rpmsg_eptdev_groups;
 	dev_set_drvdata(dev, eptdev);
 
@@ -399,6 +402,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
 
 	return ret;
 }
+EXPORT_SYMBOL(rpmsg_chrdev_eptdev_create);
 
 static int rpmsg_ctrldev_open(struct inode *inode, struct file *filp)
 {
@@ -438,7 +442,7 @@ static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd,
 	chinfo.src = eptinfo.src;
 	chinfo.dst = eptinfo.dst;
 
-	return rpmsg_eptdev_create(ctrldev, chinfo);
+	return rpmsg_chrdev_eptdev_create(ctrldev->rpdev, &ctrldev->dev, chinfo);
 };
 
 static const struct file_operations rpmsg_ctrldev_fops = {
@@ -517,7 +521,7 @@ static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev)
 	int ret;
 
 	/* Destroy all endpoints */
-	ret = device_for_each_child(&ctrldev->dev, NULL, rpmsg_eptdev_destroy);
+	ret = device_for_each_child(&ctrldev->dev, NULL, rpmsg_chrdev_eptdev_destroy);
 	if (ret)
 		dev_warn(&rpdev->dev, "failed to nuke endpoints: %d\n", ret);
 
diff --git a/drivers/rpmsg/rpmsg_char.h b/drivers/rpmsg/rpmsg_char.h
new file mode 100644
index 000000000000..117d9cbc52f0
--- /dev/null
+++ b/drivers/rpmsg/rpmsg_char.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022, STMicroelectronics
+ */
+
+#ifndef __RPMSG_CHRDEV_H__
+#define __RPMSG_CHRDEV_H__
+
+#if IS_ENABLED(CONFIG_RPMSG_CHAR)
+/**
+ * rpmsg_chrdev_eptdev_create() - register char device based on an endpoint
+ * @rpdev:  prepared rpdev to be used for creating endpoints
+ * @parent: parent device
+ * @chinfo: associated endpoint channel information.
+ *
+ * This function create a new rpmsg char endpoint device to instantiate a new
+ * endpoint based on chinfo information.
+ */
+int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
+			       struct rpmsg_channel_info chinfo);
+
+/**
+ * rpmsg_chrdev_eptdev_destroy() - destroy created char device endpoint.
+ * @data: private data associated to the endpoint device
+ *
+ * This function destroys a rpmsg char endpoint device created by the RPMSG_DESTROY_EPT_IOCTL
+ * control.
+ */
+int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data);
+
+#else  /*IS_ENABLED(CONFIG_RPMSG_CHAR) */
+
+static inline int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
+					     struct rpmsg_channel_info chinfo)
+{
+	return -ENXIO;
+}
+
+static inline int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data)
+{
+	return -ENXIO;
+}
+
+#endif /*IS_ENABLED(CONFIG_RPMSG_CHAR) */
+
+#endif /*__RPMSG_CHRDEV_H__ */
-- 
2.25.1

[PATCH v9 02/11] rpmsg: Create the rpmsg class in core instead of in rpmsg char

[Arnaud Pouliquen]

Migrate the creation of the rpmsg class from the rpmsg_char
to the core that the class is usable by the rpmsg_char and
the future rpmsg_ctrl module.

Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/rpmsg/rpmsg_char.c     | 11 +----------
 drivers/rpmsg/rpmsg_core.c     | 15 +++++++++++++--
 drivers/rpmsg/rpmsg_internal.h |  2 ++
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index 3708233cf736..fd56e7967ee3 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -27,11 +27,11 @@
 #include <uapi/linux/rpmsg.h>
 
 #include "rpmsg_char.h"
+#include "rpmsg_internal.h"
 
 #define RPMSG_DEV_MAX	(MINORMASK + 1)
 
 static dev_t rpmsg_major;
-static struct class *rpmsg_class;
 
 static DEFINE_IDA(rpmsg_ctrl_ida);
 static DEFINE_IDA(rpmsg_ept_ida);
@@ -547,17 +547,9 @@ static int rpmsg_chrdev_init(void)
 		return ret;
 	}
 
-	rpmsg_class = class_create(THIS_MODULE, "rpmsg");
-	if (IS_ERR(rpmsg_class)) {
-		pr_err("failed to create rpmsg class\n");
-		unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX);
-		return PTR_ERR(rpmsg_class);
-	}
-
 	ret = register_rpmsg_driver(&rpmsg_chrdev_driver);
 	if (ret < 0) {
 		pr_err("failed to register rpmsg driver\n");
-		class_destroy(rpmsg_class);
 		unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX);
 	}
 
@@ -568,7 +560,6 @@ postcore_initcall(rpmsg_chrdev_init);
 static void rpmsg_chrdev_exit(void)
 {
 	unregister_rpmsg_driver(&rpmsg_chrdev_driver);
-	class_destroy(rpmsg_class);
 	unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX);
 }
 module_exit(rpmsg_chrdev_exit);
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index d9e612f4f0f2..79368a957d89 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -20,6 +20,9 @@
 
 #include "rpmsg_internal.h"
 
+struct class *rpmsg_class;
+EXPORT_SYMBOL(rpmsg_class);
+
 /**
  * rpmsg_create_channel() - create a new rpmsg channel
  * using its name and address info.
@@ -662,10 +665,17 @@ static int __init rpmsg_init(void)
 {
 	int ret;
 
+	rpmsg_class = class_create(THIS_MODULE, "rpmsg");
+	if (IS_ERR(rpmsg_class)) {
+		pr_err("failed to create rpmsg class\n");
+		return PTR_ERR(rpmsg_class);
+	}
+
 	ret = bus_register(&rpmsg_bus);
-	if (ret)
+	if (ret) {
 		pr_err("failed to register rpmsg bus: %d\n", ret);
-
+		class_destroy(rpmsg_class);
+	}
 	return ret;
 }
 postcore_initcall(rpmsg_init);
@@ -673,6 +683,7 @@ postcore_initcall(rpmsg_init);
 static void __exit rpmsg_fini(void)
 {
 	bus_unregister(&rpmsg_bus);
+	class_destroy(rpmsg_class);
 }
 module_exit(rpmsg_fini);
 
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index b1245d3ed7c6..416316200bde 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -18,6 +18,8 @@
 #define to_rpmsg_device(d) container_of(d, struct rpmsg_device, dev)
 #define to_rpmsg_driver(d) container_of(d, struct rpmsg_driver, drv)
 
+extern struct class *rpmsg_class;
+
 /**
  * struct rpmsg_device_ops - indirection table for the rpmsg_device operations
  * @create_channel:	create backend-specific channel, optional
-- 
2.25.1

[PATCH v9 03/11] rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl

[Arnaud Pouliquen]

Create the rpmsg_ctrl.c module and move the code related to the
rpmsg_ctrldev device in this new module.

Add the dependency between rpmsg_char and rpmsg_ctrl in the
kconfig file:

1) RPMSG_CTRL can set as module or built-in if
  RPMSG=y || RPMSG_CHAR=y || RPMSG_CHAR=n

2) RPMSG_CTRL can not be set as built-in if
   RPMSG=m || RPMSG_CHAR=m

Note that RPMGH_CHAR and RPMSG_CTRL can be activated separately.
Therefore, the RPMSG_CTRL configuration must be set for backwards compatibility.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>

---
Update vs previous revision
 - Remove Bjorn Andersson's Reviewed-by as update after rebasing on 5.17-rc1
	- fix cdev add [1]
	- Add pr_fmt() [2]
[1] https://lore.kernel.org/linux-remoteproc/164245960510.1698571.4998090450663669237.b4-ty@linaro.org/T/#t
[2] https://lore.kernel.org/lkml/20211108135945.3364-1-arnaud.pouliquen@foss.st.com/T/
---
 drivers/rpmsg/Kconfig      |   8 ++
 drivers/rpmsg/Makefile     |   1 +
 drivers/rpmsg/rpmsg_char.c | 160 +--------------------------
 drivers/rpmsg/rpmsg_ctrl.c | 215 +++++++++++++++++++++++++++++++++++++
 4 files changed, 226 insertions(+), 158 deletions(-)
 create mode 100644 drivers/rpmsg/rpmsg_ctrl.c

diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
index 0b4407abdf13..d3795860f5c0 100644
--- a/drivers/rpmsg/Kconfig
+++ b/drivers/rpmsg/Kconfig
@@ -15,6 +15,14 @@ config RPMSG_CHAR
 	  in /dev. They make it possible for user-space programs to send and
 	  receive rpmsg packets.
 
+config RPMSG_CTRL
+	tristate "RPMSG control interface"
+	depends on RPMSG && ( RPMSG_CHAR || RPMSG_CHAR=n )
+	help
+	  Say Y here to enable the support of the /dev/rpmsg_ctrlX API. This API
+	  allows user-space programs to create endpoints with specific service name,
+	  source and destination addresses.
+
 config RPMSG_NS
 	tristate "RPMSG name service announcement"
 	depends on RPMSG
diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile
index 8d452656f0ee..58e3b382e316 100644
--- a/drivers/rpmsg/Makefile
+++ b/drivers/rpmsg/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_RPMSG)		+= rpmsg_core.o
 obj-$(CONFIG_RPMSG_CHAR)	+= rpmsg_char.o
+obj-$(CONFIG_RPMSG_CTRL)	+= rpmsg_ctrl.o
 obj-$(CONFIG_RPMSG_NS)		+= rpmsg_ns.o
 obj-$(CONFIG_RPMSG_MTK_SCP)	+= mtk_rpmsg.o
 qcom_glink-objs			:= qcom_glink_native.o qcom_glink_ssr.o
diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index fd56e7967ee3..fcd583472d5a 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -33,28 +33,12 @@
 
 static dev_t rpmsg_major;
 
-static DEFINE_IDA(rpmsg_ctrl_ida);
 static DEFINE_IDA(rpmsg_ept_ida);
 static DEFINE_IDA(rpmsg_minor_ida);
 
 #define dev_to_eptdev(dev) container_of(dev, struct rpmsg_eptdev, dev)
 #define cdev_to_eptdev(i_cdev) container_of(i_cdev, struct rpmsg_eptdev, cdev)
 
-#define dev_to_ctrldev(dev) container_of(dev, struct rpmsg_ctrldev, dev)
-#define cdev_to_ctrldev(i_cdev) container_of(i_cdev, struct rpmsg_ctrldev, cdev)
-
-/**
- * struct rpmsg_ctrldev - control device for instantiating endpoint devices
- * @rpdev:	underlaying rpmsg device
- * @cdev:	cdev for the ctrl device
- * @dev:	device for the ctrl device
- */
-struct rpmsg_ctrldev {
-	struct rpmsg_device *rpdev;
-	struct cdev cdev;
-	struct device dev;
-};
-
 /**
  * struct rpmsg_eptdev - endpoint device context
  * @dev:	endpoint device
@@ -404,162 +388,22 @@ int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent
 }
 EXPORT_SYMBOL(rpmsg_chrdev_eptdev_create);
 
-static int rpmsg_ctrldev_open(struct inode *inode, struct file *filp)
-{
-	struct rpmsg_ctrldev *ctrldev = cdev_to_ctrldev(inode->i_cdev);
-
-	get_device(&ctrldev->dev);
-	filp->private_data = ctrldev;
-
-	return 0;
-}
-
-static int rpmsg_ctrldev_release(struct inode *inode, struct file *filp)
-{
-	struct rpmsg_ctrldev *ctrldev = cdev_to_ctrldev(inode->i_cdev);
-
-	put_device(&ctrldev->dev);
-
-	return 0;
-}
-
-static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd,
-				unsigned long arg)
-{
-	struct rpmsg_ctrldev *ctrldev = fp->private_data;
-	void __user *argp = (void __user *)arg;
-	struct rpmsg_endpoint_info eptinfo;
-	struct rpmsg_channel_info chinfo;
-
-	if (cmd != RPMSG_CREATE_EPT_IOCTL)
-		return -EINVAL;
-
-	if (copy_from_user(&eptinfo, argp, sizeof(eptinfo)))
-		return -EFAULT;
-
-	memcpy(chinfo.name, eptinfo.name, RPMSG_NAME_SIZE);
-	chinfo.name[RPMSG_NAME_SIZE-1] = '\0';
-	chinfo.src = eptinfo.src;
-	chinfo.dst = eptinfo.dst;
-
-	return rpmsg_chrdev_eptdev_create(ctrldev->rpdev, &ctrldev->dev, chinfo);
-};
-
-static const struct file_operations rpmsg_ctrldev_fops = {
-	.owner = THIS_MODULE,
-	.open = rpmsg_ctrldev_open,
-	.release = rpmsg_ctrldev_release,
-	.unlocked_ioctl = rpmsg_ctrldev_ioctl,
-	.compat_ioctl = compat_ptr_ioctl,
-};
-
-static void rpmsg_ctrldev_release_device(struct device *dev)
-{
-	struct rpmsg_ctrldev *ctrldev = dev_to_ctrldev(dev);
-
-	ida_simple_remove(&rpmsg_ctrl_ida, dev->id);
-	ida_simple_remove(&rpmsg_minor_ida, MINOR(dev->devt));
-	kfree(ctrldev);
-}
-
-static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
-{
-	struct rpmsg_ctrldev *ctrldev;
-	struct device *dev;
-	int ret;
-
-	ctrldev = kzalloc(sizeof(*ctrldev), GFP_KERNEL);
-	if (!ctrldev)
-		return -ENOMEM;
-
-	ctrldev->rpdev = rpdev;
-
-	dev = &ctrldev->dev;
-	device_initialize(dev);
-	dev->parent = &rpdev->dev;
-	dev->class = rpmsg_class;
-
-	cdev_init(&ctrldev->cdev, &rpmsg_ctrldev_fops);
-	ctrldev->cdev.owner = THIS_MODULE;
-
-	ret = ida_simple_get(&rpmsg_minor_ida, 0, RPMSG_DEV_MAX, GFP_KERNEL);
-	if (ret < 0)
-		goto free_ctrldev;
-	dev->devt = MKDEV(MAJOR(rpmsg_major), ret);
-
-	ret = ida_simple_get(&rpmsg_ctrl_ida, 0, 0, GFP_KERNEL);
-	if (ret < 0)
-		goto free_minor_ida;
-	dev->id = ret;
-	dev_set_name(&ctrldev->dev, "rpmsg_ctrl%d", ret);
-
-	ret = cdev_device_add(&ctrldev->cdev, &ctrldev->dev);
-	if (ret)
-		goto free_ctrl_ida;
-
-	/* We can now rely on the release function for cleanup */
-	dev->release = rpmsg_ctrldev_release_device;
-
-	dev_set_drvdata(&rpdev->dev, ctrldev);
-
-	return ret;
-
-free_ctrl_ida:
-	ida_simple_remove(&rpmsg_ctrl_ida, dev->id);
-free_minor_ida:
-	ida_simple_remove(&rpmsg_minor_ida, MINOR(dev->devt));
-free_ctrldev:
-	put_device(dev);
-	kfree(ctrldev);
-
-	return ret;
-}
-
-static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev)
-{
-	struct rpmsg_ctrldev *ctrldev = dev_get_drvdata(&rpdev->dev);
-	int ret;
-
-	/* Destroy all endpoints */
-	ret = device_for_each_child(&ctrldev->dev, NULL, rpmsg_chrdev_eptdev_destroy);
-	if (ret)
-		dev_warn(&rpdev->dev, "failed to nuke endpoints: %d\n", ret);
-
-	cdev_device_del(&ctrldev->cdev, &ctrldev->dev);
-	put_device(&ctrldev->dev);
-}
-
-static struct rpmsg_driver rpmsg_chrdev_driver = {
-	.probe = rpmsg_chrdev_probe,
-	.remove = rpmsg_chrdev_remove,
-	.drv = {
-		.name = "rpmsg_chrdev",
-	},
-};
-
 static int rpmsg_chrdev_init(void)
 {
 	int ret;
 
-	ret = alloc_chrdev_region(&rpmsg_major, 0, RPMSG_DEV_MAX, "rpmsg");
+	ret = alloc_chrdev_region(&rpmsg_major, 0, RPMSG_DEV_MAX, "rpmsg_char");
 	if (ret < 0) {
 		pr_err("failed to allocate char dev region\n");
 		return ret;
 	}
 
-	ret = register_rpmsg_driver(&rpmsg_chrdev_driver);
-	if (ret < 0) {
-		pr_err("failed to register rpmsg driver\n");
-		unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX);
-	}
-
-	return ret;
+	return 0;
 }
 postcore_initcall(rpmsg_chrdev_init);
 
 static void rpmsg_chrdev_exit(void)
 {
-	unregister_rpmsg_driver(&rpmsg_chrdev_driver);
 	unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX);
 }
 module_exit(rpmsg_chrdev_exit);
diff --git a/drivers/rpmsg/rpmsg_ctrl.c b/drivers/rpmsg/rpmsg_ctrl.c
new file mode 100644
index 000000000000..963e6e81a83f
--- /dev/null
+++ b/drivers/rpmsg/rpmsg_ctrl.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022, STMicroelectronics
+ * Copyright (c) 2016, Linaro Ltd.
+ * Copyright (c) 2012, Michal Simek <monstr@monstr.eu>
+ * Copyright (c) 2012, PetaLogix
+ * Copyright (c) 2011, Texas Instruments, Inc.
+ * Copyright (c) 2011, Google, Inc.
+ *
+ * Based on rpmsg performance statistics driver by Michal Simek, which in turn
+ * was based on TI & Google OMX rpmsg driver.
+ */
+
+#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
+
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/idr.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/rpmsg.h>
+#include <linux/skbuff.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <uapi/linux/rpmsg.h>
+
+#include "rpmsg_char.h"
+#include "rpmsg_internal.h"
+
+#define RPMSG_DEV_MAX	(MINORMASK + 1)
+
+static dev_t rpmsg_major;
+
+static DEFINE_IDA(rpmsg_ctrl_ida);
+static DEFINE_IDA(rpmsg_minor_ida);
+
+#define dev_to_ctrldev(dev) container_of(dev, struct rpmsg_ctrldev, dev)
+#define cdev_to_ctrldev(i_cdev) container_of(i_cdev, struct rpmsg_ctrldev, cdev)
+
+/**
+ * struct rpmsg_ctrldev - control device for instantiating endpoint devices
+ * @rpdev:	underlaying rpmsg device
+ * @cdev:	cdev for the ctrl device
+ * @dev:	device for the ctrl device
+ */
+struct rpmsg_ctrldev {
+	struct rpmsg_device *rpdev;
+	struct cdev cdev;
+	struct device dev;
+};
+
+static int rpmsg_ctrldev_open(struct inode *inode, struct file *filp)
+{
+	struct rpmsg_ctrldev *ctrldev = cdev_to_ctrldev(inode->i_cdev);
+
+	get_device(&ctrldev->dev);
+	filp->private_data = ctrldev;
+
+	return 0;
+}
+
+static int rpmsg_ctrldev_release(struct inode *inode, struct file *filp)
+{
+	struct rpmsg_ctrldev *ctrldev = cdev_to_ctrldev(inode->i_cdev);
+
+	put_device(&ctrldev->dev);
+
+	return 0;
+}
+
+static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd,
+				unsigned long arg)
+{
+	struct rpmsg_ctrldev *ctrldev = fp->private_data;
+	void __user *argp = (void __user *)arg;
+	struct rpmsg_endpoint_info eptinfo;
+	struct rpmsg_channel_info chinfo;
+
+	if (cmd != RPMSG_CREATE_EPT_IOCTL)
+		return -EINVAL;
+
+	if (copy_from_user(&eptinfo, argp, sizeof(eptinfo)))
+		return -EFAULT;
+
+	memcpy(chinfo.name, eptinfo.name, RPMSG_NAME_SIZE);
+	chinfo.name[RPMSG_NAME_SIZE - 1] = '\0';
+	chinfo.src = eptinfo.src;
+	chinfo.dst = eptinfo.dst;
+
+	return rpmsg_chrdev_eptdev_create(ctrldev->rpdev, &ctrldev->dev, chinfo);
+};
+
+static const struct file_operations rpmsg_ctrldev_fops = {
+	.owner = THIS_MODULE,
+	.open = rpmsg_ctrldev_open,
+	.release = rpmsg_ctrldev_release,
+	.unlocked_ioctl = rpmsg_ctrldev_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
+};
+
+static void rpmsg_ctrldev_release_device(struct device *dev)
+{
+	struct rpmsg_ctrldev *ctrldev = dev_to_ctrldev(dev);
+
+	ida_simple_remove(&rpmsg_ctrl_ida, dev->id);
+	ida_simple_remove(&rpmsg_minor_ida, MINOR(dev->devt));
+	kfree(ctrldev);
+}
+
+static int rpmsg_ctrldev_probe(struct rpmsg_device *rpdev)
+{
+	struct rpmsg_ctrldev *ctrldev;
+	struct device *dev;
+	int ret;
+
+	ctrldev = kzalloc(sizeof(*ctrldev), GFP_KERNEL);
+	if (!ctrldev)
+		return -ENOMEM;
+
+	ctrldev->rpdev = rpdev;
+
+	dev = &ctrldev->dev;
+	device_initialize(dev);
+	dev->parent = &rpdev->dev;
+	dev->class = rpmsg_class;
+
+	cdev_init(&ctrldev->cdev, &rpmsg_ctrldev_fops);
+	ctrldev->cdev.owner = THIS_MODULE;
+
+	ret = ida_simple_get(&rpmsg_minor_ida, 0, RPMSG_DEV_MAX, GFP_KERNEL);
+	if (ret < 0)
+		goto free_ctrldev;
+	dev->devt = MKDEV(MAJOR(rpmsg_major), ret);
+
+	ret = ida_simple_get(&rpmsg_ctrl_ida, 0, 0, GFP_KERNEL);
+	if (ret < 0)
+		goto free_minor_ida;
+	dev->id = ret;
+	dev_set_name(&ctrldev->dev, "rpmsg_ctrl%d", ret);
+
+	ret = cdev_device_add(&ctrldev->cdev, &ctrldev->dev);
+	if (ret)
+		goto free_ctrl_ida;
+
+	/* We can now rely on the release function for cleanup */
+	dev->release = rpmsg_ctrldev_release_device;
+
+	dev_set_drvdata(&rpdev->dev, ctrldev);
+
+	return ret;
+
+free_ctrl_ida:
+	ida_simple_remove(&rpmsg_ctrl_ida, dev->id);
+free_minor_ida:
+	ida_simple_remove(&rpmsg_minor_ida, MINOR(dev->devt));
+free_ctrldev:
+	put_device(dev);
+	kfree(ctrldev);
+
+	return ret;
+}
+
+static void rpmsg_ctrldev_remove(struct rpmsg_device *rpdev)
+{
+	struct rpmsg_ctrldev *ctrldev = dev_get_drvdata(&rpdev->dev);
+	int ret;
+
+	/* Destroy all endpoints */
+	ret = device_for_each_child(&ctrldev->dev, NULL, rpmsg_chrdev_eptdev_destroy);
+	if (ret)
+		dev_warn(&rpdev->dev, "failed to nuke endpoints: %d\n", ret);
+
+	cdev_device_del(&ctrldev->cdev, &ctrldev->dev);
+	put_device(&ctrldev->dev);
+}
+
+static struct rpmsg_driver rpmsg_ctrldev_driver = {
+	.probe = rpmsg_ctrldev_probe,
+	.remove = rpmsg_ctrldev_remove,
+	.drv = {
+		.name = "rpmsg_chrdev",
+	},
+};
+
+static int rpmsg_ctrldev_init(void)
+{
+	int ret;
+
+	ret = alloc_chrdev_region(&rpmsg_major, 0, RPMSG_DEV_MAX, "rpmsg_ctrl");
+	if (ret < 0) {
+		pr_err("failed to allocate char dev region\n");
+		return ret;
+	}
+
+	ret = register_rpmsg_driver(&rpmsg_ctrldev_driver);
+	if (ret < 0) {
+		pr_err("failed to register rpmsg driver\n");
+		unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX);
+	}
+
+	return ret;
+}
+postcore_initcall(rpmsg_ctrldev_init);
+
+static void rpmsg_ctrldev_exit(void)
+{
+	unregister_rpmsg_driver(&rpmsg_ctrldev_driver);
+	unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX);
+}
+module_exit(rpmsg_ctrldev_exit);
+
+MODULE_DESCRIPTION("rpmsg control interface");
+MODULE_ALIAS("rpmsg:" KBUILD_MODNAME);
+MODULE_LICENSE("GPL v2");
-- 
2.25.1

[PATCH v9 04/11] arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL

[Arnaud Pouliquen]

In the patch "rpmsg: Move the rpmsg control device from rpmsg_char
to rpmsg_ctrl", we split the rpmsg_char driver in two.
By default give everyone who had the old driver enabled the rpmsg_ctrl
driver too.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/configs/qcom_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index 9981566f2096..2e7e9a4f31f6 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -241,6 +241,7 @@ CONFIG_QCOM_Q6V5_PAS=y
 CONFIG_QCOM_Q6V5_PIL=y
 CONFIG_QCOM_WCNSS_PIL=y
 CONFIG_RPMSG_CHAR=y
+CONFIG_RPMSG_CTRL=y
 CONFIG_RPMSG_QCOM_GLINK_SMEM=y
 CONFIG_RPMSG_QCOM_SMD=y
 CONFIG_QCOM_COMMAND_DB=y
-- 
2.25.1

[PATCH v9 05/11] RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL

[Arnaud Pouliquen]

In the patch "rpmsg: Move the rpmsg control device from rpmsg_char
to rpmsg_ctrl", we split the rpmsg_char driver in two.
By default give everyone who had the old driver enabled the rpmsg_ctrl
driver too.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
cc: linux-riscv@lists.infradead.org
---
 arch/riscv/configs/defconfig      | 1 +
 arch/riscv/configs/rv32_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index f120fcc43d0a..aaccb1e5f328 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -100,6 +100,7 @@ CONFIG_VIRTIO_BALLOON=y
 CONFIG_VIRTIO_INPUT=y
 CONFIG_VIRTIO_MMIO=y
 CONFIG_RPMSG_CHAR=y
+CONFIG_RPMSG_CTRL=y
 CONFIG_RPMSG_VIRTIO=y
 CONFIG_EXT4_FS=y
 CONFIG_EXT4_FS_POSIX_ACL=y
diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
index 8b56a7f1eb06..47e50c992c9c 100644
--- a/arch/riscv/configs/rv32_defconfig
+++ b/arch/riscv/configs/rv32_defconfig
@@ -92,6 +92,7 @@ CONFIG_VIRTIO_BALLOON=y
 CONFIG_VIRTIO_INPUT=y
 CONFIG_VIRTIO_MMIO=y
 CONFIG_RPMSG_CHAR=y
+CONFIG_RPMSG_CTRL=y
 CONFIG_RPMSG_VIRTIO=y
 CONFIG_EXT4_FS=y
 CONFIG_EXT4_FS_POSIX_ACL=y
-- 
2.25.1

[PATCH v9 06/11] arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL

[Arnaud Pouliquen]

In the patch "rpmsg: Move the rpmsg control device from rpmsg_char
to rpmsg_ctrl", we split the rpmsg_char driver in two.
By default give everyone who had the old driver enabled the rpmsg_ctrl
driver too.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 30516dc0b70e..485c55f19d68 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1039,6 +1039,7 @@ CONFIG_QCOM_Q6V5_PAS=m
 CONFIG_QCOM_SYSMON=m
 CONFIG_QCOM_WCNSS_PIL=m
 CONFIG_RPMSG_CHAR=m
+CONFIG_RPMSG_CTRL=m
 CONFIG_RPMSG_QCOM_GLINK_RPM=y
 CONFIG_RPMSG_QCOM_GLINK_SMEM=m
 CONFIG_RPMSG_QCOM_SMD=y
-- 
2.25.1

[PATCH v9 07/11] rpmsg: Update rpmsg_chrdev_register_device function

[Arnaud Pouliquen]

The rpmsg_chrdev driver has been replaced by the rpmsg_ctrl driver
for the /dev/rpmsg_ctrlX devices management. The reference for the
driver override is now the rpmsg_ctrl.

Update the rpmsg_chrdev_register_device function to reflect the update,
and rename the function to use the rpmsg_ctrldev prefix.

The platform drivers are updated accordingly.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/rpmsg/qcom_glink_native.c | 2 +-
 drivers/rpmsg/qcom_smd.c          | 2 +-
 drivers/rpmsg/rpmsg_ctrl.c        | 2 +-
 drivers/rpmsg/rpmsg_internal.h    | 8 ++++----
 drivers/rpmsg/virtio_rpmsg_bus.c  | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 1030cfa80e04..cf2c057b738c 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1715,7 +1715,7 @@ static int qcom_glink_create_chrdev(struct qcom_glink *glink)
 	rpdev->dev.parent = glink->dev;
 	rpdev->dev.release = qcom_glink_device_release;
 
-	return rpmsg_chrdev_register_device(rpdev);
+	return rpmsg_ctrldev_register_device(rpdev);
 }
 
 struct qcom_glink *qcom_glink_native_probe(struct device *dev,
diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
index 540e027f08c4..cd623e3e8aa9 100644
--- a/drivers/rpmsg/qcom_smd.c
+++ b/drivers/rpmsg/qcom_smd.c
@@ -1113,7 +1113,7 @@ static int qcom_smd_create_chrdev(struct qcom_smd_edge *edge)
 	qsdev->rpdev.dev.parent = &edge->dev;
 	qsdev->rpdev.dev.release = qcom_smd_release_device;
 
-	return rpmsg_chrdev_register_device(&qsdev->rpdev);
+	return rpmsg_ctrldev_register_device(&qsdev->rpdev);
 }
 
 /*
diff --git a/drivers/rpmsg/rpmsg_ctrl.c b/drivers/rpmsg/rpmsg_ctrl.c
index 963e6e81a83f..f43b5e4dbb4c 100644
--- a/drivers/rpmsg/rpmsg_ctrl.c
+++ b/drivers/rpmsg/rpmsg_ctrl.c
@@ -179,7 +179,7 @@ static struct rpmsg_driver rpmsg_ctrldev_driver = {
 	.probe = rpmsg_ctrldev_probe,
 	.remove = rpmsg_ctrldev_remove,
 	.drv = {
-		.name = "rpmsg_chrdev",
+		.name = "rpmsg_ctrl",
 	},
 };
 
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 416316200bde..d4b23fd019a8 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -86,16 +86,16 @@ struct rpmsg_device *rpmsg_create_channel(struct rpmsg_device *rpdev,
 int rpmsg_release_channel(struct rpmsg_device *rpdev,
 			  struct rpmsg_channel_info *chinfo);
 /**
- * rpmsg_chrdev_register_device() - register chrdev device based on rpdev
+ * rpmsg_ctrldev_register_device() - register a char device for control based on rpdev
  * @rpdev:	prepared rpdev to be used for creating endpoints
  *
  * This function wraps rpmsg_register_device() preparing the rpdev for use as
  * basis for the rpmsg chrdev.
  */
-static inline int rpmsg_chrdev_register_device(struct rpmsg_device *rpdev)
+static inline int rpmsg_ctrldev_register_device(struct rpmsg_device *rpdev)
 {
-	strcpy(rpdev->id.name, "rpmsg_chrdev");
-	rpdev->driver_override = "rpmsg_chrdev";
+	strcpy(rpdev->id.name, "rpmsg_ctrl");
+	rpdev->driver_override = "rpmsg_ctrl";
 
 	return rpmsg_register_device(rpdev);
 }
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index ac764e04c898..3ede25b1f2e4 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -849,7 +849,7 @@ static struct rpmsg_device *rpmsg_virtio_add_ctrl_dev(struct virtio_device *vdev
 	rpdev_ctrl->dev.release = virtio_rpmsg_release_device;
 	rpdev_ctrl->little_endian = virtio_is_little_endian(vrp->vdev);
 
-	err = rpmsg_chrdev_register_device(rpdev_ctrl);
+	err = rpmsg_ctrldev_register_device(rpdev_ctrl);
 	if (err) {
 		kfree(vch);
 		return ERR_PTR(err);
-- 
2.25.1

[PATCH v9 08/11] rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function

[Arnaud Pouliquen]

Introduce the rpmsg_chrdev_eptdev_alloc and rpmsg_chrdev_eptdev_add
internal function to split the allocation part from the device add.

This patch prepares the introduction of a rpmsg channel device for the
char device. An default endpoint will be created,
referenced in the rpmsg_eptdev structure before adding the devices.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
---
 drivers/rpmsg/rpmsg_char.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index fcd583472d5a..e7bc7dcdba63 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -327,20 +327,18 @@ static void rpmsg_eptdev_release_device(struct device *dev)
 	kfree(eptdev);
 }
 
-int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
-			       struct rpmsg_channel_info chinfo)
+static struct rpmsg_eptdev *rpmsg_chrdev_eptdev_alloc(struct rpmsg_device *rpdev,
+						      struct device *parent)
 {
 	struct rpmsg_eptdev *eptdev;
 	struct device *dev;
-	int ret;
 
 	eptdev = kzalloc(sizeof(*eptdev), GFP_KERNEL);
 	if (!eptdev)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	dev = &eptdev->dev;
 	eptdev->rpdev = rpdev;
-	eptdev->chinfo = chinfo;
 
 	mutex_init(&eptdev->ept_lock);
 	spin_lock_init(&eptdev->queue_lock);
@@ -356,6 +354,16 @@ int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent
 	cdev_init(&eptdev->cdev, &rpmsg_eptdev_fops);
 	eptdev->cdev.owner = THIS_MODULE;
 
+	return eptdev;
+}
+
+static int rpmsg_chrdev_eptdev_add(struct rpmsg_eptdev *eptdev, struct rpmsg_channel_info chinfo)
+{
+	struct device *dev = &eptdev->dev;
+	int ret;
+
+	eptdev->chinfo = chinfo;
+
 	ret = ida_simple_get(&rpmsg_minor_ida, 0, RPMSG_DEV_MAX, GFP_KERNEL);
 	if (ret < 0)
 		goto free_eptdev;
@@ -386,6 +394,21 @@ int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent
 
 	return ret;
 }
+
+int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
+			       struct rpmsg_channel_info chinfo)
+{
+	struct rpmsg_eptdev *eptdev;
+	int ret;
+
+	eptdev = rpmsg_chrdev_eptdev_alloc(rpdev, parent);
+	if (IS_ERR(eptdev))
+		return PTR_ERR(eptdev);
+
+	ret = rpmsg_chrdev_eptdev_add(eptdev, chinfo);
+
+	return ret;
+}
 EXPORT_SYMBOL(rpmsg_chrdev_eptdev_create);
 
 static int rpmsg_chrdev_init(void)
-- 
2.25.1

[PATCH v9 09/11] rpmsg: char: Add possibility to use default endpoint of the rpmsg device

[Arnaud Pouliquen]

Current implementation create/destroy a new endpoint on each
rpmsg_eptdev_open/rpmsg_eptdev_release calls.

For a rpmsg device created by the NS announcement a default endpoint is created.
In this case we have to reuse the default rpmsg device endpoint associated to
the channel instead of creating a new one.

This patch prepares the introduction of a rpmsg channel device for the
char device. The rpmsg channel device will require a default endpoint to
communicate to the remote processor.

Add the default_ept field in rpmsg_eptdev structure.This pointer
determines the behavior on rpmsg_eptdev_open and rpmsg_eptdev_release call.

- If default_ept == NULL:
  Use the legacy behavior by creating a new endpoint each time
  rpmsg_eptdev_open is called and release it when rpmsg_eptdev_release
  is called on /dev/rpmsgX device open/close.

- If default_ept is set:
  use the rpmsg device default endpoint for the communication.

Add protection in rpmsg_eptdev_ioctl to prevent to destroy a default endpoint.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
---
 drivers/rpmsg/rpmsg_char.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index e7bc7dcdba63..97843838d960 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -50,6 +50,8 @@ static DEFINE_IDA(rpmsg_minor_ida);
  * @queue_lock:	synchronization of @queue operations
  * @queue:	incoming message queue
  * @readq:	wait object for incoming queue
+ * @default_ept: set to channel default endpoint if the default endpoint should be re-used
+ *              on device open to prevent endpoint address update.
  */
 struct rpmsg_eptdev {
 	struct device dev;
@@ -60,10 +62,12 @@ struct rpmsg_eptdev {
 
 	struct mutex ept_lock;
 	struct rpmsg_endpoint *ept;
+	struct rpmsg_endpoint *default_ept;
 
 	spinlock_t queue_lock;
 	struct sk_buff_head queue;
 	wait_queue_head_t readq;
+
 };
 
 int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data)
@@ -121,7 +125,15 @@ static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
 
 	get_device(dev);
 
-	ept = rpmsg_create_ept(rpdev, rpmsg_ept_cb, eptdev, eptdev->chinfo);
+	/*
+	 * If the default_ept is set, the rpmsg device default endpoint is used.
+	 * Else a new endpoint is created on open that will be destroyed on release.
+	 */
+	if (eptdev->default_ept)
+		ept = eptdev->default_ept;
+	else
+		ept = rpmsg_create_ept(rpdev, rpmsg_ept_cb, eptdev, eptdev->chinfo);
+
 	if (!ept) {
 		dev_err(dev, "failed to open %s\n", eptdev->chinfo.name);
 		put_device(dev);
@@ -142,7 +154,8 @@ static int rpmsg_eptdev_release(struct inode *inode, struct file *filp)
 	/* Close the endpoint, if it's not already destroyed by the parent */
 	mutex_lock(&eptdev->ept_lock);
 	if (eptdev->ept) {
-		rpmsg_destroy_ept(eptdev->ept);
+		if (!eptdev->default_ept)
+			rpmsg_destroy_ept(eptdev->ept);
 		eptdev->ept = NULL;
 	}
 	mutex_unlock(&eptdev->ept_lock);
@@ -269,6 +282,10 @@ static long rpmsg_eptdev_ioctl(struct file *fp, unsigned int cmd,
 	if (cmd != RPMSG_DESTROY_EPT_IOCTL)
 		return -EINVAL;
 
+	/* Don't allow to destroy a default endpoint. */
+	if (eptdev->default_ept)
+		return -EINVAL;
+
 	return rpmsg_chrdev_eptdev_destroy(&eptdev->dev, NULL);
 }
 
-- 
2.25.1

[PATCH v9 10/11] rpmsg: char: Introduce the "rpmsg-raw" channel

[Arnaud Pouliquen]

For the rpmsg virtio backend, the current implementation of the rpmsg char
only allows to instantiate static(i.e. prefixed source and destination
addresses) end points, and only on the Linux user space initiative.

This patch defines the "rpmsg-raw" channel and registers it to the rpmsg bus.
This registration allows:
- To create the channel at the initiative of the remote processor
  relying on the name service announcement mechanism. In other words the
  /dev/rpmsgX interface is instantiate by the remote processor.
- To use the channel object instead of the endpoint, thus preventing the
  user space from having the knowledge of the remote processor's
  endpoint addresses.
- To rely on udev to be inform when a /dev/rpmsgX is created on remote
  processor request, indicating that the remote processor is ready to
  communicate.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
---
Update vs previous revision:
- rework commit message,
- remove the use of the rpmsg_create_default_ept function.
---
 drivers/rpmsg/rpmsg_char.c | 60 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index 97843838d960..6c1d9774b7b1 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -428,6 +428,54 @@ int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent
 }
 EXPORT_SYMBOL(rpmsg_chrdev_eptdev_create);
 
+static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
+{
+	struct rpmsg_channel_info chinfo;
+	struct rpmsg_eptdev *eptdev;
+	struct device *dev = &rpdev->dev;
+
+	memcpy(chinfo.name, rpdev->id.name, RPMSG_NAME_SIZE);
+	chinfo.src = rpdev->src;
+	chinfo.dst = rpdev->dst;
+
+	eptdev = rpmsg_chrdev_eptdev_alloc(rpdev, dev);
+	if (IS_ERR(eptdev))
+		return PTR_ERR(eptdev);
+
+	/* Set the default_ept to the rpmsg device endpoint */
+	eptdev->default_ept = rpdev->ept;
+
+	/*
+	 * The rpmsg_ept_cb uses *priv parameter to get its rpmsg_eptdev context.
+	 * Storedit in default_ept *priv field.
+	 */
+	eptdev->default_ept->priv = eptdev;
+
+	return rpmsg_chrdev_eptdev_add(eptdev, chinfo);
+}
+
+static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev)
+{
+	int ret;
+
+	ret = device_for_each_child(&rpdev->dev, NULL, rpmsg_chrdev_eptdev_destroy);
+	if (ret)
+		dev_warn(&rpdev->dev, "failed to destroy endpoints: %d\n", ret);
+}
+
+static struct rpmsg_device_id rpmsg_chrdev_id_table[] = {
+	{ .name	= "rpmsg-raw" },
+	{ },
+};
+
+static struct rpmsg_driver rpmsg_chrdev_driver = {
+	.probe = rpmsg_chrdev_probe,
+	.remove = rpmsg_chrdev_remove,
+	.callback = rpmsg_ept_cb,
+	.id_table = rpmsg_chrdev_id_table,
+	.drv.name = "rpmsg_chrdev",
+};
+
 static int rpmsg_chrdev_init(void)
 {
 	int ret;
@@ -438,12 +486,24 @@ static int rpmsg_chrdev_init(void)
 		return ret;
 	}
 
+	ret = register_rpmsg_driver(&rpmsg_chrdev_driver);
+	if (ret < 0) {
+		pr_err("rpmsg: failed to register rpmsg raw driver\n");
+		goto free_region;
+	}
+
 	return 0;
+
+free_region:
+	unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX);
+
+	return ret;
 }
 postcore_initcall(rpmsg_chrdev_init);
 
 static void rpmsg_chrdev_exit(void)
 {
+	unregister_rpmsg_driver(&rpmsg_chrdev_driver);
 	unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX);
 }
 module_exit(rpmsg_chrdev_exit);
-- 
2.25.1

[PATCH v9 11/11] rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls

[Arnaud Pouliquen]

Allow the user space application to create and release an rpmsg device
by adding RPMSG_CREATE_DEV_IOCTL and RPMSG_RELEASE_DEV_IOCTL ioctrls to
the /dev/rpmsg_ctrl interface

The RPMSG_CREATE_DEV_IOCTL Ioctl can be used to instantiate a local rpmsg
device.
Depending on the back-end implementation, the associated rpmsg driver is
probed and a NS announcement can be sent to the remote processor.

The RPMSG_RELEASE_DEV_IOCTL allows the user application to release a
rpmsg device created either by the remote processor or with the
RPMSG_CREATE_DEV_IOCTL call.
Depending on the back-end implementation, the associated rpmsg driver is
removed and a NS destroy rpmsg can be sent to the remote processor.

Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/rpmsg/rpmsg_ctrl.c | 36 ++++++++++++++++++++++++++++++++----
 include/uapi/linux/rpmsg.h | 10 ++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_ctrl.c b/drivers/rpmsg/rpmsg_ctrl.c
index f43b5e4dbb4c..107da70fdbaa 100644
--- a/drivers/rpmsg/rpmsg_ctrl.c
+++ b/drivers/rpmsg/rpmsg_ctrl.c
@@ -43,11 +43,13 @@ static DEFINE_IDA(rpmsg_minor_ida);
  * @rpdev:	underlaying rpmsg device
  * @cdev:	cdev for the ctrl device
  * @dev:	device for the ctrl device
+ * @ctrl_lock:	serialize the ioctrls.
  */
 struct rpmsg_ctrldev {
 	struct rpmsg_device *rpdev;
 	struct cdev cdev;
 	struct device dev;
+	struct mutex ctrl_lock;
 };
 
 static int rpmsg_ctrldev_open(struct inode *inode, struct file *filp)
@@ -76,9 +78,8 @@ static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd,
 	void __user *argp = (void __user *)arg;
 	struct rpmsg_endpoint_info eptinfo;
 	struct rpmsg_channel_info chinfo;
-
-	if (cmd != RPMSG_CREATE_EPT_IOCTL)
-		return -EINVAL;
+	struct rpmsg_device *rpdev;
+	int ret = 0;
 
 	if (copy_from_user(&eptinfo, argp, sizeof(eptinfo)))
 		return -EFAULT;
@@ -88,7 +89,33 @@ static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd,
 	chinfo.src = eptinfo.src;
 	chinfo.dst = eptinfo.dst;
 
-	return rpmsg_chrdev_eptdev_create(ctrldev->rpdev, &ctrldev->dev, chinfo);
+	mutex_lock(&ctrldev->ctrl_lock);
+	switch (cmd) {
+	case RPMSG_CREATE_EPT_IOCTL:
+		ret = rpmsg_chrdev_eptdev_create(ctrldev->rpdev, &ctrldev->dev, chinfo);
+		break;
+
+	case RPMSG_CREATE_DEV_IOCTL:
+		rpdev = rpmsg_create_channel(ctrldev->rpdev, &chinfo);
+		if (!rpdev) {
+			dev_err(&ctrldev->dev, "failed to create %s channel\n", chinfo.name);
+			ret = -ENXIO;
+		}
+		break;
+
+	case RPMSG_RELEASE_DEV_IOCTL:
+		ret = rpmsg_release_channel(ctrldev->rpdev, &chinfo);
+		if (ret)
+			dev_err(&ctrldev->dev, "failed to release %s channel (%d)\n",
+				chinfo.name, ret);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+	mutex_unlock(&ctrldev->ctrl_lock);
+
+	return ret;
 };
 
 static const struct file_operations rpmsg_ctrldev_fops = {
@@ -125,6 +152,7 @@ static int rpmsg_ctrldev_probe(struct rpmsg_device *rpdev)
 	dev->parent = &rpdev->dev;
 	dev->class = rpmsg_class;
 
+	mutex_init(&ctrldev->ctrl_lock);
 	cdev_init(&ctrldev->cdev, &rpmsg_ctrldev_fops);
 	ctrldev->cdev.owner = THIS_MODULE;
 
diff --git a/include/uapi/linux/rpmsg.h b/include/uapi/linux/rpmsg.h
index f5ca8740f3fb..1637e68177d9 100644
--- a/include/uapi/linux/rpmsg.h
+++ b/include/uapi/linux/rpmsg.h
@@ -33,4 +33,14 @@ struct rpmsg_endpoint_info {
  */
 #define RPMSG_DESTROY_EPT_IOCTL	_IO(0xb5, 0x2)
 
+/**
+ * Instantiate a new local rpmsg service device.
+ */
+#define RPMSG_CREATE_DEV_IOCTL	_IOW(0xb5, 0x3, struct rpmsg_endpoint_info)
+
+/**
+ * Release a local rpmsg device.
+ */
+#define RPMSG_RELEASE_DEV_IOCTL	_IOW(0xb5, 0x4, struct rpmsg_endpoint_info)
+
 #endif
-- 
2.25.1

Re: [PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Philipp Rossak]

Hi Arnaud,

thanks for working on this! I'm currently testing/using this patch 
series on my imx7d project because it adds the capability that the 
remote processor can register it's endpoints dynamically (as mentioned 
in the objectives).

After a few tests, debugging, and checking the openamp specification [1] 
I think that you missed the second ns_announcement that should be sent 
from linux master to the slave after it created the channel/endpoint. 
Without this second announcement the remote processor is not able to 
send messages to the linux master because it doesn't know the 
destination address until it receives a message from the linux master.

Cheers,
Philipp


[1]: 
https://github.com/OpenAMP/open-amp/blob/main/docs/img/coprocessor-rpmsg-ns.png

On 24.01.22 11:25, Arnaud Pouliquen wrote:
> Updates from V8 [1]:
> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
> - updates based on Bjorn Andersson's comments:
>    - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
>      function.
>    - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
> 
> Patchset description:
> 
> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
> remote processor.
> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
> in the generic rpmsg virtio backend.
> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
> in order to improve its compatible with the rpmsg virtio backend.
> 
> Objective:
> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
>    rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
>    rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
>    RPMSG_DESTROY_DEV_IOCTL controls).
>    An application will be able to create/establish an rpmsg communication channel to communicate
>    with the remote processor, and not only wait the remote processor initiative.
>    This is interesting for example to establish a temporary communication link for diagnosis,
>    calibration, debugging... or instantiate  new data flows on some user actions.
> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
>   (rpmsg service announcement mechanism).
>    This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
>    a rpmsg name service announcement.
> 
> Subsets:
>    - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
>    - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>    - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>      devices (patch 11)
>      The application can then create or release a channel by specifying:
>         - the name service of the device to instantiate.
>         - the source address.
>         - the destination address.
> 
> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
> rpmsg_char cdev release fixes [2][3]
> 
> [1] https://lkml.org/lkml/2021/12/7/186
> [2] https://lkml.org/lkml/2022/1/10/1129
> [3] https://lkml.org/lkml/2022/1/10/1130
> 
> Arnaud Pouliquen (11):
>    rpmsg: char: Export eptdev create and destroy functions
>    rpmsg: Create the rpmsg class in core instead of in rpmsg char
>    rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
>    arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>    RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>    arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
>    rpmsg: Update rpmsg_chrdev_register_device function
>    rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>    rpmsg: char: Add possibility to use default endpoint of the rpmsg
>      device
>    rpmsg: char: Introduce the "rpmsg-raw" channel
>    rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
> 
>   arch/arm/configs/qcom_defconfig   |   1 +
>   arch/arm64/configs/defconfig      |   1 +
>   arch/riscv/configs/defconfig      |   1 +
>   arch/riscv/configs/rv32_defconfig |   1 +
>   drivers/rpmsg/Kconfig             |   8 +
>   drivers/rpmsg/Makefile            |   1 +
>   drivers/rpmsg/qcom_glink_native.c |   2 +-
>   drivers/rpmsg/qcom_smd.c          |   2 +-
>   drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>   drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>   drivers/rpmsg/rpmsg_core.c        |  15 +-
>   drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>   drivers/rpmsg/rpmsg_internal.h    |  10 +-
>   drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>   include/uapi/linux/rpmsg.h        |  10 ++
>   15 files changed, 419 insertions(+), 155 deletions(-)
>   create mode 100644 drivers/rpmsg/rpmsg_char.h
>   create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
> 

Re: [PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Arnaud POULIQUEN]

Hi Philipp,

On 2/23/22 22:28, Philipp Rossak wrote:
> Hi Arnaud,
> 
> thanks for working on this! I'm currently testing/using this patch
> series on my imx7d project because it adds the capability that the
> remote processor can register it's endpoints dynamically (as mentioned
> in the objectives).

Thanks for your feedback on this work! 
Don't hesitate to add your tested-by, this help maintainers for the reviews. 

> 
> After a few tests, debugging, and checking the openamp specification [1]
> I think that you missed the second ns_announcement that should be sent
> from linux master to the slave after it created the channel/endpoint.
> Without this second announcement the remote processor is not able to
> send messages to the linux master because it doesn't know the
> destination address until it receives a message from the linux master.

Yes I detected this issues, it is not related to the series
but to the remoteproc_virtio backend.

As you mentioned, after the ns announcement from Linux, the remote processor
send first messages. But the Linux virtio does not do the match between the
local channel created and the remote endpoint.

This is a feature that is missing in the rpmsg virtio, and perhaps in rpmsg protocol
itself (a ns annoucement ack message or something similar).


A fix for the remoteproc virtio is available here:
https://github.com/arnopo/meta-st-stm32mp-oss/commit/3e57fe73bd19c9bb835ac5a118e50727758b0b96

Don't hesitate to give me feedback on the fix, if you test it.

I plan to propose the fix after this series.    

Thanks,
Arnaud

> 
> Cheers,
> Philipp
> 
> 
> [1]:
> https://github.com/OpenAMP/open-amp/blob/main/docs/img/coprocessor-rpmsg-ns.png
> 
> 
> On 24.01.22 11:25, Arnaud Pouliquen wrote:
>> Updates from V8 [1]:
>> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
>> - updates based on Bjorn Andersson's comments:
>>    - remove rpmsg_create_default_ept API, set directly the ept->priv
>> in rpmsg_chrdev_probe
>>      function.
>>    - rework commit message in [8/9]rpmsg: char: Introduce the
>> "rpmsg-raw" channel
>>
>> Patchset description:
>>
>> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface
>> that provides the ability to
>> instantiate char devices (/dev/rpmsgX) associated with an rpmsg
>> endpoint for communication with the
>> remote processor.
>> This implementation fits with QCOM rpmsg backend but not with the
>> magement by chanel implemented
>> in the generic rpmsg virtio backend.
>> This series restructures the rpmsg_char driver to decorrelate the
>> control part from the data part
>> in order to improve its compatible with the rpmsg virtio backend.
>>
>> Objective:
>> - Expose a /dev/rpmsg_ctrlX interface for the application that is no
>> longer dedicated to the
>>    rpmsg_char but generalized to all rpmsg services. This offers
>> capability to create and destroy
>>    rpmsg channels from a user's application initiative (using the new
>> RPMSG_CREATE_DEV_IOCTL and
>>    RPMSG_DESTROY_DEV_IOCTL controls).
>>    An application will be able to create/establish an rpmsg
>> communication channel to communicate
>>    with the remote processor, and not only wait the remote processor
>> initiative.
>>    This is interesting for example to establish a temporary
>> communication link for diagnosis,
>>    calibration, debugging... or instantiate  new data flows on some
>> user actions.
>> - Add capability to probe the rpmsg_char device at the initiative of
>> the remote processor
>>   (rpmsg service announcement mechanism).
>>    This allows platforms based on the rpmsg virtio backend to create
>> the /dev/rpmgX interface with
>>    a rpmsg name service announcement.
>>
>> Subsets:
>>    - Extract the control part of the char dev and create the
>> rpmsg_ctrl.c file (patches 1 to 6)
>>    - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>>    - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and
>> RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>>      devices (patch 11)
>>      The application can then create or release a channel by specifying:
>>         - the name service of the device to instantiate.
>>         - the source address.
>>         - the destination address.
>>
>> This series has be applied and tested on 'commit e783362eb54c ("Linux
>> 5.17-rc1") +
>> rpmsg_char cdev release fixes [2][3]
>>
>> [1] https://lkml.org/lkml/2021/12/7/186
>> [2] https://lkml.org/lkml/2022/1/10/1129
>> [3] https://lkml.org/lkml/2022/1/10/1130
>>
>> Arnaud Pouliquen (11):
>>    rpmsg: char: Export eptdev create and destroy functions
>>    rpmsg: Create the rpmsg class in core instead of in rpmsg char
>>    rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
>>    arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>    RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>    arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
>>    rpmsg: Update rpmsg_chrdev_register_device function
>>    rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>>    rpmsg: char: Add possibility to use default endpoint of the rpmsg
>>      device
>>    rpmsg: char: Introduce the "rpmsg-raw" channel
>>    rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
>>
>>   arch/arm/configs/qcom_defconfig   |   1 +
>>   arch/arm64/configs/defconfig      |   1 +
>>   arch/riscv/configs/defconfig      |   1 +
>>   arch/riscv/configs/rv32_defconfig |   1 +
>>   drivers/rpmsg/Kconfig             |   8 +
>>   drivers/rpmsg/Makefile            |   1 +
>>   drivers/rpmsg/qcom_glink_native.c |   2 +-
>>   drivers/rpmsg/qcom_smd.c          |   2 +-
>>   drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>>   drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>>   drivers/rpmsg/rpmsg_core.c        |  15 +-
>>   drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>>   drivers/rpmsg/rpmsg_internal.h    |  10 +-
>>   drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>>   include/uapi/linux/rpmsg.h        |  10 ++
>>   15 files changed, 419 insertions(+), 155 deletions(-)
>>   create mode 100644 drivers/rpmsg/rpmsg_char.h
>>   create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
>>

Re: [PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Philipp Rossak]

Hi Arnaud,

On 24.02.22 09:29, Arnaud POULIQUEN wrote:
> Hi Philipp,
> 
> On 2/23/22 22:28, Philipp Rossak wrote:
>> Hi Arnaud,
>>
>> thanks for working on this! I'm currently testing/using this patch
>> series on my imx7d project because it adds the capability that the
>> remote processor can register it's endpoints dynamically (as mentioned
>> in the objectives).
> 
> Thanks for your feedback on this work!
> Don't hesitate to add your tested-by, this help maintainers for the reviews.
> 
I will do this.
>>
>> After a few tests, debugging, and checking the openamp specification [1]
>> I think that you missed the second ns_announcement that should be sent
>> from linux master to the slave after it created the channel/endpoint.
>> Without this second announcement the remote processor is not able to
>> send messages to the linux master because it doesn't know the
>> destination address until it receives a message from the linux master.
> 
> Yes I detected this issues, it is not related to the series
> but to the remoteproc_virtio backend.
> 
> As you mentioned, after the ns announcement from Linux, the remote processor
> send first messages. But the Linux virtio does not do the match between the
> local channel created and the remote endpoint.
> 

I'm not sure if we talk about the same. I'm basically talking about the 
dynamic binding, not dynamic endpoint creation.
I think I already found the issue. I will try to get a bit more into detail.

1. Linux: starts co-processor via remoteproc
2. co-processor: boots and reaches the point where it creates the 
endpoint like it is done in this ST example[1].
Be aware the src address is RPMSG_ADDR_ANY
3. co-processor: reaches the point where it sends the ns_announcement to 
linux ns endpoint
4. linux: receives the ns announcment, creates the channel, bindes the 
endpoint and checks here [2] if the source address is not RPMSG_ADDR_ANY 
and in this case it is not sending a ns_announcement (that's the issue 
when we use dynamic endpoints)
5. linux: according the openamp spec [3] it should now send the 
ns_announcement to the co-processor (slave)
6. co-processor: should receive the ns announcement and binds now the 
endpoint
7. co-processor: can now send messages to linux

This is basically what I'm expecting.


Do you think this is a bug or is the dynamic endpoint binding not 
handled? This line is there since ever [4] ...

Any other thoughts about this?

> This is a feature that is missing in the rpmsg virtio, and perhaps in rpmsg protocol
> itself (a ns annoucement ack message or something similar).
> 
> 
> A fix for the remoteproc virtio is available here:
> https://github.com/arnopo/meta-st-stm32mp-oss/commit/3e57fe73bd19c9bb835ac5a118e50727758b0b96
> 
> Don't hesitate to give me feedback on the fix, if you test it.

I added it to my branch and till now I don't see any side effects
> 
> I plan to propose the fix after this series.
> 
> Thanks,
> Arnaud
> 
>>
>> Cheers,
>> Philipp
>>

Cheers,
Philipp

[1]: 
https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_raw/Src/openamp.c#L242

[2]: 
https://elixir.bootlin.com/linux/v5.17-rc5/source/drivers/rpmsg/virtio_rpmsg_bus.c#L425

[3]: 
https://github.com/OpenAMP/open-amp/blob/main/docs/img/coprocessor-rpmsg-ns.png

[4]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcabbccabffe7326f046f25737ba1084f463c65c

Re: [PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Arnaud POULIQUEN]

Hi Philipp,

On 2/25/22 22:45, Philipp Rossak wrote:
> Hi Arnaud,
> 
> On 24.02.22 09:29, Arnaud POULIQUEN wrote:
>> Hi Philipp,
>>
>> On 2/23/22 22:28, Philipp Rossak wrote:
>>> Hi Arnaud,
>>>
>>> thanks for working on this! I'm currently testing/using this patch
>>> series on my imx7d project because it adds the capability that the
>>> remote processor can register it's endpoints dynamically (as mentioned
>>> in the objectives).
>>
>> Thanks for your feedback on this work!
>> Don't hesitate to add your tested-by, this help maintainers for the
>> reviews.
>>
> I will do this.
>>>
>>> After a few tests, debugging, and checking the openamp specification [1]
>>> I think that you missed the second ns_announcement that should be sent
>>> from linux master to the slave after it created the channel/endpoint.
>>> Without this second announcement the remote processor is not able to
>>> send messages to the linux master because it doesn't know the
>>> destination address until it receives a message from the linux master.
>>
>> Yes I detected this issues, it is not related to the series
>> but to the remoteproc_virtio backend.
>>
>> As you mentioned, after the ns announcement from Linux, the remote
>> processor
>> send first messages. But the Linux virtio does not do the match
>> between the
>> local channel created and the remote endpoint.
>>
> 
> I'm not sure if we talk about the same. I'm basically talking about the
> dynamic binding, not dynamic endpoint creation.

Regarding your following description, yes it is not exactly the same issue.
  

> I think I already found the issue. I will try to get a bit more into
> detail.
> 
> 1. Linux: starts co-processor via remoteproc
> 2. co-processor: boots and reaches the point where it creates the
> endpoint like it is done in this ST example[1].
> Be aware the src address is RPMSG_ADDR_ANY
> 3. co-processor: reaches the point where it sends the ns_announcement to
> linux ns endpoint
> 4. linux: receives the ns announcment, creates the channel, bindes the
> endpoint and checks here [2] if the source address is not RPMSG_ADDR_ANY
> and in this case it is not sending a ns_announcement (that's the issue
> when we use dynamic endpoints)

The ns annoucement is used to notify the remote processor that a new channel
has been created locally. Today the ns anoucement is not used to inform that

The local endpoint has been binded.

This behavior is something that as already been identified as a limitation in
the virtio rpmsg.

Xiang Xiao had proposed some time ago a mechanism for the OpenAMP [5], the
Linux part is missing. We need a common solution between Linux and OpenAMP, but
that also compatible with legacy. 
From MPOV Xiang's approach seem a good starting point.  
If you are interesting on working on this enhancement of the rpmsg virtio, feel
free to do so.

> 5. linux: according the openamp spec [3] it should now send the
> ns_announcement to the co-processor (slave)
> 6. co-processor: should receive the ns announcement and binds now the
> endpoint
> 7. co-processor: can now send messages to linux
> 
> This is basically what I'm expecting.
> 
> 
> Do you think this is a bug or is the dynamic endpoint binding not
> handled? This line is there since ever [4] ...

As you mentioned this is a legacy limitation that should be addressed.
It is not related to this work.

Thanks,
Arnaud

> 
> Any other thoughts about this?
> 
>> This is a feature that is missing in the rpmsg virtio, and perhaps in
>> rpmsg protocol
>> itself (a ns annoucement ack message or something similar).
>>
>>
>> A fix for the remoteproc virtio is available here:
>> https://github.com/arnopo/meta-st-stm32mp-oss/commit/3e57fe73bd19c9bb835ac5a118e50727758b0b96
>>
>>
>> Don't hesitate to give me feedback on the fix, if you test it.
> 
> I added it to my branch and till now I don't see any side effects
>>
>> I plan to propose the fix after this series.
>>
>> Thanks,
>> Arnaud
>>
>>>
>>> Cheers,
>>> Philipp
>>>
> 
> Cheers,
> Philipp
> 
> [1]:
> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_raw/Src/openamp.c#L242
> 
> 
> [2]:
> https://elixir.bootlin.com/linux/v5.17-rc5/source/drivers/rpmsg/virtio_rpmsg_bus.c#L425
> 
> 
> [3]:
> https://github.com/OpenAMP/open-amp/blob/main/docs/img/coprocessor-rpmsg-ns.png
> 
> 
> [4]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcabbccabffe7326f046f25737ba1084f463c65c
> 

[5] https://github.com/OpenAMP/open-amp/pull/160

Re: [PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Arnaud POULIQUEN]

Hi Bjorn,

On 1/24/22 11:25, Arnaud Pouliquen wrote:
> Updates from V8 [1]:
> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
> - updates based on Bjorn Andersson's comments:
>   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
>     function.
>   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
> 
> Patchset description:
> 
> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
> remote processor.
> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
> in the generic rpmsg virtio backend.
> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
> in order to improve its compatible with the rpmsg virtio backend.
> 
> Objective:
> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
>   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
>   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
>   RPMSG_DESTROY_DEV_IOCTL controls).
>   An application will be able to create/establish an rpmsg communication channel to communicate
>   with the remote processor, and not only wait the remote processor initiative.
>   This is interesting for example to establish a temporary communication link for diagnosis,
>   calibration, debugging... or instantiate  new data flows on some user actions.
> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
>  (rpmsg service announcement mechanism).
>   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
>   a rpmsg name service announcement.
> 
> Subsets:
>   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
>   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>     devices (patch 11)
>     The application can then create or release a channel by specifying:
>        - the name service of the device to instantiate.   
>        - the source address.
>        - the destination address.
> 
> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
> rpmsg_char cdev release fixes [2][3]
> 
> [1] https://lkml.org/lkml/2021/12/7/186
> [2] https://lkml.org/lkml/2022/1/10/1129
> [3] https://lkml.org/lkml/2022/1/10/1130
> 
> Arnaud Pouliquen (11):
>   rpmsg: char: Export eptdev create and destroy functions
>   rpmsg: Create the rpmsg class in core instead of in rpmsg char
>   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl


>   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL

Thank you for merging this series!

I can't see in the "for next" branch[1] the 3 patches above that update configs
Are you expecting a specific action from me?   

[1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git

Regards,
Arnaud

>   rpmsg: Update rpmsg_chrdev_register_device function
>   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>   rpmsg: char: Add possibility to use default endpoint of the rpmsg
>     device
>   rpmsg: char: Introduce the "rpmsg-raw" channel
>   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
> 
>  arch/arm/configs/qcom_defconfig   |   1 +
>  arch/arm64/configs/defconfig      |   1 +
>  arch/riscv/configs/defconfig      |   1 +
>  arch/riscv/configs/rv32_defconfig |   1 +
>  drivers/rpmsg/Kconfig             |   8 +
>  drivers/rpmsg/Makefile            |   1 +
>  drivers/rpmsg/qcom_glink_native.c |   2 +-
>  drivers/rpmsg/qcom_smd.c          |   2 +-
>  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>  drivers/rpmsg/rpmsg_core.c        |  15 +-
>  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>  drivers/rpmsg/rpmsg_internal.h    |  10 +-
>  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>  include/uapi/linux/rpmsg.h        |  10 ++
>  15 files changed, 419 insertions(+), 155 deletions(-)
>  create mode 100644 drivers/rpmsg/rpmsg_char.h
>  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
> 

Re: [PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Mathieu Poirier]

On Thu, Mar 24, 2022 at 06:36:23PM +0100, Arnaud POULIQUEN wrote:
> Hi Bjorn,
> 
> On 1/24/22 11:25, Arnaud Pouliquen wrote:
> > Updates from V8 [1]:
> > - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
> > - updates based on Bjorn Andersson's comments:
> >   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
> >     function.
> >   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
> > 
> > Patchset description:
> > 
> > The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
> > instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
> > remote processor.
> > This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
> > in the generic rpmsg virtio backend.
> > This series restructures the rpmsg_char driver to decorrelate the control part from the data part
> > in order to improve its compatible with the rpmsg virtio backend.
> > 
> > Objective:
> > - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
> >   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
> >   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
> >   RPMSG_DESTROY_DEV_IOCTL controls).
> >   An application will be able to create/establish an rpmsg communication channel to communicate
> >   with the remote processor, and not only wait the remote processor initiative.
> >   This is interesting for example to establish a temporary communication link for diagnosis,
> >   calibration, debugging... or instantiate  new data flows on some user actions.
> > - Add capability to probe the rpmsg_char device at the initiative of the remote processor
> >  (rpmsg service announcement mechanism).
> >   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
> >   a rpmsg name service announcement.
> > 
> > Subsets:
> >   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
> >   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
> >   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
> >     devices (patch 11)
> >     The application can then create or release a channel by specifying:
> >        - the name service of the device to instantiate.   
> >        - the source address.
> >        - the destination address.
> > 
> > This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
> > rpmsg_char cdev release fixes [2][3]
> > 
> > [1] https://lkml.org/lkml/2021/12/7/186
> > [2] https://lkml.org/lkml/2022/1/10/1129
> > [3] https://lkml.org/lkml/2022/1/10/1130
> > 
> > Arnaud Pouliquen (11):
> >   rpmsg: char: Export eptdev create and destroy functions
> >   rpmsg: Create the rpmsg class in core instead of in rpmsg char
> >   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
> 
> 
> >   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
> >   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
> >   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
> 
> Thank you for merging this series!
> 
> I can't see in the "for next" branch[1] the 3 patches above that update configs
> Are you expecting a specific action from me?   

Those patches will need to go through the Arm, RISC-V and arm64 subsystems.  The
mailing list for those subsystems has been CC'ed but that isn't enough to get
the maintainers' attention.  

I suggest sending another patchset with those 3 patches that CC the maintainers
directly.  For the Arm patch I suggest adding Linus Walleij.

Thanks,
Mathieu

> 
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
> 
> Regards,
> Arnaud
> 
> >   rpmsg: Update rpmsg_chrdev_register_device function
> >   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
> >   rpmsg: char: Add possibility to use default endpoint of the rpmsg
> >     device
> >   rpmsg: char: Introduce the "rpmsg-raw" channel
> >   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
> > 
> >  arch/arm/configs/qcom_defconfig   |   1 +
> >  arch/arm64/configs/defconfig      |   1 +
> >  arch/riscv/configs/defconfig      |   1 +
> >  arch/riscv/configs/rv32_defconfig |   1 +
> >  drivers/rpmsg/Kconfig             |   8 +
> >  drivers/rpmsg/Makefile            |   1 +
> >  drivers/rpmsg/qcom_glink_native.c |   2 +-
> >  drivers/rpmsg/qcom_smd.c          |   2 +-
> >  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
> >  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
> >  drivers/rpmsg/rpmsg_core.c        |  15 +-
> >  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
> >  drivers/rpmsg/rpmsg_internal.h    |  10 +-
> >  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
> >  include/uapi/linux/rpmsg.h        |  10 ++
> >  15 files changed, 419 insertions(+), 155 deletions(-)
> >  create mode 100644 drivers/rpmsg/rpmsg_char.h
> >  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
> > 

Re: [PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Arnaud POULIQUEN]

On 3/25/22 16:59, Mathieu Poirier wrote:
> On Thu, Mar 24, 2022 at 06:36:23PM +0100, Arnaud POULIQUEN wrote:
>> Hi Bjorn,
>>
>> On 1/24/22 11:25, Arnaud Pouliquen wrote:
>>> Updates from V8 [1]:
>>> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
>>> - updates based on Bjorn Andersson's comments:
>>>   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
>>>     function.
>>>   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
>>>
>>> Patchset description:
>>>
>>> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
>>> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
>>> remote processor.
>>> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
>>> in the generic rpmsg virtio backend.
>>> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
>>> in order to improve its compatible with the rpmsg virtio backend.
>>>
>>> Objective:
>>> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
>>>   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
>>>   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
>>>   RPMSG_DESTROY_DEV_IOCTL controls).
>>>   An application will be able to create/establish an rpmsg communication channel to communicate
>>>   with the remote processor, and not only wait the remote processor initiative.
>>>   This is interesting for example to establish a temporary communication link for diagnosis,
>>>   calibration, debugging... or instantiate  new data flows on some user actions.
>>> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
>>>  (rpmsg service announcement mechanism).
>>>   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
>>>   a rpmsg name service announcement.
>>>
>>> Subsets:
>>>   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
>>>   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>>>   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>>>     devices (patch 11)
>>>     The application can then create or release a channel by specifying:
>>>        - the name service of the device to instantiate.   
>>>        - the source address.
>>>        - the destination address.
>>>
>>> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
>>> rpmsg_char cdev release fixes [2][3]
>>>
>>> [1] https://lkml.org/lkml/2021/12/7/186
>>> [2] https://lkml.org/lkml/2022/1/10/1129
>>> [3] https://lkml.org/lkml/2022/1/10/1130
>>>
>>> Arnaud Pouliquen (11):
>>>   rpmsg: char: Export eptdev create and destroy functions
>>>   rpmsg: Create the rpmsg class in core instead of in rpmsg char
>>>   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
>>
>>
>>>   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>>   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>>   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
>>
>> Thank you for merging this series!
>>
>> I can't see in the "for next" branch[1] the 3 patches above that update configs
>> Are you expecting a specific action from me?   
> 
> Those patches will need to go through the Arm, RISC-V and arm64 subsystems.  The
> mailing list for those subsystems has been CC'ed but that isn't enough to get
> the maintainers' attention.  
> 
> I suggest sending another patchset with those 3 patches that CC the maintainers
> directly.  For the Arm patch I suggest adding Linus Walleij.

I will do what you suggest. 

My concerns in this case is about the scheduling of the integration.
I suppose that sending a second patchset for configs requests that the
rpmsg char series is first applied
But on the other hand this may lead to some failures as the RPMSG_CTRL is now
needed to create the /dev/rpmsg_ctrl0

so probably, I need to do this as fixup patch.

FYI the RISC-V patch as been reviewed by Anup Patel

Thanks,
Arnaud

> 
> Thanks,
> Mathieu
> 
>>
>> [1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
>>
>> Regards,
>> Arnaud
>>
>>>   rpmsg: Update rpmsg_chrdev_register_device function
>>>   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>>>   rpmsg: char: Add possibility to use default endpoint of the rpmsg
>>>     device
>>>   rpmsg: char: Introduce the "rpmsg-raw" channel
>>>   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
>>>
>>>  arch/arm/configs/qcom_defconfig   |   1 +
>>>  arch/arm64/configs/defconfig      |   1 +
>>>  arch/riscv/configs/defconfig      |   1 +
>>>  arch/riscv/configs/rv32_defconfig |   1 +
>>>  drivers/rpmsg/Kconfig             |   8 +
>>>  drivers/rpmsg/Makefile            |   1 +
>>>  drivers/rpmsg/qcom_glink_native.c |   2 +-
>>>  drivers/rpmsg/qcom_smd.c          |   2 +-
>>>  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>>>  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>>>  drivers/rpmsg/rpmsg_core.c        |  15 +-
>>>  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>>>  drivers/rpmsg/rpmsg_internal.h    |  10 +-
>>>  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>>>  include/uapi/linux/rpmsg.h        |  10 ++
>>>  15 files changed, 419 insertions(+), 155 deletions(-)
>>>  create mode 100644 drivers/rpmsg/rpmsg_char.h
>>>  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
>>>

Re: [PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Mathieu Poirier]

On Fri, 25 Mar 2022 at 11:05, Arnaud POULIQUEN
<arnaud.pouliquen@foss.st.com> wrote:
>
>
>
> On 3/25/22 16:59, Mathieu Poirier wrote:
> > On Thu, Mar 24, 2022 at 06:36:23PM +0100, Arnaud POULIQUEN wrote:
> >> Hi Bjorn,
> >>
> >> On 1/24/22 11:25, Arnaud Pouliquen wrote:
> >>> Updates from V8 [1]:
> >>> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
> >>> - updates based on Bjorn Andersson's comments:
> >>>   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
> >>>     function.
> >>>   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
> >>>
> >>> Patchset description:
> >>>
> >>> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
> >>> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
> >>> remote processor.
> >>> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
> >>> in the generic rpmsg virtio backend.
> >>> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
> >>> in order to improve its compatible with the rpmsg virtio backend.
> >>>
> >>> Objective:
> >>> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
> >>>   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
> >>>   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
> >>>   RPMSG_DESTROY_DEV_IOCTL controls).
> >>>   An application will be able to create/establish an rpmsg communication channel to communicate
> >>>   with the remote processor, and not only wait the remote processor initiative.
> >>>   This is interesting for example to establish a temporary communication link for diagnosis,
> >>>   calibration, debugging... or instantiate  new data flows on some user actions.
> >>> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
> >>>  (rpmsg service announcement mechanism).
> >>>   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
> >>>   a rpmsg name service announcement.
> >>>
> >>> Subsets:
> >>>   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
> >>>   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
> >>>   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
> >>>     devices (patch 11)
> >>>     The application can then create or release a channel by specifying:
> >>>        - the name service of the device to instantiate.
> >>>        - the source address.
> >>>        - the destination address.
> >>>
> >>> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
> >>> rpmsg_char cdev release fixes [2][3]
> >>>
> >>> [1] https://lkml.org/lkml/2021/12/7/186
> >>> [2] https://lkml.org/lkml/2022/1/10/1129
> >>> [3] https://lkml.org/lkml/2022/1/10/1130
> >>>
> >>> Arnaud Pouliquen (11):
> >>>   rpmsg: char: Export eptdev create and destroy functions
> >>>   rpmsg: Create the rpmsg class in core instead of in rpmsg char
> >>>   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
> >>
> >>
> >>>   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
> >>>   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
> >>>   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
> >>
> >> Thank you for merging this series!
> >>
> >> I can't see in the "for next" branch[1] the 3 patches above that update configs
> >> Are you expecting a specific action from me?
> >
> > Those patches will need to go through the Arm, RISC-V and arm64 subsystems.  The
> > mailing list for those subsystems has been CC'ed but that isn't enough to get
> > the maintainers' attention.
> >
> > I suggest sending another patchset with those 3 patches that CC the maintainers
> > directly.  For the Arm patch I suggest adding Linus Walleij.
>
> I will do what you suggest.
>
> My concerns in this case is about the scheduling of the integration.
> I suppose that sending a second patchset for configs requests that the
> rpmsg char series is first applied

Right, but the rpmsg_char series has been applied.

> But on the other hand this may lead to some failures as the RPMSG_CTRL is now
> needed to create the /dev/rpmsg_ctrl0
>

Possibly, but right now there is no other way.

> so probably, I need to do this as fixup patch.
>

Indeed, this can be applied as a fix in rc1.

> FYI the RISC-V patch as been reviewed by Anup Patel
>

... but Anup does not maintain any of the defconfig files.

> Thanks,
> Arnaud
>
> >
> > Thanks,
> > Mathieu
> >
> >>
> >> [1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
> >>
> >> Regards,
> >> Arnaud
> >>
> >>>   rpmsg: Update rpmsg_chrdev_register_device function
> >>>   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
> >>>   rpmsg: char: Add possibility to use default endpoint of the rpmsg
> >>>     device
> >>>   rpmsg: char: Introduce the "rpmsg-raw" channel
> >>>   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
> >>>
> >>>  arch/arm/configs/qcom_defconfig   |   1 +
> >>>  arch/arm64/configs/defconfig      |   1 +
> >>>  arch/riscv/configs/defconfig      |   1 +
> >>>  arch/riscv/configs/rv32_defconfig |   1 +
> >>>  drivers/rpmsg/Kconfig             |   8 +
> >>>  drivers/rpmsg/Makefile            |   1 +
> >>>  drivers/rpmsg/qcom_glink_native.c |   2 +-
> >>>  drivers/rpmsg/qcom_smd.c          |   2 +-
> >>>  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
> >>>  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
> >>>  drivers/rpmsg/rpmsg_core.c        |  15 +-
> >>>  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
> >>>  drivers/rpmsg/rpmsg_internal.h    |  10 +-
> >>>  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
> >>>  include/uapi/linux/rpmsg.h        |  10 ++
> >>>  15 files changed, 419 insertions(+), 155 deletions(-)
> >>>  create mode 100644 drivers/rpmsg/rpmsg_char.h
> >>>  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
> >>>

Re: [PATCH v9 00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

[Arnaud POULIQUEN]

On 3/25/22 18:27, Mathieu Poirier wrote:
> On Fri, 25 Mar 2022 at 11:05, Arnaud POULIQUEN
> <arnaud.pouliquen@foss.st.com> wrote:
>>
>>
>>
>> On 3/25/22 16:59, Mathieu Poirier wrote:
>>> On Thu, Mar 24, 2022 at 06:36:23PM +0100, Arnaud POULIQUEN wrote:
>>>> Hi Bjorn,
>>>>
>>>> On 1/24/22 11:25, Arnaud Pouliquen wrote:
>>>>> Updates from V8 [1]:
>>>>> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
>>>>> - updates based on Bjorn Andersson's comments:
>>>>>   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
>>>>>     function.
>>>>>   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
>>>>>
>>>>> Patchset description:
>>>>>
>>>>> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
>>>>> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
>>>>> remote processor.
>>>>> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
>>>>> in the generic rpmsg virtio backend.
>>>>> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
>>>>> in order to improve its compatible with the rpmsg virtio backend.
>>>>>
>>>>> Objective:
>>>>> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
>>>>>   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
>>>>>   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
>>>>>   RPMSG_DESTROY_DEV_IOCTL controls).
>>>>>   An application will be able to create/establish an rpmsg communication channel to communicate
>>>>>   with the remote processor, and not only wait the remote processor initiative.
>>>>>   This is interesting for example to establish a temporary communication link for diagnosis,
>>>>>   calibration, debugging... or instantiate  new data flows on some user actions.
>>>>> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
>>>>>  (rpmsg service announcement mechanism).
>>>>>   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
>>>>>   a rpmsg name service announcement.
>>>>>
>>>>> Subsets:
>>>>>   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
>>>>>   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>>>>>   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>>>>>     devices (patch 11)
>>>>>     The application can then create or release a channel by specifying:
>>>>>        - the name service of the device to instantiate.
>>>>>        - the source address.
>>>>>        - the destination address.
>>>>>
>>>>> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
>>>>> rpmsg_char cdev release fixes [2][3]
>>>>>
>>>>> [1] https://lkml.org/lkml/2021/12/7/186
>>>>> [2] https://lkml.org/lkml/2022/1/10/1129
>>>>> [3] https://lkml.org/lkml/2022/1/10/1130
>>>>>
>>>>> Arnaud Pouliquen (11):
>>>>>   rpmsg: char: Export eptdev create and destroy functions
>>>>>   rpmsg: Create the rpmsg class in core instead of in rpmsg char
>>>>>   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
>>>>
>>>>
>>>>>   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>>>>   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>>>>   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
>>>>
>>>> Thank you for merging this series!
>>>>
>>>> I can't see in the "for next" branch[1] the 3 patches above that update configs
>>>> Are you expecting a specific action from me?
>>>
>>> Those patches will need to go through the Arm, RISC-V and arm64 subsystems.  The
>>> mailing list for those subsystems has been CC'ed but that isn't enough to get
>>> the maintainers' attention.
>>>
>>> I suggest sending another patchset with those 3 patches that CC the maintainers
>>> directly.  For the Arm patch I suggest adding Linus Walleij.
>>
>> I will do what you suggest.
>>
>> My concerns in this case is about the scheduling of the integration.
>> I suppose that sending a second patchset for configs requests that the
>> rpmsg char series is first applied
> 
> Right, but the rpmsg_char series has been applied.
> 
>> But on the other hand this may lead to some failures as the RPMSG_CTRL is now
>> needed to create the /dev/rpmsg_ctrl0
>>
> 
> Possibly, but right now there is no other way.
> 
>> so probably, I need to do this as fixup patch.
>>
> 
> Indeed, this can be applied as a fix in rc1.

Yes, that's what I had in mind, I'll do it that way.

Thanks,
Arnaud

> 
>> FYI the RISC-V patch as been reviewed by Anup Patel
>>
> 
> ... but Anup does not maintain any of the defconfig files.
> 
>> Thanks,
>> Arnaud
>>
>>>
>>> Thanks,
>>> Mathieu
>>>
>>>>
>>>> [1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
>>>>
>>>> Regards,
>>>> Arnaud
>>>>
>>>>>   rpmsg: Update rpmsg_chrdev_register_device function
>>>>>   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>>>>>   rpmsg: char: Add possibility to use default endpoint of the rpmsg
>>>>>     device
>>>>>   rpmsg: char: Introduce the "rpmsg-raw" channel
>>>>>   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
>>>>>
>>>>>  arch/arm/configs/qcom_defconfig   |   1 +
>>>>>  arch/arm64/configs/defconfig      |   1 +
>>>>>  arch/riscv/configs/defconfig      |   1 +
>>>>>  arch/riscv/configs/rv32_defconfig |   1 +
>>>>>  drivers/rpmsg/Kconfig             |   8 +
>>>>>  drivers/rpmsg/Makefile            |   1 +
>>>>>  drivers/rpmsg/qcom_glink_native.c |   2 +-
>>>>>  drivers/rpmsg/qcom_smd.c          |   2 +-
>>>>>  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>>>>>  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>>>>>  drivers/rpmsg/rpmsg_core.c        |  15 +-
>>>>>  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>>>>>  drivers/rpmsg/rpmsg_internal.h    |  10 +-
>>>>>  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>>>>>  include/uapi/linux/rpmsg.h        |  10 ++
>>>>>  15 files changed, 419 insertions(+), 155 deletions(-)
>>>>>  create mode 100644 drivers/rpmsg/rpmsg_char.h
>>>>>  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
>>>>>