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@xxxxxxxxx>
> > ---
> >
> > 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
