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

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

 



Hi Felipe,

On Wed, Jan 21, 2015 at 11:06 AM, Bin Liu <binmlist@xxxxxxxxx> wrote:
> 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.

Do you think this patch should be reverted? Hub stops working on
AM335x because of it.

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