On Mon, 2016-07-04 at 13:54 +0200, Bjørn Mork wrote:
> Oliver Neukum <[email protected]> writes:
>
> > On Sun, 2016-07-03 at 21:59 +0200, Bjørn Mork wrote:
> >> default:
> >> @@ -200,10 +200,29 @@ static void wdm_in_callback(struct urb *urb)
> >> desc->reslength = length;
> >> }
> >> }
> >> +
> >> + /*
> >> + * If desc->resp_count is unset, then the urb was submitted
> >> + * without a prior notification. If the device returned any
> >> + * data, then this implies that it had messages queued without
> >> + * notifying us. Continue reading until that queue is flushed.
> >> + */
> >> + if (!desc->resp_count) {
> >> + if (!length) {
> >> + /* do not propagate the expected -EPIPE */
> >> + desc->rerr = 0;
> >> + goto unlock;
> >> + }
> >> + dev_dbg(&desc->intf->dev, "got %d bytes without
> >> notification\n", length);
> >> + set_bit(WDM_RESPONDING, &desc->flags);
> >> + usb_submit_urb(desc->response, GFP_ATOMIC);
> >
> > You must check for being in an overflow condition.
>
> No, I don't think that will give the wanted/expected result. The main
> point here is to flush whatever the data the device has queued up, but
That raises the point why we store the data at all.
> haven't notified us about (or for which we've lost the notification for
> some reason). We want to continue flushing whether or not we can store
> the received data. The goal is to get back into a syncronouos state,
> not necessarily to save everything the device has queued. If we
> overflow, then the lines just before this hunk will handle that just
> fine by setting the WDM_OVERFLOW bit and dropping the data. That's what
> we want.
OK, you know the protocol better than I.
> >> + }
> >> +
> >> skip_error:
> >> wake_up(&desc->wait);
> >>
> >> set_bit(WDM_READ, &desc->flags);
> >> +unlock:
> >> spin_unlock(&desc->iuspin);
> >> }
> >>
> >> @@ -647,6 +666,16 @@ static int wdm_open(struct inode *inode, struct file
> >> *file)
> >
> > If you go for that approack you also need to do it
> > in resume()
>
> Yes, I wondered about that... But I didn't add it because I am unable
> to provoke the problem there, so I cannot really test if it has any
> positive effect. Do you still want it?
Yes. I think the window is small, but we don't want strange
irreproducible flukes.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
