Re: [PATCH v2 05/10] arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes

Coia Prant <[email protected]> Sun, 2 Aug 2026 03:19:37 +0800
Newsgroups org.kernel.vger.linux-renesas-soc,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-phy,org.infradead.lists.linux-rockchip,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <CALj3r0gqvFZPb2SKRMPwf9chCx076rwCnXEoOMFch_V-fOKV6w@mail.gmail.com>
Heiko St=C3=BCbner <[email protected]> =E4=BA=8E2026=E5=B9=B48=E6=9C=882=E6=
=97=A5=E5=91=A8=E6=97=A5 00:28=E5=86=99=E9=81=93=EF=BC=9A
>
> Am Samstag, 1. August 2026, 16:22:29 Mitteleurop=C3=A4ische Sommerzeit sc=
hrieb Coia Prant:
> > The RK3568 SoC integrates a Synopsys DesignWare XPCS that provides
> > the Physical Coding Sublayer for 1000BASE-X, SGMII, and QSGMII
> > interfaces via its four MII ports.  Add the XPCS device node and
> > its pcs-mii sub-nodes to the SoC device tree.
> >
> > The XPCS device is accessed via the APB3 bus at 0xfda00000 and
> > requires the CSR clock (PCLK_XPCS) for register access and the EEE
> > clock (CLK_XPCS_EEE) for Energy Efficient Ethernet operation.  The
> > PD_PIPE power domain must be enabled before any register access.
> >
> > Also add two fixed-clock nodes (xpcs_gmac0_clk and xpcs_gmac1_clk)
> > providing the 125 MHz reference clock for the GMACs when operating
> > with XPCS.  These clocks are used as the assigned-clock-parents
> > for the respective GMAC nodes in board-level device trees.
> >
> > The XPCS node and its pcs-mii sub-nodes are disabled by default and
> > must be enabled at the board level when 1000BASE-X/SGMII/QSGMII is
> > in use.  The fixed-clock nodes are always present and do not have a
> > status property, as they are static clock sources.
> >
> > The XPCS node also requires a reference to the appropriate Naneng
> > Combo PHY via the phys property at the board level.
> >
> > Signed-off-by: Coia Prant <[email protected]>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3568.dtsi | 45 ++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot=
/dts/rockchip/rk3568.dtsi
> > index 3bc653f027f1f..989e164c0eb39 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > @@ -110,6 +110,51 @@ sata0: sata@fc000000 {
> >               status =3D "disabled";
> >       };
> >
> > +     xpcs: pcs@fda00000 {
> > +             compatible =3D "rockchip,rk3568-xpcs";
> > +             #address-cells =3D <1>;
> > +             #size-cells =3D <0>;
> > +             reg =3D <0x0 0xfda00000 0x0 0x200000>;
> > +             clocks =3D <&cru PCLK_XPCS>, <&cru CLK_XPCS_EEE>;
> > +             clock-names =3D "csr", "eee";
> > +             power-domains =3D <&power RK3568_PD_PIPE>;
> > +             status =3D "disabled";
> > +
> > +             xpcs_mii0: pcs-mii@0 {
> > +                     reg =3D <0>;
> > +                     status =3D "disabled";
> > +             };
> > +
> > +             xpcs_mii1: pcs-mii@1 {
> > +                     reg =3D <1>;
> > +                     status =3D "disabled";
> > +             };
> > +
> > +             xpcs_mii2: pcs-mii@2 {
> > +                     reg =3D <2>;
> > +                     status =3D "disabled";
> > +             };
> > +
> > +             xpcs_mii3: pcs-mii@3 {
> > +                     reg =3D <3>;
> > +                     status =3D "disabled";
> > +             };
> > +     };
> > +
> > +     xpcs_gmac0_clk: xpcs-gmac0-clock {
> > +             compatible =3D "fixed-clock";
> > +             clock-frequency =3D <125000000>;
> > +             clock-output-names =3D "clk_gmac0_xpcs_mii";
> > +             #clock-cells =3D <0>;
> > +     };
> > +
> > +     xpcs_gmac1_clk: xpcs-gmac1-clock {
> > +             compatible =3D "fixed-clock";
> > +             clock-frequency =3D <125000000>;
> > +             clock-output-names =3D "clk_gmac1_xpcs_mii";
> > +             #clock-cells =3D <0>;
> > +     };
>
> I don't fully believe those clocks are "just there" inside the SoC.
> Instead I _assume_ they are generated by some oscillator on the
> individual boards?
>
> Because as described there, they would be just magically be there.
>
> So please aim for a real clock description (likely on a board-level)
>
>
> Heiko

Hi Heiko,

Thanks for the review.

During my tests, I found that the 125 MHz clocks (`xpcs_gmac0_clk` /
`xpcs_gmac1_clk`) are actually outputs of the Naneng Combo PHY
(SerDes) when it's initialized for SGMII/QSGMII mode, rather than
independent board-level oscillators.

Since these clocks are part of the SoC's internal design and
consistent across all RK3568 boards, I think keeping them in the SoC
dtsi makes sense to avoid unnecessary duplication. However, I also
understand your concern that they might appear to be "magically there"
without any explicit dependency.

To address this, I'm considering adding `status =3D "disabled"` to both
clock nodes in the dtsi, so boards that actually use SGMII/QSGMII can
explicitly enable them together with the XPCS node and the
corresponding Combo PHY reference. This would make the dependency and
activation requirements clearer.

What's your thought on this? Would you prefer adding `status =3D
"disabled"` to clarify the dependency, or do you think the nodes
should be moved to board-level dts entirely? I'm open to either
direction.

Thanks,
Coia