| Message ID | 20180623212408.15221-1-aleksander@aleksander.es (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Jun 23, 2018 at 11:24:08PM +0200, Aleksander Morgado wrote: > This module exposes two USB configurations: a QMI+AT capable setup on > USB config #1 and a MBIM capable setup on USB config #2. > > By default the kernel will choose the MBIM capable configuration as > long as the cdc_mbim driver is available. This patch adds support for > the serial ports in the secondary configuration. Could you please post the usb-devices output for this device? Depending on another driver to select a specific configuration seems fragile (and that behaviour is even configurable). Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 26, 2018 at 8:09 AM, Johan Hovold <johan@kernel.org> wrote: > On Sat, Jun 23, 2018 at 11:24:08PM +0200, Aleksander Morgado wrote: >> This module exposes two USB configurations: a QMI+AT capable setup on >> USB config #1 and a MBIM capable setup on USB config #2. >> >> By default the kernel will choose the MBIM capable configuration as >> long as the cdc_mbim driver is available. This patch adds support for >> the serial ports in the secondary configuration. > > Could you please post the usb-devices output for this device? > > Depending on another driver to select a specific configuration seems > fragile (and that behaviour is even configurable). > This would be when running on configuration #1: T: Bus=04 Lev=03 Prnt=04 Port=02 Cnt=01 Dev#= 7 Spd=5000 MxCh= 0 D: Ver= 3.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs= 2 P: Vendor=413c ProdID=81d7 Rev=03.18 S: Manufacturer=DELL S: Product=DW5821e Snapdragon X20 LTE S: SerialNumber=0123456789ABCDEF C: #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=896mA I: If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial I: If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan I: If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial I: If#=0x3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial I: If#=0x4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none) This would be when running on configuration #2: T: Bus=04 Lev=03 Prnt=04 Port=02 Cnt=01 Dev#= 6 Spd=5000 MxCh= 0 D: Ver= 3.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs= 2 P: Vendor=413c ProdID=81d7 Rev=03.18 S: Manufacturer=DELL S: Product=DW5821e Snapdragon X20 LTE S: SerialNumber=0123456789ABCDEF C: #Ifs= 3 Cfg#= 2 Atr=a0 MxPwr=896mA I: If#=0x0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim I: If#=0x1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim I: If#=0x2 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial
Johan Hovold <johan@kernel.org> writes: > On Sat, Jun 23, 2018 at 11:24:08PM +0200, Aleksander Morgado wrote: >> This module exposes two USB configurations: a QMI+AT capable setup on >> USB config #1 and a MBIM capable setup on USB config #2. >> >> By default the kernel will choose the MBIM capable configuration as >> long as the cdc_mbim driver is available. This patch adds support for >> the serial ports in the secondary configuration. > > Could you please post the usb-devices output for this device? > > Depending on another driver to select a specific configuration seems > fragile (and that behaviour is even configurable). I believe Aleksander might be referring to usb_choose_configuration() in drivers/usb/core/generic.c? It does some confusing things with multi-function/multi-configuration devices, explained by this comment: /* From the remaining configs, choose the first one whose * first interface is for a non-vendor-specific class. * Reason: Linux is more likely to have a class driver * than a vendor-specific driver. */ This code can make Linux default to a MBIM configuration if the MBIM function uses the first interface in that configuration, even if this configuration is not the first one. Availability of a driver is not considered. Except for RNDIS, just to make it the whole mess even more confusing.... The result is of course completely arbitrary on any multi-function device, where vendor-specific functions may be mixed with class functions in any order. The result will also change if you e.g. enable a vendor specific debug function in the MBIM configuration, and this function happens to use the first interface. The logic in usb_choose_configuration() is obviously outdated. I assume it was created for single function devices, where that single function was exposed as either a class function or a vendor specific function using separate configurations. This sort of device is still quite common for a few device classes, like for example ethernet NICs. But things have changed a lot for these as well. Linux now has extensive support for vendor sepcific USB functions of all sorts. Not that I need to tell you that :-) The legacy code in usb_choose_configuration() is now often an obstacle making it more difficult for drivers providing the best possible support in Linux. And we end up with horrible hacks like this config-dependent blacklist entry in cdc_ether: #if IS_ENABLED(CONFIG_USB_RTL8152) /* Linksys USB3GIGV1 Ethernet Adapter */ { USB_DEVICE_AND_INTERFACE_INFO(LINKSYS_VENDOR_ID, 0x0041, USB_CLASS_COMM, USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE), .driver_info = 0, }, #endif matched with code in the rtl8152 driver to switch the device back to its default configuration: static int rtl8152_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct usb_device *udev = interface_to_usbdev(intf); u8 version = rtl_get_version(intf); struct r8152 *tp; struct net_device *netdev; int ret; if (version == RTL_VER_UNKNOWN) return -ENODEV; if (udev->actconfig->desc.bConfigurationValue != 1) { usb_driver_set_configuration(udev, 1); return -ENODEV; } .. Extremely ugly, and completely unnecessary if it weren't for that extra mess in usb_choose_configuration(). And the net effect is that the users end up losing the option of using the class driver for this device, since that will be black listed if they (or the distro) built the vendor specific driver. Can we fix this? I've thought about it more than once for a few years, but so far I've rejected the thought. The Linux defaults coming out of usb_choose_configuration() are arbitrary, and often different from any other OS, confusing end users. But the Linux defaults are stable as long as the device configration is stable. Changing usb_choose_configuration() will break some existing setups. The breakages will of course be easy fixable from userspace, but still - proabably out question for Linux? Sorry if I misunderstodd you, Aleksander. I guess you could be thinking about the usb_modeswitch logic too, which does consider cdc_mbim availability. The rant is still valid, though :-) Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Aleksander Morgado <aleksander@aleksander.es> writes: > On Tue, Jun 26, 2018 at 8:09 AM, Johan Hovold <johan@kernel.org> wrote: >> On Sat, Jun 23, 2018 at 11:24:08PM +0200, Aleksander Morgado wrote: >>> This module exposes two USB configurations: a QMI+AT capable setup on >>> USB config #1 and a MBIM capable setup on USB config #2. >>> >>> By default the kernel will choose the MBIM capable configuration as >>> long as the cdc_mbim driver is available. This patch adds support for >>> the serial ports in the secondary configuration. >> >> Could you please post the usb-devices output for this device? >> >> Depending on another driver to select a specific configuration seems >> fragile (and that behaviour is even configurable). >> > > This would be when running on configuration #1: > > T: Bus=04 Lev=03 Prnt=04 Port=02 Cnt=01 Dev#= 7 Spd=5000 MxCh= 0 > D: Ver= 3.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs= 2 > P: Vendor=413c ProdID=81d7 Rev=03.18 > S: Manufacturer=DELL > S: Product=DW5821e Snapdragon X20 LTE > S: SerialNumber=0123456789ABCDEF > C: #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=896mA > I: If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial > I: If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > I: If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial > I: If#=0x3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial > I: If#=0x4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none) > > This would be when running on configuration #2: > > T: Bus=04 Lev=03 Prnt=04 Port=02 Cnt=01 Dev#= 6 Spd=5000 MxCh= 0 > D: Ver= 3.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs= 2 > P: Vendor=413c ProdID=81d7 Rev=03.18 > S: Manufacturer=DELL > S: Product=DW5821e Snapdragon X20 LTE > S: SerialNumber=0123456789ABCDEF > C: #Ifs= 3 Cfg#= 2 Atr=a0 MxPwr=896mA > I: If#=0x0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim > I: If#=0x1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim > I: If#=0x2 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial Thanks. This illustrates the arbitrary default configuration choice very well: Imagine switching the order of the qcserial and mbim functions, making qcserial use the first interface. Linux would then select cfg #1 as default, even if the set of functions in both configuratoons were the same as before. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Di, 2018-06-26 at 09:40 +0200, Bjørn Mork wrote: > This code can make Linux default to a MBIM configuration if the MBIM > function uses the first interface in that configuration, even if this > configuration is not the first one. Availability of a driver is not > considered. Except for RNDIS, just to make it the whole mess even more > confusing.... How would you consider it? We chose a configuration before we load drivers. Even if we looked at the currently available drivers we'd end up with a choice depending on which devices were used in the past. A nondeterministic choice would be awkward. We can load drivers for all configurations' interfaces, but we cannot really wait for the loads to happen at that stage. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Oliver Neukum <oneukum@suse.com> writes: > On Di, 2018-06-26 at 09:40 +0200, Bjørn Mork wrote: >> This code can make Linux default to a MBIM configuration if the MBIM >> function uses the first interface in that configuration, even if this >> configuration is not the first one. Availability of a driver is not >> considered. Except for RNDIS, just to make it the whole mess even more >> confusing.... > > How would you consider it? We chose a configuration before we load > drivers. Even if we looked at the currently available drivers we'd end > up with a choice depending on which devices were used in the past. > A nondeterministic choice would be awkward. > > We can load drivers for all configurations' interfaces, but we cannot > really wait for the loads to happen at that stage. No, we do not want more driver support guesswork. Going back in time, I believe it would be far better to simply let the firmware decide the default by using the first configuration. It's the least surprising choice, and the likely tested configuration. And it would simplify the code. You are of course correct that we have no knowledge of available, matching, or successfully probing, drivers at this point. All the problems come from the attempt to consider driver availability anyway. The preference for class functions is clearly(?) based on an assumption that class drivers are more likely to be available. But I do realize that this was more difficult when the code was written. We did not support many vendor specific functions, and there wasn't mcuh reason to believe this would change over time. And it might have been more complicated/impossible to override the kernel default in userspace, writing to bConfigurationValue from something like udev? Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 26, 2018 at 09:32:32AM +0200, Aleksander Morgado wrote: > On Tue, Jun 26, 2018 at 8:09 AM, Johan Hovold <johan@kernel.org> wrote: > > On Sat, Jun 23, 2018 at 11:24:08PM +0200, Aleksander Morgado wrote: > >> This module exposes two USB configurations: a QMI+AT capable setup on > >> USB config #1 and a MBIM capable setup on USB config #2. > >> > >> By default the kernel will choose the MBIM capable configuration as > >> long as the cdc_mbim driver is available. This patch adds support for > >> the serial ports in the secondary configuration. > > > > Could you please post the usb-devices output for this device? > > > > Depending on another driver to select a specific configuration seems > > fragile (and that behaviour is even configurable). > > This would be when running on configuration #1: > > T: Bus=04 Lev=03 Prnt=04 Port=02 Cnt=01 Dev#= 7 Spd=5000 MxCh= 0 > D: Ver= 3.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs= 2 > P: Vendor=413c ProdID=81d7 Rev=03.18 > S: Manufacturer=DELL > S: Product=DW5821e Snapdragon X20 LTE > S: SerialNumber=0123456789ABCDEF > C: #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=896mA > I: If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial > I: If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > I: If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial > I: If#=0x3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial > I: If#=0x4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none) So this doesn't really match the Sierra layout in qcserial, where the QMI interface is documented as interface 8 (but perhaps we have other examples of that already). Do you know what these serial ports are used for? > This would be when running on configuration #2: > > T: Bus=04 Lev=03 Prnt=04 Port=02 Cnt=01 Dev#= 6 Spd=5000 MxCh= 0 > D: Ver= 3.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs= 2 > P: Vendor=413c ProdID=81d7 Rev=03.18 > S: Manufacturer=DELL > S: Product=DW5821e Snapdragon X20 LTE > S: SerialNumber=0123456789ABCDEF > C: #Ifs= 3 Cfg#= 2 Atr=a0 MxPwr=896mA > I: If#=0x0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim > I: If#=0x1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim > I: If#=0x2 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial And here you happen to bind interface 2, but is that intentional? What is that port used for? Just based on the above, perhaps using option and matching on the vendor class, while blacklisting interface 1 would be more appropriate? Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 26, 2018 at 09:40:24AM +0200, Bjørn Mork wrote: > Johan Hovold <johan@kernel.org> writes: > > On Sat, Jun 23, 2018 at 11:24:08PM +0200, Aleksander Morgado wrote: > >> This module exposes two USB configurations: a QMI+AT capable setup on > >> USB config #1 and a MBIM capable setup on USB config #2. > >> > >> By default the kernel will choose the MBIM capable configuration as > >> long as the cdc_mbim driver is available. This patch adds support for > >> the serial ports in the secondary configuration. > > > > Could you please post the usb-devices output for this device? > > > > Depending on another driver to select a specific configuration seems > > fragile (and that behaviour is even configurable). > > > I believe Aleksander might be referring to usb_choose_configuration() in > drivers/usb/core/generic.c? It does some confusing things with > multi-function/multi-configuration devices, explained by this comment: > > /* From the remaining configs, choose the first one whose > * first interface is for a non-vendor-specific class. > * Reason: Linux is more likely to have a class driver > * than a vendor-specific driver. */ > > > This code can make Linux default to a MBIM configuration if the MBIM > function uses the first interface in that configuration, even if this > configuration is not the first one. Availability of a driver is not > considered. Except for RNDIS, just to make it the whole mess even more > confusing.... Ah, that may be the case. Aleksander, would you mind updating the commit message and drop the cdc_mbim driver bit? Please also include the usb-devices output in the commit message for future reference. > The result is of course completely arbitrary on any multi-function > device, where vendor-specific functions may be mixed with class > functions in any order. The result will also change if you e.g. enable > a vendor specific debug function in the MBIM configuration, and this > function happens to use the first interface. > > The logic in usb_choose_configuration() is obviously outdated. I assume > it was created for single function devices, where that single function > was exposed as either a class function or a vendor specific function > using separate configurations. This sort of device is still quite > common for a few device classes, like for example ethernet NICs. But > things have changed a lot for these as well. Linux now has extensive > support for vendor sepcific USB functions of all sorts. Not that I > need to tell you that :-) The legacy code in usb_choose_configuration() > is now often an obstacle making it more difficult for drivers providing > the best possible support in Linux. And we end up with horrible hacks > like this config-dependent blacklist entry in cdc_ether: > > #if IS_ENABLED(CONFIG_USB_RTL8152) > /* Linksys USB3GIGV1 Ethernet Adapter */ > { > USB_DEVICE_AND_INTERFACE_INFO(LINKSYS_VENDOR_ID, 0x0041, USB_CLASS_COMM, > USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE), > .driver_info = 0, > }, > #endif > > matched with code in the rtl8152 driver to switch the device back to its > default configuration: > > static int rtl8152_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > struct usb_device *udev = interface_to_usbdev(intf); > u8 version = rtl_get_version(intf); > struct r8152 *tp; > struct net_device *netdev; > int ret; > > if (version == RTL_VER_UNKNOWN) > return -ENODEV; > > if (udev->actconfig->desc.bConfigurationValue != 1) { > usb_driver_set_configuration(udev, 1); > return -ENODEV; > } > .. Wow. But from a quick glance at the corresponding commit 10c3271712f5 ("r8152: disable the ECM mode") it looks like this may at least partly have been due to firmware issues when switching configurations. > Extremely ugly, and completely unnecessary if it weren't for that extra > mess in usb_choose_configuration(). And the net effect is that the > users end up losing the option of using the class driver for this device, > since that will be black listed if they (or the distro) built the vendor > specific driver. Yeah, not nice. > Can we fix this? I've thought about it more than once for a few years, > but so far I've rejected the thought. The Linux defaults coming out of > usb_choose_configuration() are arbitrary, and often different from any > other OS, confusing end users. But the Linux defaults are stable as > long as the device configration is stable. Changing > usb_choose_configuration() will break some existing setups. The > breakages will of course be easy fixable from userspace, but still - > proabably out question for Linux? Yeah, perhaps it's too late to change, but whatever default we pick, we should at least allow user space to override it. > Sorry if I misunderstodd you, Aleksander. I guess you could be thinking > about the usb_modeswitch logic too, which does consider cdc_mbim > availability. The rant is still valid, though :-) Yup. And the commit message would still need to be amended. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >> This would be when running on configuration #1: >> >> T: Bus=04 Lev=03 Prnt=04 Port=02 Cnt=01 Dev#= 7 Spd=5000 MxCh= 0 >> D: Ver= 3.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs= 2 >> P: Vendor=413c ProdID=81d7 Rev=03.18 >> S: Manufacturer=DELL >> S: Product=DW5821e Snapdragon X20 LTE >> S: SerialNumber=0123456789ABCDEF >> C: #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=896mA >> I: If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial >> I: If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan >> I: If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial >> I: If#=0x3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial >> I: If#=0x4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none) > > So this doesn't really match the Sierra layout in qcserial, where the > QMI interface is documented as interface 8 (but perhaps we have other > examples of that already). > > Do you know what these serial ports are used for? > You know what, I kind of missed that. This Dell device is not based on Sierra Wireless, so using the Sierra layout is not a good idea. Let me try to confirm with the manufacturer the purpose of each interface. >> This would be when running on configuration #2: >> >> T: Bus=04 Lev=03 Prnt=04 Port=02 Cnt=01 Dev#= 6 Spd=5000 MxCh= 0 >> D: Ver= 3.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs= 2 >> P: Vendor=413c ProdID=81d7 Rev=03.18 >> S: Manufacturer=DELL >> S: Product=DW5821e Snapdragon X20 LTE >> S: SerialNumber=0123456789ABCDEF >> C: #Ifs= 3 Cfg#= 2 Atr=a0 MxPwr=896mA >> I: If#=0x0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim >> I: If#=0x1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim >> I: If#=0x2 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial > > And here you happen to bind interface 2, but is that intentional? What > is that port used for? > Same here; looks like interface was bound to qcserial by chance, because that is what the sierra layout would have done in the QMI capable configuration. Again, let me try to confirm what that other interface may be for. > Just based on the above, perhaps using option and matching on the vendor > class, while blacklisting interface 1 would be more appropriate? > Being a Qualcomm based chipset, I believe qcserial would be more appropriate. I'll send an updated patch, including usb-devices output, once I have an explanation for all questions.
>> I believe Aleksander might be referring to usb_choose_configuration() in >> drivers/usb/core/generic.c? It does some confusing things with >> multi-function/multi-configuration devices, explained by this comment: >> >> /* From the remaining configs, choose the first one whose >> * first interface is for a non-vendor-specific class. >> * Reason: Linux is more likely to have a class driver >> * than a vendor-specific driver. */ >> >> >> This code can make Linux default to a MBIM configuration if the MBIM >> function uses the first interface in that configuration, even if this >> configuration is not the first one. Availability of a driver is not >> considered. Except for RNDIS, just to make it the whole mess even more >> confusing.... > > Ah, that may be the case. Aleksander, would you mind updating the commit > message and drop the cdc_mbim driver bit? > Yes, will do that. I was actually completely mixing up this logic in the kernel with the logic in usb_modeswitch indeed, as Bjørn pointed out.
> > So this doesn't really match the Sierra layout in qcserial, where the > > QMI interface is documented as interface 8 (but perhaps we have other > > examples of that already). > > > > Do you know what these serial ports are used for? > > You know what, I kind of missed that. This Dell device is not based on > Sierra Wireless, so using the Sierra layout is not a good idea. Let me > try to confirm with the manufacturer the purpose of each interface. Sounds good. > > Just based on the above, perhaps using option and matching on the vendor > > class, while blacklisting interface 1 would be more appropriate? > > Being a Qualcomm based chipset, I believe qcserial would be more appropriate. > I'll send an updated patch, including usb-devices output, once I have > an explanation for all questions. Note that we have other qualcomm based devices being handled by option already. The qcserial driver was intended for a special subset of Gobi modems with a particular interface layout and firmware loading requirements. So unless your device falls in any of those categories, option may still be preferred. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Aleksander Morgado <aleksander@aleksander.es> writes:
> Being a Qualcomm based chipset, I believe qcserial would be more appropriate.
Plenty of Qualcomm devices are handled by option. IMHO, there is no
point in adding device specific interface matching to qcserial, unless
it is a reusable pattern like the ones we have there before.
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c index 613f91add03d..ed109c86e747 100644 --- a/drivers/usb/serial/qcserial.c +++ b/drivers/usb/serial/qcserial.c @@ -177,6 +177,7 @@ static const struct usb_device_id id_table[] = { {DEVICE_SWI(0x413c, 0x81d0)}, /* Dell Wireless 5819 */ {DEVICE_SWI(0x413c, 0x81d1)}, /* Dell Wireless 5818 */ {DEVICE_SWI(0x413c, 0x81d2)}, /* Dell Wireless 5818 */ + {DEVICE_SWI(0x413c, 0x81d7)}, /* Dell Wireless 5821e */ /* Huawei devices */ {DEVICE_HWI(0x03f0, 0x581d)}, /* HP lt4112 LTE/HSPA+ Gobi 4G Modem (Huawei me906e) */
This module exposes two USB configurations: a QMI+AT capable setup on USB config #1 and a MBIM capable setup on USB config #2. By default the kernel will choose the MBIM capable configuration as long as the cdc_mbim driver is available. This patch adds support for the serial ports in the secondary configuration. Signed-off-by: Aleksander Morgado <aleksander@aleksander.es> --- drivers/usb/serial/qcserial.c | 1 + 1 file changed, 1 insertion(+)