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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux