All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] USB:serial:option Add Foxconn T99W265
@ 2021-09-17 11:01 Slark Xiao
  2021-09-20  9:32 ` Johan Hovold
  0 siblings, 1 reply; 5+ messages in thread
From: Slark Xiao @ 2021-09-17 11:01 UTC (permalink / raw)
  To: johan, gregkh, linux-usb, linux-kernel; +Cc: slark_xiao

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) },
 	{ USB_DEVICE(0x1508, 0x1001),						/* Fibocom NL668 (IOT version) */
 	  .driver_info = RSVD(4) | RSVD(5) | RSVD(6) },
 	{ USB_DEVICE(0x2cb7, 0x0104),						/* Fibocom NL678 series */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] USB:serial:option Add Foxconn T99W265
  2021-09-17 11:01 [PATCH] USB:serial:option Add Foxconn T99W265 Slark Xiao
@ 2021-09-20  9:32 ` Johan Hovold
  2021-09-22  1:51   ` Slark Xiao
  0 siblings, 1 reply; 5+ messages in thread
From: Johan Hovold @ 2021-09-20  9:32 UTC (permalink / raw)
  To: Slark Xiao; +Cc: gregkh, linux-usb, linux-kernel

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.

> ---
>  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)?

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

Johan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re:Re: [PATCH] USB:serial:option Add Foxconn T99W265
  2021-09-20  9:32 ` Johan Hovold
@ 2021-09-22  1:51   ` Slark Xiao
  2021-09-23  9:38     ` Johan Hovold
  0 siblings, 1 reply; 5+ messages in thread
From: Slark Xiao @ 2021-09-22  1:51 UTC (permalink / raw)
  To: Johan Hovold; +Cc: gregkh, linux-usb, linux-kernel



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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Re: [PATCH] USB:serial:option Add Foxconn T99W265
  2021-09-22  1:51   ` Slark Xiao
@ 2021-09-23  9:38     ` Johan Hovold
  2021-09-23 11:32       ` Slark Xiao
  0 siblings, 1 reply; 5+ messages in thread
From: Johan Hovold @ 2021-09-23  9:38 UTC (permalink / raw)
  To: Slark Xiao; +Cc: gregkh, linux-usb, linux-kernel

[ 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re:Re: Re: [PATCH] USB:serial:option Add Foxconn T99W265
  2021-09-23  9:38     ` Johan Hovold
@ 2021-09-23 11:32       ` Slark Xiao
  0 siblings, 0 replies; 5+ messages in thread
From: Slark Xiao @ 2021-09-23 11:32 UTC (permalink / raw)
  To: Johan Hovold; +Cc: gregkh, linux-usb, linux-kernel


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!

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-09-23 12:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-17 11:01 [PATCH] USB:serial:option Add Foxconn T99W265 Slark Xiao
2021-09-20  9:32 ` Johan Hovold
2021-09-22  1:51   ` Slark Xiao
2021-09-23  9:38     ` Johan Hovold
2021-09-23 11:32       ` Slark Xiao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.