ACHI: Attempt to enable a single MSI if block enable fails

PATCH In review Michael Marley 4 participants 4 messages Feb 3, 2014 – Feb 4, 2014 libata

lore

Share & tools

lore

Michael Marley

author

2014-02-03 20:39 UTC #0 lore raw

PATCH 1/1 ACHI: Attempt to enable a single MSI if block enable fails

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

From: Michael Marley <[email protected]>

Commit 7b92b4f61ec49cb1a5813298f35258bd7ecd3667 ("PCI/MSI: Remove pci_enable_msi_block_auto()") causes the AHCI driver to immediately switch to INTx mode if the pci_enable_msi_block() call fails to allocate the requested number of MSIs. This behavior causes some systems (for example, the Lenovo ThinkCentre m57p) to begin using INTx when they worked fine in single MSI mode before. This patch makes the driver try to enable a single MSI if the pci_enable_msi_block() call fails instead of switching directly to INTx.

Signed-off-by Michael Marley <[email protected]>

- ---

- --- drivers/ata/ahci.c.orig 2014-02-03 20:00:04.834008843 +0000
+++ drivers/ata/ahci.c 2014-02-03 19:34:53.737257483 +0000
@@ -1171,7 +1171,7 @@ static int ahci_init_interrupts(struct p
nvec = rc;
rc = pci_enable_msi_block(pdev, nvec);
if (rc)
- - goto intx;
+ goto single_msi;

return nvec;

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJS7/5/AAoJEANTUn3blhEaLo8P/31AEbD9b61h2zFAcGlqZmN/ OEp69smHl9DbciaL0nQGIY4TRUpjA2XVxT/ZC3Fdz0huqX226g2D2G7/LLk299r3 m/GC89640FBRaVqNTASNKvIANlq0t2vgdOXGsgLXCD3EAigMdCVLiKNvdDYpUdXo uaUg7WTVM12U+bWCrTB0RVgU7kKqkPSWb59kUPFaInjrTWzZ4jf4SZEzOr1b6mB0 Ihn/BsMf76q9rzGEwTaKPV+BrS/QZY5oz23r8+Pqw+lNVgtnmCQ7UkRJ40MI3roS lU68syoG/CjI6imO/TQcr54eHMTBRl55Ut4VO/iLYaJKhXGGsuP43oPrJM0LZzpi jjwzgz4Bij0Hi5Wx7AS0sDNEERi+w4At/+eKQmUkjA6qUblzIKCpMMCmp0CvodXH 8J9TU9CqcnQs2hgGy990nFxdmiLh+Ee5NFBfKxpTGxv4eAjuK6RbxMLfndF6rRXO MpMoFMK3BdFnQoi6We5er7HKPcbqOpR2hrw8mALhWDxgv+N2pq1J5uG69mc5vwxJ YV7N0tnCtVTyxGavtMP669YCNH0KvRzXJvzJRE1TuuTvNu+oudMWJinilyhPnYJk U5DYzLWj4t42qEsGr3QjFBEv90DG/LNc5YavI9XOfepjGFbdlcGQPAe3HnaE86KD s9JiFcvAixL295414PRN
=f6LE
-----END PGP SIGNATURE-----

Tejun Heo

2014-02-03 20:46 UTC #1 lore raw

cc'ing Alexander. Alexander, can you please reply with Reviewed-by?

Thanks.

Quoted 48 lines · Michael Marley

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
From: Michael Marley <[email protected]>
Commit 7b92b4f61ec49cb1a5813298f35258bd7ecd3667 ("PCI/MSI: Remove pci_enable_msi_block_auto()") causes the AHCI driver to immediately switch to INTx mode if the pci_enable_msi_block() call fails to allocate the requested number of MSIs. This behavior causes some systems (for example, the Lenovo ThinkCentre m57p) to begin using INTx when they worked fine in single MSI mode before. This patch makes the driver try to enable a single MSI if the pci_enable_msi_block() call fails instead of switching directly to INTx.
Signed-off-by Michael Marley <[email protected]>
- ---
- --- drivers/ata/ahci.c.orig 2014-02-03 20:00:04.834008843 +0000
+++ drivers/ata/ahci.c 2014-02-03 19:34:53.737257483 +0000
(quoted diff) +1 -2
@@ -1171,7 +1171,7 @@ static int ahci_init_interrupts(struct p
 	nvec = rc;
 	rc = pci_enable_msi_block(pdev, nvec);
 	if (rc)
- -		goto intx;
+		goto single_msi;
 
 	return nvec;
 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJS7/5/AAoJEANTUn3blhEaLo8P/31AEbD9b61h2zFAcGlqZmN/ OEp69smHl9DbciaL0nQGIY4TRUpjA2XVxT/ZC3Fdz0huqX226g2D2G7/LLk299r3 m/GC89640FBRaVqNTASNKvIANlq0t2vgdOXGsgLXCD3EAigMdCVLiKNvdDYpUdXo uaUg7WTVM12U+bWCrTB0RVgU7kKqkPSWb59kUPFaInjrTWzZ4jf4SZEzOr1b6mB0 Ihn/BsMf76q9rzGEwTaKPV+BrS/QZY5oz23r8+Pqw+lNVgtnmCQ7UkRJ40MI3roS lU68syoG/CjI6imO/TQcr54eHMTBRl55Ut4VO/iLYaJKhXGGsuP43oPrJM0LZzpi jjwzgz4Bij0Hi5Wx7AS0sDNEERi+w4At/+eKQmUkjA6qUblzIKCpMMCmp0CvodXH 8J9TU9CqcnQs2hgGy990nFxdmiLh+Ee5NFBfKxpTGxv4eAjuK6RbxMLfndF6rRXO MpMoFMK3BdFnQoi6We5er7HKPcbqOpR2hrw8mALhWDxgv+N2pq1J5uG69mc5vwxJ YV7N0tnCtVTyxGavtMP669YCNH0KvRzXJvzJRE1TuuTvNu+oudMWJinilyhPnYJk U5DYzLWj4t42qEsGr3QjFBEv90DG/LNc5YavI9XOfepjGFbdlcGQPAe3HnaE86KD s9JiFcvAixL295414PRN
=f6LE
-----END PGP SIGNATURE-----

Signature · 1 line

tejun

Alexander Gordeev

2014-02-04 05:59 UTC #2 lore raw

Quoted 1 line · Tejun Heo

cc'ing Alexander. Alexander, can you please reply with Reviewed-by?

[...]

Quoted 11 lines

Review static int ahci_init_interrupts(struct p · line 1176
Quoted 11 lines
- --- drivers/ata/ahci.c.orig 2014-02-03 20:00:04.834008843 +0000
+++ drivers/ata/ahci.c 2014-02-03 19:34:53.737257483 +0000
(quoted diff) +1 -1
@@ -1171,7 +1171,7 @@ static int ahci_init_interrupts(struct p
 	nvec = rc;
 	rc = pci_enable_msi_block(pdev, nvec);
 	if (rc)
- -		goto intx;
+		goto single_msi;
 
 	return nvec;

Single MSI should not be tried in case pci_enable_msi_block() returned a negative errno. The correct fix is below. Bjorn already has pulled it to his pci.git tree, into pci/msi branch.

@Bjorn,

Mind pushing the branch to the remote?

Thanks!

drivers/ata/ahci.c +3 -1

index 8516f4d..df24eef4 100644
@@ -1117,8 +1117,10 @@ int ahci_init_interrupts(struct pci_dev *pdev, unsigned int n_ports,
 
 	nvec = rc;
 	rc = pci_enable_msi_block(pdev, nvec);
-	if (rc)
+	if (rc < 0)
 		goto intx;
+	else if (rc > 0)
+		goto single_msi;
 
 	return nvec;

Signature · 6 lines

1.7.7.6

-- 
Regards,
Alexander Gordeev
[email protected]

Bjorn Helgaas

2014-02-04 16:41 UTC #3 lore raw

Quoted 21 lines · Alexander Gordeev

Quoted 1 line · Tejun Heo
cc'ing Alexander. Alexander, can you please reply with Reviewed-by?
[...]
Quoted 11 lines
drivers/ata/ahci.c static int ahci_init_interrupts(struct p · line 1176
Quoted 11 lines
- --- drivers/ata/ahci.c.orig 2014-02-03 20:00:04.834008843 +0000
+++ drivers/ata/ahci.c 2014-02-03 19:34:53.737257483 +0000
drivers/ata/ahci.c +1 -1
@@ -1171,7 +1171,7 @@ static int ahci_init_interrupts(struct p
    nvec = rc;
    rc = pci_enable_msi_block(pdev, nvec);
    if (rc)
- -         goto intx;
+           goto single_msi;
 
    return nvec;
Single MSI should not be tried in case pci_enable_msi_block() returned a negative errno. The correct fix is below. Bjorn already has pulled it to his pci.git tree, into pci/msi branch.

Quoted 3 lines

@Bjorn,
Mind pushing the branch to the remote?

Done; it's now here:
http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/msi

I'll probably wait to merge it into -next until I pull in some more of your MSI patches.

Quoted 23 lines

drivers/ata/ahci.c +3 -2

index 8516f4d..df24eef4 100644
@@ -1117,8 +1117,10 @@ int ahci_init_interrupts(struct pci_dev *pdev, unsigned int n_ports,
 
        nvec = rc;
        rc = pci_enable_msi_block(pdev, nvec);
-       if (rc)
+       if (rc < 0)
                goto intx;
+       else if (rc > 0)
+               goto single_msi;
 
        return nvec;
 
--

1.7.7.6

--
Regards,
Alexander Gordeev
[email protected]