From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 290D021CA02 for ; Sat, 25 Apr 2026 22:14:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777155258; cv=none; b=d3d8H0IVDe292dLxwdAjY64EwJjeN/WYvo7e2aTw7I+OZ3DCJMjD+BEHQuc6wiAEmF3nlCFgIIYVdb5+LUgosDWdmlrDYUAEHaBGuceKV3S2IQM0cVwnKLVuVR4BxpryLVHhC9+99xBjUZ5Rpey5tex6HB3jXVVAUCJZ3ImWlhw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777155258; c=relaxed/simple; bh=o6sZQetaTFeAvq6dRcJ5y7xUoiKA4pLaiHX6NPMPOYY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=fhhPO6athOFcLi/mf8ncNzJCgsg6IrWnCt+CVOKipsw/mIXnotoVVdrzLRDZ9mvQFBpHD5MAjASvPBEtCmrRSKeDRqw9zqnmL5HJPdLQ/bVkRzxaNoqQOkXoxg5t15CT9pZJbVIn+urOvPiph2882uur8pWfYYJF/g61vGj23Gk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Mae9EZux; arc=none smtp.client-ip=209.85.128.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Mae9EZux" Received: by mail-wm1-f50.google.com with SMTP id 5b1f17b1804b1-488e1a8ac40so110392505e9.2 for ; Sat, 25 Apr 2026 15:14:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1777155255; x=1777760055; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=XqgYsvBF+sUONTYrvoK5noelmDSfKskdUabgceS6bHU=; b=Mae9EZux7BL522choqhuAAEC15QEhUZEoHO6BfeDLWULZdxCMYeCFC1muxUklJUw1D PmTEX2xjae3NKIWmZbEDvxN5x0dvOWmykVvjzRlQPCufy1TYVUUpx8iey9wh8s2j/fIz qVqP3evEYIefszFKKrfzVJX87ouzx+aBSqyGdQn/RPg+ItvDMNpAroR1RXeRLx/u5pfH jh3eFQhYPG30/bnz1M2po5+OBO3WK0nyHDLQmjE4cx+2OPvkV4PsDhyt9vDj9ypEUGTf C4E/KQ4eJzwWQk7aEIioXng8b8ytzchIIbkclLFVybWjkPxYpA6n5SJzx8ta/AE6LRfL ZVgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777155255; x=1777760055; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=XqgYsvBF+sUONTYrvoK5noelmDSfKskdUabgceS6bHU=; b=VVVdas13qpZO3Dev/8n7PMotCFjlc0uVEHFx4fCHwHbwrfLeBplOWYnw0KUUctbFlB CXWiW5neDsJEryMkCT42UN34kTTw/I7MittJIzzJgAAuSOwjWl53J7WZ23ihc9TktvQd TsBbjPz5dOS9nAvku4b3eXeQxZlv+f5Nhl5uU6sPGYBkaguwL87FQ30XK0ZXpLNNR8Ni 2xORa10tBVCMxlNBhBq2GvYxJOJFY0E05lwQ4xxVIE/N4vjywxyMafoXrWKlUqsJWKh/ cFP7cevKa26yLJy+JSr0Pf2EOO6A0CC7red/3ieYYskJn+a98N/QpNdGG4uu2qn0Z4iK 0Olg== X-Forwarded-Encrypted: i=1; AFNElJ9IVQB3U/Z4OOS7k4mcYnCMM7FQTIqj603yoWnHRA3IZkMm7IFf9HYPnL0fSJh7GCWlvElltq0IVfiHzy8=@vger.kernel.org X-Gm-Message-State: AOJu0Yyd67NNx/MXDtKTybetpcfK5FQkDu3sh14EgDwrxgkkwT+4TzL4 0HKMOMEO5pHSKrwGcGNfOcu8M+alRJzg4NYuOi6Gvk4UGvuNNIgRhgOqC0IM2Q== X-Gm-Gg: AeBDiesanlMFebl5JTAqCwdJQm2iJZoY4xIpWhE92yV+5hn3ZwC3SghtzFS0hUEOoVr 1tL0dAsX83YL+MyejcO6W2AgImdFh5IbsRZPDf5mJS/YkgvwNHQc8Hnax5QjWAVtn11fUvPxFtx Dqjy41Y05UjulsirLSkUuyJUwFMKTpyKLZw1FpLe5QggyRNnPkpbRscOTCIqnupLL6b0skL9DVc J5h1GKgo5NU66ndi+c/k6H80VWQBojMIizvSCZ78ACzVNVPH5zJUuX/AELl/DiLiA70foRqLY2F j4oLuHIb6PZawz9exKpNdcdEtlbMFE57ViDPZXEgGxZNIfGdhTIVKbQxDearZ66aIfGuh0OsPT0 QfwhkBhmStDEbcfCYO6ROfTpuk50efJuNg1buDNi0L7Hd4jHitrhra4oBlreNQGgXeh0vBki8J4 KKd7HEW8rXpYb045JbApyibQ7XWbtplZaA+Amr79LTzGEMaJjxbY0S7r5E+zigpEu6QX3tJUnAs WPgjW0728RT4+lcGbwj1J6w+QmZ8/yVMik7xYdAEesbcxgqtgsenJwt7+Bdp4tJrtbk4SAY0P8L X-Received: by 2002:a05:600c:a408:b0:48a:592c:e632 with SMTP id 5b1f17b1804b1-48a592ce943mr194133525e9.16.1777155255271; Sat, 25 Apr 2026 15:14:15 -0700 (PDT) Received: from ?IPV6:2003:ea:8f34:b800:6020:70f3:48c1:ca3e? (p200300ea8f34b800602070f348c1ca3e.dip0.t-ipconnect.de. [2003:ea:8f34:b800:6020:70f3:48c1:ca3e]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-488fc14a61asm626712145e9.15.2026.04.25.15.14.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 25 Apr 2026 15:14:14 -0700 (PDT) Message-ID: <6345109a-3e87-476a-9abe-b8828e0fe9b6@gmail.com> Date: Sun, 26 Apr 2026 00:14:14 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC Patch net-next v1 5/9] r8169: add support for msix To: javen , nic_swsd@realtek.com, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, horms@kernel.org Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <20260420021957.1756-1-javen_xu@realsil.com.cn> <20260420021957.1756-6-javen_xu@realsil.com.cn> Content-Language: en-US From: Heiner Kallweit In-Reply-To: <20260420021957.1756-6-javen_xu@realsil.com.cn> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 20.04.2026 04:19, javen wrote: > From: Javen Xu > > This patch add support for msix. But we still use MSI here. And we force > nvecs to 1. We will modify it in rss patch. This description is wrong. Also as of today r8169 supports MSIX. Reason likely is that you're copying code from vendor driver. > > Signed-off-by: Javen Xu > --- > drivers/net/ethernet/realtek/r8169_main.c | 162 ++++++++++++++++++++-- > 1 file changed, 151 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c > index 52e690eba644..7d493342ab4b 100644 > --- a/drivers/net/ethernet/realtek/r8169_main.c > +++ b/drivers/net/ethernet/realtek/r8169_main.c > @@ -1764,26 +1764,40 @@ static u32 rtl_get_events(struct rtl8169_private *tp) > > static void rtl_ack_events(struct rtl8169_private *tp, u32 bits) > { > - if (rtl_is_8125(tp)) > + if (rtl_is_8125(tp)) { > RTL_W32(tp, IntrStatus_8125, bits); > - else > + if (tp->features & RTL_FEATURE_MSIX) { > + RTL_W32(tp, ISR_V2_8125, 0xffffffff); > + RTL_W32(tp, ISR_V4_L2_8125, 0xffffffff); > + } > + } else { > RTL_W16(tp, IntrStatus, bits); > + } > } > > static void rtl_irq_disable(struct rtl8169_private *tp) > { > - if (rtl_is_8125(tp)) > + if (rtl_is_8125(tp)) { > RTL_W32(tp, IntrMask_8125, 0); > - else > + if (tp->features & RTL_FEATURE_MSIX) { > + RTL_W32(tp, IMR_V2_CLEAR_REG_8125, 0xffffffff); > + RTL_W32(tp, IMR_V4_L2_CLEAR_REG_8125, 0xffffffff); > + } > + } else { > RTL_W16(tp, IntrMask, 0); > + } > } > > static void rtl_irq_enable(struct rtl8169_private *tp) > { > - if (rtl_is_8125(tp)) > - RTL_W32(tp, IntrMask_8125, tp->irq_mask); > - else > + if (rtl_is_8125(tp)) { > + if (tp->features & RTL_FEATURE_MSIX) > + RTL_W32(tp, IMR_V2_SET_REG_8125, tp->irq_mask); > + else > + RTL_W32(tp, IntrMask_8125, tp->irq_mask); > + } else { > RTL_W16(tp, IntrMask, tp->irq_mask); > + } > } > > static void rtl8169_irq_mask_and_ack(struct rtl8169_private *tp) > @@ -2894,6 +2908,10 @@ static void rtl_software_parameter_initialize(struct rtl8169_private *tp) > tp->InitRxDescType = RX_DESC_RING_TYPE_DEAFULT; > tp->HwCurrIsrVer = tp->HwSuppIsrVer; > > + /* This just force nvecs, and will be remove in the following patch*/ > + tp->min_irq_nvecs = 1; > + tp->max_irq_nvecs = 1; > + > rtl_setup_mqs_reg(tp); > rtl_set_ring_size(tp, NUM_RX_DESC, NUM_TX_DESC); > } > @@ -5321,6 +5339,44 @@ static void rtl8169_free_irq(struct rtl8169_private *tp) > } > } > > +static void rtl8169_disable_hw_interrupt_msix(struct rtl8169_private *tp, int message_id) > +{ > + RTL_W32(tp, IMR_V2_CLEAR_REG_8125, BIT(message_id)); > +} > + > +static void rtl8169_clear_hw_isr(struct rtl8169_private *tp, int message_id) > +{ > + RTL_W32(tp, ISR_V2_8125, BIT(message_id)); > +} > + > +static void rtl8169_enable_hw_interrupt_msix(struct rtl8169_private *tp, int message_id) > +{ > + RTL_W32(tp, IMR_V2_SET_REG_8125, BIT(message_id)); > +} > + > +static irqreturn_t rtl8169_interrupt_msix(int irq, void *dev_instance) > +{ > + struct rtl8169_napi *napi = dev_instance; > + struct rtl8169_private *tp = napi->priv; > + int message_id = napi->index; > + > + rtl8169_disable_hw_interrupt_msix(tp, message_id); > + > + rtl8169_clear_hw_isr(tp, message_id); > + > + if (message_id == MSIX_ID_V4_LINKCHG) { > + phy_mac_interrupt(tp->phydev); > + rtl8169_enable_hw_interrupt_msix(tp, message_id); > + return IRQ_HANDLED; > + } > + > + tp->recheck_desc_ownbit = true; > + > + napi_schedule(&napi->napi); > + > + return IRQ_HANDLED; > +} > + > static int rtl8169_request_irq(struct rtl8169_private *tp) > { > struct net_device *dev = tp->dev; > @@ -5331,10 +5387,14 @@ static int rtl8169_request_irq(struct rtl8169_private *tp) > > for (int i = 0; i < tp->irq_nvecs; i++) { > irq = &tp->irq_tbl[i]; > + if (tp->features & RTL_FEATURE_MSIX && tp->HwCurrIsrVer > 1) > + irq->handler = rtl8169_interrupt_msix; > + else > + irq->handler = rtl8169_interrupt; > > napi = &tp->r8169napi[i]; > snprintf(irq->name, len, "%s-%d", dev->name, i); > - rc = pci_request_irq(tp->pci_dev, i, rtl8169_interrupt, NULL, napi, irq->name); > + rc = pci_request_irq(tp->pci_dev, i, irq->handler, NULL, napi, irq->name); > > if (rc) > break; > @@ -5786,10 +5846,18 @@ static const struct net_device_ops rtl_netdev_ops = { > > static void rtl_set_irq_mask(struct rtl8169_private *tp) > { > - tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg; > + if (tp->features & RTL_FEATURE_MSIX) { > + tp->irq_mask = ISRIMR_V6_LINKCHG; > + for (int i = 0; i < tp->num_tx_rings; i++) > + tp->irq_mask |= ISRIMR_V6_TOK_Q0 << i; > + for (int i = 0; i < tp->num_rx_rings; i++) > + tp->irq_mask |= ISRIMR_V6_ROK_Q0 << i; > + } else { > + tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg; > > - if (tp->mac_version <= RTL_GIGA_MAC_VER_06) > - tp->irq_mask |= SYSErr | RxFIFOOver; > + if (tp->mac_version <= RTL_GIGA_MAC_VER_06) > + tp->irq_mask |= SYSErr | RxFIFOOver; > + } > } > > static int rtl_alloc_irq(struct rtl8169_private *tp) > @@ -5817,6 +5885,18 @@ static int rtl_alloc_irq(struct rtl8169_private *tp) > if (nvecs < 0) > nvecs = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES); > > + tp->features &= ~(RTL_FEATURE_MSIX | RTL_FEATURE_MSI); > + > + if (nvecs > 0) { > + tp->irq_nvecs = nvecs; > + tp->irq = pci_irq_vector(pdev, 0); > + if (nvecs > 1) > + tp->features |= RTL_FEATURE_MSIX; > + else if (pci_dev_msi_enabled(pdev)) > + tp->features |= RTL_FEATURE_MSI; > + return 0; Such feature flags, especially for MSI/MSIX, are ugly. Why don't you leave the interrupt type selection to PCI core? This needs at least an explanation. > + } > + > tp->irq = pdev->irq; > tp->irq_nvecs = 1; > > @@ -6087,6 +6167,52 @@ static bool rtl_aspm_is_safe(struct rtl8169_private *tp) > return false; > } > > +static int rtl8169_poll_msix_rx(struct napi_struct *napi, int budget) > +{ > + struct rtl8169_napi *r8169_napi = container_of(napi, struct rtl8169_napi, napi); > + struct rtl8169_private *tp = r8169_napi->priv; > + struct net_device *dev = tp->dev; > + const int message_id = r8169_napi->index; > + int work_done = 0; > + > + if (message_id < tp->num_rx_rings) > + work_done += rtl_rx(dev, tp, &tp->rx_ring[message_id], budget); > + > + if (work_done < budget && napi_complete_done(napi, work_done)) > + rtl8169_enable_hw_interrupt_msix(tp, message_id); > + > + return work_done; > +} > + > +static int rtl8169_poll_msix_tx(struct napi_struct *napi, int budget) > +{ > + struct rtl8169_napi *r8169_napi = container_of(napi, struct rtl8169_napi, napi); > + struct rtl8169_private *tp = r8169_napi->priv; > + struct net_device *dev = tp->dev; > + unsigned int work_done = 0; > + const int message_id = r8169_napi->index; > + int tx_ring_idx = message_id - 8; > + > + if (tx_ring_idx >= 0 && tx_ring_idx < tp->num_tx_rings) > + work_done += rtl_tx(dev, tp, &tp->tx_ring[tx_ring_idx], budget); > + > + if (work_done < budget && napi_complete_done(napi, work_done)) > + rtl8169_enable_hw_interrupt_msix(tp, message_id); > + > + return work_done; > +} > + > +static int rtl8169_poll_msix_other(struct napi_struct *napi, int budget) > +{ > + struct rtl8169_napi *r8169_napi = container_of(napi, struct rtl8169_napi, napi); > + struct rtl8169_private *tp = r8169_napi->priv; > + const int message_id = r8169_napi->index; > + > + napi_complete_done(napi, budget); > + rtl8169_enable_hw_interrupt_msix(tp, message_id); > + > + return 1; > +} > > static void r8169_init_napi(struct rtl8169_private *tp) > { > @@ -6095,6 +6221,20 @@ static void r8169_init_napi(struct rtl8169_private *tp) > int (*poll)(struct napi_struct *napi, int budget); > > poll = rtl8169_poll; > + if (tp->features & RTL_FEATURE_MSIX) { > + switch (tp->HwCurrIsrVer) { > + case 6: > + if (i < R8127_MAX_RX_QUEUES) > + poll = rtl8169_poll_msix_rx; > + else if (i > 7 && i < 16) > + poll = rtl8169_poll_msix_tx; > + else > + poll = rtl8169_poll_msix_other; > + break; > + default: > + break; > + } > + } > netif_napi_add(tp->dev, &r8169napi->napi, poll); > r8169napi->priv = tp; > r8169napi->index = i;