Re: [PATCH v2 04/10] dt-bindings: net: pcs: add rockchip,rk3568-xpcs binding

Hi Krzysztof,

Krzysztof Kozlowski [off-list ref] 于2026年8月6日周四 14:49写道：

I don't understand any of these. There are none of my quotes. I don't
get what you are referring to.

Sorry for the confusing previous reply, I didn't quote your original
comments properly. Here's a clean response with your points quoted.

Krzysztof Kozlowski [off-list ref] 于2026年8月5日周三 15:29写道：

On Sat, Aug 01, 2026 at 10:22:28PM +0800, Coia Prant wrote:

quoted

Add device tree binding documentation for the Synopsys DesignWare
XPCS integrated on the Rockchip RK3568 SoC.

The XPCS is accessed over the APB3 bus and internally connected to
a Naneng Combo SerDes PHY.  It supports 1000BASE-X, SGMII, and
QSGMII modes, with four MII ports.

The binding describes:
- Required properties: compatible, reg, clocks, clock-names
- Optional properties: phys, phy-names, power-domains
- pcs-mii sub-nodes for each MII port (reg 0..3)

Irrelevant paragraph. We can read the diff. Drop.
A nit, subject: drop second/last, redundant "binding". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v7.1-rc7/source/Documentation/devicetree/bindings/submitting-patches.rst#L23

Okay. I will drop these in v3.

quoted

+properties:
+  compatible:
+    const: rockchip,rk3568-xpcs
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0

reg is always the second property.

Also, use consistent style of quotes.

Okay.

quoted

+
+  reg:
+    description: |
+      Base address and size of the XPCS register space mapped over the
+      APB3 bus.

Drop description, redundant.

I'll drop the redundant description paragraph from the commit message
and keep only the essential information.

quoted

+    maxItems: 1
+
+  clocks:
+    description: |
+      Clock sources for the XPCS:
+      - csr: APB3 bus interface clock (clk_csr_i), required for register
+        access.
+      - eee: EEE clock (clk_eee_i), required for Energy Efficient
+        Ethernet (EEE) operation.
+    minItems: 2
+    maxItems: 2

No, instead list items with description.

I'll change to items with descriptions.

quoted

+
+  clock-names:
+    items:
+      - const: csr
+      - const: eee
+
+  phys:
+    description: |

Do not need '|' unless you need to preserve formatting.

I'll remove unnecessary '|' where not needed.

quoted

+      The phandle of SerDes PHY (Naneng Combo PHY) that provides
+      the serial lanes for 1000BASE-X / SGMII / QSGMII.
+      The SerDes must be powered on and initialised before any XPCS
+      register access.
+    maxItems: 1
+
+  phy-names:
+    const: serdes
+
+  power-domains:
+    description: |
+      Power domain for the XPCS.

Drop sentence and |

I'll drop the redundant part.

quoted

+      On RK3568 this is typically the PD_PIPE power domain, which also
+      supplies the SerDes PHY.
+    maxItems: 1
+
+patternProperties:
+  "^pcs-mii@[0-3]$":

Why `git grep pcs-mii@` gives me no results? Are you doing this
similarly to existing devices or is this quite different device than
every other hardware?

I used "pcs-mii" because the Rockchip TRM refers to these as "MII ports"
and they are functionally PCS instances. I see a similar pattern in the
Renesas RZN1 MIIC binding (renesas,rzn1-miic.yaml), which uses
"mii-conv@[0-5]$" for its ports.

If you prefer a different name (e.g., "port@0"-"port@3"), I'm happy to
change it. Please let me know.

quoted

+    type: object
+    description: |

Same here

quoted

+      One of the four MII ports of the XPCS.
+      The port number is specified by the reg property (0..3).

Drop sentence, redundant. Schema tells that.

I'll fix this to use items with min/max.

quoted

+      The port is linked to an Ethernet MAC controller via the
+      pcs-handle property in the MAC's device tree node.
+
+    properties:
+      reg:
+        minimum: 0
+        maximum: 3
+        description: |

As well

quoted

+          MII port number of PCS.
+
+      status: true

Nope, do you see anywhere code like this?

I'll remove this generic property.

Thanks,
Coia