Hi Heiko,
On 2016/6/21 17:05, Heiko Stübner wrote:
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
Got it, thanks for your comments. I am going to correct and send out a
new version later today, so sorry for trouble you to sign 'Reviewed-by'
again if no any other issues.
BR.
Frank
--
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
