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

[Jun Li]

Hi,

Felipe Balbi <balbi@xxxxxxxxxx> ä2020å5æ8æåä äå8:33åéï
>
>
> 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;
>  }

Unfortunately this way can't work, once the SS PHY enters P3, disable
suspend_en can't force SS PHY exit P3, unless do this at the very beginning
to prevent SS PHY entering P3(e.g. add "snps,dis_u3_susphy_quirk" for test).

thanks
Li Jun
>
> --
> balbi