: Fixing default enabling of MSI for SPI and FC controllers

PATCH In review Prakash, Sathya 2 participants 5 messages May 20, 2008 – Jul 12, 2008

Subsystems (2)

SCSI LSI Logic MPT fusion drivers (FC/SAS/SPI)

lore

Share & tools

lore

Prakash, Sathya

author

2008-05-20 19:36 UTC #0 lore raw

PATCH 3/6 : Fixing default enabling of MSI for SPI and FC controllers

The patch submitted to enable the MSI by default for SAS controllers sets the MSI even for SPI and FC controllers due to a coding error This patch fixes that.

Signed-off-by Sathya Prakash <[email protected]>

---

drivers/message/fusion/mptbase.c +5 -4

index 9538df2..ff9965d 100644
@@ -2062,7 +2062,8 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u32 reason, int sleepFlag)
 	if ((ret == 0) && (reason == MPT_HOSTEVENT_IOC_BRINGUP)) {
 		ioc->pci_irq = -1;
 		if (ioc->pcidev->irq) {
-			if (ioc->msi_enable && !pci_enable_msi(ioc->pcidev))
+			if (ioc->msi_enable == 1 &&
+					!pci_enable_msi(ioc->pcidev))
 				printk(MYIOC_s_INFO_FMT "PCI-MSI enabled\n",
 				    ioc->name);
 			else

@@ -2072,7 +2073,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u32 reason, int sleepFlag)
 			if (rc < 0) {
 				printk(MYIOC_s_ERR_FMT "Unable to allocate "
 				    "interrupt %d!\n", ioc->name, ioc->pcidev->irq);
-				if (ioc->msi_enable)
+				if (ioc->msi_enable == 1)
 					pci_disable_msi(ioc->pcidev);
 				return -EBUSY;
 			}

@@ -2268,7 +2269,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u32 reason, int sleepFlag)
  out:
 	if ((ret != 0) && irq_allocated) {
 		free_irq(ioc->pci_irq, ioc);
-		if (ioc->msi_enable)
+		if (ioc->msi_enable == 1)
 			pci_disable_msi(ioc->pcidev);
 	}
 	return ret;

@@ -2450,7 +2451,7 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
 
 	if (ioc->pci_irq != -1) {
 		free_irq(ioc->pci_irq, ioc);
-		if (ioc->msi_enable)
+		if (ioc->msi_enable == 1)
 			pci_disable_msi(ioc->pcidev);
 		ioc->pci_irq = -1;
 	}

James Bottomley

maintainer

2008-07-11 03:10 UTC #1 lore raw

Quoted 3 lines · Sathya

The patch submitted to enable the MSI by default for SAS controllers sets the MSI even for SPI and FC controllers due to a coding error This patch fixes that.

Actually, now I look at this patch, it's not complete. There's a missed

	if (ioc->msi_enable)
		pci_disable_msi(ioc->pcidev);

in the suspend path which will trigger if ioc->msi_enable is -1.

Rather than trying to capture all the uses of ioc->msi_enable (and missing some of them) to check for 1 instead of true, isn't it just easier to fix the driver so that ioc->msi_enable is a logical truth value like this?

James

---

drivers/message/fusion/mptbase.c +8 -3

index db3c892..d40d6d1 100644
@@ -1686,9 +1686,14 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 		ioc->bus_type = SAS;
 	}
 
-	if (ioc->bus_type == SAS && mpt_msi_enable == -1)
-		ioc->msi_enable = 1;
-	else
+	if (mpt_msi_enable == -1) {
+		/* Enable on SAS, disable on FC and SPI */
+		if (ioc->bus_type == SAS)
+			ioc->msi_enable = 1;
+		else
+			ioc->msi_enable = 0;
+	} else
+		/* follow flag: 0 - disable; 1 - enable */
 		ioc->msi_enable = mpt_msi_enable;
 
 	if (ioc->errata_flag_1064)

Prakash, Sathya

author

2008-07-11 04:26 UTC #2 lore raw

James,
This looks better than mine, Shall I post the new patch with your suggestion or did you include it already? Thanks
Sathya

-----Original Message-----
From: James Bottomley [mailto:[email protected]]
Sent: Friday, July 11, 2008 8:41 AM
To: Prakash, Sathya
Cc: [email protected]; Moore, Eric
Subject: Re: [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers

Quoted 3 lines · Sathya

The patch submitted to enable the MSI by default for SAS controllers sets the MSI even for SPI and FC controllers due to a coding error This patch fixes that.

Actually, now I look at this patch, it's not complete. There's a missed

        if (ioc->msi_enable)
                pci_disable_msi(ioc->pcidev);

in the suspend path which will trigger if ioc->msi_enable is -1.

James

---

drivers/message/fusion/mptbase.c +8 -3

index db3c892..d40d6d1 100644
@@ -1686,9 +1686,14 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
                ioc->bus_type = SAS;
        }
 
-       if (ioc->bus_type == SAS && mpt_msi_enable == -1)
-               ioc->msi_enable = 1;
-       else
+       if (mpt_msi_enable == -1) {
+               /* Enable on SAS, disable on FC and SPI */
+               if (ioc->bus_type == SAS)
+                       ioc->msi_enable = 1;
+               else
+                       ioc->msi_enable = 0;
+       } else
+               /* follow flag: 0 - disable; 1 - enable */
                ioc->msi_enable = mpt_msi_enable;
 
        if (ioc->errata_flag_1064)

James Bottomley

maintainer

2008-07-11 14:05 UTC #3 lore raw

Quoted 1 line · Sathya

This looks better than mine, Shall I post the new patch with your suggestion or did you include it already?

If you're happy with it, just ack it and I'll add it (and sort out the rest of the problems in the tree later).

James

Prakash, Sathya

author

2008-07-12 17:10 UTC #4 lore raw

In reply to James Bottomley

ACK

Quoted 3 lines · Sathya

The patch submitted to enable the MSI by default for SAS controllers sets the MSI even for SPI and FC controllers due to a coding error This patch fixes that.

Actually, now I look at this patch, it's not complete. There's a missed

        if (ioc->msi_enable)
                pci_disable_msi(ioc->pcidev);

in the suspend path which will trigger if ioc->msi_enable is -1.

James

---

drivers/message/fusion/mptbase.c +8 -3

index db3c892..d40d6d1 100644
@@ -1686,9 +1686,14 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
                ioc->bus_type = SAS;
        }
 
-       if (ioc->bus_type == SAS && mpt_msi_enable == -1)
-               ioc->msi_enable = 1;
-       else
+       if (mpt_msi_enable == -1) {
+               /* Enable on SAS, disable on FC and SPI */
+               if (ioc->bus_type == SAS)
+                       ioc->msi_enable = 1;
+               else
+                       ioc->msi_enable = 0;
+       } else
+               /* follow flag: 0 - disable; 1 - enable */
                ioc->msi_enable = mpt_msi_enable;
 
        if (ioc->errata_flag_1064)