Re: [PATCH net-next v4 03/12] ax88179_178a: Add HW support for AX179A-based chips

On 31/07/2026 21:37, Andrew Lunn wrote:

quoted

+static int ax88179_mdiobus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
+{
+	struct usbnet *dev = bus->priv;
+	u16 res = (u16)val;

val already is a u16, so the cast is not needed.

Will fix in v5, this was code from the original driver.

quoted

+
+	return ax88179_write_cmd(dev, AX_ACCESS_PHY, phy_id, (__u16)regnum, 2, &res);

Does ax88179_write_cmd actually modify the res value? I'm just
wondering why you need the local copy.

The local copy is not needed. This was also C&P from the original driver.
Fixed in v5.

quoted

+static void ax88179a_get_pauseparam(struct net_device *net, struct ethtool_pauseparam *pause)
+{
+	struct usbnet *dev = netdev_priv(net);
+	struct ax88179_data *data;
+
+	data = dev->driver_priv;

This pattern happens enough that it is worth adding a helper,
netdev2data() or something like that.

Will introduce ntdev2data() in v5.

quoted

+
+	netdev_info(dev->net, "ax88179a - Link status is: 1, Link speed: %d, Duplex: %d\n",
+		    speed, duplex);
+}

Maybe not needed? Does phylink print something?

Removed in v5, phylink prints the same information.

quoted

+static int ax88179a_init_mdio(struct usbnet *dev)
+	netdev_err(dev->net, "Registering MDIO bus\n");

Left over debug.

Removed in v5.

quoted

+	/* Initialize MII structure */
+	dev->mii.dev = dev->net;
+	dev->mii.mdio_read = ax88179_mdio_read;
+	dev->mii.mdio_write = ax88179_mdio_write;
+	dev->mii.phy_id_mask = 0xff;
+	dev->mii.reg_num_mask = 0xff;
+	dev->mii.phy_id = AX88179_PHY_ID;

So the aim is this is removed. Do the follow up patches remove it?

Ooops, forgot to remove this. Removed in v5.

quoted

+static int ax88179a_reset(struct usbnet *dev)
+	if (ax179_data->chip_version == AX_VERSION_AX88279) {
+		*tmp16 = ax88179_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
+		*tmp16 &= ~(ADVERTISE_10FULL | ADVERTISE_10HALF);
+		*tmp16 |= AX_ADVERTISE_2500;
+		ax88179_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, *tmp16);
+	}

If the PHY driver is doing the correct thing, this is not needed.  So
long as you tell phylink that 10Mbps is not supported by the MAC, it
should disable the advertisement of 10Mps link modes.

Fixed in v5.

quoted

+	ax179_data->eee_enabled = 0;
+	ax179_data->eee_active = 0;

Are these used anywhere? Phylink should be tracking the EEE state, not
the MAC driver.

Removed in v5.