2022-05-21 13:06:44

by Arnaud Pouliquen

[permalink] [raw]
Subject: [RFC PATCH 00/10] Introduction of rpmsg flow control service

This series proposes an implementation for the rpmsg virtio transport
backend, of the signaling API proposed by Deepak Kumar Singh:
"rpmsg and glink signaling API support" [1]

The aim of the series is to offer the possibility for an endpoint to inform
a remote endpoint about its state, based on a software flow control[2].

For this a new rpmsg service( with a fixed address 64) is proposed.
It is responsible for:
- transmitting local endpoint flow control information to the remote side,
- informing a local endpoint about a remote endpoint flow control.

For the rpmsg virtio transport layer the service is negotiated thanks to the
virtio feature flag: VIRTIO_RPMSG_F_FC

Notice that this pull request introduces new feature in the rpmsg protocol,
So it has to be aligned with OpenAMP implementation.
Pending OpenAMP pull request is available here:
https://github.com/OpenAMP/open-amp/pull/394


[1]https://lkml.org/lkml/2022/1/18/867
[2]https://en.wikipedia.org/wiki/Software_flow_control

Arnaud Pouliquen (8):
rpmsg: core: Add rpmsg device remote flow control announcement ops
rpmsg: virtio: Implement the announce_remote_fc ops
rpmsg: Introduce flow control channel driver
rpmsg: virtio: Add support of the VIRTIO_RPMSG_F_FC feature
rpmsg: virtio: Implement the set_flow_control ops
rpmsg: Add the destination address in rpmsg_set_flow_control
rpmsg: tty : Add the support of the flow control
rpmsg: virtio: Set default dst address on flow control

Deepak Kumar Singh (2):
rpmsg: core: Add signal API support
rpmsg: char: Add TIOCMGET/TIOCMSET ioctl support

drivers/rpmsg/Kconfig | 11 +++
drivers/rpmsg/Makefile | 1 +
drivers/rpmsg/rpmsg_char.c | 56 +++++++++++++--
drivers/rpmsg/rpmsg_core.c | 47 +++++++++++++
drivers/rpmsg/rpmsg_fc.c | 113 +++++++++++++++++++++++++++++++
drivers/rpmsg/rpmsg_internal.h | 9 +++
drivers/rpmsg/virtio_rpmsg_bus.c | 111 +++++++++++++++++++++++++++++-
drivers/tty/rpmsg_tty.c | 50 ++++++++++++++
include/linux/rpmsg.h | 15 ++++
include/linux/rpmsg/fc.h | 51 ++++++++++++++
10 files changed, 456 insertions(+), 8 deletions(-)
create mode 100644 drivers/rpmsg/rpmsg_fc.c
create mode 100644 include/linux/rpmsg/fc.h

--
2.25.1



2022-05-22 15:59:02

by Arnaud Pouliquen

[permalink] [raw]
Subject: [RFC PATCH 10/10] rpmsg: virtio: Set default dst address on flow control

When a rpmsg channel has been created locally with a destination address
set to RPMSG_ADDR_ANY, a name service announcement message is sent to
the remote side. Then the destination address is never updated, making it
impossible to send messages to the remote.

An example of kernel trace observed:
rpmsg_tty virtio0.rpmsg-tty.29.-1: invalid addr (src 0x1d, dst 0xffffffff)

The flow control can be used to set the rpmsg device address.
If the destination address is RPMSG_ADDR_ANY, then set it to
address of the remote endpoint that send the message.

Signed-off-by: Arnaud Pouliquen <[email protected]>
---
This patch is an alternative of the fix proposed in patch [1]

[1] https://lore.kernel.org/lkml/[email protected]/
---
drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index a8e60ca4cd08..0337a07e278c 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -393,6 +393,16 @@ static int virtio_rpmsg_remote_flowctrl(struct rpmsg_device *rpdev,
if (!ept)
return -EINVAL;

+ /*
+ * If the endpoint is the rpmsg device default one, then it can not be yet associated
+ * to the remote endpoint. This can occur if a ns announcement message has been
+ * previously sent to the remote side.
+ * Update the rpmsg device destination address in such case to store the remote
+ * address as default remote endpoint.
+ */
+ if (rpdev->ept == ept && rpdev->dst == RPMSG_ADDR_ANY)
+ rpdev->dst = __rpmsg32_to_cpu(virtio_is_little_endian(vrp->vdev), chinfo->src);
+
/* Make sure ept->sig_cb doesn't go away while we use it */
mutex_lock(&ept->cb_lock);

--
2.25.1


2022-05-22 16:01:41

by Arnaud Pouliquen

[permalink] [raw]
Subject: [RFC PATCH 05/10] rpmsg: Introduce flow control channel driver

Register the channel 54 to manage flow control between endpoints.
the aim of this service is.
- to inform the local endpoint of the state of the associated remote
endpoints.
- to inform remote endpoint about the state of the local endpoint.

Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/rpmsg/Kconfig | 10 ++++
drivers/rpmsg/Makefile | 1 +
drivers/rpmsg/rpmsg_fc.c | 113 +++++++++++++++++++++++++++++++++++++++
include/linux/rpmsg/fc.h | 51 ++++++++++++++++++
4 files changed, 175 insertions(+)
create mode 100644 drivers/rpmsg/rpmsg_fc.c
create mode 100644 include/linux/rpmsg/fc.h

diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
index d3795860f5c0..c6659f27c617 100644
--- a/drivers/rpmsg/Kconfig
+++ b/drivers/rpmsg/Kconfig
@@ -31,6 +31,16 @@ config RPMSG_NS
channel that probes the associated RPMsg device on remote endpoint
service announcement.

+config RPMSG_FC
+ tristate "RPMSG endpoint flow control management"
+ depends on RPMSG
+ help
+ Say Y here to enable the support of the flow control management
+ for the rpmsg endpoints.
+
+ To compile this driver as a module, choose M here: the module will be
+ called rpmsg_fc.
+
config RPMSG_MTK_SCP
tristate "MediaTek SCP"
depends on MTK_SCP
diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile
index 58e3b382e316..c70b9864231f 100644
--- a/drivers/rpmsg/Makefile
+++ b/drivers/rpmsg/Makefile
@@ -3,6 +3,7 @@ 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_FC) += rpmsg_fc.o
obj-$(CONFIG_RPMSG_MTK_SCP) += mtk_rpmsg.o
qcom_glink-objs := qcom_glink_native.o qcom_glink_ssr.o
obj-$(CONFIG_RPMSG_QCOM_GLINK) += qcom_glink.o
diff --git a/drivers/rpmsg/rpmsg_fc.c b/drivers/rpmsg/rpmsg_fc.c
new file mode 100644
index 000000000000..d7e17c8cffb0
--- /dev/null
+++ b/drivers/rpmsg/rpmsg_fc.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) STMicroelectronics 2022 - All Rights Reserved
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/rpmsg.h>
+#include <linux/rpmsg/fc.h>
+#include <linux/slab.h>
+
+#include "rpmsg_internal.h"
+
+/**
+ * rpmsg_fc_register_device() - register name service device based on rpdev
+ * @rpdev: prepared rpdev to be used catfor creating endpoints
+ *
+ * This function wraps rpmsg_register_device() preparing the rpdev for use as
+ * basis for the rpmsg name service device.
+ */
+int rpmsg_fc_register_device(struct rpmsg_device *rpdev)
+{
+ strcpy(rpdev->id.name, "rpmsg_fc");
+ rpdev->driver_override = KBUILD_MODNAME;
+ rpdev->src = RPMSG_FC_ADDR;
+ rpdev->dst = RPMSG_FC_ADDR;
+
+ return rpmsg_register_device(rpdev);
+}
+EXPORT_SYMBOL(rpmsg_fc_register_device);
+
+/* Invoked when a name service announcement arrives */
+static int rpmsg_fc_cb(struct rpmsg_device *rpdev, void *data, int len,
+ void *priv, u32 src)
+{
+ struct rpmsg_ept_msg *msg = data;
+ struct rpmsg_channel_info chinfo;
+ struct device *dev = rpdev->dev.parent;
+ bool enable;
+ int ret;
+
+ if (len != sizeof(*msg)) {
+ dev_err(dev, "malformed fc msg (%d)\n", len);
+ return -EINVAL;
+ }
+
+ chinfo.src = rpmsg32_to_cpu(rpdev, msg->src);
+ chinfo.dst = rpmsg32_to_cpu(rpdev, msg->dst);
+ enable = rpmsg32_to_cpu(rpdev, msg->flags) & RPMSG_EPT_FC_ON;
+
+ dev_dbg(dev, "remote endpoint 0x%x in state %sable\n", chinfo.src, enable ? "en" : "dis");
+
+ ret = rpmsg_channel_remote_fc(rpdev, &chinfo, enable);
+ if (ret)
+ dev_err(dev, "rpmsg_annouce_flow_ctrl failed: %d\n", ret);
+
+ return ret;
+}
+
+static int rpmsg_fc_probe(struct rpmsg_device *rpdev)
+{
+ struct rpmsg_endpoint *fc_ept;
+ struct rpmsg_channel_info fc_chinfo = {
+ .src = RPMSG_FC_ADDR,
+ .dst = RPMSG_FC_ADDR,
+ .name = "flow_control_service",
+ };
+
+ /*
+ * Create the Flow control (FC) service endpoint associated to the RPMsg
+ * device. The endpoint will be automatically destroyed when the RPMsg
+ * device will be deleted.
+ */
+ fc_ept = rpmsg_create_ept(rpdev, rpmsg_fc_cb, NULL, fc_chinfo);
+ if (!fc_ept) {
+ dev_err(&rpdev->dev, "failed to create the FC ept\n");
+ return -ENOMEM;
+ }
+ rpdev->ept = fc_ept;
+
+ return 0;
+}
+
+static struct rpmsg_driver rpmsg_fc_driver = {
+ .drv.name = KBUILD_MODNAME,
+ .probe = rpmsg_fc_probe,
+};
+
+static int rpmsg_fc_init(void)
+{
+ int ret;
+
+ ret = register_rpmsg_driver(&rpmsg_fc_driver);
+ if (ret < 0)
+ pr_err("%s: Failed to register FC rpmsg driver\n", __func__);
+
+ return ret;
+}
+postcore_initcall(rpmsg_fc_init);
+
+static void rpmsg_fc_exit(void)
+{
+ unregister_rpmsg_driver(&rpmsg_fc_driver);
+}
+module_exit(rpmsg_fc_exit);
+
+MODULE_DESCRIPTION("Flow control service rpmsg driver");
+MODULE_AUTHOR("Arnaud Pouliquen <[email protected]>");
+MODULE_ALIAS("rpmsg:" KBUILD_MODNAME);
+MODULE_LICENSE("GPL");
diff --git a/include/linux/rpmsg/fc.h b/include/linux/rpmsg/fc.h
new file mode 100644
index 000000000000..5284ea410673
--- /dev/null
+++ b/include/linux/rpmsg/fc.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_RPMSG_FC_H
+#define _LINUX_RPMSG_FC_H
+
+#include <linux/mod_devicetable.h>
+#include <linux/rpmsg.h>
+#include <linux/rpmsg/byteorder.h>
+#include <linux/types.h>
+
+/* The feature bitmap for the endpoint flow control flags */
+#define RPMSG_EPT_FC_ON BIT(0) /* Set when endpoint is ready to communicate */
+
+/**
+ * struct rpmsg_ept_msg - dynamic endpoint announcement message
+ * @src: address of the endpoint that sends the message
+ * @dest: address of the destination endpoint.
+ * @flags: indicates the state of the endpoint based on @rpmsg_ept_flags enum.
+ *
+ * This message is sent across to inform the remote about the state of a local
+ * endpoint associated with a remote endpoint:
+ * - a RPMSG_EPT_OFF can be sent to inform that a local endpoint is suspended.
+ * - a RPMSG_EPT_ON can be sent to inform that a local endpoint is ready to communicate.
+ *
+ * When we receive these messages, the appropriate endpoint is informed.
+ */
+struct rpmsg_ept_msg {
+ __rpmsg32 src;
+ __rpmsg32 dst;
+ __rpmsg32 flags;
+} __packed;
+
+/* Address 54 is reserved for flow control advertising */
+#define RPMSG_FC_ADDR (54)
+
+#if IS_ENABLED(CONFIG_RPMSG_FC)
+
+int rpmsg_fc_register_device(struct rpmsg_device *rpdev);
+
+#else
+
+static inline int rpmsg_fc_register_device(struct rpmsg_device *rpdev)
+{
+ /* This shouldn't be possible */
+ WARN_ON(1);
+
+ return -ENXIO;
+}
+#endif /* IS_ENABLED(CONFIG_RPMSG_FC)*/
+
+#endif
--
2.25.1


2022-05-23 02:05:28

by Arnaud Pouliquen

[permalink] [raw]
Subject: [RFC PATCH 06/10] rpmsg: virtio: Add support of the VIRTIO_RPMSG_F_FC feature

Introduce the VIRTIO_RPMSG_F_FC feature in charge of the end
point flow control management.
The virtio feature is negotiated. If the remote side supports it,
the rpmsg_fc device is probed.

Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/rpmsg/Kconfig | 1 +
drivers/rpmsg/virtio_rpmsg_bus.c | 32 +++++++++++++++++++++++++++++++-
2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
index c6659f27c617..e39cf32483de 100644
--- a/drivers/rpmsg/Kconfig
+++ b/drivers/rpmsg/Kconfig
@@ -89,6 +89,7 @@ config RPMSG_VIRTIO
depends on HAS_DMA
select RPMSG
select RPMSG_NS
+ select RPMSG_FC
select VIRTIO

endmenu
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 785fda77984e..40d2ab86b395 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -19,6 +19,7 @@
#include <linux/mutex.h>
#include <linux/rpmsg.h>
#include <linux/rpmsg/byteorder.h>
+#include <linux/rpmsg/fc.h>
#include <linux/rpmsg/ns.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
@@ -70,6 +71,7 @@ struct virtproc_info {

/* The feature bitmap for virtio rpmsg */
#define VIRTIO_RPMSG_F_NS 0 /* RP supports name service notifications */
+#define VIRTIO_RPMSG_F_FC 1 /* RP supports endpoint flow control notifications */

/**
* struct rpmsg_hdr - common header for all rpmsg messages
@@ -909,7 +911,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
struct virtqueue *vqs[2];
struct virtproc_info *vrp;
struct virtio_rpmsg_channel *vch = NULL;
- struct rpmsg_device *rpdev_ns, *rpdev_ctrl;
+ struct rpmsg_device *rpdev_ns = NULL, *rpdev_ctrl, *rpdev_fc;
void *bufs_va;
int err = 0, i;
size_t total_buf_space;
@@ -1013,6 +1015,30 @@ static int rpmsg_probe(struct virtio_device *vdev)
goto free_ctrldev;
}

+ /* If supported by the remote processor, enable the flow control service */
+ if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_FC)) {
+ vch = kzalloc(sizeof(*vch), GFP_KERNEL);
+ if (!vch) {
+ err = -ENOMEM;
+ goto free_ns;
+ }
+
+ /* Link the channel to our vrp */
+ vch->vrp = vrp;
+
+ /* Assign public information to the rpmsg_device */
+ rpdev_fc = &vch->rpdev;
+ rpdev_fc->ops = &virtio_rpmsg_ops;
+ rpdev_fc->little_endian = virtio_is_little_endian(vrp->vdev);
+
+ rpdev_fc->dev.parent = &vrp->vdev->dev;
+ rpdev_fc->dev.release = virtio_rpmsg_release_device;
+
+ err = rpmsg_fc_register_device(rpdev_fc);
+ if (err)
+ goto free_ns;
+ }
+
/*
* Prepare to kick but don't notify yet - we can't do this before
* device is ready.
@@ -1034,6 +1060,9 @@ static int rpmsg_probe(struct virtio_device *vdev)

return 0;

+free_ns:
+ if (rpdev_ns)
+ device_unregister(&rpdev_ns->dev);
free_ctrldev:
rpmsg_virtio_del_ctrl_dev(rpdev_ctrl);
free_coherent:
@@ -1082,6 +1111,7 @@ static struct virtio_device_id id_table[] = {

static unsigned int features[] = {
VIRTIO_RPMSG_F_NS,
+ VIRTIO_RPMSG_F_FC,
};

static struct virtio_driver virtio_ipc_driver = {
--
2.25.1


2022-05-23 07:10:15

by Arnaud Pouliquen

[permalink] [raw]
Subject: [RFC PATCH 08/10] rpmsg: Add the destination address in rpmsg_set_flow_control

The destination address is not part of the rpmsg_endpoint structure.
For static endpoint without channel, we need to specify the
destination address for backend such as the rpmsg virtio.
It is also needed for endpoint to multi endpoint communication.

Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/rpmsg/rpmsg_char.c | 2 +-
drivers/rpmsg/rpmsg_core.c | 6 ++++--
drivers/rpmsg/rpmsg_internal.h | 2 +-
drivers/rpmsg/virtio_rpmsg_bus.c | 6 +++---
include/linux/rpmsg.h | 4 ++--
5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index 904e7c67b356..f44747e64b5b 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -316,7 +316,7 @@ static long rpmsg_eptdev_ioctl(struct file *fp, unsigned int cmd,
if (ret)
break;
set = (val & TIOCM_DTR) ? true : false;
- ret = rpmsg_set_flow_control(eptdev->ept, set);
+ ret = rpmsg_set_flow_control(eptdev->ept, eptdev->chinfo.dst, set);
break;
case RPMSG_DESTROY_EPT_IOCTL:
/* Don't allow to destroy a default endpoint. */
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 6bbc3b3ace50..ab9a170ffa93 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -357,18 +357,20 @@ EXPORT_SYMBOL(rpmsg_trysend_offchannel);
/**
* rpmsg_set_flow_control() - sets/clears serial flow control signals
* @ept: the rpmsg endpoint
+ * @dst: the remote endpoint destination addr, set to RPMSG_ADDR_ANY to send to the default
+ * remote endpoint associated to the rpmsg device.
* @enable: enable or disable serial flow control
*
* Return: 0 on success and an appropriate error value on failure.
*/
-int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable)
+int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, u32 dst, bool enable)
{
if (WARN_ON(!ept))
return -EINVAL;
if (!ept->ops->set_flow_control)
return -ENXIO;

- return ept->ops->set_flow_control(ept, enable);
+ return ept->ops->set_flow_control(ept, dst, enable);
}
EXPORT_SYMBOL(rpmsg_set_flow_control);

diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 44e2c0f2f5ea..c34175f5093c 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -79,7 +79,7 @@ struct rpmsg_endpoint_ops {
void *data, int len);
__poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp,
poll_table *wait);
- int (*set_flow_control)(struct rpmsg_endpoint *ept, bool enable);
+ int (*set_flow_control)(struct rpmsg_endpoint *ept, u32 dst, bool enable);
ssize_t (*get_mtu)(struct rpmsg_endpoint *ept);
};

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 96bd12095c8c..a8e60ca4cd08 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -154,7 +154,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct rpmsg_endpoint *ept);
static struct rpmsg_device *__rpmsg_create_channel(struct virtproc_info *vrp,
struct rpmsg_channel_info *chinfo);

-static int virtio_rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable);
+static int virtio_rpmsg_set_flow_control(struct rpmsg_endpoint *ept, u32 dst, bool enable);

static const struct rpmsg_endpoint_ops virtio_endpoint_ops = {
.destroy_ept = virtio_rpmsg_destroy_ept,
@@ -748,7 +748,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct rpmsg_endpoint *ept)
return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
}

-static int virtio_rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable)
+static int virtio_rpmsg_set_flow_control(struct rpmsg_endpoint *ept, u32 dst, bool enable)
{
struct rpmsg_device *rpdev;
struct virtio_rpmsg_channel *vch;
@@ -764,7 +764,7 @@ static int virtio_rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable
struct rpmsg_ept_msg msg;

msg.src = cpu_to_rpmsg32(rpdev, ept->addr);
- msg.dst = cpu_to_rpmsg32(rpdev, rpdev->dst);
+ msg.dst = cpu_to_rpmsg32(rpdev, dst == RPMSG_ADDR_ANY ? rpdev->dst : dst);
msg.flags = cpu_to_rpmsg32(rpdev, enable ? RPMSG_EPT_FC_ON : 0);

err = rpmsg_sendto(ept, &msg, sizeof(msg), RPMSG_FC_ADDR);
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index dbd0c591bb85..9b321e1554e3 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -193,7 +193,7 @@ __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,

ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept);

-int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable);
+int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, u32 dst, bool enable);

#else

@@ -313,7 +313,7 @@ static inline ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
return -ENXIO;
}

-static inline int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable)
+static inline int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, u32 dst, bool enable)
{
/* This shouldn't be possible */
WARN_ON(1);
--
2.25.1


2022-05-23 07:47:03

by Arnaud Pouliquen

[permalink] [raw]
Subject: [RFC PATCH 03/10] rpmsg: core: Add rpmsg device remote flow control announcement ops

This ops is called by the rpmsg flow control service to inform
a rpmsg local device of a remote endpoint flow control state.

Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/rpmsg/rpmsg_core.c | 24 ++++++++++++++++++++++++
drivers/rpmsg/rpmsg_internal.h | 7 +++++++
2 files changed, 31 insertions(+)

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 8de8aadd9b27..6bbc3b3ace50 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -67,6 +67,30 @@ int rpmsg_release_channel(struct rpmsg_device *rpdev,
}
EXPORT_SYMBOL(rpmsg_release_channel);

+/**
+ * rpmsg_channel_remote_fc() - announce remote endpoint flow control state
+ * using source and destination endpoint address info.
+ * @rpdev: rpmsg device
+ * @chinfo: channel_info
+ * @enable: state of the remote endpoint
+ *
+ * Return: 0 on success or an appropriate error value.
+ */
+int rpmsg_channel_remote_fc(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo,
+ bool enable)
+{
+ if (WARN_ON(!rpdev))
+ return -EINVAL;
+ if (!rpdev->ops || !rpdev->ops->announce_remote_fc) {
+ dev_err(&rpdev->dev, "no flow control ops found\n");
+ return -ENXIO;
+ }
+
+ return rpdev->ops->announce_remote_fc(rpdev, chinfo, enable);
+}
+EXPORT_SYMBOL(rpmsg_channel_remote_fc);
+
/**
* 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 4ce58e68af30..44e2c0f2f5ea 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -43,6 +43,9 @@ struct rpmsg_device_ops {

int (*announce_create)(struct rpmsg_device *ept);
int (*announce_destroy)(struct rpmsg_device *ept);
+ int (*announce_remote_fc)(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo,
+ bool enable);
};

/**
@@ -87,6 +90,10 @@ 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);
+
+int rpmsg_channel_remote_fc(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo,
+ bool enable);
/**
* rpmsg_ctrldev_register_device() - register a char device for control based on rpdev
* @rpdev: prepared rpdev to be used for creating endpoints
--
2.25.1


2022-05-24 18:15:14

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [RFC PATCH 00/10] Introduction of rpmsg flow control service

Hi Arnaud,

I added your patchset to my review list. Unfortunately it sits behind
a sizable backlog and as such I won't be able to get to it for some
time.

Thanks,
Mathieu

On Fri, 20 May 2022 at 02:30, Arnaud Pouliquen
<[email protected]> wrote:
>
> This series proposes an implementation for the rpmsg virtio transport
> backend, of the signaling API proposed by Deepak Kumar Singh:
> "rpmsg and glink signaling API support" [1]
>
> The aim of the series is to offer the possibility for an endpoint to inform
> a remote endpoint about its state, based on a software flow control[2].
>
> For this a new rpmsg service( with a fixed address 64) is proposed.
> It is responsible for:
> - transmitting local endpoint flow control information to the remote side,
> - informing a local endpoint about a remote endpoint flow control.
>
> For the rpmsg virtio transport layer the service is negotiated thanks to the
> virtio feature flag: VIRTIO_RPMSG_F_FC
>
> Notice that this pull request introduces new feature in the rpmsg protocol,
> So it has to be aligned with OpenAMP implementation.
> Pending OpenAMP pull request is available here:
> https://github.com/OpenAMP/open-amp/pull/394
>
>
> [1]https://lkml.org/lkml/2022/1/18/867
> [2]https://en.wikipedia.org/wiki/Software_flow_control
>
> Arnaud Pouliquen (8):
> rpmsg: core: Add rpmsg device remote flow control announcement ops
> rpmsg: virtio: Implement the announce_remote_fc ops
> rpmsg: Introduce flow control channel driver
> rpmsg: virtio: Add support of the VIRTIO_RPMSG_F_FC feature
> rpmsg: virtio: Implement the set_flow_control ops
> rpmsg: Add the destination address in rpmsg_set_flow_control
> rpmsg: tty : Add the support of the flow control
> rpmsg: virtio: Set default dst address on flow control
>
> Deepak Kumar Singh (2):
> rpmsg: core: Add signal API support
> rpmsg: char: Add TIOCMGET/TIOCMSET ioctl support
>
> drivers/rpmsg/Kconfig | 11 +++
> drivers/rpmsg/Makefile | 1 +
> drivers/rpmsg/rpmsg_char.c | 56 +++++++++++++--
> drivers/rpmsg/rpmsg_core.c | 47 +++++++++++++
> drivers/rpmsg/rpmsg_fc.c | 113 +++++++++++++++++++++++++++++++
> drivers/rpmsg/rpmsg_internal.h | 9 +++
> drivers/rpmsg/virtio_rpmsg_bus.c | 111 +++++++++++++++++++++++++++++-
> drivers/tty/rpmsg_tty.c | 50 ++++++++++++++
> include/linux/rpmsg.h | 15 ++++
> include/linux/rpmsg/fc.h | 51 ++++++++++++++
> 10 files changed, 456 insertions(+), 8 deletions(-)
> create mode 100644 drivers/rpmsg/rpmsg_fc.c
> create mode 100644 include/linux/rpmsg/fc.h
>
> --
> 2.25.1
>

2022-05-25 12:29:15

by Arnaud Pouliquen

[permalink] [raw]
Subject: Re: [RFC PATCH 00/10] Introduction of rpmsg flow control service

Hi Mathieu,

On 5/24/22 17:34, Mathieu Poirier wrote:
> Hi Arnaud,
>
> I added your patchset to my review list. Unfortunately it sits behind
> a sizable backlog and as such I won't be able to get to it for some
> time.

No worries, I hope to get some feedbacks and to have discussion on the
topic from some other people as well
FYI, as a similar Pull request exists on OpenAMP github, I requsted in the
OpenAMP PR to centralize all the discussions around the design choice in this
thread. The aim is that we have a single discussion thread to find a consensus
on the way of implementing such service on virtio backend.

Creating a specific rpmsg service is one approach, some other can exist...

Regards,
Arnaud


>
> Thanks,
> Mathieu
>
> On Fri, 20 May 2022 at 02:30, Arnaud Pouliquen
> <[email protected]> wrote:
>>
>> This series proposes an implementation for the rpmsg virtio transport
>> backend, of the signaling API proposed by Deepak Kumar Singh:
>> "rpmsg and glink signaling API support" [1]
>>
>> The aim of the series is to offer the possibility for an endpoint to inform
>> a remote endpoint about its state, based on a software flow control[2].
>>
>> For this a new rpmsg service( with a fixed address 64) is proposed.
>> It is responsible for:
>> - transmitting local endpoint flow control information to the remote side,
>> - informing a local endpoint about a remote endpoint flow control.
>>
>> For the rpmsg virtio transport layer the service is negotiated thanks to the
>> virtio feature flag: VIRTIO_RPMSG_F_FC
>>
>> Notice that this pull request introduces new feature in the rpmsg protocol,
>> So it has to be aligned with OpenAMP implementation.
>> Pending OpenAMP pull request is available here:
>> https://github.com/OpenAMP/open-amp/pull/394
>>
>>
>> [1]https://lkml.org/lkml/2022/1/18/867
>> [2]https://en.wikipedia.org/wiki/Software_flow_control
>>
>> Arnaud Pouliquen (8):
>> rpmsg: core: Add rpmsg device remote flow control announcement ops
>> rpmsg: virtio: Implement the announce_remote_fc ops
>> rpmsg: Introduce flow control channel driver
>> rpmsg: virtio: Add support of the VIRTIO_RPMSG_F_FC feature
>> rpmsg: virtio: Implement the set_flow_control ops
>> rpmsg: Add the destination address in rpmsg_set_flow_control
>> rpmsg: tty : Add the support of the flow control
>> rpmsg: virtio: Set default dst address on flow control
>>
>> Deepak Kumar Singh (2):
>> rpmsg: core: Add signal API support
>> rpmsg: char: Add TIOCMGET/TIOCMSET ioctl support
>>
>> drivers/rpmsg/Kconfig | 11 +++
>> drivers/rpmsg/Makefile | 1 +
>> drivers/rpmsg/rpmsg_char.c | 56 +++++++++++++--
>> drivers/rpmsg/rpmsg_core.c | 47 +++++++++++++
>> drivers/rpmsg/rpmsg_fc.c | 113 +++++++++++++++++++++++++++++++
>> drivers/rpmsg/rpmsg_internal.h | 9 +++
>> drivers/rpmsg/virtio_rpmsg_bus.c | 111 +++++++++++++++++++++++++++++-
>> drivers/tty/rpmsg_tty.c | 50 ++++++++++++++
>> include/linux/rpmsg.h | 15 ++++
>> include/linux/rpmsg/fc.h | 51 ++++++++++++++
>> 10 files changed, 456 insertions(+), 8 deletions(-)
>> create mode 100644 drivers/rpmsg/rpmsg_fc.c
>> create mode 100644 include/linux/rpmsg/fc.h
>>
>> --
>> 2.25.1
>>