On Wed, 24 May 2023 16:01:50 -0700 Justin Chen wrote: > Add support for the Broadcom ASP 2.0 Ethernet controller which is first > introduced with 72165. This controller features two distinct Ethernet > ports that can be independently operated. > > This patch supports: > > - Wake-on-LAN using magic packets > - basic ethtool operations (link, counters, message level) > - MAC destination address filtering (promiscuous, ALL_MULTI, etc.) > +static netdev_tx_t bcmasp_xmit(struct sk_buff *skb, struct net_device *dev) > +{ > + struct bcmasp_intf *intf = netdev_priv(dev); > + int spb_index, nr_frags, ret, i, j; > + unsigned int total_bytes, size; > + struct bcmasp_tx_cb *txcb; > + dma_addr_t mapping, valid; > + struct bcmasp_desc *desc; > + bool csum_hw = false; > + struct device *kdev; > + skb_frag_t *frag; > + > + kdev = &intf->parent->pdev->dev; > + > + spin_lock(&intf->tx_lock); What is the tx_lock for? netdevs already have a tx lock, unless you declare the device as lockless. > +static void bcmasp_tx_timeout(struct net_device *dev, unsigned int txqueue) > +{ > + struct bcmasp_intf *intf = netdev_priv(dev); > + > + netif_dbg(intf, tx_err, dev, "transmit timeout!\n"); > + > + netif_trans_update(dev); > + dev->stats.tx_errors++; > + > + netif_wake_queue(dev); If the queue is full xmit will just put it back to sleep. You want to try to reap completions if anything, no? > +static struct net_device_stats *bcmasp_get_stats(struct net_device *dev) > +{ > + return &dev->stats; > +} you don't have to do this, core will use device stats if there's no ndo > + ndev = alloc_etherdev(sizeof(struct bcmasp_intf)); > + if (!dev) { *blink* condition is typo'ed > + dev_warn(dev, "%s: unable to alloc ndev\n", ndev_dn->name); > + goto err; > + } -- pw-bot: cr