Re: umodem(4) with "no pointer to data interface"

[Ryo ONODERA <ryo_on%yk.rim.or.jp@localhost>]

Hi,

From: Brett Lymn <blymn%internode.on.net@localhost>, Date: Wed, 22 Jan 2014 
22:17:52 +1030

> On Wed, Jan 22, 2014 at 12:28:50PM +0100, Felix Deichmann wrote:
>> 
>> Adding the Conexant modem with UQ_NO_UNION_NRM (or maybe something
>> else?) to usb_quirks would be the most elegant solution, right?
> 
> only part of the solution - you would get hit by the "no data interface"
> when it tries to get that bit because the code in
> umodoem_common_attach() only looks for data endpoints on the same
> interface as the control
> (http://nxr.netbsd.org/xref/src/sys/dev/usb/umodem_common.c#149).
> 
>> Then umodem_get_caps() should be fixed, and OpenBSD (seems to
>> "brute-force" and iterate over all descriptors anyway) or Linux
>> (treats data if = 1, ctl if = 0 (?) for NO_UNION_NORMAL quirk) can
>> serve as an example.
>> But this is beyond me now :-).
> 
> Probably iterating over all the interfaces iff we are about to spit the
> "no data interface" error would be an ok thing to do.  In effect, we try
> nicely first and if that doesn't work then use brute force.  IIRC the
> rest of the driver should be fine with that because the data interface
> is referenced separately to the control interface.

Each quirky USB modem has specific data interface number. In my case it is 1.
This number should be given from dev table, I feel.
If so, brute-force or general assumption is not needed.

How about the following change?

Index: umodem.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/umodem.c,v
retrieving revision 1.65
diff -u -r1.65 umodem.c
--- umodem.c    3 Oct 2012 07:07:04 -0000       1.65
+++ umodem.c    22 Jan 2014 13:23:33 -0000
@@ -91,19 +91,44 @@
 CFATTACH_DECL_NEW(umodem, sizeof(struct umodem_softc), umodem_match,
     umodem_attach, umodem_detach, umodem_activate);
 
+/*
+ * To match or ignore forcibly, add
+ *
+ *     { { VendorID, ProductID },
+ *     UMATCH_VENDOR_PRODUCT|UMATCH_NONE, DataInterfaceNo }
+ *
+ * to the umodem_dev list.
+ */
+const struct umodem_devno {
+       struct usb_devno        devno;
+       int                     match;
+       int                     did;
+} umodem_dev[] = {
+       { { USB_VENDOR_CONEXANT, USB_PRODUCT_CONEXANT_USB56KMODEM1}, 
+         UMATCH_VENDOR_PRODUCT, 1 },
+};
+#define umodem_lookup(vendor, product) \
+       ((const struct umodem_devno *)usb_lookup(umodem_dev, vendor, product))
+
 int 
 umodem_match(device_t parent, cfdata_t match, void *aux)
 {
        struct usbif_attach_arg *uaa = aux;
        usb_interface_descriptor_t *id;
        int cm, acm;
+       const struct umodem_devno *dev;
+
+       id = usbd_get_interface_descriptor(uaa->iface);
+
+       if ((dev = umodem_lookup(uaa->vendor, uaa->product)) != NULL &&
+           dev->did == id->bInterfaceNumber )
+               return dev->match;
 
        if (uaa->class != UICLASS_CDC ||
            uaa->subclass != UISUBCLASS_ABSTRACT_CONTROL_MODEL ||
            !(uaa->proto == UIPROTO_CDC_NOCLASS || uaa->proto == 
UIPROTO_CDC_AT))
                return (UMATCH_NONE);
 
-       id = usbd_get_interface_descriptor(uaa->iface);
        if (umodem_get_caps(uaa->device, &cm, &acm, id) == -1)
                return (UMATCH_NONE);
 

With this patch,

umodem0 at uhub4 port 1 configuration 1 interface 1
umodem0: Conexant USB Modem, rev 1.10/1.00, addr 11, iclass 10/0
umodem0: data interface 1, has CM over data, has break
ucom0 at umodem0

ucom/umodem functionalty is not tested, but it should work.

--
Ryo ONODERA // ryo_on%yk.rim.or.jp@localhost
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3