Re: [PATCH net-next v2 3/3] net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL channel
Loic Poulain <[email protected]>
| Newsgroups | dev.linux.lists.mhi,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CAFEp6-18g8VOZ00m1D=ncOJd9oHq-r3pwmj9dCnsqw0L9RSu7A@mail.gmail.com> |
On Thu, Aug 6, 2026 at 5:54 PM Peter Hunt <[email protected]> wrote: > > Qualcomm/Sierra SDX55/SDX65 modems (e.g. Sierra EM9291) 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). > > Mirror TTY semantics: an AT port raises DTR/RTS on open and drops them on > close, and the new wwan ->tiocmset op lets userspace change them via > TIOCMSET. This matches the TTY-based USB and legacy PCIe drivers for the > same modems, where DTR is asserted on open. Received device->host serial > state is not needed and is ignored; ->tiocmget is left unimplemented so > the wwan core reports the cached bits. > > Signed-off-by: Peter Hunt <[email protected]> > --- > v2: Use __le32 for mhi_dtr_ctrl_msg fields and cpu_to_le32() on > assignment. Use guard(mutex) in mhi_wwan_ctrl_send_dtr. Return 0 > when IP_CTRL is not enumerated so TIOCMSET does not regress for > devices without IP_CTRL. > > drivers/net/wwan/mhi_wwan_ctrl.c | 183 ++++++++++++++++++++++++++++++- > 1 file changed, 182 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c > index a31d8540fbb8..95f4f0e6e4aa 100644 > --- a/drivers/net/wwan/mhi_wwan_ctrl.c > +++ b/drivers/net/wwan/mhi_wwan_ctrl.c > @@ -1,8 +1,12 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright (c) 2021, Linaro Ltd <[email protected]> */ > #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/wwan.h> > > /* MHI wwan flags */ > @@ -14,6 +18,30 @@ 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; > +}; > + > +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; > @@ -23,6 +51,9 @@ struct mhi_wwan_dev { > unsigned long flags; > size_t mtu; > > + /* AT (DUN) ports raise DTR/RTS on open to receive unsolicited output */ > + bool is_at_port; Do we really need that? the wwan core already has this knowledge and so will only forward/support tiocm ops to AT ports. Also this info can be retrieved from the wwan_port type. > + > /* Protect against concurrent TX and TX-completion (bh) */ > spinlock_t tx_lock; > > @@ -103,6 +134,55 @@ 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) > + return 0; /* IP_CTRL not enumerated; best-effort, not an error */ > + > + 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) > + kfree(dtr_msg); > + > + return ret; > +} > + > +static int mhi_wwan_ctrl_tiocmset(struct wwan_port *port, unsigned int mdmbits) > +{ > + return 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); > @@ -122,6 +202,10 @@ static int mhi_wwan_ctrl_start(struct wwan_port *port) > mhi_wwan_ctrl_refill_work(&mhiwwan->rx_refill); > } > > + /* Raise DTR/RTS on open so the modem forwards unsolicited output */ > + if (mhiwwan->is_at_port) > + mhi_wwan_ctrl_send_dtr(mhiwwan, TIOCM_DTR | TIOCM_RTS); Why not handling this enablement directly in the wwan core? > + > return 0; > } > > @@ -135,6 +219,10 @@ static void mhi_wwan_ctrl_stop(struct wwan_port *port) > > cancel_work_sync(&mhiwwan->rx_refill); > > + /* Drop DTR/RTS on close */ > + if (mhiwwan->is_at_port) > + mhi_wwan_ctrl_send_dtr(mhiwwan, 0); > + > mhi_unprepare_from_transfer(mhiwwan->mhi_dev); > } > > @@ -163,6 +251,7 @@ static const struct wwan_port_ops wwan_pops = { > .start = mhi_wwan_ctrl_start, > .stop = mhi_wwan_ctrl_stop, > .tx = mhi_wwan_ctrl_tx, > + .tiocmset = mhi_wwan_ctrl_tiocmset, > }; > > static void mhi_ul_xfer_cb(struct mhi_device *mhi_dev, > @@ -223,6 +312,7 @@ static int mhi_wwan_ctrl_probe(struct mhi_device *mhi_dev, > > mhiwwan->mhi_dev = mhi_dev; > mhiwwan->mtu = MHI_WWAN_MAX_MTU; > + mhiwwan->is_at_port = (id->driver_data == WWAN_PORT_AT); > INIT_WORK(&mhiwwan->rx_refill, mhi_wwan_ctrl_refill_work); > spin_lock_init(&mhiwwan->tx_lock); > spin_lock_init(&mhiwwan->rx_lock); > @@ -255,6 +345,59 @@ 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); > +} > + > +static void mhi_wwan_dtr_dl_xfer_cb(struct mhi_device *mhi_dev, > + struct mhi_result *mhi_result) > +{ > + /* Modem serial state is not needed, drop it */ > +} > + > +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; > + > + ret = mhi_prepare_for_transfer(mhi_dev); > + if (ret) { > + kfree(dtr); > + return ret; > + } > + > + dtr->cntrl = mhi_dev->mhi_cntrl; > + dtr->mhi_dev = mhi_dev; > + dev_set_drvdata(&mhi_dev->dev, dtr); > + > + 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); > + > + mhi_unprepare_from_transfer(mhi_dev); > + 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 +421,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 >