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

[Conor Dooley]

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@xxxxxxxxxx> 
> 发送时间: 2026年10月13日 2:55
> 收件人: Luo Liu (刘罗) <Luo.Liu@xxxxxxxxxxx>
> 抄送: vkoul@xxxxxxxxxx; neil.armstrong@xxxxxxxxxx; mani@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; Gary Yang (杨新伟) <gary.yang@xxxxxxxxxxx>; Fugang Duan (段富刚) <fugang.duan@xxxxxxxxxxx>; linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; cix-kernel-upstream <cix-kernel-upstream@xxxxxxxxxxx>; p.zabel@xxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Wuyi Shen (沈武义) <Wuyi.Shen@xxxxxxxxxxx>; Eastlake Yang (杨东虎) <EastLake.Yang@xxxxxxxxxxx>
> 主题: 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@xxxxxxxxxxx>
> > ---
> >  .../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@xxxxxxxxxxx>
> > +
> > +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

> 
> 
> 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?

Thanks,
Conor.

Attachment: signature.asc
Description: PGP signature