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

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

Hi Loic,

Thanks for the review.

On Thu, Oct 8, 2026 at 5:19 PM Loic Poulain wrote:
>> +               if (port->start_count == 1)
>
> You basically have the same check above

Agreed, v9 raises DTR/RTS inside the existing first-open branch, once
->start() has succeeded.

>> +               if (port->type == WWAN_PORT_AT)
>> +                       call_dtr_rts = true;
>
> Any reason not to just call dtr_rts() whenever the callback is
> implemented?

wwan_port_fops_at_ioctl() serves both AT and QCDM ports. DTR/RTS only
means something on the AT (DUN) port of these modems, QCDM is the DIAG
channel and has no DTR semantics. Open, close and removal raise and drop
the lines for AT ports only, so I kept the ioctl path on the same rule.
Review of v3 flagged the opposite mismatch, where a QCDM port could have
DTR raised through TIOCMSET but never dropped on close.

If there is a case you have in mind where a non-AT port driver would
want ->dtr_rts, I'm happy to drop the type check, but I'd do it in all
four places (open, close, removal and ioctl) so they stay consistent. Is
there something I haven't thought of?

> So now we end up with two mutexes essentially protecting the same
> state. Since dtr_rts() is always called under ops_lock, and the ioctl
> path acquires both ops_lock and data_lock, would it make sense to drop
> data_lock altogether and rely solely on ops_lock?

I looked at this, and my concern is blocking writes. wwan_port_op_tx()
holds ops_lock across the driver's ->tx_blocking(), and
rpmsg_wwan_ctrl implements that with rpmsg_send(), which can sleep until
the remote has space. If the termios and TIOCM state moved under
ops_lock, TCGETS, TIOCMGET and the rest of the AT/QCDM ioctls on those
ports would wait behind a stuck write. Today they only take data_lock
and return straight away. The automated review of v7 raised the same
point about the TIOCM path taking ops_lock.

What I'd suggest instead is to keep data_lock for the termios/TIOCM
state, and only take ops_lock (with mutex_lock_interruptible()) for the
->dtr_rts call itself. v9 already reduces how often that happens, as it
only calls ->dtr_rts when DTR or RTS actually changes. Would that work
for you, or would you still prefer a single lock?

Thanks,
Peter