| Message ID | 20240626060703.31652-2-jiawenwu@trustnetic.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: txgbe: fix MSI and INTx interrupts | expand |
On Wed, 26 Jun 2024 14:07:02 +0800 Jiawen Wu wrote: > When using MSI or INTx interrupts, request_irq() for pdev->irq will > conflict with request_threaded_irq() for txgbe->misc.irq, to cause > system crash. So remove txgbe_request_irq() for MSI/INTx case, and > rename txgbe_request_msix_irqs() since it only request for queue irqs. Do you have any users who need INTx support? Maybe you could drop the support and simplify the code? > Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller") > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com> > --- > drivers/net/ethernet/wangxun/libwx/wx_lib.c | 3 +- > .../net/ethernet/wangxun/txgbe/txgbe_irq.c | 78 ++----------------- > .../net/ethernet/wangxun/txgbe/txgbe_irq.h | 2 +- > .../net/ethernet/wangxun/txgbe/txgbe_main.c | 2 +- > 4 files changed, 10 insertions(+), 75 deletions(-) > > diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c > index 68bde91b67a0..99f55a3573c8 100644 > --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c > +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c > @@ -1996,7 +1996,8 @@ void wx_free_irq(struct wx *wx) > int vector; > > if (!(pdev->msix_enabled)) { > - free_irq(pdev->irq, wx); > + if (wx->mac.type == wx_mac_em) > + free_irq(pdev->irq, wx); It seems strange to match on type to decide whether to free an IRQ. Isn't there or shouldn't there be some IRQ related flag informing the library how to manage the IRQs?
On Fri, Jun 28, 2024 7:42 AM, Jakub Kicinski wrote: > On Wed, 26 Jun 2024 14:07:02 +0800 Jiawen Wu wrote: > > When using MSI or INTx interrupts, request_irq() for pdev->irq will > > conflict with request_threaded_irq() for txgbe->misc.irq, to cause > > system crash. So remove txgbe_request_irq() for MSI/INTx case, and > > rename txgbe_request_msix_irqs() since it only request for queue irqs. > > Do you have any users who need INTx support? Maybe you could drop > the support and simplify the code? Yes, some domestic platforms use it. > > > Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller") > > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com> > > --- > > drivers/net/ethernet/wangxun/libwx/wx_lib.c | 3 +- > > .../net/ethernet/wangxun/txgbe/txgbe_irq.c | 78 ++----------------- > > .../net/ethernet/wangxun/txgbe/txgbe_irq.h | 2 +- > > .../net/ethernet/wangxun/txgbe/txgbe_main.c | 2 +- > > 4 files changed, 10 insertions(+), 75 deletions(-) > > > > diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c > > index 68bde91b67a0..99f55a3573c8 100644 > > --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c > > +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c > > @@ -1996,7 +1996,8 @@ void wx_free_irq(struct wx *wx) > > int vector; > > > > if (!(pdev->msix_enabled)) { > > - free_irq(pdev->irq, wx); > > + if (wx->mac.type == wx_mac_em) > > + free_irq(pdev->irq, wx); > > It seems strange to match on type to decide whether to free an IRQ. > Isn't there or shouldn't there be some IRQ related flag informing > the library how to manage the IRQs? My intention is not to change the IRQ structure of ngbe driver. So it simply match the mac type. I would consider use a flag.
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c index 68bde91b67a0..99f55a3573c8 100644 --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c @@ -1996,7 +1996,8 @@ void wx_free_irq(struct wx *wx) int vector; if (!(pdev->msix_enabled)) { - free_irq(pdev->irq, wx); + if (wx->mac.type == wx_mac_em) + free_irq(pdev->irq, wx); return; } diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c index b3e3605d1edb..ac789ec0091a 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c @@ -27,57 +27,19 @@ void txgbe_irq_enable(struct wx *wx, bool queues) } /** - * txgbe_intr - msi/legacy mode Interrupt Handler - * @irq: interrupt number - * @data: pointer to a network interface device structure - **/ -static irqreturn_t txgbe_intr(int __always_unused irq, void *data) -{ - struct wx_q_vector *q_vector; - struct wx *wx = data; - struct pci_dev *pdev; - u32 eicr; - - q_vector = wx->q_vector[0]; - pdev = wx->pdev; - - eicr = wx_misc_isb(wx, WX_ISB_VEC0); - if (!eicr) { - /* shared interrupt alert! - * the interrupt that we masked before the ICR read. - */ - if (netif_running(wx->netdev)) - txgbe_irq_enable(wx, true); - return IRQ_NONE; /* Not our interrupt */ - } - wx->isb_mem[WX_ISB_VEC0] = 0; - if (!(pdev->msi_enabled)) - wr32(wx, WX_PX_INTA, 1); - - wx->isb_mem[WX_ISB_MISC] = 0; - /* would disable interrupts here but it is auto disabled */ - napi_schedule_irqoff(&q_vector->napi); - - /* re-enable link(maybe) and non-queue interrupts, no flush. - * txgbe_poll will re-enable the queue interrupts - */ - if (netif_running(wx->netdev)) - txgbe_irq_enable(wx, false); - - return IRQ_HANDLED; -} - -/** - * txgbe_request_msix_irqs - Initialize MSI-X interrupts + * txgbe_request_queue_irqs - Initialize MSI-X queue interrupts * @wx: board private structure * - * Allocate MSI-X vectors and request interrupts from the kernel. + * Allocate MSI-X queue vectors and request interrupts from the kernel. **/ -static int txgbe_request_msix_irqs(struct wx *wx) +int txgbe_request_queue_irqs(struct wx *wx) { struct net_device *netdev = wx->netdev; int vector, err; + if (!wx->pdev->msix_enabled) + return 0; + for (vector = 0; vector < wx->num_q_vectors; vector++) { struct wx_q_vector *q_vector = wx->q_vector[vector]; struct msix_entry *entry = &wx->msix_q_entries[vector]; @@ -110,34 +72,6 @@ static int txgbe_request_msix_irqs(struct wx *wx) return err; } -/** - * txgbe_request_irq - initialize interrupts - * @wx: board private structure - * - * Attempt to configure interrupts using the best available - * capabilities of the hardware and kernel. - **/ -int txgbe_request_irq(struct wx *wx) -{ - struct net_device *netdev = wx->netdev; - struct pci_dev *pdev = wx->pdev; - int err; - - if (pdev->msix_enabled) - err = txgbe_request_msix_irqs(wx); - else if (pdev->msi_enabled) - err = request_irq(wx->pdev->irq, &txgbe_intr, 0, - netdev->name, wx); - else - err = request_irq(wx->pdev->irq, &txgbe_intr, IRQF_SHARED, - netdev->name, wx); - - if (err) - wx_err(wx, "request_irq failed, Error %d\n", err); - - return err; -} - static int txgbe_request_gpio_irq(struct txgbe *txgbe) { txgbe->gpio_irq = irq_find_mapping(txgbe->misc.domain, TXGBE_IRQ_GPIO); diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.h index b77945e7a0f2..e6285b94625e 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.h +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.h @@ -2,6 +2,6 @@ /* Copyright (c) 2015 - 2024 Beijing WangXun Technology Co., Ltd. */ void txgbe_irq_enable(struct wx *wx, bool queues); -int txgbe_request_irq(struct wx *wx); +int txgbe_request_queue_irqs(struct wx *wx); void txgbe_free_misc_irq(struct txgbe *txgbe); int txgbe_setup_misc_irq(struct txgbe *txgbe); diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c index 8c7a74981b90..76b5672c0a17 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c @@ -294,7 +294,7 @@ static int txgbe_open(struct net_device *netdev) wx_configure(wx); - err = txgbe_request_irq(wx); + err = txgbe_request_queue_irqs(wx); if (err) goto err_free_isb;
When using MSI or INTx interrupts, request_irq() for pdev->irq will conflict with request_threaded_irq() for txgbe->misc.irq, to cause system crash. So remove txgbe_request_irq() for MSI/INTx case, and rename txgbe_request_msix_irqs() since it only request for queue irqs. Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller") Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com> --- drivers/net/ethernet/wangxun/libwx/wx_lib.c | 3 +- .../net/ethernet/wangxun/txgbe/txgbe_irq.c | 78 ++----------------- .../net/ethernet/wangxun/txgbe/txgbe_irq.h | 2 +- .../net/ethernet/wangxun/txgbe/txgbe_main.c | 2 +- 4 files changed, 10 insertions(+), 75 deletions(-)