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

[Coia Prant]

Thanks for the review.

> [Severity: Low]
> The commit message states these clocks are used as assigned-clock-parents
> for the respective GMAC nodes, but the patch doesn't appear to add the
> assigned-clock-parents properties to the gmac0 or gmac1 nodes. Are these
> assignments missing?

The `assigned-clock-parents` assignments are not added in the SoC-level
`rk3568.dtsi` because doing so would break existing boards that use
RGMII mode. The clock parent for GMAC0/1 is currently hardcoded to the
RGMII clock path in the mainline device tree.

Instead, the `assigned-clock-parents` are applied at the board level.
This is already done in Patch 9 of this series for the Photonicat board:

    &gmac0 {
        assigned-clocks = <&cru SCLK_GMAC0_RX_TX>;
        assigned-clock-parents = <&xpcs_gmac0_clk>;
        ...
    };

So the assignments exist, but in the board DTS rather than the SoC DTSI.

> [Severity: Low]
> Should this internal SoC clock be modeled as a clock provider from the
> generating XPCS IP node using #clock-cells, rather than using a dummy
> fixed-clock?

The XPCS block does not have a clock controller interface. It simply
outputs a fixed 125MHz clock for the GMAC when operating in SGMII mode.
There is no register interface to control or configure this clock — it
is a fixed-rate output from the PCS hardware.

Using `fixed-clock` is therefore the correct and accurate representation
of the hardware. The XPCS is not a clock controller, so it should not
be modeled as one.

> Additionally, the commit message states "All nodes are left disabled by
> default", but this node is missing the status = "disabled" property. Will
> this leave the clock unconditionally enabled?
> [Severity: Low]
>
> Similarly, does this node also need a status = "disabled" property to align
> with the behavior described in the commit message?

The fixed-clock nodes are always present and do not have a `status`
property. This is intentional: they are part of the SoC's fixed clock
infrastructure, similar to other fixed-clock nodes in the SoC DTSI.
They are not "devices" that need to be enabled/disabled, but rather
static clock providers.

The "all nodes are left disabled by default" statement in the commit
message refers to the XPCS node and its pcs-mii sub-nodes, not the
fixed-clock nodes. I will revise the commit message to clarify this.

Thanks,
Coia