Messages in this thread | | | Date | Mon, 16 May 2022 04:51:03 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH net-next v2 1/5] net: ipqess: introduce the Qualcomm IPQESS driver |
| |
> +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
| |