Hi Felipe,
On Tue, Feb 12, 2013 at 2:17 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> Add support for basic power management on
> the dwc3 driver. While there is still lots
> to improve for full PM support, this minimal
> patch will already make sure that we survive
> suspend-to-ram and suspend-to-disk without
> major issues.
>
> Cc: Vikas C Sajjan <vikas.sajjan@xxxxxxxxxx>
> Signed-off-by: Felipe Balbi <balbi@xxxxxx>
Testing this patch-series on usb-next actually giving us an issue.
We are not able to resume either of Host or Device.
Tried to find the cause, which actually came out to be missing phy_shutdown()
and phy_init() calls in suspend() and resume() respectively.
Please see more comments below.
> ---
> drivers/usb/dwc3/core.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/usb/dwc3/core.h | 33 ++++++++++++++
> drivers/usb/dwc3/gadget.c | 61 +++++++++++++++++++++++++
> 3 files changed, 205 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index fb93918..54decde 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -583,11 +583,122 @@ static int dwc3_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM
> +static int dwc3_prepare(struct device *dev)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(dev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> +
> + switch (dwc->mode) {
> + case DWC3_MODE_DEVICE:
> + case DWC3_MODE_DRD:
> + dwc3_gadget_prepare(dwc);
> + /* FALLTHROUGH */
> + case DWC3_MODE_HOST:
> + default:
> + dwc3_event_buffers_cleanup(dwc);
> + break;
> + }
> +
> + spin_unlock_irqrestore(&dwc->lock, flags);
> +
> + return 0;
> +}
> +
> +static void dwc3_complete(struct device *dev)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(dev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> +
> + switch (dwc->mode) {
> + case DWC3_MODE_DEVICE:
> + case DWC3_MODE_DRD:
> + dwc3_gadget_complete(dwc);
> + /* FALLTHROUGH */
> + case DWC3_MODE_HOST:
> + default:
> + dwc3_event_buffers_setup(dwc);
> + break;
> + }
> +
> + spin_unlock_irqrestore(&dwc->lock, flags);
> +}
> +
> +static int dwc3_suspend(struct device *dev)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(dev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> +
> + switch (dwc->mode) {
> + case DWC3_MODE_DEVICE:
> + case DWC3_MODE_DRD:
> + dwc3_gadget_suspend(dwc);
> + /* FALLTHROUGH */
> + case DWC3_MODE_HOST:
> + default:
> + /* do nothing */
> + break;
> + }
> +
Adding the calls for phy_shutdown() here
usb_phy_shutdown(dwc->usb2_phy);
usb_phy_shutdown(dwc->usb3_phy);
> + dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL);
> + spin_unlock_irqrestore(&dwc->lock, flags);
> +
> + return 0;
> +}
> +
> +static int dwc3_resume(struct device *dev)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(dev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> +
> + dwc3_writel(dwc->regs, DWC3_GCTL, dwc->gctl);
> +
and further call to phy_init() here seems to make things working for
both HOST and DEVICE.
usb_phy_init(dwc->usb2_phy);
usb_phy_init(dwc->usb3_phy);
mdelay(100);
Attaching short dmesg logs (after resuming) : "dwc3_host_not_working.txt", and
"dwc3_host_working.txt"
> + switch (dwc->mode) {
> + case DWC3_MODE_DEVICE:
> + case DWC3_MODE_DRD:
> + dwc3_gadget_resume(dwc);
> + /* FALLTHROUGH */
> + case DWC3_MODE_HOST:
> + default:
> + /* do nothing */
> + break;
> + }
> +
> + spin_unlock_irqrestore(&dwc->lock, flags);
> +
> + pm_runtime_disable(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops dwc3_dev_pm_ops = {
> + .prepare = dwc3_prepare,
> + .complete = dwc3_complete,
> +
> + SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> +};
> +
> +#define DWC3_PM_OPS &(dwc3_dev_pm_ops)
> +#else
> +#define DWC3_PM_OPS NULL
> +#endif
> +
> static struct platform_driver dwc3_driver = {
> .probe = dwc3_probe,
> .remove = dwc3_remove,
> .driver = {
> .name = "dwc3",
> + .pm = DWC3_PM_OPS,
> },
> };
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index e8d74a7..172b620 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -619,6 +619,8 @@ struct dwc3_scratchpad_array {
> * @mode: mode of operation
> * @usb2_phy: pointer to USB2 PHY
> * @usb3_phy: pointer to USB3 PHY
> + * @dcfg: saved contents of DCFG register
> + * @gctl: saved contents of GCTL register
> * @is_selfpowered: true when we are selfpowered
> * @three_stage_setup: set if we perform a three phase setup
> * @ep0_bounced: true when we used bounce buffer
> @@ -668,6 +670,10 @@ struct dwc3 {
> void __iomem *regs;
> size_t regs_size;
>
> + /* used for suspend/resume */
> + u32 dcfg;
> + u32 gctl;
> +
> u32 num_event_buffers;
> u32 u1u2;
> u32 maximum_speed;
> @@ -862,4 +868,31 @@ void dwc3_host_exit(struct dwc3 *dwc);
> int dwc3_gadget_init(struct dwc3 *dwc);
> void dwc3_gadget_exit(struct dwc3 *dwc);
>
> +/* power management interface */
> +#if !IS_ENABLED(CONFIG_USB_DWC3_HOST)
> +int dwc3_gadget_prepare(struct dwc3 *dwc);
> +void dwc3_gadget_complete(struct dwc3 *dwc);
> +int dwc3_gadget_suspend(struct dwc3 *dwc);
> +int dwc3_gadget_resume(struct dwc3 *dwc);
> +#else
> +static inline int dwc3_gadget_prepare(struct dwc3 *dwc)
> +{
> + return 0;
> +}
> +
> +static inline void dwc3_gadget_complete(struct dwc3 *dwc)
> +{
> +}
> +
> +static inline int dwc3_gadget_suspend(struct dwc3 *dwc)
> +{
> + return 0;
> +}
> +
> +static inline int dwc3_gadget_resume(struct dwc3 *dwc)
> +{
> + return 0;
> +}
> +#endif /* !IS_ENABLED(CONFIG_USB_DWC3_HOST) */
> +
> #endif /* __DRIVERS_USB_DWC3_CORE_H */
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 2656cca..83733ec 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2530,6 +2530,8 @@ err0:
> return ret;
> }
>
> +/* -------------------------------------------------------------------------- */
> +
> void dwc3_gadget_exit(struct dwc3 *dwc)
> {
> usb_del_gadget_udc(&dwc->gadget);
> @@ -2547,3 +2549,62 @@ void dwc3_gadget_exit(struct dwc3 *dwc)
> dma_free_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
> dwc->ctrl_req, dwc->ctrl_req_addr);
> }
> +
> +int dwc3_gadget_prepare(struct dwc3 *dwc)
> +{
> + if (dwc->pullups_connected)
> + dwc3_gadget_disable_irq(dwc);
> +
> + return 0;
> +}
> +
> +void dwc3_gadget_complete(struct dwc3 *dwc)
> +{
> + if (dwc->pullups_connected) {
> + dwc3_gadget_enable_irq(dwc);
> + dwc3_gadget_run_stop(dwc, true);
> + }
> +}
> +
> +int dwc3_gadget_suspend(struct dwc3 *dwc)
> +{
> + __dwc3_gadget_ep_disable(dwc->eps[0]);
> + __dwc3_gadget_ep_disable(dwc->eps[1]);
> +
> + dwc->dcfg = dwc3_readl(dwc->regs, DWC3_DCFG);
> +
> + return 0;
> +}
> +
> +int dwc3_gadget_resume(struct dwc3 *dwc)
> +{
> + struct dwc3_ep *dep;
> + int ret;
> +
> + /* Start with SuperSpeed Default */
> + dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
> +
> + dep = dwc->eps[0];
> + ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false);
> + if (ret)
> + goto err0;
> +
> + dep = dwc->eps[1];
> + ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false);
> + if (ret)
> + goto err1;
> +
> + /* begin to receive SETUP packets */
> + dwc->ep0state = EP0_SETUP_PHASE;
> + dwc3_ep0_out_start(dwc);
> +
> + dwc3_writel(dwc->regs, DWC3_DCFG, dwc->dcfg);
> +
> + return 0;
> +
> +err1:
> + __dwc3_gadget_ep_disable(dwc->eps[0]);
> +
> +err0:
> + return ret;
> +}
There's one more issue that i could notice.
While testing mass storage GADGET functionality, if we keep the target
board attached
to the Host system while suspending, a kernel warning comes while
handling interrupt
and port gets disabled untill next suspend (Note here, that this
problem continues untill we
unplug the target from the host and then suspend, thereafter plugging
it again after resume)
Please refer to attached file "dwc3_gadget_warning.txt" (showing dmesg
logs after resume).
However keeping the target board detached from the host while suspending
and then reconnecting it after resume, i could get the gadget back.
Any pointers, what could be going wrong ?
For suspend/resume in all above cases we are using following commands:
echo +20 > /sys/class/rtc/rtc0/wakealarm
echo mem > /sys/power/state
> --
--
Thanks & Regards
Vivek
[ 45.400000] s3c_pm_enter: post sleep, preparing to return
[ 45.400000] S3C PM Resume (post-restore)
[ 45.400000] restore f6100234 (restore 00000000, was 0000000f)
[ 45.400000] Enabling non-boot CPUs ...
[ 45.405000] CPU1: Booted secondary processor
[ 45.405000] CPU1 is up
[ 45.405000] PM: noirq resume of devices complete after 0.564 msecs
[ 45.410000] PM: early resume of devices complete after 0.592 msecs
[ 45.410000] s3c-rtc 101e0000.rtc: rtc disabled, re-enabling
[ 45.510000] samsung-usb2phy 12130000.usbphy: Already power on PHY
[ 45.510000] usb usb1: root hub lost power or was reset
[ 45.615000] samsung-usb2phy 12130000.usbphy: Already power on PHY
[ 45.670000] usb usb2: root hub lost power or was reset
[ 45.850000] PM: resume of devices complete after 439.778 msecs
[ 45.850000] ------------[ cut here ]------------
[ 45.850000] WARNING: at drivers/usb/dwc3/gadget.c:1712 dwc3_endpoint_transfer_complete.isra.20+0x278/0x284()
[ 45.850000] Modules linked in: g_mass_storage libcomposite
[ 45.850000] [<80014f54>] (unwind_backtrace+0x0/0xf8) from [<80021340>] (warn_slowpath_common+0x54/0x64)
[ 45.850000] [<80021340>] (warn_slowpath_common+0x54/0x64) from [<8002136c>] (warn_slowpath_null+0x1c/0x24)
[ 45.850000] [<8002136c>] (warn_slowpath_null+0x1c/0x24) from [<80257e5c>] (dwc3_endpoint_transfer_complete.isra.20+0x278/0x284)
[ 45.850000] [<80257e5c>] (dwc3_endpoint_transfer_complete.isra.20+0x278/0x284) from [<80259440>] (dwc3_interrupt+0x43c/0x4f0)
[ 45.850000] [<80259440>] (dwc3_interrupt+0x43c/0x4f0) from [<8006e2c4>] (handle_irq_event_percpu+0x54/0x194)
[ 45.850000] [<8006e2c4>] (handle_irq_event_percpu+0x54/0x194) from [<8006e440>] (handle_irq_event+0x3c/0x5c)
[ 45.850000] [<8006e440>] (handle_irq_event+0x3c/0x5c) from [<8007106c>] (handle_fasteoi_irq+0xa4/0x134)
[ 45.850000] [<8007106c>] (handle_fasteoi_irq+0xa4/0x134) from [<8006dae0>] (generic_handle_irq+0x30/0x40)
[ 45.850000] [<8006dae0>] (generic_handle_irq+0x30/0x40) from [<8000f55c>] (handle_IRQ+0x40/0x90)
[ 45.850000] [<8000f55c>] (handle_IRQ+0x40/0x90) from [<80008584>] (gic_handle_irq+0x38/0x68)
[ 45.850000] [<80008584>] (gic_handle_irq+0x38/0x68) from [<8000e240>] (__irq_svc+0x40/0x70)
[ 45.850000] Exception stack(0x81b29e80 to 0x81b29ec8)
[ 45.850000] 9e80: 00000000 00000000 00000000 0a6c0a6c a0000153 ef12cc10 804bfe94 81b29ed0
[ 45.850000] 9ea0: 804bfedc ef075f18 ef12cc44 00000001 ffffffff 81b29ec8 80347124 80347128
[ 45.850000] 9ec0: 60000153 ffffffff
[ 45.850000] [<8000e240>] (__irq_svc+0x40/0x70) from [<80347128>] (_raw_spin_unlock_irqrestore+0x10/0x38)
[ 45.850000] [<80347128>] (_raw_spin_unlock_irqrestore+0x10/0x38) from [<8021f708>] (dpm_complete+0xd4/0x1ac)
[ 45.850000] [<8021f708>] (dpm_complete+0xd4/0x1ac) from [<80057ca8>] (suspend_devices_and_enter+0x118/0x1e4)
[ 45.850000] [<80057ca8>] (suspend_devices_and_enter+0x118/0x1e4) from [<80057ef4>] (pm_suspend+0x180/0x1d8)
[ 45.850000] [<80057ef4>] (pm_suspend+0x180/0x1d8) from [<80057060>] (state_store+0x120/0x12c)
[ 45.850000] [<80057060>] (state_store+0x120/0x12c) from [<801b1b78>] (kobj_attr_store+0x14/0x20)
[ 45.850000] [<801b1b78>] (kobj_attr_store+0x14/0x20) from [<8010c514>] (sysfs_write_file+0xfc/0x17c)
[ 45.850000] [<8010c514>] (sysfs_write_file+0xfc/0x17c) from [<800b3f10>] (vfs_write+0xa8/0x138)
[ 45.850000] [<800b3f10>] (vfs_write+0xa8/0x138) from [<800b417c>] (sys_write+0x40/0x68)
[ 45.850000] [<800b417c>] (sys_write+0x40/0x68) from [<8000e640>] (ret_fast_syscall+0x0/0x30)
[ 45.850000] ---[ end trace b1f9fc3eb20e8e2d ]---
[ 46.060000] dwc3 dwc3.2.auto: ep1out-bulk's TRB (f0091000) still owned by HW
[ 46.925000] Restarting tasks ... done.
[ 11.170000] s3c_pm_enter: post sleep, preparing to return
[ 11.170000] S3C PM Resume (post-restore)
[ 11.170000] restore f6100234 (restore 00000000, was 0000000f)
[ 11.170000] Enabling non-boot CPUs ...
[ 11.180000] CPU1: Booted secondary processor
[ 11.180000] CPU1 is up
[ 11.180000] PM: noirq resume of devices complete after 0.570 msecs
[ 11.180000] PM: early resume of devices complete after 0.508 msecs
[ 11.180000] s3c-rtc 101e0000.rtc: rtc disabled, re-enabling
[ 11.185000] usb usb1: root hub lost power or was reset
[ 11.185000] usb usb2: root hub lost power or was reset
[ 11.185000] usb usb3: root hub lost power or was reset
[ 11.300000] PM: resume of devices complete after 116.868 msecs
[ 12.110000] Restarting tasks ... [ 12.115000] usb 1-1: USB disconnect, device number 2
done.
[ 12.120000] xHCI xhci_drop_endpoint called with unaddressed device
[ 12.125000] xHCI xhci_drop_endpoint called with unaddressed device
[ 12.135000] xHCI xhci_check_bandwidth called with unaddressed device
[ 12.140000] xHCI xhci_free_dev called with unaddressed device
[ 38.025000] s3c_pm_enter: post sleep, preparing to return
[ 38.025000] S3C PM Resume (post-restore)
[ 38.025000] restore f6100234 (restore 00000000, was 0000000f)
[ 38.025000] Enabling non-boot CPUs ...
[ 38.035000] CPU1: Booted secondary processor
[ 38.035000] CPU1 is up
[ 38.035000] PM: noirq resume of devices complete after 0.569 msecs
[ 38.035000] PM: early resume of devices complete after 0.512 msecs
[ 38.035000] s3c-rtc 101e0000.rtc: rtc disabled, re-enabling
[ 38.140000] usb usb1: root hub lost power or was reset
[ 38.140000] usb usb2: root hub lost power or was reset
[ 38.140000] samsung-usb2phy 12130000.usbphy: Already power on PHY
[ 38.140000] usb usb3: root hub lost power or was reset
[ 38.470000] usb 1-1: reset high-speed USB device number 4 using xhci-hcd
[ 38.485000] xhci-hcd xhci-hcd.3.auto: xHCI xhci_drop_endpoint called with disabled ep ef2f8100
[ 38.485000] xhci-hcd xhci-hcd.3.auto: xHCI xhci_drop_endpoint called with disabled ep ef2f812c
[ 38.485000] PM: resume of devices complete after 447.433 msecs
[ 39.325000] Restarting tasks ... done.
