Re: [PATCH net] net: wwan: mhi_wwan_ctrl: keep DTR drops and resend state when IP_CTRL binds

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

Hi Loic,

On Sun, Oct 25, 2026 at 3:27 PM Loic Poulain <loic.poulain@oss.qualcomm.com> wrote:
> This looks a bit over-engineered for the reported Sashiko corner
> cases, and adds quite a bit of complexity. You'll also need to handle
> the lifetime of dtr_work and ensure it cannot run after/during device
> removal.

Thanks for looking at it. On the lifetime, I think it's covered, but
please tell me if I've missed a path. dtr_work can only be queued from:

- ->dtr_rts(), which the wwan core stops calling once wwan_remove_port()
  has cleared port->ops under ops_lock,
- dtr_work itself, re-arming on -EAGAIN/-ENOMEM,
- mhi_wwan_dtr_probe() (new in this patch), which only walks the port
  list under mhi_wwan_dtr_lock, and remove() takes the port off that
  list under the same lock first.

After that, remove() flushes the work once and calls
disable_delayed_work_sync(), which cancels a pending retry and stops it
re-arming, before the kfree(). On the IP_CTRL side, the work only finds
the channel through mhi_wwan_dtr_list under mhi_wwan_dtr_lock, and
mhi_wwan_dtr_remove() unlists it under that lock before unpreparing.

> I don't have a better solution to propose at the moment, but this
> problem suggests that we should enforce some level of ordering between
> the MHI devices (channels), or introduce an explicit dependency/link
> between them.

On ordering, here's what I found. On the EM919x/EM929x IP_CTRL is
channels 18/19 and DUN is 32/33, mhi_create_devices() adds the devices
in channel order, and module init registers mhi_wwan_dtr before
mhi_wwan_ctrl. So whether the controller powers up with the module
loaded or the module loads afterwards, IP_CTRL already binds before the
AT port. As far as I can tell the late bind case is only reachable by
unbinding and rebinding mhi_wwan_dtr through sysfs. What I don't know is
whether that's worth an explicit device link, or whether other modems
would declare IP_CTRL after their AT channels.

The DTR drop case is different, it doesn't need any unusual setup. With
v10, a back-to-back drop and raise from userspace (TIOCMBIC then
TIOCMBIS, or a quick close and reopen) was mostly coalesced into just
the raise in my tests, so the modem never saw DTR go low.

So I see three options:

a) Drop this patch and leave both cases as known limitations.
b) A much smaller v2 that only handles the drop: when DTR goes low,
   ->dtr_rts() flushes dtr_work so the drop is sent before it returns
   and a following raise can't overtake it. No new list, flag or
   field, and the late bind case is left to the existing ordering.
c) Keep this patch as it is.

I'd lean towards b), but which would you prefer?

Thanks,
Peter