[PATCH 1/2] ipr: add test for MSI interrupt support

[PATCH 1/2] ipr: add test for MSI interrupt support

[Wayne Boyer]

The return value from pci_enable_msi() can not always be trusted.  This patch
adds code to generate an interrupt after MSI has been enabled and tests
whether or not we can receive and process it.  If the tests fails, then fall
back to LSI.

Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
---

 drivers/scsi/ipr.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++----
 drivers/scsi/ipr.h |    6 +-
 2 files changed, 105 insertions(+), 9 deletions(-)

Index: b/drivers/scsi/ipr.c
===================================================================
--- a/drivers/scsi/ipr.c	2009-06-03 16:04:14.000000000 -0700
+++ b/drivers/scsi/ipr.c	2009-06-15 16:36:36.000000000 -0700
@@ -7366,6 +7366,7 @@
 	INIT_LIST_HEAD(&ioa_cfg->used_res_q);
 	INIT_WORK(&ioa_cfg->work_q, ipr_worker_thread);
 	init_waitqueue_head(&ioa_cfg->reset_wait_q);
+	init_waitqueue_head(&ioa_cfg->msi_wait_q);
 	ioa_cfg->sdt_state = INACTIVE;
 	if (ipr_enable_cache)
 		ioa_cfg->cache_state = CACHE_ENABLED;
@@ -7416,6 +7417,89 @@
 }

 /**
+ * ipr_test_intr - Handle the interrupt generated in ipr_test_msi().
+ * @pdev:		PCI device struct
+ *
+ * Description: Simply set the msi_received flag to 1 indicating that
+ * Message Signaled Interrupts are supported.
+ *
+ * Return value:
+ * 	0 on success / non-zero on failure
+ **/
+static irqreturn_t __devinit ipr_test_intr(int irq, void *devp)
+{
+	struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)devp;
+	unsigned long lock_flags = 0;
+	irqreturn_t rc = IRQ_HANDLED;
+
+	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+
+	ioa_cfg->msi_received = 1;
+	wake_up(&ioa_cfg->msi_wait_q);
+
+	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+	return rc;
+}
+
+/**
+ * ipr_test_msi - Test for Message Signaled Interrupt (MSI) support.
+ * @pdev:		PCI device struct
+ *
+ * Description: The return value from pci_enable_msi() can not always be
+ * trusted.  This routine sets up and initiates a test interrupt to determine
+ * if the interrupt is received via the ipr_test_intr() service routine.
+ * If the tests fails, the driver will fall back to LSI.
+ *
+ * Return value:
+ * 	0 on success / non-zero on failure
+ **/
+static int __devinit ipr_test_msi(struct ipr_ioa_cfg *ioa_cfg,
+				  struct pci_dev *pdev)
+{
+	int rc;
+	volatile u32 int_reg;
+	unsigned long lock_flags = 0;
+
+	ENTER;
+
+	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+	init_waitqueue_head(&ioa_cfg->msi_wait_q);
+	ioa_cfg->msi_received = 0;
+	ipr_mask_and_clear_interrupts(ioa_cfg, ~IPR_PCII_IOA_TRANS_TO_OPER);
+	writel(IPR_PCII_IO_DEBUG_ACKNOWLEDGE, ioa_cfg->regs.clr_interrupt_mask_reg);
+	int_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg);
+	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+
+	rc = request_irq(pdev->irq, ipr_test_intr, 0, IPR_NAME, ioa_cfg);
+	if (rc) {
+		dev_err(&pdev->dev, "Can not assign irq %d\n", pdev->irq);
+		return rc;
+	} else if (ipr_debug)
+		dev_info(&pdev->dev, "IRQ assigned: %d\n", pdev->irq);
+
+	writel(IPR_PCII_IO_DEBUG_ACKNOWLEDGE, ioa_cfg->regs.sense_interrupt_reg);
+	int_reg = readl(ioa_cfg->regs.sense_interrupt_reg);
+	wait_event_timeout(ioa_cfg->msi_wait_q, ioa_cfg->msi_received, HZ);
+	ipr_mask_and_clear_interrupts(ioa_cfg, ~IPR_PCII_IOA_TRANS_TO_OPER);
+
+	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+	if (!ioa_cfg->msi_received) {
+		/* MSI test failed */
+		dev_info(&pdev->dev, "MSI test failed.  Falling back to LSI.\n");
+		rc = -EOPNOTSUPP;
+	} else if (ipr_debug)
+		dev_info(&pdev->dev, "MSI test succeeded.\n");
+
+	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+
+	free_irq(pdev->irq, ioa_cfg);
+
+	LEAVE;
+
+	return rc;
+}
+
+/**
  * ipr_probe_ioa - Allocates memory and does first stage of initialization
  * @pdev:		PCI device struct
  * @dev_id:		PCI device id struct
@@ -7440,11 +7524,6 @@
 		goto out;
 	}

-	if (!(rc = pci_enable_msi(pdev)))
-		dev_info(&pdev->dev, "MSI enabled\n");
-	else if (ipr_debug)
-		dev_info(&pdev->dev, "Cannot enable MSI\n");
-
 	dev_info(&pdev->dev, "Found IOA with IRQ: %d\n", pdev->irq);

 	host = scsi_host_alloc(&driver_template, sizeof(*ioa_cfg));
@@ -7518,6 +7597,18 @@
 		goto cleanup_nomem;
 	}

+	/* Enable MSI style interrupts if they are supported. */
+	if (!(rc = pci_enable_msi(pdev))) {
+		rc = ipr_test_msi(ioa_cfg, pdev);
+		if (rc == -EOPNOTSUPP)
+			pci_disable_msi(pdev);
+		else if (rc)
+			goto out_msi_disable;
+		else
+			dev_info(&pdev->dev, "MSI enabled with IRQ: %d\n", pdev->irq);
+	} else if (ipr_debug)
+		dev_info(&pdev->dev, "Cannot enable MSI.\n");
+
 	/* Save away PCI config space for use following IOA reset */
 	rc = pci_save_state(pdev);

@@ -7555,7 +7646,9 @@
 		ioa_cfg->ioa_unit_checked = 1;

 	ipr_mask_and_clear_interrupts(ioa_cfg, ~IPR_PCII_IOA_TRANS_TO_OPER);
-	rc = request_irq(pdev->irq, ipr_isr, IRQF_SHARED, IPR_NAME, ioa_cfg);
+	rc = request_irq(pdev->irq, ipr_isr,
+			 ioa_cfg->msi_received ? 0 : IRQF_SHARED,
+			 IPR_NAME, ioa_cfg);

 	if (rc) {
 		dev_err(&pdev->dev, "Couldn't register IRQ %d! rc=%d\n",
@@ -7582,12 +7675,13 @@
 	ipr_free_mem(ioa_cfg);
 cleanup_nomem:
 	iounmap(ipr_regs);
+out_msi_disable:
+	pci_disable_msi(pdev);
 out_release_regions:
 	pci_release_regions(pdev);
 out_scsi_host_put:
 	scsi_host_put(host);
 out_disable:
-	pci_disable_msi(pdev);
 	pci_disable_device(pdev);
 	goto out;
 }
Index: b/drivers/scsi/ipr.h
===================================================================
--- a/drivers/scsi/ipr.h	2009-06-03 16:04:15.000000000 -0700
+++ b/drivers/scsi/ipr.h	2009-06-15 16:35:43.000000000 -0700
@@ -37,8 +37,8 @@
 /*
  * Literals
  */
-#define IPR_DRIVER_VERSION "2.4.2"
-#define IPR_DRIVER_DATE "(January 21, 2009)"
+#define IPR_DRIVER_VERSION "2.4.3"
+#define IPR_DRIVER_DATE "(June 10, 2009)"

 /*
  * IPR_MAX_CMD_PER_LUN: This defines the maximum number of outstanding
@@ -1094,6 +1094,7 @@
 	u8 needs_hard_reset:1;
 	u8 dual_raid:1;
 	u8 needs_warm_reset:1;
+	u8 msi_received:1;

 	u8 revid;

@@ -1179,6 +1180,7 @@
 	struct work_struct work_q;

 	wait_queue_head_t reset_wait_q;
+	wait_queue_head_t msi_wait_q;

 	struct ipr_dump *dump;
 	enum ipr_sdt_state sdt_state;

[PATCH 2/2] ipr: differentiate pci-x and pci-e based adapters

[Wayne Boyer]

MSI has only been tested on and known to work with PCI-E based adapters.  This
patch adds a field to struct ipr_chip_t to indicate which type of interrupt to
use based on what is known about the chip.

Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
---

 drivers/scsi/ipr.c |   29 +++++++++++++++--------------
 drivers/scsi/ipr.h |    4 ++++
 2 files changed, 19 insertions(+), 14 deletions(-)

Index: b/drivers/scsi/ipr.c
===================================================================
--- a/drivers/scsi/ipr.c	2009-06-15 16:36:36.000000000 -0700
+++ b/drivers/scsi/ipr.c	2009-06-15 16:36:44.000000000 -0700
@@ -131,13 +131,13 @@
 };

 static const struct ipr_chip_t ipr_chip[] = {
-	{ PCI_VENDOR_ID_MYLEX, PCI_DEVICE_ID_IBM_GEMSTONE, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CITRINE, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_OBSIDIAN, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN_E, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_SNIPE, &ipr_chip_cfg[1] },
-	{ PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_SCAMP, &ipr_chip_cfg[1] }
+	{ PCI_VENDOR_ID_MYLEX, PCI_DEVICE_ID_IBM_GEMSTONE, IPR_USE_LSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CITRINE, IPR_USE_LSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_OBSIDIAN, IPR_USE_LSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN, IPR_USE_LSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN_E, IPR_USE_MSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_SNIPE, IPR_USE_LSI, &ipr_chip_cfg[1] },
+	{ PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_SCAMP, IPR_USE_LSI, &ipr_chip_cfg[1] }
 };

 static int ipr_max_bus_speeds [] = {
@@ -7398,21 +7398,21 @@
 }

 /**
- * ipr_get_chip_cfg - Find adapter chip configuration
+ * ipr_get_chip_info - Find adapter chip information
  * @dev_id:		PCI device id struct
  *
  * Return value:
- * 	ptr to chip config on success / NULL on failure
+ * 	ptr to chip information on success / NULL on failure
  **/
-static const struct ipr_chip_cfg_t * __devinit
-ipr_get_chip_cfg(const struct pci_device_id *dev_id)
+static const struct ipr_chip_t * __devinit
+ipr_get_chip_info(const struct pci_device_id *dev_id)
 {
 	int i;

 	for (i = 0; i < ARRAY_SIZE(ipr_chip); i++)
 		if (ipr_chip[i].vendor == dev_id->vendor &&
 		    ipr_chip[i].device == dev_id->device)
-			return ipr_chip[i].cfg;
+			return &ipr_chip[i];
 	return NULL;
 }

@@ -7539,7 +7539,8 @@
 	ata_host_init(&ioa_cfg->ata_host, &pdev->dev,
 		      sata_port_info.flags, &ipr_sata_ops);

-	ioa_cfg->chip_cfg = ipr_get_chip_cfg(dev_id);
+	ioa_cfg->ipr_chip = ipr_get_chip_info(dev_id);
+	ioa_cfg->chip_cfg = ioa_cfg->ipr_chip->cfg;

 	if (!ioa_cfg->chip_cfg) {
 		dev_err(&pdev->dev, "Unknown adapter chipset 0x%04X 0x%04X\n",
@@ -7598,7 +7599,7 @@
 	}

 	/* Enable MSI style interrupts if they are supported. */
-	if (!(rc = pci_enable_msi(pdev))) {
+	if (ioa_cfg->ipr_chip->intr_type == IPR_USE_MSI && !pci_enable_msi(pdev)) {
 		rc = ipr_test_msi(ioa_cfg, pdev);
 		if (rc == -EOPNOTSUPP)
 			pci_disable_msi(pdev);
Index: b/drivers/scsi/ipr.h
===================================================================
--- a/drivers/scsi/ipr.h	2009-06-15 16:35:43.000000000 -0700
+++ b/drivers/scsi/ipr.h	2009-06-15 16:36:44.000000000 -0700
@@ -1025,6 +1025,9 @@
 struct ipr_chip_t {
 	u16 vendor;
 	u16 device;
+	u16 intr_type;
+#define IPR_USE_LSI			0x00
+#define IPR_USE_MSI			0x01
 	const struct ipr_chip_cfg_t *cfg;
 };

@@ -1160,6 +1163,7 @@

 	unsigned int transop_timeout;
 	const struct ipr_chip_cfg_t *chip_cfg;
+	const struct ipr_chip_t *ipr_chip;

 	void __iomem *hdw_dma_regs;	/* iomapped PCI memory space */
 	unsigned long hdw_dma_regs_pci;	/* raw PCI memory space */

Re: [PATCH 1/2] ipr: add test for MSI interrupt support

[Brian King]

Acked-by: Brian King <brking@linux.vnet.ibm.com>

Wayne Boyer wrote:
> The return value from pci_enable_msi() can not always be trusted.  This patch
> adds code to generate an interrupt after MSI has been enabled and tests
> whether or not we can receive and process it.  If the tests fails, then fall
> back to LSI.
> 
> Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
> ---
> 
>  drivers/scsi/ipr.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++----
>  drivers/scsi/ipr.h |    6 +-
>  2 files changed, 105 insertions(+), 9 deletions(-)
> 
> Index: b/drivers/scsi/ipr.c
> ===================================================================
> --- a/drivers/scsi/ipr.c	2009-06-03 16:04:14.000000000 -0700
> +++ b/drivers/scsi/ipr.c	2009-06-15 16:36:36.000000000 -0700
> @@ -7366,6 +7366,7 @@
>  	INIT_LIST_HEAD(&ioa_cfg->used_res_q);
>  	INIT_WORK(&ioa_cfg->work_q, ipr_worker_thread);
>  	init_waitqueue_head(&ioa_cfg->reset_wait_q);
> +	init_waitqueue_head(&ioa_cfg->msi_wait_q);
>  	ioa_cfg->sdt_state = INACTIVE;
>  	if (ipr_enable_cache)
>  		ioa_cfg->cache_state = CACHE_ENABLED;
> @@ -7416,6 +7417,89 @@
>  }
> 
>  /**
> + * ipr_test_intr - Handle the interrupt generated in ipr_test_msi().
> + * @pdev:		PCI device struct
> + *
> + * Description: Simply set the msi_received flag to 1 indicating that
> + * Message Signaled Interrupts are supported.
> + *
> + * Return value:
> + * 	0 on success / non-zero on failure
> + **/
> +static irqreturn_t __devinit ipr_test_intr(int irq, void *devp)
> +{
> +	struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)devp;
> +	unsigned long lock_flags = 0;
> +	irqreturn_t rc = IRQ_HANDLED;
> +
> +	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
> +
> +	ioa_cfg->msi_received = 1;
> +	wake_up(&ioa_cfg->msi_wait_q);
> +
> +	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
> +	return rc;
> +}
> +
> +/**
> + * ipr_test_msi - Test for Message Signaled Interrupt (MSI) support.
> + * @pdev:		PCI device struct
> + *
> + * Description: The return value from pci_enable_msi() can not always be
> + * trusted.  This routine sets up and initiates a test interrupt to determine
> + * if the interrupt is received via the ipr_test_intr() service routine.
> + * If the tests fails, the driver will fall back to LSI.
> + *
> + * Return value:
> + * 	0 on success / non-zero on failure
> + **/
> +static int __devinit ipr_test_msi(struct ipr_ioa_cfg *ioa_cfg,
> +				  struct pci_dev *pdev)
> +{
> +	int rc;
> +	volatile u32 int_reg;
> +	unsigned long lock_flags = 0;
> +
> +	ENTER;
> +
> +	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
> +	init_waitqueue_head(&ioa_cfg->msi_wait_q);
> +	ioa_cfg->msi_received = 0;
> +	ipr_mask_and_clear_interrupts(ioa_cfg, ~IPR_PCII_IOA_TRANS_TO_OPER);
> +	writel(IPR_PCII_IO_DEBUG_ACKNOWLEDGE, ioa_cfg->regs.clr_interrupt_mask_reg);
> +	int_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg);
> +	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
> +
> +	rc = request_irq(pdev->irq, ipr_test_intr, 0, IPR_NAME, ioa_cfg);
> +	if (rc) {
> +		dev_err(&pdev->dev, "Can not assign irq %d\n", pdev->irq);
> +		return rc;
> +	} else if (ipr_debug)
> +		dev_info(&pdev->dev, "IRQ assigned: %d\n", pdev->irq);
> +
> +	writel(IPR_PCII_IO_DEBUG_ACKNOWLEDGE, ioa_cfg->regs.sense_interrupt_reg);
> +	int_reg = readl(ioa_cfg->regs.sense_interrupt_reg);
> +	wait_event_timeout(ioa_cfg->msi_wait_q, ioa_cfg->msi_received, HZ);
> +	ipr_mask_and_clear_interrupts(ioa_cfg, ~IPR_PCII_IOA_TRANS_TO_OPER);
> +
> +	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
> +	if (!ioa_cfg->msi_received) {
> +		/* MSI test failed */
> +		dev_info(&pdev->dev, "MSI test failed.  Falling back to LSI.\n");
> +		rc = -EOPNOTSUPP;
> +	} else if (ipr_debug)
> +		dev_info(&pdev->dev, "MSI test succeeded.\n");
> +
> +	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
> +
> +	free_irq(pdev->irq, ioa_cfg);
> +
> +	LEAVE;
> +
> +	return rc;
> +}
> +
> +/**
>   * ipr_probe_ioa - Allocates memory and does first stage of initialization
>   * @pdev:		PCI device struct
>   * @dev_id:		PCI device id struct
> @@ -7440,11 +7524,6 @@
>  		goto out;
>  	}
> 
> -	if (!(rc = pci_enable_msi(pdev)))
> -		dev_info(&pdev->dev, "MSI enabled\n");
> -	else if (ipr_debug)
> -		dev_info(&pdev->dev, "Cannot enable MSI\n");
> -
>  	dev_info(&pdev->dev, "Found IOA with IRQ: %d\n", pdev->irq);
> 
>  	host = scsi_host_alloc(&driver_template, sizeof(*ioa_cfg));
> @@ -7518,6 +7597,18 @@
>  		goto cleanup_nomem;
>  	}
> 
> +	/* Enable MSI style interrupts if they are supported. */
> +	if (!(rc = pci_enable_msi(pdev))) {
> +		rc = ipr_test_msi(ioa_cfg, pdev);
> +		if (rc == -EOPNOTSUPP)
> +			pci_disable_msi(pdev);
> +		else if (rc)
> +			goto out_msi_disable;
> +		else
> +			dev_info(&pdev->dev, "MSI enabled with IRQ: %d\n", pdev->irq);
> +	} else if (ipr_debug)
> +		dev_info(&pdev->dev, "Cannot enable MSI.\n");
> +
>  	/* Save away PCI config space for use following IOA reset */
>  	rc = pci_save_state(pdev);
> 
> @@ -7555,7 +7646,9 @@
>  		ioa_cfg->ioa_unit_checked = 1;
> 
>  	ipr_mask_and_clear_interrupts(ioa_cfg, ~IPR_PCII_IOA_TRANS_TO_OPER);
> -	rc = request_irq(pdev->irq, ipr_isr, IRQF_SHARED, IPR_NAME, ioa_cfg);
> +	rc = request_irq(pdev->irq, ipr_isr,
> +			 ioa_cfg->msi_received ? 0 : IRQF_SHARED,
> +			 IPR_NAME, ioa_cfg);
> 
>  	if (rc) {
>  		dev_err(&pdev->dev, "Couldn't register IRQ %d! rc=%d\n",
> @@ -7582,12 +7675,13 @@
>  	ipr_free_mem(ioa_cfg);
>  cleanup_nomem:
>  	iounmap(ipr_regs);
> +out_msi_disable:
> +	pci_disable_msi(pdev);
>  out_release_regions:
>  	pci_release_regions(pdev);
>  out_scsi_host_put:
>  	scsi_host_put(host);
>  out_disable:
> -	pci_disable_msi(pdev);
>  	pci_disable_device(pdev);
>  	goto out;
>  }
> Index: b/drivers/scsi/ipr.h
> ===================================================================
> --- a/drivers/scsi/ipr.h	2009-06-03 16:04:15.000000000 -0700
> +++ b/drivers/scsi/ipr.h	2009-06-15 16:35:43.000000000 -0700
> @@ -37,8 +37,8 @@
>  /*
>   * Literals
>   */
> -#define IPR_DRIVER_VERSION "2.4.2"
> -#define IPR_DRIVER_DATE "(January 21, 2009)"
> +#define IPR_DRIVER_VERSION "2.4.3"
> +#define IPR_DRIVER_DATE "(June 10, 2009)"
> 
>  /*
>   * IPR_MAX_CMD_PER_LUN: This defines the maximum number of outstanding
> @@ -1094,6 +1094,7 @@
>  	u8 needs_hard_reset:1;
>  	u8 dual_raid:1;
>  	u8 needs_warm_reset:1;
> +	u8 msi_received:1;
> 
>  	u8 revid;
> 
> @@ -1179,6 +1180,7 @@
>  	struct work_struct work_q;
> 
>  	wait_queue_head_t reset_wait_q;
> +	wait_queue_head_t msi_wait_q;
> 
>  	struct ipr_dump *dump;
>  	enum ipr_sdt_state sdt_state;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center

Re: [PATCH 2/2] ipr: differentiate pci-x and pci-e based adapters

[Brian King]

Wayne Boyer wrote:
> @@ -7539,7 +7539,8 @@
>  	ata_host_init(&ioa_cfg->ata_host, &pdev->dev,
>  		      sata_port_info.flags, &ipr_sata_ops);
> 
> -	ioa_cfg->chip_cfg = ipr_get_chip_cfg(dev_id);
> +	ioa_cfg->ipr_chip = ipr_get_chip_info(dev_id);
> +	ioa_cfg->chip_cfg = ioa_cfg->ipr_chip->cfg;

If ipr_get_chip_info returns NULL you will oops here.

-Brian

-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center

Re: [PATCH 2/2] ipr: differentiate pci-x and pci-e based adapters

[Wayne Boyer]

Brian King wrote:
> Wayne Boyer wrote:
>> @@ -7539,7 +7539,8 @@
>>  	ata_host_init(&ioa_cfg->ata_host, &pdev->dev,
>>  		      sata_port_info.flags, &ipr_sata_ops);
>>
>> -	ioa_cfg->chip_cfg = ipr_get_chip_cfg(dev_id);
>> +	ioa_cfg->ipr_chip = ipr_get_chip_info(dev_id);
>> +	ioa_cfg->chip_cfg = ioa_cfg->ipr_chip->cfg;
> 
> If ipr_get_chip_info returns NULL you will oops here.
> 

Updated patch below.

-- 
Wayne Boyer
IBM - Beaverton, Oregon
LTC S/W Development - eServerIO

---

MSI has only been tested on and known to work with PCI-E based adapters.  This
patch adds a field to struct ipr_chip_t to indicate which type of interrupt to
use based on what is known about the chip.

Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
---

 drivers/scsi/ipr.c |   32 +++++++++++++++++---------------
 drivers/scsi/ipr.h |    4 ++++
 2 files changed, 21 insertions(+), 15 deletions(-)

Index: b/drivers/scsi/ipr.c
===================================================================
--- a/drivers/scsi/ipr.c	2009-06-15 16:36:36.000000000 -0700
+++ b/drivers/scsi/ipr.c	2009-06-17 09:00:02.000000000 -0700
@@ -131,13 +131,13 @@
 };

 static const struct ipr_chip_t ipr_chip[] = {
-	{ PCI_VENDOR_ID_MYLEX, PCI_DEVICE_ID_IBM_GEMSTONE, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CITRINE, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_OBSIDIAN, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN_E, &ipr_chip_cfg[0] },
-	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_SNIPE, &ipr_chip_cfg[1] },
-	{ PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_SCAMP, &ipr_chip_cfg[1] }
+	{ PCI_VENDOR_ID_MYLEX, PCI_DEVICE_ID_IBM_GEMSTONE, IPR_USE_LSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CITRINE, IPR_USE_LSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_OBSIDIAN, IPR_USE_LSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN, IPR_USE_LSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN_E, IPR_USE_MSI, &ipr_chip_cfg[0] },
+	{ PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_SNIPE, IPR_USE_LSI, &ipr_chip_cfg[1] },
+	{ PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_SCAMP, IPR_USE_LSI, &ipr_chip_cfg[1] }
 };

 static int ipr_max_bus_speeds [] = {
@@ -7398,21 +7398,21 @@
 }

 /**
- * ipr_get_chip_cfg - Find adapter chip configuration
+ * ipr_get_chip_info - Find adapter chip information
  * @dev_id:		PCI device id struct
  *
  * Return value:
- * 	ptr to chip config on success / NULL on failure
+ * 	ptr to chip information on success / NULL on failure
  **/
-static const struct ipr_chip_cfg_t * __devinit
-ipr_get_chip_cfg(const struct pci_device_id *dev_id)
+static const struct ipr_chip_t * __devinit
+ipr_get_chip_info(const struct pci_device_id *dev_id)
 {
 	int i;

 	for (i = 0; i < ARRAY_SIZE(ipr_chip); i++)
 		if (ipr_chip[i].vendor == dev_id->vendor &&
 		    ipr_chip[i].device == dev_id->device)
-			return ipr_chip[i].cfg;
+			return &ipr_chip[i];
 	return NULL;
 }

@@ -7539,14 +7539,16 @@
 	ata_host_init(&ioa_cfg->ata_host, &pdev->dev,
 		      sata_port_info.flags, &ipr_sata_ops);

-	ioa_cfg->chip_cfg = ipr_get_chip_cfg(dev_id);
+	ioa_cfg->ipr_chip = ipr_get_chip_info(dev_id);

-	if (!ioa_cfg->chip_cfg) {
+	if (!ioa_cfg->ipr_chip) {
 		dev_err(&pdev->dev, "Unknown adapter chipset 0x%04X 0x%04X\n",
 			dev_id->vendor, dev_id->device);
 		goto out_scsi_host_put;
 	}

+	ioa_cfg->chip_cfg = ioa_cfg->ipr_chip->cfg;
+
 	if (ipr_transop_timeout)
 		ioa_cfg->transop_timeout = ipr_transop_timeout;
 	else if (dev_id->driver_data & IPR_USE_LONG_TRANSOP_TIMEOUT)
@@ -7598,7 +7600,7 @@
 	}

 	/* Enable MSI style interrupts if they are supported. */
-	if (!(rc = pci_enable_msi(pdev))) {
+	if (ioa_cfg->ipr_chip->intr_type == IPR_USE_MSI && !pci_enable_msi(pdev)) {
 		rc = ipr_test_msi(ioa_cfg, pdev);
 		if (rc == -EOPNOTSUPP)
 			pci_disable_msi(pdev);
Index: b/drivers/scsi/ipr.h
===================================================================
--- a/drivers/scsi/ipr.h	2009-06-15 16:35:43.000000000 -0700
+++ b/drivers/scsi/ipr.h	2009-06-15 16:36:44.000000000 -0700
@@ -1025,6 +1025,9 @@
 struct ipr_chip_t {
 	u16 vendor;
 	u16 device;
+	u16 intr_type;
+#define IPR_USE_LSI			0x00
+#define IPR_USE_MSI			0x01
 	const struct ipr_chip_cfg_t *cfg;
 };

@@ -1160,6 +1163,7 @@

 	unsigned int transop_timeout;
 	const struct ipr_chip_cfg_t *chip_cfg;
+	const struct ipr_chip_t *ipr_chip;

 	void __iomem *hdw_dma_regs;	/* iomapped PCI memory space */
 	unsigned long hdw_dma_regs_pci;	/* raw PCI memory space */

Re: [PATCH 2/2] ipr: differentiate pci-x and pci-e based adapters

[Brian King]

Acked-by: Brian King <brking@linux.vnet.ibm.com>

Wayne Boyer wrote:
> Brian King wrote:
>> Wayne Boyer wrote:
>>> @@ -7539,7 +7539,8 @@
>>>  	ata_host_init(&ioa_cfg->ata_host, &pdev->dev,
>>>  		      sata_port_info.flags, &ipr_sata_ops);
>>>
>>> -	ioa_cfg->chip_cfg = ipr_get_chip_cfg(dev_id);
>>> +	ioa_cfg->ipr_chip = ipr_get_chip_info(dev_id);
>>> +	ioa_cfg->chip_cfg = ioa_cfg->ipr_chip->cfg;
>> If ipr_get_chip_info returns NULL you will oops here.
>>
> 
> Updated patch below.
> 


-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center