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

From: "Slark Xiao" <slark_xiao@163.com>
To: "Johan Hovold" <johan@kernel.org>
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re:Re: Re: [PATCH] USB:serial:option Add Foxconn T99W265
Date: Thu, 23 Sep 2021 19:32:27 +0800 (CST)	[thread overview]
Message-ID: <fed0c65.8937.17c126da334.Coremail.slark_xiao@163.com> (raw)
In-Reply-To: <YUxLKCGd0Icl+Mbv@hovoldconsulting.com>


At 2021-09-23 17:38:48, "Johan Hovold" <johan@kernel.org> wrote:
>[ Please configure your mail client to wrap lines at 72 columns or so. ]
>
>On Wed, Sep 22, 2021 at 09:51:47AM +0800, Slark Xiao wrote:
>> 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>
>
>> >> ---
>> >>  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.
>
>I understand your point, but it's better to use a more specific matching
>rule were possible since it prevents driver core from even trying to
>bind the driver.
>
>Note that for T77W968 we couldn't do so since we needed to bind also to
>non-vendor-class interfaces.
>
>I'll just change this to USB_DEVICE_INTERFACE_CLASS() when applying.
>
>> 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.
>
>Ah, thanks for explaining. I only saw that T77W968 had an NMEA port and
>thought it was the same one.
>
>Now applied with the change mentioned above.
>
>Johan
Thanks for that!