* [PATCH 01/10] scsi: pm8001: Setup IRQs on resume
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
@ 2023-09-11 3:01 ` Damien Le Moal
2023-09-11 7:53 ` Jinpu Wang
2023-09-11 3:01 ` [PATCH 02/10] scsi: pm8001: Introduce pm8001_free_irq() Damien Le Moal
` (9 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:01 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
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(-)
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 01/10] scsi: pm8001: Setup IRQs on resume
2023-09-11 3:01 ` [PATCH 01/10] scsi: pm8001: Setup IRQs on resume Damien Le Moal
@ 2023-09-11 7:53 ` Jinpu Wang
0 siblings, 0 replies; 15+ messages in thread
From: Jinpu Wang @ 2023-09-11 7:53 UTC (permalink / raw)
To: Damien Le Moal; +Cc: Martin K . Petersen, linux-scsi
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 02/10] scsi: pm8001: Introduce pm8001_free_irq()
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
2023-09-11 3:01 ` [PATCH 01/10] scsi: pm8001: Setup IRQs on resume Damien Le Moal
@ 2023-09-11 3:01 ` Damien Le Moal
2023-09-11 8:11 ` Jinpu Wang
2023-09-11 3:02 ` [PATCH 03/10] scsi: pm8001: Introduce pm8001_init_tasklet() Damien Le Moal
` (8 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:01 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
Instead of repeating the same code twice in pm8001_pci_remove() and
pm8001_pci_suspend() to free IRQs, introduuce the function
pm8001_free_irq() to do that.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/pm8001/pm8001_init.c | 49 ++++++++++++++++++-------------
1 file changed, 28 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 443a3176c6c0..3b7d47cd70ba 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -274,6 +274,7 @@ static irqreturn_t pm8001_interrupt_handler_intx(int irq, void *dev_id)
}
static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha);
+static void pm8001_free_irq(struct pm8001_hba_info *pm8001_ha);
/**
* pm8001_alloc - initiate our hba structure and 6 DMAs area.
@@ -1057,6 +1058,24 @@ static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha)
SHOST_TO_SAS_HA(pm8001_ha->shost));
}
+static void pm8001_free_irq(struct pm8001_hba_info *pm8001_ha)
+{
+#ifdef PM8001_USE_MSIX
+ struct pci_dev *pdev = pm8001_ha->pdev;
+ int i;
+
+ for (i = 0; i < pm8001_ha->number_of_intr; i++)
+ synchronize_irq(pci_irq_vector(pdev, i));
+
+ for (i = 0; i < pm8001_ha->number_of_intr; i++)
+ free_irq(pci_irq_vector(pdev, i), &pm8001_ha->irq_vector[i]);
+
+ pci_free_irq_vectors(pdev);
+#else
+ free_irq(pm8001_ha->irq, pm8001_ha->sas);
+#endif
+}
+
/**
* pm8001_pci_probe - probe supported device
* @pdev: pci device which kernel has been prepared for.
@@ -1252,24 +1271,17 @@ static int pm8001_init_ccb_tag(struct pm8001_hba_info *pm8001_ha)
static void pm8001_pci_remove(struct pci_dev *pdev)
{
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
- struct pm8001_hba_info *pm8001_ha;
+ struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
int i, j;
- pm8001_ha = sha->lldd_ha;
+
sas_unregister_ha(sha);
sas_remove_host(pm8001_ha->shost);
list_del(&pm8001_ha->list);
PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha);
-#ifdef PM8001_USE_MSIX
- for (i = 0; i < pm8001_ha->number_of_intr; i++)
- synchronize_irq(pci_irq_vector(pdev, i));
- for (i = 0; i < pm8001_ha->number_of_intr; i++)
- free_irq(pci_irq_vector(pdev, i), &pm8001_ha->irq_vector[i]);
- pci_free_irq_vectors(pdev);
-#else
- free_irq(pm8001_ha->irq, sha);
-#endif
+ pm8001_free_irq(pm8001_ha);
+
#ifdef PM8001_USE_TASKLET
/* For non-msix and msix interrupts */
if ((!pdev->msix_cap || !pci_msi_enabled()) ||
@@ -1309,7 +1321,8 @@ static int __maybe_unused pm8001_pci_suspend(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
- int i, j;
+ int j;
+
sas_suspend_ha(sha);
flush_workqueue(pm8001_wq);
scsi_block_requests(pm8001_ha->shost);
@@ -1319,15 +1332,9 @@ static int __maybe_unused pm8001_pci_suspend(struct device *dev)
}
PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha);
-#ifdef PM8001_USE_MSIX
- for (i = 0; i < pm8001_ha->number_of_intr; i++)
- synchronize_irq(pci_irq_vector(pdev, i));
- for (i = 0; i < pm8001_ha->number_of_intr; i++)
- free_irq(pci_irq_vector(pdev, i), &pm8001_ha->irq_vector[i]);
- pci_free_irq_vectors(pdev);
-#else
- free_irq(pm8001_ha->irq, sha);
-#endif
+
+ pm8001_free_irq(pm8001_ha);
+
#ifdef PM8001_USE_TASKLET
/* For non-msix and msix interrupts */
if ((!pdev->msix_cap || !pci_msi_enabled()) ||
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 02/10] scsi: pm8001: Introduce pm8001_free_irq()
2023-09-11 3:01 ` [PATCH 02/10] scsi: pm8001: Introduce pm8001_free_irq() Damien Le Moal
@ 2023-09-11 8:11 ` Jinpu Wang
0 siblings, 0 replies; 15+ messages in thread
From: Jinpu Wang @ 2023-09-11 8:11 UTC (permalink / raw)
To: Damien Le Moal; +Cc: Martin K . Petersen, linux-scsi
On Mon, Sep 11, 2023 at 5:02 AM Damien Le Moal <dlemoal@kernel.org> wrote:
>
> Instead of repeating the same code twice in pm8001_pci_remove() and
> pm8001_pci_suspend() to free IRQs, introduuce the function
> pm8001_free_irq() to do that.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/scsi/pm8001/pm8001_init.c | 49 ++++++++++++++++++-------------
> 1 file changed, 28 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 443a3176c6c0..3b7d47cd70ba 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -274,6 +274,7 @@ static irqreturn_t pm8001_interrupt_handler_intx(int irq, void *dev_id)
> }
>
> static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha);
> +static void pm8001_free_irq(struct pm8001_hba_info *pm8001_ha);
>
> /**
> * pm8001_alloc - initiate our hba structure and 6 DMAs area.
> @@ -1057,6 +1058,24 @@ static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha)
> SHOST_TO_SAS_HA(pm8001_ha->shost));
> }
>
> +static void pm8001_free_irq(struct pm8001_hba_info *pm8001_ha)
> +{
> +#ifdef PM8001_USE_MSIX
> + struct pci_dev *pdev = pm8001_ha->pdev;
> + int i;
> +
> + for (i = 0; i < pm8001_ha->number_of_intr; i++)
> + synchronize_irq(pci_irq_vector(pdev, i));
> +
> + for (i = 0; i < pm8001_ha->number_of_intr; i++)
> + free_irq(pci_irq_vector(pdev, i), &pm8001_ha->irq_vector[i]);
> +
> + pci_free_irq_vectors(pdev);
> +#else
> + free_irq(pm8001_ha->irq, pm8001_ha->sas);
> +#endif
> +}
> +
> /**
> * pm8001_pci_probe - probe supported device
> * @pdev: pci device which kernel has been prepared for.
> @@ -1252,24 +1271,17 @@ static int pm8001_init_ccb_tag(struct pm8001_hba_info *pm8001_ha)
> static void pm8001_pci_remove(struct pci_dev *pdev)
> {
> struct sas_ha_struct *sha = pci_get_drvdata(pdev);
> - struct pm8001_hba_info *pm8001_ha;
> + struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
> int i, j;
> - pm8001_ha = sha->lldd_ha;
> +
> sas_unregister_ha(sha);
> sas_remove_host(pm8001_ha->shost);
> list_del(&pm8001_ha->list);
> PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
> PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha);
>
> -#ifdef PM8001_USE_MSIX
> - for (i = 0; i < pm8001_ha->number_of_intr; i++)
> - synchronize_irq(pci_irq_vector(pdev, i));
> - for (i = 0; i < pm8001_ha->number_of_intr; i++)
> - free_irq(pci_irq_vector(pdev, i), &pm8001_ha->irq_vector[i]);
> - pci_free_irq_vectors(pdev);
> -#else
> - free_irq(pm8001_ha->irq, sha);
> -#endif
> + pm8001_free_irq(pm8001_ha);
> +
> #ifdef PM8001_USE_TASKLET
> /* For non-msix and msix interrupts */
> if ((!pdev->msix_cap || !pci_msi_enabled()) ||
> @@ -1309,7 +1321,8 @@ static int __maybe_unused pm8001_pci_suspend(struct device *dev)
> struct pci_dev *pdev = to_pci_dev(dev);
> struct sas_ha_struct *sha = pci_get_drvdata(pdev);
> struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
> - int i, j;
> + int j;
> +
> sas_suspend_ha(sha);
> flush_workqueue(pm8001_wq);
> scsi_block_requests(pm8001_ha->shost);
> @@ -1319,15 +1332,9 @@ static int __maybe_unused pm8001_pci_suspend(struct device *dev)
> }
> PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
> PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha);
> -#ifdef PM8001_USE_MSIX
> - for (i = 0; i < pm8001_ha->number_of_intr; i++)
> - synchronize_irq(pci_irq_vector(pdev, i));
> - for (i = 0; i < pm8001_ha->number_of_intr; i++)
> - free_irq(pci_irq_vector(pdev, i), &pm8001_ha->irq_vector[i]);
> - pci_free_irq_vectors(pdev);
> -#else
> - free_irq(pm8001_ha->irq, sha);
> -#endif
> +
> + pm8001_free_irq(pm8001_ha);
> +
> #ifdef PM8001_USE_TASKLET
> /* For non-msix and msix interrupts */
> if ((!pdev->msix_cap || !pci_msi_enabled()) ||
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 03/10] scsi: pm8001: Introduce pm8001_init_tasklet()
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
2023-09-11 3:01 ` [PATCH 01/10] scsi: pm8001: Setup IRQs on resume Damien Le Moal
2023-09-11 3:01 ` [PATCH 02/10] scsi: pm8001: Introduce pm8001_free_irq() Damien Le Moal
@ 2023-09-11 3:02 ` Damien Le Moal
2023-09-11 3:02 ` [PATCH 04/10] scsi: pm8001: Introduce pm8001_kill_tasklet() Damien Le Moal
` (7 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:02 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
Factor out the identical code for initializing tasklets in
pm8001_pci_alloc() and pm8001_pci_resume() and instead use the new
function pm8001_init_tasklet().
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/pm8001/pm8001_init.c | 51 ++++++++++++++++---------------
1 file changed, 27 insertions(+), 24 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 3b7d47cd70ba..c490c8509494 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -218,6 +218,27 @@ static void pm8001_tasklet(unsigned long opaque)
BUG_ON(1);
PM8001_CHIP_DISP->isr(pm8001_ha, irq_vector->irq_id);
}
+
+static void pm8001_init_tasklet(struct pm8001_hba_info *pm8001_ha)
+{
+ int i;
+
+ /* Tasklet for non msi-x interrupt handler */
+ if ((!pm8001_ha->pdev->msix_cap || !pci_msi_enabled()) ||
+ (pm8001_ha->chip_id == chip_8001)) {
+ tasklet_init(&pm8001_ha->tasklet[0], pm8001_tasklet,
+ (unsigned long)&(pm8001_ha->irq_vector[0]));
+ return;
+ }
+ for (i = 0; i < PM8001_MAX_MSIX_VEC; i++)
+ tasklet_init(&pm8001_ha->tasklet[i], pm8001_tasklet,
+ (unsigned long)&(pm8001_ha->irq_vector[i]));
+}
+
+#else
+
+static void pm8001_init_tasklet(struct pm8001_hba_info *pm8001_ha) {}
+
#endif
/**
@@ -512,7 +533,6 @@ static struct pm8001_hba_info *pm8001_pci_alloc(struct pci_dev *pdev,
{
struct pm8001_hba_info *pm8001_ha;
struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
- int j;
pm8001_ha = sha->lldd_ha;
if (!pm8001_ha)
@@ -543,17 +563,8 @@ static struct pm8001_hba_info *pm8001_pci_alloc(struct pci_dev *pdev,
else
pm8001_ha->iomb_size = IOMB_SIZE_SPC;
-#ifdef PM8001_USE_TASKLET
- /* Tasklet for non msi-x interrupt handler */
- if ((!pdev->msix_cap || !pci_msi_enabled())
- || (pm8001_ha->chip_id == chip_8001))
- tasklet_init(&pm8001_ha->tasklet[0], pm8001_tasklet,
- (unsigned long)&(pm8001_ha->irq_vector[0]));
- else
- for (j = 0; j < PM8001_MAX_MSIX_VEC; j++)
- tasklet_init(&pm8001_ha->tasklet[j], pm8001_tasklet,
- (unsigned long)&(pm8001_ha->irq_vector[j]));
-#endif
+ pm8001_init_tasklet(pm8001_ha);
+
if (pm8001_ioremap(pm8001_ha))
goto failed_pci_alloc;
if (!pm8001_alloc(pm8001_ha, ent))
@@ -1362,7 +1373,7 @@ static int __maybe_unused pm8001_pci_resume(struct device *dev)
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct pm8001_hba_info *pm8001_ha;
int rc;
- u8 i = 0, j;
+ u8 i = 0;
DECLARE_COMPLETION_ONSTACK(completion);
pm8001_ha = sha->lldd_ha;
@@ -1390,17 +1401,9 @@ static int __maybe_unused pm8001_pci_resume(struct device *dev)
rc = pm8001_request_irq(pm8001_ha);
if (rc)
goto err_out_disable;
-#ifdef PM8001_USE_TASKLET
- /* Tasklet for non msi-x interrupt handler */
- if ((!pdev->msix_cap || !pci_msi_enabled()) ||
- (pm8001_ha->chip_id == chip_8001))
- tasklet_init(&pm8001_ha->tasklet[0], pm8001_tasklet,
- (unsigned long)&(pm8001_ha->irq_vector[0]));
- else
- for (j = 0; j < PM8001_MAX_MSIX_VEC; j++)
- tasklet_init(&pm8001_ha->tasklet[j], pm8001_tasklet,
- (unsigned long)&(pm8001_ha->irq_vector[j]));
-#endif
+
+ pm8001_init_tasklet(pm8001_ha);
+
PM8001_CHIP_DISP->interrupt_enable(pm8001_ha, 0);
if (pm8001_ha->chip_id != chip_8001) {
for (i = 1; i < pm8001_ha->number_of_intr; i++)
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 04/10] scsi: pm8001: Introduce pm8001_kill_tasklet()
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
` (2 preceding siblings ...)
2023-09-11 3:02 ` [PATCH 03/10] scsi: pm8001: Introduce pm8001_init_tasklet() Damien Le Moal
@ 2023-09-11 3:02 ` Damien Le Moal
2023-09-11 3:02 ` [PATCH 05/10] scsi: pm8001: Introduce pm8001_handle_irq() Damien Le Moal
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:02 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
Factor out the identical code for killing tasklets in
pm8001_pci_remove() and pm8001_pci_suspend() and instead use the new
function pm8001_kill_tasklet().
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/pm8001/pm8001_init.c | 40 +++++++++++++++----------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index c490c8509494..44a027d76fba 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -235,9 +235,25 @@ static void pm8001_init_tasklet(struct pm8001_hba_info *pm8001_ha)
(unsigned long)&(pm8001_ha->irq_vector[i]));
}
+static void pm8001_kill_tasklet(struct pm8001_hba_info *pm8001_ha)
+{
+ int i;
+
+ /* For non-msix and msix interrupts */
+ if ((!pm8001_ha->pdev->msix_cap || !pci_msi_enabled()) ||
+ (pm8001_ha->chip_id == chip_8001)) {
+ tasklet_kill(&pm8001_ha->tasklet[0]);
+ return;
+ }
+
+ for (i = 0; i < PM8001_MAX_MSIX_VEC; i++)
+ tasklet_kill(&pm8001_ha->tasklet[i]);
+}
+
#else
static void pm8001_init_tasklet(struct pm8001_hba_info *pm8001_ha) {}
+static void pm8001_kill_tasklet(struct pm8001_hba_info *pm8001_ha) {}
#endif
@@ -1283,7 +1299,7 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
{
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
- int i, j;
+ int i;
sas_unregister_ha(sha);
sas_remove_host(pm8001_ha->shost);
@@ -1292,16 +1308,7 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha);
pm8001_free_irq(pm8001_ha);
-
-#ifdef PM8001_USE_TASKLET
- /* For non-msix and msix interrupts */
- if ((!pdev->msix_cap || !pci_msi_enabled()) ||
- (pm8001_ha->chip_id == chip_8001))
- tasklet_kill(&pm8001_ha->tasklet[0]);
- else
- for (j = 0; j < PM8001_MAX_MSIX_VEC; j++)
- tasklet_kill(&pm8001_ha->tasklet[j]);
-#endif
+ pm8001_kill_tasklet(pm8001_ha);
scsi_host_put(pm8001_ha->shost);
for (i = 0; i < pm8001_ha->ccb_count; i++) {
@@ -1332,7 +1339,6 @@ static int __maybe_unused pm8001_pci_suspend(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
- int j;
sas_suspend_ha(sha);
flush_workqueue(pm8001_wq);
@@ -1345,16 +1351,8 @@ static int __maybe_unused pm8001_pci_suspend(struct device *dev)
PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha);
pm8001_free_irq(pm8001_ha);
+ pm8001_kill_tasklet(pm8001_ha);
-#ifdef PM8001_USE_TASKLET
- /* For non-msix and msix interrupts */
- if ((!pdev->msix_cap || !pci_msi_enabled()) ||
- (pm8001_ha->chip_id == chip_8001))
- tasklet_kill(&pm8001_ha->tasklet[0]);
- else
- for (j = 0; j < PM8001_MAX_MSIX_VEC; j++)
- tasklet_kill(&pm8001_ha->tasklet[j]);
-#endif
pm8001_info(pm8001_ha, "pdev=0x%p, slot=%s, entering "
"suspended state\n", pdev,
pm8001_ha->name);
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 05/10] scsi: pm8001: Introduce pm8001_handle_irq()
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
` (3 preceding siblings ...)
2023-09-11 3:02 ` [PATCH 04/10] scsi: pm8001: Introduce pm8001_kill_tasklet() Damien Le Moal
@ 2023-09-11 3:02 ` Damien Le Moal
2023-09-11 3:02 ` [PATCH 06/10] scsi: pm8001: Simplify pm8001_chip_interrupt_enable/disable() Damien Le Moal
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:02 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
Factor out the common code of pm8001_interrupt_handler_msix and of
pm8001_interrupt_handler_intx() into the new function
pm8001_handle_irq() and use this new helper in these two functions to
simplify the code.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/pm8001/pm8001_init.c | 50 ++++++++++++++-----------------
1 file changed, 22 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 44a027d76fba..0ec43e155511 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -257,6 +257,23 @@ static void pm8001_kill_tasklet(struct pm8001_hba_info *pm8001_ha) {}
#endif
+static irqreturn_t pm8001_handle_irq(struct pm8001_hba_info *pm8001_ha,
+ int irq)
+{
+ if (unlikely(!pm8001_ha))
+ return IRQ_NONE;
+
+ if (!PM8001_CHIP_DISP->is_our_interrupt(pm8001_ha))
+ return IRQ_NONE;
+
+#ifdef PM8001_USE_TASKLET
+ tasklet_schedule(&pm8001_ha->tasklet[irq]);
+ return IRQ_HANDLED;
+#else
+ return PM8001_CHIP_DISP->isr(pm8001_ha, irq);
+#endif
+}
+
/**
* pm8001_interrupt_handler_msix - main MSIX interrupt handler.
* It obtains the vector number and calls the equivalent bottom
@@ -267,22 +284,10 @@ static void pm8001_kill_tasklet(struct pm8001_hba_info *pm8001_ha) {}
*/
static irqreturn_t pm8001_interrupt_handler_msix(int irq, void *opaque)
{
- struct isr_param *irq_vector;
- struct pm8001_hba_info *pm8001_ha;
- irqreturn_t ret = IRQ_HANDLED;
- irq_vector = (struct isr_param *)opaque;
- pm8001_ha = irq_vector->drv_inst;
+ struct isr_param *irq_vector = (struct isr_param *)opaque;
+ struct pm8001_hba_info *pm8001_ha = irq_vector->drv_inst;
- if (unlikely(!pm8001_ha))
- return IRQ_NONE;
- if (!PM8001_CHIP_DISP->is_our_interrupt(pm8001_ha))
- return IRQ_NONE;
-#ifdef PM8001_USE_TASKLET
- tasklet_schedule(&pm8001_ha->tasklet[irq_vector->irq_id]);
-#else
- ret = PM8001_CHIP_DISP->isr(pm8001_ha, irq_vector->irq_id);
-#endif
- return ret;
+ return pm8001_handle_irq(pm8001_ha, irq_vector->irq_id);
}
/**
@@ -293,21 +298,10 @@ static irqreturn_t pm8001_interrupt_handler_msix(int irq, void *opaque)
static irqreturn_t pm8001_interrupt_handler_intx(int irq, void *dev_id)
{
- struct pm8001_hba_info *pm8001_ha;
- irqreturn_t ret = IRQ_HANDLED;
struct sas_ha_struct *sha = dev_id;
- pm8001_ha = sha->lldd_ha;
- if (unlikely(!pm8001_ha))
- return IRQ_NONE;
- if (!PM8001_CHIP_DISP->is_our_interrupt(pm8001_ha))
- return IRQ_NONE;
+ struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
-#ifdef PM8001_USE_TASKLET
- tasklet_schedule(&pm8001_ha->tasklet[0]);
-#else
- ret = PM8001_CHIP_DISP->isr(pm8001_ha, 0);
-#endif
- return ret;
+ return pm8001_handle_irq(pm8001_ha, 0);
}
static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha);
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 06/10] scsi: pm8001: Simplify pm8001_chip_interrupt_enable/disable()
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
` (4 preceding siblings ...)
2023-09-11 3:02 ` [PATCH 05/10] scsi: pm8001: Introduce pm8001_handle_irq() Damien Le Moal
@ 2023-09-11 3:02 ` Damien Le Moal
2023-09-11 3:02 ` [PATCH 07/10] scsi: pm8001: Remove pm80xx_chip_intx_interrupt_enable/disable() Damien Le Moal
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:02 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
pm8001_chip_msix_interrupt_enable() and
pm8001_chip_msix_interrupt_disable() are always cold with the vector
argument equal to 0. This allows simplifying the code for these
functions. With this change, the functions are simple enough and can be
removed by open coding them directly in pm8001_chip_interrupt_enable()
and pm8001_chip_interrupt_disable(). Also do the same for the functions
pm8001_chip_intx_interrupt_enable() and
pm8001_chip_intx_interrupt_disable() and remove these functions.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/pm8001/pm8001_hwi.c | 69 +++-----------------------------
1 file changed, 6 insertions(+), 63 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 33053db5a713..ef62afc425fc 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1180,65 +1180,6 @@ void pm8001_chip_iounmap(struct pm8001_hba_info *pm8001_ha)
}
}
-#ifndef PM8001_USE_MSIX
-/**
- * pm8001_chip_intx_interrupt_enable - enable PM8001 chip interrupt
- * @pm8001_ha: our hba card information
- */
-static void
-pm8001_chip_intx_interrupt_enable(struct pm8001_hba_info *pm8001_ha)
-{
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
- pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
-}
-
-/**
- * pm8001_chip_intx_interrupt_disable - disable PM8001 chip interrupt
- * @pm8001_ha: our hba card information
- */
-static void
-pm8001_chip_intx_interrupt_disable(struct pm8001_hba_info *pm8001_ha)
-{
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
-}
-
-#else
-
-/**
- * pm8001_chip_msix_interrupt_enable - enable PM8001 chip interrupt
- * @pm8001_ha: our hba card information
- * @int_vec_idx: interrupt number to enable
- */
-static void
-pm8001_chip_msix_interrupt_enable(struct pm8001_hba_info *pm8001_ha,
- u32 int_vec_idx)
-{
- u32 msi_index;
- u32 value;
- msi_index = int_vec_idx * MSIX_TABLE_ELEMENT_SIZE;
- msi_index += MSIX_TABLE_BASE;
- pm8001_cw32(pm8001_ha, 0, msi_index, MSIX_INTERRUPT_ENABLE);
- value = (1 << int_vec_idx);
- pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, value);
-
-}
-
-/**
- * pm8001_chip_msix_interrupt_disable - disable PM8001 chip interrupt
- * @pm8001_ha: our hba card information
- * @int_vec_idx: interrupt number to disable
- */
-static void
-pm8001_chip_msix_interrupt_disable(struct pm8001_hba_info *pm8001_ha,
- u32 int_vec_idx)
-{
- u32 msi_index;
- msi_index = int_vec_idx * MSIX_TABLE_ELEMENT_SIZE;
- msi_index += MSIX_TABLE_BASE;
- pm8001_cw32(pm8001_ha, 0, msi_index, MSIX_INTERRUPT_DISABLE);
-}
-#endif
-
/**
* pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
* @pm8001_ha: our hba card information
@@ -1248,9 +1189,11 @@ static void
pm8001_chip_interrupt_enable(struct pm8001_hba_info *pm8001_ha, u8 vec)
{
#ifdef PM8001_USE_MSIX
- pm8001_chip_msix_interrupt_enable(pm8001_ha, 0);
+ pm8001_cw32(pm8001_ha, 0, MSIX_TABLE_BASE, MSIX_INTERRUPT_ENABLE);
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, 1);
#else
- pm8001_chip_intx_interrupt_enable(pm8001_ha);
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
#endif
}
@@ -1263,9 +1206,9 @@ static void
pm8001_chip_interrupt_disable(struct pm8001_hba_info *pm8001_ha, u8 vec)
{
#ifdef PM8001_USE_MSIX
- pm8001_chip_msix_interrupt_disable(pm8001_ha, 0);
+ pm8001_cw32(pm8001_ha, 0, MSIX_TABLE_BASE, MSIX_INTERRUPT_DISABLE);
#else
- pm8001_chip_intx_interrupt_disable(pm8001_ha);
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
#endif
}
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 07/10] scsi: pm8001: Remove pm80xx_chip_intx_interrupt_enable/disable()
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
` (5 preceding siblings ...)
2023-09-11 3:02 ` [PATCH 06/10] scsi: pm8001: Simplify pm8001_chip_interrupt_enable/disable() Damien Le Moal
@ 2023-09-11 3:02 ` Damien Le Moal
2023-09-11 3:02 ` [PATCH 08/10] scsi: pm8001: Remove PM8001_USE_MSIX Damien Le Moal
` (3 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:02 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
Remove the functions pm80xx_chip_intx_interrupt_enable() and
pm80xx_chip_intx_interrupt_disable() and open code them respectively in
pm80xx_chip_interrupt_enable() and pm80xx_chip_interrupt_disable().
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/pm8001/pm80xx_hwi.c | 31 +++++--------------------------
1 file changed, 5 insertions(+), 26 deletions(-)
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index f6857632dc7c..b2749cfbbef1 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -1714,27 +1714,6 @@ static void pm80xx_hw_chip_rst(struct pm8001_hba_info *pm8001_ha)
pm8001_dbg(pm8001_ha, INIT, "chip reset finished\n");
}
-/**
- * pm80xx_chip_intx_interrupt_enable - enable PM8001 chip interrupt
- * @pm8001_ha: our hba card information
- */
-static void
-pm80xx_chip_intx_interrupt_enable(struct pm8001_hba_info *pm8001_ha)
-{
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
- pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
-}
-
-/**
- * pm80xx_chip_intx_interrupt_disable - disable PM8001 chip interrupt
- * @pm8001_ha: our hba card information
- */
-static void
-pm80xx_chip_intx_interrupt_disable(struct pm8001_hba_info *pm8001_ha)
-{
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR, ODMR_MASK_ALL);
-}
-
/**
* pm80xx_chip_interrupt_enable - enable PM8001 chip interrupt
* @pm8001_ha: our hba card information
@@ -1749,10 +1728,10 @@ pm80xx_chip_interrupt_enable(struct pm8001_hba_info *pm8001_ha, u8 vec)
else
pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR_U,
1U << (vec - 32));
- return;
+#else
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
#endif
- pm80xx_chip_intx_interrupt_enable(pm8001_ha);
-
}
/**
@@ -1773,9 +1752,9 @@ pm80xx_chip_interrupt_disable(struct pm8001_hba_info *pm8001_ha, u8 vec)
else
pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U,
1U << (vec - 32));
- return;
+#else
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR, ODMR_MASK_ALL);
#endif
- pm80xx_chip_intx_interrupt_disable(pm8001_ha);
}
/**
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 08/10] scsi: pm8001: Remove PM8001_USE_MSIX
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
` (6 preceding siblings ...)
2023-09-11 3:02 ` [PATCH 07/10] scsi: pm8001: Remove pm80xx_chip_intx_interrupt_enable/disable() Damien Le Moal
@ 2023-09-11 3:02 ` Damien Le Moal
2023-09-11 3:02 ` [PATCH 09/10] scsi: pm8001: Remove PM8001_USE_TASKLET Damien Le Moal
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:02 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
The pm8001 driver does not compile if PM8001_USE_MSIX is not defined in
pm8001_sas.h because various fields and functions conditionally defined
are used unconditionally without a "#ifdef PM8001_USE_MSIX" protection.
This macro is rather useless anyway and not convenient as diabling MSIX
use requires recompiling the driver.
Remove this macro and replace it with the bool module parameter
"use_msix" which defaults to true. The use of MSIX interrupts for an
adapter is gated by this module parameter for adapters that actually
support MSIX. The "use_msix" boolean field is added to struct
pm8001_hba_info and all code defined depending on PM8001_USE_MSIX is
modified to rely on pm8001_hba_info->use_msix instead.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/pm8001/pm8001_hwi.c | 32 +++++++++++-----------
drivers/scsi/pm8001/pm8001_init.c | 45 +++++++++++++++++++------------
drivers/scsi/pm8001/pm8001_sas.h | 7 ++---
drivers/scsi/pm8001/pm80xx_hwi.c | 38 +++++++++++++-------------
4 files changed, 67 insertions(+), 55 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index ef62afc425fc..6c9e7df0b349 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1188,13 +1188,14 @@ void pm8001_chip_iounmap(struct pm8001_hba_info *pm8001_ha)
static void
pm8001_chip_interrupt_enable(struct pm8001_hba_info *pm8001_ha, u8 vec)
{
-#ifdef PM8001_USE_MSIX
- pm8001_cw32(pm8001_ha, 0, MSIX_TABLE_BASE, MSIX_INTERRUPT_ENABLE);
- pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, 1);
-#else
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
- pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
-#endif
+ if (pm8001_ha->use_msix) {
+ pm8001_cw32(pm8001_ha, 0, MSIX_TABLE_BASE,
+ MSIX_INTERRUPT_ENABLE);
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, 1);
+ } else {
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
+ }
}
/**
@@ -1205,11 +1206,11 @@ pm8001_chip_interrupt_enable(struct pm8001_hba_info *pm8001_ha, u8 vec)
static void
pm8001_chip_interrupt_disable(struct pm8001_hba_info *pm8001_ha, u8 vec)
{
-#ifdef PM8001_USE_MSIX
- pm8001_cw32(pm8001_ha, 0, MSIX_TABLE_BASE, MSIX_INTERRUPT_DISABLE);
-#else
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
-#endif
+ if (pm8001_ha->use_msix)
+ pm8001_cw32(pm8001_ha, 0, MSIX_TABLE_BASE,
+ MSIX_INTERRUPT_DISABLE);
+ else
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
}
/**
@@ -4252,16 +4253,15 @@ static int pm8001_chip_phy_ctl_req(struct pm8001_hba_info *pm8001_ha,
static u32 pm8001_chip_is_our_interrupt(struct pm8001_hba_info *pm8001_ha)
{
-#ifdef PM8001_USE_MSIX
- return 1;
-#else
u32 value;
+ if (pm8001_ha->use_msix)
+ return 1;
+
value = pm8001_cr32(pm8001_ha, 0, MSGU_ODR);
if (value)
return 1;
return 0;
-#endif
}
/**
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 0ec43e155511..8e59d0d46cd3 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -56,6 +56,10 @@ MODULE_PARM_DESC(link_rate, "Enable link rate.\n"
" 4: Link rate 6.0G\n"
" 8: Link rate 12.0G\n");
+bool pm8001_use_msix = true;
+module_param_named(use_msix, pm8001_use_msix, bool, 0444);
+MODULE_PARM_DESC(zoned, "Use MSIX interrupts. Default: true");
+
static struct scsi_transport_template *pm8001_stt;
static int pm8001_init_ccb_tag(struct pm8001_hba_info *);
@@ -961,7 +965,6 @@ static int pm8001_configure_phy_settings(struct pm8001_hba_info *pm8001_ha)
}
}
-#ifdef PM8001_USE_MSIX
/**
* pm8001_setup_msix - enable MSI-X interrupt
* @pm8001_ha: our ha struct.
@@ -1043,7 +1046,6 @@ static u32 pm8001_request_msix(struct pm8001_hba_info *pm8001_ha)
return rc;
}
-#endif
/**
* pm8001_request_irq - register interrupt
@@ -1052,10 +1054,9 @@ static u32 pm8001_request_msix(struct pm8001_hba_info *pm8001_ha)
static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha)
{
struct pci_dev *pdev = pm8001_ha->pdev;
-#ifdef PM8001_USE_MSIX
int rc;
- if (pci_find_capability(pdev, PCI_CAP_ID_MSIX)) {
+ if (pm8001_use_msix && pci_find_capability(pdev, PCI_CAP_ID_MSIX)) {
rc = pm8001_setup_msix(pm8001_ha);
if (rc) {
pm8001_dbg(pm8001_ha, FAIL,
@@ -1063,14 +1064,22 @@ static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha)
return rc;
}
- if (pdev->msix_cap && pci_msi_enabled())
- return pm8001_request_msix(pm8001_ha);
+ if (!pdev->msix_cap || !pci_msi_enabled())
+ goto use_intx;
+
+ rc = pm8001_request_msix(pm8001_ha);
+ if (rc)
+ return rc;
+
+ pm8001_ha->use_msix = true;
+
+ return 0;
}
+use_intx:
+ /* Initialize the INT-X interrupt */
pm8001_dbg(pm8001_ha, INIT, "MSIX not supported!!!\n");
-#endif
-
- /* initialize the INT-X interrupt */
+ pm8001_ha->use_msix = false;
pm8001_ha->irq_vector[0].irq_id = 0;
pm8001_ha->irq_vector[0].drv_inst = pm8001_ha;
@@ -1081,20 +1090,22 @@ static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha)
static void pm8001_free_irq(struct pm8001_hba_info *pm8001_ha)
{
-#ifdef PM8001_USE_MSIX
struct pci_dev *pdev = pm8001_ha->pdev;
int i;
- for (i = 0; i < pm8001_ha->number_of_intr; i++)
- synchronize_irq(pci_irq_vector(pdev, i));
+ if (pm8001_ha->use_msix) {
+ for (i = 0; i < pm8001_ha->number_of_intr; i++)
+ synchronize_irq(pci_irq_vector(pdev, i));
- for (i = 0; i < pm8001_ha->number_of_intr; i++)
- free_irq(pci_irq_vector(pdev, i), &pm8001_ha->irq_vector[i]);
+ for (i = 0; i < pm8001_ha->number_of_intr; i++)
+ free_irq(pci_irq_vector(pdev, i), &pm8001_ha->irq_vector[i]);
- pci_free_irq_vectors(pdev);
-#else
+ pci_free_irq_vectors(pdev);
+ return;
+ }
+
+ /* INT-X */
free_irq(pm8001_ha->irq, pm8001_ha->sas);
-#endif
}
/**
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 2fadd353f1c1..612856b09187 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -83,8 +83,9 @@ do { \
pm8001_info(HBA, fmt, ##__VA_ARGS__); \
} while (0)
+extern bool pm8001_use_msix;
+
#define PM8001_USE_TASKLET
-#define PM8001_USE_MSIX
#define PM8001_READ_VPD
@@ -520,11 +521,11 @@ struct pm8001_hba_info {
struct pm8001_device *devices;
struct pm8001_ccb_info *ccb_info;
u32 ccb_count;
-#ifdef PM8001_USE_MSIX
+
+ bool use_msix;
int number_of_intr;/*will be used in remove()*/
char intr_drvname[PM8001_MAX_MSIX_VEC]
[PM8001_NAME_LENGTH+1+3+1];
-#endif
#ifdef PM8001_USE_TASKLET
struct tasklet_struct tasklet[PM8001_MAX_MSIX_VEC];
#endif
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index b2749cfbbef1..6041d3b88547 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -1722,16 +1722,16 @@ static void pm80xx_hw_chip_rst(struct pm8001_hba_info *pm8001_ha)
static void
pm80xx_chip_interrupt_enable(struct pm8001_hba_info *pm8001_ha, u8 vec)
{
-#ifdef PM8001_USE_MSIX
+ if (!pm8001_ha->use_msix) {
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
+ return;
+ }
+
if (vec < 32)
pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR, 1U << vec);
else
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR_U,
- 1U << (vec - 32));
-#else
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
- pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
-#endif
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR_U, 1U << (vec - 32));
}
/**
@@ -1742,19 +1742,20 @@ pm80xx_chip_interrupt_enable(struct pm8001_hba_info *pm8001_ha, u8 vec)
static void
pm80xx_chip_interrupt_disable(struct pm8001_hba_info *pm8001_ha, u8 vec)
{
-#ifdef PM8001_USE_MSIX
+ if (!pm8001_ha->use_msix) {
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR, ODMR_MASK_ALL);
+ return;
+ }
+
if (vec == 0xFF) {
/* disable all vectors 0-31, 32-63 */
pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, 0xFFFFFFFF);
pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U, 0xFFFFFFFF);
- } else if (vec < 32)
+ } else if (vec < 32) {
pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, 1U << vec);
- else
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U,
- 1U << (vec - 32));
-#else
- pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR, ODMR_MASK_ALL);
-#endif
+ } else {
+ pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U, 1U << (vec - 32));
+ }
}
/**
@@ -4779,16 +4780,15 @@ static int pm80xx_chip_phy_ctl_req(struct pm8001_hba_info *pm8001_ha,
static u32 pm80xx_chip_is_our_interrupt(struct pm8001_hba_info *pm8001_ha)
{
-#ifdef PM8001_USE_MSIX
- return 1;
-#else
u32 value;
+ if (pm8001_ha->use_msix)
+ return 1;
+
value = pm8001_cr32(pm8001_ha, 0, MSGU_ODR);
if (value)
return 1;
return 0;
-#endif
}
/**
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 09/10] scsi: pm8001: Remove PM8001_USE_TASKLET
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
` (7 preceding siblings ...)
2023-09-11 3:02 ` [PATCH 08/10] scsi: pm8001: Remove PM8001_USE_MSIX Damien Le Moal
@ 2023-09-11 3:02 ` Damien Le Moal
2023-09-11 3:02 ` [PATCH 10/10] scsi: pm8001: Remove PM8001_READ_VPD Damien Le Moal
2023-09-11 8:27 ` [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Jinpu Wang
10 siblings, 0 replies; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:02 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
Remove the macro PM8001_USE_TASKLET used to conditionally use tasklets
for MSIX interrupts handling and replace it with the boolean module
parameter pm8001_use_tasklet. This parameter defaults to true and can be
true only if pm8001_use_msix is also true.
Code conditionnaly defined with PM8001_USE_TASKLET is modified to
instead use the parameter pm8001_use_tasklet.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/pm8001/pm8001_init.c | 40 ++++++++++++++++---------------
drivers/scsi/pm8001/pm8001_sas.h | 3 ---
2 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 8e59d0d46cd3..78c22421d6fe 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -60,6 +60,10 @@ bool pm8001_use_msix = true;
module_param_named(use_msix, pm8001_use_msix, bool, 0444);
MODULE_PARM_DESC(zoned, "Use MSIX interrupts. Default: true");
+static bool pm8001_use_tasklet = true;
+module_param_named(use_tasklet, pm8001_use_tasklet, bool, 0444);
+MODULE_PARM_DESC(zoned, "Use MSIX interrupts. Default: true");
+
static struct scsi_transport_template *pm8001_stt;
static int pm8001_init_ccb_tag(struct pm8001_hba_info *);
@@ -204,8 +208,6 @@ static void pm8001_free(struct pm8001_hba_info *pm8001_ha)
kfree(pm8001_ha);
}
-#ifdef PM8001_USE_TASKLET
-
/**
* pm8001_tasklet() - tasklet for 64 msi-x interrupt handler
* @opaque: the passed general host adapter struct
@@ -213,13 +215,12 @@ static void pm8001_free(struct pm8001_hba_info *pm8001_ha)
*/
static void pm8001_tasklet(unsigned long opaque)
{
- struct pm8001_hba_info *pm8001_ha;
- struct isr_param *irq_vector;
+ struct isr_param *irq_vector = (struct isr_param *)opaque;
+ struct pm8001_hba_info *pm8001_ha = irq_vector->drv_inst;
+
+ if (WARN_ON_ONCE(!pm8001_ha))
+ return;
- irq_vector = (struct isr_param *)opaque;
- pm8001_ha = irq_vector->drv_inst;
- if (unlikely(!pm8001_ha))
- BUG_ON(1);
PM8001_CHIP_DISP->isr(pm8001_ha, irq_vector->irq_id);
}
@@ -227,6 +228,9 @@ static void pm8001_init_tasklet(struct pm8001_hba_info *pm8001_ha)
{
int i;
+ if (!pm8001_use_tasklet)
+ return;
+
/* Tasklet for non msi-x interrupt handler */
if ((!pm8001_ha->pdev->msix_cap || !pci_msi_enabled()) ||
(pm8001_ha->chip_id == chip_8001)) {
@@ -243,6 +247,9 @@ static void pm8001_kill_tasklet(struct pm8001_hba_info *pm8001_ha)
{
int i;
+ if (!pm8001_use_tasklet)
+ return;
+
/* For non-msix and msix interrupts */
if ((!pm8001_ha->pdev->msix_cap || !pci_msi_enabled()) ||
(pm8001_ha->chip_id == chip_8001)) {
@@ -254,13 +261,6 @@ static void pm8001_kill_tasklet(struct pm8001_hba_info *pm8001_ha)
tasklet_kill(&pm8001_ha->tasklet[i]);
}
-#else
-
-static void pm8001_init_tasklet(struct pm8001_hba_info *pm8001_ha) {}
-static void pm8001_kill_tasklet(struct pm8001_hba_info *pm8001_ha) {}
-
-#endif
-
static irqreturn_t pm8001_handle_irq(struct pm8001_hba_info *pm8001_ha,
int irq)
{
@@ -270,12 +270,11 @@ static irqreturn_t pm8001_handle_irq(struct pm8001_hba_info *pm8001_ha,
if (!PM8001_CHIP_DISP->is_our_interrupt(pm8001_ha))
return IRQ_NONE;
-#ifdef PM8001_USE_TASKLET
+ if (!pm8001_use_tasklet)
+ return PM8001_CHIP_DISP->isr(pm8001_ha, irq);
+
tasklet_schedule(&pm8001_ha->tasklet[irq]);
return IRQ_HANDLED;
-#else
- return PM8001_CHIP_DISP->isr(pm8001_ha, irq);
-#endif
}
/**
@@ -1538,6 +1537,9 @@ static int __init pm8001_init(void)
{
int rc = -ENOMEM;
+ if (pm8001_use_tasklet && !pm8001_use_msix)
+ pm8001_use_tasklet = false;
+
pm8001_wq = alloc_workqueue("pm80xx", 0, 0);
if (!pm8001_wq)
goto err;
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 612856b09187..e14c6668b0d3 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -85,7 +85,6 @@ do { \
extern bool pm8001_use_msix;
-#define PM8001_USE_TASKLET
#define PM8001_READ_VPD
@@ -526,9 +525,7 @@ struct pm8001_hba_info {
int number_of_intr;/*will be used in remove()*/
char intr_drvname[PM8001_MAX_MSIX_VEC]
[PM8001_NAME_LENGTH+1+3+1];
-#ifdef PM8001_USE_TASKLET
struct tasklet_struct tasklet[PM8001_MAX_MSIX_VEC];
-#endif
u32 logging_level;
u32 link_rate;
u32 fw_status;
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 10/10] scsi: pm8001: Remove PM8001_READ_VPD
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
` (8 preceding siblings ...)
2023-09-11 3:02 ` [PATCH 09/10] scsi: pm8001: Remove PM8001_USE_TASKLET Damien Le Moal
@ 2023-09-11 3:02 ` Damien Le Moal
2023-09-11 13:44 ` kernel test robot
2023-09-11 8:27 ` [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Jinpu Wang
10 siblings, 1 reply; 15+ messages in thread
From: Damien Le Moal @ 2023-09-11 3:02 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi; +Cc: Jack Wang
Remove the macro PM8001_READ_VPD used to define if a controller WWN
should be retrieved from the device. Instead, define the better named
boolean module parameter "read_wwn" to control this.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/scsi/pm8001/pm8001_init.c | 46 ++++++++++++++++++-------------
drivers/scsi/pm8001/pm8001_sas.h | 3 --
2 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 78c22421d6fe..52dcb95898fb 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -64,6 +64,10 @@ static bool pm8001_use_tasklet = true;
module_param_named(use_tasklet, pm8001_use_tasklet, bool, 0444);
MODULE_PARM_DESC(zoned, "Use MSIX interrupts. Default: true");
+static bool pm8001_read_wwn = true;
+module_param_named(read_wwn, pm8001_read_wwn, bool, 0444);
+MODULE_PARM_DESC(zoned, "Get WWN from the controller. Default: true");
+
static struct scsi_transport_template *pm8001_stt;
static int pm8001_init_ccb_tag(struct pm8001_hba_info *);
@@ -685,11 +689,24 @@ static int pm8001_init_sas_add(struct pm8001_hba_info *pm8001_ha)
{
u8 i, j;
u8 sas_add[8];
-#ifdef PM8001_READ_VPD
- /* For new SPC controllers WWN is stored in flash vpd
- * For SPC/SPCve controllers WWN is stored in EEPROM
- * For Older SPC WWN is stored in NVMD
- */
+
+ if (!pm8001_read_wwn) {
+ for (i = 0; i < pm8001_ha->chip->n_phy; i++) {
+ pm8001_ha->phy[i].dev_sas_addr = 0x50010c600047f9d0ULL;
+ pm8001_ha->phy[i].dev_sas_addr =
+ cpu_to_be64((u64)
+ (*(u64 *)&pm8001_ha->phy[i].dev_sas_addr));
+ }
+ memcpy(pm8001_ha->sas_addr, &pm8001_ha->phy[0].dev_sas_addr,
+ SAS_ADDR_SIZE);
+ return 0;
+ }
+
+ /*
+ * For new SPC controllers WWN is stored in flash vpd. For SPC/SPCve
+ * controllers WWN is stored in EEPROM. And for Older SPC WWN is stored
+ * in NVMD.
+ */
DECLARE_COMPLETION_ONSTACK(completion);
struct pm8001_ioctl_payload payload;
u16 deviceid;
@@ -769,16 +786,7 @@ static int pm8001_init_sas_add(struct pm8001_hba_info *pm8001_ha)
pm8001_ha->phy[i].dev_sas_addr);
}
kfree(payload.func_specific);
-#else
- for (i = 0; i < pm8001_ha->chip->n_phy; i++) {
- pm8001_ha->phy[i].dev_sas_addr = 0x50010c600047f9d0ULL;
- pm8001_ha->phy[i].dev_sas_addr =
- cpu_to_be64((u64)
- (*(u64 *)&pm8001_ha->phy[i].dev_sas_addr));
- }
- memcpy(pm8001_ha->sas_addr, &pm8001_ha->phy[0].dev_sas_addr,
- SAS_ADDR_SIZE);
-#endif
+
return 0;
}
@@ -788,13 +796,13 @@ static int pm8001_init_sas_add(struct pm8001_hba_info *pm8001_ha)
*/
static int pm8001_get_phy_settings_info(struct pm8001_hba_info *pm8001_ha)
{
-
-#ifdef PM8001_READ_VPD
- /*OPTION ROM FLASH read for the SPC cards */
DECLARE_COMPLETION_ONSTACK(completion);
struct pm8001_ioctl_payload payload;
int rc;
+ if (!pm8001_read_wwn)
+ return 0;
+
pm8001_ha->nvmd_completion = &completion;
/* SAS ADDRESS read from flash / EEPROM */
payload.minor_function = 6;
@@ -813,7 +821,7 @@ static int pm8001_get_phy_settings_info(struct pm8001_hba_info *pm8001_ha)
wait_for_completion(&completion);
pm8001_set_phy_profile(pm8001_ha, sizeof(u8), payload.func_specific);
kfree(payload.func_specific);
-#endif
+
return 0;
}
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index e14c6668b0d3..3ccb7371902f 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -85,9 +85,6 @@ do { \
extern bool pm8001_use_msix;
-#define PM8001_READ_VPD
-
-
#define IS_SPCV_12G(dev) ((dev->device == 0X8074) \
|| (dev->device == 0X8076) \
|| (dev->device == 0X8077) \
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 10/10] scsi: pm8001: Remove PM8001_READ_VPD
2023-09-11 3:02 ` [PATCH 10/10] scsi: pm8001: Remove PM8001_READ_VPD Damien Le Moal
@ 2023-09-11 13:44 ` kernel test robot
0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-09-11 13:44 UTC (permalink / raw)
To: Damien Le Moal, Martin K . Petersen, linux-scsi; +Cc: oe-kbuild-all, Jack Wang
Hi Damien,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.6-rc1 next-20230911]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Damien-Le-Moal/scsi-pm8001-Setup-IRQs-on-resume/20230911-110427
base: linus/master
patch link: https://lore.kernel.org/r/20230911030207.242917-11-dlemoal%40kernel.org
patch subject: [PATCH 10/10] scsi: pm8001: Remove PM8001_READ_VPD
config: i386-randconfig-063-20230911 (https://download.01.org/0day-ci/archive/20230911/202309112107.YfM4eB8f-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230911/202309112107.YfM4eB8f-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309112107.YfM4eB8f-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/scsi/pm8001/pm8001_init.c:696:56: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned long long [usertype] dev_sas_addr @@ got restricted __be64 [usertype] @@
drivers/scsi/pm8001/pm8001_init.c:696:56: sparse: expected unsigned long long [usertype] dev_sas_addr
drivers/scsi/pm8001/pm8001_init.c:696:56: sparse: got restricted __be64 [usertype]
vim +696 drivers/scsi/pm8001/pm8001_init.c
680
681 /**
682 * pm8001_init_sas_add - initialize sas address
683 * @pm8001_ha: our ha struct.
684 *
685 * Currently we just set the fixed SAS address to our HBA, for manufacture,
686 * it should read from the EEPROM
687 */
688 static int pm8001_init_sas_add(struct pm8001_hba_info *pm8001_ha)
689 {
690 u8 i, j;
691 u8 sas_add[8];
692
693 if (!pm8001_read_wwn) {
694 for (i = 0; i < pm8001_ha->chip->n_phy; i++) {
695 pm8001_ha->phy[i].dev_sas_addr = 0x50010c600047f9d0ULL;
> 696 pm8001_ha->phy[i].dev_sas_addr =
697 cpu_to_be64((u64)
698 (*(u64 *)&pm8001_ha->phy[i].dev_sas_addr));
699 }
700 memcpy(pm8001_ha->sas_addr, &pm8001_ha->phy[0].dev_sas_addr,
701 SAS_ADDR_SIZE);
702 return 0;
703 }
704
705 /*
706 * For new SPC controllers WWN is stored in flash vpd. For SPC/SPCve
707 * controllers WWN is stored in EEPROM. And for Older SPC WWN is stored
708 * in NVMD.
709 */
710 DECLARE_COMPLETION_ONSTACK(completion);
711 struct pm8001_ioctl_payload payload;
712 u16 deviceid;
713 int rc;
714 unsigned long time_remaining;
715
716 if (PM8001_CHIP_DISP->fatal_errors(pm8001_ha)) {
717 pm8001_dbg(pm8001_ha, FAIL, "controller is in fatal error state\n");
718 return -EIO;
719 }
720
721 pci_read_config_word(pm8001_ha->pdev, PCI_DEVICE_ID, &deviceid);
722 pm8001_ha->nvmd_completion = &completion;
723
724 if (pm8001_ha->chip_id == chip_8001) {
725 if (deviceid == 0x8081 || deviceid == 0x0042) {
726 payload.minor_function = 4;
727 payload.rd_length = 4096;
728 } else {
729 payload.minor_function = 0;
730 payload.rd_length = 128;
731 }
732 } else if ((pm8001_ha->chip_id == chip_8070 ||
733 pm8001_ha->chip_id == chip_8072) &&
734 pm8001_ha->pdev->subsystem_vendor == PCI_VENDOR_ID_ATTO) {
735 payload.minor_function = 4;
736 payload.rd_length = 4096;
737 } else {
738 payload.minor_function = 1;
739 payload.rd_length = 4096;
740 }
741 payload.offset = 0;
742 payload.func_specific = kzalloc(payload.rd_length, GFP_KERNEL);
743 if (!payload.func_specific) {
744 pm8001_dbg(pm8001_ha, FAIL, "mem alloc fail\n");
745 return -ENOMEM;
746 }
747 rc = PM8001_CHIP_DISP->get_nvmd_req(pm8001_ha, &payload);
748 if (rc) {
749 kfree(payload.func_specific);
750 pm8001_dbg(pm8001_ha, FAIL, "nvmd failed\n");
751 return -EIO;
752 }
753 time_remaining = wait_for_completion_timeout(&completion,
754 msecs_to_jiffies(60*1000)); // 1 min
755 if (!time_remaining) {
756 kfree(payload.func_specific);
757 pm8001_dbg(pm8001_ha, FAIL, "get_nvmd_req timeout\n");
758 return -EIO;
759 }
760
761
762 for (i = 0, j = 0; i <= 7; i++, j++) {
763 if (pm8001_ha->chip_id == chip_8001) {
764 if (deviceid == 0x8081)
765 pm8001_ha->sas_addr[j] =
766 payload.func_specific[0x704 + i];
767 else if (deviceid == 0x0042)
768 pm8001_ha->sas_addr[j] =
769 payload.func_specific[0x010 + i];
770 } else if ((pm8001_ha->chip_id == chip_8070 ||
771 pm8001_ha->chip_id == chip_8072) &&
772 pm8001_ha->pdev->subsystem_vendor == PCI_VENDOR_ID_ATTO) {
773 pm8001_ha->sas_addr[j] =
774 payload.func_specific[0x010 + i];
775 } else
776 pm8001_ha->sas_addr[j] =
777 payload.func_specific[0x804 + i];
778 }
779 memcpy(sas_add, pm8001_ha->sas_addr, SAS_ADDR_SIZE);
780 for (i = 0; i < pm8001_ha->chip->n_phy; i++) {
781 if (i && ((i % 4) == 0))
782 sas_add[7] = sas_add[7] + 4;
783 memcpy(&pm8001_ha->phy[i].dev_sas_addr,
784 sas_add, SAS_ADDR_SIZE);
785 pm8001_dbg(pm8001_ha, INIT, "phy %d sas_addr = %016llx\n", i,
786 pm8001_ha->phy[i].dev_sas_addr);
787 }
788 kfree(payload.func_specific);
789
790 return 0;
791 }
792
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/10] scsi: pm8001: Bug fix and cleanup
2023-09-11 3:01 [PATCH 00/10] scsi: pm8001: Bug fix and cleanup Damien Le Moal
` (9 preceding siblings ...)
2023-09-11 3:02 ` [PATCH 10/10] scsi: pm8001: Remove PM8001_READ_VPD Damien Le Moal
@ 2023-09-11 8:27 ` Jinpu Wang
10 siblings, 0 replies; 15+ messages in thread
From: Jinpu Wang @ 2023-09-11 8:27 UTC (permalink / raw)
To: Damien Le Moal; +Cc: Martin K . Petersen, linux-scsi
On Mon, Sep 11, 2023 at 5:02 AM Damien Le Moal <dlemoal@kernel.org> wrote:
>
> The first patch of this series fixes an issue with IRQ setup which
> prevents the controller from resuming after a system suspend.
> The following patches are code cleanup without any functional changes.
>
> Damien Le Moal (10):
> scsi: pm8001: Setup IRQs on resume
> scsi: pm8001: Introduce pm8001_free_irq()
> scsi: pm8001: Introduce pm8001_init_tasklet()
> scsi: pm8001: Introduce pm8001_kill_tasklet()
> scsi: pm8001: Introduce pm8001_handle_irq()
> scsi: pm8001: Simplify pm8001_chip_interrupt_enable/disable()
> scsi: pm8001: Remove pm80xx_chip_intx_interrupt_enable/disable()
> scsi: pm8001: Remove PM8001_USE_MSIX
> scsi: pm8001: Remove PM8001_USE_TASKLET
> scsi: pm8001: Remove PM8001_READ_VPD
>
> drivers/scsi/pm8001/pm8001_hwi.c | 89 ++-------
> drivers/scsi/pm8001/pm8001_init.c | 322 +++++++++++++++---------------
> drivers/scsi/pm8001/pm8001_sas.h | 11 +-
> drivers/scsi/pm8001/pm80xx_hwi.c | 59 ++----
> 4 files changed, 202 insertions(+), 279 deletions(-)
>
> --
> 2.41.0
>
Thanks for the patchset.
Acked-by: Jack Wang <jinpu.wang@ionos.com>
^ permalink raw reply [flat|nested] 15+ messages in thread