Re: [PATCH net-next v11 06/15] ax88179_178a: Add HW support for AX179A-based chips

Hi Nicolai,

thanks so much for reviewing this patch-series!
On 9/14/26 16:12, Nicolai Buchwitz wrote:

quoted

 static const struct usb_device_id products[] = {
 {
+    /* ASIX AX88179A USB 3.2 1000Mbit Ethernet */
+    USB_DEVICE_VER(0x0b95, 0x1790, 0, 0x0200),

USB_DEVICE_VER(0x0b95, 0x1790, 0x0200, 0x0200),

Otherwise it would select the wrong callback the existing AX88179 (0x0100)?

Oops, that is a good catch! I took this from manufacturer code and 
probably that range includes also prototypes. The older devices are
handled there with a different driver, however, but indeed we need to be 
sure the 0x0100 versions are not caught. Will fix in the v12.

Same as with AX88179 above.

Indeed. Will fix.

quoted

+static const struct net_device_ops ax88179a_netdev_ops = {
+    .ndo_open        = usbnet_open,
+    .ndo_stop        = usbnet_stop,
+    .ndo_start_xmit        = usbnet_start_xmit,
+    .ndo_tx_timeout        = usbnet_tx_timeout,
+    .ndo_get_stats64    = dev_get_tstats64,
+    .ndo_change_mtu        = ax88179_change_mtu,
+    .ndo_set_mac_address    = ax88179_set_mac_addr,
+    .ndo_validate_addr    = eth_validate_addr,
+    .ndo_eth_ioctl        = usbnet_mii_ioctl,

dev->mii.mdio_read is never initialized for ax88179a, so dev- 
 >mii.mdio_read()
through usbnet_mii_ioctl() is a NULL function pointer.

Something like:

   static int ax88179a_mii_ioctl(...)
   {
       struct ax88179_data *data = netdev2data(net);
       return phylink_mii_ioctl(data->phylink, ifr, cmd);
   }

I will remove the entire
+    .ndo_eth_ioctl        = usbnet_mii_ioctl,
line. It should not be necessary.

quoted

+
+        if (pkt_desc & AX179A_RX_PD_VLAN) {
+            vlan_tag = pkt_desc >> AX179A_RX_PD_VLAN_SHIFT;
+            __vlan_hwaccel_put_tag(ax_skb, htons(ETH_P_8021Q),
+                           vlan_tag & VLAN_VID_MASK);

Drop the mask and pass the complete vlan_tag, so priority is preserved?

Indeed. Will fix.

quoted

+    if (!dev->can_dma_sg && (dev->net->features & NETIF_F_SG) && 
skb_linearize(skb))
+        return NULL;

Call dev_kfree_skb_any() before returning, so the skb can't leak when 
skb_linearize() fails.

Will fix.

Birger