Am Dienstag, 15. Februar 2005 22:10 schrieb Jeroen Vreeken:
> Thanks for the tips Oliver.
> Here is the third attempt.
+/* Incomming data */
+void zd1201_usbrx(struct urb *urb, struct pt_regs *regs)
[..]
+ switch(urb->status) {
+ case -EILSEQ:
+ case -ENODEV:
+ case -ETIMEDOUT:
+ case -ENOENT:
+ case -EPIPE:
ESHUTDOWN
+ if (type == ZD1201_PACKET_EVENTSTAT || type == ZD1201_PACKET_RESOURCE) {
+ memcpy(zd->rxdata, data, urb->actual_length);
+ zd->rxlen = urb->actual_length;
+ zd->rxdatas = 1;
+ wake_up(&zd->rxdataq);
+ }
What will happen to the waiting task if the error path is taken
or resubmission fails?
+ if (wait) {
+ wait_event_interruptible(zd->rxdataq, zd->rxdatas);
+ if (!zd->rxlen || zd2host16(*(short*)&zd->rxdata[6]) != rid) {
+ dev_dbg(&zd->usb->dev, "wrong or no RID received\n");
+ }
+ }
You could avoid some GFP_ATOMIC allocations here checking the flag.
+int zd1201_drvr_start(struct zd1201 *zd)
+{
+ int err, i;
+ short max;
+ unsigned char *buffer;
+
+ buffer = kmalloc(ZD1201_RXSIZE, GFP_ATOMIC);
This is called from probe() only. Use GFP_KERNEL.
+err_urb:
+ zd->tx_urb->transfer_flags |= URB_ASYNC_UNLINK;
+ usb_unlink_urb(zd->rx_urb);
+ return err;
You must make sure that the descriptor referenced in the irq handler is not
freed before this operation completes. Best use usb_kill_urb(). You don't need
to be atomic.
+static void zd1201_tx_timeout(struct net_device *dev)
+{
+ struct zd1201 *zd = (struct zd1201 *)dev->priv;
+
+ if (!zd)
+ return;
+ dev_warn(&zd->usb->dev, "%s: TX timeout, shooting down urb\n",
+ dev->name);
+ zd->tx_urb->transfer_flags |= URB_ASYNC_UNLINK;
+ usb_unlink_urb(zd->tx_urb);
+ zd->stats.tx_errors++;
+ /* Restart the timeout to quiet the watchdog: */
+ dev->trans_start = jiffies;
+ usb_free_urb(zd->tx_urb);
+ /* Restart the net queue: */
+ zd->tx_urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (zd->tx_urb)
+ netif_wake_queue(dev);
+}
This is very ingenious, but it has another race. The interrupt handler
will activate the queue at some random time in the future.
May I suggest that you use a work queue and a synchronous usb_kill_urb()
in that?
+ zd->endp_in = 1;
+ zd->endp_out = 1;
+ zd->endp_out2 = 2;
+ zd->rx_urb = usb_alloc_urb(0, GFP_ATOMIC);
+ zd->tx_urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (!zd->rx_urb || !zd->tx_urb)
+ goto err_zd;
Use GFP_KERNEL
+void zd1201_disconnect(struct usb_interface *interface)
You should use usb_kill_urb() on all URBs in question.
Regards
Oliver
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
