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

[Felipe Balbi]

Hi,

Jun Li <lijun.kernel@xxxxxxxxx> writes:
> John Stultz <john.stultz@xxxxxxxxxx> ä2019å10æ30æåä äå5:18åéï
>>
>> On Tue, Oct 29, 2019 at 2:11 AM Felipe Balbi <balbi@xxxxxxxxxx> wrote:
>> > John Stultz <john.stultz@xxxxxxxxxx> writes:
>> > > From: Yu Chen <chenyu56@xxxxxxxxxx>
>> > >
>> > > 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: signature.asc
Description: PGP signature