Re: [PATCH net-next v3 09/10] net: ethernet: mtk-star-emac: separate tx/rx handling with two NAPIs
From: Jakub Kicinski <kuba@kernel.org>
Date: 2022-06-24 04:37:08
Also in:
linux-devicetree, linux-mediatek, lkml, netdev
On Wed, 22 Jun 2022 17:05:44 +0800 Biao Huang wrote:
+ if (rx || tx) {
+ spin_lock_irqsave(&priv->lock, flags);
+ /* mask Rx and TX Complete interrupt */
+ mtk_star_disable_dma_irq(priv, rx, tx);
+ spin_unlock_irqrestore(&priv->lock, flags);You do _irqsave / _irqrestore here
+ if (rx) + __napi_schedule_irqoff(&priv->rx_napi); + if (tx) + __napi_schedule_irqoff(&priv->tx_napi);
Yet assume _irqoff here. So can this be run from non-IRQ context or not?
- if (mtk_star_ring_full(ring)) + if (unlikely(mtk_star_tx_ring_avail(ring) < MAX_SKB_FRAGS + 1)) netif_stop_queue(ndev);
Please look around other drivers (like ixgbe) and copy the way they handle safe stopping of the queues. You need to add some barriers and re-check after disabling.
- spin_unlock_bh(&priv->lock); - mtk_star_dma_resume_tx(priv); return NETDEV_TX_OK;
+ while ((entry != head) && (count < MTK_STAR_RING_NUM_DESCS - 1)) {
Parenthesis unnecessary, so is the empty line after the while ().
ret = mtk_star_tx_complete_one(priv); if (ret < 0) break; + + count++; + pkts_compl++; + bytes_compl += ret; + entry = ring->tail; } + __netif_tx_lock_bh(netdev_get_tx_queue(priv->ndev, 0)); netdev_completed_queue(ndev, pkts_compl, bytes_compl); + __netif_tx_unlock_bh(netdev_get_tx_queue(priv->ndev, 0));
what are you taking this lock for?
- if (wake && netif_queue_stopped(ndev))
+ if (unlikely(netif_queue_stopped(ndev)) &&
+ (mtk_star_tx_ring_avail(ring) > MTK_STAR_TX_THRESH))
netif_wake_queue(ndev);
- spin_unlock(&priv->lock);
+ if (napi_complete(napi)) {
+ spin_lock_irqsave(&priv->lock, flags);
+ mtk_star_enable_dma_irq(priv, false, true);
+ spin_unlock_irqrestore(&priv->lock, flags);
+ }
+
+ return 0;
}quoted hunk ↗ jump to hunk
@@ -1475,6 +1514,7 @@ static int mtk_star_set_timing(struct mtk_star_priv *priv) return regmap_write(priv->regs, MTK_STAR_REG_TEST0, delay_val); } + static int mtk_star_probe(struct platform_device *pdev) { struct device_node *of_node;
spurious whitespace change _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel