回复: 回复: [PATCH v2 1/3] dt-bindings: phy: Add CIX Sky1 DP PHY

[Luo Liu (刘罗)]

HI Conor,

	Thank you very much for your review and suggestions.

-----邮件原件-----
发件人: Conor Dooley <conor at kernel.org> 
发送时间: 2026年10月14日 0:17
收件人: Luo Liu (刘罗) <Luo.Liu at cixtech.com>
抄送: vkoul at kernel.org; neil.armstrong at linaro.org; mani at kernel.org; robh at kernel.org; krzk+dt at kernel.org; conor+dt at kernel.org; Gary Yang (杨新伟) <Gary.Yang at cixtech.com>; Fugang Duan (段富刚) <fugang.duan at cixtech.com>; linux-phy at lists.infradead.org; devicetree at vger.kernel.org; linux-arm-kernel at lists.infradead.org; cix-kernel-upstream <cix-kernel-upstream at cixtech.com>; p.zabel at pengutronix.de; linux-kernel at vger.kernel.org; Wuyi Shen (沈武义) <wuyi.shen at cixtech.com>; Eastlake Yang (杨东虎) <Eastlake.Yang at cixtech.com>
主题: Re: 回复: [PATCH v2 1/3] dt-bindings: phy: Add CIX Sky1 DP PHY

On Tue, Oct 13, 2026 at 08:50:48AM +0000, Luo Liu (刘罗) wrote:
> HI Conor,
> 
> 	Thank you very much for your reply and suggestions.
> 
> -----邮件原件-----
> 发件人: Conor Dooley <conor at kernel.org>
> 发送时间: 2026年10月13日 2:55
> 收件人: Luo Liu (刘罗) <Luo.Liu at cixtech.com>
> 抄送: vkoul at kernel.org; neil.armstrong at linaro.org; mani at kernel.org; 
> robh at kernel.org; krzk+dt at kernel.org; conor+dt at kernel.org; Gary Yang 
> (杨新伟) <gary.yang at cixtech.com>; Fugang Duan (段富刚) 
> <fugang.duan at cixtech.com>; linux-phy at lists.infradead.org; 
> devicetree at vger.kernel.org; linux-arm-kernel at lists.infradead.org; 
> cix-kernel-upstream <cix-kernel-upstream at cixtech.com>; 
> p.zabel at pengutronix.de; linux-kernel at vger.kernel.org; Wuyi Shen (沈武义) 
> <Wuyi.Shen at cixtech.com>; Eastlake Yang (杨东虎) 
> <EastLake.Yang at cixtech.com>
> 主题: Re: [PATCH v2 1/3] dt-bindings: phy: Add CIX Sky1 DP PHY
> 
> On Mon, Oct 12, 2026 at 08:36:16PM +0800, Luo Liu wrote:
> > Document the dedicated DisplayPort PHY on CIX Sky1, including the 
> > optional cix,external-dp board property for external connector swing.
> > Add a MAINTAINERS entry for the binding and driver.
> > 
> > Signed-off-by: Luo Liu <Luo.Liu at cixtech.com>
> > ---
> >  .../bindings/phy/cix,sky1-dp-phy.yaml         | 53 +++++++++++++++++++
> >  MAINTAINERS                                   |  8 +++
> >  2 files changed, 61 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/phy/cix,sky1-dp-phy.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/phy/cix,sky1-dp-phy.yaml
> > b/Documentation/devicetree/bindings/phy/cix,sky1-dp-phy.yaml
> > new file mode 100644
> > index 000000000000..c6914cab74e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/cix,sky1-dp-phy.yaml
> > @@ -0,0 +1,53 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/cix,sky1-dp-phy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: CIX Sky1 DisplayPort PHY
> > +
> > +maintainers:
> > +  - Luo Liu <Luo.Liu at cixtech.com>
> > +
> > +description:
> > +  The CIX Sky1 DP PHY provides the PMA/CDB register interface for 
> > +the
> > +  dedicated DisplayPort instance on CIX Sky1.
> > +
> > +properties:
> > +  compatible:
> > +    const: cix,sky1-dp-phy
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +  resets:
> > +    maxItems: 1
> > +    description: PHY hardware reset
> > +
> > +  cix,external-dp:
> > +    type: boolean
> > +    description:
> > +      Use voltage swing values suitable for an external DisplayPort
> > +      connector instead of the default low-voltage values.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#phy-cells"
> > +  - resets
> 
> A reset but no clocks? That's a bit hard to believe.
> 
> Seems fine otherwise, unless I am missing some sort of common property for the voltage swing stuff.
> 
> Thanks,
> Conor.
> 
> 
> 1，Regarding the missing clocks property: 
> 
> 	The PHY APB clock is physically the same clock as the DPTx controller 
> APB clock. It is described and
> 
> managed only by the DPTx controller driver, so the PHY node does not list a separate clocks property.
> 
> The PHY driver only implements the standard PHY callbacks. It does not 
> touch the hardware in probe;
> 
> All PHY programming happens later, when the DPTx driver actively calls 
> phy_init() / phy_power_on() / phy_configure()
> 
> and similar interfaces. 
> 
> 	By that time the shared APB clock is already under DPTx control.
> 
> So from this binding's point of view there are no clocks to describe — only the reset line owned by the PHY node.

This sounds wrong, if the PHY uses this clock then the "DPTx" (no idea what that actually is) it should also have this clock as an input.

pw-bot: changes-requested


You're right, thanks. If the PHY consumes the APB clock, the PHY node should also list it as a clock input even when it is shared with the DPTx controller. 

I'll add clocks to the binding, DT and driver in the next revision.


> 
> 
> 2，On the voltage swing property: there is no suitable common DT property for this.
> 
> 	The flag only selects which vendor calibration table to use for the 
> same PHY IP — the DisplayPort swing/EQ table
> 
> versus the eDP table. 
> 
> 	Runtime swing/pre-emphasis levels still come from phy_configure_opts_dp as usual.
> 
> The old name cix,external-dp was misleading. I will rename it to 
> cix,edp (present => eDP table, absent => DP table)

I dunno, I think "external" is clearer than "e", since eDP is also used for embedded displayport?


DP and eDP use the same PHY IP and driver here; only the board wiring differs.

This property just selects which swing table to use.

Agreed that "edp" is ambiguous — if you prefer cix,external-dp, I'll switch it back in the next revision.


Thanks,
Luo.