Hi Daniele,
On Mon, 30 May 2022 at 17:31, Daniele Palmas <dnlplm@xxxxxxxxx> wrote:
>
> Hi Carlo,
>
> Il giorno lun 30 mag 2022 alle ore 15:53 Carlo Lobrano
> <c.lobrano@xxxxxxxxx> ha scritto:
> >
> > 0x1250: rmnet, tty, tty, tty, tty
> >
> > Signed-off-by: Carlo Lobrano <c.lobrano@xxxxxxxxx>
> > ---
> >
> > 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
