On Fri, Dec 02, 2016 at 03:23:50PM +0100, Alexandre Bailon wrote: > On 11/29/2016 06:56 PM, Bin Liu wrote: > > On Tue, Nov 29, 2016 at 06:41:04PM +0100, Alexandre Bailon wrote: > >> On 11/29/2016 05:16 PM, Bin Liu wrote: > >>> Hi, > >>> > >>> On Mon, Nov 28, 2016 at 05:26:20PM +0100, Alexandre Bailon wrote: > >>>> On da8xx, VBUS is not maintained during suspend when musb is in host > >>>> mode. > >>>> On resume, all the connected devices will be disconnected and then will > >>>> be enumerated again. > >>>> This happens because MUSB_DEVCTL is cleared during suspend. > >>>> Add a quirk to preserve MUSB_DEVCTL during a suspend. > >>> > >>> Will preserving MUSB_DEVCTL prevent the device getting disconnected? > >> Yes. Preserving MUSB_DEVCTL keeps VBUS on though the PHY is off during > >> suspend. > > > > VBUS will be on, but does it prevent disconnecting the usb device on > > resume? I don't have a DA8xx to test, but I doubt it, since the PHY is > > off. > Actually, the PHY is not really off.
I guess the PHY should be off when the system is suspended for an aggressive power saving. Sekhar, any comments? Regards, -Bin. > The name is probably confusing but the PHY off only put the PHY in the > lowest power state (chapter 10.11.1 of omapl138 TRM). > I have tried with the TI kernel and it was able to suspend and resume > the omapl138-lcdk without without getting a disconnect. > That why I wrote this series. > > Regards, > Alexandre > > > >> Note that in device mode, the disconnect still happen but I think it's > >> the expected behavior. > > > > Right, the PHY off disables the pullup. > > > >>> This doesn't happen on am335x. The PHY is off during suspend, during > >>> resume, PHY gets on, MUSB generates MUSB_CONNECT interrupt, then > >>> re-enumeration happens. > >> I haven't been able to try on the BBB. I don't know if my config is > >> incorrect or if some patches are missing but I was not able to > >> suspend it. > > > > I heard the mainline kernel currently is broken on am335x PM, so > > suspend/resume doesn't work. I had to test it on TI kernel. > > > > Regards, > > -Bin. > > > >> > >> Regards, > >> Alexandre > >>> > >>> Regards, > >>> -Bin. > >>> > >>>> > >>>> Signed-off-by: Alexandre Bailon <[email protected]> > >>>> --- > >>>> drivers/usb/musb/musb_core.c | 13 +++++++------ > >>>> drivers/usb/musb/musb_core.h | 1 + > >>>> 2 files changed, 8 insertions(+), 6 deletions(-) > >>>> > >>>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > >>>> index 9e22646..7e2cd98 100644 > >>>> --- a/drivers/usb/musb/musb_core.c > >>>> +++ b/drivers/usb/musb/musb_core.c > >>>> @@ -1040,14 +1040,15 @@ static void musb_enable_interrupts(struct musb > >>>> *musb) > >>>> > >>>> } > >>>> > >>>> -static void musb_generic_disable(struct musb *musb) > >>>> +static void musb_generic_disable(struct musb *musb, bool suspend) > >>>> { > >>>> void __iomem *mbase = musb->mregs; > >>>> > >>>> musb_disable_interrupts(musb); > >>>> > >>>> /* off */ > >>>> - musb_writeb(mbase, MUSB_DEVCTL, 0); > >>>> + if (!suspend || !(musb->io.quirks & MUSB_PRESERVE_DEVCTL)) > >>>> + musb_writeb(mbase, MUSB_DEVCTL, 0); > >>>> } > >>>> > >>>> /* > >>>> @@ -1106,7 +1107,7 @@ void musb_stop(struct musb *musb) > >>>> { > >>>> /* stop IRQs, timers, ... */ > >>>> musb_platform_disable(musb); > >>>> - musb_generic_disable(musb); > >>>> + musb_generic_disable(musb, false); > >>>> musb_dbg(musb, "HDRC disabled"); > >>>> > >>>> /* FIXME > >>>> @@ -2310,7 +2311,7 @@ musb_init_controller(struct device *dev, int nIrq, > >>>> void __iomem *ctrl) > >>>> > >>>> /* be sure interrupts are disabled before connecting ISR */ > >>>> musb_platform_disable(musb); > >>>> - musb_generic_disable(musb); > >>>> + musb_generic_disable(musb, false); > >>>> > >>>> /* Init IRQ workqueue before request_irq */ > >>>> INIT_DELAYED_WORK(&musb->irq_work, musb_irq_work); > >>>> @@ -2486,7 +2487,7 @@ static int musb_remove(struct platform_device > >>>> *pdev) > >>>> musb_gadget_cleanup(musb); > >>>> spin_lock_irqsave(&musb->lock, flags); > >>>> musb_platform_disable(musb); > >>>> - musb_generic_disable(musb); > >>>> + musb_generic_disable(musb, false); > >>>> spin_unlock_irqrestore(&musb->lock, flags); > >>>> musb_writeb(musb->mregs, MUSB_DEVCTL, 0); > >>>> pm_runtime_dont_use_autosuspend(musb->controller); > >>>> @@ -2663,7 +2664,7 @@ static int musb_suspend(struct device *dev) > >>>> unsigned long flags; > >>>> > >>>> musb_platform_disable(musb); > >>>> - musb_generic_disable(musb); > >>>> + musb_generic_disable(musb, true); > >>>> WARN_ON(!list_empty(&musb->pending_list)); > >>>> > >>>> spin_lock_irqsave(&musb->lock, flags); > >>>> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > >>>> index a611e2f..22961ef 100644 > >>>> --- a/drivers/usb/musb/musb_core.h > >>>> +++ b/drivers/usb/musb/musb_core.h > >>>> @@ -172,6 +172,7 @@ struct musb_io; > >>>> */ > >>>> struct musb_platform_ops { > >>>> > >>>> +#define MUSB_PRESERVE_DEVCTL BIT(7) > >>>> #define MUSB_DMA_UX500 BIT(6) > >>>> #define MUSB_DMA_CPPI41 BIT(5) > >>>> #define MUSB_DMA_CPPI BIT(4) > >>>> -- > >>>> 2.7.3 > >>>> > >> > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
