Re: [PATCH net-next v3 2/3] bus: mhi: host: pci_generic: enumerate IP_CTRL channel for Sierra EM919x/EM929x

Jakub Kicinski <[email protected]>
Newsgroups dev.linux.lists.mhi,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
bus: mhi: host: pci_generic: enumerate IP_CTRL channel for Sierra EM919x/EM929x

This adds the standard Qualcomm MHI control channels 18/19 ("IP_CTRL") to
the Sierra em919x channel configuration, using the control event ring.
The stated purpose is to let the host drive DTR/RTS towards the modem.

> Enumerate it in the Sierra em919x channel config (shared by the EM919x
> and EM929x) using the control event ring. This is the device the change
> was tested against; other Qualcomm modem configs that expose IP_CTRL can
> enable it the same way, since the mhi_wwan_ctrl DTR support that consumes
> it is device-agnostic and binds purely by channel name.

[Severity: Low]
This isn't a bug, but the phrase "the mhi_wwan_ctrl DTR support that
consumes it is device-agnostic and binds purely by channel name" is written
in the present tense, while that consumer only appears later in the same
series.

At this commit, drivers/net/wwan/mhi_wwan_ctrl.c has no IP_CTRL entry in its
mhi_device_id match table and no ->dtr_rts implementation in wwan_pops; both
arrive with "net: wwan: mhi_wwan_ctrl: drive DTR/RTS via the IP_CTRL
channel".

Would it be clearer to phrase this as support added by a following patch in
the series?

For the record, the intermediate state looks harmless: the newly enumerated
channels only create mhi_device instances with no bound driver, and such a
channel is never started because mhi_prepare_for_transfer() is only called
by a client. pci_generic already enumerates channels with no in-tree
consumer for a given config, for example "IPCR" and "IP_SW0".

> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 0d0d9c7ffa4b..ba47dd153837 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -728,6 +728,8 @@ static const struct mhi_channel_config mhi_sierra_em919x_channels[] = {
>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 128, 0),
>  	MHI_CHANNEL_CONFIG_UL(14, "QMI", 32, 0),
>  	MHI_CHANNEL_CONFIG_DL(15, "QMI", 32, 0),
> +	MHI_CHANNEL_CONFIG_UL(18, "IP_CTRL", 32, 0),
> +	MHI_CHANNEL_CONFIG_DL(19, "IP_CTRL", 32, 0),
>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0", 512, 1),

Cross-instance finding from sashiko-gemini (7294b6d42c500c3db0e0cd5f3fe50b00b9c09cbd890618c9d32841fb863f7603):
[Severity: High]
Unchecked dereference of `port->ops` in `wwan_port_fops_at_ioctl` can cause a kernel panic during device hot-unplug.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.