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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKgpwJXfWv5=MDqBCADhe2iXf6eiP0GQ13Bwo9fkuU5kGO7dsw@mail.gmail.com>
Date:   Sat, 9 May 2020 16:10:19 +0800
From:   Jun Li <lijun.kernel@...il.com>
To:     Felipe Balbi <balbi@...nel.org>
Cc:     John Stultz <john.stultz@...aro.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Yu Chen <chenyu56@...wei.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        ShuFan Lee <shufan_lee@...htek.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Chunfeng Yun <chunfeng.yun@...iatek.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Valentin Schneider <valentin.schneider@....com>,
        Jack Pham <jackp@...eaurora.org>,
        Linux USB List <linux-usb@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, peter.chen@....com,
        Li Jun <jun.li@....com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Subject: Re: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by
 device controller

Hi,

Felipe Balbi <balbi@...nel.org> 于2020年5月8日周五 下午8:33写道：
>
>
> Hi,
>
> Jun Li <lijun.kernel@...il.com> writes:
> > John Stultz <john.stultz@...aro.org> 于2019年10月30日周三 上午5:18写道：
> >>
> >> On Tue, Oct 29, 2019 at 2:11 AM Felipe Balbi <balbi@...nel.org> wrote:
> >> > John Stultz <john.stultz@...aro.org> writes:
> >> > > From: Yu Chen <chenyu56@...wei.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;
>  }

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