Re: [PATCH v2] net: usb: qmi_wwan: add Telit Cinterion FE990D50 composition

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: usb: qmi_wwan: add Telit Cinterion FE990D50 composition

Adds a qmi_wwan products[] entry for USB ID 1bc7:0991 interface 0, using
the SET_DTR quirk, for the Telit Cinterion FE990D50 rmnet + tty + ADPL +
adb composition.

> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 94cdb61dca83d..147beaa61340d 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -1360,6 +1360,7 @@ static const struct usb_device_id products[] = {
>  	{QMI_FIXED_INTF(0x1bbb, 0x0203, 2)},	/* Alcatel L800MA */
>  	{QMI_FIXED_INTF(0x2357, 0x0201, 4)},	/* TP-LINK HSUPA Modem MA180 */
>  	{QMI_FIXED_INTF(0x2357, 0x9000, 4)},	/* TP-LINK MA260 */
> +	{QMI_QUIRK_SET_DTR(0x1bc7, 0x0991, 0)}, /* Telit LE910C1-EUX */
                                                   ^^^^^^^^^^^^^^^^^^

[Severity: Low]
Is the device name in this comment the intended one?  The subject line and
the descriptor dump in the changelog both describe 1bc7:0991 as the
FE990D50:

    P:  Vendor=1bc7 ProdID=0991 Rev=06.06
    S:  Manufacturer=Telit Cinterion
    S:  Product=FE990

The comment looks like a copy of the next line, which is the entry for the
real LE910C1-EUX at a different product ID and interface number:

>  	{QMI_QUIRK_SET_DTR(0x1bc7, 0x1031, 3)}, /* Telit LE910C1-EUX */

drivers/usb/serial/option.c already labels the same product ID as the
FE990D50 in option_ids[]:

	{ USB_DEVICE_AND_INTERFACE_INFO(TELIT_VENDOR_ID, 0x0991, 0xff, 0xff, 0x30),	/* Telit FE990D50 (rmnet) */

Would /* Telit FE990D50 */ be a better fit here, matching the existing
naming for the rest of the family in this table (/* Telit FE990A */ for
0x1080 and /* Telit FE990B */ for 0x10b0)?  As it stands, two entries for
different hardware carry the same label, which makes it easy for a later
addition to touch the wrong line.

The match itself looks fine: interface 0 of 0x0991 has bInterfaceProtocol
0x50, which none of the option.c filters (0x30, 0x40, 0x60) claim, so only
the comment appears to need a change.

>  	{QMI_QUIRK_SET_DTR(0x1bc7, 0x1034, 2)}, /* Telit LE910C4-WWX */
>  	{QMI_QUIRK_SET_DTR(0x1bc7, 0x1037, 4)}, /* Telit LE910C4-WWX */
-- 
pw-bot: cr