diff mbox series

[01/10] scsi: pm8001: Setup IRQs on resume

Message ID 20230911030207.242917-2-dlemoal@kernel.org (mailing list archive)
State Superseded
Headers show
Series scsi: pm8001: Bug fix and cleanup | expand

Commit Message

Damien Le Moal Sept. 11, 2023, 3:01 a.m. UTC 
The function pm8001_pci_resume() only calls pm8001_request_irq() without
calling pm8001_setup_irq(). This causes the IRQ allocation to fail,
whihc leads all drives being removed from the system.

Fix this issue by integrating the code for pm8001_setup_irq() directly
inside pm8001_request_irq() so that msix setup is performed both during
normal initialization and resume operations.

Fixes: dbf9bfe61571 ("[SCSI] pm8001: add SAS/SATA HBA driver")
Cc: stable@vger.kernel.org
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/scsi/pm8001/pm8001_init.c | 51 +++++++++++--------------------
 1 file changed, 17 insertions(+), 34 deletions(-)

Comments

Jinpu Wang Sept. 11, 2023, 7:53 a.m. UTC | #1 
On Mon, Sep 11, 2023 at 5:02 AM Damien Le Moal <dlemoal@kernel.org> wrote:
>
> The function pm8001_pci_resume() only calls pm8001_request_irq() without
> calling pm8001_setup_irq(). This causes the IRQ allocation to fail,
> whihc leads all drives being removed from the system.
>
> Fix this issue by integrating the code for pm8001_setup_irq() directly
> inside pm8001_request_irq() so that msix setup is performed both during
> normal initialization and resume operations.
>
> Fixes: dbf9bfe61571 ("[SCSI] pm8001: add SAS/SATA HBA driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
lgtm, thx!
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
>  drivers/scsi/pm8001/pm8001_init.c | 51 +++++++++++--------------------
>  1 file changed, 17 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 5e5ce1e74c3b..443a3176c6c0 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -273,7 +273,6 @@ static irqreturn_t pm8001_interrupt_handler_intx(int irq, void *dev_id)
>         return ret;
>  }
>
> -static u32 pm8001_setup_irq(struct pm8001_hba_info *pm8001_ha);
>  static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha);
>
>  /**
> @@ -294,13 +293,6 @@ static int pm8001_alloc(struct pm8001_hba_info *pm8001_ha,
>         pm8001_dbg(pm8001_ha, INIT, "pm8001_alloc: PHY:%x\n",
>                    pm8001_ha->chip->n_phy);
>
> -       /* Setup Interrupt */
> -       rc = pm8001_setup_irq(pm8001_ha);
> -       if (rc) {
> -               pm8001_dbg(pm8001_ha, FAIL,
> -                          "pm8001_setup_irq failed [ret: %d]\n", rc);
> -               goto err_out;
> -       }
>         /* Request Interrupt */
>         rc = pm8001_request_irq(pm8001_ha);
>         if (rc)
> @@ -1031,47 +1023,38 @@ static u32 pm8001_request_msix(struct pm8001_hba_info *pm8001_ha)
>  }
>  #endif
>
> -static u32 pm8001_setup_irq(struct pm8001_hba_info *pm8001_ha)
> -{
> -       struct pci_dev *pdev;
> -
> -       pdev = pm8001_ha->pdev;
> -
> -#ifdef PM8001_USE_MSIX
> -       if (pci_find_capability(pdev, PCI_CAP_ID_MSIX))
> -               return pm8001_setup_msix(pm8001_ha);
> -       pm8001_dbg(pm8001_ha, INIT, "MSIX not supported!!!\n");
> -#endif
> -       return 0;
> -}
> -
>  /**
>   * pm8001_request_irq - register interrupt
>   * @pm8001_ha: our ha struct.
>   */
>  static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha)
>  {
> -       struct pci_dev *pdev;
> +       struct pci_dev *pdev = pm8001_ha->pdev;
> +#ifdef PM8001_USE_MSIX
>         int rc;
>
> -       pdev = pm8001_ha->pdev;
> +       if (pci_find_capability(pdev, PCI_CAP_ID_MSIX)) {
> +               rc = pm8001_setup_msix(pm8001_ha);
> +               if (rc) {
> +                       pm8001_dbg(pm8001_ha, FAIL,
> +                                  "pm8001_setup_irq failed [ret: %d]\n", rc);
> +                       return rc;
> +               }
>
> -#ifdef PM8001_USE_MSIX
> -       if (pdev->msix_cap && pci_msi_enabled())
> -               return pm8001_request_msix(pm8001_ha);
> -       else {
> -               pm8001_dbg(pm8001_ha, INIT, "MSIX not supported!!!\n");
> -               goto intx;
> +               if (pdev->msix_cap && pci_msi_enabled())
> +                       return pm8001_request_msix(pm8001_ha);
>         }
> +
> +       pm8001_dbg(pm8001_ha, INIT, "MSIX not supported!!!\n");
>  #endif
>
> -intx:
>         /* initialize the INT-X interrupt */
>         pm8001_ha->irq_vector[0].irq_id = 0;
>         pm8001_ha->irq_vector[0].drv_inst = pm8001_ha;
> -       rc = request_irq(pdev->irq, pm8001_interrupt_handler_intx, IRQF_SHARED,
> -               pm8001_ha->name, SHOST_TO_SAS_HA(pm8001_ha->shost));
> -       return rc;
> +
> +       return request_irq(pdev->irq, pm8001_interrupt_handler_intx,
> +                          IRQF_SHARED, pm8001_ha->name,
> +                          SHOST_TO_SAS_HA(pm8001_ha->shost));
>  }
>
>  /**
> --
> 2.41.0
>
diff mbox series

Patch

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 5e5ce1e74c3b..443a3176c6c0 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -273,7 +273,6 @@  static irqreturn_t pm8001_interrupt_handler_intx(int irq, void *dev_id)
 	return ret;
 }
 
-static u32 pm8001_setup_irq(struct pm8001_hba_info *pm8001_ha);
 static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha);
 
 /**
@@ -294,13 +293,6 @@  static int pm8001_alloc(struct pm8001_hba_info *pm8001_ha,
 	pm8001_dbg(pm8001_ha, INIT, "pm8001_alloc: PHY:%x\n",
 		   pm8001_ha->chip->n_phy);
 
-	/* Setup Interrupt */
-	rc = pm8001_setup_irq(pm8001_ha);
-	if (rc) {
-		pm8001_dbg(pm8001_ha, FAIL,
-			   "pm8001_setup_irq failed [ret: %d]\n", rc);
-		goto err_out;
-	}
 	/* Request Interrupt */
 	rc = pm8001_request_irq(pm8001_ha);
 	if (rc)
@@ -1031,47 +1023,38 @@  static u32 pm8001_request_msix(struct pm8001_hba_info *pm8001_ha)
 }
 #endif
 
-static u32 pm8001_setup_irq(struct pm8001_hba_info *pm8001_ha)
-{
-	struct pci_dev *pdev;
-
-	pdev = pm8001_ha->pdev;
-
-#ifdef PM8001_USE_MSIX
-	if (pci_find_capability(pdev, PCI_CAP_ID_MSIX))
-		return pm8001_setup_msix(pm8001_ha);
-	pm8001_dbg(pm8001_ha, INIT, "MSIX not supported!!!\n");
-#endif
-	return 0;
-}
-
 /**
  * pm8001_request_irq - register interrupt
  * @pm8001_ha: our ha struct.
  */
 static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha)
 {
-	struct pci_dev *pdev;
+	struct pci_dev *pdev = pm8001_ha->pdev;
+#ifdef PM8001_USE_MSIX
 	int rc;
 
-	pdev = pm8001_ha->pdev;
+	if (pci_find_capability(pdev, PCI_CAP_ID_MSIX)) {
+		rc = pm8001_setup_msix(pm8001_ha);
+		if (rc) {
+			pm8001_dbg(pm8001_ha, FAIL,
+				   "pm8001_setup_irq failed [ret: %d]\n", rc);
+			return rc;
+		}
 
-#ifdef PM8001_USE_MSIX
-	if (pdev->msix_cap && pci_msi_enabled())
-		return pm8001_request_msix(pm8001_ha);
-	else {
-		pm8001_dbg(pm8001_ha, INIT, "MSIX not supported!!!\n");
-		goto intx;
+		if (pdev->msix_cap && pci_msi_enabled())
+			return pm8001_request_msix(pm8001_ha);
 	}
+
+	pm8001_dbg(pm8001_ha, INIT, "MSIX not supported!!!\n");
 #endif
 
-intx:
 	/* initialize the INT-X interrupt */
 	pm8001_ha->irq_vector[0].irq_id = 0;
 	pm8001_ha->irq_vector[0].drv_inst = pm8001_ha;
-	rc = request_irq(pdev->irq, pm8001_interrupt_handler_intx, IRQF_SHARED,
-		pm8001_ha->name, SHOST_TO_SAS_HA(pm8001_ha->shost));
-	return rc;
+
+	return request_irq(pdev->irq, pm8001_interrupt_handler_intx,
+			   IRQF_SHARED, pm8001_ha->name,
+			   SHOST_TO_SAS_HA(pm8001_ha->shost));
 }
 
 /**