linux-kernel - Re: [PATCH 3/4] phy: rockchip-typec: support DP phy switch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <58C0BBB0.6080204@rock-chips.com>
Date:   Thu, 9 Mar 2017 10:19:28 +0800
From:   Chris Zhong <zyw@...k-chips.com>
To:     Heiko Stübner <heiko@...ech.de>,
        Brian Norris <briannorris@...omium.org>
Cc:     dri-devel@...ts.freedesktop.org, kishon@...com, robh@...nel.org,
        linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
        seanpaul@...omium.org, groeck@...omium.org,
        linux-arm-kernel@...ts.infradead.org, mark.yao@...k-chips.com
Subject: Re: [PATCH 3/4] phy: rockchip-typec: support DP phy switch

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:
>> On Fri, Feb 10, 2017 at 03:44:13PM +0800, Chris Zhong wrote:
>>> 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 <zyw@...k-chips.com>
>>> ---
>>>
>>>   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.
>>>   				 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?

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

-- 
Chris Zhong