* [PATCH net-next v9 0/2] net: wwan: support DTR/RTS on AT ports via MHI IP_CTRL
@ 2026-10-12 17:22 Peter Hunt
2026-10-12 17:22 ` [PATCH net-next v9 1/2] net: wwan: core: propagate modem control signals to port drivers Peter Hunt
2026-10-12 17:22 ` [PATCH net-next v9 2/2] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel Peter Hunt
0 siblings, 2 replies; 8+ messages in thread
From: Peter Hunt @ 2026-10-12 17:22 UTC (permalink / raw)
To: loic.poulain, ryazanov.s.a
Cc: Daniele Palmas, johannes, mani, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, mhi, linux-arm-msm, linux-kernel,
Peter Hunt
Qualcomm/Sierra SDX55/SDX65 modems (e.g. EM9291) withhold unsolicited AT
result codes until the host asserts DTR. The in-tree mhi_wwan_ctrl driver
exposes AT ports but never signals DTR, so URCs never reach userspace.
Patch 1 extends the wwan core with an optional ->dtr_rts(port, mdmbits)
port op. The TIOCM bitmask state is tracked in the wwan core, which raises
DTR/RTS on first open of an AT port whose driver implements ->dtr_rts,
drops them on last close (if HUPCL is set, which is the default for AT
ports) and on port removal, and passes the resolved bitmask to the
driver on TIOCMSET/TIOCMBIC/TIOCMBIS.
Patch 2 adds a second mhi_driver to mhi_wwan_ctrl that binds the IP_CTRL
channel and implements ->dtr_rts by sending the host serial state to the
modem over that channel. The existing AT/QMI/MBIM data path is untouched.
This is a two-patch series. The pci_generic patch from v5 that
enumerates the IP_CTRL channel for the Sierra EM919x/EM929x has been
applied to mhi-next by Mani as commit 83c29a55b89e ("bus: mhi: host:
pci_generic: Add IP_CTRL channel for Sierra EM919x/EM929x"). There is no
build dependency between the two, without that commit the IP_CTRL driver
simply never binds and ->dtr_rts is a no-op.
Note on the ->dtr_rts signature (Loic):
In v3 I replaced ->tiocmget/->tiocmset with ->dtr_rts(port, bool on)
modelled on tty_port_operations, and moved the TIOCM handling into the
wwan core, as you suggested on v2. Review of v3 and v4 then pointed out
that a single bool cannot represent the two lines independently. With
TIOCMBIC/TIOCMBIS on one line while the other is in the opposite state,
the line state sent to the modem no longer matches what TIOCMGET reports
(e.g. RTS re-asserted after TIOCMBIC(TIOCM_RTS)).
Since v5 the op keeps the dtr_rts name and the core still owns all TIOCM
handling, but it is passed the resolved TIOCM bitmask instead of a bool,
so the driver can drive DTR and RTS independently. I kept the dtr_rts
name deliberately, following your v2 preference over ->tiocmset, even
though the signature now differs from tty_port_operations.dtr_rts. The
open/close paths still behave as tty_port dtr_rts does. Is this
acceptable to you, or would you prefer a different shape, for example a
bool ->dtr_rts for open/close plus a separate op for the ioctl path?
Changes in v9:
- Patch 1: drop data_lock, so the termios/TIOCM state and ->dtr_rts
calls are all serialised by ops_lock, which the AT/QCDM ioctl handler
now takes interruptibly (Loic Poulain)
- Patch 1: raise DTR/RTS inside the existing first-open branch of
wwan_port_op_start() (Loic Poulain)
- Patch 1: mask TIOCMSET/TIOCMBIS/TIOCMBIC to the output bits, only call
->dtr_rts when DTR or RTS changes, and commit TCSETS/TCSETS2 only on a
successful copy (Sashiko review of v7)
- Patch 2: requeue the IP_CTRL DL sink buffer only from the refill work,
and disable it with disable_delayed_work_sync() before
mhi_unprepare_from_transfer() in remove() (Loic Poulain, Sashiko
review of v7 and v8)
- Patch 2: leave unpreparing to mhi_probe() when probe fails, and name
the wwan core commit in the commit message (Sashiko review of v7)
- DTR/RTS stays limited to AT ports, as agreed with Loic and Daniele,
and the ->dtr_rts kernel-doc now says why
Changes in v8 (Loic Poulain's review of v7, no functional change):
- Patch 1: add wwan_port_raise_dtr_rts() as the counterpart of
wwan_port_drop_dtr_rts() and move the open-path handling into it
- Patch 1: do the AT port and ->dtr_rts checks inside both helpers
- Patch 1: use scoped_guard() for data_lock in the helpers
Changes in v7 (all from the Sashiko review of v6):
- Patch 1: drop DTR/RTS on last close only if HUPCL is set, as a TTY
does, and default HUPCL on for AT ports so close behaviour is
unchanged unless userspace clears it with TCSETS
- Patch 1: wwan_remove_port() passes the masked mdmbits rather than 0,
via a helper shared with the close path
- Patch 2: retry a failed DL sink buffer requeue from a delayed work
item on -ENOMEM, and log other failures
- Patch 2: note in the commit message that the IP_CTRL channel entry
for the EM919x/EM929x is 83c29a55b89e in the MHI tree
Changes in v6:
- Rebased onto net-next, dropped the pci_generic patch (now in mhi-next)
- Patch 1: snapshot mdmbits under data_lock before calling ->dtr_rts from
the open/close paths
- Patch 2: allocate the IP_CTRL DL sink buffer separately instead of
embedding it in struct mhi_wwan_dtr (DMA safety on non-coherent
platforms), and do not requeue it when the DL transfer completes with an
error such as -ENOTCONN during channel teardown
v8: https://lore.kernel.org/netdev/20261007182238.1154851-1-peter.hunt@opengear.com/
v7: https://lore.kernel.org/netdev/20261006040422.3208888-1-peter.hunt@opengear.com/
v6: https://lore.kernel.org/netdev/20261001204614.3481089-1-peter.hunt@opengear.com/
v5: https://lore.kernel.org/netdev/20260819224927.2274790-1-peter.hunt@opengear.com/
v4: https://lore.kernel.org/netdev/20260816121705.858013-1-peter.hunt@opengear.com/
v3: https://lore.kernel.org/netdev/20260807215042.2714442-1-peter.hunt@opengear.com/
v2: https://lore.kernel.org/netdev/20260806155253.3378294-1-peter.hunt@opengear.com/
v1: https://lore.kernel.org/netdev/20260804233411.1953445-1-peter.hunt@opengear.com/
Peter Hunt (2):
net: wwan: core: propagate modem control signals to port drivers
net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel
drivers/net/wwan/mhi_wwan_ctrl.c | 233 ++++++++++++++++++++++++++++++-
drivers/net/wwan/wwan_core.c | 96 ++++++++++---
include/linux/wwan.h | 8 ++
3 files changed, 319 insertions(+), 18 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net-next v9 1/2] net: wwan: core: propagate modem control signals to port drivers 2026-10-12 17:22 [PATCH net-next v9 0/2] net: wwan: support DTR/RTS on AT ports via MHI IP_CTRL Peter Hunt @ 2026-10-12 17:22 ` Peter Hunt 2026-10-13 17:23 ` netdev-bot+sashiko 2026-10-12 17:22 ` [PATCH net-next v9 2/2] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel Peter Hunt 1 sibling, 1 reply; 8+ messages in thread From: Peter Hunt @ 2026-10-12 17:22 UTC (permalink / raw) To: loic.poulain, ryazanov.s.a Cc: Daniele Palmas, johannes, mani, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, mhi, linux-arm-msm, linux-kernel, Peter Hunt 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v9 1/2] net: wwan: core: propagate modem control signals to port drivers 2026-10-12 17:22 ` [PATCH net-next v9 1/2] net: wwan: core: propagate modem control signals to port drivers Peter Hunt @ 2026-10-13 17:23 ` netdev-bot+sashiko 2026-10-13 18:28 ` Peter Hunt 0 siblings, 1 reply; 8+ messages in thread From: netdev-bot+sashiko @ 2026-10-13 17:23 UTC (permalink / raw) To: peter.hunt Cc: loic.poulain, ryazanov.s.a, dnlplm, johannes, mani, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, mhi, linux-arm-msm, linux-kernel Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider. Critical: 0 · High: 0 · Medium: 2 · Low: 3 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [Medium] `wwan_port_fops_at_ioctl()` now does `ACQUIRE(mutex_intr, lock)(&port->ops_lock)` before its `switch`, so every ioctl on an… - [Medium] `wwan_port_fops_at_ioctl()` now holds `port->ops_lock` for the whole function, via a function-scope ACQUIRE guard, across every user-… - [Low] Not a valid finding (baseline decision: false). - [Low] The comments say the wwan open/close DTR handling works 'as a TTY does', but the code ignores the baud field of the termios state it… - [Low] The new `->dtr_rts` kernel-doc in include/linux/wwan.h says the op is 'Called on first open, on last close if HUPCL is set, on port… -- net: wwan: core: propagate modem control signals to port drivers This adds an optional ->dtr_rts() operation to struct wwan_port_ops. The wwan core calls it on first open, on last close when HUPCL is set, on removal of an open port, and when TIOCMSET/TIOCMBIS/TIOCMBIC change DTR or RTS on an AT port. The separate data_lock is removed, and the AT/QCDM termios and TIOCM state is now protected by ops_lock, which the ioctl handler takes interruptibly. > diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c > index ffbcf11e4e681..27ae6583d3e08 100644 > --- a/drivers/net/wwan/wwan_core.c > +++ b/drivers/net/wwan/wwan_core.c [ ... ] > @@ -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); > +} [Severity: Low] The comments here and in wwan_create_port() say this behaves as a TTY does. Is the baud field of the termios taken into account anywhere? wwan_create_port() sets c_cflag to just HUPCL, so TCGETS reports B0 to userspace. Even so, wwan_port_raise_dtr_rts() asserts DTR/RTS on every first open. On a TTY, the lines are only raised when the baud rate is non-zero: drivers/tty/tty_port.c:tty_port_block_til_ready() { ... if (C_BAUD(tty)) tty_port_raise_dtr_rts(port); ... } The TTY default is also tty_std_termios (B38400 | CS8 | CREAD | HUPCL), not bare HUPCL. In the same way, a TCSETS with B0 only stores the termios here and never calls ->dtr_rts(). A TTY would drop DTR in that case. Examples are stty -F /dev/wwan0at0 0, or the POSIX hang-up of cfsetospeed(B0) followed by tcsetattr(). Separately, the kernel-doc says "Called on first open", but the call only happens when ->start() succeeds. Should the comments make a weaker claim? Alternatively, could the default termios use a non-zero baud rate so that TCGETS matches what the driver was told? > + > +/* 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; > } [Severity: Low] The comment above says "on port removal they are always dropped". The ->dtr_rts kernel-doc in include/linux/wwan.h also lists "on port removal" without any condition. Is that accurate, given that the drop sits inside the if (port->start_count) block? The commit message describes clearing HUPCL as the way to keep DTR asserted across close. If userspace does that and then closes the port, wwan_port_op_stop() returns early in wwan_port_drop_dtr_rts(). A later wwan_remove_port() then sees start_count == 0, so the driver never gets a ->dtr_rts() call on removal. The behaviour itself matches a TTY. Could the comment and the kernel-doc say that the lines are only dropped when a port is removed while still open? [ ... ] > @@ -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; [Severity: Medium] The lock is now taken before the switch, so every ioctl on an AT/QCDM port waits on ops_lock. That includes TCGETS, TCFLSH, TIOCEXCL/TIOCNXCL/TIOCGEXCL, and commands that end up in the default -ENOIOCTLCMD case. FIONREAD is one of those, because wwan_port_fops_ioctl() tries the AT handler first: drivers/net/wwan/wwan_core.c:wwan_port_fops_ioctl() { ... res = wwan_port_fops_at_ioctl(port, cmd, arg); if (res != -ENOIOCTLCMD) return res; } switch (cmd) { case TIOCINQ: { /* aka SIOCINQ aka FIONREAD */ ... } wwan_port_op_tx() holds ops_lock across ->tx_blocking(). For rpmsg_wwan_ctrl that is a blocking rpmsg_send(). Suppose thread A is in a blocking write(): wwan_port_fops_write() wwan_port_op_tx() mutex_lock(&port->ops_lock) port->ops->tx_blocking() rpmsg_send() Thread B's ioctl(FIONREAD) or tcgetattr() now waits behind it. A signal during that wait makes mutex_lock_interruptible() return -EINTR. That value is not -ENOIOCTLCMD, so it goes straight back to userspace, and SA_RESTART won't restart the call. Before this patch these paths only took data_lock, which the write path never held. Could ops_lock be taken only for the commands that touch the termios/TIOCM state or call ->dtr_rts()? [ ... ] > @@ -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))) { [Severity: Medium] The ACQUIRE guard holds ops_lock for the whole function, so several user memory accesses now run with ops_lock held: - this copy_from_user() - the copy_to_user() calls for TCGETS/TCGETS2 - the copy_from_user() calls for TCSETS*/TCSETS2* - the put_user() for TIOCMGET Can userspace hold ops_lock indefinitely this way? Suppose arg points into memory whose page fault never completes, for example an mmap of a file on a FUSE mount whose daemon never answers. The thread then sleeps in the fault handler with ops_lock held. The interruptible acquisition doesn't help once the lock is owned. wwan_remove_port(), wwan_port_op_start(), wwan_port_op_stop(), wwan_port_op_tx() and wwan_port_fops_poll() all take ops_lock with plain mutex_lock(). Device removal, for example via mhi_wwan_ctrl_remove() or a cdc-wdm disconnect, would then hang in D state: ioctl(fd, TIOCMSET, ptr_into_fuse_mapping) wwan_port_fops_at_ioctl() ACQUIRE(mutex_intr, lock)(&port->ops_lock) copy_from_user() <- waits on FUSE wwan_remove_port() mutex_lock(&port->ops_lock) <- blocks Any other process that opens, closes or polls the port would block in the same way. Before this patch the ioctl user copies ran under the ioctl-private data_lock. wwan_port_fops_write() copies user data before calling wwan_port_op_tx(), so nothing touched user memory under ops_lock. Would it work to copy the argument in before taking the lock, and copy results out after dropping it, as the TTY core does? [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012172219.3668624-1-peter.hunt%40opengear.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v9 1/2] net: wwan: core: propagate modem control signals to port drivers 2026-10-13 17:23 ` netdev-bot+sashiko @ 2026-10-13 18:28 ` Peter Hunt 0 siblings, 0 replies; 8+ messages in thread From: Peter Hunt @ 2026-10-13 18:28 UTC (permalink / raw) To: netdev-bot+sashiko Cc: loic.poulain, Daniele Palmas, ryazanov.s.a, johannes, mani, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, mhi, linux-arm-msm, linux-kernel Thanks for the review, I'll send a v10. [Medium] user memory accessed with ops_lock held: agreed. v10 copies the argument in before taking ops_lock and copies results out after dropping it, as the TTY core does, so ops_lock is never held across a user-memory access and a stalled copy can't block port removal. [Medium] every AT/QCDM ioctl waits on ops_lock: agreed for the ioctls that don't touch the termios/TIOCM state. In v10 TCFLSH, TIOCEXCL/TIOCNXCL/TIOCGEXCL and unknown commands, and so the FIONREAD fallback, don't take ops_lock at all, and the rest return -ERESTARTSYS when interrupted. The termios/TIOCM ioctls still take ops_lock, as agreed with Loic for now, so on rpmsg ports they can wait behind a blocking write. [Low] TTY comparisons and the baud rate: v10 words the comments, kernel-doc and commit message more precisely and says the baud rate is not used. I've kept the existing termios defaults. [Low] "on port removal": fixed, the comment and kernel-doc now say the lines are dropped when a port is removed while still open, and that the open-time raise only happens once ->start() has succeeded. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next v9 2/2] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel 2026-10-12 17:22 [PATCH net-next v9 0/2] net: wwan: support DTR/RTS on AT ports via MHI IP_CTRL Peter Hunt 2026-10-12 17:22 ` [PATCH net-next v9 1/2] net: wwan: core: propagate modem control signals to port drivers Peter Hunt @ 2026-10-12 17:22 ` Peter Hunt 2026-10-13 10:00 ` Loic Poulain 2026-10-13 17:23 ` netdev-bot+sashiko 1 sibling, 2 replies; 8+ messages in thread From: Peter Hunt @ 2026-10-12 17:22 UTC (permalink / raw) To: loic.poulain, ryazanov.s.a Cc: Daniele Palmas, johannes, mani, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, mhi, linux-arm-msm, linux-kernel, Peter Hunt Qualcomm/Sierra SDX55/SDX65 modems withhold unsolicited AT result codes (URCs such as +CREG, and the +DMI OMA-DM/LwM2M session indications) on an AT port until the host asserts DTR. mhi_wwan_ctrl exposed the AT (DUN) ports but had no way to signal DTR, so URCs never reached userspace. Carry the host serial-control lines to the modem over the dedicated IP_CTRL MHI channel, which this module now also binds. IP_CTRL uses a separate mhi_driver with its own callbacks so the AT/QMI/MBIM data path is untouched; the control-channel device for each MHI controller is tracked in a small registry so an AT port drives the IP_CTRL channel of its own modem (multiple modems are supported). The IP_CTRL channel has to be declared in the controller's channel config for this driver to bind. For the Sierra EM919x/EM929x this is done by commit 83c29a55b89e ("bus: mhi: host: pci_generic: Add IP_CTRL channel for Sierra EM919x/EM929x") in the MHI tree. On controllers that do not declare IP_CTRL the driver does not bind and ->dtr_rts is a no-op. The wwan core, with "net: wwan: core: propagate modem control signals to port drivers", raises DTR/RTS on first open and drops them on last close (if HUPCL is set) for any AT port whose driver implements ->dtr_rts, so no open/close handling is needed here. The new ->dtr_rts op lets userspace assert or de-assert them via TIOCMSET/TIOCMBIC/TIOCMBIS. Received device->host serial state messages are silently discarded; a single recycled sink buffer keeps the IP_CTRL DL ring live so the modem's transmit path does not stall. ->dtr_rts is void, following the tty_port_operations model it is modelled on. Delivery is best-effort: at_data.mdmbits always reflects the committed userspace intent for TIOCMGET regardless of whether the IP_CTRL message was queued, and the next ioctl or open/close cycle will resynchronise the modem state. The DL sink buffer is requeued from a work item, which retries a transient -ENOMEM so the DL ring does not stay empty, and which remove() disables before tearing down the channel. Signed-off-by: Peter Hunt <peter.hunt@opengear.com> --- v9: Requeue the DL sink buffer only from the refill work and disable it with disable_delayed_work_sync() before mhi_unprepare_from_transfer() in remove(), so no requeue can race with channel teardown (Loic Poulain, Sashiko review of v7). Leave unpreparing to mhi_probe() when probe fails. Name the wwan core commit instead of "patch 1" v8: No changes v7: Retry a failed DL sink buffer requeue from a delayed work item on -ENOMEM, and log other failures. Note the dependency on the IP_CTRL channel entry in pci_generic, now in the MHI tree. Both from the Sashiko review of v6 v6: Rebase onto net-next. Allocate the IP_CTRL DL sink buffer separately instead of embedding it in struct mhi_wwan_dtr, so DMA cache maintenance on non-coherent platforms cannot touch neighbouring fields. Do not requeue the sink buffer from the DL callback when the transfer completes with an error (e.g. -ENOTCONN during channel teardown) v5: Implement ->dtr_rts(port, mdmbits) and drive DTR and RTS independently. Pre-queue an RX sink buffer in probe and requeue it in the DL callback so the IP_CTRL DL ring does not run empty and stall the modem's transmit path v4: Add dev_dbg when IP_CTRL channel is not enumerated for this controller and when mhi_queue_buf fails, to aid diagnosis of the "URCs missing" case on controllers that do not declare IP_CTRL v3: Remove is_at_port, implement ->dtr_rts instead of ->tiocmset, open/close DTR raise/drop handled by wwan core (Loic Poulain) --- drivers/net/wwan/mhi_wwan_ctrl.c | 233 ++++++++++++++++++++++++++++++- 1 file changed, 232 insertions(+), 1 deletion(-) diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c index a31d8540fbb8..1cbbfcb3b920 100644 --- a/drivers/net/wwan/mhi_wwan_ctrl.c +++ b/drivers/net/wwan/mhi_wwan_ctrl.c @@ -1,8 +1,13 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright (c) 2021, Linaro Ltd <loic.poulain@linaro.org> */ #include <linux/kernel.h> +#include <linux/list.h> #include <linux/mhi.h> #include <linux/module.h> +#include <linux/mutex.h> +#include <linux/slab.h> +#include <linux/termios.h> +#include <linux/workqueue.h> #include <linux/wwan.h> /* MHI wwan flags */ @@ -14,6 +19,32 @@ enum mhi_wwan_flags { #define MHI_WWAN_MAX_MTU 0x8000 +/* IP_CTRL channel message that sets the modem's DTR/RTS control lines */ +struct mhi_dtr_ctrl_msg { + __le32 preamble; + __le32 msg_id; + __le32 dest_id; + __le32 size; + __le32 msg; +} __packed; + +#define MHI_DTR_CTRL_MAGIC 0x4C525443 /* 'CTRL' */ +#define MHI_DTR_MSG_DTR BIT(0) +#define MHI_DTR_MSG_RTS BIT(1) +#define MHI_DTR_HOST_STATE 0x10 + +/* Per-controller IP_CTRL channel, used to signal DTR/RTS to that modem */ +struct mhi_wwan_dtr { + struct mhi_controller *cntrl; + struct mhi_device *mhi_dev; + struct list_head node; + struct mhi_dtr_ctrl_msg *rx_buf; /* DL sink, separate allocation for DMA safety */ + struct delayed_work rx_refill; /* retries a failed sink buffer requeue */ +}; + +static LIST_HEAD(mhi_wwan_dtr_list); +static DEFINE_MUTEX(mhi_wwan_dtr_lock); + struct mhi_wwan_dev { /* Lower level is a mhi dev, upper level is a wwan port */ struct mhi_device *mhi_dev; @@ -103,6 +134,61 @@ static void mhi_wwan_ctrl_refill_work(struct work_struct *work) } } +/* Signal the modem's DTR/RTS lines over its own controller's IP_CTRL channel */ +static int mhi_wwan_ctrl_send_dtr(struct mhi_wwan_dev *mhiwwan, unsigned int mdmbits) +{ + struct mhi_controller *cntrl = mhiwwan->mhi_dev->mhi_cntrl; + struct mhi_device *ctrl_dev = NULL; + struct mhi_dtr_ctrl_msg *dtr_msg; + struct mhi_wwan_dtr *dtr; + u32 msg = 0; + int ret; + + guard(mutex)(&mhi_wwan_dtr_lock); + + list_for_each_entry(dtr, &mhi_wwan_dtr_list, node) { + if (dtr->cntrl == cntrl) { + ctrl_dev = dtr->mhi_dev; + break; + } + } + if (!ctrl_dev) { + dev_dbg(&mhiwwan->mhi_dev->dev, + "IP_CTRL not enumerated; DTR/RTS not signalled to modem\n"); + return 0; + } + + dtr_msg = kzalloc_obj(*dtr_msg); + if (!dtr_msg) + return -ENOMEM; + + if (mdmbits & TIOCM_DTR) + msg |= MHI_DTR_MSG_DTR; + if (mdmbits & TIOCM_RTS) + msg |= MHI_DTR_MSG_RTS; + + dtr_msg->preamble = cpu_to_le32(MHI_DTR_CTRL_MAGIC); + dtr_msg->msg_id = cpu_to_le32(MHI_DTR_HOST_STATE); + dtr_msg->dest_id = cpu_to_le32(mhiwwan->mhi_dev->ul_chan_id); + dtr_msg->size = cpu_to_le32(sizeof(__le32)); + dtr_msg->msg = cpu_to_le32(msg); + + ret = mhi_queue_buf(ctrl_dev, DMA_TO_DEVICE, dtr_msg, sizeof(*dtr_msg), + MHI_EOT); + if (ret) { + dev_dbg(&mhiwwan->mhi_dev->dev, + "failed to queue DTR/RTS signal: %d\n", ret); + kfree(dtr_msg); + } + + return ret; +} + +static void mhi_wwan_ctrl_dtr_rts(struct wwan_port *port, unsigned int mdmbits) +{ + mhi_wwan_ctrl_send_dtr(wwan_port_get_drvdata(port), mdmbits); +} + static int mhi_wwan_ctrl_start(struct wwan_port *port) { struct mhi_wwan_dev *mhiwwan = wwan_port_get_drvdata(port); @@ -163,6 +249,7 @@ static const struct wwan_port_ops wwan_pops = { .start = mhi_wwan_ctrl_start, .stop = mhi_wwan_ctrl_stop, .tx = mhi_wwan_ctrl_tx, + .dtr_rts = mhi_wwan_ctrl_dtr_rts, }; static void mhi_ul_xfer_cb(struct mhi_device *mhi_dev, @@ -255,6 +342,112 @@ static void mhi_wwan_ctrl_remove(struct mhi_device *mhi_dev) kfree(mhiwwan); } +/* IP_CTRL channel driver, bound separately so the data-port path is untouched */ +static void mhi_wwan_dtr_ul_xfer_cb(struct mhi_device *mhi_dev, + struct mhi_result *mhi_result) +{ + /* MHI core has done with the buffer, release it */ + kfree(mhi_result->buf_addr); +} + +/* Requeue the single DL sink buffer so the IP_CTRL DL ring never runs empty. + * This only runs from the work item, so remove() can stop it before the + * channel is torn down. A transient mapping failure is retried, other errors + * mean the channel is going away and the next probe queues a fresh buffer. + */ +static void mhi_wwan_dtr_refill_work(struct work_struct *work) +{ + struct mhi_wwan_dtr *dtr = container_of(to_delayed_work(work), + struct mhi_wwan_dtr, rx_refill); + int ret; + + ret = mhi_queue_buf(dtr->mhi_dev, DMA_FROM_DEVICE, dtr->rx_buf, + sizeof(*dtr->rx_buf), MHI_EOT); + if (ret == -ENOMEM) { + dev_warn_ratelimited(&dtr->mhi_dev->dev, + "failed to requeue IP_CTRL RX buffer, retrying\n"); + schedule_delayed_work(&dtr->rx_refill, msecs_to_jiffies(100)); + } else if (ret) { + dev_dbg(&dtr->mhi_dev->dev, + "failed to requeue IP_CTRL RX buffer: %d\n", ret); + } +} + +static void mhi_wwan_dtr_dl_xfer_cb(struct mhi_device *mhi_dev, + struct mhi_result *mhi_result) +{ + struct mhi_wwan_dtr *dtr = dev_get_drvdata(&mhi_dev->dev); + + /* Channel is being torn down (e.g. -ENOTCONN), do not requeue */ + if (mhi_result->transaction_status && + mhi_result->transaction_status != -EOVERFLOW) + return; + + /* Modem serial state not needed, requeue the sink buffer to keep DL ring live */ + schedule_delayed_work(&dtr->rx_refill, 0); +} + +static int mhi_wwan_dtr_probe(struct mhi_device *mhi_dev, + const struct mhi_device_id *id) +{ + struct mhi_wwan_dtr *dtr; + int ret; + + dtr = kzalloc_obj(*dtr); + if (!dtr) + return -ENOMEM; + + dtr->rx_buf = kmalloc_obj(*dtr->rx_buf); + if (!dtr->rx_buf) { + ret = -ENOMEM; + goto err_free_dtr; + } + INIT_DELAYED_WORK(&dtr->rx_refill, mhi_wwan_dtr_refill_work); + + ret = mhi_prepare_for_transfer(mhi_dev); + if (ret) + goto err_free_buf; + + dtr->cntrl = mhi_dev->mhi_cntrl; + dtr->mhi_dev = mhi_dev; + dev_set_drvdata(&mhi_dev->dev, dtr); + + /* On failure mhi_probe() unprepares the channels itself */ + ret = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, dtr->rx_buf, + sizeof(*dtr->rx_buf), MHI_EOT); + if (ret) + goto err_free_buf; + + mutex_lock(&mhi_wwan_dtr_lock); + list_add(&dtr->node, &mhi_wwan_dtr_list); + mutex_unlock(&mhi_wwan_dtr_lock); + + return 0; + +err_free_buf: + kfree(dtr->rx_buf); +err_free_dtr: + kfree(dtr); + return ret; +} + +static void mhi_wwan_dtr_remove(struct mhi_device *mhi_dev) +{ + struct mhi_wwan_dtr *dtr = dev_get_drvdata(&mhi_dev->dev); + + mutex_lock(&mhi_wwan_dtr_lock); + list_del(&dtr->node); + mutex_unlock(&mhi_wwan_dtr_lock); + + /* Stop the refill work, and stop a late DL callback re-arming it, before + * the channel is torn down so no requeue can race with the teardown + */ + disable_delayed_work_sync(&dtr->rx_refill); + mhi_unprepare_from_transfer(mhi_dev); + kfree(dtr->rx_buf); + kfree(dtr); +} + static const struct mhi_device_id mhi_wwan_ctrl_match_table[] = { { .chan = "DUN", .driver_data = WWAN_PORT_AT }, { .chan = "DUN2", .driver_data = WWAN_PORT_AT }, @@ -278,7 +471,45 @@ static struct mhi_driver mhi_wwan_ctrl_driver = { }, }; -module_mhi_driver(mhi_wwan_ctrl_driver); +static const struct mhi_device_id mhi_wwan_dtr_match_table[] = { + { .chan = "IP_CTRL" }, + {}, +}; +MODULE_DEVICE_TABLE(mhi, mhi_wwan_dtr_match_table); + +static struct mhi_driver mhi_wwan_dtr_driver = { + .id_table = mhi_wwan_dtr_match_table, + .remove = mhi_wwan_dtr_remove, + .probe = mhi_wwan_dtr_probe, + .ul_xfer_cb = mhi_wwan_dtr_ul_xfer_cb, + .dl_xfer_cb = mhi_wwan_dtr_dl_xfer_cb, + .driver = { + .name = "mhi_wwan_dtr", + }, +}; + +static int __init mhi_wwan_ctrl_init(void) +{ + int ret; + + ret = mhi_driver_register(&mhi_wwan_dtr_driver); + if (ret) + return ret; + + ret = mhi_driver_register(&mhi_wwan_ctrl_driver); + if (ret) + mhi_driver_unregister(&mhi_wwan_dtr_driver); + + return ret; +} +module_init(mhi_wwan_ctrl_init); + +static void __exit mhi_wwan_ctrl_exit(void) +{ + mhi_driver_unregister(&mhi_wwan_ctrl_driver); + mhi_driver_unregister(&mhi_wwan_dtr_driver); +} +module_exit(mhi_wwan_ctrl_exit); MODULE_LICENSE("GPL v2"); MODULE_DESCRIPTION("MHI WWAN CTRL Driver"); -- 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v9 2/2] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel 2026-10-12 17:22 ` [PATCH net-next v9 2/2] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel Peter Hunt @ 2026-10-13 10:00 ` Loic Poulain 2026-10-13 17:23 ` netdev-bot+sashiko 1 sibling, 0 replies; 8+ messages in thread From: Loic Poulain @ 2026-10-13 10:00 UTC (permalink / raw) To: Peter Hunt Cc: ryazanov.s.a, Daniele Palmas, johannes, mani, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, mhi, linux-arm-msm, linux-kernel On Mon, Oct 12, 2026 at 7:23 PM Peter Hunt <peter.hunt@opengear.com> wrote: > > Qualcomm/Sierra SDX55/SDX65 modems withhold unsolicited AT result codes > (URCs such as +CREG, and the +DMI OMA-DM/LwM2M session indications) on > an AT port until the host asserts DTR. mhi_wwan_ctrl exposed the AT > (DUN) ports but had no way to signal DTR, so URCs never reached > userspace. > > Carry the host serial-control lines to the modem over the dedicated > IP_CTRL MHI channel, which this module now also binds. IP_CTRL uses a > separate mhi_driver with its own callbacks so the AT/QMI/MBIM data path > is untouched; the control-channel device for each MHI controller is > tracked in a small registry so an AT port drives the IP_CTRL channel of > its own modem (multiple modems are supported). > > The IP_CTRL channel has to be declared in the controller's channel config > for this driver to bind. For the Sierra EM919x/EM929x this is done by > commit 83c29a55b89e ("bus: mhi: host: pci_generic: Add IP_CTRL channel > for Sierra EM919x/EM929x") in the MHI tree. On controllers that do not > declare IP_CTRL the driver does not bind and ->dtr_rts is a no-op. > > The wwan core, with "net: wwan: core: propagate modem control signals to > port drivers", raises DTR/RTS on first open and drops them on last close > (if HUPCL is set) for any AT port whose driver implements ->dtr_rts, so > no open/close handling is needed here. The new ->dtr_rts op lets > userspace assert or de-assert them via TIOCMSET/TIOCMBIC/TIOCMBIS. > Received device->host serial state messages are silently discarded; a > single recycled sink buffer keeps the IP_CTRL DL ring live so the > modem's transmit path does not stall. > > ->dtr_rts is void, following the tty_port_operations model it is modelled > on. Delivery is best-effort: at_data.mdmbits always reflects the committed > userspace intent for TIOCMGET regardless of whether the IP_CTRL message was > queued, and the next ioctl or open/close cycle will resynchronise the modem > state. The DL sink buffer is requeued from a work item, which retries a > transient -ENOMEM so the DL ring does not stay empty, and which remove() > disables before tearing down the channel. > > Signed-off-by: Peter Hunt <peter.hunt@opengear.com> Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com> > --- > v9: Requeue the DL sink buffer only from the refill work and disable it > with disable_delayed_work_sync() before mhi_unprepare_from_transfer() > in remove(), so no requeue can race with channel teardown (Loic > Poulain, Sashiko review of v7). Leave unpreparing to mhi_probe() when > probe fails. Name the wwan core commit instead of "patch 1" > v8: No changes > v7: Retry a failed DL sink buffer requeue from a delayed work item on > -ENOMEM, and log other failures. Note the dependency on the IP_CTRL > channel entry in pci_generic, now in the MHI tree. Both from the > Sashiko review of v6 > v6: Rebase onto net-next. Allocate the IP_CTRL DL sink buffer separately > instead of embedding it in struct mhi_wwan_dtr, so DMA cache > maintenance on non-coherent platforms cannot touch neighbouring fields. > Do not requeue the sink buffer from the DL callback when the transfer > completes with an error (e.g. -ENOTCONN during channel teardown) > v5: Implement ->dtr_rts(port, mdmbits) and drive DTR and RTS > independently. Pre-queue an RX sink buffer in probe and requeue it in > the DL callback so the IP_CTRL DL ring does not run empty and stall > the modem's transmit path > v4: Add dev_dbg when IP_CTRL channel is not enumerated for this controller > and when mhi_queue_buf fails, to aid diagnosis of the "URCs missing" > case on controllers that do not declare IP_CTRL > v3: Remove is_at_port, implement ->dtr_rts instead of ->tiocmset, > open/close DTR raise/drop handled by wwan core (Loic Poulain) > --- > drivers/net/wwan/mhi_wwan_ctrl.c | 233 ++++++++++++++++++++++++++++++- > 1 file changed, 232 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c > index a31d8540fbb8..1cbbfcb3b920 100644 > --- a/drivers/net/wwan/mhi_wwan_ctrl.c > +++ b/drivers/net/wwan/mhi_wwan_ctrl.c > @@ -1,8 +1,13 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright (c) 2021, Linaro Ltd <loic.poulain@linaro.org> */ > #include <linux/kernel.h> > +#include <linux/list.h> > #include <linux/mhi.h> > #include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/slab.h> > +#include <linux/termios.h> > +#include <linux/workqueue.h> > #include <linux/wwan.h> > > /* MHI wwan flags */ > @@ -14,6 +19,32 @@ enum mhi_wwan_flags { > > #define MHI_WWAN_MAX_MTU 0x8000 > > +/* IP_CTRL channel message that sets the modem's DTR/RTS control lines */ > +struct mhi_dtr_ctrl_msg { > + __le32 preamble; > + __le32 msg_id; > + __le32 dest_id; > + __le32 size; > + __le32 msg; > +} __packed; > + > +#define MHI_DTR_CTRL_MAGIC 0x4C525443 /* 'CTRL' */ > +#define MHI_DTR_MSG_DTR BIT(0) > +#define MHI_DTR_MSG_RTS BIT(1) > +#define MHI_DTR_HOST_STATE 0x10 > + > +/* Per-controller IP_CTRL channel, used to signal DTR/RTS to that modem */ > +struct mhi_wwan_dtr { > + struct mhi_controller *cntrl; > + struct mhi_device *mhi_dev; > + struct list_head node; > + struct mhi_dtr_ctrl_msg *rx_buf; /* DL sink, separate allocation for DMA safety */ > + struct delayed_work rx_refill; /* retries a failed sink buffer requeue */ > +}; > + > +static LIST_HEAD(mhi_wwan_dtr_list); > +static DEFINE_MUTEX(mhi_wwan_dtr_lock); > + > struct mhi_wwan_dev { > /* Lower level is a mhi dev, upper level is a wwan port */ > struct mhi_device *mhi_dev; > @@ -103,6 +134,61 @@ static void mhi_wwan_ctrl_refill_work(struct work_struct *work) > } > } > > +/* Signal the modem's DTR/RTS lines over its own controller's IP_CTRL channel */ > +static int mhi_wwan_ctrl_send_dtr(struct mhi_wwan_dev *mhiwwan, unsigned int mdmbits) > +{ > + struct mhi_controller *cntrl = mhiwwan->mhi_dev->mhi_cntrl; > + struct mhi_device *ctrl_dev = NULL; > + struct mhi_dtr_ctrl_msg *dtr_msg; > + struct mhi_wwan_dtr *dtr; > + u32 msg = 0; > + int ret; > + > + guard(mutex)(&mhi_wwan_dtr_lock); > + > + list_for_each_entry(dtr, &mhi_wwan_dtr_list, node) { > + if (dtr->cntrl == cntrl) { > + ctrl_dev = dtr->mhi_dev; > + break; > + } > + } > + if (!ctrl_dev) { > + dev_dbg(&mhiwwan->mhi_dev->dev, > + "IP_CTRL not enumerated; DTR/RTS not signalled to modem\n"); > + return 0; > + } > + > + dtr_msg = kzalloc_obj(*dtr_msg); > + if (!dtr_msg) > + return -ENOMEM; > + > + if (mdmbits & TIOCM_DTR) > + msg |= MHI_DTR_MSG_DTR; > + if (mdmbits & TIOCM_RTS) > + msg |= MHI_DTR_MSG_RTS; > + > + dtr_msg->preamble = cpu_to_le32(MHI_DTR_CTRL_MAGIC); > + dtr_msg->msg_id = cpu_to_le32(MHI_DTR_HOST_STATE); > + dtr_msg->dest_id = cpu_to_le32(mhiwwan->mhi_dev->ul_chan_id); > + dtr_msg->size = cpu_to_le32(sizeof(__le32)); > + dtr_msg->msg = cpu_to_le32(msg); > + > + ret = mhi_queue_buf(ctrl_dev, DMA_TO_DEVICE, dtr_msg, sizeof(*dtr_msg), > + MHI_EOT); > + if (ret) { > + dev_dbg(&mhiwwan->mhi_dev->dev, > + "failed to queue DTR/RTS signal: %d\n", ret); > + kfree(dtr_msg); > + } > + > + return ret; > +} > + > +static void mhi_wwan_ctrl_dtr_rts(struct wwan_port *port, unsigned int mdmbits) > +{ > + mhi_wwan_ctrl_send_dtr(wwan_port_get_drvdata(port), mdmbits); > +} > + > static int mhi_wwan_ctrl_start(struct wwan_port *port) > { > struct mhi_wwan_dev *mhiwwan = wwan_port_get_drvdata(port); > @@ -163,6 +249,7 @@ static const struct wwan_port_ops wwan_pops = { > .start = mhi_wwan_ctrl_start, > .stop = mhi_wwan_ctrl_stop, > .tx = mhi_wwan_ctrl_tx, > + .dtr_rts = mhi_wwan_ctrl_dtr_rts, > }; > > static void mhi_ul_xfer_cb(struct mhi_device *mhi_dev, > @@ -255,6 +342,112 @@ static void mhi_wwan_ctrl_remove(struct mhi_device *mhi_dev) > kfree(mhiwwan); > } > > +/* IP_CTRL channel driver, bound separately so the data-port path is untouched */ > +static void mhi_wwan_dtr_ul_xfer_cb(struct mhi_device *mhi_dev, > + struct mhi_result *mhi_result) > +{ > + /* MHI core has done with the buffer, release it */ > + kfree(mhi_result->buf_addr); > +} > + > +/* Requeue the single DL sink buffer so the IP_CTRL DL ring never runs empty. > + * This only runs from the work item, so remove() can stop it before the > + * channel is torn down. A transient mapping failure is retried, other errors > + * mean the channel is going away and the next probe queues a fresh buffer. > + */ > +static void mhi_wwan_dtr_refill_work(struct work_struct *work) > +{ > + struct mhi_wwan_dtr *dtr = container_of(to_delayed_work(work), > + struct mhi_wwan_dtr, rx_refill); > + int ret; > + > + ret = mhi_queue_buf(dtr->mhi_dev, DMA_FROM_DEVICE, dtr->rx_buf, > + sizeof(*dtr->rx_buf), MHI_EOT); > + if (ret == -ENOMEM) { > + dev_warn_ratelimited(&dtr->mhi_dev->dev, > + "failed to requeue IP_CTRL RX buffer, retrying\n"); > + schedule_delayed_work(&dtr->rx_refill, msecs_to_jiffies(100)); > + } else if (ret) { > + dev_dbg(&dtr->mhi_dev->dev, > + "failed to requeue IP_CTRL RX buffer: %d\n", ret); > + } > +} > + > +static void mhi_wwan_dtr_dl_xfer_cb(struct mhi_device *mhi_dev, > + struct mhi_result *mhi_result) > +{ > + struct mhi_wwan_dtr *dtr = dev_get_drvdata(&mhi_dev->dev); > + > + /* Channel is being torn down (e.g. -ENOTCONN), do not requeue */ > + if (mhi_result->transaction_status && > + mhi_result->transaction_status != -EOVERFLOW) > + return; > + > + /* Modem serial state not needed, requeue the sink buffer to keep DL ring live */ > + schedule_delayed_work(&dtr->rx_refill, 0); > +} > + > +static int mhi_wwan_dtr_probe(struct mhi_device *mhi_dev, > + const struct mhi_device_id *id) > +{ > + struct mhi_wwan_dtr *dtr; > + int ret; > + > + dtr = kzalloc_obj(*dtr); > + if (!dtr) > + return -ENOMEM; > + > + dtr->rx_buf = kmalloc_obj(*dtr->rx_buf); > + if (!dtr->rx_buf) { > + ret = -ENOMEM; > + goto err_free_dtr; > + } > + INIT_DELAYED_WORK(&dtr->rx_refill, mhi_wwan_dtr_refill_work); > + > + ret = mhi_prepare_for_transfer(mhi_dev); > + if (ret) > + goto err_free_buf; > + > + dtr->cntrl = mhi_dev->mhi_cntrl; > + dtr->mhi_dev = mhi_dev; > + dev_set_drvdata(&mhi_dev->dev, dtr); > + > + /* On failure mhi_probe() unprepares the channels itself */ > + ret = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, dtr->rx_buf, > + sizeof(*dtr->rx_buf), MHI_EOT); > + if (ret) > + goto err_free_buf; > + > + mutex_lock(&mhi_wwan_dtr_lock); > + list_add(&dtr->node, &mhi_wwan_dtr_list); > + mutex_unlock(&mhi_wwan_dtr_lock); > + > + return 0; > + > +err_free_buf: > + kfree(dtr->rx_buf); > +err_free_dtr: > + kfree(dtr); > + return ret; > +} > + > +static void mhi_wwan_dtr_remove(struct mhi_device *mhi_dev) > +{ > + struct mhi_wwan_dtr *dtr = dev_get_drvdata(&mhi_dev->dev); > + > + mutex_lock(&mhi_wwan_dtr_lock); > + list_del(&dtr->node); > + mutex_unlock(&mhi_wwan_dtr_lock); > + > + /* Stop the refill work, and stop a late DL callback re-arming it, before > + * the channel is torn down so no requeue can race with the teardown > + */ > + disable_delayed_work_sync(&dtr->rx_refill); > + mhi_unprepare_from_transfer(mhi_dev); > + kfree(dtr->rx_buf); > + kfree(dtr); > +} > + > static const struct mhi_device_id mhi_wwan_ctrl_match_table[] = { > { .chan = "DUN", .driver_data = WWAN_PORT_AT }, > { .chan = "DUN2", .driver_data = WWAN_PORT_AT }, > @@ -278,7 +471,45 @@ static struct mhi_driver mhi_wwan_ctrl_driver = { > }, > }; > > -module_mhi_driver(mhi_wwan_ctrl_driver); > +static const struct mhi_device_id mhi_wwan_dtr_match_table[] = { > + { .chan = "IP_CTRL" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(mhi, mhi_wwan_dtr_match_table); > + > +static struct mhi_driver mhi_wwan_dtr_driver = { > + .id_table = mhi_wwan_dtr_match_table, > + .remove = mhi_wwan_dtr_remove, > + .probe = mhi_wwan_dtr_probe, > + .ul_xfer_cb = mhi_wwan_dtr_ul_xfer_cb, > + .dl_xfer_cb = mhi_wwan_dtr_dl_xfer_cb, > + .driver = { > + .name = "mhi_wwan_dtr", > + }, > +}; > + > +static int __init mhi_wwan_ctrl_init(void) > +{ > + int ret; > + > + ret = mhi_driver_register(&mhi_wwan_dtr_driver); > + if (ret) > + return ret; > + > + ret = mhi_driver_register(&mhi_wwan_ctrl_driver); > + if (ret) > + mhi_driver_unregister(&mhi_wwan_dtr_driver); > + > + return ret; > +} > +module_init(mhi_wwan_ctrl_init); > + > +static void __exit mhi_wwan_ctrl_exit(void) > +{ > + mhi_driver_unregister(&mhi_wwan_ctrl_driver); > + mhi_driver_unregister(&mhi_wwan_dtr_driver); > +} > +module_exit(mhi_wwan_ctrl_exit); > > MODULE_LICENSE("GPL v2"); > MODULE_DESCRIPTION("MHI WWAN CTRL Driver"); > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v9 2/2] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel 2026-10-12 17:22 ` [PATCH net-next v9 2/2] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel Peter Hunt 2026-10-13 10:00 ` Loic Poulain @ 2026-10-13 17:23 ` netdev-bot+sashiko 2026-10-13 18:31 ` Peter Hunt 1 sibling, 1 reply; 8+ messages in thread From: netdev-bot+sashiko @ 2026-10-13 17:23 UTC (permalink / raw) To: peter.hunt Cc: loic.poulain, ryazanov.s.a, dnlplm, johannes, mani, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, mhi, linux-arm-msm, linux-kernel Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider. Critical: 0 · High: 0 · Medium: 1 · Low: 1 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [Medium] When a DTR/RTS message fails to send, nothing retries it, and the wwan core's change-only filter stops userspace from re-sending it. - [Low] (Baseline verification: the main trigger is disproven; only a root-only sysfs corner case remains.) The concern says an AT port can… -- net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel This patch makes mhi_wwan_ctrl also bind the IP_CTRL MHI channel and uses it to send the host DTR/RTS state to the modem, so SDX55/SDX65 modems deliver URCs on the AT ports. A per-controller registry maps each AT port to the IP_CTRL device of its own modem, and a new ->dtr_rts op sends the line state, while a recycled DL sink buffer keeps the IP_CTRL DL ring live. > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c > index a31d8540fbb85..1cbbfcb3b920a 100644 > --- a/drivers/net/wwan/mhi_wwan_ctrl.c > +++ b/drivers/net/wwan/mhi_wwan_ctrl.c [ ... ] > @@ -103,6 +134,61 @@ static void mhi_wwan_ctrl_refill_work(struct work_struct *work) > } > } > > +/* Signal the modem's DTR/RTS lines over its own controller's IP_CTRL channel */ > +static int mhi_wwan_ctrl_send_dtr(struct mhi_wwan_dev *mhiwwan, unsigned int mdmbits) > +{ [ ... ] > + if (!ctrl_dev) { > + dev_dbg(&mhiwwan->mhi_dev->dev, > + "IP_CTRL not enumerated; DTR/RTS not signalled to modem\n"); > + return 0; > + } > + > + dtr_msg = kzalloc_obj(*dtr_msg); > + if (!dtr_msg) > + return -ENOMEM; [ ... ] > + ret = mhi_queue_buf(ctrl_dev, DMA_TO_DEVICE, dtr_msg, sizeof(*dtr_msg), > + MHI_EOT); > + if (ret) { > + dev_dbg(&mhiwwan->mhi_dev->dev, > + "failed to queue DTR/RTS signal: %d\n", ret); > + kfree(dtr_msg); > + } > + > + return ret; > +} > + > +static void mhi_wwan_ctrl_dtr_rts(struct wwan_port *port, unsigned int mdmbits) > +{ > + mhi_wwan_ctrl_send_dtr(wwan_port_get_drvdata(port), mdmbits); > +} [Severity: Medium] What happens to the modem's line state when this send fails? mhi_wwan_ctrl_send_dtr() can fail with -ENOMEM from kzalloc_obj(). It can also fail from mhi_queue_buf() with -EAGAIN (UL ring full), -EIO (MHI error state), -ENODEV, or a map_single error. In each case the message is freed, and mhi_wwan_ctrl_dtr_rts() ignores the return value. Nothing retries the send. The DL sink path in this same patch does retry -ENOMEM. The wwan core has already stored the new value in port->at_data.mdmbits before it calls ->dtr_rts: drivers/net/wwan/wwan_core.c:wwan_port_raise_dtr_rts() { ... port->at_data.mdmbits |= TIOCM_DTR | TIOCM_RTS; port->ops->dtr_rts(port, port->at_data.mdmbits); } The TIOCMSET/TIOCMBIC/TIOCMBIS handler only calls the driver when the bits change: drivers/net/wwan/wwan_core.c:wwan_port_fops_at_ioctl() { ... /* 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); ... } So after a lost DTR raise, TIOCMGET reports DTR as asserted, but the modem still sees it de-asserted and keeps withholding +CREG/+DMI. A repeated TIOCMBIS(TIOCM_DTR), or a TIOCMSET with the same bits, never reaches the driver. Only clearing and then setting DTR, or a full last-close/first-open cycle, brings the modem back in step. A long-lived opener such as ModemManager rarely does either. This also seems to conflict with the commit message: the next ioctl or open/close cycle will resynchronise the modem state. That only holds for an ioctl that actually changes DTR or RTS. The companion wwan core commit also says that the TIOCM state and the line state sent to the driver cannot get out of step. Could a failed send be retried from a work item, as the DL refill is? Or could the driver track the last state the modem accepted, so that a later ioctl can resend it? [ ... ] > + mutex_lock(&mhi_wwan_dtr_lock); > + list_add(&dtr->node, &mhi_wwan_dtr_list); > + mutex_unlock(&mhi_wwan_dtr_lock); > + > + return 0; [ ... ] > +static void mhi_wwan_dtr_remove(struct mhi_device *mhi_dev) > +{ > + struct mhi_wwan_dtr *dtr = dev_get_drvdata(&mhi_dev->dev); > + > + mutex_lock(&mhi_wwan_dtr_lock); > + list_del(&dtr->node); > + mutex_unlock(&mhi_wwan_dtr_lock); > + > + /* Stop the refill work, and stop a late DL callback re-arming it, before > + * the channel is torn down so no requeue can race with the teardown > + */ > + disable_delayed_work_sync(&dtr->rx_refill); > + mhi_unprepare_from_transfer(mhi_dev); [Severity: Low] This is a narrow corner case, but can the IP_CTRL device and the open AT ports get out of step across a sysfs unbind and rebind of mhi_wwan_dtr? The normal boot and hot-plug order is fine. mhi_create_devices() probes channels synchronously in channel-number order, so IP_CTRL (18/19) binds before DUN (32/33). mhi_wwan_ctrl_init() also registers mhi_wwan_dtr_driver first. Suppose root unbinds mhi_wwan_dtr while a DUN port is open. Then mhi_wwan_dtr_remove() tears down the channel without first sending DTR/RTS low. The drop on last close finds no ctrl_dev in mhi_wwan_ctrl_send_dtr() and returns 0 without sending anything. On a later rebind, mhi_wwan_dtr_probe() adds the registry entry but does not replay the stored mdmbits of AT ports that are already open. The core filters ioctls that do not change the bits, so TIOCMBIS(TIOCM_DTR) cannot recover this either. The ordering assumption above also does not hold when the async_probe module parameter is used. During mhi_destroy_devices(), IP_CTRL is also removed before DUN, so the final drop does nothing. The modem is being reset or powered off at that point anyway. -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012172219.3668624-1-peter.hunt%40opengear.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v9 2/2] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel 2026-10-13 17:23 ` netdev-bot+sashiko @ 2026-10-13 18:31 ` Peter Hunt 0 siblings, 0 replies; 8+ messages in thread From: Peter Hunt @ 2026-10-13 18:31 UTC (permalink / raw) To: netdev-bot+sashiko Cc: loic.poulain, Daniele Palmas, ryazanov.s.a, johannes, mani, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, mhi, linux-arm-msm, linux-kernel Thanks for the review. [Medium] a failed DTR/RTS send is not retried: agreed, and stress testing confirmed it. With the change-only filter added in v9, rapidly toggling DTR/RTS filled the IP_CTRL UL ring and updates were dropped with -EAGAIN, so the modem could be left with a stale state. In v10 ->dtr_rts only records the requested state and schedules a per-port work item that sends the latest state, so a burst is coalesced into one message, and a send that fails with -EAGAIN or -ENOMEM is retried with whatever state is latest by then. The modem therefore ends up with the final state requested. [Low] sysfs unbind/rebind of mhi_wwan_dtr while an AT port is open: agreed it's a root-only corner case. Reopening the port, or changing DTR or RTS, sends the state again, so I've left it as is. pw-bot: cr ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-13 18:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-12 17:22 [PATCH net-next v9 0/2] net: wwan: support DTR/RTS on AT ports via MHI IP_CTRL Peter Hunt 2026-10-12 17:22 ` [PATCH net-next v9 1/2] net: wwan: core: propagate modem control signals to port drivers Peter Hunt 2026-10-13 17:23 ` netdev-bot+sashiko 2026-10-13 18:28 ` Peter Hunt 2026-10-12 17:22 ` [PATCH net-next v9 2/2] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel Peter Hunt 2026-10-13 10:00 ` Loic Poulain 2026-10-13 17:23 ` netdev-bot+sashiko 2026-10-13 18:31 ` Peter Hunt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®