On Tue, 2012-01-24 at 16:29 -0600, Dan Williams wrote: > On Mon, 2012-01-23 at 21:31 +0100, Bjørn Mork wrote: > > Dan Williams <dcbw@xxxxxxxxxx> writes: > > > > > On Mon, 2012-01-23 at 19:56 +0100, Bjørn Mork wrote: > > >> Dan Williams <dcbw@xxxxxxxxxx> writes: > > >> > > >> > 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 > > >> > > >> 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. > > > > OK, nice. > > > > >> > 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. > > > > > >> 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. > > >> > > >> > 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. > > > > > >> 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... > > > > Right, forgot about that part. You could hack it so that it was happily > > ignoring the extra bulk endpoints as long as it found an interrupt > > endpoint. > > > > > > >> > 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 > > > > 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 :-) > > > > > 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. > > > > That is for Oliver to decide I guess. > > > > 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. > > > > 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. > 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 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 > > 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[] = { > .bInterfaceClass = USB_CLASS_COMM, > .bInterfaceSubClass = USB_CDC_SUBCLASS_DMM > }, > + { > + .match_flags = USB_DEVICE_ID_MATCH_VENDOR | USB_DEVICE_ID_MATCH_PRODUCT, > + USB_DEVICE_ID_MATCH_INT_INFO, > + .idVendor = 0x106c, > + .idProduct = 0x3718, > + .bInterfaceClass = 0xFF, > + .bInterfaceSubClass = 0xF0, > + .bInterfaceProtocol = 0xFF > + }, > { } > }; > > @@ -626,11 +652,12 @@ static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id) > struct usb_device *udev = interface_to_usbdev(intf); > struct wdm_device *desc; > struct usb_host_interface *iface; > - struct usb_endpoint_descriptor *ep; > + struct usb_endpoint_descriptor *ep = NULL, *tmpep; > struct usb_cdc_dmm_desc *dmhd; > u8 *buffer = intf->altsetting->extra; > int buflen = intf->altsetting->extralen; > u16 maxcom = WDM_DEFAULT_BUFSIZE; > + int i; > > if (!buffer) > goto out; > @@ -661,6 +688,22 @@ next_desc: > buffer += buffer[0]; > } > > + /* Find the interrupt endpoint */ > + rv = -EINVAL; > + iface = intf->cur_altsetting; > + for (i = 0; i < iface->desc.bNumEndpoints; i++) { > + tmpep = &iface->endpoint[i].desc; > + if (tmpep && usb_endpoint_is_int_in (tmpep)) { > + ep = tmpep; > + break; > + } > + } > + > + if (!ep) { > + dev_dbg(&intf->dev, "failed to find interrupt endpoint\n"); > + goto out; > + } > + > rv = -ENOMEM; > desc = kzalloc(sizeof(struct wdm_device), GFP_KERNEL); > if (!desc) > @@ -674,13 +717,6 @@ next_desc: > desc->intf = intf; > INIT_WORK(&desc->rxwork, wdm_rxwork); > > - rv = -EINVAL; > - iface = intf->cur_altsetting; > - if (iface->desc.bNumEndpoints != 1) > - goto err; > - ep = &iface->endpoint[0].desc; > - if (!ep || !usb_endpoint_is_int_in(ep)) > - goto err; > > desc->wMaxPacketSize = usb_endpoint_maxp(ep); > desc->bMaxPacketSize0 = udev->descriptor.bMaxPacketSize0; > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
