Re: [PATCH v2 07/10] net: pcs: xpcs: add Rockchip RK3568 platform glue driver

On Sun, Aug 02, 2026 at 11:20:27AM +0800, Coia Prant wrote:

quoted

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

quoted

The XPCS block contains four MII ports (0..3), each of which can be
routed to GMAC0 or GMAC1 via the pcs-handle property in the MAC node.
The hardware maps these ports to different MMDs:
  - port 0: MMD 7 (ROCKCHIP_MMD_MII)
  - port 1: MMD 2 (ROCKCHIP_MMD_MII1)
  - port 2: MMD 3 (ROCKCHIP_MMD_MII2)
  - port 3: MMD 4 (ROCKCHIP_MMD_MII3)

Why is port 0 called ROCKCHIP_MMD_MII not ROCKCHIP_MMD_MII0 ?

Hi Andrew,

The naming follows the Rockchip TRM. The hardware documentation refers to
the MMD for the first port as simply "MII" without a numeric suffix, while
the other ports are named "MII1", "MII2", "MII3". I kept the naming
consistent with the TRM to make it easier to cross-reference.

As I understand it, this is probably because the MII controls not only
Port 0, but
also the entire PCS (these registers are read-only in Ports 1-3 and reflected
back to the MII).

If you prefer, I can rename it to ROCKCHIP_MMD_MII0 for consistency. Let me
know and I'll update it in v3.

If the TRM gives it this name, then O.K. It just makes the code look
odd, unbalanced.

quoted

quoted

+static int xpcs_rk_read_c22(struct mii_bus *bus, int addr, int reg)
+{
+     struct dw_xpcs_rk *pxpcs = bus->priv;
+     int dev;
+
+     if (!xpcs_rk_mdio_addr_validate(addr))
+             return -ENODEV;
+
+     dev = xpcs_rk_mdio_read_remapping(addr, MDIO_MMD_VEND2, reg);

Does this mean C22 registers are mapped into the first 32 of C45
MDIO_MMD_VEND2?

Yes, exactly. The C22 register space (reg 0-31) is mapped into the first
32 registers of the VEND2 MMD (MMD 7). This is how the hardware is designed
and matches the standard C22 to C45 address mapping.

Does the xpcs code actually perform any C22 access? A quick look
suggests it is C45 only. xpcs_read() calls mdiodev_c45_read(). If C22
is not needed, i would not provide these functions.

quoted

quoted

+static int xpcs_rk_read_c45(struct mii_bus *bus, int addr, int dev, int reg)
+{
+     struct dw_xpcs_rk *pxpcs = bus->priv;
+
+     if (!xpcs_rk_mdio_addr_validate(addr))
+             return -ENODEV;
+
+     dev = xpcs_rk_mdio_read_remapping(addr, dev, reg);

Should it be returning an error for dev == MDIO_MMD_VEND2? Or at least
if reg < 32?

No, we cannot simply return an error here.

Thanks for the explanation.

       Andrew