On 11/13/2013 02:00 PM, Jackey Shen wrote:
> On Wed, Nov 13, 2013 at 01:55:08PM +0800, Jackey Shen wrote:
>>>>> What about we only call pci_enable_msi in sdhci-pci.c and then assign
>>>>> host->irq appropriately before calling sdhci_add_host, will that work?
>>>>> The only difference would be, request_irq will have SHARED flag, but I
>>>>> suppose that's not a problem.
>>>>>
>>>> There are 2 points for this part:
>>>> 1. fall back to legacy interrupt right after MSI request fails;
>>>
>>> If we call pci_enable_msi somewhere in sdhci_pci_probe_slot, then if it
>>> failed, we can also fall back I suppose?
>>>
>> Yes, it is.
>> But, sdhci_pci_disable_msi should be kept since pci_disable_msi is
>> pci bus releted and better not used in sdhci.c.
>> So, enable msi and disable msi are NOT symmetric in style.
>> What's your opinion?
>
> Sorry for missing word.
> So, enable msi and disable msi are NOT symmetric in style.
>
I meant something like this:
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index d7d6bc8..96461a3 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -1339,12 +1339,17 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
host->quirks = chip->quirks;
host->quirks2 = chip->quirks2;
+ ret = pci_enable_msi(pdev);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot enable msi\n");
+ goto cleanup;
+ }
host->irq = pdev->irq;
ret = pci_request_region(pdev, bar, mmc_hostname(host->mmc));
if (ret) {
dev_err(&pdev->dev, "cannot request region\n");
- goto cleanup;
+ goto disable_msi;
}
host->ioaddr = pci_ioremap_bar(pdev, bar);
@@ -1396,6 +1401,9 @@ unmap:
release:
pci_release_region(pdev, bar);
+disable_msi:
+ pci_disable_msi(pdev);
+
cleanup:
if (slot->data && slot->data->cleanup)
slot->data->cleanup(slot->data);
@@ -1431,6 +1439,8 @@ static void sdhci_pci_remove_slot(struct sdhci_pci_slot *slot)
pci_release_region(slot->chip->pdev, slot->pci_bar);
+ pci_disable_msi(slot->chip->pdev);
+
sdhci_free_host(slot->host);
}
We can do all these MSI stuffs in sdhci-pci.c, but I'm not sure if this
is correct.
Thanks,
Aaron
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
