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

[Loic Poulain]

Hi Peter,

On Mon, Oct 26, 2026 at 4:43 PM Peter Hunt <peter.hunt@xxxxxxxxxxxx> wrote:
>
> Hi Loic,
>
> On Sun, Oct 25, 2026 at 3:27 PM Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx> 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.

Right, it's handled correctly. I overlooked the disable_work cleanup
in the remove path.

>
> 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.

Ok thanks for confirming, for now I think we can safely ignore this
corner-case then.

> 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.

Ack.

>
> 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?

Yes b) looks good to me, I guess you can also flush dtr work
unconditionally in mhi_wwan_ctrl_stop as well.

Thanks,
Loic