Re: [PATCH 1/2] dt-bindings: net: rockchip-dwmac: add rk3568 xpcs compatible

On Tue, Nov 29, 2022 at 11:22:28AM +0100, Heiko Stübner wrote:

Am Dienstag, 29. November 2022, 10:59:34 CET schrieb Krzysztof Kozlowski:

quoted

On 29/11/2022 10:56, Heiko Stübner wrote:

quoted

Am Dienstag, 29. November 2022, 09:49:08 CET schrieb Krzysztof Kozlowski:

quoted

On 29/11/2022 08:27, Chukun Pan wrote:

quoted

The gmac of RK3568 supports RGMII/SGMII/QSGMII interface.
This patch adds a compatible string for the required clock.

Signed-off-by: Chukun Pan <redacted>
---
 Documentation/devicetree/bindings/net/rockchip-dwmac.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
index 42fb72b6909d..36b1e82212e7 100644
--- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml

@@ -68,6 +68,7 @@ properties:
         - mac_clk_rx
         - aclk_mac
         - pclk_mac
+        - pclk_xpcs
         - clk_mac_ref
         - clk_mac_refout
         - clk_mac_speed

@@ -90,6 +91,11 @@ properties:
       The phandle of the syscon node for the peripheral general register file.
     $ref: /schemas/types.yaml#/definitions/phandle
 
+  rockchip,xpcs:
+    description:
+      The phandle of the syscon node for the peripheral general register file.

You used the same description as above, so no, you cannot have two
properties which are the same. syscons for GRF are called
"rockchip,grf", aren't they?

Not necessarily :-) .

OK, then description should have something like "...GRF for foo bar".

Actually looking deeper in the TRM, having these registers "just" written
to from the dwmac-glue-layer feels quite a bit like a hack.

The "pcs" thingy referenced in patch2 actually looks more like a real device
with its own section in the TRM and own iomem area. This pcs device then
itself has some more settings stored in said pipe-grf.

So this looks more like it wants to be an actual phy-driver.

There's a PCS binding now. Seems like it should be used if there is 
also a PHY already. PCS may be part of the PHY or separate block AIUI.

Rob