[PATCH net-next v9 1/2] net: wwan: core: propagate modem control signals to port drivers

[Peter Hunt <peter.hunt@opengear.com>]

The WWAN character device emulates the TTY modem-control ioctls
(TIOCMGET/TIOCMSET/TIOCMBIC/TIOCMBIS) for AT and QCDM ports, but the
result is only stored in port->at_data.mdmbits and never reaches the port
driver. A driver therefore cannot act on the host raising or dropping
DTR/RTS, even though some modems depend on it (e.g. they withhold
unsolicited AT result codes until the host asserts DTR).

Add an optional ->dtr_rts(port, mdmbits) operation to struct wwan_port_ops.
Drivers that implement it receive the TIOCM output bitmask so they can
assert or de-assert DTR and RTS independently. As in tty_tiocmset(), the
wwan core keeps only the output bits (DTR, RTS, OUT1, OUT2, LOOP) of the
TIOCMSET/TIOCMBIS/TIOCMBIC value in port->at_data.mdmbits, and calls
->dtr_rts when DTR or RTS actually changes, gated on WWAN_PORT_AT to match
the open/close raise/drop behaviour.

Also raise DTR/RTS in wwan_port_op_start on first open of an AT port when
the driver implements ->dtr_rts, and drop them in wwan_port_op_stop on
last close if HUPCL is set in the port's termios. This mirrors TTY
semantics and means individual drivers do not need to implement this
themselves. As on a TTY, HUPCL defaults to on for AT ports, and userspace
can clear it with TCSETS to keep DTR asserted across close, for example so
that closing the port does not end a call on a modem set to AT&D1/AT&D2.
TCSETS and TCSETS2 now only commit the new termios when the copy from
userspace succeeds, so a faulting call cannot silently clear HUPCL.

->dtr_rts must be called under ops_lock, which serialises port ops and
protects against port removal (removal clears port->ops under it). So
that the TIOCM state and the line state sent to the driver cannot get
out of step, the termios/TIOCM state now also lives under ops_lock and
the separate data_lock is removed. The AT/QCDM ioctl handler takes
ops_lock interruptibly, since a driver's blocking write holds it while
it waits.

wwan_remove_port() also drops DTR/RTS before ->stop() when a port is
removed while still open, regardless of HUPCL since the device is going
away. The raise and drop paths go through two small helpers, which do
the AT port and ->dtr_rts checks and pass the driver the resulting
bitmask.

Signed-off-by: Peter Hunt <peter.hunt@opengear.com>
---
v9: Raise DTR/RTS inside the existing first-open branch of
    wwan_port_op_start(), and drop data_lock so the termios/TIOCM state
    and ->dtr_rts calls are all serialised by ops_lock, taken
    interruptibly in the ioctl handler (Loic Poulain). Mask
    TIOCMSET/TIOCMBIS/TIOCMBIC to the output bits and only call
    ->dtr_rts when DTR or RTS changes, commit TCSETS/TCSETS2 only on a
    successful copy, and describe the calls in the ->dtr_rts kernel-doc
    (Sashiko review of v7). Document in the ->dtr_rts kernel-doc why it
    is AT-only (Loic Poulain, Daniele Palmas)
v8: Add wwan_port_raise_dtr_rts() as the counterpart of
    wwan_port_drop_dtr_rts() and move the open-path handling there, move
    the AT port and ->dtr_rts checks into both helpers, use
    scoped_guard() for data_lock (Loic Poulain)
v7: Drop DTR/RTS on last close only if HUPCL is set, and default HUPCL on
    for AT ports. Pass the masked mdmbits rather than 0 from
    wwan_remove_port(), via a helper shared with the close path. Both
    from the Sashiko review of v6
v6: Rebase onto net-next. Snapshot mdmbits under data_lock before calling
    ->dtr_rts from wwan_port_op_start()/wwan_port_op_stop(), rather than
    reading it after data_lock has been released
v5: Change ->dtr_rts signature from bool to unsigned int mdmbits so DTR
    and RTS can be driven independently; re-read mdmbits inside ops_lock
    in the ioctl path to close a concurrent-ioctl ordering race; add
    de-assert call to wwan_remove_port() for the hot-unplug case; update
    kernel-doc to note the op is AT-only and describe the mdmbits argument
v4: Protect at_data.mdmbits in wwan_port_op_start/stop under data_lock;
    release data_lock and acquire ops_lock with a NULL check before calling
    ->dtr_rts from the ioctl path; gate ioctl ->dtr_rts on WWAN_PORT_AT to
    match open/close behaviour; reduce boolean to TIOCM_DTR only
v3: Replace ->tiocmget/->tiocmset with ->dtr_rts(port, bool on) modelled
    on tty_port_operations.dtr_rts; raise/drop DTR/RTS in
    wwan_port_op_start/stop rather than in the driver (Loic Poulain)
---
 drivers/net/wwan/wwan_core.c | 96 +++++++++++++++++++++++++++++-------
 include/linux/wwan.h         |  8 +++
 2 files changed, 87 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index ffbcf11e4e68..27ae6583d3e0 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -25,6 +25,10 @@
 #include <net/rtnetlink.h>
 #include <uapi/linux/wwan.h>
 
+/* Modem control bits userspace may set on an AT port, as for tty_tiocmset() */
+#define WWAN_TIOCM_OUTPUTS	(TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 | \
+				 TIOCM_OUT2 | TIOCM_LOOP)
+
 /* Maximum number of minors in use */
 #define WWAN_MAX_MINORS		(1 << MINORBITS)
 
@@ -73,11 +77,10 @@ struct wwan_device {
  * @start_count: Port start counter
  * @flags: Store port state and capabilities
  * @ops: Pointer to WWAN port operations
- * @ops_lock: Protect port ops
+ * @ops_lock: Protect port ops, and the AT/QCDM termios and TIOCM state
  * @dev: Underlying device
  * @rxq: Buffer inbound queue
  * @waitqueue: The waitqueue for port fops (read/write/poll)
- * @data_lock: Port specific data access serialization
  * @headroom_len: SKB reserved headroom size
  * @frag_len: Length to fragment packet
  * @at_data: AT port specific data
@@ -88,11 +91,10 @@ struct wwan_port {
 	unsigned int start_count;
 	unsigned long flags;
 	const struct wwan_port_ops *ops;
-	struct mutex ops_lock; /* Serialize ops + protect against removal */
+	struct mutex ops_lock; /* Serialize ops + at_data, protect against removal */
 	struct device dev;
 	struct sk_buff_head rxq;
 	wait_queue_head_t waitqueue;
-	struct mutex data_lock;	/* Port specific data access serialization */
 	size_t headroom_len;
 	size_t frag_len;
 	union {
@@ -374,7 +376,6 @@ static void wwan_port_destroy(struct device *dev)
 
 	if (dev->class == &wwan_class)
 		ida_free(&minors, MINOR(dev->devt));
-	mutex_destroy(&port->data_lock);
 	mutex_destroy(&port->ops_lock);
 	kfree(port);
 }
@@ -653,7 +654,10 @@ struct wwan_port *wwan_create_port(struct device *parent,
 	mutex_init(&port->ops_lock);
 	skb_queue_head_init(&port->rxq);
 	init_waitqueue_head(&port->waitqueue);
-	mutex_init(&port->data_lock);
+
+	/* AT ports hang up on last close by default, as a TTY does */
+	if (type == WWAN_PORT_AT)
+		port->at_data.termios.c_cflag = HUPCL;
 
 	port->dev.parent = &wwandev->dev;
 	port->dev.type = &wwan_port_dev_type;
@@ -679,12 +683,44 @@ struct wwan_port *wwan_create_port(struct device *parent,
 }
 EXPORT_SYMBOL_GPL(wwan_create_port);
 
+/* Raise DTR/RTS on first open of an AT port, as a TTY does. Called with
+ * ops_lock held.
+ */
+static void wwan_port_raise_dtr_rts(struct wwan_port *port)
+{
+	lockdep_assert_held(&port->ops_lock);
+
+	if (port->type != WWAN_PORT_AT || !port->ops->dtr_rts)
+		return;
+
+	port->at_data.mdmbits |= TIOCM_DTR | TIOCM_RTS;
+	port->ops->dtr_rts(port, port->at_data.mdmbits);
+}
+
+/* Drop DTR/RTS on an AT port. Called with ops_lock held. On last close
+ * the lines are only dropped if HUPCL is set, as for a TTY, on port
+ * removal they are always dropped.
+ */
+static void wwan_port_drop_dtr_rts(struct wwan_port *port, bool hupcl_only)
+{
+	lockdep_assert_held(&port->ops_lock);
+
+	if (port->type != WWAN_PORT_AT || !port->ops->dtr_rts)
+		return;
+	if (hupcl_only && !(port->at_data.termios.c_cflag & HUPCL))
+		return;
+
+	port->at_data.mdmbits &= ~(TIOCM_DTR | TIOCM_RTS);
+	port->ops->dtr_rts(port, port->at_data.mdmbits);
+}
+
 void wwan_remove_port(struct wwan_port *port)
 {
 	struct wwan_device *wwandev = to_wwan_dev(port->dev.parent);
 
 	mutex_lock(&port->ops_lock);
 	if (port->start_count) {
+		wwan_port_drop_dtr_rts(port, false);
 		port->ops->stop(port);
 		port->start_count = 0;
 	}
@@ -756,8 +792,11 @@ static int wwan_port_op_start(struct wwan_port *port)
 	}
 
 	/* If port is already started, don't start again */
-	if (!port->start_count)
+	if (!port->start_count) {
 		ret = port->ops->start(port);
+		if (!ret)
+			wwan_port_raise_dtr_rts(port);
+	}
 
 	if (!ret)
 		port->start_count++;
@@ -773,8 +812,10 @@ static void wwan_port_op_stop(struct wwan_port *port)
 	mutex_lock(&port->ops_lock);
 	port->start_count--;
 	if (!port->start_count) {
-		if (port->ops)
+		if (port->ops) {
+			wwan_port_drop_dtr_rts(port, true);
 			port->ops->stop(port);
+		}
 		skb_queue_purge(&port->rxq);
 		clear_bit(WWAN_PORT_EXCLUSIVE, &port->flags);
 	}
@@ -979,9 +1020,15 @@ static __poll_t wwan_port_fops_poll(struct file *filp, poll_table *wait)
 static long wwan_port_fops_at_ioctl(struct wwan_port *port, unsigned int cmd,
 				    unsigned long arg)
 {
-	int ret = 0;
+	int ret;
 
-	mutex_lock(&port->data_lock);
+	/* ops_lock serialises the termios/TIOCM state with port ops and
+	 * removal. A blocking write holds it too, so wait interruptibly.
+	 */
+	ACQUIRE(mutex_intr, lock)(&port->ops_lock);
+	ret = ACQUIRE_ERR(mutex_intr, &lock);
+	if (ret)
+		return ret;
 
 	switch (cmd) {
 	case TCFLSH:
@@ -995,11 +1042,17 @@ static long wwan_port_fops_at_ioctl(struct wwan_port *port, unsigned int cmd,
 
 	case TCSETS:
 	case TCSETSW:
-	case TCSETSF:
-		if (copy_from_user(&port->at_data.termios, (void __user *)arg,
+	case TCSETSF: {
+		struct ktermios termios = port->at_data.termios;
+
+		/* Commit only on success, a partial copy must not clear HUPCL */
+		if (copy_from_user(&termios, (void __user *)arg,
 				   sizeof(struct termios)))
 			ret = -EFAULT;
+		else
+			port->at_data.termios = termios;
 		break;
+	}
 
 #ifdef TCGETS2
 	case TCGETS2:
@@ -1010,11 +1063,16 @@ static long wwan_port_fops_at_ioctl(struct wwan_port *port, unsigned int cmd,
 
 	case TCSETS2:
 	case TCSETSW2:
-	case TCSETSF2:
-		if (copy_from_user(&port->at_data.termios, (void __user *)arg,
+	case TCSETSF2: {
+		struct ktermios termios = port->at_data.termios;
+
+		if (copy_from_user(&termios, (void __user *)arg,
 				   sizeof(struct termios2)))
 			ret = -EFAULT;
+		else
+			port->at_data.termios = termios;
 		break;
+	}
 #endif
 
 	case TIOCMGET:
@@ -1024,18 +1082,24 @@ static long wwan_port_fops_at_ioctl(struct wwan_port *port, unsigned int cmd,
 	case TIOCMSET:
 	case TIOCMBIC:
 	case TIOCMBIS: {
-		int mdmbits;
+		int mdmbits, old = port->at_data.mdmbits;
 
 		if (copy_from_user(&mdmbits, (int __user *)arg, sizeof(int))) {
 			ret = -EFAULT;
 			break;
 		}
+		mdmbits &= WWAN_TIOCM_OUTPUTS;
 		if (cmd == TIOCMBIC)
 			port->at_data.mdmbits &= ~mdmbits;
 		else if (cmd == TIOCMBIS)
 			port->at_data.mdmbits |= mdmbits;
 		else
 			port->at_data.mdmbits = mdmbits;
+		/* Only signal the driver when DTR or RTS actually changes */
+		if (port->type == WWAN_PORT_AT && port->ops &&
+		    port->ops->dtr_rts &&
+		    (old ^ port->at_data.mdmbits) & (TIOCM_DTR | TIOCM_RTS))
+			port->ops->dtr_rts(port, port->at_data.mdmbits);
 		break;
 	}
 
@@ -1059,8 +1123,6 @@ static long wwan_port_fops_at_ioctl(struct wwan_port *port, unsigned int cmd,
 		ret = -ENOIOCTLCMD;
 	}
 
-	mutex_unlock(&port->data_lock);
-
 	return ret;
 }
 
diff --git a/include/linux/wwan.h b/include/linux/wwan.h
index 1e0e2cb53579..5bd9842885bd 100644
--- a/include/linux/wwan.h
+++ b/include/linux/wwan.h
@@ -57,6 +57,13 @@ struct wwan_port;
  * @tx_blocking: Optional blocking routine that sends WWAN port protocol data
  *               to the device.
  * @tx_poll: Optional routine that sets additional TX poll flags.
+ * @dtr_rts: Optional routine that updates the modem control lines to match
+ *           @mdmbits, the TIOCM_* output bits (DTR, RTS, OUT1, OUT2, LOOP)
+ *           currently set for the port. Called on first open, on last close
+ *           if HUPCL is set, on port removal, and when TIOCMSET, TIOCMBIS or
+ *           TIOCMBIC changes DTR or RTS. Only called for WWAN_PORT_AT ports,
+ *           as only the AT (DUN) port carries the modem's virtual RS-232
+ *           control signals.
  *
  * The wwan_port_ops structure contains a list of low-level operations
  * that control a WWAN port device. All functions are mandatory unless specified.
@@ -70,6 +77,7 @@ struct wwan_port_ops {
 	int (*tx_blocking)(struct wwan_port *port, struct sk_buff *skb);
 	__poll_t (*tx_poll)(struct wwan_port *port, struct file *filp,
 			    poll_table *wait);
+	void (*dtr_rts)(struct wwan_port *port, unsigned int mdmbits);
 };
 
 /** struct wwan_port_caps - The WWAN port capbilities
-- 
2.43.0