| Message ID | 20210611135842.14415-1-stefan.bruens@rwth-aachen.de |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] USB: serial: qcserial: Support for SDX55 based Sierra Wireless 5G modules | expand |
[ +CC: Daniele and Bjørn ] On Fri, Jun 11, 2021 at 03:58:41PM +0200, Stefan Brüns wrote: > The devices exposes two different interface compositions: > - QDL mode, single interface > - MBIM mode, MBIM class compliant plus AT/DM(/ADB) > > Current firmware versions (up to 01.07.19) do not expose an NMEA port. We already have at least one SDX55 based modem (FN980) supported by the option driver. Any particular reason why you chose to add it to qcserial instead of option? Note that the FN980 also needs the ZLP flag set in QDL (flashing) mode, I'd assume this one needs it too. Could you please also post the output of usb-devices (or lsusb -v) for this device in MBIM mode? > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> > --- > drivers/usb/serial/qcserial.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c > index 83da8236e3c8..4ff325a14c98 100644 > --- a/drivers/usb/serial/qcserial.c > +++ b/drivers/usb/serial/qcserial.c > @@ -26,12 +26,15 @@ enum qcserial_layouts { > QCSERIAL_G1K = 1, /* Gobi 1000 */ > QCSERIAL_SWI = 2, /* Sierra Wireless */ > QCSERIAL_HWI = 3, /* Huawei */ > + QCSERIAL_SWI2 = 4, /* Sierra Wireless */ > }; > > #define DEVICE_G1K(v, p) \ > USB_DEVICE(v, p), .driver_info = QCSERIAL_G1K > #define DEVICE_SWI(v, p) \ > USB_DEVICE(v, p), .driver_info = QCSERIAL_SWI > +#define DEVICE_SWI2(v, p) \ > + USB_DEVICE(v, p), .driver_info = QCSERIAL_SWI2 > #define DEVICE_HWI(v, p) \ > USB_DEVICE(v, p), .driver_info = QCSERIAL_HWI > > @@ -181,6 +184,10 @@ static const struct usb_device_id id_table[] = { > {DEVICE_SWI(0x413c, 0x81d1)}, /* Dell Wireless 5818 */ > {DEVICE_SWI(0x413c, 0x81d2)}, /* Dell Wireless 5818 */ > > + /* SDX55 based Sierra Wireless devices */ > + {DEVICE_SWI2(0x1199, 0x90d2)}, /* Sierra Wireless EM919x QDL */ > + {DEVICE_SWI2(0x1199, 0x90d3)}, /* Sierra Wireless EM919x */ > + > /* Huawei devices */ > {DEVICE_HWI(0x03f0, 0x581d)}, /* HP lt4112 LTE/HSPA+ Gobi 4G Modem (Huawei me906e) */ > > @@ -359,6 +366,28 @@ static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id) > break; > } > break; > + case QCSERIAL_SWI2: > + /* > + * Sierra Wireless SDX55 in MBIM mode: > + * 0/1: MBIM Control/Data > + * 3: AT-capable modem port > + * 4: DM/DIAG (use libqcdm from ModemManager for communication) > + * 5: ADB > + */ > + switch (ifnum) { > + case 3: > + dev_dbg(dev, "Modem port found\n"); > + sendsetup = true; > + break; > + case 4: > + dev_dbg(dev, "DM/DIAG interface found\n"); > + break; > + default: > + /* don't claim any unsupported interface */ > + altsetting = -1; > + break; > + } > + break; > case QCSERIAL_HWI: > /* > * Huawei devices map functions by subclass + protocol Johan
Johan Hovold <johan@kernel.org> writes: > Could you please also post the output of usb-devices (or lsusb -v) for > this device in MBIM mode? Yes, this would be nice to have. I suspect that this device is like other SDX55 devices we've seen, using class/subclass/function to map the vendor specific functions too. Dropping static interface numbers. If correct, then the patch is bogus and the interface numbers might change based on firmware version and configuration. Bjørn
On Donnerstag, 24. Juni 2021 13:15:07 CEST Bjørn Mork wrote: > Johan Hovold <johan@kernel.org> writes: > > Could you please also post the output of usb-devices (or lsusb -v) for > > this device in MBIM mode? > > Yes, this would be nice to have. See below. > I suspect that this device is like other SDX55 devices we've seen, using > class/subclass/function to map the vendor specific functions > too. Dropping static interface numbers. If correct, then the patch is > bogus and the interface numbers might change based on firmware version > and configuration. Do you really expect Sierra do to something sensible? According to their documentation functions are matched by interface numbers. They still use broken interface descriptors with holes in interface numbering (i.e. interface number 2 does not exist, which violates the USB standard). Regards, Stefan lsusb -v -d 1199: Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.10 bDeviceClass 0 bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 idVendor 0x1199 Sierra Wireless, Inc. idProduct 0x90d3 bcdDevice 0.06 iManufacturer 1 Sierra Wireless, Incorporated iProduct 2 Sierra Wireless EM9191 iSerial 3 8W<nnnnnnnnnnnnn> bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 0x00a7 bNumInterfaces 4 bConfigurationValue 1 iConfiguration 4 bmAttributes 0xa0 (Bus Powered) Remote Wakeup MaxPower 500mA Interface Association: bLength 8 bDescriptorType 11 bFirstInterface 0 bInterfaceCount 2 bFunctionClass 2 Communications bFunctionSubClass 14 bFunctionProtocol 0 iFunction 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 2 Communications bInterfaceSubClass 14 bInterfaceProtocol 0 iInterface 0 CDC Header: bcdCDC 1.10 CDC Union: bMasterInterface 0 bSlaveInterface 1 CDC MBIM: bcdMBIMVersion 1.00 wMaxControlMessage 4096 bNumberFilters 32 bMaxFilterSize 128 wMaxSegmentSize 2048 bmNetworkCapabilities 0x20 8-byte ntb input size CDC MBIM Extended: bcdMBIMExtendedVersion 1.00 bMaxOutstandingCommandMessages 64 wMTU 1500 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 9 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 0 bNumEndpoints 0 bInterfaceClass 10 CDC Data bInterfaceSubClass 0 bInterfaceProtocol 2 iInterface 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 1 bNumEndpoints 2 bInterfaceClass 10 CDC Data bInterfaceSubClass 0 bInterfaceProtocol 2 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x8e EP 14 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 0x0f EP 15 OUT bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 3 bAlternateSetting 0 bNumEndpoints 3 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 0 iInterface 0 ** UNRECOGNIZED: 05 24 00 10 01 ** UNRECOGNIZED: 05 24 01 00 00 ** UNRECOGNIZED: 04 24 02 02 ** UNRECOGNIZED: 05 24 06 00 00 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x000a 1x 10 bytes bInterval 9 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 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 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 4 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 255 Vendor Specific Subclass bInterfaceProtocol 48 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 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 0x02 EP 2 OUT bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 -- Stefan Brüns / Bergstraße 21 / 52062 Aachen home: +49 241 53809034 mobile: +49 151 50412019
On Donnerstag, 24. Juni 2021 09:28:59 CEST Johan Hovold wrote: > [ +CC: Daniele and Bjørn ] > > On Fri, Jun 11, 2021 at 03:58:41PM +0200, Stefan Brüns wrote: > > The devices exposes two different interface compositions: > > - QDL mode, single interface > > - MBIM mode, MBIM class compliant plus AT/DM(/ADB) > > > > Current firmware versions (up to 01.07.19) do not expose an NMEA port. > > We already have at least one SDX55 based modem (FN980) supported by the > option driver. Any particular reason why you chose to add it to qcserial > instead of option? > Support for qualcomm based modems are spread over option and qcserial. All other Sierra devices are supported by qcserial. > Note that the FN980 also needs the ZLP flag set in QDL (flashing) mode, > I'd assume this one needs it too. It depends if you implement the Firehose protocol in accordance to the specification or not. 80-NG319-1 (Firehose specification) explicitly states to pad any XML command packet which is an exact multiple of 512 bytes with an extra newline or other whitespace character. Regards, Stefan -- Stefan Brüns / Bergstraße 21 / 52062 Aachen home: +49 241 53809034 mobile: +49 151 50412019
Stefan Brüns <stefan.bruens@rwth-aachen.de> writes: > On Donnerstag, 24. Juni 2021 13:15:07 CEST Bjørn Mork wrote: >> Johan Hovold <johan@kernel.org> writes: >> > Could you please also post the output of usb-devices (or lsusb -v) for >> > this device in MBIM mode? >> >> Yes, this would be nice to have. > > See below. > >> I suspect that this device is like other SDX55 devices we've seen, using >> class/subclass/function to map the vendor specific functions >> too. Dropping static interface numbers. If correct, then the patch is >> bogus and the interface numbers might change based on firmware version >> and configuration. > > Do you really expect Sierra do to something sensible? According to their > documentation functions are matched by interface numbers. Well... I expect them to be as sensible as any other vendor. And I expect docs to be intendend as guidance only ;-) > They still use broken interface descriptors with holes in interface numbering > (i.e. interface number 2 does not exist, which violates the USB standard). Right. Wrt the violation, I think that train left a decade ago. > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber 3 > bAlternateSetting 0 > bNumEndpoints 3 > bInterfaceClass 255 Vendor Specific Class > bInterfaceSubClass 0 > bInterfaceProtocol 0 [..] > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber 4 > bAlternateSetting 0 > bNumEndpoints 2 > bInterfaceClass 255 Vendor Specific Class > bInterfaceSubClass 255 Vendor Specific Subclass > bInterfaceProtocol 48 So Sierra do follow the same pattern we've seen on other X55 devices: ff/00/00 - AT ff/ff/30 - QCDM See commits accf227de4d2 ("USB: serial: option: Add support for Quectel RM500Q") d6c1ddd938d8 ("USB: serial: option: add Quectel EM160R-GL") for other examples. This obviously doesn't make any difference if your configuration is the only one. But I believe that is unlikely. There are probably ways the layout can be changed, even if currenly not documented. The advantage of class/subclass/protocol matching to function is that it works regardless of the number of functions and their interface number. Bjørn
On Thu, Jul 01, 2021 at 06:41:26PM +0200, Stefan Brüns wrote: > On Donnerstag, 24. Juni 2021 09:28:59 CEST Johan Hovold wrote: > > [ +CC: Daniele and Bjørn ] > > > > On Fri, Jun 11, 2021 at 03:58:41PM +0200, Stefan Brüns wrote: > > > The devices exposes two different interface compositions: > > > - QDL mode, single interface > > > - MBIM mode, MBIM class compliant plus AT/DM(/ADB) > > > > > > Current firmware versions (up to 01.07.19) do not expose an NMEA port. > > > > We already have at least one SDX55 based modem (FN980) supported by the > > option driver. Any particular reason why you chose to add it to qcserial > > instead of option? > > > Support for qualcomm based modems are spread over option and qcserial. All > other Sierra devices are supported by qcserial. Ok, but we may still end up adding this one to option if matching on the interface protocol works. > > Note that the FN980 also needs the ZLP flag set in QDL (flashing) mode, > > I'd assume this one needs it too. > > It depends if you implement the Firehose protocol in accordance to the > specification or not. 80-NG319-1 (Firehose specification) explicitly states to > pad any XML command packet which is an exact multiple of 512 bytes with an > extra newline or other whitespace character. Thanks for the details. If you're referring to the device-side implementation it seems a bit fragile to not just set the ZLP flag since apparently there are some non-standard implementations out there. But sure, we can do that later if needed. Johan
diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c index 83da8236e3c8..4ff325a14c98 100644 --- a/drivers/usb/serial/qcserial.c +++ b/drivers/usb/serial/qcserial.c @@ -26,12 +26,15 @@ enum qcserial_layouts { QCSERIAL_G1K = 1, /* Gobi 1000 */ QCSERIAL_SWI = 2, /* Sierra Wireless */ QCSERIAL_HWI = 3, /* Huawei */ + QCSERIAL_SWI2 = 4, /* Sierra Wireless */ }; #define DEVICE_G1K(v, p) \ USB_DEVICE(v, p), .driver_info = QCSERIAL_G1K #define DEVICE_SWI(v, p) \ USB_DEVICE(v, p), .driver_info = QCSERIAL_SWI +#define DEVICE_SWI2(v, p) \ + USB_DEVICE(v, p), .driver_info = QCSERIAL_SWI2 #define DEVICE_HWI(v, p) \ USB_DEVICE(v, p), .driver_info = QCSERIAL_HWI @@ -181,6 +184,10 @@ static const struct usb_device_id id_table[] = { {DEVICE_SWI(0x413c, 0x81d1)}, /* Dell Wireless 5818 */ {DEVICE_SWI(0x413c, 0x81d2)}, /* Dell Wireless 5818 */ + /* SDX55 based Sierra Wireless devices */ + {DEVICE_SWI2(0x1199, 0x90d2)}, /* Sierra Wireless EM919x QDL */ + {DEVICE_SWI2(0x1199, 0x90d3)}, /* Sierra Wireless EM919x */ + /* Huawei devices */ {DEVICE_HWI(0x03f0, 0x581d)}, /* HP lt4112 LTE/HSPA+ Gobi 4G Modem (Huawei me906e) */ @@ -359,6 +366,28 @@ static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id) break; } break; + case QCSERIAL_SWI2: + /* + * Sierra Wireless SDX55 in MBIM mode: + * 0/1: MBIM Control/Data + * 3: AT-capable modem port + * 4: DM/DIAG (use libqcdm from ModemManager for communication) + * 5: ADB + */ + switch (ifnum) { + case 3: + dev_dbg(dev, "Modem port found\n"); + sendsetup = true; + break; + case 4: + dev_dbg(dev, "DM/DIAG interface found\n"); + break; + default: + /* don't claim any unsupported interface */ + altsetting = -1; + break; + } + break; case QCSERIAL_HWI: /* * Huawei devices map functions by subclass + protocol
The devices exposes two different interface compositions: - QDL mode, single interface - MBIM mode, MBIM class compliant plus AT/DM(/ADB) Current firmware versions (up to 01.07.19) do not expose an NMEA port. Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> --- drivers/usb/serial/qcserial.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)