Dan Williams <dcbw@xxxxxxxxxx> wrote:
>With Gobi 1K devices, USB interface #3's altsetting is 0 by default,
>but
>altsetting 0 only provides one interrupt endpoint and is not sufficent
>for QMI. Altsetting 1 provides all 3 endpoints required for qmi_wwan
>and works with QMI.
>
>IIRC the altsetting used to be set by qcserial back before we made
>qcserial stop touching interfaces that qmi_wwan was going to use. But
>now that qcserial only touches the modem interface, we need qmi_wwan to
>set the correct altsetting on the QMI interface.
No, I broke this when I tried to be smart and merged the 1 and 2 interface probes. That's the second time I break probing of these devices by making too many unfounded assumptions.
This used to work because usbnet_get_endpoints selects the first usable altsetting. It doesn't work anymore because qmi_wwan checks the number of endpoints before calling usbnet_get_endpoints.
>The attached patch works for my Gobi1K (and should work for all other
>1Ks too) but seems somewhat ugly. What approach should we take?
>Basically, we need to know that a device is Gobi1K at probe time so we
>can set the right altsetting on it.
>
>Gobi 1K layout for intf#3 is:
>
> Interface Descriptor: 255/255/255
> bInterfaceNumber 3
> bAlternateSetting 0
> Endpoint Descriptor: Interrupt IN
> Interface Descriptor: 255/255/255
> bInterfaceNumber 3
> bAlternateSetting 1
> Endpoint Descriptor: Interrupt IN
> Endpoint Descriptor: Bulk IN
> Endpoint Descriptor: Bulk OUT
Ouch. I did not know this. I should get one of these devices, I guess. ..
>Dan
>
>---
>diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
>index efb5c7c..50e1b7c 100644
>--- a/drivers/net/usb/qmi_wwan.c
>+++ b/drivers/net/usb/qmi_wwan.c
>@@ -43,6 +43,9 @@
> * commands on a serial interface
> */
>
>+/* Quirks for the usbnet 'struct driver_info' data field */
>+#define QUIRK_GOBI_1K (1 << 0) /* QMI interface requires altsetting
>1 */
>+
> /* driver specific data */
> struct qmi_wwan_state {
> struct usb_driver *subdriver;
>@@ -326,6 +329,15 @@ static const struct driver_info qmi_wwan_info = {
> .manage_power = qmi_wwan_manage_power,
> };
>
>+static const struct driver_info gobi1k_info = {
>+ .description = "WWAN/QMI device",
>+ .flags = FLAG_WWAN,
>+ .bind = qmi_wwan_bind,
>+ .unbind = qmi_wwan_unbind,
>+ .manage_power = qmi_wwan_manage_power,
>+ .data = QUIRK_GOBI_1K,
>+};
>+
> #define HUAWEI_VENDOR_ID 0x12D1
>
> /* map QMI/wwan function by a fixed interface number */
>@@ -335,7 +347,8 @@ static const struct driver_info qmi_wwan_info = {
>
> /* Gobi 1000 QMI/wwan interface number is 3 according to qcserial */
> #define QMI_GOBI1K_DEVICE(vend, prod) \
>- QMI_FIXED_INTF(vend, prod, 3)
>+ USB_DEVICE_INTERFACE_NUMBER(vend, prod, 3), \
>+ .driver_info = (unsigned long)&gobi1k_info
>
>/* Gobi 2000/3000 QMI/wwan interface number is 0 according to qcserial
>*/
> #define QMI_GOBI_DEVICE(vend, prod) \
>@@ -541,6 +554,9 @@ MODULE_DEVICE_TABLE(usb, products);
>static int qmi_wwan_probe(struct usb_interface *intf, const struct
>usb_device_id *prod)
> {
> struct usb_device_id *id = (struct usb_device_id *)prod;
>+ struct driver_info *info;
>+ u8 intf_num = intf->cur_altsetting->desc.bInterfaceNumber;
>+ int retval;
>
> /* Workaround to enable dynamic IDs. This disables usbnet
> * blacklisting functionality. Which, if required, can be
>@@ -552,6 +568,20 @@ static int qmi_wwan_probe(struct usb_interface
>*intf, const struct usb_device_id
> id->driver_info = (unsigned long)&qmi_wwan_info;
> }
>
>+ info = (struct driver_info *) id->driver_info;
>+ if (info->data & QUIRK_GOBI_1K) {
>+ /* Gobi 1K's QMI interface is always USB interface #3 */
>+ BUG_ON(intf_num != 3);
Definitely not. We return ENODEV if the probe logic fails for whatever reason.
>+ retval = usb_set_interface(interface_to_usbdev (intf),
>+ intf_num, 1);
>+ if (retval < 0) {
>+ dev_err(&intf->dev, "Failed to set altsetting 1: %d\n",
>+ retval);
>+ return -ENODEV;
>+ }
>+ }
Risking that I am trying to be too smart again. .. But I really think we can fix this in a more generic way by making the probe behave more like it did before. How about just letting it continue without erring out until it has called usbnet_get_endpoints? That should work without needing any quirks.
Bjørn
--
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
