2020-11-18 21:40:23

by Mathieu Poirier

[permalink] [raw]
Subject: [PATCH v6 0/8] rpmsg: Make RPMSG name service modular

This revision addresses comments received from the previous revision,
i.e V5. Please see details below.

It starts by making the RPMSG protocol transport agnostic by
moving the headers it uses to generic types and using those in the
current implementation. From there it re-uses the work that Arnaud
published[1] to make the name service modular.

Tested on stm32mp157 with the RPMSG client sample application. Applies
cleanly on rpmsg-next.

Thanks,
Mathieu

[1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=338335

-------
New for V6:
- Rebased to rpmsg-next
- Move rpmsg_ns_register_device() to rpmsg_ns.c for automatic module loading

Arnaud Pouliquen (4):
rpmsg: virtio: Rename rpmsg_create_channel
rpmsg: core: Add channel creation internal API
rpmsg: virtio: Add rpmsg channel device ops
rpmsg: Turn name service into a stand alone driver

Mathieu Poirier (4):
rpmsg: Introduce __rpmsg{16|32|64} types
rpmsg: virtio: Move from virtio to rpmsg byte conversion
rpmsg: Move structure rpmsg_ns_msg to header file
rpmsg: Make rpmsg_{register|unregister}_device() public

drivers/rpmsg/Kconfig | 9 ++
drivers/rpmsg/Makefile | 1 +
drivers/rpmsg/rpmsg_core.c | 44 ++++++++
drivers/rpmsg/rpmsg_internal.h | 14 ++-
drivers/rpmsg/rpmsg_ns.c | 126 +++++++++++++++++++++
drivers/rpmsg/virtio_rpmsg_bus.c | 185 +++++++++++--------------------
include/linux/rpmsg.h | 63 ++++++++++-
include/linux/rpmsg/byteorder.h | 67 +++++++++++
include/linux/rpmsg/ns.h | 45 ++++++++
include/uapi/linux/rpmsg_types.h | 11 ++
10 files changed, 438 insertions(+), 127 deletions(-)
create mode 100644 drivers/rpmsg/rpmsg_ns.c
create mode 100644 include/linux/rpmsg/byteorder.h
create mode 100644 include/linux/rpmsg/ns.h
create mode 100644 include/uapi/linux/rpmsg_types.h

--
2.25.1


2020-11-18 21:41:02

by Mathieu Poirier

[permalink] [raw]
Subject: [PATCH v6 2/8] rpmsg: virtio: Move from virtio to rpmsg byte conversion

Use rpmsg byte conversion functions in order for the RPMSG
headers and generic functions to be used by external entities.

Signed-off-by: Mathieu Poirier <[email protected]>
Reviewed-by: Arnaud Pouliquen <[email protected]>
---
drivers/rpmsg/virtio_rpmsg_bus.c | 53 +++++++++++++++++---------------
1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 7d7ed4e5cce7..5259fbbc8e68 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -19,11 +19,11 @@
#include <linux/mutex.h>
#include <linux/of_device.h>
#include <linux/rpmsg.h>
+#include <linux/rpmsg/byteorder.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
#include <linux/sched.h>
#include <linux/virtio.h>
-#include <linux/virtio_byteorder.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_config.h>
#include <linux/wait.h>
@@ -85,11 +85,11 @@ struct virtproc_info {
* Every message sent(/received) on the rpmsg bus begins with this header.
*/
struct rpmsg_hdr {
- __virtio32 src;
- __virtio32 dst;
- __virtio32 reserved;
- __virtio16 len;
- __virtio16 flags;
+ __rpmsg32 src;
+ __rpmsg32 dst;
+ __rpmsg32 reserved;
+ __rpmsg16 len;
+ __rpmsg16 flags;
u8 data[];
} __packed;

@@ -107,8 +107,8 @@ struct rpmsg_hdr {
*/
struct rpmsg_ns_msg {
char name[RPMSG_NAME_SIZE];
- __virtio32 addr;
- __virtio32 flags;
+ __rpmsg32 addr;
+ __rpmsg32 flags;
} __packed;

/**
@@ -341,8 +341,8 @@ static int virtio_rpmsg_announce_create(struct rpmsg_device *rpdev)
struct rpmsg_ns_msg nsm;

strncpy(nsm.name, rpdev->id.name, RPMSG_NAME_SIZE);
- nsm.addr = cpu_to_virtio32(vrp->vdev, rpdev->ept->addr);
- nsm.flags = cpu_to_virtio32(vrp->vdev, RPMSG_NS_CREATE);
+ nsm.addr = cpu_to_rpmsg32(rpdev, rpdev->ept->addr);
+ nsm.flags = cpu_to_rpmsg32(rpdev, RPMSG_NS_CREATE);

err = rpmsg_sendto(rpdev->ept, &nsm, sizeof(nsm), RPMSG_NS_ADDR);
if (err)
@@ -365,8 +365,8 @@ static int virtio_rpmsg_announce_destroy(struct rpmsg_device *rpdev)
struct rpmsg_ns_msg nsm;

strncpy(nsm.name, rpdev->id.name, RPMSG_NAME_SIZE);
- nsm.addr = cpu_to_virtio32(vrp->vdev, rpdev->ept->addr);
- nsm.flags = cpu_to_virtio32(vrp->vdev, RPMSG_NS_DESTROY);
+ nsm.addr = cpu_to_rpmsg32(rpdev, rpdev->ept->addr);
+ nsm.flags = cpu_to_rpmsg32(rpdev, RPMSG_NS_DESTROY);

err = rpmsg_sendto(rpdev->ept, &nsm, sizeof(nsm), RPMSG_NS_ADDR);
if (err)
@@ -425,6 +425,7 @@ static struct rpmsg_device *rpmsg_create_channel(struct virtproc_info *vrp,
rpdev->src = chinfo->src;
rpdev->dst = chinfo->dst;
rpdev->ops = &virtio_rpmsg_ops;
+ rpdev->little_endian = virtio_is_little_endian(vrp->vdev);

/*
* rpmsg server channels has predefined local address (for now),
@@ -618,10 +619,10 @@ static int rpmsg_send_offchannel_raw(struct rpmsg_device *rpdev,
}
}

- msg->len = cpu_to_virtio16(vrp->vdev, len);
+ msg->len = cpu_to_rpmsg16(rpdev, len);
msg->flags = 0;
- msg->src = cpu_to_virtio32(vrp->vdev, src);
- msg->dst = cpu_to_virtio32(vrp->vdev, dst);
+ msg->src = cpu_to_rpmsg32(rpdev, src);
+ msg->dst = cpu_to_rpmsg32(rpdev, dst);
msg->reserved = 0;
memcpy(msg->data, data, len);

@@ -710,14 +711,15 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
{
struct rpmsg_endpoint *ept;
struct scatterlist sg;
- unsigned int msg_len = virtio16_to_cpu(vrp->vdev, msg->len);
+ bool little_endian = virtio_is_little_endian(vrp->vdev);
+ unsigned int msg_len = __rpmsg16_to_cpu(little_endian, msg->len);
int err;

dev_dbg(dev, "From: 0x%x, To: 0x%x, Len: %d, Flags: %d, Reserved: %d\n",
- virtio32_to_cpu(vrp->vdev, msg->src),
- virtio32_to_cpu(vrp->vdev, msg->dst), msg_len,
- virtio16_to_cpu(vrp->vdev, msg->flags),
- virtio32_to_cpu(vrp->vdev, msg->reserved));
+ __rpmsg32_to_cpu(little_endian, msg->src),
+ __rpmsg32_to_cpu(little_endian, msg->dst), msg_len,
+ __rpmsg16_to_cpu(little_endian, msg->flags),
+ __rpmsg32_to_cpu(little_endian, msg->reserved));
#if defined(CONFIG_DYNAMIC_DEBUG)
dynamic_hex_dump("rpmsg_virtio RX: ", DUMP_PREFIX_NONE, 16, 1,
msg, sizeof(*msg) + msg_len, true);
@@ -736,7 +738,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
/* use the dst addr to fetch the callback of the appropriate user */
mutex_lock(&vrp->endpoints_lock);

- ept = idr_find(&vrp->endpoints, virtio32_to_cpu(vrp->vdev, msg->dst));
+ ept = idr_find(&vrp->endpoints, __rpmsg32_to_cpu(little_endian, msg->dst));

/* let's make sure no one deallocates ept while we use it */
if (ept)
@@ -750,7 +752,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,

if (ept->cb)
ept->cb(ept->rpdev, msg->data, msg_len, ept->priv,
- virtio32_to_cpu(vrp->vdev, msg->src));
+ __rpmsg32_to_cpu(little_endian, msg->src));

mutex_unlock(&ept->cb_lock);

@@ -830,6 +832,7 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
struct rpmsg_channel_info chinfo;
struct virtproc_info *vrp = priv;
struct device *dev = &vrp->vdev->dev;
+ bool little_endian = virtio_is_little_endian(vrp->vdev);
int ret;

#if defined(CONFIG_DYNAMIC_DEBUG)
@@ -858,13 +861,13 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,

strncpy(chinfo.name, msg->name, sizeof(chinfo.name));
chinfo.src = RPMSG_ADDR_ANY;
- chinfo.dst = virtio32_to_cpu(vrp->vdev, msg->addr);
+ chinfo.dst = __rpmsg32_to_cpu(little_endian, msg->addr);

dev_info(dev, "%sing channel %s addr 0x%x\n",
- virtio32_to_cpu(vrp->vdev, msg->flags) & RPMSG_NS_DESTROY ?
+ __rpmsg32_to_cpu(little_endian, msg->flags) & RPMSG_NS_DESTROY ?
"destroy" : "creat", msg->name, chinfo.dst);

- if (virtio32_to_cpu(vrp->vdev, msg->flags) & RPMSG_NS_DESTROY) {
+ if (__rpmsg32_to_cpu(little_endian, msg->flags) & RPMSG_NS_DESTROY) {
ret = rpmsg_unregister_device(&vrp->vdev->dev, &chinfo);
if (ret)
dev_err(dev, "rpmsg_destroy_channel failed: %d\n", ret);
--
2.25.1

2020-11-18 21:41:39

by Mathieu Poirier

[permalink] [raw]
Subject: [PATCH v6 5/8] rpmsg: core: Add channel creation internal API

From: Arnaud Pouliquen <[email protected]>

Add the channel creation API as a first step to be able to define the
name service announcement as a rpmsg driver independent from the RPMsg
virtio bus.

Signed-off-by: Arnaud Pouliquen <[email protected]>
Signed-off-by: Mathieu Poirier <[email protected]>
Reviewed-by: Guennadi Liakhovetski <[email protected]>
---
drivers/rpmsg/rpmsg_core.c | 44 ++++++++++++++++++++++++++++++++++
drivers/rpmsg/rpmsg_internal.h | 10 ++++++++
2 files changed, 54 insertions(+)

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 91de940896e3..e5daee4f9373 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -20,6 +20,50 @@

#include "rpmsg_internal.h"

+/**
+ * rpmsg_create_channel() - create a new rpmsg channel
+ * using its name and address info.
+ * @rpdev: rpmsg device
+ * @chinfo: channel_info to bind
+ *
+ * Returns a pointer to the new rpmsg device on success, or NULL on error.
+ */
+struct rpmsg_device *rpmsg_create_channel(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo)
+{
+ if (WARN_ON(!rpdev))
+ return NULL;
+ if (!rpdev->ops || !rpdev->ops->create_channel) {
+ dev_err(&rpdev->dev, "no create_channel ops found\n");
+ return NULL;
+ }
+
+ return rpdev->ops->create_channel(rpdev, chinfo);
+}
+EXPORT_SYMBOL(rpmsg_create_channel);
+
+/**
+ * rpmsg_release_channel() - release a rpmsg channel
+ * using its name and address info.
+ * @rpdev: rpmsg device
+ * @chinfo: channel_info to bind
+ *
+ * Returns 0 on success or an appropriate error value.
+ */
+int rpmsg_release_channel(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo)
+{
+ if (WARN_ON(!rpdev))
+ return -EINVAL;
+ if (!rpdev->ops || !rpdev->ops->release_channel) {
+ dev_err(&rpdev->dev, "no release_channel ops found\n");
+ return -ENXIO;
+ }
+
+ return rpdev->ops->release_channel(rpdev, chinfo);
+}
+EXPORT_SYMBOL(rpmsg_release_channel);
+
/**
* rpmsg_create_ept() - create a new rpmsg_endpoint
* @rpdev: rpmsg channel device
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 3fc83cd50e98..f1de73e0f2d6 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -20,6 +20,8 @@

/**
* struct rpmsg_device_ops - indirection table for the rpmsg_device operations
+ * @create_channel: create backend-specific channel, optional
+ * @release_channel: release backend-specific channel, optional
* @create_ept: create backend-specific endpoint, required
* @announce_create: announce presence of new channel, optional
* @announce_destroy: announce destruction of channel, optional
@@ -29,6 +31,10 @@
* advertise new channels implicitly by creating the endpoints.
*/
struct rpmsg_device_ops {
+ struct rpmsg_device *(*create_channel)(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo);
+ int (*release_channel)(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo);
struct rpmsg_endpoint *(*create_ept)(struct rpmsg_device *rpdev,
rpmsg_rx_cb_t cb, void *priv,
struct rpmsg_channel_info chinfo);
@@ -75,6 +81,10 @@ int rpmsg_unregister_device(struct device *parent,
struct device *rpmsg_find_device(struct device *parent,
struct rpmsg_channel_info *chinfo);

+struct rpmsg_device *rpmsg_create_channel(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo);
+int rpmsg_release_channel(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo);
/**
* rpmsg_chrdev_register_device() - register chrdev device based on rpdev
* @rpdev: prepared rpdev to be used for creating endpoints
--
2.25.1