On 09/08/2015 05:16 PM, Reinhard Speyerer wrote: > On Mon, Aug 31, 2015 at 07:18:05PM +0000, Ward, David - 0665 - MITLL wrote: >> On 01/06/2015 03:58 AM, Bj=F8rn Mork wrote: >>> Johan Hovold <johan@xxxxxxxxxx> writes: >>> >>>> Ok, let's move the PID to option and if it turns out that more of these >>>> devices require the modem-control signals (e.g. with more recent >>>> firmware) we can consider moving it back after adding such support to >>>> qcserial. >>>> >> This also affects the Dell Wireless 5808e, a branded version of the >> Sierra Wireless EM7355. Again, moving it from the qcserial driver to the >> option driver, which calls send_setup, allows URCs to appear. >> >> Is it possible to test if the MC7710 also works with the option driver >> (does send_setup cause problems with it)? If that still works, are there >> any reasons not to move all of the devices using the Sierra Wireless >> layout (DEVICE_SWI) from the qcserial driver to the option driver? > Hi David, > > due to lack of time I could not test your patch > you sent me yesterday. I checked however using > > # rmmod qcserial > # modprobe option > # echo 1199 68a2 > /sys/bus/usb-serial/drivers/option1/new_id > > that AT URCs with a MC7710 still work when option_send_setup() is used. > > Since AT URCs already work with the qcserial driver for the MC7710 > "out of the box" > > at+gmr > SWI9200X_03.05.29.03ap r6485 CNSHZ-ED-XP0031 2014/12/02 17:53:15 > OK > at+cscs=3D"GSM" > OK > at+cusd=3D1,"*101#",15 > OK > > +CUSD: 0,"Ihr Guthaben betr{gt: 42,00. Jetzt auch Ihr Guthaben aufladen: einfach *103*Aufladenummer# und die H|rertaste eingeben.",15 > > my suggestion would be to keep the MC77xx in the qcserial driver to > avoid having to add additional blacklist entries. The current > version of your patch incorrectly uses the MC73xx blacklist also for > the MC77xx which would cause the option driver to bind to the MC77xx > QMI interfaces on USB IF 19 and 20 which it should not do. > > Instead of moving the MC77xx to the option driver it might be more > worthwhile to check whether other EM7305/EM7355-based devices like the > recently added Dell Wireless 5809e device are also affected by the > missing AT URCs and should be moved from the qcserial to the option driver. Reinhard, thanks for trying this on the MC7710 (and pointing out the difference in device layout). Are there enough devices with this behavior now to re-consider adding the control request to the qcserial driver, rather than moving another device to the option driver? The request doesn't seem to cause issues on the MC7710 where it is not needed, and it is already implemented in the "sierra" driver as well. 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. David diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c index ebcec8c..c12836e 100644 --- a/drivers/usb/serial/qcserial.c +++ b/drivers/usb/serial/qcserial.c @@ -22,6 +22,10 @@ #define DRIVER_AUTHOR "Qualcomm Inc" #define DRIVER_DESC "Qualcomm USB Serial driver" +struct qcserial_private { + u8 bInterfaceNumber; +}; + /* standard device layouts supported by this driver */ enum qcserial_layouts { QCSERIAL_G2K = 0, /* Gobi 2000 */ @@ -175,6 +179,7 @@ static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id) __u8 nintf; __u8 ifnum; int altsetting = -1; + bool sendsetup = false; nintf = serial->dev->actconfig->desc.bNumInterfaces; dev_dbg(dev, "Num Interfaces = %d\n", nintf); @@ -286,6 +291,7 @@ static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id) break; case 3: dev_dbg(dev, "Modem port found\n"); + sendsetup = true; break; default: /* don't claim any unsupported interface */ @@ -337,17 +343,65 @@ done: } } + if (!retval) + usb_set_serial_data(serial, (void *)(unsigned long)sendsetup); + return retval; } +/* Send RTS/DTR state to the port (CDC ACM SET_CONTROL_LINE_STATE request) */ +static int qc_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 usb_wwan_port_private *portdata = usb_get_serial_port_data(port); + struct qcserial_private *priv = intfdata->private; + int val = 0; + int res; + + if (portdata->dtr_state) + val |= 0x01; + if (portdata->rts_state) + val |= 0x02; + + res = usb_autopm_get_interface(serial->interface); + if (res) + return res; + + res = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0), + 0x22, 0x21, val, priv->bInterfaceNumber, NULL, + 0, USB_CTRL_SET_TIMEOUT); + + usb_autopm_put_interface(serial->interface); + + return res; +} + static int qc_attach(struct usb_serial *serial) { + struct usb_interface_descriptor *iface_desc; struct usb_wwan_intf_private *data; + struct qcserial_private *priv; + bool sendsetup; data = kzalloc(sizeof(*data), GFP_KERNEL); if (!data) return -ENOMEM; + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) { + kfree(data); + return -ENOMEM; + } + + iface_desc = &serial->interface->cur_altsetting->desc; + priv->bInterfaceNumber = iface_desc->bInterfaceNumber; + data->private = priv; + + sendsetup = !!(unsigned long)(usb_get_serial_data(serial)); + if (sendsetup) + data->send_setup = qc_send_setup; + spin_lock_init(&data->susp_lock); usb_set_serial_data(serial, data); @@ -357,10 +411,12 @@ static int qc_attach(struct usb_serial *serial) static void qc_release(struct usb_serial *serial) { - struct usb_wwan_intf_private *priv = usb_get_serial_data(serial); + struct usb_wwan_intf_private *intfdata = usb_get_serial_data(serial); + struct qcserial_private *priv = intfdata->private; usb_set_serial_data(serial, NULL); kfree(priv); + kfree(intfdata); } static struct usb_serial_driver qcdevice = { @@ -374,6 +430,7 @@ static struct usb_serial_driver qcdevice = { .probe = qcprobe, .open = usb_wwan_open, .close = usb_wwan_close, + .dtr_rts = usb_wwan_dtr_rts, .write = usb_wwan_write, .write_room = usb_wwan_write_room, .chars_in_buffer = usb_wwan_chars_in_buffer, -- 1.7.1 -- 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