lkml.org 
[lkml]   [2001]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: PROBLEM: usb not working with 2.4.8-ac8
    On Wed, Aug 22, 2001, Pete Zaitcev <zaitcev@redhat.com> wrote:
    > The usb-uhci and ohci worked fine, so I poked uhci a little bit
    > and made it all work, at least with my setup. Apparently,
    > sometimes uhci returns a success (0) from submit_urb, but
    > forgets to deliver a completion callback.
    >
    > The fix is a Johannes' territory and I not so sure of my way there,
    > so perhaps a better one may be forthcoming.

    Forgets may or may not be true. Success (0) from submit_urb only means
    the URB was scheduled.

    You may not receive a completion callback if the device NAK's the
    transfer forever, or if there is a bug in uhci.c where the URB didn't
    really get scheduled.

    What was you test scenario where you didn't see a completion callback?

    > diff -ur -X dontdiff linux-2.4.8-ac9/drivers/usb/uhci.c linux-2.4.8-ac9-niph/drivers/usb/uhci.c
    > --- linux-2.4.8-ac9/drivers/usb/uhci.c Wed Aug 22 11:01:57 2001
    > +++ linux-2.4.8-ac9-niph/drivers/usb/uhci.c Wed Aug 22 13:25:00 2001
    > @@ -1471,6 +1471,7 @@
    >
    > static int uhci_submit_urb(struct urb *urb)
    > {
    > + struct usb_device *dev;
    > int ret = -EINVAL;
    > struct uhci *uhci;
    > unsigned long flags;
    > @@ -1480,15 +1481,16 @@
    > if (!urb)
    > return -EINVAL;
    >
    > - if (!urb->dev || !urb->dev->bus || !urb->dev->bus->hcpriv) {
    > + dev = urb->dev;
    > + if (!dev || !dev->bus || !dev->bus->hcpriv) {
    > warn("uhci_submit_urb: urb %p belongs to disconnected device or bus?", urb);
    > return -ENODEV;
    > }
    >
    > - uhci = (struct uhci *)urb->dev->bus->hcpriv;
    > + uhci = (struct uhci *)dev->bus->hcpriv;
    >
    > INIT_LIST_HEAD(&urb->urb_list);
    > - usb_inc_dev_use(urb->dev);
    > + usb_inc_dev_use(dev);
    >
    > spin_lock_irqsave(&urb->lock, flags);
    >
    > @@ -1497,7 +1499,7 @@
    > dbg("uhci_submit_urb: urb not available to submit (status = %d)", urb->status);
    > /* Since we can have problems on the out path */
    > spin_unlock_irqrestore(&urb->lock, flags);
    > - usb_dec_dev_use(urb->dev);
    > + usb_dec_dev_use(dev);
    >
    > return ret;
    > }
    > @@ -1516,7 +1518,7 @@
    > }
    >
    > /* Short circuit the virtual root hub */
    > - if (urb->dev == uhci->rh.dev) {
    > + if (dev == uhci->rh.dev) {
    > ret = rh_submit_urb(urb);
    >
    > goto out;
    > @@ -1528,13 +1530,13 @@
    > break;
    > case PIPE_INTERRUPT:
    > if (urb->bandwidth == 0) { /* not yet checked/allocated */
    > - bustime = usb_check_bandwidth(urb->dev, urb);
    > + bustime = usb_check_bandwidth(dev, urb);
    > if (bustime < 0)
    > ret = bustime;
    > else {
    > ret = uhci_submit_interrupt(urb);
    > if (ret == -EINPROGRESS)
    > - usb_claim_bandwidth(urb->dev, urb, bustime, 0);
    > + usb_claim_bandwidth(dev, urb, bustime, 0);
    > }
    > } else /* bandwidth is already set */
    > ret = uhci_submit_interrupt(urb);
    > @@ -1548,7 +1550,7 @@
    > ret = -EINVAL;
    > break;
    > }
    > - bustime = usb_check_bandwidth(urb->dev, urb);
    > + bustime = usb_check_bandwidth(dev, urb);
    > if (bustime < 0) {
    > ret = bustime;
    > break;
    > @@ -1556,7 +1558,7 @@
    >
    > ret = uhci_submit_isochronous(urb);
    > if (ret == -EINPROGRESS)
    > - usb_claim_bandwidth(urb->dev, urb, bustime, 1);
    > + usb_claim_bandwidth(dev, urb, bustime, 1);
    > } else /* bandwidth is already set */
    > ret = uhci_submit_isochronous(urb);
    > break;
    > @@ -1578,8 +1580,14 @@
    >
    > uhci_unlink_generic(uhci, urb);
    > uhci_destroy_urb_priv(urb);
    > + if (ret == 0) { /* N.B. Done, must notify */
    > + /* uhci_call_completion(urb); */ /* ->> uhci_destroy_urb_priv */
    > + urb->dev = NULL;
    > + if (urb->complete)
    > + urb->complete(urb);
    > + }
    >
    > - usb_dec_dev_use(urb->dev);
    > + usb_dec_dev_use(dev);
    >
    > return ret;
    > }

    What's all of this for? Protecting against an urb->dev race condition?

    JE

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 12:58    [W:0.033 / U:0.048 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site