On 11/25/2013 09:46 PM, Felipe Balbi wrote: > Hi, > > On Mon, Nov 25, 2013 at 09:26:55PM +0100, Daniel Mack wrote: >> On 11/25/2013 09:08 PM, Felipe Balbi wrote: >>> On Mon, Nov 25, 2013 at 09:04:16PM +0100, Daniel Mack wrote: >>>>>> diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c >>>>>> index e977441..24e46c0 100644 >>>>>> --- a/drivers/usb/musb/musb_virthub.c >>>>>> +++ b/drivers/usb/musb/musb_virthub.c >>>>>> @@ -109,7 +109,7 @@ void musb_port_suspend(struct musb *musb, bool do_suspend) >>>>>> } >>>>>> } >>>>>> >>>>>> -static void musb_port_reset(struct musb *musb, bool do_reset) >>>>>> +void musb_port_reset(struct musb *musb, bool do_reset) >>>>> >>>>> NAK, this should not be called from the glue layers at all. If anything >>>>> call from musb_core's resume callback. That will only be called after >>>>> parent's resume has been called anyway. >>>> >>>> Given that this driver is successfully used with suspend/resume on other >>>> platforms without that reset, but it's clearly needed for dsps, I'm >>>> fairly sure this should be a glue-layer specific thing. Hence the change. >>>> >>>> I think it'll break things on other platforms if we unconditionally >>>> reset the port after resume, but I can't test it. Anyone? >>> >>> your original commit log only says "we need to issue port reset" but it >>> never explains why. It could very well be you just found a device which >>> needs to be reset when coming out of suspend and it misses a quirk flag ? >> >> I think I can really rule that out, as I tested with multiple USB sticks >> and also tested the same sticks on other embedded platforms. >> >>> In any case, those functions should never be called by the glue, if >>> reset needs to be called, it must be called by musb-hdrc.ko, if you need >>> a flag, pass a flag but I need a really good explanation in your commit >>> log of why that's necessary. >> >> Well, if I'd only knew exactly why. All these patches are really the >> result of many days of trial and error, with multiple drivers (musb, >> cppi41, ...) involved. And as I said - I have no documentation of the >> musb core itself. >> >> So yes, I can do it the other way around and pass a flag, but what I >> can't provide is a good explanantion why the dsps glue behaves >> differently here than others. I'm curious myself, and all I know is that >> with this reset in place, things work as expected on AM33xx. > > ok, then let's pass a flag. Meanwhile I'll try to figure out internally > why we need that reset. Well, we don't need a reset, I wasn't quite precise here. What we need is to manually *de*assert the reset when we resume. Note the 'false' flag that is passed to musb_port_reset(). > As Alan said, usb-persist should already for a > bus-reset when resuming, right ? The explicit un-reset is really needed, otherwise the port will reenumerate the device, and a previously mounted filesystem will become invalid of course. The log looks like this in that case: [ 17.045641] PM: resume of devices complete after 166.084 msecs [ 17.056461] PM: Sending message for resetting M3 state machine [ 17.063451] Restarting tasks ... [ 17.071767] usb 1-1: USB disconnect, device number 2 done. [ 17.403402] usb 1-1: new high-speed USB device number 3 using musb-hdrc [ 17.766959] usb 1-1: New USB device found, idVendor=058f, idProduct=6387 [ 17.774849] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Thanks, Daniel -- 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
