From nobody Sat Sep 21 23:28:23 2024 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B8C61C433EF for ; Wed, 29 Jun 2022 03:19:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231237AbiF2DTE (ORCPT ); Tue, 28 Jun 2022 23:19:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45074 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230489AbiF2DSk (ORCPT ); Tue, 28 Jun 2022 23:18:40 -0400 Received: from mailgw02.mediatek.com (unknown [210.61.82.184]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 73E98FD3C; Tue, 28 Jun 2022 20:18:12 -0700 (PDT) X-UUID: 47b5714ed8d743e8a04a6a3d58ccf645-20220629 X-CID-P-RULE: Release_Ham X-CID-O-INFO: VERSION:1.1.7,REQID:01188897-4b2c-481c-94ca-093fad74f652,OB:0,LO B:0,IP:0,URL:0,TC:0,Content:0,EDM:0,RT:0,SF:0,FILE:0,RULE:Release_Ham,ACTI ON:release,TS:0 X-CID-META: VersionHash:87442a2,CLOUDID:b8ace862-0b3f-4b2c-b3a6-ed5c044366a0,C OID:IGNORED,Recheck:0,SF:nil,TC:nil,Content:0,EDM:-3,IP:nil,URL:0,File:nil ,QS:nil,BEC:nil,COL:0 X-UUID: 47b5714ed8d743e8a04a6a3d58ccf645-20220629 Received: from mtkmbs10n2.mediatek.inc [(172.21.101.183)] by mailgw02.mediatek.com (envelope-from ) (Generic MTA with TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384 256/256) with ESMTP id 13461248; Wed, 29 Jun 2022 11:18:08 +0800 Received: from mtkmbs11n2.mediatek.inc (172.21.101.187) by mtkmbs11n1.mediatek.inc (172.21.101.185) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.792.3; Wed, 29 Jun 2022 11:18:06 +0800 Received: from localhost.localdomain (10.17.3.154) by mtkmbs11n2.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.2.792.3 via Frontend Transport; Wed, 29 Jun 2022 11:18:04 +0800 From: Biao Huang To: David Miller , Jakub Kicinski , Rob Herring , Bartosz Golaszewski , Fabien Parent CC: Felix Fietkau , John Crispin , Sean Wang , Mark Lee , Matthias Brugger , , , , , , Biao Huang , Yinghua Pan , Macpaul Lin Subject: [PATCH net-next v5 09/10] net: ethernet: mtk-star-emac: separate tx/rx handling with two NAPIs Date: Wed, 29 Jun 2022 11:17:42 +0800 Message-ID: <20220629031743.22115-10-biao.huang@mediatek.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220629031743.22115-1-biao.huang@mediatek.com> References: <20220629031743.22115-1-biao.huang@mediatek.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-MTK: N Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Current driver may lost tx interrupts under bidirectional test with iperf3, which leads to some unexpected issues. This patch let rx/tx interrupt enable/disable separately, and rx/tx are handled in different NAPIs. Signed-off-by: Biao Huang Signed-off-by: Yinghua Pan --- drivers/net/ethernet/mediatek/mtk_star_emac.c | 340 ++++++++++-------- 1 file changed, 199 insertions(+), 141 deletions(-) diff --git a/drivers/net/ethernet/mediatek/mtk_star_emac.c b/drivers/net/et= hernet/mediatek/mtk_star_emac.c index a1165f293494..87c6c9bc221d 100644 --- a/drivers/net/ethernet/mediatek/mtk_star_emac.c +++ b/drivers/net/ethernet/mediatek/mtk_star_emac.c @@ -33,6 +33,7 @@ #define MTK_STAR_SKB_ALIGNMENT 16 #define MTK_STAR_HASHTABLE_MC_LIMIT 256 #define MTK_STAR_HASHTABLE_SIZE_MAX 512 +#define MTK_STAR_DESC_NEEDED (MAX_SKB_FRAGS + 4) =20 /* Normally we'd use NET_IP_ALIGN but on arm64 its value is 0 and it doesn= 't * work for this controller. @@ -228,7 +229,8 @@ struct mtk_star_ring_desc_data { struct sk_buff *skb; }; =20 -#define MTK_STAR_RING_NUM_DESCS 128 +#define MTK_STAR_RING_NUM_DESCS 512 +#define MTK_STAR_TX_THRESH (MTK_STAR_RING_NUM_DESCS / 4) #define MTK_STAR_NUM_TX_DESCS MTK_STAR_RING_NUM_DESCS #define MTK_STAR_NUM_RX_DESCS MTK_STAR_RING_NUM_DESCS #define MTK_STAR_NUM_DESCS_TOTAL (MTK_STAR_RING_NUM_DESCS * 2) @@ -263,7 +265,8 @@ struct mtk_star_priv { struct mtk_star_ring rx_ring; =20 struct mii_bus *mii; - struct napi_struct napi; + struct napi_struct tx_napi; + struct napi_struct rx_napi; =20 struct device_node *phy_node; phy_interface_t phy_intf; @@ -379,19 +382,16 @@ mtk_star_ring_push_head_tx(struct mtk_star_ring *ring, mtk_star_ring_push_head(ring, desc_data, flags); } =20 -static unsigned int mtk_star_ring_num_used_descs(struct mtk_star_ring *rin= g) +static unsigned int mtk_star_tx_ring_avail(struct mtk_star_ring *ring) { - return abs(ring->head - ring->tail); -} + u32 avail; =20 -static bool mtk_star_ring_full(struct mtk_star_ring *ring) -{ - return mtk_star_ring_num_used_descs(ring) =3D=3D MTK_STAR_RING_NUM_DESCS; -} + if (ring->tail > ring->head) + avail =3D ring->tail - ring->head - 1; + else + avail =3D MTK_STAR_RING_NUM_DESCS - ring->head + ring->tail - 1; =20 -static bool mtk_star_ring_descs_available(struct mtk_star_ring *ring) -{ - return mtk_star_ring_num_used_descs(ring) > 0; + return avail; } =20 static dma_addr_t mtk_star_dma_map_rx(struct mtk_star_priv *priv, @@ -436,6 +436,36 @@ static void mtk_star_nic_disable_pd(struct mtk_star_pr= iv *priv) MTK_STAR_BIT_MAC_CFG_NIC_PD); } =20 +static void mtk_star_enable_dma_irq(struct mtk_star_priv *priv, + bool rx, bool tx) +{ + u32 value; + + regmap_read(priv->regs, MTK_STAR_REG_INT_MASK, &value); + + if (tx) + value &=3D ~MTK_STAR_BIT_INT_STS_TNTC; + if (rx) + value &=3D ~MTK_STAR_BIT_INT_STS_FNRC; + + regmap_write(priv->regs, MTK_STAR_REG_INT_MASK, value); +} + +static void mtk_star_disable_dma_irq(struct mtk_star_priv *priv, + bool rx, bool tx) +{ + u32 value; + + regmap_read(priv->regs, MTK_STAR_REG_INT_MASK, &value); + + if (tx) + value |=3D MTK_STAR_BIT_INT_STS_TNTC; + if (rx) + value |=3D MTK_STAR_BIT_INT_STS_FNRC; + + regmap_write(priv->regs, MTK_STAR_REG_INT_MASK, value); +} + /* Unmask the three interrupts we care about, mask all others. */ static void mtk_star_intr_enable(struct mtk_star_priv *priv) { @@ -451,20 +481,11 @@ static void mtk_star_intr_disable(struct mtk_star_pri= v *priv) regmap_write(priv->regs, MTK_STAR_REG_INT_MASK, ~0); } =20 -static unsigned int mtk_star_intr_read(struct mtk_star_priv *priv) -{ - unsigned int val; - - regmap_read(priv->regs, MTK_STAR_REG_INT_STS, &val); - - return val; -} - static unsigned int mtk_star_intr_ack_all(struct mtk_star_priv *priv) { unsigned int val; =20 - val =3D mtk_star_intr_read(priv); + regmap_read(priv->regs, MTK_STAR_REG_INT_STS, &val); regmap_write(priv->regs, MTK_STAR_REG_INT_STS, val); =20 return val; @@ -736,25 +757,44 @@ static void mtk_star_free_tx_skbs(struct mtk_star_pri= v *priv) mtk_star_ring_free_skbs(priv, ring, mtk_star_dma_unmap_tx); } =20 -/* All processing for TX and RX happens in the napi poll callback. - * - * FIXME: The interrupt handling should be more fine-grained with each - * interrupt enabled/disabled independently when needed. Unfortunatly this - * turned out to impact the driver's stability and until we have something - * working properly, we're disabling all interrupts during TX & RX process= ing - * or when resetting the counter registers. - */ +/** + * mtk_star_handle_irq - Interrupt Handler. + * @irq: interrupt number. + * @data: pointer to a network interface device structure. + * Description : this is the driver interrupt service routine. + * it mainly handles: + * 1. tx complete interrupt for frame transmission. + * 2. rx complete interrupt for frame reception. + * 3. MAC Management Counter interrupt to avoid counter overflow. + **/ static irqreturn_t mtk_star_handle_irq(int irq, void *data) { - struct mtk_star_priv *priv; - struct net_device *ndev; - - ndev =3D data; - priv =3D netdev_priv(ndev); + struct net_device *ndev =3D data; + struct mtk_star_priv *priv =3D netdev_priv(ndev); + unsigned int intr_status =3D mtk_star_intr_ack_all(priv); + bool rx, tx; + + rx =3D (intr_status & MTK_STAR_BIT_INT_STS_FNRC) && + napi_schedule_prep(&priv->rx_napi); + tx =3D (intr_status & MTK_STAR_BIT_INT_STS_TNTC) && + napi_schedule_prep(&priv->tx_napi); + + if (rx || tx) { + spin_lock(&priv->lock); + /* mask Rx and TX Complete interrupt */ + mtk_star_disable_dma_irq(priv, rx, tx); + spin_unlock(&priv->lock); + + if (rx) + __napi_schedule(&priv->rx_napi); + if (tx) + __napi_schedule(&priv->tx_napi); + } =20 - if (netif_running(ndev)) { - mtk_star_intr_disable(priv); - napi_schedule(&priv->napi); + /* interrupt is triggered once any counters reach 0x8000000 */ + if (intr_status & MTK_STAR_REG_INT_STS_MIB_CNT_TH) { + mtk_star_update_stats(priv); + mtk_star_reset_counters(priv); } =20 return IRQ_HANDLED; @@ -970,7 +1010,8 @@ static int mtk_star_enable(struct net_device *ndev) if (ret) goto err_free_skbs; =20 - napi_enable(&priv->napi); + napi_enable(&priv->tx_napi); + napi_enable(&priv->rx_napi); =20 mtk_star_intr_ack_all(priv); mtk_star_intr_enable(priv); @@ -1003,7 +1044,8 @@ static void mtk_star_disable(struct net_device *ndev) struct mtk_star_priv *priv =3D netdev_priv(ndev); =20 netif_stop_queue(ndev); - napi_disable(&priv->napi); + napi_disable(&priv->tx_napi); + napi_disable(&priv->rx_napi); mtk_star_intr_disable(priv); mtk_star_dma_disable(priv); mtk_star_intr_ack_all(priv); @@ -1035,13 +1077,45 @@ static int mtk_star_netdev_ioctl(struct net_device = *ndev, return phy_mii_ioctl(ndev->phydev, req, cmd); } =20 -static int mtk_star_netdev_start_xmit(struct sk_buff *skb, - struct net_device *ndev) +static int __mtk_star_maybe_stop_tx(struct mtk_star_priv *priv, u16 size) +{ + netif_stop_queue(priv->ndev); + + /* Might race with mtk_star_tx_poll, check again */ + smp_mb(); + if (likely(mtk_star_tx_ring_avail(&priv->tx_ring) < size)) + return -EBUSY; + + netif_start_queue(priv->ndev); + + return 0; +} + +static inline int mtk_star_maybe_stop_tx(struct mtk_star_priv *priv, u16 s= ize) +{ + if (likely(mtk_star_tx_ring_avail(&priv->tx_ring) >=3D size)) + return 0; + + return __mtk_star_maybe_stop_tx(priv, size); +} + +static netdev_tx_t mtk_star_netdev_start_xmit(struct sk_buff *skb, + struct net_device *ndev) { struct mtk_star_priv *priv =3D netdev_priv(ndev); struct mtk_star_ring *ring =3D &priv->tx_ring; struct device *dev =3D mtk_star_get_dev(priv); struct mtk_star_ring_desc_data desc_data; + int nfrags =3D skb_shinfo(skb)->nr_frags; + + if (unlikely(mtk_star_tx_ring_avail(ring) < nfrags + 1)) { + if (!netif_queue_stopped(ndev)) { + netif_stop_queue(ndev); + /* This is a hard error, log it. */ + pr_err_ratelimited("Tx ring full when queue awake\n"); + } + return NETDEV_TX_BUSY; + } =20 desc_data.dma_addr =3D mtk_star_dma_map_tx(priv, skb); if (dma_mapping_error(dev, desc_data.dma_addr)) @@ -1049,17 +1123,11 @@ static int mtk_star_netdev_start_xmit(struct sk_buf= f *skb, =20 desc_data.skb =3D skb; desc_data.len =3D skb->len; - - spin_lock_bh(&priv->lock); - mtk_star_ring_push_head_tx(ring, &desc_data); =20 netdev_sent_queue(ndev, skb->len); =20 - if (mtk_star_ring_full(ring)) - netif_stop_queue(ndev); - - spin_unlock_bh(&priv->lock); + mtk_star_maybe_stop_tx(priv, MTK_STAR_DESC_NEEDED); =20 mtk_star_dma_resume_tx(priv); =20 @@ -1091,31 +1159,40 @@ static int mtk_star_tx_complete_one(struct mtk_star= _priv *priv) return ret; } =20 -static void mtk_star_tx_complete_all(struct mtk_star_priv *priv) +static int mtk_star_tx_poll(struct napi_struct *napi, int budget) { + struct mtk_star_priv *priv =3D container_of(napi, struct mtk_star_priv, + tx_napi); + int ret =3D 0, pkts_compl =3D 0, bytes_compl =3D 0, count =3D 0; struct mtk_star_ring *ring =3D &priv->tx_ring; struct net_device *ndev =3D priv->ndev; - int ret, pkts_compl, bytes_compl; - bool wake =3D false; - - spin_lock(&priv->lock); - - for (pkts_compl =3D 0, bytes_compl =3D 0;; - pkts_compl++, bytes_compl +=3D ret, wake =3D true) { - if (!mtk_star_ring_descs_available(ring)) - break; + unsigned int head =3D ring->head; + unsigned int entry =3D ring->tail; =20 + while (entry !=3D head && count < (MTK_STAR_RING_NUM_DESCS - 1)) { ret =3D mtk_star_tx_complete_one(priv); if (ret < 0) break; + + count++; + pkts_compl++; + bytes_compl +=3D ret; + entry =3D ring->tail; } =20 netdev_completed_queue(ndev, pkts_compl, bytes_compl); =20 - 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); =20 - spin_unlock(&priv->lock); + if (napi_complete(napi)) { + spin_lock(&priv->lock); + mtk_star_enable_dma_irq(priv, false, true); + spin_unlock(&priv->lock); + } + + return 0; } =20 static void mtk_star_netdev_get_stats64(struct net_device *ndev, @@ -1195,7 +1272,7 @@ static const struct ethtool_ops mtk_star_ethtool_ops = =3D { .set_link_ksettings =3D phy_ethtool_set_link_ksettings, }; =20 -static int mtk_star_receive_packet(struct mtk_star_priv *priv) +static int mtk_star_rx(struct mtk_star_priv *priv, int budget) { struct mtk_star_ring *ring =3D &priv->rx_ring; struct device *dev =3D mtk_star_get_dev(priv); @@ -1203,107 +1280,85 @@ static int mtk_star_receive_packet(struct mtk_star= _priv *priv) struct net_device *ndev =3D priv->ndev; struct sk_buff *curr_skb, *new_skb; dma_addr_t new_dma_addr; - int ret; + int ret, count =3D 0; =20 - spin_lock(&priv->lock); - ret =3D mtk_star_ring_pop_tail(ring, &desc_data); - spin_unlock(&priv->lock); - if (ret) - return -1; + while (count < budget) { + ret =3D mtk_star_ring_pop_tail(ring, &desc_data); + if (ret) + return -1; =20 - curr_skb =3D desc_data.skb; + curr_skb =3D desc_data.skb; =20 - if ((desc_data.flags & MTK_STAR_DESC_BIT_RX_CRCE) || - (desc_data.flags & MTK_STAR_DESC_BIT_RX_OSIZE)) { - /* Error packet -> drop and reuse skb. */ - new_skb =3D curr_skb; - goto push_new_skb; - } + if ((desc_data.flags & MTK_STAR_DESC_BIT_RX_CRCE) || + (desc_data.flags & MTK_STAR_DESC_BIT_RX_OSIZE)) { + /* Error packet -> drop and reuse skb. */ + new_skb =3D curr_skb; + goto push_new_skb; + } =20 - /* Prepare new skb before receiving the current one. Reuse the current - * skb if we fail at any point. - */ - new_skb =3D mtk_star_alloc_skb(ndev); - if (!new_skb) { - ndev->stats.rx_dropped++; - new_skb =3D curr_skb; - goto push_new_skb; - } + /* Prepare new skb before receiving the current one. + * Reuse the current skb if we fail at any point. + */ + new_skb =3D mtk_star_alloc_skb(ndev); + if (!new_skb) { + ndev->stats.rx_dropped++; + new_skb =3D curr_skb; + goto push_new_skb; + } =20 - new_dma_addr =3D mtk_star_dma_map_rx(priv, new_skb); - if (dma_mapping_error(dev, new_dma_addr)) { - ndev->stats.rx_dropped++; - dev_kfree_skb(new_skb); - new_skb =3D curr_skb; - netdev_err(ndev, "DMA mapping error of RX descriptor\n"); - goto push_new_skb; - } + new_dma_addr =3D mtk_star_dma_map_rx(priv, new_skb); + if (dma_mapping_error(dev, new_dma_addr)) { + ndev->stats.rx_dropped++; + dev_kfree_skb(new_skb); + new_skb =3D curr_skb; + netdev_err(ndev, "DMA mapping error of RX descriptor\n"); + goto push_new_skb; + } =20 - /* We can't fail anymore at this point: it's safe to unmap the skb. */ - mtk_star_dma_unmap_rx(priv, &desc_data); + /* We can't fail anymore at this point: + * it's safe to unmap the skb. + */ + mtk_star_dma_unmap_rx(priv, &desc_data); =20 - skb_put(desc_data.skb, desc_data.len); - desc_data.skb->ip_summed =3D CHECKSUM_NONE; - desc_data.skb->protocol =3D eth_type_trans(desc_data.skb, ndev); - desc_data.skb->dev =3D ndev; - netif_receive_skb(desc_data.skb); + skb_put(desc_data.skb, desc_data.len); + desc_data.skb->ip_summed =3D CHECKSUM_NONE; + desc_data.skb->protocol =3D eth_type_trans(desc_data.skb, ndev); + desc_data.skb->dev =3D ndev; + netif_receive_skb(desc_data.skb); =20 - /* update dma_addr for new skb */ - desc_data.dma_addr =3D new_dma_addr; + /* update dma_addr for new skb */ + desc_data.dma_addr =3D new_dma_addr; =20 push_new_skb: - desc_data.len =3D skb_tailroom(new_skb); - desc_data.skb =3D new_skb; =20 - spin_lock(&priv->lock); - mtk_star_ring_push_head_rx(ring, &desc_data); - spin_unlock(&priv->lock); - - return 0; -} - -static int mtk_star_process_rx(struct mtk_star_priv *priv, int budget) -{ - int received, ret; + count++; =20 - for (received =3D 0, ret =3D 0; received < budget && ret =3D=3D 0; receiv= ed++) - ret =3D mtk_star_receive_packet(priv); + desc_data.len =3D skb_tailroom(new_skb); + desc_data.skb =3D new_skb; + mtk_star_ring_push_head_rx(ring, &desc_data); + } =20 mtk_star_dma_resume_rx(priv); =20 - return received; + return count; } =20 -static int mtk_star_poll(struct napi_struct *napi, int budget) +static int mtk_star_rx_poll(struct napi_struct *napi, int budget) { struct mtk_star_priv *priv; - unsigned int status; - int received =3D 0; - - priv =3D container_of(napi, struct mtk_star_priv, napi); - - status =3D mtk_star_intr_read(priv); - mtk_star_intr_ack_all(priv); - - if (status & MTK_STAR_BIT_INT_STS_TNTC) - /* Clean-up all TX descriptors. */ - mtk_star_tx_complete_all(priv); + int work_done =3D 0; =20 - if (status & MTK_STAR_BIT_INT_STS_FNRC) - /* Receive up to $budget packets. */ - received =3D mtk_star_process_rx(priv, budget); + priv =3D container_of(napi, struct mtk_star_priv, rx_napi); =20 - if (unlikely(status & MTK_STAR_REG_INT_STS_MIB_CNT_TH)) { - mtk_star_update_stats(priv); - mtk_star_reset_counters(priv); + work_done =3D mtk_star_rx(priv, budget); + if (work_done < budget) { + napi_complete_done(napi, work_done); + spin_lock(&priv->lock); + mtk_star_enable_dma_irq(priv, true, false); + spin_unlock(&priv->lock); } =20 - if (received < budget) - napi_complete_done(napi, received); - - mtk_star_intr_enable(priv); - - return received; + return work_done; } =20 static void mtk_star_mdio_rwok_clear(struct mtk_star_priv *priv) @@ -1602,7 +1657,10 @@ static int mtk_star_probe(struct platform_device *pd= ev) ndev->netdev_ops =3D &mtk_star_netdev_ops; ndev->ethtool_ops =3D &mtk_star_ethtool_ops; =20 - netif_napi_add(ndev, &priv->napi, mtk_star_poll, NAPI_POLL_WEIGHT); + netif_napi_add(ndev, &priv->rx_napi, mtk_star_rx_poll, + NAPI_POLL_WEIGHT); + netif_tx_napi_add(ndev, &priv->tx_napi, mtk_star_tx_poll, + NAPI_POLL_WEIGHT); =20 return devm_register_netdev(dev, ndev); } --=20 2.25.1