Messages in this thread | | | From | Jun Li <> | Subject | RE: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by device controller | Date | Thu, 21 May 2020 07:33:25 +0000 |
| |
Hi Thinh, > -----Original Message----- > From: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > Sent: 2020年5月21日 9:56 > To: Jun Li <jun.li@nxp.com>; Felipe Balbi <balbi@kernel.org>; Jun Li > <lijun.kernel@gmail.com> > Cc: John Stultz <john.stultz@linaro.org>; lkml <linux-kernel@vger.kernel.org>; Yu > Chen <chenyu56@huawei.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Rob > Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; ShuFan Lee > <shufan_lee@richtek.com>; Heikki Krogerus <heikki.krogerus@linux.intel.com>; > Suzuki K Poulose <suzuki.poulose@arm.com>; Chunfeng Yun > <chunfeng.yun@mediatek.com>; Hans de Goede <hdegoede@redhat.com>; Andy Shevchenko > <andy.shevchenko@gmail.com>; Valentin Schneider <valentin.schneider@arm.com>; > Jack Pham <jackp@codeaurora.org>; Linux USB List <linux-usb@vger.kernel.org>; open > list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; > Peter Chen <peter.chen@nxp.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@synopsys.com> > >>> Sent: 2020年5月19日 14:46 > >>> To: Jun Li <jun.li@nxp.com>; Felipe Balbi <balbi@kernel.org>; Jun Li > >>> <lijun.kernel@gmail.com> > >>> Cc: John Stultz <john.stultz@linaro.org>; lkml > >>> <linux-kernel@vger.kernel.org>; Yu Chen <chenyu56@huawei.com>; Greg > >>> Kroah-Hartman <gregkh@linuxfoundation.org>; Rob Herring > >>> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; ShuFan > >>> Lee <shufan_lee@richtek.com>; Heikki Krogerus > >>> <heikki.krogerus@linux.intel.com>; > >>> Suzuki K Poulose <suzuki.poulose@arm.com>; Chunfeng Yun > >>> <chunfeng.yun@mediatek.com>; Hans de Goede <hdegoede@redhat.com>; > >>> Andy Shevchenko <andy.shevchenko@gmail.com>; Valentin Schneider > >>> <valentin.schneider@arm.com>; Jack Pham <jackp@codeaurora.org>; > >>> Linux USB List <linux-usb@vger.kernel.org>; open list:OPEN FIRMWARE > >>> AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; > >>> Peter Chen <peter.chen@nxp.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@gmail.com> On Behalf Of Felipe Balbi > >>>>>> Sent: 2020年5月16日 19:57 > >>>>>> To: Jun Li <jun.li@nxp.com>; Thinh Nguyen > >>>>>> <Thinh.Nguyen@synopsys.com>; Jun Li <lijun.kernel@gmail.com> > >>>>>> Cc: John Stultz <john.stultz@linaro.org>; lkml > >>>>>> <linux-kernel@vger.kernel.org>; Yu Chen <chenyu56@huawei.com>; > >>>>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Rob Herring > >>>>>> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; ShuFan > >>>>>> Lee <shufan_lee@richtek.com>; Heikki Krogerus > >>>>>> <heikki.krogerus@linux.intel.com>; > >>>>>> Suzuki K Poulose <suzuki.poulose@arm.com>; Chunfeng Yun > >>>>>> <chunfeng.yun@mediatek.com>; Hans de Goede <hdegoede@redhat.com>; > >>>>>> Andy Shevchenko <andy.shevchenko@gmail.com>; Valentin Schneider > >>>>>> <valentin.schneider@arm.com>; Jack Pham <jackp@codeaurora.org>; > >>>>>> Linux USB List <linux-usb@vger.kernel.org>; open list:OPEN > >>>>>> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS > >>>>>> <devicetree@vger.kernel.org>; Peter Chen <peter.chen@nxp.com>; > >>>>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> > >>>>>> Subject: RE: [PATCH v4 3/9] usb: dwc3: Increase timeout for > >>>>>> CmdAct cleared by device controller > >>>>>> > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> Jun Li <jun.li@nxp.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
| |