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

[Birger Koblitz]

On 31/07/2026 21:37, Andrew Lunn wrote:
> > +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.

> > +
> > +	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.

> > +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.

> > +
> > +	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.

> > +static int ax88179a_init_mdio(struct usbnet *dev)
> > +	netdev_err(dev->net, "Registering MDIO bus\n");
>
> Left over debug.
Removed in v5.

> > +	/* 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.

> > +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.

> > +	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.