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

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

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 :-) .

The GRF is Rockchip's way of not sorting their own invented
additional registers. (aka a bunch of registers 

While on the older models there only ever was the one GRF
as dumping ground, newer SoCs now end up with multiple ones :-)

These are still iomem areas separate from the actual device-iomem they
work with/for but SoCs like the rk3568 now have at least 13 of them.


_But_ for the patch in question I fail to see what this set does at all.
The rk3568 (only) has XPCS_CON0 and XPCS_STATUS in its PIPE_GRF syscon
(according to the TRM), but the patch2 does strange things with
offset calculations and names that do not seem to have a match in the TRM.

So definitely more explanation on what happens here would be necessary.

Heiko