2020-10-14 09:31:59

by Mathieu Poirier

[permalink] [raw]
Subject: [PATCH v2 0/9] rpmsg: Make RPMSG name service modular

Good afternoon,

This set 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 v5.9.

Thanks,
Mathieu

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

------
New for V2:
- Created new RPMSG types (Guennadi).
- Re-worked byte conversion functions(Guennadi).
- Added a single ->is_little_endian() operation (Arnaud).
- Fixed byte conversion before making name service modular.

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 (5):
rpmsg: Move rpmsg_endpoint_ops to rpmsg.h
rpmsg: Introduce __rpmsg{16|32|64} types
rpmsg: virtio: Move from virtio to rpmsg byte conversion
rpmsg: Move rpmsg_hr and rpmsg_ns_msg to header file
rpmsg: Make rpmsg_{register|unregister}_device() public

drivers/rpmsg/Kconfig | 8 ++
drivers/rpmsg/Makefile | 1 +
drivers/rpmsg/rpmsg_core.c | 44 +++++++
drivers/rpmsg/rpmsg_internal.h | 27 +---
drivers/rpmsg/rpmsg_ns.c | 110 +++++++++++++++++
drivers/rpmsg/virtio_rpmsg_bus.c | 206 ++++++++++---------------------
include/linux/rpmsg.h | 93 ++++++++++++--
include/linux/rpmsg_byteorder.h | 67 ++++++++++
include/linux/rpmsg_ns.h | 79 ++++++++++++
include/uapi/linux/rpmsg.h | 3 +
include/uapi/linux/rpmsg_types.h | 11 ++
11 files changed, 481 insertions(+), 168 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-10-14 09:33:42

by Mathieu Poirier

[permalink] [raw]
Subject: [PATCH v2 1/9] rpmsg: Move rpmsg_endpoint_ops to rpmsg.h

Move structure rpmsg_endpoint_ops to header rpmsg.h so that it can
be used by other entities.

Signed-off-by: Mathieu Poirier <[email protected]>
---
drivers/rpmsg/rpmsg_internal.h | 19 -------------------
include/linux/rpmsg.h | 24 +++++++++++++++++++++---
2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 3fc83cd50e98..094cf968d2d3 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -18,25 +18,6 @@
#define to_rpmsg_device(d) container_of(d, struct rpmsg_device, dev)
#define to_rpmsg_driver(d) container_of(d, struct rpmsg_driver, drv)

-/**
- * struct rpmsg_device_ops - indirection table for the rpmsg_device operations
- * @create_ept: create backend-specific endpoint, required
- * @announce_create: announce presence of new channel, optional
- * @announce_destroy: announce destruction of channel, optional
- *
- * Indirection table for the operations that a rpmsg backend should implement.
- * @announce_create and @announce_destroy are optional as the backend might
- * advertise new channels implicitly by creating the endpoints.
- */
-struct rpmsg_device_ops {
- struct rpmsg_endpoint *(*create_ept)(struct rpmsg_device *rpdev,
- rpmsg_rx_cb_t cb, void *priv,
- struct rpmsg_channel_info chinfo);
-
- int (*announce_create)(struct rpmsg_device *ept);
- int (*announce_destroy)(struct rpmsg_device *ept);
-};
-
/**
* struct rpmsg_endpoint_ops - indirection table for rpmsg_endpoint operations
* @destroy_ept: see @rpmsg_destroy_ept(), required
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 9fe156d1c018..9fe1c54ae995 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -22,7 +22,6 @@

struct rpmsg_device;
struct rpmsg_endpoint;
-struct rpmsg_device_ops;
struct rpmsg_endpoint_ops;

/**
@@ -37,6 +36,27 @@ struct rpmsg_channel_info {
u32 dst;
};

+typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32);
+
+/**
+ * struct rpmsg_device_ops - indirection table for the rpmsg_device operations
+ * @create_ept: create backend-specific endpoint, required
+ * @announce_create: announce presence of new channel, optional
+ * @announce_destroy: announce destruction of channel, optional
+ *
+ * Indirection table for the operations that a rpmsg backend should implement.
+ * @announce_create and @announce_destroy are optional as the backend might
+ * advertise new channels implicitly by creating the endpoints.
+ */
+struct rpmsg_device_ops {
+ struct rpmsg_endpoint *(*create_ept)(struct rpmsg_device *rpdev,
+ rpmsg_rx_cb_t cb, void *priv,
+ struct rpmsg_channel_info chinfo);
+
+ int (*announce_create)(struct rpmsg_device *ept);
+ int (*announce_destroy)(struct rpmsg_device *ept);
+};
+
/**
* rpmsg_device - device that belong to the rpmsg bus
* @dev: the device struct
@@ -59,8 +79,6 @@ struct rpmsg_device {
const struct rpmsg_device_ops *ops;
};

-typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32);
-
/**
* struct rpmsg_endpoint - binds a local rpmsg address to its user
* @rpdev: rpmsg channel device
--
2.25.1

2020-10-14 15:21:06

by Mathieu Poirier

[permalink] [raw]
Subject: [PATCH v2 5/9] rpmsg: virtio: Rename rpmsg_create_channel

From: Arnaud Pouliquen <[email protected]>

Rename the internal function as it is internal, and as
the name will be used in rpmsg_core.

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

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 85f2acc4ed9f..fca982b61c3b 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -345,8 +345,8 @@ static void virtio_rpmsg_release_device(struct device *dev)
* this function will be used to create both static and dynamic
* channels.
*/
-static struct rpmsg_device *rpmsg_create_channel(struct virtproc_info *vrp,
- struct rpmsg_channel_info *chinfo)
+static struct rpmsg_device * __rpmsg_create_channel(struct virtproc_info *vrp,
+ struct rpmsg_channel_info *chinfo)
{
struct virtio_rpmsg_channel *vch;
struct rpmsg_device *rpdev;
@@ -820,7 +820,7 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
if (ret)
dev_err(dev, "rpmsg_destroy_channel failed: %d\n", ret);
} else {
- newch = rpmsg_create_channel(vrp, &chinfo);
+ newch = __rpmsg_create_channel(vrp, &chinfo);
if (!newch)
dev_err(dev, "rpmsg_create_channel failed\n");
}
--
2.25.1

2020-10-14 15:49:12

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH v2 0/9] rpmsg: Make RPMSG name service modular

On Tue, Oct 13, 2020 at 05:25:10PM -0600, Mathieu Poirier wrote:
> Good afternoon,
>
> This set 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 v5.9.

Reviewed-by: Guennadi Liakhovetski <[email protected]>

Thanks
Guennadi

>
> Thanks,
> Mathieu
>
> [1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=338335
>
> ------
> New for V2:
> - Created new RPMSG types (Guennadi).
> - Re-worked byte conversion functions(Guennadi).
> - Added a single ->is_little_endian() operation (Arnaud).
> - Fixed byte conversion before making name service modular.
>
> 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 (5):
> rpmsg: Move rpmsg_endpoint_ops to rpmsg.h
> rpmsg: Introduce __rpmsg{16|32|64} types
> rpmsg: virtio: Move from virtio to rpmsg byte conversion
> rpmsg: Move rpmsg_hr and rpmsg_ns_msg to header file
> rpmsg: Make rpmsg_{register|unregister}_device() public
>
> drivers/rpmsg/Kconfig | 8 ++
> drivers/rpmsg/Makefile | 1 +
> drivers/rpmsg/rpmsg_core.c | 44 +++++++
> drivers/rpmsg/rpmsg_internal.h | 27 +---
> drivers/rpmsg/rpmsg_ns.c | 110 +++++++++++++++++
> drivers/rpmsg/virtio_rpmsg_bus.c | 206 ++++++++++---------------------
> include/linux/rpmsg.h | 93 ++++++++++++--
> include/linux/rpmsg_byteorder.h | 67 ++++++++++
> include/linux/rpmsg_ns.h | 79 ++++++++++++
> include/uapi/linux/rpmsg.h | 3 +
> include/uapi/linux/rpmsg_types.h | 11 ++
> 11 files changed, 481 insertions(+), 168 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-10-15 17:58:20

by Arnaud POULIQUEN

[permalink] [raw]
Subject: Re: [PATCH v2 1/9] rpmsg: Move rpmsg_endpoint_ops to rpmsg.h

On 10/14/20 1:25 AM, Mathieu Poirier wrote:
> Move structure rpmsg_endpoint_ops to header rpmsg.h so that it can
> be used by other entities.
>
> Signed-off-by: Mathieu Poirier <[email protected]>
> ---
> drivers/rpmsg/rpmsg_internal.h | 19 -------------------
> include/linux/rpmsg.h | 24 +++++++++++++++++++++---
> 2 files changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
> index 3fc83cd50e98..094cf968d2d3 100644
> --- a/drivers/rpmsg/rpmsg_internal.h
> +++ b/drivers/rpmsg/rpmsg_internal.h
> @@ -18,25 +18,6 @@
> #define to_rpmsg_device(d) container_of(d, struct rpmsg_device, dev)
> #define to_rpmsg_driver(d) container_of(d, struct rpmsg_driver, drv)
>
> -/**
> - * struct rpmsg_device_ops - indirection table for the rpmsg_device operations
> - * @create_ept: create backend-specific endpoint, required
> - * @announce_create: announce presence of new channel, optional
> - * @announce_destroy: announce destruction of channel, optional
> - *
> - * Indirection table for the operations that a rpmsg backend should implement.
> - * @announce_create and @announce_destroy are optional as the backend might
> - * advertise new channels implicitly by creating the endpoints.
> - */
> -struct rpmsg_device_ops {
> - struct rpmsg_endpoint *(*create_ept)(struct rpmsg_device *rpdev,
> - rpmsg_rx_cb_t cb, void *priv,
> - struct rpmsg_channel_info chinfo);
> -
> - int (*announce_create)(struct rpmsg_device *ept);
> - int (*announce_destroy)(struct rpmsg_device *ept);
> -};
> -
> /**
> * struct rpmsg_endpoint_ops - indirection table for rpmsg_endpoint operations
> * @destroy_ept: see @rpmsg_destroy_ept(), required
> diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
> index 9fe156d1c018..9fe1c54ae995 100644
> --- a/include/linux/rpmsg.h
> +++ b/include/linux/rpmsg.h
> @@ -22,7 +22,6 @@
>
> struct rpmsg_device;
> struct rpmsg_endpoint;
> -struct rpmsg_device_ops;
> struct rpmsg_endpoint_ops;
>
> /**
> @@ -37,6 +36,27 @@ struct rpmsg_channel_info {
> u32 dst;
> };
>
> +typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32);
> +
> +/**
> + * struct rpmsg_device_ops - indirection table for the rpmsg_device operations
> + * @create_ept: create backend-specific endpoint, required
> + * @announce_create: announce presence of new channel, optional
> + * @announce_destroy: announce destruction of channel, optional
> + *
> + * Indirection table for the operations that a rpmsg backend should implement.
> + * @announce_create and @announce_destroy are optional as the backend might
> + * advertise new channels implicitly by creating the endpoints.
> + */
> +struct rpmsg_device_ops {
> + struct rpmsg_endpoint *(*create_ept)(struct rpmsg_device *rpdev,
> + rpmsg_rx_cb_t cb, void *priv,
> + struct rpmsg_channel_info chinfo);
> +
> + int (*announce_create)(struct rpmsg_device *ept);
> + int (*announce_destroy)(struct rpmsg_device *ept);
> +};
> +
> /**
> * rpmsg_device - device that belong to the rpmsg bus
> * @dev: the device struct
> @@ -59,8 +79,6 @@ struct rpmsg_device {
> const struct rpmsg_device_ops *ops;
> };
>
> -typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32);
> -
> /**
> * struct rpmsg_endpoint - binds a local rpmsg address to its user
> * @rpdev: rpmsg channel device
>

Reviewed-by: Arnaud Pouliquen <[email protected]>

Thanks,
Arnaud