Hi, On Thu, Apr 11, 2019 at 6:43 PM elaine.zhang <zhangq...@rock-chips.com> wrote: > >>> - "pmu_hclk_otg0", > >>> > >>> It's a soc bug, pmu_hclk_otg0 must always on. > >>> > >>> So you said in your previous commit message. However we've shipped > >>> lots and lots of Chromebooks with this clock off. Can you explain > >>> what is broken? Is this only needed for gadget mode (which we don't > >>> use), for instance? > >>> > >>> test case: > >>> > >>> recovery test, < 1 hour , system crash. > >>> > >>> log: > >>> > >>> [ 127.569629] I[0: swapper/0: 0] GOTGCTL > >>> @0xFFFFFF8000B80000 : 0x00400010 > >>> [ 127.569644] I[0: swapper/0: 0] GOTGINT > >>> @0xFFFFFF8000B80004 : 0x00400010 > >>> [ 127.569659] I[0: swapper/0: 0] GAHBCFG > >>> @0xFFFFFF8000B80008 : 0x00400010 > >>> [ 127.569673] I[0: swapper/0: 0] GUSBCFG > >>> @0xFFFFFF8000B8000C : 0x00400010 > >>> [ 127.569688] I[0: swapper/0: 0] GRSTCTL > >>> @0xFFFFFF8000B80010 : 0x00400010 > >>> [ 127.569702] I[0: swapper/0: 0] GINTSTS > >>> @0xFFFFFF8000B80014 : 0x00400010 > >>> [ 127.569718] I[0: swapper/0: 0] GINTMSK > >>> @0xFFFFFF8000B80018 : 0x00400010 > >>> [ 127.569733] I[0: swapper/0: 0] GRXSTSR > >>> @0xFFFFFF8000B8001C : 0x00400010 > >>> [ 127.569748] I[0: swapper/0: 0] GRXFSIZ > >>> @0xFFFFFF8000B80024 : 0x00400010 > >> I don't know what a "recovery test" is and I don't understand your logs. > >> > >> Can you explain we do not run into this on Chromebooks? > >> > >> > >>> reason: > >>> > >>> USB OTG controller supports turning off most logic power, and then only > >>> one PMU module is left. This clock cannot be turned off, which is similar > >>> to the always on module in USB OTG. > >> Can't you just add a patch to the dwc2 driver to have it grab this > >> clock? I assume this clock doesn't need to be turned on unless you're > >> using the OTG contoller in a certain way? > > So far we don't really know where the clock in question is sitting > > in the clock hirarchy. For example the kernel got a new interconnect > > framework recently, so handling non-device clocks in a device may haunt > > us later on. > > > > @Elaine: could you elaborate what pmu_hclk_otg0 actually is for please? > > Doug: > > Recovry test: Regular factory tests, including restart, adb debugging, > clear data/factory Settings, and clear cache. > I'm not clear whether the test was added by chromebooks. > > Heiko: > > pmu_hclk_otg0: pmu ahb clock > > Function: Clock to pmu module when hibernation and/or ADP is > enabled.Must be greater than or equal to 30 MHz.
Does this mean we can enable hibernation in dwc2 once we turn this clock on? I think right now hibernation doesn't work for dwc2 on rk3288. Not that I know a ton about dwc2's hibernation modes, but I've certainly bumped up against it when enabling power down modes. In fact I'm planning to post some patches soon... I'll CC you. > If the SOC design does not support hibernation/ADP function, only have > hclk_otg, this clk can be switched according to the usage of otg. > If the SOC design support hibernation/ADP, has two clocks, hclk_otg and > pmu_hclk_otg0. > Hclk_otg belongs to the closed part of otg logic, which can be switched > according to the use of otg. > > pmu_hclk_otg0 belongs to the always on part. > > As for whether pmu_hclk_otg0 can be turned off when otg is not in use, > we have not tested. IC suggest make pmu_hclk_otg0 always on. OK. I'm fine with this clock staying as a critical clock for now. I'll send out a v2 shortly. -Doug