[PATCH net v2] net: wwan: mhi_wwan_ctrl: send a DTR drop before it can be coalesced

[PATCH net v2] net: wwan: mhi_wwan_ctrl: send a DTR drop before it can be coalesced

[Peter Hunt]

DTR/RTS changes are sent from a work item that only sends the latest
state, so a drop followed quickly by a raise, for example TIOCMBIC then
TIOCMBIS or a quick close and reopen, can reach the modem as just the
raise. A drop matters on its own, a modem set to AT&D2 hangs up on it.

Flush the work when DTR goes low so the drop is sent before ->dtr_rts()
returns, and flush it in stop() so any pending state is sent before the
channel is stopped. A drop that hits a full UL ring is still retried
with whatever state is latest by then.

Fixes: d509edf90bc6 ("net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel")
Suggested-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Signed-off-by: Peter Hunt <peter.hunt@opengear.com>
---
v2: Only handle the DTR drop, by flushing the work when DTR goes low,
    and flush it in stop() as well (Loic Poulain). Drop the port list
    and resend on a late IP_CTRL bind, which is only reachable by
    unbinding and rebinding mhi_wwan_dtr through sysfs.
v1: https://lore.kernel.org/all/20261023174113.2388353-1-peter.hunt@opengear.com/

Tested on a Sierra EM9291 (CM8100): back-to-back DTR drop+raise and
close+reopen now always send the drop, bursts of 10,000 DTR changes,
HUPCL, teardown and lock stress tests.

 drivers/net/wwan/mhi_wwan_ctrl.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
index e3c207588956..772319927867 100644
--- a/drivers/net/wwan/mhi_wwan_ctrl.c
+++ b/drivers/net/wwan/mhi_wwan_ctrl.c
@@ -209,9 +209,17 @@ static void mhi_wwan_ctrl_dtr_work(struct work_struct *work)
 static void mhi_wwan_ctrl_dtr_rts(struct wwan_port *port, unsigned int mdmbits)
 {
 	struct mhi_wwan_dev *mhiwwan = wwan_port_get_drvdata(port);
+	bool drop = (READ_ONCE(mhiwwan->dtr_mdmbits) & TIOCM_DTR) &&
+		    !(mdmbits & TIOCM_DTR);
 
 	WRITE_ONCE(mhiwwan->dtr_mdmbits, mdmbits);
 	schedule_delayed_work(&mhiwwan->dtr_work, 0);
+
+	/* A DTR drop matters on its own, e.g. to hang up a modem set to AT&D2,
+	 * so send it now rather than let a following raise coalesce it away
+	 */
+	if (drop)
+		flush_delayed_work(&mhiwwan->dtr_work);
 }
 
 static int mhi_wwan_ctrl_start(struct wwan_port *port)
@@ -246,6 +254,9 @@ static void mhi_wwan_ctrl_stop(struct wwan_port *port)
 
 	cancel_work_sync(&mhiwwan->rx_refill);
 
+	/* Send any pending DTR/RTS state before the channel stops */
+	flush_delayed_work(&mhiwwan->dtr_work);
+
 	mhi_unprepare_from_transfer(mhiwwan->mhi_dev);
 }
 
-- 
2.43.0

Re: [PATCH net v2] net: wwan: mhi_wwan_ctrl: send a DTR drop before it can be coalesced

[Loic Poulain]

On Tue, Oct 27, 2026 at 6:11 PM Peter Hunt <peter.hunt@opengear.com> wrote:
>
> DTR/RTS changes are sent from a work item that only sends the latest
> state, so a drop followed quickly by a raise, for example TIOCMBIC then
> TIOCMBIS or a quick close and reopen, can reach the modem as just the
> raise. A drop matters on its own, a modem set to AT&D2 hangs up on it.
>
> Flush the work when DTR goes low so the drop is sent before ->dtr_rts()
> returns, and flush it in stop() so any pending state is sent before the
> channel is stopped. A drop that hits a full UL ring is still retried
> with whatever state is latest by then.
>
> Fixes: d509edf90bc6 ("net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel")
> Suggested-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Signed-off-by: Peter Hunt <peter.hunt@opengear.com>

Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

> ---
> v2: Only handle the DTR drop, by flushing the work when DTR goes low,
>     and flush it in stop() as well (Loic Poulain). Drop the port list
>     and resend on a late IP_CTRL bind, which is only reachable by
>     unbinding and rebinding mhi_wwan_dtr through sysfs.
> v1: https://lore.kernel.org/all/20261023174113.2388353-1-peter.hunt@opengear.com/
>
> Tested on a Sierra EM9291 (CM8100): back-to-back DTR drop+raise and
> close+reopen now always send the drop, bursts of 10,000 DTR changes,
> HUPCL, teardown and lock stress tests.
>
>  drivers/net/wwan/mhi_wwan_ctrl.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
> index e3c207588956..772319927867 100644
> --- a/drivers/net/wwan/mhi_wwan_ctrl.c
> +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> @@ -209,9 +209,17 @@ static void mhi_wwan_ctrl_dtr_work(struct work_struct *work)
>  static void mhi_wwan_ctrl_dtr_rts(struct wwan_port *port, unsigned int mdmbits)
>  {
>         struct mhi_wwan_dev *mhiwwan = wwan_port_get_drvdata(port);
> +       bool drop = (READ_ONCE(mhiwwan->dtr_mdmbits) & TIOCM_DTR) &&
> +                   !(mdmbits & TIOCM_DTR);
>
>         WRITE_ONCE(mhiwwan->dtr_mdmbits, mdmbits);
>         schedule_delayed_work(&mhiwwan->dtr_work, 0);
> +
> +       /* A DTR drop matters on its own, e.g. to hang up a modem set to AT&D2,
> +        * so send it now rather than let a following raise coalesce it away
> +        */
> +       if (drop)
> +               flush_delayed_work(&mhiwwan->dtr_work);
>  }
>
>  static int mhi_wwan_ctrl_start(struct wwan_port *port)
> @@ -246,6 +254,9 @@ static void mhi_wwan_ctrl_stop(struct wwan_port *port)
>
>         cancel_work_sync(&mhiwwan->rx_refill);
>
> +       /* Send any pending DTR/RTS state before the channel stops */
> +       flush_delayed_work(&mhiwwan->dtr_work);
> +
>         mhi_unprepare_from_transfer(mhiwwan->mhi_dev);
>  }
>
> --
> 2.43.0
>

Re: [PATCH net v2] net: wwan: mhi_wwan_ctrl: send a DTR drop before it can be coalesced

[netdev-bot+sashiko]

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] The fix still loses a DTR drop when the transport refuses it.
- [Low] Two existing comments in drivers/net/wwan/mhi_wwan_ctrl.c no
  longer match the code after this patch.

--

net: wwan: mhi_wwan_ctrl: send a DTR drop before it can be coalesced

This patch flushes the DTR/RTS work item in mhi_wwan_ctrl_dtr_rts() when
DTR goes from high to low. The aim is to keep a following raise from
coalescing the drop away. It also flushes the work in mhi_wwan_ctrl_stop()
so that pending state is sent before the channel is stopped.

> diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
> index e3c2075889565..7723199278671 100644
> --- a/drivers/net/wwan/mhi_wwan_ctrl.c
> +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> @@ -209,9 +209,17 @@ static void mhi_wwan_ctrl_dtr_work(struct work_struct *work)
>  static void mhi_wwan_ctrl_dtr_rts(struct wwan_port *port, unsigned int mdmbits)
>  {
>  	struct mhi_wwan_dev *mhiwwan = wwan_port_get_drvdata(port);
> +	bool drop = (READ_ONCE(mhiwwan->dtr_mdmbits) & TIOCM_DTR) &&
> +		    !(mdmbits & TIOCM_DTR);
>  
>  	WRITE_ONCE(mhiwwan->dtr_mdmbits, mdmbits);
>  	schedule_delayed_work(&mhiwwan->dtr_work, 0);
> +
> +	/* A DTR drop matters on its own, e.g. to hang up a modem set to AT&D2,
> +	 * so send it now rather than let a following raise coalesce it away
> +	 */
> +	if (drop)
> +		flush_delayed_work(&mhiwwan->dtr_work);
>  }

[Severity: Medium]
Can the drop still be lost if the first send attempt fails? On a transient
error, mhi_wwan_ctrl_dtr_work() re-arms itself:

    ret = mhi_wwan_ctrl_send_dtr(mhiwwan, READ_ONCE(mhiwwan->dtr_mdmbits));
    if (ret == -EAGAIN || ret == -ENOMEM)
        schedule_delayed_work(&mhiwwan->dtr_work, msecs_to_jiffies(10));

mhi_queue() returns -EAGAIN when the IP_CTRL UL ring is full. -ENOMEM comes
from the dtr_msg allocation in mhi_wwan_ctrl_send_dtr().

flush_delayed_work() only waits for the instance it queued itself. Its
kernel-doc says it "only considers the last queueing instance". So it
returns after one failed attempt, and the 10ms retry timer is still armed.

>From userspace on the AT port, the sequence would be:

TIOCMBIC(DTR)
  mhi_wwan_ctrl_dtr_rts(port, 0)
    drop = true
    flush_delayed_work()
      mhi_wwan_ctrl_dtr_work() -> -EAGAIN, re-arms 10ms timer

TIOCMBIS(DTR)
  mhi_wwan_ctrl_dtr_rts(port, DTR|RTS)
    drop = false, since dtr_mdmbits is already 0
    WRITE_ONCE(mhiwwan->dtr_mdmbits, DTR|RTS)
    schedule_delayed_work(..., 0) is a no-op, work is already pending

10ms later
  mhi_wwan_ctrl_dtr_work() sends DTR|RTS

Wouldn't the modem then see only the raise? That is the case this patch is
meant to fix. drop is computed against the last requested state, not the
last state that was actually sent.

The commit message says a drop that hits a full UL ring "is still retried
with whatever state is latest by then". The new comment says the drop is
sent "now rather than let a following raise coalesce it away".

Would it be better to keep track of an unsent drop until the transport
accepts it? One option is a drop-pending flag that dtr_work sends before the
latest state. Otherwise, the comment could say this is best effort.

[ ... ]

> @@ -246,6 +254,9 @@ static void mhi_wwan_ctrl_stop(struct wwan_port *port)
>  
>  	cancel_work_sync(&mhiwwan->rx_refill);
>  
> +	/* Send any pending DTR/RTS state before the channel stops */
> +	flush_delayed_work(&mhiwwan->dtr_work);
> +
>  	mhi_unprepare_from_transfer(mhiwwan->mhi_dev);
>  }

The same limitation applies to this flush. After one failed attempt,
mhi_unprepare_from_transfer() runs while the 10ms retry is still armed.

On a plain last close nothing cancels that retry. disable_delayed_work_sync()
is only called from mhi_wwan_ctrl_remove().

Could the DTR message then be queued after the channel named in its dest_id
has been stopped? Could it race with a later reopen?

[Severity: Low]
This isn't a bug, but two existing comments in mhi_wwan_ctrl.c no longer
match the code after this change.

The struct mhi_wwan_dev comment still describes pure coalescing:

	/* Latest DTR/RTS state for the modem, sent from dtr_work so a burst of
	 * changes is coalesced and only the final state has to get through
	 */

mhi_wwan_ctrl_dtr_rts() now flushes each DTR drop so that it is not
coalesced.

mhi_wwan_ctrl_remove() has this comment:

	wwan_remove_port(mhiwwan->wwan_port);
	/* Give the DTR/RTS drop from removal one attempt, then stop the work */
	flush_delayed_work(&mhiwwan->dtr_work);

When start_count is set, wwan_remove_port() calls
wwan_port_drop_dtr_rts(port, false) and then port->ops->stop(). After this
patch both of those paths flush dtr_work, so a removal drop can be tried up
to three times.

Should these two comments be updated to match?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261027171055.1796482-1-peter.hunt%40opengear.com

Re: [PATCH net v2] net: wwan: mhi_wwan_ctrl: send a DTR drop before it can be coalesced

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Dave Miller <davem@davem-thinkpadp1gen8.ibmlond.csb>:

On Tue, 27 Oct 2026 11:10:55 -0600 you wrote:
> DTR/RTS changes are sent from a work item that only sends the latest
> state, so a drop followed quickly by a raise, for example TIOCMBIC then
> TIOCMBIS or a quick close and reopen, can reach the modem as just the
> raise. A drop matters on its own, a modem set to AT&D2 hangs up on it.
> 
> Flush the work when DTR goes low so the drop is sent before ->dtr_rts()
> returns, and flush it in stop() so any pending state is sent before the
> channel is stopped. A drop that hits a full UL ring is still retried
> with whatever state is latest by then.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: wwan: mhi_wwan_ctrl: send a DTR drop before it can be coalesced
    https://git.kernel.org/netdev/net/c/10f51f4fb99d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net v2] net: wwan: mhi_wwan_ctrl: send a DTR drop before it can be coalesced

[Peter Hunt]

Thanks for the review. This has since been applied to net, so for the
record:

[Medium] Drop lost when the transport refuses it: right, a drop that
hits a full UL ring (or a failed allocation) and is followed by a raise
within the 10ms retry still reaches the modem as just the raise. The
commit message calls this out, but the new comment overstates it.
Tracking an unsent drop until it is queued is roughly what v1 did, and
Loic found that too heavy for this case, so the flush stays best effort.

On the flush in stop(), a retry left armed after a failed attempt can
fire after mhi_unprepare_from_transfer(). That isn't new, v10 had no
flush in stop() at all, so the work could already run after the channel
stopped. The message goes out on the IP_CTRL channel, which is still
running, the AT channel only appears as dest_id. If the port is
reopened before the retry fires, the retry sends the latest state, i.e.
the raise from the reopen, so the modem ends up in the right state.

[Low] Stale comments: agreed. I'll send a comment-only patch for
net-next that says the drop flush is best effort, mentions it in the
dtr_mdmbits comment and drops "one attempt" from the remove() comment.