Skip to contentSkip to conversation

Add TIOCM Signals support for RPMSG char devices

PATCH Superseded Arun Kumar Neelakantam 2 participants 10 messages Oct 3, 2018 – Oct 4, 2018

Subsystems (2)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

cover Add TIOCM Signals support for RPMSG char devices

Glink transport support signals to exchange state notification between local and remote side clients. Adding support to send/receive the signal command and notify the clients through callback and POLL notification.

Arun Kumar Neelakantam (5):
rpmsg: glink: Add GLINK signal support for RPMSG
rpmsg: Add signal callback to rpmsg char device
rpmsg: Add TIOCMGET/TIOCMSET ioctl support
rpmsg: wakeup poll to notify signal update
rpmsg: glink: Convert the native signals to TIOCM

 drivers/rpmsg/qcom_glink_native.c | 109 ++++++++++++++++++++++++++++++++++++++
 drivers/rpmsg/rpmsg_char.c        |  76 ++++++++++++++++++++++++--
 drivers/rpmsg/rpmsg_core.c        |  41 ++++++++++++++
 drivers/rpmsg/rpmsg_internal.h    |   5 ++
 include/linux/rpmsg.h             |  25 +++++++++
 5 files changed, 253 insertions(+), 3 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw

PATCH 1/5 rpmsg: glink: Add GLINK signal support for RPMSG

Add support to handle SMD signals to RPMSG over GLINK. SMD signals mimic serial protocol signals to notify of ports opening and closing. This change affects the rpmsg core, rpmsg char and glink drivers.

---
 drivers/rpmsg/qcom_glink_native.c | 80 +++++++++++++++++++++++++++++++++++++++
 drivers/rpmsg/rpmsg_core.c        | 41 ++++++++++++++++++++
 drivers/rpmsg/rpmsg_internal.h    |  5 +++
 include/linux/rpmsg.h             | 25 ++++++++++++
 4 files changed, 151 insertions(+)
drivers/rpmsg/qcom_glink_native.c +80 -0
index f6988fd..ff8751b 100644
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (c) 2016-2017, Linaro Ltd
+ * Copyright (c) 2018, The Linux Foundation.
  */
 
 #include <linux/idr.h>
@@ -150,6 +151,8 @@ enum {
  * @intent_req_lock: Synchronises multiple intent requests
  * @intent_req_result: Result of intent request
  * @intent_req_comp: Completion for intent_req signalling
+ * @lsigs:	Local side signals
+ * @rsigs:	Remote side signals
  */
 struct glink_channel {
 	struct rpmsg_endpoint ept;
@@ -181,6 +184,9 @@ struct glink_channel {
 	struct mutex intent_req_lock;
 	bool intent_req_result;
 	struct completion intent_req_comp;
+
+	unsigned int lsigs;
+	unsigned int rsigs;
 };
 
 #define to_glink_channel(_ept) container_of(_ept, struct glink_channel, ept)
@@ -201,6 +207,7 @@ struct glink_channel {
 #define RPM_CMD_TX_DATA_CONT		12
 #define RPM_CMD_READ_NOTIF		13
 #define RPM_CMD_RX_DONE_W_REUSE		14
+#define RPM_CMD_SIGNALS			15
 
 #define GLINK_FEATURE_INTENTLESS	BIT(1)
 
@@ -954,6 +961,52 @@ static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid)
 	return 0;
 }
 
+/**
+ * qcom_glink_send_signals() - convert a signal  cmd to wire format and transmit
+ * @glink:	The transport to transmit on.
+ * @channel:	The glink channel
+ * @sigs:	The signals to encode.
+ *
+ * Return: 0 on success or standard Linux error code.
+ */
+static int qcom_glink_send_signals(struct qcom_glink *glink,
+				   struct glink_channel *channel,
+				   u32 sigs)
+{
+	struct glink_msg msg;
+
+	msg.cmd = cpu_to_le16(RPM_CMD_SIGNALS);
+	msg.param1 = cpu_to_le16(channel->lcid);
+	msg.param2 = cpu_to_le32(sigs);
+
+	return qcom_glink_tx(glink, &msg, sizeof(msg), NULL, 0, true);
+}
+
+static int qcom_glink_handle_signals(struct qcom_glink *glink,
+				     unsigned int rcid, unsigned int signals)
+{
+	struct glink_channel *channel;
+	unsigned long flags;
+	u32 old;
+
+	spin_lock_irqsave(&glink->idr_lock, flags);
+	channel = idr_find(&glink->rcids, rcid);
+	spin_unlock_irqrestore(&glink->idr_lock, flags);
+	if (!channel) {
+		dev_err(glink->dev, "signal for non-existing channel\n");
+		return -EINVAL;
+	}
+
+	old = channel->rsigs;
+	channel->rsigs = signals;
+
+	if (channel->ept.sig_cb)
+		channel->ept.sig_cb(channel->ept.rpdev, channel->ept.priv,
+				    old, channel->rsigs);
+
+	return 0;
+}
+
 static irqreturn_t qcom_glink_native_intr(int irq, void *data)
 {
 	struct qcom_glink *glink = data;
@@ -1015,6 +1068,10 @@ static irqreturn_t qcom_glink_native_intr(int irq, void *data)
 			qcom_glink_handle_intent_req_ack(glink, param1, param2);
 			qcom_glink_rx_advance(glink, ALIGN(sizeof(msg), 8));
 			break;
+		case RPM_CMD_SIGNALS:
+			qcom_glink_handle_signals(glink, param1, param2);
+			qcom_glink_rx_advance(glink, ALIGN(sizeof(msg), 8));
+			break;
 		default:
 			dev_err(glink->dev, "unhandled rx cmd: %d\n", cmd);
 			ret = -EINVAL;
@@ -1312,6 +1369,27 @@ static int qcom_glink_trysend(struct rpmsg_endpoint *ept, void *data, int len)
 	return __qcom_glink_send(channel, data, len, false);
 }
 
+static int qcom_glink_get_sigs(struct rpmsg_endpoint *ept,
+			       u32 *lsigs, u32 *rsigs)
+{
+	struct glink_channel *channel = to_glink_channel(ept);
+
+	*lsigs = channel->lsigs;
+	*rsigs = channel->rsigs;
+
+	return 0;
+}
+
+static int qcom_glink_set_sigs(struct rpmsg_endpoint *ept, u32 sigs)
+{
+	struct glink_channel *channel = to_glink_channel(ept);
+	struct qcom_glink *glink = channel->glink;
+
+	channel->lsigs = sigs;
+
+	return qcom_glink_send_signals(glink, channel, sigs);
+}
+
 /*
  * Finds the device_node for the glink child interested in this channel.
  */
@@ -1345,6 +1423,8 @@ static struct device_node *qcom_glink_match_channel(struct device_node *node,
 	.destroy_ept = qcom_glink_destroy_ept,
 	.send = qcom_glink_send,
 	.trysend = qcom_glink_trysend,
+	.get_sigs = qcom_glink_get_sigs,
+	.set_sigs = qcom_glink_set_sigs,
 };
 
 static void qcom_glink_rpdev_release(struct device *dev)
drivers/rpmsg/rpmsg_core.c +41 -0

59 lines not on this page · raw message

drivers/rpmsg/rpmsg_internal.h +5 -0
index 0d791c3..034ac07 100644
@@ -4,6 +4,7 @@
  *
  * Copyright (C) 2011 Texas Instruments, Inc.
  * Copyright (C) 2011 Google, Inc.
+ * Copyright (c) 2018 The Linux Foundation.
  *
  * Ohad Ben-Cohen <[email protected]>
  * Brian Swetland <[email protected]>
@@ -46,6 +47,8 @@ struct rpmsg_device_ops {
  * @trysend:		see @rpmsg_trysend(), required
  * @trysendto:		see @rpmsg_trysendto(), optional
  * @trysend_offchannel:	see @rpmsg_trysend_offchannel(), optional
+ * @get_sigs:		see @rpmsg_get_sigs(), optional
+ * @set_sigs:		see @rpmsg_set_sigs(), 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 +68,8 @@ struct rpmsg_endpoint_ops {
 			     void *data, int len);
 	__poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp,
 			     poll_table *wait);
+	int (*get_sigs)(struct rpmsg_endpoint *ept, u32 *lsigs, u32 *rsigs);
+	int (*set_sigs)(struct rpmsg_endpoint *ept, u32 sigs);
 };
 
 int rpmsg_register_device(struct rpmsg_device *rpdev);
include/linux/rpmsg.h +25 -0
index 9fe156d..580726d 100644
@@ -4,6 +4,7 @@
  *
  * Copyright (C) 2011 Texas Instruments, Inc.
  * Copyright (C) 2011 Google, Inc.
+ * Copyright (c) 2018 The Linux Foundation.
  * All rights reserved.
  */
 
@@ -60,6 +61,7 @@ struct rpmsg_device {
 };
 
 typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32);
+typedef int (*rpmsg_rx_sig_t)(struct rpmsg_device *, void *, u32, u32);
 
 /**
  * struct rpmsg_endpoint - binds a local rpmsg address to its user
@@ -67,6 +69,7 @@ struct rpmsg_device {
  * @refcount: when this drops to zero, the ept is deallocated
  * @cb: rx callback handler
  * @cb_lock: must be taken before accessing/changing @cb
+ * @sig_cb: rx serial signal handler
  * @addr: local rpmsg address
  * @priv: private data for the driver's use
  *
@@ -89,6 +92,7 @@ struct rpmsg_endpoint {
 	struct kref refcount;
 	rpmsg_rx_cb_t cb;
 	struct mutex cb_lock;
+	rpmsg_rx_sig_t sig_cb;
 	u32 addr;
 	void *priv;
 
@@ -102,6 +106,7 @@ struct rpmsg_endpoint {
  * @probe: invoked when a matching rpmsg channel (i.e. device) is found
  * @remove: invoked when the rpmsg channel is removed
  * @callback: invoked when an inbound message is received on the channel
+ * @signals: invoked when a serial signal change is received on the channel
  */
 struct rpmsg_driver {
 	struct device_driver drv;
@@ -109,6 +114,7 @@ struct rpmsg_driver {
 	int (*probe)(struct rpmsg_device *dev);
 	void (*remove)(struct rpmsg_device *dev);
 	int (*callback)(struct rpmsg_device *, void *, int, void *, u32);
+	int (*signals)(struct rpmsg_device *, void *, u32, u32);
 };
 
 #if IS_ENABLED(CONFIG_RPMSG)
@@ -135,6 +141,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);
 
+int rpmsg_get_sigs(struct rpmsg_endpoint *ept, u32 *lsigs, u32 *rsigs);
+int rpmsg_set_sigs(struct rpmsg_endpoint *ept, u32 sigs);
 #else
 
 static inline int register_rpmsg_device(struct rpmsg_device *dev)
@@ -242,6 +250,23 @@ static inline __poll_t rpmsg_poll(struct rpmsg_endpoint *ept,
 	return 0;
 }
 
+static inline int rpmsg_get_sigs(struct rpmsg_endpoint *ept, u32 *lsigs,
+				 u32 *rsigs)
+{
+	/* This shouldn't be possible */
+	WARN_ON(1);
+
+	return -ENXIO;
+}
+
+static inline int rpmsg_set_sigs(struct rpmsg_endpoint *ept, u32 sigs)
+{
+	/* 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 */
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#2 lore raw

Re: [PATCH 1/5] rpmsg: glink: Add GLINK signal support for RPMSG

Quoted 11 lines · Arun Kumar Neelakantam

Add support to handle SMD signals to RPMSG over GLINK. SMD signals mimic serial protocol signals to notify of ports opening and closing. This change affects the rpmsg core, rpmsg char and glink drivers.

---
 drivers/rpmsg/qcom_glink_native.c | 80 +++++++++++++++++++++++++++++++++++++++
 drivers/rpmsg/rpmsg_core.c        | 41 ++++++++++++++++++++
 drivers/rpmsg/rpmsg_internal.h    |  5 +++
 include/linux/rpmsg.h             | 25 ++++++++++++

Please split this patch in one for rpmsg and one for GLINK, and incorporate the translation from patch 5 in the latter.

Quoted 3 lines
 4 files changed, 151 insertions(+)
drivers/rpmsg/qcom_glink_native.c +0 -0

[..]

Quoted 6 lines
drivers/rpmsg/qcom_glink_native.c +2 -0
@@ -954,6 +961,52 @@ static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid)
 	return 0;
 }
 
+/**
+ * qcom_glink_send_signals() - convert a signal  cmd to wire format and transmit

drivers/rpmsg/qcom_glink_native.c static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid) · line 975

Quoted 10 lines
drivers/rpmsg/qcom_glink_native.c +10 -0
+ * @glink:	The transport to transmit on.
+ * @channel:	The glink channel
+ * @sigs:	The signals to encode.
+ *
+ * Return: 0 on success or standard Linux error code.
+ */
+static int qcom_glink_send_signals(struct qcom_glink *glink,
+				   struct glink_channel *channel,
+				   u32 sigs)
+{

As this deals with TIOCM_* flags the individual bits are defined as having "local" or "remote" status. So I believe you should be able to just pass around one set of bits and extract the ones that makes sense to send here.

Quoted 5 lines
drivers/rpmsg/qcom_glink_native.c +5 -0
+	struct glink_msg msg;
+
+	msg.cmd = cpu_to_le16(RPM_CMD_SIGNALS);
+	msg.param1 = cpu_to_le16(channel->lcid);
+	msg.param2 = cpu_to_le32(sigs);

drivers/rpmsg/qcom_glink_native.c static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid) · line 990

Quoted 10 lines
drivers/rpmsg/qcom_glink_native.c +10 -0
+
+	return qcom_glink_tx(glink, &msg, sizeof(msg), NULL, 0, true);
+}
+
+static int qcom_glink_handle_signals(struct qcom_glink *glink,
+				     unsigned int rcid, unsigned int signals)
+{
+	struct glink_channel *channel;
+	unsigned long flags;
+	u32 old;

As with send above you should be able to parse out the bits that makes sense for the remote to set and update the one set of status bits for the channel here.

Quoted 5 lines
drivers/rpmsg/qcom_glink_native.c +5 -0
+
+	spin_lock_irqsave(&glink->idr_lock, flags);
+	channel = idr_find(&glink->rcids, rcid);
+	spin_unlock_irqrestore(&glink->idr_lock, flags);
+	if (!channel) {

drivers/rpmsg/qcom_glink_native.c static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid) · line 1005

Quoted 10 lines
drivers/rpmsg/qcom_glink_native.c +10 -0
+		dev_err(glink->dev, "signal for non-existing channel\n");
+		return -EINVAL;
+	}
+
+	old = channel->rsigs;
+	channel->rsigs = signals;
+
+	if (channel->ept.sig_cb)
+		channel->ept.sig_cb(channel->ept.rpdev, channel->ept.priv,
+				    old, channel->rsigs);

Add {} when block is multiline.

Quoted 3 lines
drivers/rpmsg/qcom_glink_native.c +3 -0
+
+	return 0;
+}

[..]

Quoted 1 line
drivers/rpmsg/rpmsg_core.c +0 -0

[..]

Quoted 4 lines
drivers/rpmsg/rpmsg_core.c +4 -0
+/**
+ * rpmsg_get_sigs() - get the signals for this endpoint
+ * @ept: the rpmsg endpoint
+ * @sigs: serial signals bitmask

drivers/rpmsg/rpmsg_core.c int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst, · line 299

Quoted 10 lines
drivers/rpmsg/rpmsg_core.c +9 -0
+ *
+ * Returns 0 on success and an appropriate error value on failure.
+ */
+int rpmsg_get_sigs(struct rpmsg_endpoint *ept, u32 *lsigs, u32 *rsigs)
 
+{
+	if (WARN_ON(!ept))
+		return -EINVAL;
+	if (!ept->ops->get_sigs)
+		return -ENXIO;

I think EOPNOTSUPP would be better here.

Quoted 8 lines
drivers/rpmsg/rpmsg_core.c +8 -0
+
+	return ept->ops->get_sigs(ept, lsigs, rsigs);
+}
+EXPORT_SYMBOL(rpmsg_get_sigs);
+
+/**
+ * rpmsg_set_sigs() - set the remote signals for this endpoint
+ * @ept: the rpmsg endpoint

drivers/rpmsg/rpmsg_core.c int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst, · line 317

Quoted 10 lines
drivers/rpmsg/rpmsg_core.c +10 -0
+ * @sigs: serial signals bitmask
+ *
+ * Returns 0 on success and an appropriate error value on failure.
+ */
+int rpmsg_set_sigs(struct rpmsg_endpoint *ept, u32 sigs)
+{
+	if (WARN_ON(!ept))
+		return -EINVAL;
+	if (!ept->ops->set_sigs)
+		return -ENXIO;

Ditto

drivers/rpmsg/rpmsg_core.c int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst, · line 322

Quoted 5 lines
drivers/rpmsg/rpmsg_core.c +5 -0
+
+	return ept->ops->set_sigs(ept, sigs);
+}
+EXPORT_SYMBOL(rpmsg_set_sigs);
+

Regards,
Bjorn

#3 lore raw

PATCH 2/5 rpmsg: Add signal callback to rpmsg char device

In reply to Arun Kumar Neelakantam

Register a callback to get the signal notifications from rpmsg.

---
 drivers/rpmsg/rpmsg_char.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/rpmsg/rpmsg_char.c +8 -0
index a76b963..86003d5 100644
@@ -5,6 +5,7 @@
  * Copyright (c) 2012, PetaLogix
  * Copyright (c) 2011, Texas Instruments, Inc.
  * Copyright (c) 2011, Google, Inc.
+ * Copyright (c) 2018, The Linux Foundation.
  *
  * Based on rpmsg performance statistics driver by Michal Simek, which in turn
  * was based on TI & Google OMX rpmsg driver.
@@ -120,6 +121,12 @@ static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
 	return 0;
 }
 
+static int rpmsg_sigs_cb(struct rpmsg_device *rpdev, void *priv,
+			 u32 old, u32 new)
+{
+	return 0;
+}
+
 static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
 {
 	struct rpmsg_eptdev *eptdev = cdev_to_eptdev(inode->i_cdev);
@@ -136,6 +143,7 @@ static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
 		return -EINVAL;
 	}
 
+	ept->sig_cb = rpmsg_sigs_cb;
 	eptdev->ept = ept;
 	filp->private_data = eptdev;
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#4 lore raw

Re: [PATCH 2/5] rpmsg: Add signal callback to rpmsg char device

Quoted 3 lines · Arun Kumar Neelakantam

Register a callback to get the signal notifications from rpmsg.

Please incorporate this into patch 4.

Regards,
Bjorn

Quoted 41 lines
---
 drivers/rpmsg/rpmsg_char.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/rpmsg/rpmsg_char.c +8 -0
index a76b963..86003d5 100644
@@ -5,6 +5,7 @@
  * Copyright (c) 2012, PetaLogix
  * Copyright (c) 2011, Texas Instruments, Inc.
  * Copyright (c) 2011, Google, Inc.
+ * Copyright (c) 2018, The Linux Foundation.
  *
  * Based on rpmsg performance statistics driver by Michal Simek, which in turn
  * was based on TI & Google OMX rpmsg driver.
@@ -120,6 +121,12 @@ static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
 	return 0;
 }
 
+static int rpmsg_sigs_cb(struct rpmsg_device *rpdev, void *priv,
+			 u32 old, u32 new)
+{
+	return 0;
+}
+
 static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
 {
 	struct rpmsg_eptdev *eptdev = cdev_to_eptdev(inode->i_cdev);
@@ -136,6 +143,7 @@ static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
 		return -EINVAL;
 	}
 
+	ept->sig_cb = rpmsg_sigs_cb;
 	eptdev->ept = ept;
 	filp->private_data = eptdev;
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#5 lore raw

PATCH 3/5 rpmsg: Add TIOCMGET/TIOCMSET ioctl support

In reply to Arun Kumar Neelakantam

Add TICOMGET and TIOCMSET ioctl support for rpmsg char device nodes to get/set the low level transport signals.

---
 drivers/rpmsg/rpmsg_char.c | 54 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 3 deletions(-)
drivers/rpmsg/rpmsg_char.c +51 -3
index 86003d5..c983d6c 100644
@@ -20,6 +20,7 @@
 #include <linux/rpmsg.h>
 #include <linux/skbuff.h>
 #include <linux/slab.h>
+#include <linux/termios.h>
 #include <linux/uaccess.h>
 #include <uapi/linux/rpmsg.h>
 
@@ -274,15 +275,62 @@ static __poll_t rpmsg_eptdev_poll(struct file *filp, poll_table *wait)
 	return mask;
 }
 
+static int rpmsg_eptdev_tiocmset(struct file *fp, unsigned int cmd,
+				 unsigned long arg)
+{
+	struct rpmsg_eptdev *eptdev = fp->private_data;
+	u32 lsigs, rsigs, val;
+	int ret;
+
+	ret = get_user(val, (u32 *)arg);
+	if (ret)
+		return ret;
+
+	ret = rpmsg_get_sigs(eptdev->ept, &lsigs, &rsigs);
+	if (ret < 0)
+		return ret;
+
+	switch (cmd) {
+	case TIOCMBIS:
+		lsigs |= val;
+		break;
+	case TIOCMBIC:
+		lsigs &= ~val;
+		break;
+	case TIOCMSET:
+		lsigs = val;
+		break;
+	}
+
+	ret = rpmsg_set_sigs(eptdev->ept, lsigs);
+	return ret;
+}
+
 static long rpmsg_eptdev_ioctl(struct file *fp, unsigned int cmd,
 			       unsigned long arg)
 {
 	struct rpmsg_eptdev *eptdev = fp->private_data;
+	u32 lsigs, rsigs;
+	int ret;
 
-	if (cmd != RPMSG_DESTROY_EPT_IOCTL)
-		return -EINVAL;
+	switch (cmd) {
+	case TIOCMGET:
+		ret = rpmsg_get_sigs(eptdev->ept, &lsigs, &rsigs);
+		if (!ret)
+			ret = put_user(rsigs, (u32 *)arg);
+		break;
+	case TIOCMSET:
+	case TIOCMBIS:
+	case TIOCMBIC:
+		ret = rpmsg_eptdev_tiocmset(fp, cmd, arg);
+		break;
+	case RPMSG_DESTROY_EPT_IOCTL:
+		ret = rpmsg_eptdev_destroy(&eptdev->dev, NULL);
+	default:
+		ret = -EINVAL;
+	}
 
-	return rpmsg_eptdev_destroy(&eptdev->dev, NULL);
+	return ret;
 }
 
 static const struct file_operations rpmsg_eptdev_fops = {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#6 lore raw

Re: [PATCH 3/5] rpmsg: Add TIOCMGET/TIOCMSET ioctl support

Quoted 21 lines · Arun Kumar Neelakantam

Add TICOMGET and TIOCMSET ioctl support for rpmsg char device nodes to get/set the low level transport signals.

---
 drivers/rpmsg/rpmsg_char.c | 54 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 3 deletions(-)
drivers/rpmsg/rpmsg_char.c +1 -0
index 86003d5..c983d6c 100644
@@ -20,6 +20,7 @@
 #include <linux/rpmsg.h>
 #include <linux/skbuff.h>
 #include <linux/slab.h>
+#include <linux/termios.h>
 #include <linux/uaccess.h>
 #include <uapi/linux/rpmsg.h>
 

drivers/rpmsg/rpmsg_char.c static __poll_t rpmsg_eptdev_poll(struct file *filp, poll_table *wait) · line 279

Quoted 5 lines
drivers/rpmsg/rpmsg_char.c +2 -0
@@ -274,15 +275,62 @@ static __poll_t rpmsg_eptdev_poll(struct file *filp, poll_table *wait)
 	return mask;
 }
 
+static int rpmsg_eptdev_tiocmset(struct file *fp, unsigned int cmd,
+				 unsigned long arg)

Make arg type int __user *

Quoted 16 lines
drivers/rpmsg/rpmsg_char.c +16 -0
+{
+	struct rpmsg_eptdev *eptdev = fp->private_data;
+	u32 lsigs, rsigs, val;
+	int ret;
+
+	ret = get_user(val, (u32 *)arg);
+	if (ret)
+		return ret;
+
+	ret = rpmsg_get_sigs(eptdev->ept, &lsigs, &rsigs);
+	if (ret < 0)
+		return ret;
+
+	switch (cmd) {
+	case TIOCMBIS:
+		lsigs |= val;

drivers/rpmsg/rpmsg_char.c static __poll_t rpmsg_eptdev_poll(struct file *filp, poll_table *wait) · line 305

Quoted 10 lines
drivers/rpmsg/rpmsg_char.c +10 -0
+		break;
+	case TIOCMBIC:
+		lsigs &= ~val;
+		break;
+	case TIOCMSET:
+		lsigs = val;
+		break;
+	}
+
+	ret = rpmsg_set_sigs(eptdev->ept, lsigs);

This is racy, as the signals might change while we update them. Mimic tty_tiocmset() instead and make rpmsg_set_sigs() take a clear and set mask.

And please do spell out rpmsg_set_signal() and rpmsg_get_signal().

drivers/rpmsg/rpmsg_char.c static __poll_t rpmsg_eptdev_poll(struct file *filp, poll_table *wait) · line 306

Quoted 1 line
drivers/rpmsg/rpmsg_char.c +1 -0
+	return ret;

return rpmsg_set_signals();

Quoted 6 lines
drivers/rpmsg/rpmsg_char.c +2 -0
+}
+
 static long rpmsg_eptdev_ioctl(struct file *fp, unsigned int cmd,
 			       unsigned long arg)
 {
 	struct rpmsg_eptdev *eptdev = fp->private_data;

drivers/rpmsg/rpmsg_char.c static __poll_t rpmsg_eptdev_poll(struct file *filp, poll_table *wait) · line 320

Quoted 10 lines
drivers/rpmsg/rpmsg_char.c +7 -2
+	u32 lsigs, rsigs;
+	int ret;
 
-	if (cmd != RPMSG_DESTROY_EPT_IOCTL)
-		return -EINVAL;
+	switch (cmd) {
+	case TIOCMGET:
+		ret = rpmsg_get_sigs(eptdev->ept, &lsigs, &rsigs);
+		if (!ret)
+			ret = put_user(rsigs, (u32 *)arg);

Cast to int __user *

Quoted 5 lines
drivers/rpmsg/rpmsg_char.c +5 -0
+		break;
+	case TIOCMSET:
+	case TIOCMBIS:
+	case TIOCMBIC:
+		ret = rpmsg_eptdev_tiocmset(fp, cmd, arg);

drivers/rpmsg/rpmsg_char.c static __poll_t rpmsg_eptdev_poll(struct file *filp, poll_table *wait) · line 334

Quoted 10 lines
drivers/rpmsg/rpmsg_char.c +7 -1
+		break;
+	case RPMSG_DESTROY_EPT_IOCTL:
+		ret = rpmsg_eptdev_destroy(&eptdev->dev, NULL);
+	default:
+		ret = -EINVAL;
+	}
 
-	return rpmsg_eptdev_destroy(&eptdev->dev, NULL);
+	return ret;
 }

Regards,
Bjorn

#7 lore raw

PATCH 4/5 rpmsg: wakeup poll to notify signal update

In reply to Arun Kumar Neelakantam

Add support to wait on poll to get signal notifications. Send POLLPRI mask to indicate the signal change.

---
 drivers/rpmsg/rpmsg_char.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
drivers/rpmsg/rpmsg_char.c +14 -0
index c983d6c..aee5561 100644
@@ -64,6 +64,7 @@ struct rpmsg_ctrldev {
  * @queue_lock:	synchronization of @queue operations
  * @queue:	incoming message queue
  * @readq:	wait object for incoming queue
+ * @sig_pending:state of signal notification
  */
 struct rpmsg_eptdev {
 	struct device dev;
@@ -78,6 +79,8 @@ struct rpmsg_eptdev {
 	spinlock_t queue_lock;
 	struct sk_buff_head queue;
 	wait_queue_head_t readq;
+
+	atomic_t sig_pending;
 };
 
 static int rpmsg_eptdev_destroy(struct device *dev, void *data)
@@ -125,6 +128,12 @@ static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
 static int rpmsg_sigs_cb(struct rpmsg_device *rpdev, void *priv,
 			 u32 old, u32 new)
 {
+	struct rpmsg_eptdev *eptdev = priv;
+
+	atomic_set(&eptdev->sig_pending, 1);
+
+	/* wake up any blocking processes, waiting for signal notification */
+	wake_up_interruptible(&eptdev->readq);
 	return 0;
 }
 
@@ -164,6 +173,7 @@ static int rpmsg_eptdev_release(struct inode *inode, struct file *filp)
 		eptdev->ept = NULL;
 	}
 	mutex_unlock(&eptdev->ept_lock);
+	atomic_set(&eptdev->sig_pending, 0);
 
 	/* Discard all SKBs */
 	while (!skb_queue_empty(&eptdev->queue)) {
@@ -270,6 +280,9 @@ static __poll_t rpmsg_eptdev_poll(struct file *filp, poll_table *wait)
 	if (!skb_queue_empty(&eptdev->queue))
 		mask |= EPOLLIN | EPOLLRDNORM;
 
+	if (atomic_read(&eptdev->sig_pending))
+		mask |= POLLPRI;
+
 	mask |= rpmsg_poll(eptdev->ept, filp, wait);
 
 	return mask;
@@ -318,6 +331,7 @@ static long rpmsg_eptdev_ioctl(struct file *fp, unsigned int cmd,
 		ret = rpmsg_get_sigs(eptdev->ept, &lsigs, &rsigs);
 		if (!ret)
 			ret = put_user(rsigs, (u32 *)arg);
+		atomic_set(&eptdev->sig_pending, 0);
 		break;
 	case TIOCMSET:
 	case TIOCMBIS:
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#8 lore raw

Re: [PATCH 4/5] rpmsg: wakeup poll to notify signal update

Quoted 30 lines · Arun Kumar Neelakantam

Add support to wait on poll to get signal notifications. Send POLLPRI mask to indicate the signal change.

---
 drivers/rpmsg/rpmsg_char.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
drivers/rpmsg/rpmsg_char.c +3 -0
index c983d6c..aee5561 100644
@@ -64,6 +64,7 @@ struct rpmsg_ctrldev {
  * @queue_lock:	synchronization of @queue operations
  * @queue:	incoming message queue
  * @readq:	wait object for incoming queue
+ * @sig_pending:state of signal notification
  */
 struct rpmsg_eptdev {
 	struct device dev;
@@ -78,6 +79,8 @@ struct rpmsg_eptdev {
 	spinlock_t queue_lock;
 	struct sk_buff_head queue;
 	wait_queue_head_t readq;
+
+	atomic_t sig_pending;
 };
 
 static int rpmsg_eptdev_destroy(struct device *dev, void *data)

drivers/rpmsg/rpmsg_char.c static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len, · line 136

Quoted 9 lines
drivers/rpmsg/rpmsg_char.c +6 -0
@@ -125,6 +128,12 @@ static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
 static int rpmsg_sigs_cb(struct rpmsg_device *rpdev, void *priv,
 			 u32 old, u32 new)
 {
+	struct rpmsg_eptdev *eptdev = priv;
+
+	atomic_set(&eptdev->sig_pending, 1);
+
+	/* wake up any blocking processes, waiting for signal notification */
+	wake_up_interruptible(&eptdev->readq);

This will execute a full memory barrier, so you shouldn't need to use atomic_t to carry this information.

drivers/rpmsg/rpmsg_char.c static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len, · line 139

Quoted 3 lines
drivers/rpmsg/rpmsg_char.c +0 -0
 	return 0;
 }
 

The rest looks good.

Regards,
Bjorn

#9 lore raw

PATCH 5/5 rpmsg: glink: Convert the native signals to TIOCM

In reply to Arun Kumar Neelakantam

Native signals over SMEM transport are different from Linux TIOCM signals. Add a macro to convert signals when sent or received from clients.

---
 drivers/rpmsg/qcom_glink_native.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
drivers/rpmsg/qcom_glink_native.c +29 -0
index ff8751b..3054c86 100644
@@ -18,6 +18,7 @@
 #include <linux/rpmsg.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/termios.h>
 #include <linux/workqueue.h>
 #include <linux/mailbox_client.h>
 
@@ -211,6 +212,11 @@ struct glink_channel {
 
 #define GLINK_FEATURE_INTENTLESS	BIT(1)
 
+#define NATIVE_DTR_SIG BIT(31)
+#define NATIVE_CTS_SIG BIT(30)
+#define NATIVE_CD_SIG BIT(29)
+#define NATIVE_RI_SIG BIT(28)
+
 static void qcom_glink_rx_done_work(struct work_struct *work);
 
 static struct glink_channel *qcom_glink_alloc_channel(struct qcom_glink *glink,
@@ -998,6 +1004,18 @@ static int qcom_glink_handle_signals(struct qcom_glink *glink,
 	}
 
 	old = channel->rsigs;
+
+	/* convert signals from NATIVE to TIOCM */
+	if (signals & NATIVE_DTR_SIG)
+		signals |= TIOCM_DSR;
+	if (signals & NATIVE_CTS_SIG)
+		signals |= TIOCM_CTS;
+	if (signals & NATIVE_CD_SIG)
+		signals |= TIOCM_CD;
+	if (signals & NATIVE_RI_SIG)
+		signals |= TIOCM_RI;
+	signals &= 0x0fff;
+
 	channel->rsigs = signals;
 
 	if (channel->ept.sig_cb)
@@ -1387,6 +1405,17 @@ static int qcom_glink_set_sigs(struct rpmsg_endpoint *ept, u32 sigs)
 
 	channel->lsigs = sigs;
 
+	/* convert signals from TIOCM to NATIVE */
+	sigs &= 0x0fff;
+	if (sigs & TIOCM_DTR)
+		sigs |= NATIVE_DTR_SIG;
+	if (sigs & TIOCM_RTS)
+		sigs |= NATIVE_CTS_SIG;
+	if (sigs & TIOCM_CD)
+		sigs |= NATIVE_CD_SIG;
+	if (sigs & TIOCM_RI)
+		sigs |= NATIVE_RI_SIG;
+
 	return qcom_glink_send_signals(glink, channel, sigs);
 }
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project