linux-kernel - Re: [v2 PATCH 1/4] Documentation: bindings: add dt doc for Rockchip USB Type-C PHY

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: <CAAFQd5BY23zdKCxUnLDz6TT+6ro3qnQPP+bH-uu3KDCzdO50nw@mail.gmail.com>
Date:	Thu, 16 Jun 2016 16:49:42 +0900
From:	Tomasz Figa <tfiga@...omium.org>
To:	Chris Zhong <zyw@...k-chips.com>
Cc:	Heiko Stuebner <heiko@...ech.de>,
	Douglas Anderson <dianders@...omium.org>, yzq@...k-chips.com,
	groeck@...omium.org,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>, devicetree@...r.kernel.org,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [v2 PATCH 1/4] Documentation: bindings: add dt doc for Rockchip
 USB Type-C PHY

On Thu, Jun 16, 2016 at 9:31 AM, Chris Zhong <zyw@...k-chips.com> wrote:
> Hi Heiko
>
>
> On 06/16/2016 06:11 AM, Heiko Stuebner wrote:
>>
>> Am Montag, 13. Juni 2016, 17:39:46 schrieb Chris Zhong:
>>>
>>> This patch adds a binding that describes the Rockchip USB Type-C PHY
>>> for rk3399
>>>
>>> Signed-off-by: Chris Zhong <zyw@...k-chips.com>
>>>
>>> ---
>>>
>>> Changes in v2:
>>> - add some registers description
>>>
>>> Changes in v1:
>>> - add extcon node description
>>> - move the registers in phy driver
>>> - remove the suffix of reset
>>>
>>>   .../devicetree/bindings/phy/phy-rockchip-typec.txt | 77
>>> ++++++++++++++++++++++ 1 file changed, 77 insertions(+)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
>>> b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt new file
>>> mode 100644
>>> index 0000000..430920c
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
>>> @@ -0,0 +1,77 @@
>>> +* ROCKCHIP type-c PHY
>>> +---------------------
>>> +
>>> +Required properties:
>>> + - compatible : should be "rockchip,rk3399-typec-phy0" or
>>> +                        "rockchip,rk3399-typec-phy1"
>>> + - reg: Address and length of the usb phy control register set
>>> + - rockchip,grf : phandle to the syscon managing the "general
>>> +   register files"
>>> + - clocks : phandle + clock specifier for the phy clocks
>>> + - clock-names : string, clock name, must be "tcpdcore", "tcpdphy_ref";
>>> + - resets : a list of phandle + reset specifier pairs
>>> + - reset-names : string reset name, must be:
>>> +                "tcphy", "tcphy_pipe", "uphy_tcphy"
>>> + - #phy-cells : Must be 0.  See ./phy-bindings.txt for details.
>>> + - extcon : extcon specifier for the Power Delivery
>>> +
>>> +Note, there are 2 type-c phys for RK3399, and they are almost identical,
>>> except +these registers(description below), every register node contains
>>> 3 sections: +offset, enable bit, write mask bit.
>>> + - rockchip,typec_conn_dir : the register of type-c connector direction,
>>> +   for type-c phy0, it must be <0xe580 0 16>;
>>> +   for type-c phy1, it must be <0xe58c 0 16>;
>>
>> I think I said this already, but these register-lists would be much
>> happier
>> being defined inside the driver - see how Frank managed this on his
>> usb2phy
>> please.
>>
>> Heiko
>>
> Yes, I move them into driver in v1 patch, but Rob Herring and Guenter Roeck
> recommend to put them back to dts.
> These phy0 and phy1 are exactly the same, except these grf registers.

I think having the registers in dts isn't a bad idea. It might be
actually even useful to bring up this PHY on new SoCs, if they happen
to have the same PHY, just different plumbing.

But I suppose we don't need two separate compatible strings for phy0
and phy1 anymore as Rob pointed, just one "rockchip,rk3399-typec-phy".

After the above is fixed and underscores in property names are
replaced with dashes, feel free to add my Reviewed-by.

Best regards,
Tomasz