| Message ID | 20240125120740.111330-3-dmantipov@yandex.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | Re: Re: [lvc-project] [PATCH 2/2] [v4] wifi: brcmfmac: handle possible PCI irq handling errors | expand |
On January 25, 2024 1:08:26 PM Dmitry Antipov <dmantipov@yandex.ru> wrote: > Switch to newer 'pci_{alloc,free}_irq_vectors()' API and handle > possible errors in 'brcmf_pcie_request_irq()'. Compile tested only. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Suggested-by: Bjorn Helgaas <helgaas@kernel.org> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> > --- > v4: prefer devm_{devm_request_threaded_irq,free_irq}()', use > 'pci_irq_vector()' and fix title (Arend, Bjorn) > v3: switch to 'pci_{alloc,free}_irq_vectors()' per Bjorn's review > v2: rebase against wireless-next tree > --- > .../broadcom/brcm80211/brcmfmac/pcie.c | 20 ++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-)
On January 25, 2024 1:08:26 PM Dmitry Antipov <dmantipov@yandex.ru> wrote: > Switch to newer 'pci_{alloc,free}_irq_vectors()' API and handle > possible errors in 'brcmf_pcie_request_irq()'. Compile tested only. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. Crap. Did notice something else... > Suggested-by: Bjorn Helgaas <helgaas@kernel.org> > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> > --- > v4: prefer devm_{devm_request_threaded_irq,free_irq}()', use > 'pci_irq_vector()' and fix title (Arend, Bjorn) > v3: switch to 'pci_{alloc,free}_irq_vectors()' per Bjorn's review > v2: rebase against wireless-next tree > --- > .../broadcom/brcm80211/brcmfmac/pcie.c | 20 ++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c > index 80220685f5e4..17b855164025 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c [...] > @@ -972,11 +973,16 @@ static int brcmf_pcie_request_irq(struct > brcmf_pciedev_info *devinfo) > > brcmf_dbg(PCIE, "Enter\n"); > > - pci_enable_msi(pdev); > - if (request_threaded_irq(pdev->irq, brcmf_pcie_quick_check_isr, > - brcmf_pcie_isr_thread, IRQF_SHARED, > - "brcmf_pcie_intr", devinfo)) { > - pci_disable_msi(pdev); > + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES); > + if (ret < 0) > + return ret; > + > + if (devm_request_threaded_irq(&pdev->dev, > + pci_irq_vector(pdev, 0), > + brcmf_pcie_quick_check_isr, > + brcmf_pcie_isr_thread, IRQF_SHARED, > + "brcmf_pcie_intr", devinfo)) { > + pci_free_irq_vectors(pdev); > brcmf_err(bus, "Failed to request IRQ %d\n", pdev->irq); Maybe better to use pci_irq_vector() here as well. > return -EIO; > }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 80220685f5e4..17b855164025 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -965,6 +965,7 @@ static irqreturn_t brcmf_pcie_isr_thread(int irq, void *arg) static int brcmf_pcie_request_irq(struct brcmf_pciedev_info *devinfo) { + int ret; struct pci_dev *pdev = devinfo->pdev; struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); @@ -972,11 +973,16 @@ static int brcmf_pcie_request_irq(struct brcmf_pciedev_info *devinfo) brcmf_dbg(PCIE, "Enter\n"); - pci_enable_msi(pdev); - if (request_threaded_irq(pdev->irq, brcmf_pcie_quick_check_isr, - brcmf_pcie_isr_thread, IRQF_SHARED, - "brcmf_pcie_intr", devinfo)) { - pci_disable_msi(pdev); + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES); + if (ret < 0) + return ret; + + if (devm_request_threaded_irq(&pdev->dev, + pci_irq_vector(pdev, 0), + brcmf_pcie_quick_check_isr, + brcmf_pcie_isr_thread, IRQF_SHARED, + "brcmf_pcie_intr", devinfo)) { + pci_free_irq_vectors(pdev); brcmf_err(bus, "Failed to request IRQ %d\n", pdev->irq); return -EIO; } @@ -996,8 +1002,8 @@ static void brcmf_pcie_release_irq(struct brcmf_pciedev_info *devinfo) return; brcmf_pcie_intr_disable(devinfo); - free_irq(pdev->irq, devinfo); - pci_disable_msi(pdev); + devm_free_irq(&pdev->dev, pdev->irq, devinfo); + pci_free_irq_vectors(pdev); msleep(50); count = 0;
Switch to newer 'pci_{alloc,free}_irq_vectors()' API and handle possible errors in 'brcmf_pcie_request_irq()'. Compile tested only. Found by Linux Verification Center (linuxtesting.org) with SVACE. Suggested-by: Bjorn Helgaas <helgaas@kernel.org> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- v4: prefer devm_{devm_request_threaded_irq,free_irq}()', use 'pci_irq_vector()' and fix title (Arend, Bjorn) v3: switch to 'pci_{alloc,free}_irq_vectors()' per Bjorn's review v2: rebase against wireless-next tree --- .../broadcom/brcm80211/brcmfmac/pcie.c | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)