| Message ID | 20241210070320.836260-1-dmantipov@yandex.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: brcmfmac: handle possible pci_enable_msi() error | expand |
On 12/10/2024 8:03 AM, Dmitry Antipov wrote: > Generally it's a good idea to handle error which may be returned from > 'pci_enable_msi()', so add relevant check to 'brcmf_pcie_request_irq()' > and adjust the latter to return actual 'request_threaded_irq()' error > instead of hardcoded -EIO. Compile tested only. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. Hi Dmitry, I had a deja-vu regarding this patch. So scavenging around in linux-wireless patchwork I found this. May look familiar ;-) https://patchwork.kernel.org/project/linux-wireless/patch/20230614075848.80536-2-dmantipov@yandex.ru/ Regards, Arend > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> > --- > .../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 e4395b1f8c11..f0e05cb0cfa7 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c > @@ -963,6 +963,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); > > @@ -970,16 +971,21 @@ 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)) { > + ret = pci_enable_msi(pdev); > + if (ret) > + return ret; > + > + ret = request_threaded_irq(pdev->irq, brcmf_pcie_quick_check_isr, > + brcmf_pcie_isr_thread, IRQF_SHARED, > + "brcmf_pcie_intr", devinfo); > + if (ret) { > pci_disable_msi(pdev); > brcmf_err(bus, "Failed to request IRQ %d\n", pdev->irq); > - return -EIO; > + } else { > + devinfo->irq_allocated = true; > } > - devinfo->irq_allocated = true; > - return 0; > + > + return ret; > } > >
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index e4395b1f8c11..f0e05cb0cfa7 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -963,6 +963,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); @@ -970,16 +971,21 @@ 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)) { + ret = pci_enable_msi(pdev); + if (ret) + return ret; + + ret = request_threaded_irq(pdev->irq, brcmf_pcie_quick_check_isr, + brcmf_pcie_isr_thread, IRQF_SHARED, + "brcmf_pcie_intr", devinfo); + if (ret) { pci_disable_msi(pdev); brcmf_err(bus, "Failed to request IRQ %d\n", pdev->irq); - return -EIO; + } else { + devinfo->irq_allocated = true; } - devinfo->irq_allocated = true; - return 0; + + return ret; }
Generally it's a good idea to handle error which may be returned from 'pci_enable_msi()', so add relevant check to 'brcmf_pcie_request_irq()' and adjust the latter to return actual 'request_threaded_irq()' error instead of hardcoded -EIO. Compile tested only. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- .../broadcom/brcm80211/brcmfmac/pcie.c | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)