Hi, On Wed, Sep 02, 2015 at 06:33:11PM +0200, Pascal Huerst wrote: > Hey, > > On 02.09.2015 14:28, Felipe Balbi wrote: > > Hi, > > > > On Wed, Sep 02, 2015 at 11:35:45AM +0200, Pascal Huerst wrote: > >> Hey Felipe, > >> > >> on a custom built, am335x based board, running v4.0, I'm facing an issue > >> with usb: > >> > >> After suspend, storage devices are not being enumerated correctly. I > > > > hmm, mainline kernel doesn't support suspend/resume on AM335x devices so > > I'm gonna say you need to ask for support from whoever gave you this > > kernel. If that's TI, then you need to either ask for help on TI's e2e > > forums or talk to your FAE... > > Well, that would be me. It's a vanilla kernel, except for some ASoC > patches and v5 of Dave's PM series, which was for v3.19-rc5, if I > remember that correctly, which I rebased on v4.0.0 later. > > >> have seen and applied your recent patch: > >> > >> usb: musb: host: rely on port_mode to call musb_start() > >> > >> Now, the issue only happens once. So if the device resumes the first > >> time, I face the same enumeration problem, but all following times, it > >> seems to work properly. > >> > >> I figureed out, that in a working situation: > >> > >> void musb_start(...) > >> > >> -> http://lxr.free-electrons.com/source/drivers/usb/musb/musb_core.c#L1027 > >> > >> is called before the isr: > >> > >> static irqreturn_t musb_stage0_irq(...) > >> > >> -> http://lxr.free-electrons.com/source/drivers/usb/musb/musb_core.c#L539 > >> > >> And in a non working situation, its the other way arround. (Which should > >> not happen!?) I'm not too familiar with usb. Do you have any ideas, what > >> could cause that behavior? > > > > ... with that said, you didn't give me logs of the problem happening nor > > gave me any instructions on how to reproduce. > > > > based on the description alone, I think making sure MUSB IRQs are masked > > on suspen and unmask on resume might be enough, though you'd have to > > patch that up and check. > > To reproduce, I just have an usb block device inserted, trigger suspend by: > > echo mem > /sys/power/state > > wake it up by GPIO on bank0 and then I get: > > usb 1-1: new high-speed USB device number 3 using musb-hdrc > usb 1-1: new high-speed USB device number 4 using musb-hdrc > usb 1-1: new high-speed USB device number 5 using musb-hdrc > usb 1-1: new high-speed USB device number 6 using musb-hdrc > > Since I applied your patch mentioned above, this only happens once > (first suspend). So I had to reboot every time, to provoke it. Without > your patch applied, it happened on almost all suspend/resume cycles. > > If I disable the interrupt in musb_suspend() and reactivate them in > musb_resume() everything seems to works fine. (At least the for ~20 > times I tried to reproduce afterwards) > > I guess this is the case on other hw was well, such as BBB. If you > think, that this is worth further investigation in order to fix it > upstream, I can do some testing on BBB and provide you with more detail? > > Just for clarification about my changes, I applied a patch. > > regards > pascal > > > --- > drivers/usb/musb/musb_core.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > index 68bf6d8..d861c21 100644 > --- a/drivers/usb/musb/musb_core.c > +++ b/drivers/usb/musb/musb_core.c > @@ -2421,6 +2421,9 @@ static int musb_suspend(struct device *dev) > struct musb *musb = dev_to_musb(dev); > unsigned long flags; > > + musb_platform_disable(musb); > + musb_generic_disable(musb); > + > spin_lock_irqsave(&musb->lock, flags); > > if (is_peripheral_active(musb)) { > @@ -2474,6 +2477,9 @@ static int musb_resume(struct device *dev) > pm_runtime_disable(dev); > pm_runtime_set_active(dev); > pm_runtime_enable(dev); > + > + musb_start(musb); this looks correct to me. Can you send it as a proper patch ? -- balbi
Attachment:
signature.asc
Description: Digital signature
