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. As Alan said, usb-persist should already for a bus-reset when resuming, right ? -- balbi
Attachment:
signature.asc
Description: Digital signature
