linux-kernel - Re: Re: [PATCH] USB:serial:option Add Foxconn T99W265

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YUxLKCGd0Icl+Mbv@hovoldconsulting.com>
Date:   Thu, 23 Sep 2021 11:38:48 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Slark Xiao <slark_xiao@....com>
Cc:     gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: Re: [PATCH] USB:serial:option Add Foxconn T99W265

[ 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@...nel.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@....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