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

[Jakub Kicinski <kuba@kernel.org>]

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.