Re: [PATCH 4/5] usb: musb: dsps: add support for suspend and resume

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1573 bytes --]

Hi,

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 ?

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.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]