From patchwork Fri Apr 27 13:00:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Daniel_Gl=C3=B6ckner?= X-Patchwork-Id: 10368513 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A2A8A601D3 for ; Fri, 27 Apr 2018 13:07:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 90B1627F89 for ; Fri, 27 Apr 2018 13:07:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 83D1528471; Fri, 27 Apr 2018 13:07:40 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A0F9A27F89 for ; Fri, 27 Apr 2018 13:07:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758116AbeD0NHi (ORCPT ); Fri, 27 Apr 2018 09:07:38 -0400 Received: from mx1.emlix.com ([46.4.235.150]:35822 "EHLO mx1.emlix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757982AbeD0NHh (ORCPT ); Fri, 27 Apr 2018 09:07:37 -0400 X-Greylist: delayed 416 seconds by postgrey-1.27 at vger.kernel.org; Fri, 27 Apr 2018 09:07:37 EDT Received: from mailer.emlix.com (unknown [81.20.119.6]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mx1.emlix.com (Postfix) with ESMTPS id 606C325F8F6; Fri, 27 Apr 2018 15:00:40 +0200 (CEST) Received: by mailer.emlix.com id 1fC2zW-0001wd-BX; Fri, 27 Apr 2018 15:00:46 +0200 Received: by devpool05.emlix.com (Postfix, from userid 2059) id DFE7935D13; Fri, 27 Apr 2018 15:00:39 +0200 (CEST) From: =?UTF-8?q?Daniel=20Gl=C3=B6ckner?= To: linux-usb@vger.kernel.org Cc: Bin Liu , Greg Kroah-Hartman , =?UTF-8?q?Daniel=20Gl=C3=B6ckner?= Subject: [PATCH] musb: fix remote wakeup racing with suspend Date: Fri, 27 Apr 2018 15:00:05 +0200 Message-Id: <20180427130005.6711-1-dg@emlix.com> MIME-Version: 1.0 Organization: emlix gmbh, Goettingen, Germany Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP It has been observed that writing 0xF2 to the power register while it reads as 0xF4 results in the register having the value 0xF0, i.e. clearing RESUME and setting SUSPENDM in one go does not work. It might also violate the USB spec to transition directly from resume to suspend, especially when not taking T_DRSMDN into account. But this is what happens when a remote wakeup occurs between SetPortFeature USB_PORT_FEAT_SUSPEND on the root hub and musb_bus_suspend being called. This commit returns -EBUSY when musb_bus_suspend is called while remote wakeup is signalled and thus avoids to reset the RESUME bit. Remember that resume can be signalled only when the bus was already suspended, so it is safe to skip the following steps even when musb_hub_control ignores the error returned by musb_port_suspend. The resume part of musb_port_suspend is modified to also accept a pending remote wakeup, to bring it to the end after T_DRSMDN has passed. Signed-off-by: Daniel Glöckner --- drivers/usb/musb/musb_host.c | 5 ++++- drivers/usb/musb/musb_host.h | 7 +++++-- drivers/usb/musb/musb_virthub.c | 27 ++++++++++++++++----------- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index 3a8451a..d05cb03 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -2522,8 +2522,11 @@ static int musb_bus_suspend(struct usb_hcd *hcd) { struct musb *musb = hcd_to_musb(hcd); u8 devctl; + int ret; - musb_port_suspend(musb, true); + ret = musb_port_suspend(musb, true); + if (ret) + return ret; if (!is_host_active(musb)) return 0; diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h index 72392bb..2999845 100644 --- a/drivers/usb/musb/musb_host.h +++ b/drivers/usb/musb/musb_host.h @@ -67,7 +67,7 @@ extern void musb_host_rx(struct musb *, u8); extern void musb_root_disconnect(struct musb *musb); extern void musb_host_resume_root_hub(struct musb *musb); extern void musb_host_poke_root_hub(struct musb *musb); -extern void musb_port_suspend(struct musb *musb, bool do_suspend); +extern int musb_port_suspend(struct musb *musb, bool do_suspend); extern void musb_port_reset(struct musb *musb, bool do_reset); extern void musb_host_finish_resume(struct work_struct *work); #else @@ -99,7 +99,10 @@ static inline void musb_root_disconnect(struct musb *musb) {} static inline void musb_host_resume_root_hub(struct musb *musb) {} static inline void musb_host_poll_rh_status(struct musb *musb) {} static inline void musb_host_poke_root_hub(struct musb *musb) {} -static inline void musb_port_suspend(struct musb *musb, bool do_suspend) {} +static inline int musb_port_suspend(struct musb *musb, bool do_suspend) +{ + return 0; +} static inline void musb_port_reset(struct musb *musb, bool do_reset) {} static inline void musb_host_finish_resume(struct work_struct *work) {} #endif diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c index 5165d2b..41b44ce 100644 --- a/drivers/usb/musb/musb_virthub.c +++ b/drivers/usb/musb/musb_virthub.c @@ -48,14 +48,14 @@ void musb_host_finish_resume(struct work_struct *work) spin_unlock_irqrestore(&musb->lock, flags); } -void musb_port_suspend(struct musb *musb, bool do_suspend) +int musb_port_suspend(struct musb *musb, bool do_suspend) { struct usb_otg *otg = musb->xceiv->otg; u8 power; void __iomem *mbase = musb->mregs; if (!is_host_active(musb)) - return; + return 0; /* NOTE: this doesn't necessarily put PHY into low power mode, * turning off its clock; that's a function of PHY integration and @@ -66,16 +66,20 @@ void musb_port_suspend(struct musb *musb, bool do_suspend) if (do_suspend) { int retries = 10000; - power &= ~MUSB_POWER_RESUME; - power |= MUSB_POWER_SUSPENDM; - musb_writeb(mbase, MUSB_POWER, power); + if (power & MUSB_POWER_RESUME) + return -EBUSY; - /* Needed for OPT A tests */ - power = musb_readb(mbase, MUSB_POWER); - while (power & MUSB_POWER_SUSPENDM) { + if (!(power & MUSB_POWER_SUSPENDM)) { + power |= MUSB_POWER_SUSPENDM; + musb_writeb(mbase, MUSB_POWER, power); + + /* Needed for OPT A tests */ power = musb_readb(mbase, MUSB_POWER); - if (retries-- < 1) - break; + while (power & MUSB_POWER_SUSPENDM) { + power = musb_readb(mbase, MUSB_POWER); + if (retries-- < 1) + break; + } } musb_dbg(musb, "Root port suspended, power %02x", power); @@ -100,7 +104,7 @@ void musb_port_suspend(struct musb *musb, bool do_suspend) musb_dbg(musb, "bogus rh suspend? %s", usb_otg_state_string(musb->xceiv->otg->state)); } - } else if (power & MUSB_POWER_SUSPENDM) { + } else if (power & (MUSB_POWER_SUSPENDM | MUSB_POWER_RESUME)) { power &= ~MUSB_POWER_SUSPENDM; power |= MUSB_POWER_RESUME; musb_writeb(mbase, MUSB_POWER, power); @@ -111,6 +115,7 @@ void musb_port_suspend(struct musb *musb, bool do_suspend) schedule_delayed_work(&musb->finish_resume_work, msecs_to_jiffies(USB_RESUME_TIMEOUT)); } + return 0; } void musb_port_reset(struct musb *musb, bool do_reset)