[PATCH v5 0/2] TTY: add rpmsg tty driver

[PATCH v5 0/2] TTY: add rpmsg tty driver

[Arnaud Pouliquen]

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

[PATCH v5 0/2] TTY: add rpmsg tty driver

[Arnaud Pouliquen]

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

[PATCH v5 1/2] rpmsg: core: add API to get message length

[Arnaud Pouliquen]

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 <arnaud.pouliquen@st.com>
---
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

[PATCH v5 1/2] rpmsg: core: add API to get message length

[Arnaud Pouliquen]

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 <arnaud.pouliquen@st.com>
---
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

[PATCH v5 2/2] tty: add rpmsg driver

[Arnaud Pouliquen]

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 <arnaud.pouliquen@st.com>
---
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 <arnaud.pouliquen@st.com> 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 <arnaud.pouliquen@st.com>");
+MODULE_DESCRIPTION("remote processor messaging tty driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH v5 2/2] tty: add rpmsg driver

[Arnaud Pouliquen]

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 <arnaud.pouliquen@st.com>
---
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 <arnaud.pouliquen@st.com> 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 <arnaud.pouliquen@st.com>");
+MODULE_DESCRIPTION("remote processor messaging tty driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH v5 1/2] rpmsg: core: add API to get message length

[Suman Anna]

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 <arnaud.pouliquen@st.com>
> ---
> 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 */
> 

Re: [PATCH v5 1/2] rpmsg: core: add API to get message length

[Suman Anna]

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 <arnaud.pouliquen@st.com>
> ---
> 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 */
> 

Re: [PATCH v5 2/2] tty: add rpmsg driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3226 bytes --]

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 <lkp@intel.com>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58672 bytes --]

Re: [PATCH v5 2/2] tty: add rpmsg driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3226 bytes --]

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 <lkp@intel.com>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58672 bytes --]

Re: [PATCH v5 2/2] tty: add rpmsg driver

[kbuild test robot]

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 <lkp@intel.com>


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

Re: [PATCH v5 2/2] tty: add rpmsg driver

[kbuild test robot]

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 <lkp@intel.com>


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

Re: [PATCH v5 1/2] rpmsg: core: add API to get message length

[Arnaud Pouliquen]

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 <arnaud.pouliquen@st.com>
>> ---
>> 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 */
>>
> 

Re: [PATCH v5 1/2] rpmsg: core: add API to get message length

[Arnaud Pouliquen]

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 <arnaud.pouliquen@st.com>
>> ---
>> 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 */
>>
> 

Re: [PATCH v5 1/2] rpmsg: core: add API to get message length

[Suman Anna]

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 <arnaud.pouliquen@st.com>
>>> ---
>>> 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 */
>>>
>>

Re: [PATCH v5 1/2] rpmsg: core: add API to get message length

[Suman Anna]

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 <arnaud.pouliquen@st.com>
>>> ---
>>> 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 */
>>>
>>

Re: [PATCH v5 1/2] rpmsg: core: add API to get message length

[Arnaud Pouliquen]

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 <arnaud.pouliquen@st.com>
>>>> ---
>>>> 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 */
>>>>
>>>
> 

Re: [PATCH v5 1/2] rpmsg: core: add API to get message length

[Arnaud Pouliquen]

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 <arnaud.pouliquen@st.com>
>>>> ---
>>>> 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 */
>>>>
>>>
>