Message-ID: <VE1PR04MB652863A8406D25EEF2CE483389B70@VE1PR04MB6528.eurprd04.prod.outlook.com>
Date:   Thu, 21 May 2020 07:33:25 +0000
From:   Jun Li <jun.li@....com>
To:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Felipe Balbi <balbi@...nel.org>,
        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>
Subject: RE: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by
 device controller

Hi Thinh,
> -----Original Message-----
> From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
> Sent: 2020年5月21日 9:56
> To: Jun Li <jun.li@....com>; Felipe Balbi <balbi@...nel.org>; 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>
> Subject: Re: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by device
> controller
> 
> Thinh Nguyen wrote:
> > Jun Li wrote:
> >> Hi
> >>
> >>> -----Original Message-----
> >>> From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
> >>> Sent: 2020年5月19日 14:46
> >>> To: Jun Li <jun.li@....com>; Felipe Balbi <balbi@...nel.org>; 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>
> >>> Subject: Re: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct
> >>> cleared by device controller
> >>>
> >>> Thinh Nguyen wrote:
> >>>> Jun Li wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Felipe Balbi <balbif@...il.com> On Behalf Of Felipe Balbi
> >>>>>> Sent: 2020年5月16日 19:57
> >>>>>> To: Jun Li <jun.li@....com>; 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,
> >>>>>>
> >>>>>> Jun Li <jun.li@....com> writes:
> >>>>>>>>>> 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"
> >>>>>> Cool, now do we have an upper bound for how many clock cycles it
> >>>>>> takes to wake up the PHY?
> >>>>> My understanding is this ep command does not wake up the SS PHY,
> >>>>> the SS PHY still stays at P3 when execute this ep command. The
> >>>>> time required here is to wait controller complete something for
> >>>>> this ep command with 32K clock.
> >>>> Sorry I made a mistake. You're right. Just checked with one of the
> >>>> RTL engineers, and it doesn't need to wake up the phy. However, if
> >>>> it is eSS speed, it may take longer time as the command may be
> >>>> completing with the suspend clock.
> >>>>
> >>> What's the value for GCTL[7:6]?
> >> 2'b00
> >>
> >> Thanks
> >> Li Jun
> > (Sorry for the delay reply)
> >
> > If it's 0, then the ram clock should be the same as the bus_clk, which
> > is odd since you mentioned that the suspend_clk is used instead while in P3.
> 
> Just checked with the RTL engineer, even if GCTL[7:6] is set to 0, internally it
> can still run with suspend clock during P3.

Thanks for your check.
> 
> > Anyway, I was looking for a way maybe to improve the speed during
> > issuing a command. One way is to set GUSB3PIPECTL[17]=0, and it should
> > wakeup the phy anytime. I think Felipe suggested it. It's odd that it
> > doesn't work for you. I don't have other ideas beside increasing the
> > command timeout.
> >
> 
> In any case, increasing the timeout should be fine with me. It maybe difficult to
> determine the max timeout base on the slowest clock rate and number of cycles.
> Different controller and controller versions behave differently and may have
> different number of clock cycles to complete a command.
> 
> The RTL engineer recommended timeout to be at least 1ms (which maybe more than the
> polling rate of this patch). I'm fine with either the rate provided by this tested
> patch or higher.

OK, I will change the timeout to be 1ms if no object from Felipe.

thanks
Li Jun
> 
> BR,
> Thinh

Powered by blists - more mailing lists