All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jörgen Storvist" <jorgen.storvist@gmail.com>
To: Johan Hovold <johan@kernel.org>
Cc: Lars Melin <larsm17@gmail.com>, linux-usb@vger.kernel.org
Subject: USB: serial: option: add Fibocom NL668 series
Date: Tue, 11 Dec 2018 14:22:50 +0100	[thread overview]
Message-ID: <20181211142250.00005b1d@gmail.com> (raw)

Den Tue, 11 Dec 2018 09:32:36 +0100
skrev Re: [PATCH] USB: serial: option: add Fibocom NL668 series:

> On Tue, Dec 11, 2018 at 08:41:24AM +0100, Jörgen Storvist wrote:
> > 
> > Added USB serial option driver support for Fibocom NL668 series cellular modules.
> > Reserved USB endpoints 4, 5 and 6 for network + ADB interface.
> > 
> > Signed-off-by: Jörgen Storvist <jorgen.storvist@gmail.com>
> > ---
> > 
> > Thanks for feedback!
> > Changes:
> > Removed name declarations for VID/PID
> > Added reserved endpoint for ADB interface
> >
> > usb-devices
> > T:  Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  7 Spd=480 MxCh= 0
> > D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> > P:  Vendor=1508 ProdID=1001 Rev=03.18
> > S:  Manufacturer=Nodecom NL668 Modem
> > S:  Product=Nodecom NL668-CN Modem
> > S:  SerialNumber=5ced6a52
> > C:  #Ifs= 7 Cfg#= 1 Atr=a0 MxPwr=500mA
> > I:  If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > I:  If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > I:  If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > I:  If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > I:  If#= 4 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether
> > I:  If#= 5 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
> > I:  If#= 6 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none)  
> 
> Thanks for the update and usb-devices info (you can even put this in the
> changelog).
> 
> >  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 e24ff16..ca3e398 100644
> > --- a/drivers/usb/serial/option.c
> > +++ b/drivers/usb/serial/option.c
> > @@ -1941,6 +1941,8 @@ static const struct usb_device_id option_ids[] = {
> >  	{ USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, WETELECOM_PRODUCT_6802, 0xff, 0xff, 0xff) },
> >  	{ USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, WETELECOM_PRODUCT_WMD300, 0xff, 0xff, 0xff) },
> >  	{ USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x421d, 0xff, 0xff, 0xff) }, /* HP lt2523 (Novatel E371) */
> > +	{ USB_DEVICE(0x1508, 0x1001), /* Fibocom NL668 series */
> > +	  .driver_info = RSVD(4) | RSVD(5) | RSVD(6) },  
> 
> Looks like you can use USB_DEVICE_INTERFACE_CLASS() to match on the
> vendor class instead of blacklisting interface 4 and 5.
> 
> A more specific match is generally preferred over blacklisting (which
> adds some overhead).
> 
> Care to address that in a v3? Remember to include the patch revision in
> the subject line as well (e.g. "[PATCH v3] USB: ...").
> 
> Thanks,
> Johan

It seems it would become problematic then if we change USB mode on the module to 
QMI instead of ECM network interface as they share same VID/PID value.

Or is there other way to still get endpoints 0-3 bound to option driver on class / interface info?
Subclass is different on the first diagnostical serial interface so USB_DEVICE_AND_INTERFACE_INFO() wouldn't be successful then either?

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=1508 ProdID=1001 Rev=03.18
S:  Manufacturer=Nodecom NL668 Modem
S:  Product=Nodecom NL668-CN Modem
S:  SerialNumber=5ced6a52
C:  #Ifs= 6 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option	(Serial)
I:  If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option	(Serial)
I:  If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option	(Serial)
I:  If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option	(Serial)
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)	(QMI/RMNET)
I:  If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none)	(ADB)

Thanks,
Jörgen

             reply	other threads:[~2018-12-11 13:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-11 13:22 Jörgen Storvist [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-12-12 15:57 USB: serial: option: add Fibocom NL668 series Johan Hovold
2018-12-11  8:32 Johan Hovold
2018-12-11  7:41 Jörgen Storvist
2018-12-10  7:16 Greg Kroah-Hartman
2018-12-09 20:41 Jörgen Storvist

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181211142250.00005b1d@gmail.com \
    --to=jorgen.storvist@gmail.com \
    --cc=johan@kernel.org \
    --cc=larsm17@gmail.com \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.