Hi Frank, Am Dienstag, 21. Juni 2016, 15:52:45 schrieb Frank Wang: > On 2016/6/20 12:56, Guenter Roeck wrote: > > On Sun, Jun 19, 2016 at 8:32 PM, Frank Wang <frank.wang@xxxxxxxxxxxxxx> wrote: > >>> Turns out my problem was one of terminology. Using "suspend" and > >>> "resume" to me suggested the common use of suspend and resume > >>> functions. That is not the case here. After mentally replacing > >>> "suspend" with "power_off" and "resume" with "power_on", you are > >>> right, no problem exists. Sorry for the noise. > >>> > >>> Maybe it would be useful to replace "resume" with "power_on" and > >>> "suspend" with "power_off" in the function and variable names to > >>> reduce confusion and misunderstandings. > >>> > >>> Thanks, > >>> Guenter > >> > >> Well, it does have a bits confusion, however, the phy-port always just > >> goes > >> to suspend and resume mode (Not power off and power on) in a fact. So > >> must > >> it be renamed? > > > > Other phy drivers name the functions _power_off and _power_on and > > avoid the confusion. The callbacks are named .power_off and .power_on, > > which gives a clear indication of its intended purpose. Other drivers > > implementing suspend/resume (such as the omap usb phy driver) tie > > those functions not into the power_off/power_on callbacks, but into > > the driver's suspend/resume callbacks. At least the omap driver has > > separate power management functions. > > > > Do the functions _have_ to be renamed ? Surely not. But, if the > > functions are really suspend/resume functions and not > > power_off/power_on functions, maybe they should tie to the > > suspend/resume functions and not register themselves as > > power_off/power_on functions ? > > As Guenter mentioned above, I doped out two solutions, one is that keep > current process but renaming *_resume/*_suspend to > *_power_on/*_power_off; in a way, naming stuff "power_off", "power_on" actually matches. For one, the phy-block goes from unusable to usable by usb-devices and also will power-on a phy-supply regulator (often named vcc_host* on Rockchip boards) from the phy- core. > another is that do not assign power_on/power_off > functions for phy_ops at phy creating time, then, shorten > _SCHEDULE_DELAY_ delay time less that 10 Seconds, and the phy-port > suspend/resume mechanism depend on _sm_work_ completely. Which in turn would mean that we would always depend on a fully controllable phy block. Right now it seems, rk3036, rk3228, rk3368 (probably forgot some) have the same type of phy, but with at least the unplug-detection missing. In its current form the driver can very well support those (later on) by simply working statically (only acting on phy_power callbacks and not going to suspend on its own). Also having the work running in 10-second intervall seems wasteful. > > So which is the better way from your view? or would you like to give > other unique perceptions please? As obvious from the above, I would prefer just renaming the functions :-) Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
