Re: [PATCH net-next v14 4/4] net: dsa: add basic initial driver for MxL862xx switches

[Vladimir Oltean]

On Sat, Feb 07, 2026 at 03:07:27AM +0000, Daniel Golle wrote:
> +/* PHY access via firmware relay */
> +static int mxl862xx_phy_read_mmd(struct mxl862xx_priv *priv, int port,
> +				 int devadd, int reg)
> +{
> +	struct mdio_relay_data param = {
> +		.phy = port,
> +		.mmd = devadd,
> +		.reg = cpu_to_le16(reg),
> +	};
> +	int ret;
> +
> +	ret = MXL862XX_API_READ(priv, INT_GPHY_READ, param);
> +	if (ret)
> +		return ret;
> +
> +	return le16_to_cpu(param.data);
> +}
> +
> +static int mxl862xx_phy_write_mmd(struct mxl862xx_priv *priv, int port,
> +				  int devadd, int reg, u16 data)
> +{
> +	struct mdio_relay_data param = {
> +		.phy = port,
> +		.mmd = devadd,
> +		.reg = cpu_to_le16(reg),
> +		.data = cpu_to_le16(data),
> +	};
> +
> +	return MXL862XX_API_WRITE(priv, INT_GPHY_WRITE, param);
> +}
> +
> +static int mxl862xx_phy_read_mii_bus(struct mii_bus *bus, int port, int regnum)
> +{
> +	return mxl862xx_phy_read_mmd(bus->priv, port, 0, regnum);
> +}
> +
> +static int mxl862xx_phy_write_mii_bus(struct mii_bus *bus, int port,
> +				      int regnum, u16 val)
> +{
> +	return mxl862xx_phy_write_mmd(bus->priv, port, 0, regnum, val);
> +}
> +
> +static int mxl862xx_phy_read_c45_mii_bus(struct mii_bus *bus, int port,
> +					 int devadd, int regnum)
> +{
> +	return mxl862xx_phy_read_mmd(bus->priv, port, devadd, regnum);
> +}
> +
> +static int mxl862xx_phy_write_c45_mii_bus(struct mii_bus *bus, int port,
> +					  int devadd, int regnum, u16 val)
> +{
> +	return mxl862xx_phy_write_mmd(bus->priv, port, devadd, regnum, val);
> +}

You took inspiration from the wrong place with the mii_bus ops prototypes,
specifically with the "int port" argument.

The second argument does not hold the port, it holds the PHY address.
I.e. in this case:
                port@6 {
                    reg = <6>;
                    phy-handle = <&phy5>;
                    phy-mode = "internal";
                };
                phy5: ethernet-phy@5 {
                    reg = <5>;
                };

"int port" is 5, not 6.

Your source of inspiration are the prototypes of an mii_bus used as
ds->user_mii_bus. We have a different set of requirements there, because
ds->user_mii_bus exists for the case where the PHY is not described in
the device tree, so the port index is given as argument and the
user_mii_bus is responsible for internally translating the port index to
a PHY address.

So while the use of "int port" as argument name for these operations is
justifiable in some cases, it is not applicable to this driver, and will
be a pitfall for anyone who has to modify or debug this code.