Hi Alan, On 07/09/2012 05:23 AM, Laurent Pinchart wrote: > On Sunday 08 July 2012 11:18:35 Alan Stern wrote: >> On Sun, 8 Jul 2012, Eric Ding wrote: >>>>> I think the reason was the way rpm_suspend() worked at the time of that >>>>> commit (it works a bit differently now, but not as much as to avoid the >>>>> problem). >>>>> >>>>> Namely, before commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2 the >>>>> device had a device type without runtime PM callbacks. So, >>>>> rpm_suspend() saw that dev->type was set and dev->type->pm was set, so >>>>> it assigned NULL to callback. As a result, nothing happened when >>>>> rpm_callback(callback, dev) was run. >>>> >>>> I don't follow. If that were the reason then no USB device would have >>>> been runtime-suspended before the e1620d commit. >>>> >>>> Are you saying this actually was true for some period of time (such as >>>> between the commit that added the "callback" variable and the e1620d >>>> commit)? >>> >>> I think this is, in fact, what happened. See rpm_suspend() before and >>> after commit 9659cc0; I suspect that between commit 9659cc0 and commit >>> e1620d5, no USB device was being runtime-suspended. Since this was >>> after v2.6.38 and before v2.6.39-rc1, though, it wouldn't have been >>> widely seen or tested, right? >> >> I guess so. That would explain it. [clipped] >> In the meantime, can you try out this patch in place of your own? (I >> haven't even tried to compile it, so there may be one or two small errors.) > > The patch looks good (although I'd replace dev_dbg with uvc_trace). It > compiles but I haven't tested it yet, I have no Logitech webcam with me right > now.
The patch compiles cleanly and seems to work here (video and audio) without any problem. One question, though: is it really necessary to do the reset in this block "in case the device was just resumed"? I think you said before that this is to guard against the possibility that the device was already autoresumed before this code is called, but usb_enable_autosuspend() isn't called until the end of uvc_probe(); doesn't that mean that your scenario wouldn't ever happen? As long as the quirk is set before usb_enable_autosuspend() is called, isn't a precautionary reset unnecessary? Eric -- 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
