usb: serial: qcserial: add support for the Dell DW5821e module

usb: serial: qcserial: add support for the Dell DW5821e module

[Aleksander Morgado]

>>
>> 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.

usb: serial: qcserial: add support for the Dell DW5821e module

[Bjørn Mork]

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

usb: serial: qcserial: add support for the Dell DW5821e module

[Johan Hovold]

> > 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

usb: serial: qcserial: add support for the Dell DW5821e module

[Aleksander Morgado]

>> 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.

usb: serial: qcserial: add support for the Dell DW5821e module

[Johan Hovold]

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

usb: serial: qcserial: add support for the Dell DW5821e module

[Johan Hovold]

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

usb: serial: qcserial: add support for the Dell DW5821e module

[Bjørn Mork]

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

usb: serial: qcserial: add support for the Dell DW5821e module

[Oliver Neukum]

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

usb: serial: qcserial: add support for the Dell DW5821e module

[Bjørn Mork]

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

usb: serial: qcserial: add support for the Dell DW5821e module

[Bjørn Mork]

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

usb: serial: qcserial: add support for the Dell DW5821e module

[Aleksander Morgado]

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

usb: serial: qcserial: add support for the Dell DW5821e module

[Johan Hovold]

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

usb: serial: qcserial: add support for the Dell DW5821e module

[Aleksander Morgado]

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(+)

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) */