Re: [PATCH 2/2] usb: musb: try a race-free wakeup

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

 



Hi Sebastian,

On Mon, Oct 27, 2014 at 1:06 PM, Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> wrote:
> Attaching a keyboard, using it as a wakeup via
> |for f in $(find /sys/devices/ocp.3/47400000.usb -name wakeup)
> |do
> |       echo enabled > $f
> |done
>
> going into standby
> |  echo standby >  /sys/power/state
>
> and now a wake up by a pressing a key.
> What happens is that the system wakes up but the USB device is dead. The
> USB stack tries to send a few control URBs but nothing comes back.
> Eventually it gaves up and the device remains dead:
> |[  632.559678] PM: Wakeup source USB1_PHY
> |[  632.581074] PM: noirq resume of devices complete after 21.261 msecs
> |[  632.607521] PM: early resume of devices complete after 10.360 msecs
> |[  632.616854] net eth2: initializing cpsw version 1.12 (0)
> |[  632.704126] net eth2: phy found : id is : 0x4dd074
> |[  636.704048] libphy: 4a101000.mdio:00 - Link is Up - 1000/Full
> |[  638.444620] usb 1-1: reset low-speed USB device number 2 using musb-hdrc
> |[  653.713435] usb 1-1: device descriptor read/64, error -110
> |[  669.093435] usb 1-1: device descriptor read/64, error -110
> |[  669.473424] usb 1-1: reset low-speed USB device number 2 using musb-hdrc
> |[  684.743436] usb 1-1: device descriptor read/64, error -110
> |[  690.065097] PM: resume of devices complete after 57450.744 msecs
> |[  690.076601] PM: Finishing wakeup.
> |[  690.076627] Restarting tasks ...
>
> It seems that since we got woken up via MUSB_INTR_RESUME the
> musb_host_finish_resume() callback is executed before the
> resume-callbacks of the PHY and glue layer are invoked. If I delay it
> until the glue layer resumed then I don't see this problem.
>
> I also move musb_host_resume_root_hub() into that callback since I don't
> see any reason in doing anything resume-link if there are still pieces
> not restored.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>

With this patch, hubs stop working on TI AM335x EVMs when autosuspend
is enabled.

I booted the board, connected a hub to the USB1 host port, it got
enumerated properly, then connected a thumb drive to the hub, but the
drive was not enumerated, no any log from kernel. Removing the drive,
no any kernel message either. Finally removed the hub, no disconnect
for the hub. Now check MUSB1 DEVCTL register, it value is 0x5D.

Reverted this patch, the issue disappeared. Or disable usbcore
autosuspend, the issue did not happen.

I tested 7+ hubs, all have the same issue.

I tested on git://git.ti.com/ti-linux-kernel/ti-linux-kernel.git, tag
ti2014.10.01. have not tested with mainline kernel yet.

Have you validated this test case?

Thanks,
-Bin.

> ---
>  drivers/usb/musb/musb_core.c    | 10 ++++++----
>  drivers/usb/musb/musb_core.h    |  1 +
>  drivers/usb/musb/musb_virthub.c |  1 +
>  3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 749360d63bbe..2ff1b37a46a6 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -478,13 +478,10 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
>                                                 | MUSB_PORT_STAT_RESUME;
>                                 musb->rh_timer = jiffies
>                                                  + msecs_to_jiffies(20);
> -                               schedule_delayed_work(
> -                                       &musb->finish_resume_work,
> -                                       msecs_to_jiffies(20));
> +                               musb->need_finish_resume = 1;
>
>                                 musb->xceiv->state = OTG_STATE_A_HOST;
>                                 musb->is_active = 1;
> -                               musb_host_resume_root_hub(musb);
>                                 break;
>                         case OTG_STATE_B_WAIT_ACON:
>                                 musb->xceiv->state = OTG_STATE_B_PERIPHERAL;
> @@ -2319,6 +2316,11 @@ static int musb_resume(struct device *dev)
>         mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV;
>         if ((devctl & mask) != (musb->context.devctl & mask))
>                 musb->port1_status = 0;
> +       if (musb->need_finish_resume) {
> +               musb->need_finish_resume = 0;
> +               schedule_delayed_work(&musb->finish_resume_work,
> +                                     msecs_to_jiffies(20));
> +       }
>         return 0;
>  }
>
> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
> index 414e57a984bb..803a997e56d2 100644
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -390,6 +390,7 @@ struct musb {
>
>         /* is_suspended means USB B_PERIPHERAL suspend */
>         unsigned                is_suspended:1;
> +       unsigned                need_finish_resume :1;
>
>         /* may_wakeup means remote wakeup is enabled */
>         unsigned                may_wakeup:1;
> diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c
> index e2d2d8c9891b..66d2996e9ed0 100644
> --- a/drivers/usb/musb/musb_virthub.c
> +++ b/drivers/usb/musb/musb_virthub.c
> @@ -72,6 +72,7 @@ void musb_host_finish_resume(struct work_struct *work)
>         musb->xceiv->state = OTG_STATE_A_HOST;
>
>         spin_unlock_irqrestore(&musb->lock, flags);
> +       musb_host_resume_root_hub(musb);
>  }
>
>  void musb_port_suspend(struct musb *musb, bool do_suspend)
> --
> 2.1.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux