Skip to contentSkip to conversation

ahci/sata_vsc: drop intx manipulation on msi enable

PATCH In review Tejun Heo 2 participants 3 messages Jan 23, 2009 – Feb 17, 2009 libata

lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 ahci/sata_vsc: drop intx manipulation on msi enable

There's no need to turn off intx explicitly on msi enable. This is automatically handled by pci. Drop it.

Note that, in sata_vsc's case, it was completely redundant; however, for ahci, it might be needed on machines if the BIOS turns intx off during boot. However, there's no evidence of such behavior for ahci and the only such case seems to be ICH5 PATA according to ata_piix. Also, given the way ahci operates, it's highly unlikely BIOS ever disables IRQ for the controller. However, as this change has slight possibility of introducing failure, please schedule it for #upstream.

Signed-off-by Tejun Heo <[email protected]>

---
There are similar places in sata_sis and sata_uli. I have no idea whether they're there because they're needed or just copied over from other drivers. Eh...

Thanks.

 drivers/ata/ahci.c     |    4 ++--
 drivers/ata/sata_vsc.c |    3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)
drivers/ata/ahci.c +2 -2
index 7f701cb..18fac60 100644
@@ -2616,8 +2616,8 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (board_id == board_ahci_sb700 && pdev->revision >= 0x40)
 		hpriv->flags &= ~AHCI_HFLAG_IGN_SERR_INTERNAL;
 
-	if ((hpriv->flags & AHCI_HFLAG_NO_MSI) || pci_enable_msi(pdev))
-		pci_intx(pdev, 1);
+	if (!(hpriv->flags & AHCI_HFLAG_NO_MSI))
+		pci_enable_msi(pdev);
 
 	/* save initial config */
 	ahci_save_initial_config(pdev, hpriv);
drivers/ata/sata_vsc.c +1 -2
index c57cdff..398328a 100644
@@ -411,8 +411,7 @@ static int __devinit vsc_sata_init_one(struct pci_dev *pdev,
 	if (cls == 0x00)
 		pci_write_config_byte(pdev, PCI_CACHE_LINE_SIZE, 0x80);
 
-	if (pci_enable_msi(pdev) == 0)
-		pci_intx(pdev, 0);
+	pci_enable_msi(pdev);
 
 	/*
 	 * Config offset 0x98 is "Extended Control and Status Register 0"
author
#1 lore raw
Quoted 11 lines · Tejun Heo

There's no need to turn off intx explicitly on msi enable. This is automatically handled by pci. Drop it.

Note that, in sata_vsc's case, it was completely redundant; however, for ahci, it might be needed on machines if the BIOS turns intx off during boot. However, there's no evidence of such behavior for ahci and the only such case seems to be ICH5 PATA according to ata_piix. Also, given the way ahci operates, it's highly unlikely BIOS ever disables IRQ for the controller. However, as this change has slight possibility of introducing failure, please schedule it for #upstream.

Quoted 1 line
Signed-off-by Tejun Heo <[email protected]>

Ping.

Signature · 1 line
tejun
#2 lore raw
In reply to Tejun Heo
Quoted 11 lines · Tejun Heo

There's no need to turn off intx explicitly on msi enable. This is automatically handled by pci. Drop it.

Note that, in sata_vsc's case, it was completely redundant; however, for ahci, it might be needed on machines if the BIOS turns intx off during boot. However, there's no evidence of such behavior for ahci and the only such case seems to be ICH5 PATA according to ata_piix. Also, given the way ahci operates, it's highly unlikely BIOS ever disables IRQ for the controller. However, as this change has slight possibility of introducing failure, please schedule it for #upstream.

Quoted 5 lines
Signed-off-by Tejun Heo <[email protected]>

---
There are similar places in sata_sis and sata_uli. I have no idea whether they're there because they're needed or just copied over from other drivers. Eh...

Most of them were just copied, IIRC.

as for this patch: applied the ahci portion.

For sata_vsc, ISTR there being a hardware-specific reason why the MSI+INTX manipulation was done in that fashion. Needs a bit of research, or at least, real world testing results for the MSI and !MSI cases...

Jeff