David Ward <david.w...@ll.mit.edu> writes:

> An earlier e-mail from Reinhard contained a patch that did this for the
> MC7304. That patch could be modified as shown below so that the control
> request is enabled or disabled from qcprobe() instead. This way, it is
> straightforward to enable it for specific interface(s) on a particular
> device layout (here, it is only enabled on the AT port in the Sierra
> Wireless layout). I tested this with the Dell Wireless 5808e and it was
> able to connect to the mobile network as usual and also send AT URCs.

I think this is a good idea.

But I believe it would be nicer to consolidate the send_setup handling
for all usb-wwan based drivers instead of making multiple copies of it.
The code here seems to be modelled after the current option
implementation.  But there isn't really much driver-specific stuff in
that at all.  The only thing is the option_private/qcserial_private
thingy, which I don't understand the need for...

Johan, apologies for not commenting on this when you added it, but I
fail to understand why you added the option_private struct in commit
e463c6dda8f5 ("USB: option: handle send_setup blacklisting at probe")
Care to explain?

Moving the blacklisting logic from send_setup to probe made sense, but
the ifNum change seems completely unrelated (and unnecessary?):


@@ -1429,16 +1449,10 @@ static int option_send_setup(struct usb_serial_port 
*port)
 {
        struct usb_serial *serial = port->serial;
        struct usb_wwan_intf_private *intfdata = usb_get_serial_data(serial);
+       struct option_private *priv = intfdata->private;
        struct usb_wwan_port_private *portdata;
-       int ifNum = serial->interface->cur_altsetting->desc.bInterfaceNumber;
        int val = 0;
 
-       if (is_blacklisted(ifNum, OPTION_BLACKLIST_SENDSETUP,
-                       (struct option_blacklist_info *) intfdata->private)) {
-               dbg("No send_setup on blacklisted interface #%d\n", ifNum);
-               return -EIO;
-       }
-
        portdata = usb_get_serial_port_data(port);
 
        if (portdata->dtr_state)
@@ -1446,9 +1460,9 @@ static int option_send_setup(struct usb_serial_port *port)
        if (portdata->rts_state)
                val |= 0x02;
 
-       return usb_control_msg(serial->dev,
-               usb_rcvctrlpipe(serial->dev, 0),
-               0x22, 0x21, val, ifNum, NULL, 0, USB_CTRL_SET_TIMEOUT);
+       return usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
+                               0x22, 0x21, val, priv->bInterfaceNumber, NULL,
+                               0, USB_CTRL_SET_TIMEOUT);
 }
 
 MODULE_AUTHOR(DRIVER_AUTHOR);



If we go back to using
serial->interface->cur_altsetting->desc.bInterfaceNumber for the control
message instead of priv->bInterfaceNumber, then we can completely drop
'struct option_private' and make option_send_setup() into a generic
usb_wwan_send_setup() for use in both option and qcserial.



Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to