Hi Heiko and Brain

On 03/09/2017 09:02 AM, Heiko St?bner wrote:

Am Mittwoch, 8. M?rz 2017, 16:39:23 CET schrieb Brian Norris:

quoted

On Fri, Feb 10, 2017 at 03:44:13PM +0800, Chris Zhong wrote:

quoted

There are 2 Type-c PHYs in RK3399, but only one DP controller. Hence
only one PHY can connect to DP controller at one time, the other should
be disconnected. The GRF_SOC_CON26 register has a switch bit to do it,
set this bit means enable PHY 1, clear this bit means enable PHY 0.

Signed-off-by: Chris Zhong <redacted>
---

  drivers/phy/phy-rockchip-typec.c | 9 +++++++++
  1 file changed, 9 insertions(+)

diff --git a/drivers/phy/phy-rockchip-typec.c

b/drivers/phy/phy-rockchip-typec.c index 7cfb0f8..1604aaa 100644

--- a/drivers/phy/phy-rockchip-typec.c
+++ b/drivers/phy/phy-rockchip-typec.c

@@ -267,6 +267,7 @@ struct rockchip_usb3phy_port_cfg {

  	struct usb3phy_reg usb3tousb2_en;
  	struct usb3phy_reg external_psm;
  	struct usb3phy_reg pipe_status;

+	struct usb3phy_reg uphy_dp_sel;

  };
  
  struct rockchip_typec_phy {

@@ -736,6 +737,7 @@ static const struct phy_ops rockchip_usb3_phy_ops = {

  static int rockchip_dp_phy_power_on(struct phy *phy)
  {
  
  	struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);

+	struct rockchip_usb3phy_port_cfg *cfg = &tcphy->port_cfgs;

  	int new_mode, ret = 0;
  	u32 val;

@@ -766,6 +768,8 @@ static int rockchip_dp_phy_power_on(struct phy *phy)

  		tcphy_phy_init(tcphy, new_mode);
  	
  	}

+	property_enable(tcphy, &cfg->uphy_dp_sel, 1);
+

  	ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,

Idea for future work: this should just be readl_poll_timeout() here, and
throughout the driver.

Yes, the readl_poll_timeout is better, if next version series is needed, 
I am going to
add it in a separate patch behind this patch.

quoted

quoted

  				 val, val & DP_MODE_A2, 1000,
  				 PHY_MODE_SET_TIMEOUT);

@@ -869,6 +873,11 @@ static int tcphy_parse_dt(struct rockchip_typec_phy

*tcphy,>
  	if (ret)
  	
  		return ret;

+	ret = tcphy_get_param(dev, &cfg->uphy_dp_sel,
+			      "rockchip,uphy-dp-sel");
+	if (ret)
+		return ret;

What about existing device trees? You're essentially adding this
new property and requiring it at the same time.

Or are we considering no RK3399 DP stable at the moment? I guess we
haven't actually merged any device trees that support this yet, no?

An interesting situation we're in here. On the one hand, you're right this
breaks "backwards compatiblity".

But on the other hand, the type-c phy is currently very much unused. The only
current board rk3399-evb.dts does not enable them (so they're disabled
everywhere) and we have neither dwc3 nor dp nodes in any rk3399 devicetrees so
far. Also Rob was ok with the binding change :-) .

So from my pov, I'd say it _should_ be ok, as nothing is using the phys at all
yet and thus there is nothing that could get broken.


Heiko

Thanks Heiko. On the other hand, these is no any display node at rk3399 
dtsi,
so I can not add the DP node, do you or Mark.Yao have plan to complete it?

quoted

Brian

quoted

+

  	tcphy->grf_regs = syscon_regmap_lookup_by_phandle(dev->of_node,
  	
  							  "rockchip,grf");
  	
  	if (IS_ERR(tcphy->grf_regs)) {

-- 
Chris Zhong