[PATCH v2] usb: serial: Add support for Telit LN910Cx 0x1250 composition

[PATCH v2] usb: serial: Add support for Telit LN910Cx 0x1250 composition

[Carlo Lobrano]

0x1250: rmnet, tty, tty, tty, tty

Signed-off-by: Carlo Lobrano <c.lobrano@gmail.com>
---

v2: use RSVD in place of NCTRL for interface 0 (rmnet)

 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 152ad882657d..cd9aa61ec801 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1275,6 +1275,8 @@ static const struct usb_device_id option_ids[] = {
 	  .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) },
 	{ USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1231, 0xff),	/* Telit LE910Cx (RNDIS) */
 	  .driver_info = NCTRL(2) | RSVD(3) },
+	{ USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1250, 0xff),	/* Telit LE910Cx (rmnet) */
+	  .driver_info = RSVD(0) },
 	{ USB_DEVICE(TELIT_VENDOR_ID, 0x1260),
 	  .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) },
 	{ USB_DEVICE(TELIT_VENDOR_ID, 0x1261),
-- 
2.25.1

Re: [PATCH v2] usb: serial: Add support for Telit LN910Cx 0x1250 composition

[Daniele Palmas]

Hi Carlo,

Il giorno lun 30 mag 2022 alle ore 15:53 Carlo Lobrano
<c.lobrano@gmail.com> ha scritto:
>
> 0x1250: rmnet, tty, tty, tty, tty
>
> Signed-off-by: Carlo Lobrano <c.lobrano@gmail.com>
> ---
>
> v2: use RSVD in place of NCTRL for interface 0 (rmnet)
>
>  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 152ad882657d..cd9aa61ec801 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -1275,6 +1275,8 @@ static const struct usb_device_id option_ids[] = {
>           .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) },
>         { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1231, 0xff),    /* Telit LE910Cx (RNDIS) */
>           .driver_info = NCTRL(2) | RSVD(3) },
> +       { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1250, 0xff),    /* Telit LE910Cx (rmnet) */
> +         .driver_info = RSVD(0) },

The content of the patch looks good, but the subject needs to be
fixed, since the composition does not belong to "LN910Cx", but
"LE910Cx": sorry for not having noticed that in v1.

Since you are there, it would be also preferred to have the prefix as:

USB: serial: option: ....

and maybe it could be worth adding something more in the body than
only the single composition, e.g.

Add support for...

Thanks,
Daniele

>         { USB_DEVICE(TELIT_VENDOR_ID, 0x1260),
>           .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) },
>         { USB_DEVICE(TELIT_VENDOR_ID, 0x1261),
> --
> 2.25.1
>

Re: [PATCH v2] usb: serial: Add support for Telit LN910Cx 0x1250 composition

[Carlo Lobrano]

Hi Daniele,

On Mon, 30 May 2022 at 17:31, Daniele Palmas <dnlplm@gmail.com> wrote:
>
> Hi Carlo,
>
> Il giorno lun 30 mag 2022 alle ore 15:53 Carlo Lobrano
> <c.lobrano@gmail.com> ha scritto:
> >
> > 0x1250: rmnet, tty, tty, tty, tty
> >
> > Signed-off-by: Carlo Lobrano <c.lobrano@gmail.com>
> > ---
> >
> > v2: use RSVD in place of NCTRL for interface 0 (rmnet)
> >
> >  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 152ad882657d..cd9aa61ec801 100644
> > --- a/drivers/usb/serial/option.c
> > +++ b/drivers/usb/serial/option.c
> > @@ -1275,6 +1275,8 @@ static const struct usb_device_id option_ids[] = {
> >           .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) },
> >         { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1231, 0xff),    /* Telit LE910Cx (RNDIS) */
> >           .driver_info = NCTRL(2) | RSVD(3) },
> > +       { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1250, 0xff),    /* Telit LE910Cx (rmnet) */
> > +         .driver_info = RSVD(0) },
>
> The content of the patch looks good, but the subject needs to be
> fixed, since the composition does not belong to "LN910Cx", but
> "LE910Cx": sorry for not having noticed that in v1.
>
> Since you are there, it would be also preferred to have the prefix as:
>
> USB: serial: option: ....
>
> and maybe it could be worth adding something more in the body than
> only the single composition, e.g.
>
> Add support for...
>
> Thanks,
> Daniele
>
> >         { USB_DEVICE(TELIT_VENDOR_ID, 0x1260),
> >           .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) },
> >         { USB_DEVICE(TELIT_VENDOR_ID, 0x1261),
> > --
> > 2.25.1
> >

thank you for your review. There is definitely a typo in modem's name,
and I also agree with your other notes.
I will update the patch with what you suggested

Thanks,
Carlo