> +static int ipqess_tx_ring_alloc(struct ipqess *ess) > +{ > + struct device *dev = &ess->pdev->dev; > + int i; > + > + for (i = 0; i < IPQESS_NETDEV_QUEUES; i++) { > + struct ipqess_tx_ring *tx_ring = &ess->tx_ring[i]; > + size_t size; > + u32 idx; > + > + tx_ring->ess = ess; > + tx_ring->ring_id = i; > + tx_ring->idx = i * 4; > + tx_ring->count = IPQESS_TX_RING_SIZE; > + tx_ring->nq = netdev_get_tx_queue(ess->netdev, i); > + > + size = sizeof(struct ipqess_buf) * IPQESS_TX_RING_SIZE; > + tx_ring->buf = devm_kzalloc(dev, size, GFP_KERNEL); > + if (!tx_ring->buf) { > + netdev_err(ess->netdev, "buffer alloc of tx ring failed"); > + return -ENOMEM; > + } kzalloc() is pretty loud when there is no memory. So you see patches removing such warning messages. > +static int ipqess_rx_napi(struct napi_struct *napi, int budget) > +{ > + struct ipqess_rx_ring *rx_ring = container_of(napi, struct ipqess_rx_ring, > + napi_rx); > + struct ipqess *ess = rx_ring->ess; > + u32 rx_mask = BIT(rx_ring->idx); > + int remain_budget = budget; > + int rx_done; > + u32 status; > + > +poll_again: > + ipqess_w32(ess, IPQESS_REG_RX_ISR, rx_mask); > + rx_done = ipqess_rx_poll(rx_ring, remain_budget); > + > + if (rx_done == remain_budget) > + return budget; > + > + status = ipqess_r32(ess, IPQESS_REG_RX_ISR); > + if (status & rx_mask) { > + remain_budget -= rx_done; > + goto poll_again; > + } Could this be turned into a do while() loop? > +static void ipqess_irq_enable(struct ipqess *ess) > +{ > + int i; > + > + ipqess_w32(ess, IPQESS_REG_RX_ISR, 0xff); > + ipqess_w32(ess, IPQESS_REG_TX_ISR, 0xffff); > + for (i = 0; i < IPQESS_NETDEV_QUEUES; i++) { > + ipqess_w32(ess, IPQESS_REG_RX_INT_MASK_Q(ess->rx_ring[i].idx), 1); > + ipqess_w32(ess, IPQESS_REG_TX_INT_MASK_Q(ess->tx_ring[i].idx), 1); > + } > +} > + > +static void ipqess_irq_disable(struct ipqess *ess) > +{ > + int i; > + > + for (i = 0; i < IPQESS_NETDEV_QUEUES; i++) { > + ipqess_w32(ess, IPQESS_REG_RX_INT_MASK_Q(ess->rx_ring[i].idx), 0); > + ipqess_w32(ess, IPQESS_REG_TX_INT_MASK_Q(ess->tx_ring[i].idx), 0); > + } > +} Enable and disable are not symmetric? > +static inline void ipqess_kick_tx(struct ipqess_tx_ring *tx_ring) No inline functions please in .c files. Let the compiler decide. Andrew