From patchwork Thu Dec 10 08:50:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Serge Semin X-Patchwork-Id: 11963781 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B06D6C433FE for ; Thu, 10 Dec 2020 08:57:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 74A2223BE5 for ; Thu, 10 Dec 2020 08:57:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388329AbgLJIvb (ORCPT ); Thu, 10 Dec 2020 03:51:31 -0500 Received: from mx.baikalelectronics.ru ([94.125.187.42]:36032 "EHLO mail.baikalelectronics.ru" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2388304AbgLJIvL (ORCPT ); Thu, 10 Dec 2020 03:51:11 -0500 From: Serge Semin To: Felipe Balbi , John Youn , Greg Kroah-Hartman , Felipe Balbi , David Cohen , Heikki Krogerus CC: Serge Semin , Serge Semin , Alexey Malahov , Pavel Parkhomenko , , Subject: [PATCH v5 3/3] usb: dwc3: ulpi: Fix USB2.0 HS/FS/LS PHY suspend regression Date: Thu, 10 Dec 2020 11:50:08 +0300 Message-ID: <20201210085008.13264-4-Sergey.Semin@baikalelectronics.ru> In-Reply-To: <20201210085008.13264-1-Sergey.Semin@baikalelectronics.ru> References: <20201210085008.13264-1-Sergey.Semin@baikalelectronics.ru> MIME-Version: 1.0 X-ClientProxiedBy: MAIL.baikal.int (192.168.51.25) To mail (192.168.51.25) Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org First of all the commit e0082698b689 ("usb: dwc3: ulpi: conditionally resume ULPI PHY") introduced the Suspend USB2.0 HS/FS/LS PHY regression, as by design of the fix any attempt to read/write from/to the PHY control registers will completely disable the PHY suspension, which consequently will increase the USB bus power consumption. Secondly the fix won't work well for the very first attempt of the ULPI PHY control registers IO, because after disabling the USB2.0 PHY suspension functionality it will still take some time for the bus to resume from the sleep state if one has been reached before it. So the very first PHY register read/write operation will take more time than the busy-loop provides and the IO timeout error might be returned anyway. Here we suggest to fix the denoted problems in the following way. First of all let's not disable the Suspend USB2.0 HS/FS/LS PHY functionality so to make the controller and the USB2.0 bus more power efficient. Secondly instead of that we'll extend the PHY IO op wait procedure with 1 - 1.2 ms sleep if the PHY suspension is enabled (1ms should be enough as by LPM specification it is at most how long it takes for the USB2.0 bus to resume from L1 (Sleep) state). Finally in case if the USB2.0 PHY suspension functionality has been disabled on the DWC USB3 controller setup procedure we'll compensate the USB bus resume process latency by extending the busy-loop attempts counter. Fixes: e0082698b689 ("usb: dwc3: ulpi: conditionally resume ULPI PHY") Signed-off-by: Serge Semin Acked-by: Heikki Krogerus --- drivers/usb/dwc3/ulpi.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c index 54c877f7b51d..f23f4c9a557e 100644 --- a/drivers/usb/dwc3/ulpi.c +++ b/drivers/usb/dwc3/ulpi.c @@ -24,7 +24,7 @@ static int dwc3_ulpi_busyloop(struct dwc3 *dwc, u8 addr, bool read) { unsigned long ns = 5L * DWC3_ULPI_BASE_DELAY; - unsigned int count = 1000; + unsigned int count = 10000; u32 reg; if (addr >= ULPI_EXT_VENDOR_SPECIFIC) @@ -33,6 +33,10 @@ static int dwc3_ulpi_busyloop(struct dwc3 *dwc, u8 addr, bool read) if (read) ns += DWC3_ULPI_BASE_DELAY; + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)); + if (reg & DWC3_GUSB2PHYCFG_SUSPHY) + usleep_range(1000, 1200); + while (count--) { ndelay(ns); reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0)); @@ -50,12 +54,6 @@ static int dwc3_ulpi_read(struct device *dev, u8 addr) u32 reg; int ret; - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)); - if (reg & DWC3_GUSB2PHYCFG_SUSPHY) { - reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); - } - reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr); dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg); @@ -73,12 +71,6 @@ static int dwc3_ulpi_write(struct device *dev, u8 addr, u8 val) struct dwc3 *dwc = dev_get_drvdata(dev); u32 reg; - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)); - if (reg & DWC3_GUSB2PHYCFG_SUSPHY) { - reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); - } - reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr); reg |= DWC3_GUSB2PHYACC_WRITE | val; dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);