Re: [PATCH] USB: serial: option: Add support for ZTE MF871A

[Johan Hovold <johan@kernel.org>]

On Thu, Jul 11, 2019 at 03:53:24PM +0900, Yoshiaki Okamoto wrote:
> This patch adds support for MF871A USB modem (aka Speed USB STICK U03)
> to option driver. This modem is manufactured by ZTE corporation, and
> sold by KDDI.
> 
> Interface layout:
> 0: AT
> 1: MODEM
> 
> usb-devices output:
> T:  Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  9 Spd=480 MxCh= 0
> D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> P:  Vendor=19d2 ProdID=1481 Rev=52.87
> S:  Manufacturer=ZTE,Incorporated
> S:  Product=ZTE Technologies MSM
> S:  SerialNumber=1234567890ABCDEF
> C:  #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
> I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> I:  If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option

Thanks for the patch. Looks good, but please fix up the two minor issues
pointed out below and resend, and I'll apply it after the merge window
closes.
 
> Signed-off-by: Yoshiaki Okamoto <yokamoto@allied-telesis.co.jp>
> Signed-off-by: Hiroyuki Yamamoto <hyamamo@allied-telesis.co.jp>

Since you are the one submitting the patch your SoB should go last. We
have a Co-developed-by tag which can you use to indicate co-authorship
(the second SoB is still required). The documentation for this was
recently updated in commit

	24a2bb90741b ("docs: Clarify the usage and sign-off requirements for Co-developed-by")

> ---
>  drivers/usb/serial/option.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index a0aaf0635359..e11ae2092229 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -308,6 +308,7 @@ static void option_instat_callback(struct urb *urb);
>  #define ZTE_PRODUCT_ME3620_MBIM			0x0426
>  #define ZTE_PRODUCT_ME3620_X			0x1432
>  #define ZTE_PRODUCT_ME3620_L			0x1433
> +#define ZTE_PRODUCT_MF871A			0x1481
>  #define ZTE_PRODUCT_AC2726			0xfff1
>  #define ZTE_PRODUCT_MG880			0xfffd
>  #define ZTE_PRODUCT_CDMA_TECH			0xfffe
> @@ -1548,6 +1549,7 @@ static const struct usb_device_id option_ids[] = {
>  	{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x1428, 0xff, 0xff, 0xff),  /* Telewell TW-LTE 4G v2 */
>  	  .driver_info = RSVD(2) },
>  	{ USB_DEVICE_INTERFACE_CLASS(ZTE_VENDOR_ID, 0x1476, 0xff) },	/* GosunCn ZTE WeLink ME3630 (ECM/NCM mode) */
> +	{ USB_DEVICE_INTERFACE_CLASS(ZTE_VENDOR_ID, ZTE_PRODUCT_MF871A, 0xff) },

We're trying to move away from adding product-id defines, so please just
use a constant here as most ZTE entries do and add a short comment after
the entry. It's fine to go above 80 columns here even if checkpatch.pl
complains.

>  	{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x1533, 0xff, 0xff, 0xff) },
>  	{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x1534, 0xff, 0xff, 0xff) },
>  	{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x1535, 0xff, 0xff, 0xff) },

Thanks,
Johan