Re: [PATCH RESEND 1/1] usb: serial: option: add ZLP support for 0x1bc7/0x9010

[Johan Hovold <johan@kernel.org>]

On Thu, Dec 05, 2019 at 05:06:59PM +0100, Daniele Palmas wrote:
> Il giorno gio 5 dic 2019 alle ore 15:57 Johan Hovold
> <johan@kernel.org> ha scritto:
> >
> > On Mon, Nov 18, 2019 at 02:32:51PM +0100, Daniele Palmas wrote:
> > > Telit FN980 flashing device 0x1bc7/0x9010 requires zero packet
> > > to be sent if out data size is equal to the endpoint max size.
> > >
> > > Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
> > > ---
> > >  drivers/usb/serial/option.c   | 8 ++++++++
> > >  drivers/usb/serial/usb-wwan.h | 1 +
> > >  drivers/usb/serial/usb_wwan.c | 9 +++++++++
> > >  3 files changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> > > index e9491d400a24..c13e063cb399 100644
> > > --- a/drivers/usb/serial/option.c
> > > +++ b/drivers/usb/serial/option.c
> > > @@ -567,6 +567,9 @@ static void option_instat_callback(struct urb *urb);
> > >  /* Interface must have two endpoints */
> > >  #define NUMEP2               BIT(16)
> > >
> > > +/* Device needs ZLP */
> > > +#define ZLP          BIT(17)
> > > +
> > >
> > >  static const struct usb_device_id option_ids[] = {
> > >       { USB_DEVICE(OPTION_VENDOR_ID, OPTION_PRODUCT_COLT) },
> > > @@ -1196,6 +1199,8 @@ static const struct usb_device_id option_ids[] = {
> > >         .driver_info = NCTRL(0) | RSVD(1) },
> > >       { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1901, 0xff),    /* Telit LN940 (MBIM) */
> > >         .driver_info = NCTRL(0) },
> > > +     { USB_DEVICE(TELIT_VENDOR_ID, 0x9010),                          /* Telit SBL FN980 flashing device */
> > > +       .driver_info = NCTRL(0) | ZLP },
> >
> > Hmm. Could you post the lsusb -v output for this device?
> >
> 
> Bus 003 Device 004: ID 1bc7:9010 Telit Wireless Solutions
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               2.10
>   bDeviceClass            0 (Defined at Interface level)
>   bDeviceSubClass         0
>   bDeviceProtocol         0
>   bMaxPacketSize0        64
>   idVendor           0x1bc7 Telit Wireless Solutions
>   idProduct          0x9010
>   bcdDevice            0.00
>   iManufacturer           1 Telit Wireless Solutions
>   iProduct                2 FN980m
>   iSerial                 0
>   bNumConfigurations      1
>   Configuration Descriptor:
>     bLength                 9
>     bDescriptorType         2
>     wTotalLength           32
>     bNumInterfaces          1
>     bConfigurationValue     1
>     iConfiguration          0
>     bmAttributes         0xa0
>       (Bus Powered)
>       Remote Wakeup
>     MaxPower                2mA
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        0
>       bAlternateSetting       0
>       bNumEndpoints           2
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol     16
>       iInterface              0
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x81  EP 1 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval               0
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x01  EP 1 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval               0
> Binary Object Store Descriptor:
>   bLength                 5
>   bDescriptorType        15
>   wTotalLength           42
>   bNumDeviceCaps          3
>   ** UNRECOGNIZED:  14 10 0a 00 01 00 00 00 00 11 00 00 30 40 0a 00 b0 40 0a 00
>   SuperSpeed USB Device Capability:
>     bLength                10
>     bDescriptorType        16
>     bDevCapabilityType      3
>     bmAttributes         0x00
>     wSpeedsSupported   0x000e
>       Device can operate at Full Speed (12Mbps)
>       Device can operate at High Speed (480Mbps)
>       Device can operate at SuperSpeed (5Gbps)
>     bFunctionalitySupport   1
>       Lowest fully-functional device speed is Full Speed (12Mbps)
>     bU1DevExitLat          10 micro seconds
>     bU2DevExitLat        2047 micro seconds
>   USB 2.0 Extension Device Capability:
>     bLength                 7
>     bDescriptorType        16
>     bDevCapabilityType      2
>     bmAttributes   0x00000002
>       Link Power Management (LPM) Supported
> Device Status:     0x0000
>   (Bus Powered)
> 
> > I wonder whether using option is the right thing to do here. We're just
> > currently throwing everything modem related in here it seems. :)
> >
> 
> I understand your concern. At first, in fact, I tried to add support
> for this in usb-serial-simple, but the change seemed to me more
> impacting than the one I had to do for adding support in option.
> 
> Let me know if you want me to continue investigating the
> usb-serial-simple way or you have another advice.

Yeah, I was thinking along those lines as well. Let me get me get back
to you on that.

Johan