Re: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by device controller

From: Felipe Balbi <balbi@kernel.org>
To: Jun Li <lijun.kernel@gmail.com>, John Stultz <john.stultz@linaro.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	Yu Chen <chenyu56@huawei.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	ShuFan Lee <shufan_lee@richtek.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Chunfeng Yun <chunfeng.yun@mediatek.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Valentin Schneider <valentin.schneider@arm.com>,
	Jack Pham <jackp@codeaurora.org>,
	Linux USB List <linux-usb@vger.kernel.org>,
	"open list\:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>
Subject: Re: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by device controller
Date: Fri, 08 May 2020 15:33:24 +0300	[thread overview]
Message-ID: <87h7wqmwrv.fsf@kernel.org> (raw)
In-Reply-To: <CAKgpwJVaKpsgMjKcnYyJsfNj0ibkPt=mdn-NxfOkeX1jfL=9iQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3160 bytes --]


Hi,

Jun Li <lijun.kernel@gmail.com> writes:
> John Stultz <john.stultz@linaro.org> 于2019年10月30日周三 上午5:18写道：
>>
>> On Tue, Oct 29, 2019 at 2:11 AM Felipe Balbi <balbi@kernel.org> wrote:
>> > John Stultz <john.stultz@linaro.org> writes:
>> > > From: Yu Chen <chenyu56@huawei.com>
>> > >
>> > > It needs more time for the device controller to clear the CmdAct of
>> > > DEPCMD on Hisilicon Kirin Soc.
>> >
>> > Why does it need more time? Why is it so that no other platform needs
>> > more time, only this one? And which command, specifically, causes
>> > problem?
>
> Sorry for my back to this so late.
>
> This change is required on my dwc3 based HW too, I gave a check
> and the reason is suspend_clk is used in case the PIPE phy is at P3,
> this slow clock makes my EP command below timeout.

The phy needs to woken up before the command is triggered. Currently we
only wake up the HS PHY. Does it help you if we wake up the SS phy as
well?

Something like below ought to do it:

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a0555252dee0..ee46c2dacaeb 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -274,7 +274,8 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd,
 	const struct usb_endpoint_descriptor *desc = dep->endpoint.desc;
 	struct dwc3		*dwc = dep->dwc;
 	u32			timeout = 1000;
-	u32			saved_config = 0;
+	u32			saved_hs_config = 0;
+	u32			saved_ss_config = 0;
 	u32			reg;
 
 	int			cmd_status = 0;
@@ -293,19 +294,28 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd,
 	if (dwc->gadget.speed <= USB_SPEED_HIGH) {
 		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
 		if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
-			saved_config |= DWC3_GUSB2PHYCFG_SUSPHY;
+			saved_hs_config |= DWC3_GUSB2PHYCFG_SUSPHY;
 			reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
 		}
 
 		if (reg & DWC3_GUSB2PHYCFG_ENBLSLPM) {
-			saved_config |= DWC3_GUSB2PHYCFG_ENBLSLPM;
+			saved_hs_config |= DWC3_GUSB2PHYCFG_ENBLSLPM;
 			reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
 		}
 
-		if (saved_config)
+		if (saved_hs_config)
 			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 	}
 
+	reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
+	if (unlikely(reg & DWC3_GUSB3PIPECTL_SUSPHY)) {
+		saved_ss_config |= DWC3_GUSB3PIPECTL_SUSPHY;
+		reg &= ~DWC3_GUSB3PIPECTL_SUSPHY;
+	}
+
+	if (saved_ss_config)
+		dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+
 	if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) {
 		int		needs_wakeup;
 
@@ -397,12 +407,18 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd,
 			dwc3_gadget_ep_get_transfer_index(dep);
 	}
 
-	if (saved_config) {
+	if (saved_hs_config) {
 		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
-		reg |= saved_config;
+		reg |= saved_hs_config;
 		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 	}
 
+	if (saved_ss_config) {
+		reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
+		reg |= saved_ss_config;
+		dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+	}
+
 	return ret;
 }
 
-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
