This patch set introduces a TTY console on top of the RPMsg framework which
enables the following use cases:
- Provide a console to communicate easily with the remote processor
application.
- Provide an interface to get the remote processor log traces without
ring buffer limitation.
- Ease the migration from MPU + MCU processors to multi core processors
(MPU and MCU integrated in one processor)
An alternative of this proposed solution would consist in using the virtio
console:
The drawback with that solution is that it requires a specific virtio buffer
(in addition to the one already used for RPMsg) which does not fit with remote
processors with little memory. The proposed solution allows to multiplex the
console with the other rpmsg services, optimizing the memory.
The first patch adds an API to the rpmsg framework ('get max transmission unit') and the
second one is the rpmsg tty driver itself.
History:
-V4 to V5:
rework RPMSG channels to offer 2 modes: with and without flow control
- remove the use of the first message byte to differentiate data and control
- allow communication without flow control using a single RPMsg channel
- implement flow control with creating of 2 endpoints
- default one for the control
- second one for the data
- data endpoint address is transmitted to the remote side trougnt the control channel
-V3 to V4:
- reformat documentation in rst format
- use tty_insert_flip_string_fixed_flag helper
- suppress some poinrter check (overprotection)
- move low_latency set from probe to activate ops.
- various corrections and improvements relative to Jiri's comments
-V2 to V3:
- suppress error return on rpmsg callback as not tested in rpmsg framework
- change some flow messages level to debug
- add missing out of memory checks
-V1 to V2:
- modify message structure to allow to data transmission but also
flow control
- add documentation file to describe message structure for remote
implementation
- add dtr/rts management
- disable termios modes that generates non optimized behavior on RPMsg
transfers
- replace rpmsg_send by rpmsg_trysend to not block the write
- suppress useless spinlock on read
- miscellaneous fixes to improve robustness
Arnaud Pouliquen (2):
rpmsg: core: add API to get message length
tty: add rpmsg driver
Documentation/serial/tty_rpmsg.rst | 45 ++++
drivers/rpmsg/rpmsg_core.c | 21 ++
drivers/rpmsg/rpmsg_internal.h | 2 +
drivers/rpmsg/virtio_rpmsg_bus.c | 10 +
drivers/tty/Kconfig | 9 +
drivers/tty/Makefile | 1 +
drivers/tty/rpmsg_tty.c | 418 +++++++++++++++++++++++++++++++++++++
include/linux/rpmsg.h | 10 +
8 files changed, 516 insertions(+)
create mode 100644 Documentation/serial/tty_rpmsg.rst
create mode 100644 drivers/tty/rpmsg_tty.c
--
2.7.4
Return the rpmsg buffer size for sending message, so rpmsg users
can split a long message in several sub rpmsg buffers.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
V4 to V5 :
- rename rpmsg_get_buf_payload_size to rpmsg_get_mtu
drivers/rpmsg/rpmsg_core.c | 21 +++++++++++++++++++++
drivers/rpmsg/rpmsg_internal.h | 2 ++
drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++++
include/linux/rpmsg.h | 10 ++++++++++
4 files changed, 43 insertions(+)
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 8122807db380..daca2e24fc71 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -283,6 +283,27 @@ int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
}
EXPORT_SYMBOL(rpmsg_trysend_offchannel);
+/**
+ * rpmsg_get_mtu() - get maximum transmission buffer size for sending message.
+ * @ept: the rpmsg endpoint
+ *
+ * This function returns maximum buffer size available for a single message.
+ *
+ * Return: the maximum transmission size on success and an appropriate error
+ * value on failure.
+ */
+
+ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
+{
+ if (WARN_ON(!ept))
+ return -EINVAL;
+ if (!ept->ops->get_buf_mtu)
+ return -ENXIO;
+
+ return ept->ops->get_buf_mtu(ept);
+}
+EXPORT_SYMBOL(rpmsg_get_mtu);
+
/*
* match an rpmsg channel with a channel info struct.
* this is used to make sure we're not creating rpmsg devices for channels
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 0d791c30b7ea..645c402569ac 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -46,6 +46,7 @@ struct rpmsg_device_ops {
* @trysend: see @rpmsg_trysend(), required
* @trysendto: see @rpmsg_trysendto(), optional
* @trysend_offchannel: see @rpmsg_trysend_offchannel(), optional
+ * @get_buf_payload_size: see @rpmsg_get_buf_payload_size(), optional
*
* Indirection table for the operations that a rpmsg backend should implement.
* In addition to @destroy_ept, the backend must at least implement @send and
@@ -65,6 +66,7 @@ struct rpmsg_endpoint_ops {
void *data, int len);
__poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp,
poll_table *wait);
+ ssize_t (*get_buf_mtu)(struct rpmsg_endpoint *ept);
};
int rpmsg_register_device(struct rpmsg_device *rpdev);
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index e757f0038a1c..f80b1ad23e7e 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -178,6 +178,7 @@ static int virtio_rpmsg_trysendto(struct rpmsg_endpoint *ept, void *data,
int len, u32 dst);
static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
u32 dst, void *data, int len);
+static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept);
static const struct rpmsg_endpoint_ops virtio_endpoint_ops = {
.destroy_ept = virtio_rpmsg_destroy_ept,
@@ -187,6 +188,7 @@ static const struct rpmsg_endpoint_ops virtio_endpoint_ops = {
.trysend = virtio_rpmsg_trysend,
.trysendto = virtio_rpmsg_trysendto,
.trysend_offchannel = virtio_rpmsg_trysend_offchannel,
+ .get_buf_mtu = virtio_get_buf_mtu,
};
/**
@@ -702,6 +704,14 @@ static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
return rpmsg_send_offchannel_raw(rpdev, src, dst, data, len, false);
}
+static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept)
+{
+ struct rpmsg_device *rpdev = ept->rpdev;
+ struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
+
+ return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
+}
+
static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
struct rpmsg_hdr *msg, unsigned int len)
{
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 9fe156d1c018..9d638bf2bdce 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -135,6 +135,8 @@ int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
__poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
poll_table *wait);
+ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept);
+
#else
static inline int register_rpmsg_device(struct rpmsg_device *dev)
@@ -242,6 +244,14 @@ static inline __poll_t rpmsg_poll(struct rpmsg_endpoint *ept,
return 0;
}
+static ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
+{
+ /* This shouldn't be possible */
+ WARN_ON(1);
+
+ return -ENXIO;
+}
+
#endif /* IS_ENABLED(CONFIG_RPMSG) */
/* use a macro to avoid include chaining to get THIS_MODULE */
--
2.7.4
This driver exposes a standard tty interface on top of the rpmsg
framework through a rpmsg service.
This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
per rpmsg endpoint.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
V4 to V5 :
- suppress the use of the first byte of the message to differenciate control and stream.
- define 2 rpmsg services (for legacy support): with and without CTS flow control.
Documentation/serial/tty_rpmsg.rst | 45 ++++
drivers/tty/Kconfig | 9 +
drivers/tty/Makefile | 1 +
drivers/tty/rpmsg_tty.c | 418 +++++++++++++++++++++++++++++++++++++
4 files changed, 473 insertions(+)
create mode 100644 Documentation/serial/tty_rpmsg.rst
create mode 100644 drivers/tty/rpmsg_tty.c
diff --git a/Documentation/serial/tty_rpmsg.rst b/Documentation/serial/tty_rpmsg.rst
new file mode 100644
index 000000000000..fc1d3fba73c5
--- /dev/null
+++ b/Documentation/serial/tty_rpmsg.rst
@@ -0,0 +1,45 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+The rpmsg TTY
+=============
+
+The rpmsg tty driver implements serial communication on the RPMsg bus to makes possible for user-space programs to send and receive rpmsg messages as a standard tty protocol.
+
+The remote processor can instantiate a new tty by requesting:
+- a "rpmsg-tty-raw" RPMsg service, for TTY raw data support without flow control
+- a "rpmsg-tty-ctrl" RPMSg service, for TTY support with flow control.
+
+Information related to the RPMsg and associated tty device is available in
+/sys/bus/rpmsg/devices/.
+
+RPMsg TTY without control
+---------------------
+
+The default end point associated with the "rpmsg-tty-raw" service is directly
+used for data exchange. No flow control is available.
+
+To be compliant with this driver, the remote firmware must create its data end point associated with the "rpmsg-tty-raw" service.
+
+RPMsg TTY with control
+---------------------
+
+The default end point associated with the "rpmsg-tty-ctrl" service is reserved for
+the control. A second endpoint must be created for data exchange.
+
+The control channel is used to transmit to the remote processor the CTS status,
+as well as the end point address for data transfer.
+
+To be compatible with this driver, the remote firmware must create or use its end point associated with "rpmsg-tty-ctrl" service, plus a second endpoint for the data flow.
+On Linux rpmsg_tty probes, the data endpoint address and the CTS (set to disable)
+is sent to the remote processor.
+The remote processor has to respect following rules:
+- It only transmits data when Linux remote cts is enable, otherwise message
+ could be lost.
+- It can pause/resume reception by sending a control message (rely on CTS state).
+
+Control message structure:
+struct rpmsg_tty_ctrl {
+ u8 cts; /* remote reception status */
+ u16 d_ept_addr; /* data endpoint address */
+};
diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 3b1d312bb175..8f3378f8b9f1 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -454,6 +454,15 @@ config VCC
help
Support for Sun logical domain consoles.
+config RPMSG_TTY
+ tristate "RPMSG tty driver"
+ depends on RPMSG
+ help
+ Say y here to export rpmsg endpoints as tty devices, usually found
+ in /dev/ttyRPMSGx.
+ This makes it possible for user-space programs to send and receive
+ rpmsg messages as a standard tty protocol.
+
config LDISC_AUTOLOAD
bool "Automatically load TTY Line Disciplines"
default y
diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
index 020b1cd9294f..c2465e7ebc2a 100644
--- a/drivers/tty/Makefile
+++ b/drivers/tty/Makefile
@@ -34,5 +34,6 @@ obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o
obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
obj-$(CONFIG_VCC) += vcc.o
+obj-$(CONFIG_RPMSG_TTY) += rpmsg_tty.o
obj-y += ipwireless/
diff --git a/drivers/tty/rpmsg_tty.c b/drivers/tty/rpmsg_tty.c
new file mode 100644
index 000000000000..d5901e3eb7bc
--- /dev/null
+++ b/drivers/tty/rpmsg_tty.c
@@ -0,0 +1,418 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2019 - All Rights Reserved
+ * Authors: Arnaud Pouliquen <[email protected]> for STMicroelectronics.
+ */
+
+#include <linux/module.h>
+#include <linux/rpmsg.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+#define MAX_TTY_RPMSG 32
+
+#define TTY_CH_NAME_RAW "rpmsg-tty-raw"
+#define TTY_CH_NAME_WITH_CTS "rpmsg-tty-ctrl"
+
+static DEFINE_IDR(tty_idr); /* tty instance id */
+static DEFINE_MUTEX(idr_lock); /* protects tty_idr */
+
+static struct tty_driver *rpmsg_tty_driver;
+
+struct rpmsg_tty_ctrl {
+ u8 cts; /* remote reception status */
+ u16 d_ept_addr; /* data endpoint address */
+};
+
+struct rpmsg_tty_port {
+ struct tty_port port; /* TTY port data */
+ int id; /* TTY rpmsg index */
+ bool cts; /* remote reception status */
+ struct rpmsg_device *rpdev; /* rpmsg device */
+ struct rpmsg_endpoint *cs_ept; /* channel control endpoint */
+ struct rpmsg_endpoint *d_ept; /* data endpoint */
+ u32 data_dst; /* data destination endpoint address */
+};
+
+typedef void (*rpmsg_tty_rx_cb_t)(struct rpmsg_device *, void *, int, void *,
+ u32);
+
+static int rpmsg_tty_cb(struct rpmsg_device *rpdev, void *data, int len,
+ void *priv, u32 src)
+{
+ struct rpmsg_tty_port *cport = dev_get_drvdata(&rpdev->dev);
+ int copied;
+
+ if (src == cport->data_dst) {
+ /* data message */
+ if (!len)
+ return -EINVAL;
+ /* data message */
+ copied = tty_insert_flip_string_fixed_flag(&cport->port, data,
+ TTY_NORMAL, len);
+ if (copied != len)
+ dev_dbg(&rpdev->dev, "trunc buffer: available space is %d\n",
+ copied);
+ tty_flip_buffer_push(&cport->port);
+ } else {
+ /* control message */
+ struct rpmsg_tty_ctrl *msg = data;
+
+ if (len != sizeof(*msg))
+ return -EINVAL;
+
+ cport->data_dst = msg->d_ept_addr;
+
+ /* Update remote cts state */
+ cport->cts = msg->cts ? 1 : 0;
+
+ if (cport->cts)
+ tty_port_tty_wakeup(&cport->port);
+ }
+
+ return 0;
+}
+
+static void rpmsg_tty_send_term_ready(struct tty_struct *tty, u8 state)
+{
+ struct rpmsg_tty_port *cport = tty->driver_data;
+ struct rpmsg_tty_ctrl m_ctrl;
+ int ret;
+
+ m_ctrl.cts = state;
+ m_ctrl.d_ept_addr = cport->d_ept->addr;
+
+ ret = rpmsg_trysend(cport->cs_ept, &m_ctrl, sizeof(m_ctrl));
+ if (ret < 0)
+ dev_dbg(tty->dev, "cannot send control (%d)\n", ret);
+};
+
+static void rpmsg_tty_throttle(struct tty_struct *tty)
+{
+ struct rpmsg_tty_port *cport = tty->driver_data;
+
+ /* Disable remote transmission */
+ if (cport->cs_ept)
+ rpmsg_tty_send_term_ready(tty, 0);
+};
+
+static void rpmsg_tty_unthrottle(struct tty_struct *tty)
+{
+ struct rpmsg_tty_port *cport = tty->driver_data;
+
+ /* Enable remote transmission */
+ if (cport->cs_ept)
+ rpmsg_tty_send_term_ready(tty, 1);
+};
+
+static int rpmsg_tty_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+ struct rpmsg_tty_port *cport = idr_find(&tty_idr, tty->index);
+
+ if (!cport) {
+ dev_err(tty->dev, "cannot get cport\n");
+ return -ENODEV;
+ }
+
+ tty->driver_data = cport;
+
+ return tty_port_install(&cport->port, driver, tty);
+}
+
+static int rpmsg_tty_open(struct tty_struct *tty, struct file *filp)
+{
+ return tty_port_open(tty->port, tty, filp);
+}
+
+static void rpmsg_tty_close(struct tty_struct *tty, struct file *filp)
+{
+ return tty_port_close(tty->port, tty, filp);
+}
+
+static int rpmsg_tty_write(struct tty_struct *tty, const u8 *buf, int len)
+{
+ struct rpmsg_tty_port *cport = tty->driver_data;
+ struct rpmsg_device *rpdev;
+ ssize_t msg_max_size, msg_size;
+ int ret;
+ u8 *tmpbuf;
+
+ /* If cts not set, the message is not sent*/
+ if (!cport->cts)
+ return 0;
+
+ rpdev = cport->rpdev;
+
+ dev_dbg(&rpdev->dev, "%s: send msg from tty->index = %d, len = %d\n",
+ __func__, tty->index, len);
+
+ msg_max_size = rpmsg_get_mtu(rpdev->ept);
+
+ msg_size = min(len, msg_max_size);
+ tmpbuf = kzalloc(msg_size, GFP_KERNEL);
+ if (!tmpbuf)
+ return -ENOMEM;
+
+ memcpy(tmpbuf, buf, msg_size);
+
+ /*
+ * Try to send the message to remote processor, if failed return 0 as
+ * no data sent
+ */
+ ret = rpmsg_trysendto(cport->d_ept, tmpbuf, msg_size, cport->data_dst);
+ kfree(tmpbuf);
+ if (ret) {
+ dev_dbg(&rpdev->dev, "rpmsg_send failed: %d\n", ret);
+ return 0;
+ }
+
+ return msg_size;
+}
+
+static int rpmsg_tty_write_room(struct tty_struct *tty)
+{
+ struct rpmsg_tty_port *cport = tty->driver_data;
+
+ return cport->cts ? rpmsg_get_mtu(cport->rpdev->ept) : 0;
+}
+
+static const struct tty_operations rpmsg_tty_ops = {
+ .install = rpmsg_tty_install,
+ .open = rpmsg_tty_open,
+ .close = rpmsg_tty_close,
+ .write = rpmsg_tty_write,
+ .write_room = rpmsg_tty_write_room,
+ .throttle = rpmsg_tty_throttle,
+ .unthrottle = rpmsg_tty_unthrottle,
+};
+
+static struct rpmsg_tty_port *rpmsg_tty_alloc_cport(void)
+{
+ struct rpmsg_tty_port *cport;
+
+ cport = kzalloc(sizeof(*cport), GFP_KERNEL);
+ if (!cport)
+ return ERR_PTR(-ENOMEM);
+
+ mutex_lock(&idr_lock);
+ cport->id = idr_alloc(&tty_idr, cport, 0, MAX_TTY_RPMSG, GFP_KERNEL);
+ mutex_unlock(&idr_lock);
+
+ if (cport->id < 0) {
+ kfree(cport);
+ return ERR_PTR(-ENOSPC);
+ }
+
+ return cport;
+}
+
+static void rpmsg_tty_release_cport(struct rpmsg_tty_port *cport)
+{
+ mutex_lock(&idr_lock);
+ idr_remove(&tty_idr, cport->id);
+ mutex_unlock(&idr_lock);
+
+ kfree(cport);
+}
+
+static int rpmsg_tty_port_activate(struct tty_port *p, struct tty_struct *tty)
+{
+ p->low_latency = (p->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
+
+ /* Allocate the buffer we use for writing data */
+ return tty_port_alloc_xmit_buf(p);
+}
+
+static void rpmsg_tty_port_shutdown(struct tty_port *p)
+{
+ /* Free the write buffer */
+ tty_port_free_xmit_buf(p);
+}
+
+static void rpmsg_tty_dtr_rts(struct tty_port *port, int raise)
+{
+ dev_dbg(port->tty->dev, "%s: dtr_rts state %d\n", __func__, raise);
+
+ if (raise)
+ rpmsg_tty_unthrottle(port->tty);
+ else
+ rpmsg_tty_throttle(port->tty);
+}
+
+static const struct tty_port_operations rpmsg_tty_port_ops = {
+ .activate = rpmsg_tty_port_activate,
+ .shutdown = rpmsg_tty_port_shutdown,
+ .dtr_rts = rpmsg_tty_dtr_rts,
+};
+
+static int rpmsg_tty_probe(struct rpmsg_device *rpdev)
+{
+ struct rpmsg_tty_port *cport;
+ struct device *dev = &rpdev->dev;
+ struct rpmsg_channel_info chinfo;
+ struct device *tty_dev;
+ int ret;
+
+ cport = rpmsg_tty_alloc_cport();
+ if (IS_ERR(cport)) {
+ dev_err(dev, "failed to alloc tty port\n");
+ return PTR_ERR(cport);
+ }
+
+ if (!strncmp(rpdev->id.name, TTY_CH_NAME_WITH_CTS,
+ sizeof(TTY_CH_NAME_WITH_CTS))) {
+ /*
+ * the default endpoint is used for control. Create a second
+ * endpoint for the data that would be exchanges trough control
+ * endpoint. address of the data endpoint will be provided with
+ * the cts state
+ */
+ cport->cs_ept = rpdev->ept;
+ cport->data_dst = RPMSG_ADDR_ANY;
+
+ strscpy(chinfo.name, TTY_CH_NAME_WITH_CTS, sizeof(chinfo.name));
+ chinfo.src = RPMSG_ADDR_ANY;
+ chinfo.dst = RPMSG_ADDR_ANY;
+
+ cport->d_ept = rpmsg_create_ept(rpdev, rpmsg_tty_cb, cport,
+ chinfo);
+ if (!cport->d_ept) {
+ dev_err(dev, "failed to create tty control channel\n");
+ ret = -ENOMEM;
+ goto err_r_cport;
+ }
+ dev_dbg(dev, "%s: creating data endpoint with address %#x\n",
+ __func__, cport->d_ept->addr);
+ } else {
+ /*
+ * TTY over rpmsg without CTS management the default endpoint
+ * is use for raw data transmission.
+ */
+ cport->cs_ept = NULL;
+ cport->cts = 1;
+ cport->d_ept = rpdev->ept;
+ cport->data_dst = rpdev->dst;
+ }
+
+ tty_port_init(&cport->port);
+ cport->port.ops = &rpmsg_tty_port_ops;
+
+ tty_dev = tty_port_register_device(&cport->port, rpmsg_tty_driver,
+ cport->id, dev);
+ if (IS_ERR(tty_dev)) {
+ dev_err(dev, "failed to register tty port\n");
+ ret = PTR_ERR(tty_dev);
+ goto err_destroy;
+ }
+
+ cport->rpdev = rpdev;
+
+ dev_set_drvdata(dev, cport);
+
+ dev_dbg(dev, "new channel: 0x%x -> 0x%x : ttyRPMSG%d\n",
+ rpdev->src, rpdev->dst, cport->id);
+
+ return 0;
+
+err_destroy:
+ tty_port_destroy(&cport->port);
+ if (cport->cs_ept)
+ rpmsg_destroy_ept(cport->d_ept);
+err_r_cport:
+ rpmsg_tty_release_cport(cport);
+
+ return ret;
+}
+
+static void rpmsg_tty_remove(struct rpmsg_device *rpdev)
+{
+ struct rpmsg_tty_port *cport = dev_get_drvdata(&rpdev->dev);
+
+ dev_dbg(&rpdev->dev, "removing rpmsg tty device %d\n", cport->id);
+
+ /* User hang up to release the tty */
+ if (tty_port_initialized(&cport->port))
+ tty_port_tty_hangup(&cport->port, false);
+
+ tty_unregister_device(rpmsg_tty_driver, cport->id);
+
+ tty_port_destroy(&cport->port);
+ if (cport->cs_ept)
+ rpmsg_destroy_ept(cport->d_ept);
+ rpmsg_tty_release_cport(cport);
+}
+
+static struct rpmsg_device_id rpmsg_driver_tty_id_table[] = {
+ { .name = TTY_CH_NAME_RAW },
+ { .name = TTY_CH_NAME_WITH_CTS},
+ { },
+};
+MODULE_DEVICE_TABLE(rpmsg, rpmsg_driver_tty_id_table);
+
+static struct rpmsg_driver rpmsg_tty_rpmsg_drv = {
+ .drv.name = KBUILD_MODNAME,
+ .id_table = rpmsg_driver_tty_id_table,
+ .probe = rpmsg_tty_probe,
+ .callback = rpmsg_tty_cb,
+ .remove = rpmsg_tty_remove,
+};
+
+static int __init rpmsg_tty_init(void)
+{
+ int err;
+
+ rpmsg_tty_driver = tty_alloc_driver(MAX_TTY_RPMSG, TTY_DRIVER_REAL_RAW |
+ TTY_DRIVER_DYNAMIC_DEV);
+ if (IS_ERR(rpmsg_tty_driver))
+ return PTR_ERR(rpmsg_tty_driver);
+
+ rpmsg_tty_driver->driver_name = "rpmsg_tty";
+ rpmsg_tty_driver->name = "ttyRPMSG";
+ rpmsg_tty_driver->major = 0;
+ rpmsg_tty_driver->type = TTY_DRIVER_TYPE_CONSOLE;
+
+ /* Disable unused mode by default */
+ rpmsg_tty_driver->init_termios = tty_std_termios;
+ rpmsg_tty_driver->init_termios.c_lflag &= ~(ECHO | ICANON);
+ rpmsg_tty_driver->init_termios.c_oflag &= ~(OPOST | ONLCR);
+
+ tty_set_operations(rpmsg_tty_driver, &rpmsg_tty_ops);
+
+ err = tty_register_driver(rpmsg_tty_driver);
+ if (err < 0) {
+ pr_err("Couldn't install rpmsg tty driver: err %d\n", err);
+ goto error_put;
+ }
+
+ err = register_rpmsg_driver(&rpmsg_tty_rpmsg_drv);
+ if (err < 0) {
+ pr_err("Couldn't register rpmsg tty driver: err %d\n", err);
+ goto error_unregister;
+ }
+
+ return 0;
+
+error_unregister:
+ tty_unregister_driver(rpmsg_tty_driver);
+
+error_put:
+ put_tty_driver(rpmsg_tty_driver);
+
+ return err;
+}
+
+static void __exit rpmsg_tty_exit(void)
+{
+ unregister_rpmsg_driver(&rpmsg_tty_rpmsg_drv);
+ tty_unregister_driver(rpmsg_tty_driver);
+ put_tty_driver(rpmsg_tty_driver);
+ idr_destroy(&tty_idr);
+}
+
+module_init(rpmsg_tty_init);
+module_exit(rpmsg_tty_exit);
+
+MODULE_AUTHOR("Arnaud Pouliquen <[email protected]>");
+MODULE_DESCRIPTION("remote processor messaging tty driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
Hi Arnaud,
On 8/28/19 10:19 AM, Arnaud Pouliquen wrote:
> Return the rpmsg buffer size for sending message, so rpmsg users
> can split a long message in several sub rpmsg buffers.
Thanks for the patch, I also have a need for the same to be able to
compute permissible payload size. Minor comments below.
>
> Signed-off-by: Arnaud Pouliquen <[email protected]>
> ---
> V4 to V5 :
> - rename rpmsg_get_buf_payload_size to rpmsg_get_mtu
>
> drivers/rpmsg/rpmsg_core.c | 21 +++++++++++++++++++++
> drivers/rpmsg/rpmsg_internal.h | 2 ++
> drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++++
> include/linux/rpmsg.h | 10 ++++++++++
> 4 files changed, 43 insertions(+)
>
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index 8122807db380..daca2e24fc71 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -283,6 +283,27 @@ int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
> }
> EXPORT_SYMBOL(rpmsg_trysend_offchannel);
>
> +/**
> + * rpmsg_get_mtu() - get maximum transmission buffer size for sending message.
> + * @ept: the rpmsg endpoint
> + *
> + * This function returns maximum buffer size available for a single message.
> + *
> + * Return: the maximum transmission size on success and an appropriate error
> + * value on failure.
> + */
> +
> +ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
> +{
> + if (WARN_ON(!ept))
> + return -EINVAL;
> + if (!ept->ops->get_buf_mtu)
How about calling the ops just get_mtu or rename the function to follow
the ops name, like all the others.
> + return -ENXIO;
Perhaps ENOTSUPP or EOPNOTSUPP.
> +
> + return ept->ops->get_buf_mtu(ept);
> +}
> +EXPORT_SYMBOL(rpmsg_get_mtu);
> +
> /*
> * match an rpmsg channel with a channel info struct.
> * this is used to make sure we're not creating rpmsg devices for channels
> diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
> index 0d791c30b7ea..645c402569ac 100644
> --- a/drivers/rpmsg/rpmsg_internal.h
> +++ b/drivers/rpmsg/rpmsg_internal.h
> @@ -46,6 +46,7 @@ struct rpmsg_device_ops {
> * @trysend: see @rpmsg_trysend(), required
> * @trysendto: see @rpmsg_trysendto(), optional
> * @trysend_offchannel: see @rpmsg_trysend_offchannel(), optional
> + * @get_buf_payload_size: see @rpmsg_get_buf_payload_size(), optional
Missed updating the kerneldoc to the new name.
> *
> * Indirection table for the operations that a rpmsg backend should implement.
> * In addition to @destroy_ept, the backend must at least implement @send and
> @@ -65,6 +66,7 @@ struct rpmsg_endpoint_ops {
> void *data, int len);
> __poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp,
> poll_table *wait);
> + ssize_t (*get_buf_mtu)(struct rpmsg_endpoint *ept);
> };
>
> int rpmsg_register_device(struct rpmsg_device *rpdev);
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index e757f0038a1c..f80b1ad23e7e 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -178,6 +178,7 @@ static int virtio_rpmsg_trysendto(struct rpmsg_endpoint *ept, void *data,
> int len, u32 dst);
> static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
> u32 dst, void *data, int len);
> +static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept);
Minor nit, virtio_rpmsg_ prefix similar to all the other ops.
regards
Suman
>
> static const struct rpmsg_endpoint_ops virtio_endpoint_ops = {
> .destroy_ept = virtio_rpmsg_destroy_ept,
> @@ -187,6 +188,7 @@ static const struct rpmsg_endpoint_ops virtio_endpoint_ops = {
> .trysend = virtio_rpmsg_trysend,
> .trysendto = virtio_rpmsg_trysendto,
> .trysend_offchannel = virtio_rpmsg_trysend_offchannel,
> + .get_buf_mtu = virtio_get_buf_mtu,
> };
>
> /**
> @@ -702,6 +704,14 @@ static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
> return rpmsg_send_offchannel_raw(rpdev, src, dst, data, len, false);
> }
>
> +static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept)
> +{
> + struct rpmsg_device *rpdev = ept->rpdev;
> + struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
> +
> + return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
> +}
> +
> static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
> struct rpmsg_hdr *msg, unsigned int len)
> {
> diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
> index 9fe156d1c018..9d638bf2bdce 100644
> --- a/include/linux/rpmsg.h
> +++ b/include/linux/rpmsg.h
> @@ -135,6 +135,8 @@ int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
> __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
> poll_table *wait);
>
> +ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept);
> +
> #else
>
> static inline int register_rpmsg_device(struct rpmsg_device *dev)
> @@ -242,6 +244,14 @@ static inline __poll_t rpmsg_poll(struct rpmsg_endpoint *ept,
> return 0;
> }
>
> +static ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
> +{
> + /* This shouldn't be possible */
> + WARN_ON(1);
> +
> + return -ENXIO;
> +}
> +
> #endif /* IS_ENABLED(CONFIG_RPMSG) */
>
> /* use a macro to avoid include chaining to get THIS_MODULE */
>
Hi Arnaud,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc6 next-20190829]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Arnaud-Pouliquen/rpmsg-core-add-API-to-get-message-length/20190829-222443
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from include/linux/list.h:9:0,
from include/linux/module.h:9,
from drivers/tty/rpmsg_tty.c:7:
drivers/tty/rpmsg_tty.c: In function 'rpmsg_tty_write':
include/linux/kernel.h:821:29: warning: comparison of distinct pointer types lacks a cast
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
^
include/linux/kernel.h:835:4: note: in expansion of macro '__typecheck'
(__typecheck(x, y) && __no_side_effects(x, y))
^~~~~~~~~~~
include/linux/kernel.h:845:24: note: in expansion of macro '__safe_cmp'
__builtin_choose_expr(__safe_cmp(x, y), \
^~~~~~~~~~
include/linux/kernel.h:854:19: note: in expansion of macro '__careful_cmp'
#define min(x, y) __careful_cmp(x, y, <)
^~~~~~~~~~~~~
>> drivers/tty/rpmsg_tty.c:152:13: note: in expansion of macro 'min'
msg_size = min(len, msg_max_size);
^~~
vim +/min +152 drivers/tty/rpmsg_tty.c
132
133 static int rpmsg_tty_write(struct tty_struct *tty, const u8 *buf, int len)
134 {
135 struct rpmsg_tty_port *cport = tty->driver_data;
136 struct rpmsg_device *rpdev;
137 ssize_t msg_max_size, msg_size;
138 int ret;
139 u8 *tmpbuf;
140
141 /* If cts not set, the message is not sent*/
142 if (!cport->cts)
143 return 0;
144
145 rpdev = cport->rpdev;
146
147 dev_dbg(&rpdev->dev, "%s: send msg from tty->index = %d, len = %d\n",
148 __func__, tty->index, len);
149
150 msg_max_size = rpmsg_get_mtu(rpdev->ept);
151
> 152 msg_size = min(len, msg_max_size);
153 tmpbuf = kzalloc(msg_size, GFP_KERNEL);
154 if (!tmpbuf)
155 return -ENOMEM;
156
157 memcpy(tmpbuf, buf, msg_size);
158
159 /*
160 * Try to send the message to remote processor, if failed return 0 as
161 * no data sent
162 */
163 ret = rpmsg_trysendto(cport->d_ept, tmpbuf, msg_size, cport->data_dst);
164 kfree(tmpbuf);
165 if (ret) {
166 dev_dbg(&rpdev->dev, "rpmsg_send failed: %d\n", ret);
167 return 0;
168 }
169
170 return msg_size;
171 }
172
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Arnaud,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc6 next-20190829]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Arnaud-Pouliquen/rpmsg-core-add-API-to-get-message-length/20190829-222443
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
include/linux/sched.h:609:43: sparse: sparse: bad integer constant expression
include/linux/sched.h:609:73: sparse: sparse: invalid named zero-width bitfield `value'
include/linux/sched.h:610:43: sparse: sparse: bad integer constant expression
include/linux/sched.h:610:67: sparse: sparse: invalid named zero-width bitfield `bucket_id'
>> drivers/tty/rpmsg_tty.c:152:20: sparse: sparse: incompatible types in comparison expression (different type sizes):
>> drivers/tty/rpmsg_tty.c:152:20: sparse: int *
>> drivers/tty/rpmsg_tty.c:152:20: sparse: long *
vim +152 drivers/tty/rpmsg_tty.c
132
133 static int rpmsg_tty_write(struct tty_struct *tty, const u8 *buf, int len)
134 {
135 struct rpmsg_tty_port *cport = tty->driver_data;
136 struct rpmsg_device *rpdev;
137 ssize_t msg_max_size, msg_size;
138 int ret;
139 u8 *tmpbuf;
140
141 /* If cts not set, the message is not sent*/
142 if (!cport->cts)
143 return 0;
144
145 rpdev = cport->rpdev;
146
147 dev_dbg(&rpdev->dev, "%s: send msg from tty->index = %d, len = %d\n",
148 __func__, tty->index, len);
149
150 msg_max_size = rpmsg_get_mtu(rpdev->ept);
151
> 152 msg_size = min(len, msg_max_size);
153 tmpbuf = kzalloc(msg_size, GFP_KERNEL);
154 if (!tmpbuf)
155 return -ENOMEM;
156
157 memcpy(tmpbuf, buf, msg_size);
158
159 /*
160 * Try to send the message to remote processor, if failed return 0 as
161 * no data sent
162 */
163 ret = rpmsg_trysendto(cport->d_ept, tmpbuf, msg_size, cport->data_dst);
164 kfree(tmpbuf);
165 if (ret) {
166 dev_dbg(&rpdev->dev, "rpmsg_send failed: %d\n", ret);
167 return 0;
168 }
169
170 return msg_size;
171 }
172
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
hi Suman
On 8/29/19 12:34 AM, Suman Anna wrote:
> Hi Arnaud,
>
> On 8/28/19 10:19 AM, Arnaud Pouliquen wrote:
>> Return the rpmsg buffer size for sending message, so rpmsg users
>> can split a long message in several sub rpmsg buffers.
>
> Thanks for the patch, I also have a need for the same to be able to
> compute permissible payload size. Minor comments below.
Thanks for your review. i will update it ASAP. Then if you need it and
ack it, i suppose that we could request Bjorn to integrate it in a first
step, if the rpmsg tty driver has not a level of quality sufficient to
be accepted...
Regards
Arnaud
>
>>
>> Signed-off-by: Arnaud Pouliquen <[email protected]>
>> ---
>> V4 to V5 :
>> - rename rpmsg_get_buf_payload_size to rpmsg_get_mtu
>>
>> drivers/rpmsg/rpmsg_core.c | 21 +++++++++++++++++++++
>> drivers/rpmsg/rpmsg_internal.h | 2 ++
>> drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++++
>> include/linux/rpmsg.h | 10 ++++++++++
>> 4 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
>> index 8122807db380..daca2e24fc71 100644
>> --- a/drivers/rpmsg/rpmsg_core.c
>> +++ b/drivers/rpmsg/rpmsg_core.c
>> @@ -283,6 +283,27 @@ int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
>> }
>> EXPORT_SYMBOL(rpmsg_trysend_offchannel);
>>
>> +/**
>> + * rpmsg_get_mtu() - get maximum transmission buffer size for sending message.
>> + * @ept: the rpmsg endpoint
>> + *
>> + * This function returns maximum buffer size available for a single message.
>> + *
>> + * Return: the maximum transmission size on success and an appropriate error
>> + * value on failure.
>> + */
>> +
>> +ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
>> +{
>> + if (WARN_ON(!ept))
>> + return -EINVAL;
>> + if (!ept->ops->get_buf_mtu)
>
> How about calling the ops just get_mtu or rename the function to follow
> the ops name, like all the others.
>
>> + return -ENXIO;
>
> Perhaps ENOTSUPP or EOPNOTSUPP.
>
>> +
>> + return ept->ops->get_buf_mtu(ept);
>> +}
>> +EXPORT_SYMBOL(rpmsg_get_mtu);
>> +
>> /*
>> * match an rpmsg channel with a channel info struct.
>> * this is used to make sure we're not creating rpmsg devices for channels
>> diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
>> index 0d791c30b7ea..645c402569ac 100644
>> --- a/drivers/rpmsg/rpmsg_internal.h
>> +++ b/drivers/rpmsg/rpmsg_internal.h
>> @@ -46,6 +46,7 @@ struct rpmsg_device_ops {
>> * @trysend: see @rpmsg_trysend(), required
>> * @trysendto: see @rpmsg_trysendto(), optional
>> * @trysend_offchannel: see @rpmsg_trysend_offchannel(), optional
>> + * @get_buf_payload_size: see @rpmsg_get_buf_payload_size(), optional
>
> Missed updating the kerneldoc to the new name.
>
>> *
>> * Indirection table for the operations that a rpmsg backend should implement.
>> * In addition to @destroy_ept, the backend must at least implement @send and
>> @@ -65,6 +66,7 @@ struct rpmsg_endpoint_ops {
>> void *data, int len);
>> __poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp,
>> poll_table *wait);
>> + ssize_t (*get_buf_mtu)(struct rpmsg_endpoint *ept);
>> };
>>
>> int rpmsg_register_device(struct rpmsg_device *rpdev);
>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
>> index e757f0038a1c..f80b1ad23e7e 100644
>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
>> @@ -178,6 +178,7 @@ static int virtio_rpmsg_trysendto(struct rpmsg_endpoint *ept, void *data,
>> int len, u32 dst);
>> static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
>> u32 dst, void *data, int len);
>> +static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept);
>
> Minor nit, virtio_rpmsg_ prefix similar to all the other ops.
>
> regards
> Suman
>
>>
>> static const struct rpmsg_endpoint_ops virtio_endpoint_ops = {
>> .destroy_ept = virtio_rpmsg_destroy_ept,
>> @@ -187,6 +188,7 @@ static const struct rpmsg_endpoint_ops virtio_endpoint_ops = {
>> .trysend = virtio_rpmsg_trysend,
>> .trysendto = virtio_rpmsg_trysendto,
>> .trysend_offchannel = virtio_rpmsg_trysend_offchannel,
>> + .get_buf_mtu = virtio_get_buf_mtu,
>> };
>>
>> /**
>> @@ -702,6 +704,14 @@ static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
>> return rpmsg_send_offchannel_raw(rpdev, src, dst, data, len, false);
>> }
>>
>> +static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept)
>> +{
>> + struct rpmsg_device *rpdev = ept->rpdev;
>> + struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>> +
>> + return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
>> +}
>> +
>> static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
>> struct rpmsg_hdr *msg, unsigned int len)
>> {
>> diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
>> index 9fe156d1c018..9d638bf2bdce 100644
>> --- a/include/linux/rpmsg.h
>> +++ b/include/linux/rpmsg.h
>> @@ -135,6 +135,8 @@ int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
>> __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
>> poll_table *wait);
>>
>> +ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept);
>> +
>> #else
>>
>> static inline int register_rpmsg_device(struct rpmsg_device *dev)
>> @@ -242,6 +244,14 @@ static inline __poll_t rpmsg_poll(struct rpmsg_endpoint *ept,
>> return 0;
>> }
>>
>> +static ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
>> +{
>> + /* This shouldn't be possible */
>> + WARN_ON(1);
>> +
>> + return -ENXIO;
>> +}
>> +
>> #endif /* IS_ENABLED(CONFIG_RPMSG) */
>>
>> /* use a macro to avoid include chaining to get THIS_MODULE */
>>
>
Hi Arnaud,
On 9/3/19 4:49 AM, Arnaud Pouliquen wrote:
> hi Suman
>
> On 8/29/19 12:34 AM, Suman Anna wrote:
>> Hi Arnaud,
>>
>> On 8/28/19 10:19 AM, Arnaud Pouliquen wrote:
>>> Return the rpmsg buffer size for sending message, so rpmsg users
>>> can split a long message in several sub rpmsg buffers.
>>
>> Thanks for the patch, I also have a need for the same to be able to
>> compute permissible payload size. Minor comments below.
>
> Thanks for your review. i will update it ASAP. Then if you need it and
> ack it, i suppose that we could request Bjorn to integrate it in a first
> step, if the rpmsg tty driver has not a level of quality sufficient to
> be accepted...
Yeah, this patch can always be merged independently ahead of the rpmsg
tty driver. Anyways, the tty patch will have to be picked up by a
separate maintainer right. So, it would be nice to get the revised
version get into 5.4
regards
Suman
>
> Regards
> Arnaud
>>
>>>
>>> Signed-off-by: Arnaud Pouliquen <[email protected]>
>>> ---
>>> V4 to V5 :
>>> - rename rpmsg_get_buf_payload_size to rpmsg_get_mtu
>>>
>>> drivers/rpmsg/rpmsg_core.c | 21 +++++++++++++++++++++
>>> drivers/rpmsg/rpmsg_internal.h | 2 ++
>>> drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++++
>>> include/linux/rpmsg.h | 10 ++++++++++
>>> 4 files changed, 43 insertions(+)
>>>
>>> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
>>> index 8122807db380..daca2e24fc71 100644
>>> --- a/drivers/rpmsg/rpmsg_core.c
>>> +++ b/drivers/rpmsg/rpmsg_core.c
>>> @@ -283,6 +283,27 @@ int rpmsg_trysend_offchannel(struct
>>> rpmsg_endpoint *ept, u32 src, u32 dst,
>>> }
>>> EXPORT_SYMBOL(rpmsg_trysend_offchannel);
>>> +/**
>>> + * rpmsg_get_mtu() - get maximum transmission buffer size for
>>> sending message.
>>> + * @ept: the rpmsg endpoint
>>> + *
>>> + * This function returns maximum buffer size available for a single
>>> message.
>>> + *
>>> + * Return: the maximum transmission size on success and an
>>> appropriate error
>>> + * value on failure.
>>> + */
>>> +
>>> +ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
>>> +{
>>> + if (WARN_ON(!ept))
>>> + return -EINVAL;
>>> + if (!ept->ops->get_buf_mtu)
>>
>> How about calling the ops just get_mtu or rename the function to follow
>> the ops name, like all the others.
>>
>>> + return -ENXIO;
>>
>> Perhaps ENOTSUPP or EOPNOTSUPP.
>>
>>> +
>>> + return ept->ops->get_buf_mtu(ept);
>>> +}
>>> +EXPORT_SYMBOL(rpmsg_get_mtu);
>>> +
>>> /*
>>> * match an rpmsg channel with a channel info struct.
>>> * this is used to make sure we're not creating rpmsg devices for
>>> channels
>>> diff --git a/drivers/rpmsg/rpmsg_internal.h
>>> b/drivers/rpmsg/rpmsg_internal.h
>>> index 0d791c30b7ea..645c402569ac 100644
>>> --- a/drivers/rpmsg/rpmsg_internal.h
>>> +++ b/drivers/rpmsg/rpmsg_internal.h
>>> @@ -46,6 +46,7 @@ struct rpmsg_device_ops {
>>> * @trysend: see @rpmsg_trysend(), required
>>> * @trysendto: see @rpmsg_trysendto(), optional
>>> * @trysend_offchannel: see @rpmsg_trysend_offchannel(), optional
>>> + * @get_buf_payload_size: see @rpmsg_get_buf_payload_size(), optional
>>
>> Missed updating the kerneldoc to the new name.
>>
>>> *
>>> * Indirection table for the operations that a rpmsg backend should
>>> implement.
>>> * In addition to @destroy_ept, the backend must at least implement
>>> @send and
>>> @@ -65,6 +66,7 @@ struct rpmsg_endpoint_ops {
>>> void *data, int len);
>>> __poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp,
>>> poll_table *wait);
>>> + ssize_t (*get_buf_mtu)(struct rpmsg_endpoint *ept);
>>> };
>>> int rpmsg_register_device(struct rpmsg_device *rpdev);
>>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c
>>> b/drivers/rpmsg/virtio_rpmsg_bus.c
>>> index e757f0038a1c..f80b1ad23e7e 100644
>>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
>>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
>>> @@ -178,6 +178,7 @@ static int virtio_rpmsg_trysendto(struct
>>> rpmsg_endpoint *ept, void *data,
>>> int len, u32 dst);
>>> static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint
>>> *ept, u32 src,
>>> u32 dst, void *data, int len);
>>> +static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept);
>>
>> Minor nit, virtio_rpmsg_ prefix similar to all the other ops.
>>
>> regards
>> Suman
>>
>>> static const struct rpmsg_endpoint_ops virtio_endpoint_ops = {
>>> .destroy_ept = virtio_rpmsg_destroy_ept,
>>> @@ -187,6 +188,7 @@ static const struct rpmsg_endpoint_ops
>>> virtio_endpoint_ops = {
>>> .trysend = virtio_rpmsg_trysend,
>>> .trysendto = virtio_rpmsg_trysendto,
>>> .trysend_offchannel = virtio_rpmsg_trysend_offchannel,
>>> + .get_buf_mtu = virtio_get_buf_mtu,
>>> };
>>> /**
>>> @@ -702,6 +704,14 @@ static int
>>> virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
>>> return rpmsg_send_offchannel_raw(rpdev, src, dst, data, len,
>>> false);
>>> }
>>> +static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept)
>>> +{
>>> + struct rpmsg_device *rpdev = ept->rpdev;
>>> + struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>>> +
>>> + return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
>>> +}
>>> +
>>> static int rpmsg_recv_single(struct virtproc_info *vrp, struct
>>> device *dev,
>>> struct rpmsg_hdr *msg, unsigned int len)
>>> {
>>> diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
>>> index 9fe156d1c018..9d638bf2bdce 100644
>>> --- a/include/linux/rpmsg.h
>>> +++ b/include/linux/rpmsg.h
>>> @@ -135,6 +135,8 @@ int rpmsg_trysend_offchannel(struct
>>> rpmsg_endpoint *ept, u32 src, u32 dst,
>>> __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
>>> poll_table *wait);
>>> +ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept);
>>> +
>>> #else
>>> static inline int register_rpmsg_device(struct rpmsg_device *dev)
>>> @@ -242,6 +244,14 @@ static inline __poll_t rpmsg_poll(struct
>>> rpmsg_endpoint *ept,
>>> return 0;
>>> }
>>> +static ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
>>> +{
>>> + /* This shouldn't be possible */
>>> + WARN_ON(1);
>>> +
>>> + return -ENXIO;
>>> +}
>>> +
>>> #endif /* IS_ENABLED(CONFIG_RPMSG) */
>>> /* use a macro to avoid include chaining to get THIS_MODULE */
>>>
>>
Hi Suman
On 9/3/19 6:06 PM, Suman Anna wrote:
> Hi Arnaud,
>
> On 9/3/19 4:49 AM, Arnaud Pouliquen wrote:
>> hi Suman
>>
>> On 8/29/19 12:34 AM, Suman Anna wrote:
>>> Hi Arnaud,
>>>
>>> On 8/28/19 10:19 AM, Arnaud Pouliquen wrote:
>>>> Return the rpmsg buffer size for sending message, so rpmsg users
>>>> can split a long message in several sub rpmsg buffers.
>>>
>>> Thanks for the patch, I also have a need for the same to be able to
>>> compute permissible payload size. Minor comments below.
>>
>> Thanks for your review. i will update it ASAP. Then if you need it and
>> ack it, i suppose that we could request Bjorn to integrate it in a first
>> step, if the rpmsg tty driver has not a level of quality sufficient to
>> be accepted...
>
> Yeah, this patch can always be merged independently ahead of the rpmsg
> tty driver. Anyways, the tty patch will have to be picked up by a
> separate maintainer right. So, it would be nice to get the revised
> version get into 5.4
Sure, I plan to send a new version of the series today.
I would prefer not to split the series, just to simplify the review and
the tests. if this patch is cherry-picked and integrated independently
by Bjorn, I will simply sent a new version of the rpmsg tty driver
without it.
Regards
Arnaud
>
> regards
> Suman
>
>>
>> Regards
>> Arnaud
>>>
>>>>
>>>> Signed-off-by: Arnaud Pouliquen <[email protected]>
>>>> ---
>>>> V4 to V5 :
>>>> - rename rpmsg_get_buf_payload_size to rpmsg_get_mtu
>>>>
>>>> drivers/rpmsg/rpmsg_core.c | 21 +++++++++++++++++++++
>>>> drivers/rpmsg/rpmsg_internal.h | 2 ++
>>>> drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++++
>>>> include/linux/rpmsg.h | 10 ++++++++++
>>>> 4 files changed, 43 insertions(+)
>>>>
>>>> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
>>>> index 8122807db380..daca2e24fc71 100644
>>>> --- a/drivers/rpmsg/rpmsg_core.c
>>>> +++ b/drivers/rpmsg/rpmsg_core.c
>>>> @@ -283,6 +283,27 @@ int rpmsg_trysend_offchannel(struct
>>>> rpmsg_endpoint *ept, u32 src, u32 dst,
>>>> }
>>>> EXPORT_SYMBOL(rpmsg_trysend_offchannel);
>>>> +/**
>>>> + * rpmsg_get_mtu() - get maximum transmission buffer size for
>>>> sending message.
>>>> + * @ept: the rpmsg endpoint
>>>> + *
>>>> + * This function returns maximum buffer size available for a single
>>>> message.
>>>> + *
>>>> + * Return: the maximum transmission size on success and an
>>>> appropriate error
>>>> + * value on failure.
>>>> + */
>>>> +
>>>> +ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
>>>> +{
>>>> + if (WARN_ON(!ept))
>>>> + return -EINVAL;
>>>> + if (!ept->ops->get_buf_mtu)
>>>
>>> How about calling the ops just get_mtu or rename the function to follow
>>> the ops name, like all the others.
>>>
>>>> + return -ENXIO;
>>>
>>> Perhaps ENOTSUPP or EOPNOTSUPP.
>>>
>>>> +
>>>> + return ept->ops->get_buf_mtu(ept);
>>>> +}
>>>> +EXPORT_SYMBOL(rpmsg_get_mtu);
>>>> +
>>>> /*
>>>> * match an rpmsg channel with a channel info struct.
>>>> * this is used to make sure we're not creating rpmsg devices for
>>>> channels
>>>> diff --git a/drivers/rpmsg/rpmsg_internal.h
>>>> b/drivers/rpmsg/rpmsg_internal.h
>>>> index 0d791c30b7ea..645c402569ac 100644
>>>> --- a/drivers/rpmsg/rpmsg_internal.h
>>>> +++ b/drivers/rpmsg/rpmsg_internal.h
>>>> @@ -46,6 +46,7 @@ struct rpmsg_device_ops {
>>>> * @trysend: see @rpmsg_trysend(), required
>>>> * @trysendto: see @rpmsg_trysendto(), optional
>>>> * @trysend_offchannel: see @rpmsg_trysend_offchannel(), optional
>>>> + * @get_buf_payload_size: see @rpmsg_get_buf_payload_size(), optional
>>>
>>> Missed updating the kerneldoc to the new name.
>>>
>>>> *
>>>> * Indirection table for the operations that a rpmsg backend should
>>>> implement.
>>>> * In addition to @destroy_ept, the backend must at least implement
>>>> @send and
>>>> @@ -65,6 +66,7 @@ struct rpmsg_endpoint_ops {
>>>> void *data, int len);
>>>> __poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp,
>>>> poll_table *wait);
>>>> + ssize_t (*get_buf_mtu)(struct rpmsg_endpoint *ept);
>>>> };
>>>> int rpmsg_register_device(struct rpmsg_device *rpdev);
>>>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c
>>>> b/drivers/rpmsg/virtio_rpmsg_bus.c
>>>> index e757f0038a1c..f80b1ad23e7e 100644
>>>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
>>>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
>>>> @@ -178,6 +178,7 @@ static int virtio_rpmsg_trysendto(struct
>>>> rpmsg_endpoint *ept, void *data,
>>>> int len, u32 dst);
>>>> static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint
>>>> *ept, u32 src,
>>>> u32 dst, void *data, int len);
>>>> +static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept);
>>>
>>> Minor nit, virtio_rpmsg_ prefix similar to all the other ops.
>>>
>>> regards
>>> Suman
>>>
>>>> static const struct rpmsg_endpoint_ops virtio_endpoint_ops = {
>>>> .destroy_ept = virtio_rpmsg_destroy_ept,
>>>> @@ -187,6 +188,7 @@ static const struct rpmsg_endpoint_ops
>>>> virtio_endpoint_ops = {
>>>> .trysend = virtio_rpmsg_trysend,
>>>> .trysendto = virtio_rpmsg_trysendto,
>>>> .trysend_offchannel = virtio_rpmsg_trysend_offchannel,
>>>> + .get_buf_mtu = virtio_get_buf_mtu,
>>>> };
>>>> /**
>>>> @@ -702,6 +704,14 @@ static int
>>>> virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
>>>> return rpmsg_send_offchannel_raw(rpdev, src, dst, data, len,
>>>> false);
>>>> }
>>>> +static ssize_t virtio_get_buf_mtu(struct rpmsg_endpoint *ept)
>>>> +{
>>>> + struct rpmsg_device *rpdev = ept->rpdev;
>>>> + struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>>>> +
>>>> + return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
>>>> +}
>>>> +
>>>> static int rpmsg_recv_single(struct virtproc_info *vrp, struct
>>>> device *dev,
>>>> struct rpmsg_hdr *msg, unsigned int len)
>>>> {
>>>> diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
>>>> index 9fe156d1c018..9d638bf2bdce 100644
>>>> --- a/include/linux/rpmsg.h
>>>> +++ b/include/linux/rpmsg.h
>>>> @@ -135,6 +135,8 @@ int rpmsg_trysend_offchannel(struct
>>>> rpmsg_endpoint *ept, u32 src, u32 dst,
>>>> __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
>>>> poll_table *wait);
>>>> +ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept);
>>>> +
>>>> #else
>>>> static inline int register_rpmsg_device(struct rpmsg_device *dev)
>>>> @@ -242,6 +244,14 @@ static inline __poll_t rpmsg_poll(struct
>>>> rpmsg_endpoint *ept,
>>>> return 0;
>>>> }
>>>> +static ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept)
>>>> +{
>>>> + /* This shouldn't be possible */
>>>> + WARN_ON(1);
>>>> +
>>>> + return -ENXIO;
>>>> +}
>>>> +
>>>> #endif /* IS_ENABLED(CONFIG_RPMSG) */
>>>> /* use a macro to avoid include chaining to get THIS_MODULE */
>>>>
>>>
>