USB: serial: option: add Fibocom NL668 series (fixed line break issue)

USB: serial: option: add Fibocom NL668 series (fixed line break issue)

[Jörgen Storvist]

Added USB serial option driver support for Fibocom NL668 series cellular module.
Reserved USB endpoints 4 and 5 for network interfaces.

Signed-off-by: Jörgen Storvist <jorgen.storvist@gmail.com>

drivers/usb/serial/option.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index e24ff16..f2cc85c 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -552,6 +552,9 @@ static void option_instat_callback(struct urb *urb);
 #define WETELECOM_PRODUCT_6802			0x6802
 #define WETELECOM_PRODUCT_WMD300		0x6803
 
+/* Fibocom products */
+#define FIBOCOM_VENDOR_ID   			0x1508
+#define FIBOCOM_PRODUCT_NL668   		0x1001
 
 /* Device flags */
 
@@ -1941,6 +1944,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(FIBOCOM_VENDOR_ID, FIBOCOM_PRODUCT_NL668), /* Fibocom NL668 series */
+	  .driver_info = RSVD(4) | RSVD(5) },
 	{ } /* Terminating entry */
 };
 MODULE_DEVICE_TABLE(usb, option_ids);

USB: serial: option: add Fibocom NL668 series (fixed line break issue)

[Lars Melin]

On 12/10/2018 06:21, Jörgen Storvist wrote:
> 
> Added USB serial option driver support for Fibocom NL668 series cellular module.
> Reserved USB endpoints 4 and 5 for network interfaces.
> 
> Signed-off-by: Jörgen Storvist <jorgen.storvist@gmail.com>
> 
> drivers/usb/serial/option.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index e24ff16..f2cc85c 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -552,6 +552,9 @@ static void option_instat_callback(struct urb *urb);
>   #define WETELECOM_PRODUCT_6802			0x6802
>   #define WETELECOM_PRODUCT_WMD300		0x6803
>   
> +/* Fibocom products */
> +#define FIBOCOM_VENDOR_ID   			0x1508
> +#define FIBOCOM_PRODUCT_NL668   		0x1001
>   
>   /* Device flags */
>   
> @@ -1941,6 +1944,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(FIBOCOM_VENDOR_ID, FIBOCOM_PRODUCT_NL668), /* Fibocom NL668 series */
> +	  .driver_info = RSVD(4) | RSVD(5) },
>   	{ } /* Terminating entry */
>   };
>   MODULE_DEVICE_TABLE(usb, option_ids);
> 


Listed owner of the 0x1508 VID is "MAATEL" which probably is a customer 
or subsidiary of FIBOCOM (which has the VID 0x2cb7).
Please don't do this value-to-name conversion, it is completely useless 
and in this case most likely wrong as well.
Just skip the declaration part and use the VID and PID values directly 
in the device structure list.

rgds
/Lars

USB: serial: option: add Fibocom NL668 series (fixed line break issue)

[Johan Hovold]

On Mon, Dec 10, 2018 at 01:54:14PM +0700, Lars Melin wrote:
> On 12/10/2018 06:21, Jörgen Storvist wrote:
> > 
> > Added USB serial option driver support for Fibocom NL668 series cellular module.
> > Reserved USB endpoints 4 and 5 for network interfaces.
> > 
> > Signed-off-by: Jörgen Storvist <jorgen.storvist@gmail.com>
> > 
> > drivers/usb/serial/option.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> > index e24ff16..f2cc85c 100644
> > --- a/drivers/usb/serial/option.c
> > +++ b/drivers/usb/serial/option.c
> > @@ -552,6 +552,9 @@ static void option_instat_callback(struct urb *urb);
> >   #define WETELECOM_PRODUCT_6802			0x6802
> >   #define WETELECOM_PRODUCT_WMD300		0x6803
> >   
> > +/* Fibocom products */
> > +#define FIBOCOM_VENDOR_ID   			0x1508
> > +#define FIBOCOM_PRODUCT_NL668   		0x1001
> >   
> >   /* Device flags */
> >   
> > @@ -1941,6 +1944,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(FIBOCOM_VENDOR_ID, FIBOCOM_PRODUCT_NL668), /* Fibocom NL668 series */
> > +	  .driver_info = RSVD(4) | RSVD(5) },
> >   	{ } /* Terminating entry */
> >   };
> >   MODULE_DEVICE_TABLE(usb, option_ids);
> 
> Listed owner of the 0x1508 VID is "MAATEL" which probably is a customer 
> or subsidiary of FIBOCOM (which has the VID 0x2cb7).
> Please don't do this value-to-name conversion, it is completely useless 
> and in this case most likely wrong as well.
> Just skip the declaration part and use the VID and PID values directly 
> in the device structure list.

Checkpatch also detects some white space issues, but that should be
taken care of by the above.

Please also move your changelog comment to after a so called cut-off
line (---, underneath your sign-off); it doesn't belong in the commit
summary (Subject).

And please provide the output of usb-devices (or lsusb -v) for
reference.

Thanks,
Johan