* [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers
@ 2008-05-20 19:27 Prakash, Sathya
2008-07-11 3:10 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: Prakash, Sathya @ 2008-05-20 19:27 UTC (permalink / raw)
To: linux-scsi; +Cc: eric.moore
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 <sathya.prakash@lsi.com>
---
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 9538df2..ff9965d 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers
2008-05-20 19:27 [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers Prakash, Sathya
@ 2008-07-11 3:10 ` James Bottomley
2008-07-11 4:26 ` Prakash, Sathya
2008-07-12 17:10 ` Prakash, Sathya
0 siblings, 2 replies; 5+ messages in thread
From: James Bottomley @ 2008-07-11 3:10 UTC (permalink / raw)
To: Prakash, Sathya; +Cc: linux-scsi, eric.moore
On Wed, 2008-05-21 at 00:57 +0530, Prakash, Sathya wrote:
> 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
---
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index db3c892..d40d6d1 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -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)
^ permalink raw reply related [flat|nested] 5+ messages in thread* RE: [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers
2008-07-11 3:10 ` James Bottomley
@ 2008-07-11 4:26 ` Prakash, Sathya
2008-07-11 14:05 ` James Bottomley
2008-07-12 17:10 ` Prakash, Sathya
1 sibling, 1 reply; 5+ messages in thread
From: Prakash, Sathya @ 2008-07-11 4:26 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-scsi@vger.kernel.org, Moore, Eric
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:James.Bottomley@HansenPartnership.com]
Sent: Friday, July 11, 2008 8:41 AM
To: Prakash, Sathya
Cc: linux-scsi@vger.kernel.org; Moore, Eric
Subject: Re: [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers
On Wed, 2008-05-21 at 00:57 +0530, Prakash, Sathya wrote:
> 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
---
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index db3c892..d40d6d1 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -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)
^ permalink raw reply related [flat|nested] 5+ messages in thread* RE: [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers
2008-07-11 4:26 ` Prakash, Sathya
@ 2008-07-11 14:05 ` James Bottomley
0 siblings, 0 replies; 5+ messages in thread
From: James Bottomley @ 2008-07-11 14:05 UTC (permalink / raw)
To: Prakash, Sathya; +Cc: linux-scsi@vger.kernel.org, Moore, Eric
On Fri, 2008-07-11 at 12:26 +0800, Prakash, Sathya wrote:
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers
2008-07-11 3:10 ` James Bottomley
2008-07-11 4:26 ` Prakash, Sathya
@ 2008-07-12 17:10 ` Prakash, Sathya
1 sibling, 0 replies; 5+ messages in thread
From: Prakash, Sathya @ 2008-07-12 17:10 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-scsi@vger.kernel.org, Moore, Eric
ACK
-----Original Message-----
From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
Sent: Friday, July 11, 2008 8:41 AM
To: Prakash, Sathya
Cc: linux-scsi@vger.kernel.org; Moore, Eric
Subject: Re: [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers
On Wed, 2008-05-21 at 00:57 +0530, Prakash, Sathya wrote:
> 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
---
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index db3c892..d40d6d1 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -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)
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-07-12 17:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-20 19:27 [PATCH 3/6] : Fixing default enabling of MSI for SPI and FC controllers Prakash, Sathya
2008-07-11 3:10 ` James Bottomley
2008-07-11 4:26 ` Prakash, Sathya
2008-07-11 14:05 ` James Bottomley
2008-07-12 17:10 ` Prakash, Sathya
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.