Discussion:
[PATCH v2] usb: cdc-wdm: Add device-id for Huawei 3G/LTE modems
Bjørn Mork
2012-01-21 21:53:40 UTC
Permalink
[v2: Editorial changes suggested by Sergei Shtylyov]

These modems use the Qualcomm MSM Interface (QMI) protocol for
management of their CDC ECM like wwan interface. This driver
is perfect for exporting the protocol to userspace.

The created character device will be indistinguishable from a
common AT command based Device Management interface, so
userspace applications must do some intelligent matching
on the USB device.

Cc: Sergei Shtylyov <sshtylyov-Igf4POYTYCDQT0dZR+***@public.gmane.org>
Signed-off-by: Bj=C3=B8rn Mork <bjorn-***@public.gmane.org>
---
As Marcel pointed out, this is the only change which is really
required to have support for this modem and start developing
QMI applications. Please note that this should only be applied=20
after the patch titled

USB: cdc-wdm: Avoid hanging on interface with no USB_CDC_DMM_TYPE

which, unless I've misunderstood something, should already be=20
accepted both for next and stable.


Thanks,
Bj=C3=B8rn

drivers/usb/class/cdc-wdm.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 8faa2f7..f63601a 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -31,6 +31,8 @@
#define DRIVER_AUTHOR "Oliver Neukum"
#define DRIVER_DESC "USB Abstract Control Model driver for USB WCM Dev=
ice Management"
=20
+#define HUAWEI_VENDOR_ID 0x12D1
+
static const struct usb_device_id wdm_ids[] =3D {
{
.match_flags =3D USB_DEVICE_ID_MATCH_INT_CLASS |
@@ -38,6 +40,20 @@ static const struct usb_device_id wdm_ids[] =3D {
.bInterfaceClass =3D USB_CLASS_COMM,
.bInterfaceSubClass =3D USB_CDC_SUBCLASS_DMM
},
+ {
+ /*=20
+ * Huawei E392, E398 and possibly other Qualcomm based modems
+ * embed the Qualcomm QMI protocol inside CDC on CDC ECM like
+ * control interfaces. Userspace access to this is required
+ * to configure the accompanying data interface
+ */
+ .match_flags =3D USB_DEVICE_ID_MATCH_VENDOR |
+ USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor =3D HUAWEI_VENDOR_ID,
+ .bInterfaceClass =3D USB_CLASS_VENDOR_SPEC,
+ .bInterfaceSubClass =3D 1,
+ .bInterfaceProtocol =3D 9, /* NOTE: CDC ECM control interface! */
+ },
{ }
};
=20
--=20
1.7.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Oliver Neukum
2012-01-21 21:58:23 UTC
Permalink
Post by Bjørn Mork
[v2: Editorial changes suggested by Sergei Shtylyov]
=20
These modems use the Qualcomm MSM Interface (QMI) protocol for
management of their CDC ECM like wwan interface. This driver
is perfect for exporting the protocol to userspace.
=20
The created character device will be indistinguishable from a
common AT command based Device Management interface, so
userspace applications must do some intelligent matching
on the USB device.
=20
Acked-by: Oliver Neukum <oneukum-***@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Marcel Holtmann
2012-01-22 10:08:30 UTC
Permalink
Hi Bjorn,
Post by Bjørn Mork
[v2: Editorial changes suggested by Sergei Shtylyov]
=20
These modems use the Qualcomm MSM Interface (QMI) protocol for
management of their CDC ECM like wwan interface. This driver
is perfect for exporting the protocol to userspace.
=20
The created character device will be indistinguishable from a
common AT command based Device Management interface, so
userspace applications must do some intelligent matching
on the USB device.
=20
=20
Acked-by: Marcel Holtmann <marcel-kz+***@public.gmane.org>

Regards

Marcel


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dan Williams
2012-01-23 17:33:41 UTC
Permalink
Post by Bjørn Mork
[v2: Editorial changes suggested by Sergei Shtylyov]
=20
These modems use the Qualcomm MSM Interface (QMI) protocol for
management of their CDC ECM like wwan interface. This driver
is perfect for exporting the protocol to userspace.
=20
The created character device will be indistinguishable from a
common AT command based Device Management interface, so
userspace applications must do some intelligent matching
on the USB device.
=20
---
As Marcel pointed out, this is the only change which is really
required to have support for this modem and start developing
QMI applications. Please note that this should only be applied=20
after the patch titled
=20
USB: cdc-wdm: Avoid hanging on interface with no USB_CDC_DMM_TYPE
=20
which, unless I've misunderstood something, should already be=20
accepted both for next and stable.
=20
=20
Thanks,
Bj=C3=B8rn
=20
drivers/usb/class/cdc-wdm.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
=20
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.=
c
Post by Bjørn Mork
index 8faa2f7..f63601a 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -31,6 +31,8 @@
#define DRIVER_AUTHOR "Oliver Neukum"
#define DRIVER_DESC "USB Abstract Control Model driver for USB WCM D=
evice Management"
Post by Bjørn Mork
=20
+#define HUAWEI_VENDOR_ID 0x12D1
+
static const struct usb_device_id wdm_ids[] =3D {
{
.match_flags =3D USB_DEVICE_ID_MATCH_INT_CLASS |
@@ -38,6 +40,20 @@ static const struct usb_device_id wdm_ids[] =3D {
.bInterfaceClass =3D USB_CLASS_COMM,
.bInterfaceSubClass =3D USB_CDC_SUBCLASS_DMM
},
+ {
+ /*=20
+ * Huawei E392, E398 and possibly other Qualcomm based modems
+ * embed the Qualcomm QMI protocol inside CDC on CDC ECM like
+ * control interfaces. Userspace access to this is required
+ * to configure the accompanying data interface
+ */
+ .match_flags =3D USB_DEVICE_ID_MATCH_VENDOR |
+ USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor =3D HUAWEI_VENDOR_ID,
+ .bInterfaceClass =3D USB_CLASS_VENDOR_SPEC,
+ .bInterfaceSubClass =3D 1,
+ .bInterfaceProtocol =3D 9, /* NOTE: CDC ECM control interface! */
+ },
{ }
};
So the Pantech UML290 has the following layout:

0 (2/2/1): CDC-ACM for AT commands
1 (10/0/0): CDC-DATA for interface 0
2 (ff/ff/ff): Qualcomm DIAG
3 (ff/fd/ff): NMEA
4 (ff/fe/ff): Pantech WMC
5 (ff/f0/ff): RMNET/QMI port

Interface 5 is obviously the one we want here. And the WDM driver is
only looking for certain descriptors. Do we hack CDC-WDM and qmi_wwan
up for these types of devices?

Second, on Gobi devices, we have four USB interfaces, all FF/FF/FF.
Intf 0 and 2 have interrupt endpoints. One of them is a DIAG interface=
,
one is NMEA, and the other two are AT and RMNET/QMI.

I think so far the Huawei device is the only one that I've seen that
exposes descriptors that are quasi-CDC at all. How should we handle th=
e
rest of these?

Dan

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-01-23 18:56:51 UTC
Permalink
Post by Dan Williams
0 (2/2/1): CDC-ACM for AT commands
1 (10/0/0): CDC-DATA for interface 0
2 (ff/ff/ff): Qualcomm DIAG
3 (ff/fd/ff): NMEA
4 (ff/fe/ff): Pantech WMC
5 (ff/f0/ff): RMNET/QMI port
I assume this is after som modeswitch command? The same as the Windows
driver uses? And you don't know if there are other options?
Post by Dan Williams
Interface 5 is obviously the one we want here. And the WDM driver is
only looking for certain descriptors. Do we hack CDC-WDM and qmi_wwa=
n
Post by Dan Williams
up for these types of devices?
Interface 5 has three endpoints? Bulk in/out and interrupt in?

OK, this is where the fun begins. I knew there was some reason why I
was struggling with the interface sharing... I guess we cannot expect
every vendor to provide a "Linux mode" with two separate interfaces for
the RMNET/QMI port.
Post by Dan Williams
Second, on Gobi devices, we have four USB interfaces, all FF/FF/FF.
Intf 0 and 2 have interrupt endpoints. One of them is a DIAG interfa=
ce,
Post by Dan Williams
one is NMEA, and the other two are AT and RMNET/QMI.
What kind of endpoints are on the RMNET/QMI interface?

If you have these devices, then it would be useful to verify that the
cdc-wdm driver can be used to provide access to QMI without any further
changes. This should in theory just work if you unload any other
drivers binding to the RMNET/QMI interface, and bind cdc-wdm to it
instead. This requires that you have the minimum buffer size patch
installed, but should not require any other changes.
Post by Dan Williams
I think so far the Huawei device is the only one that I've seen that
exposes descriptors that are quasi-CDC at all. How should we handle =
the
Post by Dan Williams
rest of these?
Good question. Guess I'm going to see if I can make qmi_wwan and
cdc-wdm share an interface after all.


Bj=C3=B8rn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dan Williams
2012-01-23 20:07:40 UTC
Permalink
Post by Bjørn Mork
Post by Dan Williams
0 (2/2/1): CDC-ACM for AT commands
1 (10/0/0): CDC-DATA for interface 0
2 (ff/ff/ff): Qualcomm DIAG
3 (ff/fd/ff): NMEA
4 (ff/fe/ff): Pantech WMC
5 (ff/f0/ff): RMNET/QMI port
I assume this is after som modeswitch command? The same as the Windows
driver uses? And you don't know if there are other options?
The UML290 does not require modeswitching at all.
Post by Bjørn Mork
Post by Dan Williams
Interface 5 is obviously the one we want here. And the WDM driver is
only looking for certain descriptors. Do we hack CDC-WDM and qmi_wwan
up for these types of devices?
Interface 5 has three endpoints? Bulk in/out and interrupt in?
Yes, three endpoints like you describe.
Post by Bjørn Mork
OK, this is where the fun begins. I knew there was some reason why I
was struggling with the interface sharing... I guess we cannot expect
every vendor to provide a "Linux mode" with two separate interfaces for
the RMNET/QMI port.
Post by Dan Williams
Second, on Gobi devices, we have four USB interfaces, all FF/FF/FF.
Intf 0 and 2 have interrupt endpoints. One of them is a DIAG interface,
one is NMEA, and the other two are AT and RMNET/QMI.
What kind of endpoints are on the RMNET/QMI interface?
see Elly's cleaned up Gobi driver here: http://lwn.net/Articles/439173/
as it has the logic to set everything up for Gobi devices. I expect
that it would work with the UML290 as well if things were hacked up a
bit. It should be pretty clear here how to talk to them.

In short we have:

0: (ff/ff/ff) (in/out/interrupt) - rmnet/QMI
1: (ff/ff/ff) (in/out) - DIAG
2: (ff/ff/ff) (in/out) - AT commands & PPP
3: (ff/ff/ff) (in/out/interrupt) - NMEA (?)

lsusb attached for both a Gobi 2K device and the UML290.
Post by Bjørn Mork
If you have these devices, then it would be useful to verify that the
cdc-wdm driver can be used to provide access to QMI without any further
changes. This should in theory just work if you unload any other
drivers binding to the RMNET/QMI interface, and bind cdc-wdm to it
instead. This requires that you have the minimum buffer size patch
installed, but should not require any other changes.
I tried that quickly but of course the WDM driver fails with EINVAL
because there are 3 endpoints on the RMNET/QMI interface (bulk
in/out/interrupt) instead of the 1 expected:

rv = -EINVAL;
iface = intf->cur_altsetting;
if (iface->desc.bNumEndpoints != 1)
goto err;

I briefly thought about hacking it to find the interrupt endpoing, but
if the cdc-wdm driver apparently only cares about 1 endpoint there's not
much point to handing it an interface with 3 I don't think...
Post by Bjørn Mork
Post by Dan Williams
I think so far the Huawei device is the only one that I've seen that
exposes descriptors that are quasi-CDC at all. How should we handle the
rest of these?
Good question. Guess I'm going to see if I can make qmi_wwan and
cdc-wdm share an interface after all.
Also you may not have seen, but the QUIC codeaurora people pushed a new
rmnet USB driver to their MSM kernel tree:

https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=37c35e4ee5226099374a1a1eda637d0f26fc023d

In the end, if the Huawei device mostly presents itself as WDM, and
other Gobi/MSM8xxx/MSM9xxx devices present themselves like Gobi cards
do, perhaps we should have separate drivers? Does it make sense to keep
hacking up CDC-WDM for devices that are clearly not exposed that way,
even if the internal operation would be similar? Also in the end they
are all just character devices to talk QMI so it doesn't really matter
what driver exposes that interface. It would be nice to share if we
can, but making too many changes to WDM probably isn't the right
approach either.

Dan
Bjørn Mork
2012-01-23 20:31:03 UTC
Permalink
Post by Dan Williams
=20
Post by Dan Williams
0 (2/2/1): CDC-ACM for AT commands
1 (10/0/0): CDC-DATA for interface 0
2 (ff/ff/ff): Qualcomm DIAG
3 (ff/fd/ff): NMEA
4 (ff/fe/ff): Pantech WMC
5 (ff/f0/ff): RMNET/QMI port
=20
I assume this is after som modeswitch command? The same as the Wind=
ows
Post by Dan Williams
driver uses? And you don't know if there are other options?
The UML290 does not require modeswitching at all.
OK, nice.
Post by Dan Williams
Post by Dan Williams
Interface 5 is obviously the one we want here. And the WDM driver=
is
Post by Dan Williams
Post by Dan Williams
only looking for certain descriptors. Do we hack CDC-WDM and qmi_=
wwan
Post by Dan Williams
Post by Dan Williams
up for these types of devices?
=20
=20
Interface 5 has three endpoints? Bulk in/out and interrupt in?
Yes, three endpoints like you describe.
OK, this is where the fun begins. I knew there was some reason why =
I
Post by Dan Williams
was struggling with the interface sharing... I guess we cannot expe=
ct
Post by Dan Williams
every vendor to provide a "Linux mode" with two separate interfaces =
for
Post by Dan Williams
the RMNET/QMI port.
=20
Post by Dan Williams
Second, on Gobi devices, we have four USB interfaces, all FF/FF/FF=
=2E
Post by Dan Williams
Post by Dan Williams
Intf 0 and 2 have interrupt endpoints. One of them is a DIAG inte=
rface,
Post by Dan Williams
Post by Dan Williams
one is NMEA, and the other two are AT and RMNET/QMI.
=20
What kind of endpoints are on the RMNET/QMI interface?
see Elly's cleaned up Gobi driver here: http://lwn.net/Articles/43917=
3/
Post by Dan Williams
as it has the logic to set everything up for Gobi devices. I expect
that it would work with the UML290 as well if things were hacked up a
bit. It should be pretty clear here how to talk to them.
0: (ff/ff/ff) (in/out/interrupt) - rmnet/QMI
1: (ff/ff/ff) (in/out) - DIAG
2: (ff/ff/ff) (in/out) - AT commands & PPP
3: (ff/ff/ff) (in/out/interrupt) - NMEA (?)
lsusb attached for both a Gobi 2K device and the UML290.
If you have these devices, then it would be useful to verify that th=
e
Post by Dan Williams
cdc-wdm driver can be used to provide access to QMI without any furt=
her
Post by Dan Williams
changes. This should in theory just work if you unload any other
drivers binding to the RMNET/QMI interface, and bind cdc-wdm to it
instead. This requires that you have the minimum buffer size patch
installed, but should not require any other changes.
I tried that quickly but of course the WDM driver fails with EINVAL
because there are 3 endpoints on the RMNET/QMI interface (bulk
rv =3D -EINVAL;
iface =3D intf->cur_altsetting;
if (iface->desc.bNumEndpoints !=3D 1)
goto err;
I briefly thought about hacking it to find the interrupt endpoing, bu=
t
Post by Dan Williams
if the cdc-wdm driver apparently only cares about 1 endpoint there's =
not
Post by Dan Williams
much point to handing it an interface with 3 I don't think...
Right, forgot about that part. You could hack it so that it was happil=
y
ignoring the extra bulk endpoints as long as it found an interrupt
endpoint.=20
Post by Dan Williams
Post by Dan Williams
I think so far the Huawei device is the only one that I've seen th=
at
Post by Dan Williams
Post by Dan Williams
exposes descriptors that are quasi-CDC at all. How should we hand=
le the
Post by Dan Williams
Post by Dan Williams
rest of these?
=20
Good question. Guess I'm going to see if I can make qmi_wwan and
cdc-wdm share an interface after all.
Also you may not have seen, but the QUIC codeaurora people pushed a n=
ew
Post by Dan Williams
https://www.codeaurora.org/gitweb/quic/le/?p=3Dkernel/msm.git;a=3Dcom=
mit;h=3D37c35e4ee5226099374a1a1eda637d0f26fc023d

Thanks for the pointer. Will take a look at it. But I guess these are
still doing the integrated QMI approach. Which most likely isn't
acceptable to anyone here, after you managed to convince me :-)
Post by Dan Williams
In the end, if the Huawei device mostly presents itself as WDM, and
other Gobi/MSM8xxx/MSM9xxx devices present themselves like Gobi cards
do, perhaps we should have separate drivers? Does it make sense to k=
eep
Post by Dan Williams
hacking up CDC-WDM for devices that are clearly not exposed that way,
even if the internal operation would be similar? Also in the end the=
y
Post by Dan Williams
are all just character devices to talk QMI so it doesn't really matte=
r
Post by Dan Williams
what driver exposes that interface. It would be nice to share if we
can, but making too many changes to WDM probably isn't the right
approach either.
That is for Oliver to decide I guess. =20

But I believe it can make sense to reuse the cdc-wdm driver, even if it
needs some small changes to allow other drivers to call it. The actual
work it does is pretty standalone: Listen on the interrupt ep, send
control messages and do file operations on the char device. Provided
the other netdev drivers don't need notifications, then this can all be
done without them being involved at all. The only issues are
register/deregister instead of probe/disconnect, and doing the mapping
to the device specific struct. =20

And having all these devices expose their QMI interface as a
/dev/cdc-wdmX should make things look more consistent for users and
userspace.


Bj=C3=B8rn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dan Williams
2012-01-23 20:45:03 UTC
Permalink
Post by Bjørn Mork
=20
Post by Dan Williams
=20
Post by Dan Williams
0 (2/2/1): CDC-ACM for AT commands
1 (10/0/0): CDC-DATA for interface 0
2 (ff/ff/ff): Qualcomm DIAG
3 (ff/fd/ff): NMEA
4 (ff/fe/ff): Pantech WMC
5 (ff/f0/ff): RMNET/QMI port
=20
I assume this is after som modeswitch command? The same as the Wi=
ndows
Post by Bjørn Mork
Post by Dan Williams
driver uses? And you don't know if there are other options?
The UML290 does not require modeswitching at all.
=20
OK, nice.
=20
Post by Dan Williams
Post by Dan Williams
Interface 5 is obviously the one we want here. And the WDM driv=
er is
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
only looking for certain descriptors. Do we hack CDC-WDM and qm=
i_wwan
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
up for these types of devices?
=20
=20
Interface 5 has three endpoints? Bulk in/out and interrupt in?
Yes, three endpoints like you describe.
OK, this is where the fun begins. I knew there was some reason wh=
y I
Post by Bjørn Mork
Post by Dan Williams
was struggling with the interface sharing... I guess we cannot ex=
pect
Post by Bjørn Mork
Post by Dan Williams
every vendor to provide a "Linux mode" with two separate interface=
s for
Post by Bjørn Mork
Post by Dan Williams
the RMNET/QMI port.
=20
Post by Dan Williams
Second, on Gobi devices, we have four USB interfaces, all FF/FF/=
=46F.
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
Intf 0 and 2 have interrupt endpoints. One of them is a DIAG in=
terface,
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
one is NMEA, and the other two are AT and RMNET/QMI.
=20
What kind of endpoints are on the RMNET/QMI interface?
see Elly's cleaned up Gobi driver here: http://lwn.net/Articles/439=
173/
Post by Bjørn Mork
Post by Dan Williams
as it has the logic to set everything up for Gobi devices. I expec=
t
Post by Bjørn Mork
Post by Dan Williams
that it would work with the UML290 as well if things were hacked up=
a
Post by Bjørn Mork
Post by Dan Williams
bit. It should be pretty clear here how to talk to them.
0: (ff/ff/ff) (in/out/interrupt) - rmnet/QMI
1: (ff/ff/ff) (in/out) - DIAG
2: (ff/ff/ff) (in/out) - AT commands & PPP
3: (ff/ff/ff) (in/out/interrupt) - NMEA (?)
lsusb attached for both a Gobi 2K device and the UML290.
If you have these devices, then it would be useful to verify that =
the
Post by Bjørn Mork
Post by Dan Williams
cdc-wdm driver can be used to provide access to QMI without any fu=
rther
Post by Bjørn Mork
Post by Dan Williams
changes. This should in theory just work if you unload any other
drivers binding to the RMNET/QMI interface, and bind cdc-wdm to it
instead. This requires that you have the minimum buffer size patc=
h
Post by Bjørn Mork
Post by Dan Williams
installed, but should not require any other changes.
I tried that quickly but of course the WDM driver fails with EINVAL
because there are 3 endpoints on the RMNET/QMI interface (bulk
rv =3D -EINVAL;
iface =3D intf->cur_altsetting;
if (iface->desc.bNumEndpoints !=3D 1)
goto err;
I briefly thought about hacking it to find the interrupt endpoing, =
but
Post by Bjørn Mork
Post by Dan Williams
if the cdc-wdm driver apparently only cares about 1 endpoint there'=
s not
Post by Bjørn Mork
Post by Dan Williams
much point to handing it an interface with 3 I don't think...
=20
Right, forgot about that part. You could hack it so that it was happ=
ily
Post by Bjørn Mork
ignoring the extra bulk endpoints as long as it found an interrupt
endpoint.=20
Ok, I'll try that. I'll just do a loop to find the interrupt endpoint
and see if I can talk QMI to the devices.
Post by Bjørn Mork
=20
Post by Dan Williams
Post by Dan Williams
I think so far the Huawei device is the only one that I've seen =
that
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
exposes descriptors that are quasi-CDC at all. How should we ha=
ndle the
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
rest of these?
=20
Good question. Guess I'm going to see if I can make qmi_wwan and
cdc-wdm share an interface after all.
Also you may not have seen, but the QUIC codeaurora people pushed a=
new
Post by Bjørn Mork
Post by Dan Williams
https://www.codeaurora.org/gitweb/quic/le/?p=3Dkernel/msm.git;a=3Dc=
ommit;h=3D37c35e4ee5226099374a1a1eda637d0f26fc023d
Post by Bjørn Mork
=20
Thanks for the pointer. Will take a look at it. But I guess these a=
re
Post by Bjørn Mork
still doing the integrated QMI approach. Which most likely isn't
acceptable to anyone here, after you managed to convince me :-)
Yeah, its the integrated approach. Obviously not quite what we care
about but just used as a reference.
Post by Bjørn Mork
Post by Dan Williams
In the end, if the Huawei device mostly presents itself as WDM, and
other Gobi/MSM8xxx/MSM9xxx devices present themselves like Gobi car=
ds
Post by Bjørn Mork
Post by Dan Williams
do, perhaps we should have separate drivers? Does it make sense to=
keep
Post by Bjørn Mork
Post by Dan Williams
hacking up CDC-WDM for devices that are clearly not exposed that wa=
y,
Post by Bjørn Mork
Post by Dan Williams
even if the internal operation would be similar? Also in the end t=
hey
Post by Bjørn Mork
Post by Dan Williams
are all just character devices to talk QMI so it doesn't really mat=
ter
Post by Bjørn Mork
Post by Dan Williams
what driver exposes that interface. It would be nice to share if w=
e
Post by Bjørn Mork
Post by Dan Williams
can, but making too many changes to WDM probably isn't the right
approach either.
=20
That is for Oliver to decide I guess. =20
=20
But I believe it can make sense to reuse the cdc-wdm driver, even if =
it
Post by Bjørn Mork
needs some small changes to allow other drivers to call it. The actu=
al
Post by Bjørn Mork
work it does is pretty standalone: Listen on the interrupt ep, send
control messages and do file operations on the char device. Provided
the other netdev drivers don't need notifications, then this can all =
be
Post by Bjørn Mork
done without them being involved at all. The only issues are
register/deregister instead of probe/disconnect, and doing the mappin=
g
Post by Bjørn Mork
to the device specific struct. =20
=20
And having all these devices expose their QMI interface as a
/dev/cdc-wdmX should make things look more consistent for users and
userspace.
Agreed.

Dan

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-01-23 23:51:58 UTC
Permalink
This is coded up in a couple of hours, so it's a bit rough. Even worse =
than=20
my average code...

And it's not exactly tested. But I have *tried* it with success. So
it is sort of a demo of how little really is necessary to make this
concept fly.

I tested it using the "Windows mode" of my Huawei modem. This makes th=
e
wwan/qmi interface look like this:

Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 3
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 1=20
bInterfaceProtocol 17=20
iInterface 0=20
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x85 EP 5 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 5
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x86 EP 6 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 32
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x04 EP 4 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 32


binding:

Jan 24 00:33:30 nemi kernel: [ 3097.680318] usb 2-1: new high-speed USB=
device number 6 using ehci_hcd
Jan 24 00:33:30 nemi kernel: [ 3097.815574] usb 2-1: New USB device fou=
nd, idVendor=3D12d1, idProduct=3D1506
Jan 24 00:33:30 nemi kernel: [ 3097.815585] usb 2-1: New USB device str=
ings: Mfr=3D3, Product=3D2, SerialNumber=3D0
Jan 24 00:33:30 nemi kernel: [ 3097.815592] usb 2-1: Product: HUAWEI Mo=
bile
Jan 24 00:33:30 nemi kernel: [ 3097.815598] usb 2-1: Manufacturer: Huaw=
ei Technologies
Jan 24 00:33:30 nemi kernel: [ 3097.818088] option 2-1:1.0: GSM modem (=
1-port) converter detected
Jan 24 00:33:30 nemi kernel: [ 3097.818371] usb 2-1: GSM modem (1-port)=
converter now attached to ttyUSB0
Jan 24 00:33:30 nemi kernel: [ 3097.818624] option 2-1:1.1: GSM modem (=
1-port) converter detected
Jan 24 00:33:30 nemi kernel: [ 3097.818843] usb 2-1: GSM modem (1-port)=
converter now attached to ttyUSB1
Jan 24 00:33:30 nemi kernel: [ 3097.819061] option 2-1:1.2: GSM modem (=
1-port) converter detected
Jan 24 00:33:30 nemi kernel: [ 3097.819253] usb 2-1: GSM modem (1-port)=
converter now attached to ttyUSB2
Jan 24 00:33:30 nemi kernel: [ 3097.819657] qmi_wwan 2-1:1.3: cdc-wdm2:=
USB WDM device
Jan 24 00:33:30 nemi kernel: [ 3097.822481] qmi_wwan 2-1:1.3: wwan1: re=
gister 'qmi_wwan' at usb-0000:00:1d.7-1, QMI speaking wwan device with =
combined interface, 0e:b8:5e:cd:9c:78
Jan 24 00:33:30 nemi kernel: [ 3097.823777] scsi8 : usb-storage 2-1:1.4
Jan 24 00:33:30 nemi kernel: [ 3097.824601] scsi9 : usb-storage 2-1:1.5
Jan 24 00:33:31 nemi kernel: [ 3098.827129] scsi 9:0:0:0: Direct-Access=
HUAWEI SD Storage 2.31 PQ: 0 ANSI: 2
Jan 24 00:33:31 nemi kernel: [ 3098.827142] scsi 8:0:0:0: CD-ROM =
HUAWEI Mass Storage 2.31 PQ: 0 ANSI: 0
Jan 24 00:33:32 nemi kernel: [ 3098.832089] sd 9:0:0:0: [sdb] Attached =
SCSI removable disk
Jan 24 00:33:32 nemi kernel: [ 3098.858700] sr1: scsi-1 drive
Jan 24 00:33:32 nemi kernel: [ 3098.859251] sr 8:0:0:0: Attached scsi C=
D-ROM sr1

Connecting:

nemi:/home/bjorn# ifup wwan1
wwan1: will use /dev/cdc-wdm2 for management
wwan1: Manufacturer: QUALCOMM INCORPORATED
wwan1: Revision: M9200B-SCAQDBZD-3.0.350025T 1 [Aug 11 2011 02:00:00]
wwan1: QMI_WDS cid=3D1, wds_handle=3D0x00000000
wwan1: PIN1 status: enabled, not veri=EF=AC=81ed, verify_left: 3, unblo=
ck_left: 10
wwan1: PIN2 status: enabled, not veri=EF=AC=81ed, verify_left: 2, unblo=
ck_left: 10
wwan1: less than 3 verification attempts left for PIN2 - must be entere=
d manually!
wwan1: connecting...
wwan1: got QMI_WDS handle 0x021590e8
wwan1: not releasing QMI_WDS cid=3D1 while connected
wwan1: released QMI_DMS cid=3D1 with status=3D0
Internet Systems Consortium DHCP Client 4.1.1-P1
Copyright 2004-2010 Internet Systems Consortium.
All rights reserved.
=46or info, please visit https://www.isc.org/software/dhcp/

Listening on LPF/wwan1/0e:b8:5e:cd:9c:78
Sending on LPF/wwan1/0e:b8:5e:cd:9c:78
Sending on Socket/fallback
DHCPDISCOVER on wwan1 to 255.255.255.255 port 67 interval 4
DHCPOFFER from 10.152.3.57
DHCPREQUEST on wwan1 to 255.255.255.255 port 67
DHCPACK from 10.152.3.57
bound to 10.152.3.59 -- renewal in 3546 seconds.


with some related kernel log messages:

Jan 24 00:35:06 nemi kernel: [ 3193.653155] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:06 nemi kernel: [ 3193.653167] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:06 nemi kernel: [ 3193.655155] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:06 nemi kernel: [ 3193.655164] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:06 nemi kernel: [ 3193.657171] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:06 nemi kernel: [ 3193.657180] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:06 nemi kernel: [ 3193.659153] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:06 nemi kernel: [ 3193.659160] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:06 nemi kernel: [ 3193.661173] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:06 nemi kernel: [ 3193.661182] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:06 nemi kernel: [ 3193.663153] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:06 nemi kernel: [ 3193.663160] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:06 nemi kernel: [ 3193.665172] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:06 nemi kernel: [ 3193.665180] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:07 nemi kernel: [ 3193.665205] qmi_wwan 2-1:1.3: Tx URB ha=
s been submitted index=3D3
Jan 24 00:35:07 nemi kernel: [ 3194.665609] qmi_wwan 2-1:1.3: wdm_relea=
se: cleanup
Jan 24 00:35:07 nemi kernel: [ 3194.665961] wdm_get_device_by_minor: lo=
oking at 2-1:1.3 (ffff88022fef8400)
Jan 24 00:35:07 nemi kernel: [ 3194.665969] wdm_get_device_by_minor: re=
turning desc->intf=3Dffff88022fef8400 for minor=3D2
Jan 24 00:35:07 nemi kernel: [ 3194.666143] qmi_wwan 2-1:1.3: Tx URB ha=
s been submitted index=3D3
Jan 24 00:35:07 nemi kernel: [ 3194.669191] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:07 nemi kernel: [ 3194.669208] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:07 nemi kernel: [ 3194.669854] qmi_wwan 2-1:1.3: wdm_relea=
se: cleanup
Jan 24 00:35:07 nemi kernel: [ 3194.670096] wdm_get_device_by_minor: lo=
oking at 2-1:1.3 (ffff88022fef8400)
Jan 24 00:35:07 nemi kernel: [ 3194.670104] wdm_get_device_by_minor: re=
turning desc->intf=3Dffff88022fef8400 for minor=3D2
Jan 24 00:35:07 nemi kernel: [ 3194.670259] qmi_wwan 2-1:1.3: Tx URB ha=
s been submitted index=3D3
Jan 24 00:35:07 nemi kernel: [ 3194.675171] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:07 nemi kernel: [ 3194.675186] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:07 nemi kernel: [ 3194.675808] qmi_wwan 2-1:1.3: wdm_relea=
se: cleanup
Jan 24 00:35:07 nemi kernel: [ 3194.676085] wdm_get_device_by_minor: lo=
oking at 2-1:1.3 (ffff88022fef8400)
Jan 24 00:35:07 nemi kernel: [ 3194.676093] wdm_get_device_by_minor: re=
turning desc->intf=3Dffff88022fef8400 for minor=3D2
Jan 24 00:35:07 nemi kernel: [ 3194.676321] qmi_wwan 2-1:1.3: Tx URB ha=
s been submitted index=3D3
Jan 24 00:35:07 nemi kernel: [ 3194.679168] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:07 nemi kernel: [ 3194.679181] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:07 nemi kernel: [ 3194.679794] qmi_wwan 2-1:1.3: wdm_relea=
se: cleanup
Jan 24 00:35:07 nemi kernel: [ 3194.680056] wdm_get_device_by_minor: lo=
oking at 2-1:1.3 (ffff88022fef8400)
Jan 24 00:35:07 nemi kernel: [ 3194.680064] wdm_get_device_by_minor: re=
turning desc->intf=3Dffff88022fef8400 for minor=3D2
Jan 24 00:35:07 nemi kernel: [ 3194.680190] qmi_wwan 2-1:1.3: Tx URB ha=
s been submitted index=3D3
Jan 24 00:35:07 nemi kernel: [ 3194.683182] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:07 nemi kernel: [ 3194.683195] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:07 nemi kernel: [ 3194.683806] qmi_wwan 2-1:1.3: wdm_relea=
se: cleanup
Jan 24 00:35:07 nemi kernel: [ 3194.684200] wdm_get_device_by_minor: lo=
oking at 2-1:1.3 (ffff88022fef8400)
Jan 24 00:35:07 nemi kernel: [ 3194.684209] wdm_get_device_by_minor: re=
turning desc->intf=3Dffff88022fef8400 for minor=3D2
Jan 24 00:35:07 nemi kernel: [ 3194.684333] qmi_wwan 2-1:1.3: Tx URB ha=
s been submitted index=3D3
Jan 24 00:35:07 nemi kernel: [ 3194.687169] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:07 nemi kernel: [ 3194.687183] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:07 nemi kernel: [ 3194.687787] qmi_wwan 2-1:1.3: wdm_relea=
se: cleanup
Jan 24 00:35:07 nemi kernel: [ 3194.687973] wdm_get_device_by_minor: lo=
oking at 2-1:1.3 (ffff88022fef8400)
Jan 24 00:35:07 nemi kernel: [ 3194.687981] wdm_get_device_by_minor: re=
turning desc->intf=3Dffff88022fef8400 for minor=3D2
Jan 24 00:35:07 nemi kernel: [ 3194.688121] qmi_wwan 2-1:1.3: Tx URB ha=
s been submitted index=3D3
Jan 24 00:35:07 nemi kernel: [ 3194.691169] qmi_wwan 2-1:1.3: NOTIFY_RE=
SPONSE_AVAILABLE received: index 3 len 0
Jan 24 00:35:07 nemi kernel: [ 3194.691183] qmi_wwan 2-1:1.3: wdm_int_c=
allback: usb_submit_urb 0
Jan 24 00:35:07 nemi kernel: [ 3194.691792] qmi_wwan 2-1:1.3: wdm_relea=
se: cleanup
Jan 24 00:35:07 nemi kernel: [ 3194.692106] wdm_get_device_by_minor: lo=
oking at 2-1:1.3 (ffff88022fef8400)
Jan 24 00:35:07 nemi kernel: [ 3194.692114] wdm_get_device_by_minor: re=
turning desc->intf=3Dffff88022fef8400 for minor=3D2
Jan 24 00:35:07 nemi kernel: [ 3194.693048] qmi_wwan 2-1:1.3: Tx URB ha=
s been submitted index=3D3


Looking at the sysfs:

nemi:/home/bjorn# ls -l /sys/bus/usb/devices/2-1:1.3/
total 0
-r--r--r-- 1 root root 4096 Jan 24 00:34 bAlternateSetting
-r--r--r-- 1 root root 4096 Jan 24 00:33 bInterfaceClass
-r--r--r-- 1 root root 4096 Jan 24 00:33 bInterfaceNumber
-r--r--r-- 1 root root 4096 Jan 24 00:34 bInterfaceProtocol
-r--r--r-- 1 root root 4096 Jan 24 00:34 bInterfaceSubClass
-r--r--r-- 1 root root 4096 Jan 24 00:34 bNumEndpoints
lrwxrwxrwx 1 root root 0 Jan 24 00:33 driver -> ../../../../../../bu=
s/usb/drivers/qmi_wwan
drwxr-xr-x 3 root root 0 Jan 24 00:34 ep_04
drwxr-xr-x 3 root root 0 Jan 24 00:34 ep_85
drwxr-xr-x 3 root root 0 Jan 24 00:34 ep_86
-r--r--r-- 1 root root 4096 Jan 24 00:34 modalias
drwxr-xr-x 3 root root 0 Jan 24 00:33 net
drwxr-xr-x 2 root root 0 Jan 24 00:34 power
lrwxrwxrwx 1 root root 0 Jan 24 00:33 subsystem -> ../../../../../..=
/bus/usb
-r--r--r-- 1 root root 4096 Jan 24 00:34 supports_autosuspend
-rw-r--r-- 1 root root 4096 Jan 24 00:33 uevent
drwxr-xr-x 3 root root 0 Jan 24 00:33 usb

nemi:/home/bjorn# ls -l /sys/bus/usb/devices/2-1:1.3/{net,usb}
/sys/bus/usb/devices/2-1:1.3/net:
total 0
drwxr-xr-x 5 root root 0 Jan 24 00:33 wwan1

/sys/bus/usb/devices/2-1:1.3/usb:
total 0
drwxr-xr-x 3 root root 0 Jan 24 00:33 cdc-wdm2


What do you think? Continue with something like this or not?


Oh, yeah, the Ericsson modem isn't broken either:

***@nemi:~$ echo -e "ATI\r" >/dev/cdc-wdm0 && cat /dev/cdc-wdm0

*EMRDY: 1
ATI
=463507g

OK
ATI
=463507g

OK
^C



Bj=C3=B8rn Mork (6):
usb: cdc-wdm: split out reusable parts of probe and destroy
usb: cdc-wdm: prepare endpoint detection for interfaces with > 1
endpoint
usb: cdc-wdm: create an extra indirection when looking up device data
usb: cdc-wdm: adding interface =3D> wdm_device list lookup
usb: cdc-wdm: adding register/deregister
net: usb: qmi_wwan: add support for devices sharing interface for QMI
and wwan

drivers/net/usb/qmi_wwan.c | 44 ++++++++
drivers/usb/class/cdc-wdm.c | 239 ++++++++++++++++++++++++++++++++---=
--------
drivers/usb/class/cdc-wdm.h | 10 ++
3 files changed, 232 insertions(+), 61 deletions(-)
create mode 100644 drivers/usb/class/cdc-wdm.h

--=20
1.7.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dan Williams
2012-01-24 22:29:42 UTC
Permalink
Post by Bjørn Mork
=20
Post by Dan Williams
=20
Post by Dan Williams
0 (2/2/1): CDC-ACM for AT commands
1 (10/0/0): CDC-DATA for interface 0
2 (ff/ff/ff): Qualcomm DIAG
3 (ff/fd/ff): NMEA
4 (ff/fe/ff): Pantech WMC
5 (ff/f0/ff): RMNET/QMI port
=20
I assume this is after som modeswitch command? The same as the Wi=
ndows
Post by Bjørn Mork
Post by Dan Williams
driver uses? And you don't know if there are other options?
The UML290 does not require modeswitching at all.
=20
OK, nice.
=20
Post by Dan Williams
Post by Dan Williams
Interface 5 is obviously the one we want here. And the WDM driv=
er is
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
only looking for certain descriptors. Do we hack CDC-WDM and qm=
i_wwan
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
up for these types of devices?
=20
=20
Interface 5 has three endpoints? Bulk in/out and interrupt in?
Yes, three endpoints like you describe.
OK, this is where the fun begins. I knew there was some reason wh=
y I
Post by Bjørn Mork
Post by Dan Williams
was struggling with the interface sharing... I guess we cannot ex=
pect
Post by Bjørn Mork
Post by Dan Williams
every vendor to provide a "Linux mode" with two separate interface=
s for
Post by Bjørn Mork
Post by Dan Williams
the RMNET/QMI port.
=20
Post by Dan Williams
Second, on Gobi devices, we have four USB interfaces, all FF/FF/=
=46F.
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
Intf 0 and 2 have interrupt endpoints. One of them is a DIAG in=
terface,
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
one is NMEA, and the other two are AT and RMNET/QMI.
=20
What kind of endpoints are on the RMNET/QMI interface?
see Elly's cleaned up Gobi driver here: http://lwn.net/Articles/439=
173/
Post by Bjørn Mork
Post by Dan Williams
as it has the logic to set everything up for Gobi devices. I expec=
t
Post by Bjørn Mork
Post by Dan Williams
that it would work with the UML290 as well if things were hacked up=
a
Post by Bjørn Mork
Post by Dan Williams
bit. It should be pretty clear here how to talk to them.
0: (ff/ff/ff) (in/out/interrupt) - rmnet/QMI
1: (ff/ff/ff) (in/out) - DIAG
2: (ff/ff/ff) (in/out) - AT commands & PPP
3: (ff/ff/ff) (in/out/interrupt) - NMEA (?)
lsusb attached for both a Gobi 2K device and the UML290.
If you have these devices, then it would be useful to verify that =
the
Post by Bjørn Mork
Post by Dan Williams
cdc-wdm driver can be used to provide access to QMI without any fu=
rther
Post by Bjørn Mork
Post by Dan Williams
changes. This should in theory just work if you unload any other
drivers binding to the RMNET/QMI interface, and bind cdc-wdm to it
instead. This requires that you have the minimum buffer size patc=
h
Post by Bjørn Mork
Post by Dan Williams
installed, but should not require any other changes.
I tried that quickly but of course the WDM driver fails with EINVAL
because there are 3 endpoints on the RMNET/QMI interface (bulk
rv =3D -EINVAL;
iface =3D intf->cur_altsetting;
if (iface->desc.bNumEndpoints !=3D 1)
goto err;
I briefly thought about hacking it to find the interrupt endpoing, =
but
Post by Bjørn Mork
Post by Dan Williams
if the cdc-wdm driver apparently only cares about 1 endpoint there'=
s not
Post by Bjørn Mork
Post by Dan Williams
much point to handing it an interface with 3 I don't think...
=20
Right, forgot about that part. You could hack it so that it was happ=
ily
Post by Bjørn Mork
ignoring the extra bulk endpoints as long as it found an interrupt
endpoint.=20
=20
=20
Post by Dan Williams
Post by Dan Williams
I think so far the Huawei device is the only one that I've seen =
that
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
exposes descriptors that are quasi-CDC at all. How should we ha=
ndle the
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
rest of these?
=20
Good question. Guess I'm going to see if I can make qmi_wwan and
cdc-wdm share an interface after all.
Also you may not have seen, but the QUIC codeaurora people pushed a=
new
Post by Bjørn Mork
Post by Dan Williams
https://www.codeaurora.org/gitweb/quic/le/?p=3Dkernel/msm.git;a=3Dc=
ommit;h=3D37c35e4ee5226099374a1a1eda637d0f26fc023d
Post by Bjørn Mork
=20
Thanks for the pointer. Will take a look at it. But I guess these a=
re
Post by Bjørn Mork
still doing the integrated QMI approach. Which most likely isn't
acceptable to anyone here, after you managed to convince me :-)
=20
Post by Dan Williams
In the end, if the Huawei device mostly presents itself as WDM, and
other Gobi/MSM8xxx/MSM9xxx devices present themselves like Gobi car=
ds
Post by Bjørn Mork
Post by Dan Williams
do, perhaps we should have separate drivers? Does it make sense to=
keep
Post by Bjørn Mork
Post by Dan Williams
hacking up CDC-WDM for devices that are clearly not exposed that wa=
y,
Post by Bjørn Mork
Post by Dan Williams
even if the internal operation would be similar? Also in the end t=
hey
Post by Bjørn Mork
Post by Dan Williams
are all just character devices to talk QMI so it doesn't really mat=
ter
Post by Bjørn Mork
Post by Dan Williams
what driver exposes that interface. It would be nice to share if w=
e
Post by Bjørn Mork
Post by Dan Williams
can, but making too many changes to WDM probably isn't the right
approach either.
=20
That is for Oliver to decide I guess. =20
=20
But I believe it can make sense to reuse the cdc-wdm driver, even if =
it
Post by Bjørn Mork
needs some small changes to allow other drivers to call it. The actu=
al
Post by Bjørn Mork
work it does is pretty standalone: Listen on the interrupt ep, send
control messages and do file operations on the char device. Provided
the other netdev drivers don't need notifications, then this can all =
be
Post by Bjørn Mork
done without them being involved at all. The only issues are
register/deregister instead of probe/disconnect, and doing the mappin=
g
Post by Bjørn Mork
to the device specific struct. =20
=20
And having all these devices expose their QMI interface as a
/dev/cdc-wdmX should make things look more consistent for users and
userspace.
The following patch to cdc-wdm allows me to talk to the UML290 with QMI=
=2E
Quite straightforward, just find the interrupt endpoint like you said.
Any comments on it, or should I submit as a format patch?

One oddity is that cdc-wdm seems to be looking at interfaces 3 and 4
too, but it shouldn't because the device list says to look only at
interface 5 (ff/f0/ff). Any idea why that's happening? 3 and 4 should
be driven by 'qcaux' not cdc-wdm.

[ 5524.707877] cdc_acm 2-1:1.0: ttyACM0: USB ACM device
[ 5524.708969] qcaux 2-1:1.2: qcaux converter detected
[ 5524.709205] usb 2-1: qcaux converter now attached to ttyUSB1
[ 5524.709346] cdc_wdm 2-1:1.3: failed to find interrupt endpoint
[ 5524.709356] cdc_wdm: probe of 2-1:1.3 failed with error -22
[ 5524.709484] cdc_wdm 2-1:1.4: failed to find interrupt endpoint
[ 5524.709493] cdc_wdm: probe of 2-1:1.4 failed with error -22
[ 5524.709793] cdc_wdm 2-1:1.5: cdc-wdm0: USB WDM device

Dan

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index efe6849..f5b07e7 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -38,6 +47,15 @@ static const struct usb_device_id wdm_ids[] =3D {
.bInterfaceClass =3D USB_CLASS_COMM,
.bInterfaceSubClass =3D USB_CDC_SUBCLASS_DMM
},
+ {
+ .match_flags =3D USB_DEVICE_ID_MATCH_VENDOR | US=
B_DEVICE_ID_MATCH_PRODUCT,
+ USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor =3D 0x106c,
+ .idProduct =3D 0x3718,
+ .bInterfaceClass =3D 0xFF,
+ .bInterfaceSubClass =3D 0xF0,
+ .bInterfaceProtocol =3D 0xFF
+ },
{ }
};
=20
@@ -626,11 +652,12 @@ static int wdm_probe(struct usb_interface *intf, =
const struct usb_device_id *id)
struct usb_device *udev =3D interface_to_usbdev(intf);
struct wdm_device *desc;
struct usb_host_interface *iface;
- struct usb_endpoint_descriptor *ep;
+ struct usb_endpoint_descriptor *ep =3D NULL, *tmpep;
struct usb_cdc_dmm_desc *dmhd;
u8 *buffer =3D intf->altsetting->extra;
int buflen =3D intf->altsetting->extralen;
u16 maxcom =3D WDM_DEFAULT_BUFSIZE;
+ int i;
=20
if (!buffer)
goto out;
@@ -661,6 +688,22 @@ next_desc:
buffer +=3D buffer[0];
}
=20
+ /* Find the interrupt endpoint */
+ rv =3D -EINVAL;
+ iface =3D intf->cur_altsetting;
+ for (i =3D 0; i < iface->desc.bNumEndpoints; i++) {
+ tmpep =3D &iface->endpoint[i].desc;
+ if (tmpep && usb_endpoint_is_int_in (tmpep)) {
+ ep =3D tmpep;
+ break;
+ }
+ }
+
+ if (!ep) {
+ dev_dbg(&intf->dev, "failed to find interrupt endpoint\n");
+ goto out;
+ }
+
rv =3D -ENOMEM;
desc =3D kzalloc(sizeof(struct wdm_device), GFP_KERNEL);
if (!desc)
@@ -674,13 +717,6 @@ next_desc:
desc->intf =3D intf;
INIT_WORK(&desc->rxwork, wdm_rxwork);
=20
- rv =3D -EINVAL;
- iface =3D intf->cur_altsetting;
- if (iface->desc.bNumEndpoints !=3D 1)
- goto err;
- ep =3D &iface->endpoint[0].desc;
- if (!ep || !usb_endpoint_is_int_in(ep))
- goto err;
=20
desc->wMaxPacketSize =3D usb_endpoint_maxp(ep);
desc->bMaxPacketSize0 =3D udev->descriptor.bMaxPacketSize0;


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dan Williams
2012-01-24 23:09:28 UTC
Permalink
Post by Bjørn Mork
=20
Post by Dan Williams
=20
Post by Dan Williams
0 (2/2/1): CDC-ACM for AT commands
1 (10/0/0): CDC-DATA for interface 0
2 (ff/ff/ff): Qualcomm DIAG
3 (ff/fd/ff): NMEA
4 (ff/fe/ff): Pantech WMC
5 (ff/f0/ff): RMNET/QMI port
=20
I assume this is after som modeswitch command? The same as the =
Windows
Post by Bjørn Mork
Post by Dan Williams
driver uses? And you don't know if there are other options?
The UML290 does not require modeswitching at all.
=20
OK, nice.
=20
Post by Dan Williams
Post by Dan Williams
Interface 5 is obviously the one we want here. And the WDM dr=
iver is
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
only looking for certain descriptors. Do we hack CDC-WDM and =
qmi_wwan
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
up for these types of devices?
=20
=20
Interface 5 has three endpoints? Bulk in/out and interrupt in?
Yes, three endpoints like you describe.
OK, this is where the fun begins. I knew there was some reason =
why I
Post by Bjørn Mork
Post by Dan Williams
was struggling with the interface sharing... I guess we cannot =
expect
Post by Bjørn Mork
Post by Dan Williams
every vendor to provide a "Linux mode" with two separate interfa=
ces for
Post by Bjørn Mork
Post by Dan Williams
the RMNET/QMI port.
=20
Post by Dan Williams
Second, on Gobi devices, we have four USB interfaces, all FF/F=
=46/FF.
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
Intf 0 and 2 have interrupt endpoints. One of them is a DIAG =
interface,
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
one is NMEA, and the other two are AT and RMNET/QMI.
=20
What kind of endpoints are on the RMNET/QMI interface?
see Elly's cleaned up Gobi driver here: http://lwn.net/Articles/4=
39173/
Post by Bjørn Mork
Post by Dan Williams
as it has the logic to set everything up for Gobi devices. I exp=
ect
Post by Bjørn Mork
Post by Dan Williams
that it would work with the UML290 as well if things were hacked =
up a
Post by Bjørn Mork
Post by Dan Williams
bit. It should be pretty clear here how to talk to them.
0: (ff/ff/ff) (in/out/interrupt) - rmnet/QMI
1: (ff/ff/ff) (in/out) - DIAG
2: (ff/ff/ff) (in/out) - AT commands & PPP
3: (ff/ff/ff) (in/out/interrupt) - NMEA (?)
lsusb attached for both a Gobi 2K device and the UML290.
If you have these devices, then it would be useful to verify tha=
t the
Post by Bjørn Mork
Post by Dan Williams
cdc-wdm driver can be used to provide access to QMI without any =
further
Post by Bjørn Mork
Post by Dan Williams
changes. This should in theory just work if you unload any othe=
r
Post by Bjørn Mork
Post by Dan Williams
drivers binding to the RMNET/QMI interface, and bind cdc-wdm to =
it
Post by Bjørn Mork
Post by Dan Williams
instead. This requires that you have the minimum buffer size pa=
tch
Post by Bjørn Mork
Post by Dan Williams
installed, but should not require any other changes.
I tried that quickly but of course the WDM driver fails with EINV=
AL
Post by Bjørn Mork
Post by Dan Williams
because there are 3 endpoints on the RMNET/QMI interface (bulk
rv =3D -EINVAL;
iface =3D intf->cur_altsetting;
if (iface->desc.bNumEndpoints !=3D 1)
goto err;
I briefly thought about hacking it to find the interrupt endpoing=
, but
Post by Bjørn Mork
Post by Dan Williams
if the cdc-wdm driver apparently only cares about 1 endpoint ther=
e's not
Post by Bjørn Mork
Post by Dan Williams
much point to handing it an interface with 3 I don't think...
=20
Right, forgot about that part. You could hack it so that it was ha=
ppily
Post by Bjørn Mork
ignoring the extra bulk endpoints as long as it found an interrupt
endpoint.=20
=20
=20
Post by Dan Williams
Post by Dan Williams
I think so far the Huawei device is the only one that I've see=
n that
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
exposes descriptors that are quasi-CDC at all. How should we =
handle the
Post by Bjørn Mork
Post by Dan Williams
Post by Dan Williams
rest of these?
=20
Good question. Guess I'm going to see if I can make qmi_wwan an=
d
Post by Bjørn Mork
Post by Dan Williams
cdc-wdm share an interface after all.
Also you may not have seen, but the QUIC codeaurora people pushed=
a new
Post by Bjørn Mork
Post by Dan Williams
https://www.codeaurora.org/gitweb/quic/le/?p=3Dkernel/msm.git;a=3D=
commit;h=3D37c35e4ee5226099374a1a1eda637d0f26fc023d
Post by Bjørn Mork
=20
Thanks for the pointer. Will take a look at it. But I guess these=
are
Post by Bjørn Mork
still doing the integrated QMI approach. Which most likely isn't
acceptable to anyone here, after you managed to convince me :-)
=20
Post by Dan Williams
In the end, if the Huawei device mostly presents itself as WDM, a=
nd
Post by Bjørn Mork
Post by Dan Williams
other Gobi/MSM8xxx/MSM9xxx devices present themselves like Gobi c=
ards
Post by Bjørn Mork
Post by Dan Williams
do, perhaps we should have separate drivers? Does it make sense =
to keep
Post by Bjørn Mork
Post by Dan Williams
hacking up CDC-WDM for devices that are clearly not exposed that =
way,
Post by Bjørn Mork
Post by Dan Williams
even if the internal operation would be similar? Also in the end=
they
Post by Bjørn Mork
Post by Dan Williams
are all just character devices to talk QMI so it doesn't really m=
atter
Post by Bjørn Mork
Post by Dan Williams
what driver exposes that interface. It would be nice to share if=
we
Post by Bjørn Mork
Post by Dan Williams
can, but making too many changes to WDM probably isn't the right
approach either.
=20
That is for Oliver to decide I guess. =20
=20
But I believe it can make sense to reuse the cdc-wdm driver, even i=
f it
Post by Bjørn Mork
needs some small changes to allow other drivers to call it. The ac=
tual
Post by Bjørn Mork
work it does is pretty standalone: Listen on the interrupt ep, send
control messages and do file operations on the char device. Provid=
ed
Post by Bjørn Mork
the other netdev drivers don't need notifications, then this can al=
l be
Post by Bjørn Mork
done without them being involved at all. The only issues are
register/deregister instead of probe/disconnect, and doing the mapp=
ing
Post by Bjørn Mork
to the device specific struct. =20
=20
And having all these devices expose their QMI interface as a
/dev/cdc-wdmX should make things look more consistent for users and
userspace.
=20
The following patch to cdc-wdm allows me to talk to the UML290 with Q=
MI.
Quite straightforward, just find the interrupt endpoint like you said=
=2E
Any comments on it, or should I submit as a format patch?
=20
One oddity is that cdc-wdm seems to be looking at interfaces 3 and 4
too, but it shouldn't because the device list says to look only at
interface 5 (ff/f0/ff). Any idea why that's happening? 3 and 4 shou=
ld
be driven by 'qcaux' not cdc-wdm.
=20
[ 5524.707877] cdc_acm 2-1:1.0: ttyACM0: USB ACM device
[ 5524.708969] qcaux 2-1:1.2: qcaux converter detected
[ 5524.709205] usb 2-1: qcaux converter now attached to ttyUSB1
[ 5524.709346] cdc_wdm 2-1:1.3: failed to find interrupt endpoint
[ 5524.709356] cdc_wdm: probe of 2-1:1.3 failed with error -22
[ 5524.709484] cdc_wdm 2-1:1.4: failed to find interrupt endpoint
[ 5524.709493] cdc_wdm: probe of 2-1:1.4 failed with error -22
[ 5524.709793] cdc_wdm 2-1:1.5: cdc-wdm0: USB WDM device
I'm also able to talk to my Gobi 2000 card, but I need to restrict the
driver to interface 0 because there are two interfaces with interrupt
endpoints, and both are ff/ff/ff so USB_DEVICE_AND_INTERFACE_INFO
matching doesn't help us there. I suppose we could start putting the
interface number into the driver_data field and using that in
wdm_probe() if it's present.

Dan
=20
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.=
c
index efe6849..f5b07e7 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -38,6 +47,15 @@ static const struct usb_device_id wdm_ids[] =3D {
.bInterfaceClass =3D USB_CLASS_COMM,
.bInterfaceSubClass =3D USB_CDC_SUBCLASS_DMM
},
+ {
+ .match_flags =3D USB_DEVICE_ID_MATCH_VENDOR | =
USB_DEVICE_ID_MATCH_PRODUCT,
+ USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor =3D 0x106c,
+ .idProduct =3D 0x3718,
+ .bInterfaceClass =3D 0xFF,
+ .bInterfaceSubClass =3D 0xF0,
+ .bInterfaceProtocol =3D 0xFF
+ },
{ }
};
=20
@@ -626,11 +652,12 @@ static int wdm_probe(struct usb_interface *intf=
, const struct usb_device_id *id)
struct usb_device *udev =3D interface_to_usbdev(intf);
struct wdm_device *desc;
struct usb_host_interface *iface;
- struct usb_endpoint_descriptor *ep;
+ struct usb_endpoint_descriptor *ep =3D NULL, *tmpep;
struct usb_cdc_dmm_desc *dmhd;
u8 *buffer =3D intf->altsetting->extra;
int buflen =3D intf->altsetting->extralen;
u16 maxcom =3D WDM_DEFAULT_BUFSIZE;
+ int i;
=20
if (!buffer)
goto out;
buffer +=3D buffer[0];
}
=20
+ /* Find the interrupt endpoint */
+ rv =3D -EINVAL;
+ iface =3D intf->cur_altsetting;
+ for (i =3D 0; i < iface->desc.bNumEndpoints; i++) {
+ tmpep =3D &iface->endpoint[i].desc;
+ if (tmpep && usb_endpoint_is_int_in (tmpep)) {
+ ep =3D tmpep;
+ break;
+ }
+ }
+
+ if (!ep) {
+ dev_dbg(&intf->dev, "failed to find interrupt endpoint\n");
+ goto out;
+ }
+
rv =3D -ENOMEM;
desc =3D kzalloc(sizeof(struct wdm_device), GFP_KERNEL);
if (!desc)
desc->intf =3D intf;
INIT_WORK(&desc->rxwork, wdm_rxwork);
=20
- rv =3D -EINVAL;
- iface =3D intf->cur_altsetting;
- if (iface->desc.bNumEndpoints !=3D 1)
- goto err;
- ep =3D &iface->endpoint[0].desc;
- if (!ep || !usb_endpoint_is_int_in(ep))
- goto err;
=20
desc->wMaxPacketSize =3D usb_endpoint_maxp(ep);
desc->bMaxPacketSize0 =3D udev->descriptor.bMaxPacketSize0;
=20
=20
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" =
in
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Oliver Neukum
2012-01-25 07:25:54 UTC
Permalink
Post by Dan Williams
Post by Dan Williams
One oddity is that cdc-wdm seems to be looking at interfaces 3 and 4
too, but it shouldn't because the device list says to look only at
interface 5 (ff/f0/ff). Any idea why that's happening? 3 and 4 should
be driven by 'qcaux' not cdc-wdm.
[ 5524.707877] cdc_acm 2-1:1.0: ttyACM0: USB ACM device
[ 5524.708969] qcaux 2-1:1.2: qcaux converter detected
[ 5524.709205] usb 2-1: qcaux converter now attached to ttyUSB1
[ 5524.709346] cdc_wdm 2-1:1.3: failed to find interrupt endpoint
[ 5524.709356] cdc_wdm: probe of 2-1:1.3 failed with error -22
[ 5524.709484] cdc_wdm 2-1:1.4: failed to find interrupt endpoint
[ 5524.709493] cdc_wdm: probe of 2-1:1.4 failed with error -22
[ 5524.709793] cdc_wdm 2-1:1.5: cdc-wdm0: USB WDM device
I'm also able to talk to my Gobi 2000 card, but I need to restrict the
driver to interface 0 because there are two interfaces with interrupt
endpoints, and both are ff/ff/ff so USB_DEVICE_AND_INTERFACE_INFO
matching doesn't help us there. I suppose we could start putting the
interface number into the driver_data field and using that in
wdm_probe() if it's present.
No. We are not starting with a kludge. If the existing macros for binding
don't do the job, define a new macro in usbcore.

Regards
Oliver

PS: Please trim your posts a bit
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-01-25 09:36:10 UTC
Permalink
I'm also able to talk to my Gobi 2000 card, but I need to restrict t=
he
driver to interface 0 because there are two interfaces with interrup=
t
endpoints, and both are ff/ff/ff so USB_DEVICE_AND_INTERFACE_INFO
matching doesn't help us there. I suppose we could start putting th=
e
interface number into the driver_data field and using that in
wdm_probe() if it's present.
No. We are not starting with a kludge. If the existing macros for bin=
ding
don't do the job, define a new macro in usbcore.
I'm afraid that won't be possible without modifying the whole matching
logic in usbcore. USB_DEVICE_AND_INTERFACE_INFO is already matching on
all available fields:

struct usb_device_id {
/* which fields to match against? */
__u16 match_flags;

/* Used for product specific matches; range is inclusive */
__u16 idVendor;
__u16 idProduct;
__u16 bcdDevice_lo;
__u16 bcdDevice_hi;

/* Used for device class matches */
__u8 bDeviceClass;
__u8 bDeviceSubClass;
__u8 bDeviceProtocol;

/* Used for interface class matches */
__u8 bInterfaceClass;
__u8 bInterfaceSubClass;
__u8 bInterfaceProtocol;

/* not matched against */
kernel_ulong_t driver_info;
};


But I don't think it's a good idea to add this device to cdc-wdm
permanently anyway. It's useless without the network driver for
anything but playing with QMI commands. It's better to get Gobi
specific probing into qmi_wwan (or similar if making a Gobi-only
driver), and just use cdc-wdm as a subdriver from that.

And in fact I'm also starting to wonder if it was a good idea to add th=
e
Huawei modem. If the subdriver approach is accepted, then maybe it
would be better to use that for every such device including those with
separate control and data interfaces? It would make the network device
look consistent with the common cdc_ether type devices, and would leave
all the device specific probing to the already device specific network
driver.=20

What do you think?


Bj=C3=B8rn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Oliver Neukum
2012-01-25 14:07:07 UTC
Permalink
Post by Bjørn Mork
But I don't think it's a good idea to add this device to cdc-wdm
permanently anyway. It's useless without the network driver for
anything but playing with QMI commands. It's better to get Gobi
specific probing into qmi_wwan (or similar if making a Gobi-only
driver), and just use cdc-wdm as a subdriver from that.
Better or necessary?
Post by Bjørn Mork
And in fact I'm also starting to wonder if it was a good idea to add =
the
Post by Bjørn Mork
Huawei modem. If the subdriver approach is accepted, then maybe it
would be better to use that for every such device including those wit=
h

No. We leave policy to user space. If it is possible we let the user
do as he will. Whether it makes sense doesn't interest in the kernel
(first order approximation)

Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-01-25 15:11:18 UTC
Permalink
Post by Oliver Neukum
Post by Bjørn Mork
But I don't think it's a good idea to add this device to cdc-wdm
permanently anyway. It's useless without the network driver for
anything but playing with QMI commands. It's better to get Gobi
specific probing into qmi_wwan (or similar if making a Gobi-only
driver), and just use cdc-wdm as a subdriver from that.
Better or necessary?
Well, necessary if you are ever going to use it as a netdevice.

But you could duplicate the probing logic to let the user bind cdc-wdm
as a standalone driver to a Gobi device. So it's not strictly necessar=
y
*not* having it in cdc-wdm...=20
Post by Oliver Neukum
Post by Bjørn Mork
And in fact I'm also starting to wonder if it was a good idea to add=
the
Post by Oliver Neukum
Post by Bjørn Mork
Huawei modem. If the subdriver approach is accepted, then maybe it
would be better to use that for every such device including those wi=
th
Post by Oliver Neukum
No. We leave policy to user space. If it is possible we let the user
do as he will. Whether it makes sense doesn't interest in the kernel
(first order approximation)
OK. Just thought that the user expectations would be that of a common
CDC ECM device: A usbnet based driver binds to both control + data
interfaces, like cdc_ether does on this Ericsson F3507g modem:

T: Bus=3D02 Lev=3D01 Prnt=3D01 Port=3D03 Cnt=3D02 Dev#=3D 25 Spd=3D480=
MxCh=3D 0
D: Ver=3D 2.00 Cls=3D02(comm.) Sub=3D00 Prot=3D00 MxPS=3D64 #Cfgs=3D =
2
P: Vendor=3D0bdb ProdID=3D1900 Rev=3D 0.00
S: Manufacturer=3DEricsson
S: Product=3DEricsson F3507g Mobile Broadband Minicard Composite Devic=
e
C:* #Ifs=3D11 Cfg#=3D 1 Atr=3De0 MxPwr=3D 20mA
I:* If#=3D 0 Alt=3D 0 #EPs=3D 0 Cls=3D02(comm.) Sub=3D08 Prot=3D00 Driv=
er=3D(none)
I:* If#=3D 1 Alt=3D 0 #EPs=3D 1 Cls=3D02(comm.) Sub=3D02 Prot=3D01 Driv=
er=3Dcdc_acm
E: Ad=3D8a(I) Atr=3D03(Int.) MxPS=3D 16 Ivl=3D16ms
I:* If#=3D 2 Alt=3D 0 #EPs=3D 2 Cls=3D0a(data ) Sub=3D00 Prot=3D00 Driv=
er=3Dcdc_acm
E: Ad=3D01(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D81(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
I:* If#=3D 3 Alt=3D 0 #EPs=3D 1 Cls=3D02(comm.) Sub=3D02 Prot=3D01 Driv=
er=3Dcdc_acm
E: Ad=3D89(I) Atr=3D03(Int.) MxPS=3D 16 Ivl=3D16ms
I:* If#=3D 4 Alt=3D 0 #EPs=3D 2 Cls=3D0a(data ) Sub=3D00 Prot=3D00 Driv=
er=3Dcdc_acm
E: Ad=3D02(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D82(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
I:* If#=3D 5 Alt=3D 0 #EPs=3D 1 Cls=3D02(comm.) Sub=3D09 Prot=3D01 Driv=
er=3Dcdc_wdm
E: Ad=3D8b(I) Atr=3D03(Int.) MxPS=3D 16 Ivl=3D16ms
I:* If#=3D 6 Alt=3D 0 #EPs=3D 1 Cls=3D02(comm.) Sub=3D09 Prot=3D01 Driv=
er=3Dcdc_wdm
E: Ad=3D8c(I) Atr=3D03(Int.) MxPS=3D 16 Ivl=3D16ms
I:* If#=3D 7 Alt=3D 0 #EPs=3D 1 Cls=3D02(comm.) Sub=3D0a Prot=3D00 Driv=
er=3Dcdc_ether
E: Ad=3D87(I) Atr=3D03(Int.) MxPS=3D 8 Ivl=3D8ms
I: If#=3D 8 Alt=3D 0 #EPs=3D 0 Cls=3D0a(data ) Sub=3D00 Prot=3D00 Driv=
er=3Dcdc_ether
I:* If#=3D 8 Alt=3D 1 #EPs=3D 2 Cls=3D0a(data ) Sub=3D00 Prot=3D00 Driv=
er=3Dcdc_ether
E: Ad=3D04(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D84(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
I: If#=3D 8 Alt=3D 2 #EPs=3D 2 Cls=3D0a(data ) Sub=3D00 Prot=3Dee Driv=
er=3Dcdc_ether
E: Ad=3D04(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D84(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
I:* If#=3D 9 Alt=3D 0 #EPs=3D 1 Cls=3D02(comm.) Sub=3D02 Prot=3D01 Driv=
er=3Dcdc_acm
E: Ad=3D85(I) Atr=3D03(Int.) MxPS=3D 16 Ivl=3D16ms
I:* If#=3D10 Alt=3D 0 #EPs=3D 2 Cls=3D0a(data ) Sub=3D00 Prot=3D00 Driv=
er=3Dcdc_acm
E: Ad=3D06(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D86(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms


As it is now, if you use the Huawei modem in "Linux mode" you will get
cdc_wdm on the control interface and qmi_wwan on the data interface:

T: Bus=3D02 Lev=3D01 Prnt=3D01 Port=3D00 Cnt=3D01 Dev#=3D 34 Spd=3D480=
MxCh=3D 0
D: Ver=3D 2.00 Cls=3D00(>ifc ) Sub=3D00 Prot=3D00 MxPS=3D64 #Cfgs=3D =
1
P: Vendor=3D12d1 ProdID=3D1506 Rev=3D 0.00
S: Manufacturer=3DHuawei Technologies
S: Product=3DHUAWEI Mobile
C:* #Ifs=3D 7 Cfg#=3D 1 Atr=3Dc0 MxPwr=3D500mA
I:* If#=3D 0 Alt=3D 0 #EPs=3D 3 Cls=3Dff(vend.) Sub=3D01 Prot=3D01 Driv=
er=3Doption
E: Ad=3D81(I) Atr=3D03(Int.) MxPS=3D 64 Ivl=3D2ms
E: Ad=3D82(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D01(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D4ms
I:* If#=3D 1 Alt=3D 0 #EPs=3D 2 Cls=3Dff(vend.) Sub=3D01 Prot=3D02 Driv=
er=3Doption
E: Ad=3D83(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D02(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D4ms
I:* If#=3D 2 Alt=3D 0 #EPs=3D 2 Cls=3Dff(vend.) Sub=3D01 Prot=3D03 Driv=
er=3Doption
E: Ad=3D84(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D03(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D4ms
I:* If#=3D 3 Alt=3D 0 #EPs=3D 1 Cls=3Dff(vend.) Sub=3D01 Prot=3D09 Driv=
er=3Dcdc_wdm
E: Ad=3D85(I) Atr=3D03(Int.) MxPS=3D 64 Ivl=3D2ms
I:* If#=3D 4 Alt=3D 0 #EPs=3D 2 Cls=3Dff(vend.) Sub=3D01 Prot=3D08 Driv=
er=3Dqmi_wwan
E: Ad=3D86(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D04(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D4ms
I:* If#=3D 5 Alt=3D 0 #EPs=3D 2 Cls=3D08(stor.) Sub=3D06 Prot=3D50 Driv=
er=3Dusb-storage
E: Ad=3D87(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D05(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
I:* If#=3D 6 Alt=3D 0 #EPs=3D 2 Cls=3D08(stor.) Sub=3D06 Prot=3D50 Driv=
er=3Dusb-storage
E: Ad=3D06(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D88(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms


If you use the same modem in "Windows mode" you will see qmi_wwan
binding to the combined control+data interface, and no cdc_wdm at
all:

T: Bus=3D01 Lev=3D01 Prnt=3D01 Port=3D00 Cnt=3D01 Dev#=3D 5 Spd=3D480=
MxCh=3D 0
D: Ver=3D 2.00 Cls=3D00(>ifc ) Sub=3D00 Prot=3D00 MxPS=3D64 #Cfgs=3D =
1
P: Vendor=3D12d1 ProdID=3D1506 Rev=3D 0.00
S: Manufacturer=3DHuawei Technologies
S: Product=3DHUAWEI Mobile
C:* #Ifs=3D 6 Cfg#=3D 1 Atr=3Dc0 MxPwr=3D500mA
I:* If#=3D 0 Alt=3D 0 #EPs=3D 3 Cls=3Dff(vend.) Sub=3D01 Prot=3D10 Driv=
er=3Doption
E: Ad=3D81(I) Atr=3D03(Int.) MxPS=3D 64 Ivl=3D2ms
E: Ad=3D82(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D01(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D4ms
I:* If#=3D 1 Alt=3D 0 #EPs=3D 2 Cls=3Dff(vend.) Sub=3D01 Prot=3D12 Driv=
er=3Doption
E: Ad=3D83(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D02(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D4ms
I:* If#=3D 2 Alt=3D 0 #EPs=3D 2 Cls=3Dff(vend.) Sub=3D01 Prot=3D13 Driv=
er=3Doption
E: Ad=3D84(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D03(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D4ms
I:* If#=3D 3 Alt=3D 0 #EPs=3D 3 Cls=3Dff(vend.) Sub=3D01 Prot=3D11 Driv=
er=3Dqmi_wwan
E: Ad=3D85(I) Atr=3D03(Int.) MxPS=3D 64 Ivl=3D2ms
E: Ad=3D86(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D04(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D4ms
I:* If#=3D 4 Alt=3D 0 #EPs=3D 2 Cls=3D08(stor.) Sub=3D06 Prot=3D50 Driv=
er=3Dusb-storage
E: Ad=3D87(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D05(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
I:* If#=3D 5 Alt=3D 0 #EPs=3D 2 Cls=3D08(stor.) Sub=3D06 Prot=3D50 Driv=
er=3Dusb-storage
E: Ad=3D06(O) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms
E: Ad=3D88(I) Atr=3D02(Bulk) MxPS=3D 512 Ivl=3D0ms


But if you say so, then we keep things like they currently are.



Bj=C3=B8rn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Oliver Neukum
2012-01-25 15:59:40 UTC
Permalink
No. We leave policy to user space. If it is possible we let the use=
r
do as he will. Whether it makes sense doesn't interest in the kerne=
l
(first order approximation)
=20
OK. Just thought that the user expectations would be that of a commo=
n
CDC ECM device: A usbnet based driver binds to both control + data
Hm. I'd consider cdc-ncm the model you'd like to emulate.

Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-05-27 13:03:04 UTC
Permalink
Post by Oliver Neukum
Post by Bjørn Mork
But I don't think it's a good idea to add this device to cdc-wdm
permanently anyway. It's useless without the network driver for
anything but playing with QMI commands. It's better to get Gobi
specific probing into qmi_wwan (or similar if making a Gobi-only
driver), and just use cdc-wdm as a subdriver from that.
Better or necessary?
Post by Bjørn Mork
And in fact I'm also starting to wonder if it was a good idea to add=
the
Post by Oliver Neukum
Post by Bjørn Mork
Huawei modem. If the subdriver approach is accepted, then maybe it
would be better to use that for every such device including those wi=
th
Post by Oliver Neukum
No. We leave policy to user space. If it is possible we let the user
do as he will. Whether it makes sense doesn't interest in the kernel
(first order approximation)
Hello,

Reasons to revisit this discussion just came up.

Summarizing current status: QMI/wwan devices supported by the
combination of cdc-wdm and qmi_wwan may present the three QMI/wwan
endpoints as either
=20
a) "CDC Ethernet like":

control interface
interrupt in endpoint
data interface
bulk in endpoint
bulk out endpoint

b) or "Combined":

control/data interface
interrupt in endpoint
bulk in endpoint
bulk out endpoint

"b" gives us no options: A single interface - a single driver. qmi_wwa=
n
matches against the combined control/data interface and loads cdc-wdm a=
s
a subdriver.

When it comes to "a", cdc_wdm matches and binds to the control interfac=
e
as an ordinary interface driver and qmi_wwan matches and binds to the
data interface. This strategy does work with the currently supported
devices and has the advantage of a clear interface <-> driver
relationship.

It does have a few disadvantages though, and that's why I bring up this
discussion again:
- product IDs must be added to both cdc-wdm driver and qmi_wwan driver
- probing is different from cdc_ether, which is the model the firmware
is trying to emulate, confusing users
- qmi_wwan must peek at an interface it doesn't drive to pick up
e.g. the MAC address descriptor
- reliable matching against the data interface may be difficult because
the interface descriptors lack unique properties
- coordinated matching between control interface based drivers like
cdc_ether and data interface based drivers like qmi_wwan may be very
difficult (aka "impossible")

The two last points have just been brought to my attention by Andrew.
We currenly have a situation where all Huawei, class COMM, subclass
Ethernet, protocol 255 (sic) devices are bound to cdc-ether. We'd
really like these to be handled by cdc-wdm + qmi_wwan instead. But how=
?

Transferring the control interface to cdc-wdm is easy. But we need to
make qmi_wwan match the accompanying data interface and *no other* data
interface. How?

The data interfaces have class CDC_DATA, subclass 0, protocol 0 like an=
y
standard compliant CDC data interface. We cannot let qmi_wwan just bin=
d
to any such interface on a Huawei device. That would match a fixed-lin=
e
modem with standad CDC ACM, or an Ethernet USB dongle, or whatever.

To solve this problem, qmi_wwan can peek at the descriptors on the
control interface to verify that the data interface is in fact part of =
a
QMI/wwan interface set. But this is quickly becoming very ugly. What i=
f
we are moving only some of the mentioned devices (because only some of
them support QMI) from cdc_ether to qmi_wwan? You'd then need a contro=
l
interface based blacklist in cdc_ether matching a data interface based
rule in qmi_wwan, and having a similar control interface based match
list in cdc-wdm.

So you'd end up having a single QMI/wwan function requiring device ID
entries in 3(!) drivers, using complex probing logic to make it all
work.

IMHO the three endpoints providing a QMI/wwan function should alwasy be
regarded as a single functional unit, whether the firmware decides to
present it as separate control and data interfaces with a CDC Union
descriptor binding them together, or as a single USB interface. The
data interface is a slave interface which should not be driven
independently of the control interface. It's one function, one driver.

Changing qmi_wwan to bind always bind to the control interface will
simplify a number of things:
- no need at all to add QMI devices to cdc-wdm. cdc-wdm will be used
as a subdriver for all QMI devices regardless of descriptor layout
- similar probe logic in cdc_ether and qmi_wwan makes it easy to creat=
e
a matching entry and it's inverse, avoiding partially overlapping
matches
- wwanX interfaces would be bound to the control interface regardless
of whether the driver was qmi_wwan, cdc_ether or some other driver,
providing consistency to users and userspace

And these changes are IMHO necessary to reliably and consistenly bind
some devices, like the above mentioned Huawei devices.

So I have ended up with the conclusion that qmi_wwan should bind to the
control interface, and claiming any dedicated data interface just like
cdc_ether does. In fact, I believe we never would have considered
anything else if we had started with the single interface + subdriver
and later added two interface support.

I am going to code up the changes and send as RFC patches, removing the
explicit QMI device matches from cdc_wdm and use the subdriver support
instead.


Bj=C3=B8rn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-05-29 18:09:05 UTC
Permalink
qmi_wwan has been changed to drive both the control and data
interface for all QMI/wwan devices, using cdc-wdm as a subdriver.
Remove the stale device ID entry from cdc-wdm.

Cc: Oliver Neukum <oliver-GvhC2dPhHPQdnm+***@public.gmane.org>
Signed-off-by: Bj=C3=B8rn Mork <bjorn-***@public.gmane.org>
---
drivers/usb/class/cdc-wdm.c | 16 ----------------
1 file changed, 16 deletions(-)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index ea8b304..25e7d72 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -32,8 +32,6 @@
#define DRIVER_AUTHOR "Oliver Neukum"
#define DRIVER_DESC "USB Abstract Control Model driver for USB WCM Dev=
ice Management"
=20
-#define HUAWEI_VENDOR_ID 0x12D1
-
static const struct usb_device_id wdm_ids[] =3D {
{
.match_flags =3D USB_DEVICE_ID_MATCH_INT_CLASS |
@@ -41,20 +39,6 @@ static const struct usb_device_id wdm_ids[] =3D {
.bInterfaceClass =3D USB_CLASS_COMM,
.bInterfaceSubClass =3D USB_CDC_SUBCLASS_DMM
},
- {
- /*=20
- * Huawei E392, E398 and possibly other Qualcomm based modems
- * embed the Qualcomm QMI protocol inside CDC on CDC ECM like
- * control interfaces. Userspace access to this is required
- * to configure the accompanying data interface
- */
- .match_flags =3D USB_DEVICE_ID_MATCH_VENDOR |
- USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor =3D HUAWEI_VENDOR_ID,
- .bInterfaceClass =3D USB_CLASS_VENDOR_SPEC,
- .bInterfaceSubClass =3D 1,
- .bInterfaceProtocol =3D 9, /* NOTE: CDC ECM control interface! */
- },
{ }
};
=20
--=20
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-05-29 18:09:04 UTC
Permalink
So how about something like this? It really simplifies the probing
logic in all affected drivers, including indirectly affected drivers
like cdc-ether.

I am aware that the last patch is too large. I couldn't help refactori=
ng
the code somewhat to share subdriver registration code. Will most like=
ly
have to split that up before real submission.

But I'd appreciate comments on this at this point anyway. And do note
that this is not so much about the currently supported devices, as thos=
e
not supported mainly because we don't know how to match the data=20
interface



Bj=C3=B8rn Mork (3):
USB: cdc-wdm: QMI devices are now handled by qmi_wwan
net: qmi_wwan: Define a struct for driver specific state
net: qmi_wwan: Bind to both control and data interface

drivers/net/usb/qmi_wwan.c | 278 +++++++++++++++++++++++------------=
--------
drivers/usb/class/cdc-wdm.c | 16 ---
2 files changed, 151 insertions(+), 143 deletions(-)

--=20
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-05-29 18:09:07 UTC
Permalink
Always bind to control interface regardless of whether
it is a shared interface or not.

Most devices supported by this driver use a single
USB interface for both control and data. But some of
the supported devices split control and data into
two interfaces, bound together by a CDC Union descriptor
on the control interface. Before the cdc-wdm subdriver
support was added, this split was used to let cdc-wdm
drive the control interface and qmi_wwan drive the data
interface.

This does however require exact matching in the data
interface, and consistent device matching between
drivers matching on the control interface (cdc-wdm,
cdc-ether and others) and this driver. Some devices
use generic descriptors for the data interface, making
such matching unreliable at best.

Changing to control interface matching makes the driver
behave like users expect, and removes the need to add
some of the supported devices to both this driver and
cdc-wdm.

Signed-off-by: Bj=C3=B8rn Mork <bjorn-***@public.gmane.org>
---
drivers/net/usb/qmi_wwan.c | 242 ++++++++++++++++++++++++------------=
--------
1 file changed, 130 insertions(+), 112 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 9c6631a..7765ad4 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -15,11 +15,7 @@
#include <linux/usb/usbnet.h>
#include <linux/usb/cdc-wdm.h>
=20
-/* The name of the CDC Device Management driver */
-#define DM_DRIVER "cdc_wdm"
-
-/*
- * This driver supports wwan (3G/LTE/?) devices using a vendor
+/* This driver supports wwan (3G/LTE/?) devices using a vendor
* specific management protocol called Qualcomm MSM Interface (QMI) -
* in addition to the more common AT commands over serial interface
* management
@@ -31,40 +27,95 @@
* management protocol is used in place of the standard CDC
* notifications NOTIFY_NETWORK_CONNECTION and NOTIFY_SPEED_CHANGE
*
+ * Alternatively, control and data functions can be combined in a
+ * single USB interface.
+ *
* Handling a protocol like QMI is out of the scope for any driver.
- * It can be exported as a character device using the cdc-wdm driver,
- * which will enable userspace applications ("modem managers") to
- * handle it. This may be required to use the network interface
- * provided by the driver.
+ * It is exported as a character device using the cdc-wdm driver as
+ * a subdriver, enabling userspace applications ("modem managers") to
+ * handle it.
*
* These devices may alternatively/additionally be configured using AT
- * commands on any of the serial interfaces driven by the option drive=
r
- *
- * This driver binds only to the data ("slave") interface to enable
- * the cdc-wdm driver to bind to the control interface. It still
- * parses the CDC functional descriptors on the control interface to
- * a) verify that this is indeed a handled interface (CDC Union
- * header lists it as slave)
- * b) get MAC address and other ethernet config from the CDC Ethernet
- * header
- * c) enable user bind requests against the control interface, which
- * is the common way to bind to CDC Ethernet Control Model type
- * interfaces
- * d) provide a hint to the user about which interface is the
- * corresponding management interface
+ * commands on a serial interface
*/
=20
/* driver specific data */
struct qmi_wwan_state {
struct usb_driver *subdriver;
atomic_t pmcount;
- unsigned long unused[3];
+ unsigned long unused;
+ struct usb_interface *control;
+ struct usb_interface *data;
};
=20
+/* using a counter to merge subdriver requests with our own into a com=
bined state */
+static int qmi_wwan_manage_power(struct usbnet *dev, int on)
+{
+ struct qmi_wwan_state *info =3D (void *)&dev->data;
+ int rv =3D 0;
+
+ dev_dbg(&dev->intf->dev, "%s() pmcount=3D%d, on=3D%d\n", __func__, at=
omic_read(&info->pmcount), on);
+
+ if ((on && atomic_add_return(1, &info->pmcount) =3D=3D 1) || (!on && =
atomic_dec_and_test(&info->pmcount))) {
+ /* need autopm_get/put here to ensure the usbcore sees the new value=
*/
+ rv =3D usb_autopm_get_interface(dev->intf);
+ if (rv < 0)
+ goto err;
+ dev->intf->needs_remote_wakeup =3D on;
+ usb_autopm_put_interface(dev->intf);
+ }
+err:
+ return rv;
+}
+
+static int qmi_wwan_cdc_wdm_manage_power(struct usb_interface *intf, i=
nt on)
+{
+ struct usbnet *dev =3D usb_get_intfdata(intf);
+ return qmi_wwan_manage_power(dev, on);
+}
+
+/* collect all three endpoints and register subdriver */
+static int qmi_wwan_register_subdriver(struct usbnet *dev)
+{
+ int rv;
+ struct qmi_wwan_state *info =3D (void *)&dev->data;
+
+ /* collect bulk endpoints */
+ rv =3D usbnet_get_endpoints(dev, info->data);
+ if (rv < 0)
+ goto err;
+
+ /* update status endpoint if separate control interface */
+ if (info->control !=3D info->data)
+ dev->status =3D &info->control->cur_altsetting->endpoint[0];
+
+ /* require interrupt endpoint for subdriver */
+ if (!dev->status) {
+ rv =3D -EINVAL;
+ goto err;
+ }
+
+ /* for subdriver power management */
+ atomic_set(&info->pmcount, 0);
+
+ /* register subdriver */
+ info->subdriver =3D usb_cdc_wdm_register(info->control, &dev->status-=
desc, 512, &qmi_wwan_cdc_wdm_manage_power);
+ if (IS_ERR(info->subdriver)) {
+ dev_err(&info->control->dev, "subdriver registration failed\n");
+ rv =3D PTR_ERR(info->subdriver);
+ info->subdriver =3D NULL;
+ goto err;
+ }
+
+ /* prevent usbnet from using status endpoint */
+ dev->status =3D NULL;
+err:
+ return rv;
+}
+
static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *int=
f)
{
int status =3D -1;
- struct usb_interface *control =3D NULL;
u8 *buf =3D intf->cur_altsetting->extra;
int len =3D intf->cur_altsetting->extralen;
struct usb_interface_descriptor *desc =3D &intf->cur_altsetting->desc=
;
@@ -72,27 +123,14 @@ static int qmi_wwan_bind(struct usbnet *dev, struc=
t usb_interface *intf)
struct usb_cdc_ether_desc *cdc_ether =3D NULL;
u32 required =3D 1 << USB_CDC_HEADER_TYPE | 1 << USB_CDC_UNION_TYPE;
u32 found =3D 0;
+ struct usb_driver *driver =3D driver_of(intf);
struct qmi_wwan_state *info =3D (void *)&dev->data;
=20
BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_=
wwan_state)));
=20
- atomic_set(&info->pmcount, 0);
-
- /*
- * assume a data interface has no additional descriptors and
- * that the control and data interface are numbered
- * consecutively - this holds for the Huawei device at least
- */
- if (len =3D=3D 0 && desc->bInterfaceNumber > 0) {
- control =3D usb_ifnum_to_if(dev->udev, desc->bInterfaceNumber - 1);
- if (!control)
- goto err;
-
- buf =3D control->cur_altsetting->extra;
- len =3D control->cur_altsetting->extralen;
- dev_dbg(&intf->dev, "guessing \"control\" =3D> %s, \"data\" =3D> thi=
s\n",
- dev_name(&control->dev));
- }
+ /* require a single interrupt status endpoint for subdriver */
+ if (intf->cur_altsetting->desc.bNumEndpoints !=3D 1)
+ goto err;
=20
while (len > 3) {
struct usb_descriptor_header *h =3D (void *)buf;
@@ -156,10 +194,17 @@ next_desc:
goto err;
}
=20
- /* give the user a helpful hint if trying to bind to the wrong interf=
ace */
- if (cdc_union && desc->bInterfaceNumber =3D=3D cdc_union->bMasterInte=
rface0) {
- dev_err(&intf->dev, "leaving \"control\" interface for " DM_DRIVER "=
- try binding to %s instead!\n",
- dev_name(&usb_ifnum_to_if(dev->udev, cdc_union->bSlaveInterface0)->=
dev));
+ /* verify CDC Union */
+ if (desc->bInterfaceNumber !=3D cdc_union->bMasterInterface0) {
+ dev_err(&intf->dev, "bogus CDC Union: master=3D%u\n", cdc_union->bMa=
sterInterface0);
+ goto err;
+ }
+
+ /* need to save these for unbind */
+ info->control =3D intf;
+ info->data =3D usb_ifnum_to_if(dev->udev, cdc_union->bSlaveInterface0=
);
+ if (!info->data) {
+ dev_err(&intf->dev, "bogus CDC Union: slave=3D%u\n", cdc_union->bSla=
veInterface0);
goto err;
}
=20
@@ -169,46 +214,21 @@ next_desc:
usbnet_get_ethernet_addr(dev, cdc_ether->iMACAddress);
}
=20
- /* success! point the user to the management interface */
- if (control)
- dev_info(&intf->dev, "Use \"" DM_DRIVER "\" for QMI interface %s\n",
- dev_name(&control->dev));
-
- /* XXX: add a sysfs symlink somewhere to help management applications=
find it? */
+ /* claim data interface and set it up */
+ status =3D usb_driver_claim_interface(driver, info->data, dev);
+ if (status < 0)
+ goto err;
=20
- /* collect bulk endpoints now that we know intf =3D=3D "data" interfa=
ce */
- status =3D usbnet_get_endpoints(dev, intf);
+ status =3D qmi_wwan_register_subdriver(dev);
+ if (status < 0) {
+ usb_set_intfdata(info->data, NULL);
+ usb_driver_release_interface(driver, info->data);
+ }
=20
err:
return status;
}
=20
-/* using a counter to merge subdriver requests with our own into a com=
bined state */
-static int qmi_wwan_manage_power(struct usbnet *dev, int on)
-{
- struct qmi_wwan_state *info =3D (void *)&dev->data;
- int rv =3D 0;
-
- dev_dbg(&dev->intf->dev, "%s() pmcount=3D%d, on=3D%d\n", __func__, at=
omic_read(&info->pmcount), on);
-
- if ((on && atomic_add_return(1, &info->pmcount) =3D=3D 1) || (!on && =
atomic_dec_and_test(&info->pmcount))) {
- /* need autopm_get/put here to ensure the usbcore sees the new value=
*/
- rv =3D usb_autopm_get_interface(dev->intf);
- if (rv < 0)
- goto err;
- dev->intf->needs_remote_wakeup =3D on;
- usb_autopm_put_interface(dev->intf);
- }
-err:
- return rv;
-}
-
-static int qmi_wwan_cdc_wdm_manage_power(struct usb_interface *intf, i=
nt on)
-{
- struct usbnet *dev =3D usb_get_intfdata(intf);
- return qmi_wwan_manage_power(dev, on);
-}
-
/* Some devices combine the "control" and "data" functions into a
* single interface with all three endpoints: interrupt + bulk in and
* out
@@ -236,29 +256,10 @@ static int qmi_wwan_bind_shared(struct usbnet *de=
v, struct usb_interface *intf)
goto err;
}
=20
- atomic_set(&info->pmcount, 0);
-
- /* collect all three endpoints */
- rv =3D usbnet_get_endpoints(dev, intf);
- if (rv < 0)
- goto err;
-
- /* require interrupt endpoint for subdriver */
- if (!dev->status) {
- rv =3D -EINVAL;
- goto err;
- }
-
- info->subdriver =3D usb_cdc_wdm_register(intf, &dev->status->desc, 51=
2, &qmi_wwan_cdc_wdm_manage_power);
- if (IS_ERR(info->subdriver)) {
- rv =3D PTR_ERR(info->subdriver);
- info->subdriver =3D NULL;
- goto err;
- }
-
- /* can't let usbnet use the interrupt endpoint */
- dev->status =3D NULL;
-
+ /* control and data is shared */
+ info->control =3D intf;
+ info->data =3D intf;
+ rv =3D qmi_wwan_register_subdriver(dev);
err:
return rv;
}
@@ -286,14 +287,30 @@ err:
return rv;
}
=20
-static void qmi_wwan_unbind_shared(struct usbnet *dev, struct usb_inte=
rface *intf)
+static void qmi_wwan_unbind(struct usbnet *dev, struct usb_interface *=
intf)
{
struct qmi_wwan_state *info =3D (void *)&dev->data;
+ struct usb_driver *driver =3D driver_of(intf);
+ struct usb_interface *other;
=20
if (info->subdriver && info->subdriver->disconnect)
- info->subdriver->disconnect(intf);
+ info->subdriver->disconnect(info->control);
+
+ /* allow user to unbind using either control or data */
+ if (intf =3D=3D info->control)
+ other =3D info->data;
+ else
+ other =3D info->control;
+
+ /* only if not shared */
+ if (other && intf !=3D other) {
+ usb_set_intfdata(other, NULL);
+ usb_driver_release_interface(driver, other);
+ }
=20
info->subdriver =3D NULL;
+ info->data =3D NULL;
+ info->control =3D NULL;
}
=20
/* suspend/resume wrappers calling both usbnet and the cdc-wdm
@@ -342,6 +359,7 @@ static const struct driver_info qmi_wwan_info =3D {
.description =3D "QMI speaking wwan device",
.flags =3D FLAG_WWAN,
.bind =3D qmi_wwan_bind,
+ .unbind =3D qmi_wwan_unbind,
.manage_power =3D qmi_wwan_manage_power,
};
=20
@@ -349,7 +367,7 @@ static const struct driver_info qmi_wwan_shared =3D=
{
.description =3D "QMI speaking wwan device with combined interface",
.flags =3D FLAG_WWAN,
.bind =3D qmi_wwan_bind_shared,
- .unbind =3D qmi_wwan_unbind_shared,
+ .unbind =3D qmi_wwan_unbind,
.manage_power =3D qmi_wwan_manage_power,
};
=20
@@ -357,7 +375,7 @@ static const struct driver_info qmi_wwan_gobi =3D {
.description =3D "Qualcomm Gobi wwan/QMI device",
.flags =3D FLAG_WWAN,
.bind =3D qmi_wwan_bind_gobi,
- .unbind =3D qmi_wwan_unbind_shared,
+ .unbind =3D qmi_wwan_unbind,
.manage_power =3D qmi_wwan_manage_power,
};
=20
@@ -366,7 +384,7 @@ static const struct driver_info qmi_wwan_force_int1=
=3D {
.description =3D "Qualcomm WWAN/QMI device",
.flags =3D FLAG_WWAN,
.bind =3D qmi_wwan_bind_shared,
- .unbind =3D qmi_wwan_unbind_shared,
+ .unbind =3D qmi_wwan_unbind,
.manage_power =3D qmi_wwan_manage_power,
.data =3D BIT(1), /* interface whitelist bitmap */
};
@@ -375,7 +393,7 @@ static const struct driver_info qmi_wwan_force_int4=
=3D {
.description =3D "Qualcomm WWAN/QMI device",
.flags =3D FLAG_WWAN,
.bind =3D qmi_wwan_bind_shared,
- .unbind =3D qmi_wwan_unbind_shared,
+ .unbind =3D qmi_wwan_unbind,
.manage_power =3D qmi_wwan_manage_power,
.data =3D BIT(4), /* interface whitelist bitmap */
};
@@ -397,7 +415,7 @@ static const struct driver_info qmi_wwan_sierra =3D=
{
.description =3D "Sierra Wireless wwan/QMI device",
.flags =3D FLAG_WWAN,
.bind =3D qmi_wwan_bind_gobi,
- .unbind =3D qmi_wwan_unbind_shared,
+ .unbind =3D qmi_wwan_unbind,
.manage_power =3D qmi_wwan_manage_power,
.data =3D BIT(8) | BIT(19), /* interface whitelist bitmap */
};
@@ -413,7 +431,7 @@ static const struct usb_device_id products[] =3D {
.idVendor =3D HUAWEI_VENDOR_ID,
.bInterfaceClass =3D USB_CLASS_VENDOR_SPEC,
.bInterfaceSubClass =3D 1,
- .bInterfaceProtocol =3D 8, /* NOTE: This is the *slave* interface of=
the CDC Union! */
+ .bInterfaceProtocol =3D 9, /* CDC Ethernet *control* interface */
.driver_info =3D (unsigned long)&qmi_wwan_info,
},
{ /* Vodafone/Huawei K5005 (12d1:14c8) and similar modems */
@@ -421,7 +439,7 @@ static const struct usb_device_id products[] =3D {
.idVendor =3D HUAWEI_VENDOR_ID,
.bInterfaceClass =3D USB_CLASS_VENDOR_SPEC,
.bInterfaceSubClass =3D 1,
- .bInterfaceProtocol =3D 56, /* NOTE: This is the *slave* interface o=
f the CDC Union! */
+ .bInterfaceProtocol =3D 57, /* XXX: verify this! */
.driver_info =3D (unsigned long)&qmi_wwan_info,
},
{ /* Huawei E392, E398 and possibly others in "Windows mode"
--=20
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-05-29 18:09:06 UTC
Permalink
Naming the fields and taking advantage of type checking
is a bit more failsafe as more and more driver state is
put into the usbnet "data" array.

Signed-off-by: Bj=C3=B8rn Mork <bjorn-***@public.gmane.org>
---
drivers/net/usb/qmi_wwan.c | 58 ++++++++++++++++++++++++------------=
--------
1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 380dbea..9c6631a 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -54,6 +54,13 @@
* corresponding management interface
*/
=20
+/* driver specific data */
+struct qmi_wwan_state {
+ struct usb_driver *subdriver;
+ atomic_t pmcount;
+ unsigned long unused[3];
+};
+
static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *int=
f)
{
int status =3D -1;
@@ -65,9 +72,11 @@ static int qmi_wwan_bind(struct usbnet *dev, struct =
usb_interface *intf)
struct usb_cdc_ether_desc *cdc_ether =3D NULL;
u32 required =3D 1 << USB_CDC_HEADER_TYPE | 1 << USB_CDC_UNION_TYPE;
u32 found =3D 0;
- atomic_t *pmcount =3D (void *)&dev->data[1];
+ struct qmi_wwan_state *info =3D (void *)&dev->data;
=20
- atomic_set(pmcount, 0);
+ BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_=
wwan_state)));
+
+ atomic_set(&info->pmcount, 0);
=20
/*
* assume a data interface has no additional descriptors and
@@ -177,12 +186,12 @@ err:
/* using a counter to merge subdriver requests with our own into a com=
bined state */
static int qmi_wwan_manage_power(struct usbnet *dev, int on)
{
- atomic_t *pmcount =3D (void *)&dev->data[1];
+ struct qmi_wwan_state *info =3D (void *)&dev->data;
int rv =3D 0;
=20
- dev_dbg(&dev->intf->dev, "%s() pmcount=3D%d, on=3D%d\n", __func__, at=
omic_read(pmcount), on);
+ dev_dbg(&dev->intf->dev, "%s() pmcount=3D%d, on=3D%d\n", __func__, at=
omic_read(&info->pmcount), on);
=20
- if ((on && atomic_add_return(1, pmcount) =3D=3D 1) || (!on && atomic_=
dec_and_test(pmcount))) {
+ if ((on && atomic_add_return(1, &info->pmcount) =3D=3D 1) || (!on && =
atomic_dec_and_test(&info->pmcount))) {
/* need autopm_get/put here to ensure the usbcore sees the new value=
*/
rv =3D usb_autopm_get_interface(dev->intf);
if (rv < 0)
@@ -211,8 +220,7 @@ static int qmi_wwan_cdc_wdm_manage_power(struct usb=
_interface *intf, int on)
static int qmi_wwan_bind_shared(struct usbnet *dev, struct usb_interfa=
ce *intf)
{
int rv;
- struct usb_driver *subdriver =3D NULL;
- atomic_t *pmcount =3D (void *)&dev->data[1];
+ struct qmi_wwan_state *info =3D (void *)&dev->data;
=20
/* ZTE makes devices where the interface descriptors and endpoint
* configurations of two or more interfaces are identical, even
@@ -228,7 +236,7 @@ static int qmi_wwan_bind_shared(struct usbnet *dev,=
struct usb_interface *intf)
goto err;
}
=20
- atomic_set(pmcount, 0);
+ atomic_set(&info->pmcount, 0);
=20
/* collect all three endpoints */
rv =3D usbnet_get_endpoints(dev, intf);
@@ -241,18 +249,16 @@ static int qmi_wwan_bind_shared(struct usbnet *de=
v, struct usb_interface *intf)
goto err;
}
=20
- subdriver =3D usb_cdc_wdm_register(intf, &dev->status->desc, 512, &qm=
i_wwan_cdc_wdm_manage_power);
- if (IS_ERR(subdriver)) {
- rv =3D PTR_ERR(subdriver);
+ info->subdriver =3D usb_cdc_wdm_register(intf, &dev->status->desc, 51=
2, &qmi_wwan_cdc_wdm_manage_power);
+ if (IS_ERR(info->subdriver)) {
+ rv =3D PTR_ERR(info->subdriver);
+ info->subdriver =3D NULL;
goto err;
}
=20
/* can't let usbnet use the interrupt endpoint */
dev->status =3D NULL;
=20
- /* save subdriver struct for suspend/resume wrappers */
- dev->data[0] =3D (unsigned long)subdriver;
-
err:
return rv;
}
@@ -282,12 +288,12 @@ err:
=20
static void qmi_wwan_unbind_shared(struct usbnet *dev, struct usb_inte=
rface *intf)
{
- struct usb_driver *subdriver =3D (void *)dev->data[0];
+ struct qmi_wwan_state *info =3D (void *)&dev->data;
=20
- if (subdriver && subdriver->disconnect)
- subdriver->disconnect(intf);
+ if (info->subdriver && info->subdriver->disconnect)
+ info->subdriver->disconnect(intf);
=20
- dev->data[0] =3D (unsigned long)NULL;
+ info->subdriver =3D NULL;
}
=20
/* suspend/resume wrappers calling both usbnet and the cdc-wdm
@@ -299,15 +305,15 @@ static void qmi_wwan_unbind_shared(struct usbnet =
*dev, struct usb_interface *int
static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t m=
essage)
{
struct usbnet *dev =3D usb_get_intfdata(intf);
- struct usb_driver *subdriver =3D (void *)dev->data[0];
+ struct qmi_wwan_state *info =3D (void *)&dev->data;
int ret;
=20
ret =3D usbnet_suspend(intf, message);
if (ret < 0)
goto err;
=20
- if (subdriver && subdriver->suspend)
- ret =3D subdriver->suspend(intf, message);
+ if (info->subdriver && info->subdriver->suspend)
+ ret =3D info->subdriver->suspend(intf, message);
if (ret < 0)
usbnet_resume(intf);
err:
@@ -317,16 +323,16 @@ err:
static int qmi_wwan_resume(struct usb_interface *intf)
{
struct usbnet *dev =3D usb_get_intfdata(intf);
- struct usb_driver *subdriver =3D (void *)dev->data[0];
+ struct qmi_wwan_state *info =3D (void *)&dev->data;
int ret =3D 0;
=20
- if (subdriver && subdriver->resume)
- ret =3D subdriver->resume(intf);
+ if (info->subdriver && info->subdriver->resume)
+ ret =3D info->subdriver->resume(intf);
if (ret < 0)
goto err;
ret =3D usbnet_resume(intf);
- if (ret < 0 && subdriver && subdriver->resume && subdriver->suspend)
- subdriver->suspend(intf, PMSG_SUSPEND);
+ if (ret < 0 && info->subdriver && info->subdriver->resume && info->su=
bdriver->suspend)
+ info->subdriver->suspend(intf, PMSG_SUSPEND);
err:
return ret;
}
--=20
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-06-18 08:24:34 UTC
Permalink
Post by Bjørn Mork
So how about something like this? It really simplifies the probing
logic in all affected drivers, including indirectly affected drivers
like cdc-ether.
I am aware that the last patch is too large. I couldn't help refacto=
ring
Post by Bjørn Mork
the code somewhat to share subdriver registration code. Will most li=
kely
Post by Bjørn Mork
have to split that up before real submission.
But I'd appreciate comments on this at this point anyway. And do not=
e
Post by Bjørn Mork
that this is not so much about the currently supported devices, as th=
ose
Post by Bjørn Mork
not supported mainly because we don't know how to match the data=20
interface
Not many comments here... I take that as "go ahead and submit patches"=
=2E

Another argument for this solution just came up: Splitting the control
and data interface between two drivers create the illusion that you may
apply new device ID patches independently. In particular that you can
apply the patches for cdc-wdm to stable/longterm kernels not having
qmi_wwan.


Bj=C3=B8rn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-01-25 14:45:24 UTC
Permalink
Post by Bjørn Mork
Also you may not have seen, but the QUIC codeaurora people pushed a =
new
Post by Bjørn Mork
https://www.codeaurora.org/gitweb/quic/le/?p=3Dkernel/msm.git;a=3Dco=
mmit;h=3D37c35e4ee5226099374a1a1eda637d0f26fc023d
Post by Bjørn Mork
Thanks for the pointer. Will take a look at it. But I guess these a=
re
Post by Bjørn Mork
still doing the integrated QMI approach. Which most likely isn't
acceptable to anyone here, after you managed to convince me :-)
Looked at it, and I believe I've seen most of it before. It does
support the IP mode, which is nice. But that depends on the core
ARPHRD_RAWIP changes. And the driver uses netdev ioctls to switch
modes. BTW, it also supports a "QMI QOS header" mode, which obviously
adds some QoS header to the outgoing packets.=20

The driver exports the full QMI interface to userspace using a new
character device in its own class, and it also exports one driver
control command using sysfs and some stats using debugfs. I like
that. It does however, statically create 4 such devices on driver load,
which I don't think is a design which will fly nowadays..

Reusing cdc-wdm is far more elegant IMHO, but I'm bound to be biased...
Post by Bjørn Mork
And having all these devices expose their QMI interface as a
/dev/cdc-wdmX should make things look more consistent for users and
userspace.
I was given another Huawei modem with newer firmware today, which leave=
s
me with two of them + the internal Ericsson modem in the laptop. So I
no have 4 cdc-wdm devices to play with, two AT and two QMI:

Jan 25 15:32:06 nemi kernel: [60970.442946] cdc_wdm 2-4:1.5: Finding ma=
ximum buffer length: 2048
Jan 25 15:32:06 nemi kernel: [60970.443623] cdc_wdm 2-4:1.5: cdc-wdm0: =
USB WDM device
Jan 25 15:32:06 nemi kernel: [60970.443638] cdc_wdm 2-4:1.6: Finding ma=
ximum buffer length: 2048
Jan 25 15:32:06 nemi kernel: [60970.443889] cdc_wdm 2-4:1.6: cdc-wdm1: =
USB WDM device
Jan 25 15:32:06 nemi kernel: [60970.443932] usbcore: registered new int=
erface driver cdc_wdm
Jan 25 15:32:17 nemi kernel: [60981.492525] qmi_wwan 2-1:1.3: cdc-wdm2:=
USB WDM device
Jan 25 15:32:17 nemi kernel: [60981.498160] qmi_wwan 2-1:1.3: wwan1: re=
gister 'qmi_wwan' at usb-0000:00:1d.7-1, QMI speaking wwan device with =
combined interface, 0e:b8:5e:cd:9c:78
Jan 25 15:32:17 nemi kernel: [60981.498943] qmi_wwan 1-1:1.3: cdc-wdm3:=
USB WDM device
Jan 25 15:32:17 nemi kernel: [60981.502264] qmi_wwan 1-1:1.3: wwan2: re=
gister 'qmi_wwan' at usb-0000:00:1a.7-1, QMI speaking wwan device with =
combined interface, 0e:b8:5e:cd:9c:78
Jan 25 15:32:17 nemi kernel: [60981.502601] usbcore: registered new int=
erface driver qmi_wwan


***@nemi:~$ ls -l /dev/cdc-wdm*
crw-rw---T 1 root dialout 180, 0 Jan 25 15:32 /dev/cdc-wdm0
crw-rw---T 1 root dialout 180, 1 Jan 25 15:32 /dev/cdc-wdm1
crw-rw---T 1 root dialout 180, 2 Jan 25 15:32 /dev/cdc-wdm2
crw-rw---T 1 root dialout 180, 3 Jan 25 15:32 /dev/cdc-wdm3


Detecting netdev =3D> wdm mapping based on USB device connection works
well enough as long as there is only one netdev per USB device. And
detecting AT command interface is as easy as just trying AT or ATI and
see if you get an OK back. The Huawei modem does not seem to have any
problems with this kind of noise, so protocol probing in the order
AT,QMI is possible.

We could also put known protocols per device into the usbnet driver and
export that to userspace using sysfs, but I don't know if it has enough
value?


Bj=C3=B8rn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Marcel Holtmann
2012-01-25 16:10:41 UTC
Permalink
Hi Bjorn,
Post by Bjørn Mork
Post by Bjørn Mork
Post by Dan Williams
Also you may not have seen, but the QUIC codeaurora people pushed a new
https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=37c35e4ee5226099374a1a1eda637d0f26fc023d
Thanks for the pointer. Will take a look at it. But I guess these are
still doing the integrated QMI approach. Which most likely isn't
acceptable to anyone here, after you managed to convince me :-)
Looked at it, and I believe I've seen most of it before. It does
support the IP mode, which is nice. But that depends on the core
ARPHRD_RAWIP changes. And the driver uses netdev ioctls to switch
modes. BTW, it also supports a "QMI QOS header" mode, which obviously
adds some QoS header to the outgoing packets.
The driver exports the full QMI interface to userspace using a new
character device in its own class, and it also exports one driver
control command using sysfs and some stats using debugfs. I like
that. It does however, statically create 4 such devices on driver load,
which I don't think is a design which will fly nowadays..
Reusing cdc-wdm is far more elegant IMHO, but I'm bound to be biased...
I prefer a simpler approach where the kernel is not involved in QMI as
well. If we involve the kernel, then it needs to become a socket based
design and not character interfaces. I think that I mentioned this
before, CAIF and Phonet are also socket based and that for a reason.
Post by Bjørn Mork
Post by Bjørn Mork
And having all these devices expose their QMI interface as a
/dev/cdc-wdmX should make things look more consistent for users and
userspace.
I was given another Huawei modem with newer firmware today, which leaves
me with two of them + the internal Ericsson modem in the laptop. So I
Jan 25 15:32:06 nemi kernel: [60970.442946] cdc_wdm 2-4:1.5: Finding maximum buffer length: 2048
Jan 25 15:32:06 nemi kernel: [60970.443623] cdc_wdm 2-4:1.5: cdc-wdm0: USB WDM device
Jan 25 15:32:06 nemi kernel: [60970.443638] cdc_wdm 2-4:1.6: Finding maximum buffer length: 2048
Jan 25 15:32:06 nemi kernel: [60970.443889] cdc_wdm 2-4:1.6: cdc-wdm1: USB WDM device
Jan 25 15:32:06 nemi kernel: [60970.443932] usbcore: registered new interface driver cdc_wdm
Jan 25 15:32:17 nemi kernel: [60981.492525] qmi_wwan 2-1:1.3: cdc-wdm2: USB WDM device
Jan 25 15:32:17 nemi kernel: [60981.498160] qmi_wwan 2-1:1.3: wwan1: register 'qmi_wwan' at usb-0000:00:1d.7-1, QMI speaking wwan device with combined interface, 0e:b8:5e:cd:9c:78
Jan 25 15:32:17 nemi kernel: [60981.498943] qmi_wwan 1-1:1.3: cdc-wdm3: USB WDM device
Jan 25 15:32:17 nemi kernel: [60981.502264] qmi_wwan 1-1:1.3: wwan2: register 'qmi_wwan' at usb-0000:00:1a.7-1, QMI speaking wwan device with combined interface, 0e:b8:5e:cd:9c:78
Jan 25 15:32:17 nemi kernel: [60981.502601] usbcore: registered new interface driver qmi_wwan
crw-rw---T 1 root dialout 180, 0 Jan 25 15:32 /dev/cdc-wdm0
crw-rw---T 1 root dialout 180, 1 Jan 25 15:32 /dev/cdc-wdm1
crw-rw---T 1 root dialout 180, 2 Jan 25 15:32 /dev/cdc-wdm2
crw-rw---T 1 root dialout 180, 3 Jan 25 15:32 /dev/cdc-wdm3
Detecting netdev => wdm mapping based on USB device connection works
well enough as long as there is only one netdev per USB device. And
detecting AT command interface is as easy as just trying AT or ATI and
see if you get an OK back. The Huawei modem does not seem to have any
problems with this kind of noise, so protocol probing in the order
AT,QMI is possible.
I personally never liked the port probing crazy, but that is up to
ModemManager. Inside oFono we do not need this. And we can nicely figure
out which device nodes belongs to which device. Especially since we are
also understanding the USB interface order as well.
Post by Bjørn Mork
We could also put known protocols per device into the usbnet driver and
export that to userspace using sysfs, but I don't know if it has enough
value?
If you know everything up front, this would make it easier. With cdc-wdm
you might have a chance to do this, with the "option" driver it is
almost hopeless ;)

Regards

Marcel


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dan Williams
2012-01-25 16:27:10 UTC
Permalink
Post by Marcel Holtmann
Hi Bjorn,
Post by Bjørn Mork
Post by Bjørn Mork
Post by Dan Williams
Also you may not have seen, but the QUIC codeaurora people pushed a new
https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=37c35e4ee5226099374a1a1eda637d0f26fc023d
Thanks for the pointer. Will take a look at it. But I guess these are
still doing the integrated QMI approach. Which most likely isn't
acceptable to anyone here, after you managed to convince me :-)
Looked at it, and I believe I've seen most of it before. It does
support the IP mode, which is nice. But that depends on the core
ARPHRD_RAWIP changes. And the driver uses netdev ioctls to switch
modes. BTW, it also supports a "QMI QOS header" mode, which obviously
adds some QoS header to the outgoing packets.
The driver exports the full QMI interface to userspace using a new
character device in its own class, and it also exports one driver
control command using sysfs and some stats using debugfs. I like
that. It does however, statically create 4 such devices on driver load,
which I don't think is a design which will fly nowadays..
Reusing cdc-wdm is far more elegant IMHO, but I'm bound to be biased...
I prefer a simpler approach where the kernel is not involved in QMI as
well. If we involve the kernel, then it needs to become a socket based
design and not character interfaces. I think that I mentioned this
before, CAIF and Phonet are also socket based and that for a reason.
Post by Bjørn Mork
Post by Bjørn Mork
And having all these devices expose their QMI interface as a
/dev/cdc-wdmX should make things look more consistent for users and
userspace.
I was given another Huawei modem with newer firmware today, which leaves
me with two of them + the internal Ericsson modem in the laptop. So I
Jan 25 15:32:06 nemi kernel: [60970.442946] cdc_wdm 2-4:1.5: Finding maximum buffer length: 2048
Jan 25 15:32:06 nemi kernel: [60970.443623] cdc_wdm 2-4:1.5: cdc-wdm0: USB WDM device
Jan 25 15:32:06 nemi kernel: [60970.443638] cdc_wdm 2-4:1.6: Finding maximum buffer length: 2048
Jan 25 15:32:06 nemi kernel: [60970.443889] cdc_wdm 2-4:1.6: cdc-wdm1: USB WDM device
Jan 25 15:32:06 nemi kernel: [60970.443932] usbcore: registered new interface driver cdc_wdm
Jan 25 15:32:17 nemi kernel: [60981.492525] qmi_wwan 2-1:1.3: cdc-wdm2: USB WDM device
Jan 25 15:32:17 nemi kernel: [60981.498160] qmi_wwan 2-1:1.3: wwan1: register 'qmi_wwan' at usb-0000:00:1d.7-1, QMI speaking wwan device with combined interface, 0e:b8:5e:cd:9c:78
Jan 25 15:32:17 nemi kernel: [60981.498943] qmi_wwan 1-1:1.3: cdc-wdm3: USB WDM device
Jan 25 15:32:17 nemi kernel: [60981.502264] qmi_wwan 1-1:1.3: wwan2: register 'qmi_wwan' at usb-0000:00:1a.7-1, QMI speaking wwan device with combined interface, 0e:b8:5e:cd:9c:78
Jan 25 15:32:17 nemi kernel: [60981.502601] usbcore: registered new interface driver qmi_wwan
crw-rw---T 1 root dialout 180, 0 Jan 25 15:32 /dev/cdc-wdm0
crw-rw---T 1 root dialout 180, 1 Jan 25 15:32 /dev/cdc-wdm1
crw-rw---T 1 root dialout 180, 2 Jan 25 15:32 /dev/cdc-wdm2
crw-rw---T 1 root dialout 180, 3 Jan 25 15:32 /dev/cdc-wdm3
Detecting netdev => wdm mapping based on USB device connection works
well enough as long as there is only one netdev per USB device. And
detecting AT command interface is as easy as just trying AT or ATI and
see if you get an OK back. The Huawei modem does not seem to have any
problems with this kind of noise, so protocol probing in the order
AT,QMI is possible.
I personally never liked the port probing crazy, but that is up to
ModemManager. Inside oFono we do not need this. And we can nicely figure
out which device nodes belongs to which device. Especially since we are
also understanding the USB interface order as well.
Obviously it can be optimized, but we tried the tagging thing with early
versions of 3G support in NetworkManager (via HAL .fdi files). It
really breaks down for a lot of devices, because you simply cannot
depend on the manufacturers for sane USB VID/PID in many cases. While
data cards sometimes have this issue (I'm looking at you, ZTE), they
weren't the major problem: tethered phones were. It turns out there
are so many phones out there, and many of those use the same VID/PID for
wildly different devices with different command sets and port layouts.

So if you can't depend on VID/PID to know what command set the device
supports, or even what protocols each port speaks, what else can you do?
You have to probe, at least to see if it's an AT port or DIAG or QMI or
WMC or CnS or whatever. Nice modems include AT commands to tell you the
port layouts, but there aren't many of those.

And probing made MM work with a much larger variety of hardware than
tagging ever could, since we'd always be chasing manufacturers releasing
new devices. Second, while the tagging approach does allow users to
test out new devices themselves, we found lots of confusion and issues
reported by users that tried to tag devices manually themselves and got
it wrong. Probing also staunched that flow of bug reports.

So in the end, a combination of port tagging and port probing is the
best way to go. If you don't have tags, you need to probe, otherwise
you'll never achieve any sort of plug and play or good user experience.
Of course on more embedded systems where you have full control over the
hardware it's easier to use tags.

The probing behavior of MM is not optimal right now, and that's
something we're working on, and I expect it to be much quicker in the
near future.
Post by Marcel Holtmann
Post by Bjørn Mork
We could also put known protocols per device into the usbnet driver and
export that to userspace using sysfs, but I don't know if it has enough
value?
If you know everything up front, this would make it easier. With cdc-wdm
you might have a chance to do this, with the "option" driver it is
almost hopeless ;)
The problem here is that the drivers can only know this based on VID/PID
since they don't really do any inspection of the ports themselves. And
if the driver gets it wrong, it takes a lot more time to change the
kernel than it does to update a package with new udev tags. I'd love it
if we could do this, but I'm worried that encoding too much logic like
this in the kernel will bite us in the future when new devices come out
with different layouts but the same VID/PID or such.

Dan



--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjørn Mork
2012-01-25 22:05:57 UTC
Permalink
Post by Bjørn Mork
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 255 Vendor Specific Subclass
bInterfaceProtocol 255 Vendor Specific Protocol
iInterface 0=20
** UNRECOGNIZED: 05 24 00 10 01
** UNRECOGNIZED: 05 24 15 00 01
** UNRECOGNIZED: 05 24 06 01 01
** UNRECOGNIZED: 15 24 12 20 01 98 b0 6a 49 b0 9e 48 96 94 46 =
d9 9a 28 ca 4e 5d
Post by Bjørn Mork
** UNRECOGNIZED: 06 24 13 00 01 10
These extra descriptors are sort of interesting when it comes to
reliably detecting the correct interfaces on the Gobi devices.
Lets for a moment assume that these are CDC devices. Then you've got:

05 24 00 10 01 =3D> CDC Header version 1.10
05 24 15 00 01 =3D> CDC OBEX version 1.00
05 24 06 01 01 =3D> CDC Union master=3D1, slave=3D1 (weird...)
15 24 12 20... =3D> CDC MDLM with a GUID
06 24 13 00 01 10 =3D> MDLM detail descriptor
Post by Bjørn Mork
bLength 9
bDescriptorType 4
bInterfaceNumber 2
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 255 Vendor Specific Subclass
bInterfaceProtocol 255 Vendor Specific Protocol
iInterface 0=20
** UNRECOGNIZED: 05 24 00 10 01
** UNRECOGNIZED: 04 24 02 02
** UNRECOGNIZED: 05 24 01 03 02
** UNRECOGNIZED: 05 24 06 02 02
** UNRECOGNIZED: 15 24 12 20 01 98 b0 6a 49 b0 9e 48 96 94 46 =
d9 9a 28 ca 4e 5d
Post by Bjørn Mork
** UNRECOGNIZED: 06 24 13 00 01 10
Much the same with additionally:
04 24 02 02 =3D> CDC ACM with line coding and serial state capabilit=
ies
05 24 01 03 02 =3D> CDC Call Management using data interface=3D2
Post by Bjørn Mork
bLength 9
bDescriptorType 4
bInterfaceNumber 3
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 255 Vendor Specific Subclass
bInterfaceProtocol 255 Vendor Specific Protocol
iInterface 0=20
** UNRECOGNIZED: 05 24 00 10 01
** UNRECOGNIZED: 05 24 15 00 01
** UNRECOGNIZED: 05 24 06 03 03
** UNRECOGNIZED: 15 24 12 20 01 98 b0 6a 49 b0 9e 48 96 94 46 =
d9 9a 28 ca 4e 5d
Post by Bjørn Mork
** UNRECOGNIZED: 06 24 13 00 01 30
similar to interface 1, except that the CDC MDLM detail data is 30
instead of 10. Whatever that means. But judging from your description=
,
this probably tells whether it's a DIAG or NMEA port.

So all these three seem to have a CDC Union descriptor with
master =3D=3D slave. An attempt to describe the combined interface typ=
e?
Interface 2 is probably just a CDC ACM interface in disguise, which
matches your description of it. =20

Hmm, looking at the Gobi driver and what you write, the correct
interface to use for rmnet/qmi is 0, which is the one *without* any suc=
h
CDC descriptors. Well, we can certainly make a rule of that as well :-=
)

The UML290 should be easy to match precisely as it uses different
subclass codes for the different functions.



Bj=C3=B8rn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Oliver Neukum
2012-01-25 22:15:41 UTC
Permalink
Post by Bjørn Mork
So all these three seem to have a CDC Union descriptor with
master =3D=3D slave. An attempt to describe the combined interface t=
ype?

Very likely. This is common in POTS modems. Look at cdc_acm.c and combi=
ned_interfaces

Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Oliver Neukum
2012-01-23 17:51:54 UTC
Permalink
Post by Dan Williams
0 (2/2/1): CDC-ACM for AT commands
1 (10/0/0): CDC-DATA for interface 0
2 (ff/ff/ff): Qualcomm DIAG
3 (ff/fd/ff): NMEA
4 (ff/fe/ff): Pantech WMC
5 (ff/f0/ff): RMNET/QMI port
Interface 5 is obviously the one we want here. And the WDM driver is
only looking for certain descriptors. Do we hack CDC-WDM and qmi_wwan
up for these types of devices?
Second, on Gobi devices, we have four USB interfaces, all FF/FF/FF.
Intf 0 and 2 have interrupt endpoints. One of them is a DIAG interface,
one is NMEA, and the other two are AT and RMNET/QMI.
I think so far the Huawei device is the only one that I've seen that
exposes descriptors that are quasi-CDC at all. How should we handle the
rest of these?
As long as they speak the same protocol, we have no way but to code them
hard into cdc-wdm as special cases. They have to be special cases as it is
unpredictable what interface to talk to. So we can just as well keep the code
in a single driver.

Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...