Re:Re: [PATCH] USB:serial:option Add Foxconn T99W265

["Slark Xiao" <slark_xiao@163.com>]

At 2021-09-20 17:32:26, "Johan Hovold" <johan@kernel.org> wrote:
>On Fri, Sep 17, 2021 at 07:01:06PM +0800, Slark Xiao wrote:
>> Adding support for Foxconn device T99W265 for enumeration with
>> PID 0xe0db.
>> 
>> usb-devices output for 0xe0db
>> T:  Bus=04 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 19 Spd=5000 MxCh= 0
>> D:  Ver= 3.20 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs=  1
>> P:  Vendor=0489 ProdID=e0db Rev=05.04
>> S:  Manufacturer=Microsoft
>> S:  Product=Generic Mobile Broadband Adapter
>> S:  SerialNumber=6c50f452
>> C:  #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=896mA
>> I:  If#=0x0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim
>> I:  If#=0x1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
>> I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
>> I:  If#=0x3 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
>> I:  If#=0x4 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
>> 
>> if0/1: MBIM, if2:Diag, if3:GNSS, if4: Modem
>> 
>> Signed-off-by: Slark Xiao <slark_xiao@163.com>
>
>Thanks for the patch and for including all the necessary details in the
>commit message.
>
>First, a minor style nit: Please add spaces after the ':'s in the patch
>Subject.
>

Thanks for your warm notice. I will mind this in future.

>> ---
>>  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 29c765cc8495..fde599fa2d73 100644
>> --- a/drivers/usb/serial/option.c
>> +++ b/drivers/usb/serial/option.c
>> @@ -2068,6 +2068,8 @@ static const struct usb_device_id option_ids[] = {
>>  	  .driver_info = RSVD(0) | RSVD(1) | RSVD(6) },
>>  	{ USB_DEVICE(0x0489, 0xe0b5),						/* Foxconn T77W968 ESIM */
>>  	  .driver_info = RSVD(0) | RSVD(1) | RSVD(6) },
>> +	{ USB_DEVICE(0x0489, 0xe0db),						/* Foxconn T99W265 MBIM extension*/
>> +	  .driver_info = RSVD(0) | RSVD(1) | RSVD(3) },
>
>If you use USB_DEVICE_INTERFACE_CLASS() instead you don't need to
>explicitly reserve the MBIM interfaces. 
>
>Also, why are you reserving the GNSS interface (e.g. unlike T77W968)?

I just want to keep same style as previous products. That would be more coordinated, I think.
And for GNSS port, it can't be supported with serial driver. It doesn't like a  NMEA port which is using serial driver.
I checked it for T77W968(MBIM mode) and found settings as below:
if0/if1: MBIM, if2: Modem, if3:AT,  if4: NMEA, if5: Diag, if6: GNSS
GNSS is also reserved.

Thanks.
>
>>  	{ USB_DEVICE(0x1508, 0x1001),						/* Fibocom NL668 (IOT version) */
>>  	  .driver_info = RSVD(4) | RSVD(5) | RSVD(6) },
>>  	{ USB_DEVICE(0x2cb7, 0x0104),						/* Fibocom NL678 series */
>
>Johan