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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 16 May 2020 09:20:43 +0000
From:   Jun Li <jun.li@....com>
To:     Felipe Balbi <balbi@...nel.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Jun Li <lijun.kernel@...il.com>
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 <peter.chen@....com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Subject: RE: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by
 device controller

Hi,
> -----Original Message-----
> From: Felipe Balbi <balbif@...il.com> On Behalf Of Felipe Balbi
> Sent: 2020年5月16日 15:13
> To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>; Jun Li <jun.li@....com>; Jun Li
> <lijun.kernel@...il.com>
> 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 <peter.chen@....com>; Thinh Nguyen <Thinh.Nguyen@...opsys.com>
> Subject: Re: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by device
> controller
> 
> 
> Hi,
> 
> Thinh Nguyen <Thinh.Nguyen@...opsys.com> writes:
> > Jun Li wrote:
> >>> -----Original Message-----
> >>> From: Felipe Balbi <balbif@...il.com> On Behalf Of Felipe Balbi
> >>> Sent: 2020年5月15日 17:31
> >>> To: Jun Li <lijun.kernel@...il.com>
> >>> 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 <peter.chen@....com>; Jun Li <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,
> >>>
> >>> Jun Li <lijun.kernel@...il.com> writes:
> >>>>> @@ -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).
> >>>
> >>> It sounds like you have a quirky PHY.
> >>  From what I got from the IC design, the behavior of
> >> DWC3_GUSB3PIPECTL_SUSPHY bit should be as what I said, not a quirky.
> >>
> >> Hi Thinh, could you comment this?
> >
> > You only need to wake up the usb2 phy when issuing the command while
> > running in highspeed or below. If you're running in SS or higher,
> > internally the controller does it for you for usb3 phy. In Jun's case,
> > it seems like it takes longer for his phy to wake up.
> >
> > IMO, in this case, I think it's fine to increase the command timeout.
> 
> Is there an upper limit to this? Is 32k clock the slowest that can be fed to the
> PHY as a suspend clock?

Yes, 32K clock is the slowest, Per DWC3 document on Power Down Scale
(bits 31:19 of GCTL):

"Power Down Scale (PwrDnScale)
The USB3 suspend_clk input replaces pipe3_rx_pclk as a clock source
to a small part of the USB3 controller that operates when the SS PHY
is in its lowest power (P3) state, and therefore does not provide a clock.
The Power Down Scale field specifies how many suspend_clk periods
fit into a 16 kHz clock period. When performing the division, round up
the remainder.
For example, when using an 8-bit/16-bit/32-bit PHY and 25-MHz Suspend clock,
Power Down Scale = 25000 kHz/16 kHz = 13'd1563 (rounder up)
Note:
- Minimum Suspend clock frequency is 32 kHz
- Maximum Suspend clock frequency is 125 MHz"

Li Jun
> 
> --
> balbi