Re: [PATCH v3 5/6] rpmsg: char: Introduce a rpmsg driver for the rpmsg char device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mathieu, Arnaud,

On 5/5/21 8:25 PM, Arnaud POULIQUEN wrote:
Hi Mathieu,

On 5/5/21 6:41 PM, Mathieu Poirier wrote:
Hi Arnaud,

On Thu, Apr 29, 2021 at 03:55:06PM +0200, Arnaud Pouliquen wrote:
A rpmsg char device allows to probe the endpoint device on a remote name
service announcement.

With this patch the /dev/rpmsgX interface is created either by a user
application or by the remote firmware.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@xxxxxxxxxxx>

---
update from V1:

  - add missing unregister_rpmsg_driver call on module exit.
---
  drivers/rpmsg/rpmsg_char.c | 53 +++++++++++++++++++++++++++++++++++++-
  1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index 5c6a7da6e4d7..9166454c1310 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -18,6 +18,8 @@
#include "rpmsg_char.h" +#define RPMSG_CHAR_DEVNAME "rpmsg-raw"
+
  static dev_t rpmsg_major;
  static struct class *rpmsg_class;
@@ -413,6 +415,40 @@ 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;
+
+	memcpy(chinfo.name, RPMSG_CHAR_DEVNAME, sizeof(RPMSG_CHAR_DEVNAME));
+	chinfo.src = rpdev->src;
+	chinfo.dst = rpdev->dst;
+
+	return __rpmsg_chrdev_eptdev_create(rpdev, &rpdev->dev, chinfo, true);
+}
+
+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_CHAR_DEVNAME },
+	{ },
+};
+
+static struct rpmsg_driver rpmsg_chrdev_driver = {
+	.probe = rpmsg_chrdev_probe,
+	.remove = rpmsg_chrdev_remove,
+	.id_table = rpmsg_chrdev_id_table,
+	.drv = {
+		.name = "rpmsg_chrdev",
+	},
+};

The sole purpose of doing this is to create instances of rpmsg_chrdevs from the
name service - but is it really needed?  Up to now and aside from GLINK and SMD,
there asn't been other users of it so I'm wondering if it is worth going through
all this trouble.

It is a good point.

Just as a reminder, the need of ST and, I assume, some other companies, is to
have a basic/generic communication channel to control a remote processor
application.

Nothing generic exists today for a virtio transport based implementation.
Companies have to create their own driver.

The purpose of my work is to allow our customer to use RPMsg without developing
a specific driver to control remote applications.

The rpmsg_chrdev char is a good candidate for this. No protocol, just a simple
inter-processor link to send and receive data. The rpmsg_tty is another one.

Focusing on the rpmsg_chrdev:
We did a part of the work with the first patch set that would be in 5.13.
But is it simple to use it for virtio transport based platforms?
If we don't implement the NS announcement support in rpmsg_chrdev, using
rpmsg_chrdev for a user application seems rather tricky.
How to instantiate the communication?
The application will probably has to scan the /sys/bus/rpmsg/devices/ folder to
determine the services and associated remote address.

I don't think the QCOM drivers have the same problem because they seems to
initiate the communication and work directly with the RPMsg endpoints ( new
channel creation on endpoint creation) while Virtio works with the RPMsg channel.

By introducing the ability to instantiate rpmsg_chrdevs through the NS
announcement, we make this easy for applications to use.

And without rpmsg_chrdevs instantiation, It also means that we can't create an
RPMsg channel for the rpmsg_chrdevs using a new RPMSG_CREATE_DEV_IOCTL control,
right?

That said, If we consider that the aim was only to extract the rpmsg_ctrl part,
I'm not against leaving the rpmsg_char in this state and switching to the
rpmsg_tty driver upstream including the work on the rpmsg_ctrl to create rpmsg
channels.

We could come back on this if requested by someone else.

I'm personnaly following this thread, our project is to be able to do RPC call
from Linux to an RTOS (Zephyr). Our plan is to do that in userspace using the nameservice
announcement from virtio/rpmsg.

We did an hackish patch to do that internally:
https://github.com/iotbzh/meta-rcar-zephyr/blob/master/recipes-kernel/linux/linux-renesas/0001-Add-device-driver-for-rcar-r7-
rpmsg.patch

That we will be really happy to drop by any cleaner solution.

Thanks for your work !
Julien



[Index of Archives]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Photo Sharing]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux