Hi Guenter On 06/27/2016 12:01 PM, Guenter Roeck wrote: > On Sun, Jun 26, 2016 at 7:19 PM, Chris Zhong <zyw at rock-chips.com> wrote: >> Hi Heiko >> >> >> On 06/25/2016 03:39 AM, Heiko Stuebner wrote: >>> Am Donnerstag, 23. Juni 2016, 18:27:52 schrieb Kishon Vijay Abraham I: >>>> Hi, >>>> >>>> On Thursday 23 June 2016 06:21 PM, Chris Zhong wrote: >>>>> Add a PHY provider driver for the rk3399 SoC Type-c PHY. The USB >>>>> Type-C PHY is designed to support the USB3 and DP applications. The >>>>> PHY basically has two main components: USB3 and DisplyPort. USB3 >>>>> operates in SuperSpeed mode and the DP can operate at RBR, HBR and >>>>> HBR2 data rates. >>>>> >>>>> Signed-off-by: Chris Zhong <zyw at rock-chips.com> >>>>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com> >>>>> >>>>> --- >>>>> >>>>> Changes in v3: >>>>> - remove the phy framework(Kishon Vijay Abraham I) >>>>> - add parentheses around the macro >>>>> - use a single space between type and name >>>>> - add spaces after opening and before closing braces. >>>>> - use u16 for register value >>>>> - remove type-c phy header file >>>>> - CodingStyle optimization >>>>> - use some cable extcon to get type-c port information >>>>> - add a extcon to notify Display Port >>>>> >>>>> Changes in v2: >>>>> - select RESET_CONTROLLER >>>>> - alphabetic order >>>>> - modify some spelling mistakes >>>>> - make mode cleaner >>>>> - use bool for enable/disable >>>>> - check all of the return value >>>>> - return a better err number >>>>> - use more readx_poll_timeout() >>>>> - clk_disable_unprepare(tcphy->clk_ref); >>>>> - remove unuse functions, rockchip_typec_phy_power_on/off >>>>> - remove unnecessary typecast from void * >>>>> - use dts node to distinguish between phys. >>>>> >>>>> Changes in v1: >>>>> - update the licence note >>>>> - init core clock to 50MHz >>>>> - use extcon API >>>>> - remove unused global >>>>> - add some comments for magic num >>>>> - change usleep_range(1000, 2000) tousleep_range(1000, 1050) >>>>> - remove __func__ from dev_err >>>>> - return err number when get clk failed >>>>> - remove ADDR_ADJ define >>>>> - use devm_clk_get(&pdev->dev, "tcpdcore") >>>>> >>>>> drivers/phy/Kconfig | 8 + >>>>> drivers/phy/Makefile | 1 + >>>>> drivers/phy/phy-rockchip-typec.c | 1027 >>>>> ++++++++++++++++++++++++++++++++++++++ 3 files changed, 1036 >>>>> insertions(+) >>>>> create mode 100644 drivers/phy/phy-rockchip-typec.c >>>>> >>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >>>>> index 26566db..ec87b3a 100644 >>>>> --- a/drivers/phy/Kconfig >>>>> +++ b/drivers/phy/Kconfig >>>>> @@ -351,6 +351,14 @@ config PHY_ROCKCHIP_DP >>>>> >>>>> help >>>>> >>>>> Enable this to support the Rockchip Display Port PHY. >>>>> >>>>> +config PHY_ROCKCHIP_TYPEC >>>>> + tristate "Rockchip TYPEC PHY Driver" >>>>> + depends on ARCH_ROCKCHIP && OF >>>>> + select GENERIC_PHY >>>> Why? None of the generic PHY API's are used here. Why do you want select >>>> generic PHY? >>> I'm actually wondering, why there are no phy ops to start and stop the py. >>> Right now the device seems to be on and handling all the extcon notifies >>> all >>> the time even if no-one is using the phy. >>> >>> There are two users of this phy, the dp-controller as well as some usb >>> component. The phy framework is nicely refcounted, so can handle any >>> number >>> of phy users and somehow I'd expect the phy to do nothing (and try to not >>> consume power) if neither the dp-controller nor the usb-part is actually >>> running. >>> >>> It may very well be my ignorance, but Chris could you explain, if there is >>> a >>> reason for this? >>> >>> >>> Thanks >>> Heiko >>> >>> >> It is good idea, The USB3 and DP controller detect the extcon cable state: >> USB/USB HOST/DP. If a Type-C device plugged, call phy power on, the phy >> driver get all the state of extcon: dfp, ufp, dp, flip, pin assignment, and >> then finish the phy init. So the phy driver need not register extcon >> notification. >> But this mechanism allows phy driver only focus plug/unplug event, if some >> other things happen, such as data role change, the phy will not be notified. >> I'm not sure if this situation exists. > Sorry, I think I am lost (again). > > All roles can change on the fly. Role changes can be triggered from > user space, or by the remote partner. If we restrict such role > changes, we would have to reject all locally triggered role change > requests, as well as all role change requests from the remote, after > the initial connection was established. I don't really see the point > of doing that, though. Wasn't this what the notifications were all > about ? > > Guenter > The following are the all 3 cases about data role changing: 1. "USB host only" mode(power_count ==1), switch to "USB device" mode we can re-init phy by phy_power_off and then phy_power_on 2. "USB host + DP" mode(power_count ==2), switch to "USB device" mode The phy can not support ufp+dp, so it is reasonable to do nothing. Moreover, does this situation exist? 3. "USB device" mode (power_count ==1), switch to "USB host only" or "USB host + DP" it is similar with case 1, dp or usb host can re-init the phy again. >