[PATCH] mmc: sdhci-pci: mention DMA mode only once

[PATCH] mmc: sdhci-pci: mention DMA mode only once

[Paul Bolle]

A laptop I use prints a warning twice at every boot and every resume:
    sdhci-pci 0000:05:00.2: Will use DMA mode even though HW doesn't fully claim to support it.
    sdhci-pci 0000:05:00.2: Will use DMA mode even though HW doesn't fully claim to support it.

These warnings are always printed in pairs.

This message shouldn't be a warning, but a notice, as there's little the
user is able to do about the choice for DMA mode. The only way to
overrule it seems to be the use of (undocumented) debug quirks for the
sdhci module.

And, furthermore, this notice needs only be printed once. Everything here
depends on the hardware of the SD host controller used, which can't
change.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) Tested on v3.10.3. Compile tested for v3.11-rc2.

1) Note that this warning is printed twice at boot (or module load)
because the call chain is:
    sdhci_add_host()
        host->ops->enable_dma() = sdhci_pci_enable_dma()
        sdhci_init()
            sdhci_reset()
                host->ops->enable_dma() = sdhci_pci_enable_dma()

(And, if I understand the code correctly, for non-quirky host
controllers it can even print the warning three times!)

The call chain at resume is:
    sdhci_resume_host()
        host->ops->enable_dma() = sdhci_pci_enable_dma()
        sdhci_init()
            sdhci_reset()
                host->ops->enable_dma() = sdhci_pci_enable_dma()

2) Note that the patch would be simpler if it's OK to print this message
once per _module_ and not, as this patch does, once per _host
controller_. Are there systems that use more than one SD host
controller, both with different (advertised) DMA capabilities? 

 drivers/mmc/host/sdhci-pci.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index d7d6bc8..69fc509 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -88,6 +88,7 @@ struct sdhci_pci_chip {
 	unsigned int		quirks;
 	unsigned int		quirks2;
 	bool			allow_runtime_pm;
+	bool			use_sdma_notified;
 	const struct sdhci_pci_fixes *fixes;
 
 	int			num_slots;	/* Slots on controller */
@@ -997,17 +998,21 @@ MODULE_DEVICE_TABLE(pci, pci_ids);
 static int sdhci_pci_enable_dma(struct sdhci_host *host)
 {
 	struct sdhci_pci_slot *slot;
+	struct sdhci_pci_chip *chip;
 	struct pci_dev *pdev;
 	int ret;
 
 	slot = sdhci_priv(host);
-	pdev = slot->chip->pdev;
+	chip = slot->chip;
+	pdev = chip->pdev;
 
 	if (((pdev->class & 0xFFFF00) == (PCI_CLASS_SYSTEM_SDHCI << 8)) &&
 		((pdev->class & 0x0000FF) != PCI_SDHCI_IFDMA) &&
-		(host->flags & SDHCI_USE_SDMA)) {
-		dev_warn(&pdev->dev, "Will use DMA mode even though HW "
-			"doesn't fully claim to support it.\n");
+		(host->flags & SDHCI_USE_SDMA) &&
+		(chip->use_sdma_notified == false)) {
+		dev_notice(&pdev->dev, "Will use DMA mode even though HW "
+			   "doesn't fully claim to support it.\n");
+		chip->use_sdma_notified = true;
 	}
 
 	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
@@ -1504,6 +1509,7 @@ static int sdhci_pci_probe(struct pci_dev *pdev,
 		chip->allow_runtime_pm = chip->fixes->allow_runtime_pm;
 	}
 	chip->num_slots = slots;
+	chip->use_sdma_notified = false;
 
 	pci_set_drvdata(pdev, chip);
 
-- 
1.8.1.4

Re: [PATCH] mmc: sdhci-pci: mention DMA mode only once

[Paul Bolle]

On Fri, 2013-07-26 at 23:52 +0200, Paul Bolle wrote:
> A laptop I use prints a warning twice at every boot and every resume:
>     sdhci-pci 0000:05:00.2: Will use DMA mode even though HW doesn't fully claim to support it.
>     sdhci-pci 0000:05:00.2: Will use DMA mode even though HW doesn't fully claim to support it.
> 
> These warnings are always printed in pairs.
> 
> This message shouldn't be a warning, but a notice, as there's little the
> user is able to do about the choice for DMA mode. The only way to
> overrule it seems to be the use of (undocumented) debug quirks for the
> sdhci module.
> 
> And, furthermore, this notice needs only be printed once. Everything here
> depends on the hardware of the SD host controller used, which can't
> change.
> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> 0) Tested on v3.10.3. Compile tested for v3.11-rc2.

Did anyone have a look at this patch? It compiles cleanly on top of
v3.13-rc4. It also does what it claims to do when running v3.13-rc4.

> 1) Note that this warning is printed twice at boot (or module load)
> because the call chain is:
>     sdhci_add_host()
>         host->ops->enable_dma() = sdhci_pci_enable_dma()
>         sdhci_init()
>             sdhci_reset()
>                 host->ops->enable_dma() = sdhci_pci_enable_dma()
> 
> (And, if I understand the code correctly, for non-quirky host
> controllers it can even print the warning three times!)
> 
> The call chain at resume is:
>     sdhci_resume_host()
>         host->ops->enable_dma() = sdhci_pci_enable_dma()
>         sdhci_init()
>             sdhci_reset()
>                 host->ops->enable_dma() = sdhci_pci_enable_dma()
> 
> 2) Note that the patch would be simpler if it's OK to print this message
> once per _module_ and not, as this patch does, once per _host
> controller_. Are there systems that use more than one SD host
> controller, both with different (advertised) DMA capabilities? 
> 
>  drivers/mmc/host/sdhci-pci.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
> index d7d6bc8..69fc509 100644
> --- a/drivers/mmc/host/sdhci-pci.c
> +++ b/drivers/mmc/host/sdhci-pci.c
> @@ -88,6 +88,7 @@ struct sdhci_pci_chip {
>  	unsigned int		quirks;
>  	unsigned int		quirks2;
>  	bool			allow_runtime_pm;
> +	bool			use_sdma_notified;
>  	const struct sdhci_pci_fixes *fixes;
>  
>  	int			num_slots;	/* Slots on controller */
> @@ -997,17 +998,21 @@ MODULE_DEVICE_TABLE(pci, pci_ids);
>  static int sdhci_pci_enable_dma(struct sdhci_host *host)
>  {
>  	struct sdhci_pci_slot *slot;
> +	struct sdhci_pci_chip *chip;
>  	struct pci_dev *pdev;
>  	int ret;
>  
>  	slot = sdhci_priv(host);
> -	pdev = slot->chip->pdev;
> +	chip = slot->chip;
> +	pdev = chip->pdev;
>  
>  	if (((pdev->class & 0xFFFF00) == (PCI_CLASS_SYSTEM_SDHCI << 8)) &&
>  		((pdev->class & 0x0000FF) != PCI_SDHCI_IFDMA) &&
> -		(host->flags & SDHCI_USE_SDMA)) {
> -		dev_warn(&pdev->dev, "Will use DMA mode even though HW "
> -			"doesn't fully claim to support it.\n");
> +		(host->flags & SDHCI_USE_SDMA) &&
> +		(chip->use_sdma_notified == false)) {
> +		dev_notice(&pdev->dev, "Will use DMA mode even though HW "
> +			   "doesn't fully claim to support it.\n");
> +		chip->use_sdma_notified = true;
>  	}
>  
>  	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> @@ -1504,6 +1509,7 @@ static int sdhci_pci_probe(struct pci_dev *pdev,
>  		chip->allow_runtime_pm = chip->fixes->allow_runtime_pm;
>  	}
>  	chip->num_slots = slots;
> +	chip->use_sdma_notified = false;
>  
>  	pci_set_drvdata(pdev, chip);
>  

Paul Bolle