Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1351257 > unrolled thread

[PATCH v4 1/3] mmc: sdhci: Set DMA mask when adding host

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 1/3] mmc: sdhci: Set DMA mask when adding host Alexandre Courbot <acourbot@nvidia.com> - 2016-03-07 03:10 +0100
    Re: [PATCH v4 1/3] mmc: sdhci: Set DMA mask when adding host Adrian Hunter <adrian.hunter@intel.com> - 2016-03-16 09:50 +0100
      Re: [PATCH v4 1/3] mmc: sdhci: Set DMA mask when adding host Arnd Bergmann <arnd@arndb.de> - 2016-03-16 10:10 +0100
        Re: [PATCH v4 1/3] mmc: sdhci: Set DMA mask when adding host Adrian Hunter <adrian.hunter@intel.com> - 2016-03-16 10:40 +0100
          Re: [PATCH v4 1/3] mmc: sdhci: Set DMA mask when adding host Arnd Bergmann <arnd@arndb.de> - 2016-03-16 11:10 +0100

#1351257 — [PATCH v4 1/3] mmc: sdhci: Set DMA mask when adding host

Set the DMA mask in sdhci_add_host() after we determined the
capabilities of the device. 64-bit devices in particular are given the
proper mask that ensures bounce buffers are not used.

Also disable DMA if no proper DMA mask can be set, as the DMA-API
documentation specifies.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/mmc/host/sdhci.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fd9139947fa3..920e1c996280 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2857,6 +2857,36 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
 
 EXPORT_SYMBOL_GPL(sdhci_alloc_host);
 
+static int sdhci_set_dma_mask(struct sdhci_host *host)
+{
+	struct mmc_host *mmc = host->mmc;
+	struct device *dev = mmc_dev(mmc);
+	int ret = -EINVAL;
+
+	if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA)
+		host->flags &= ~SDHCI_USE_64_BIT_DMA;
+
+	/* Try 64-bit mask if hardware is capable  of it */
+	if (host->flags & SDHCI_USE_64_BIT_DMA) {
+		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+		if (ret) {
+			pr_warn("%s: Failed to set 64-bit DMA mask.\n",
+				mmc_hostname(mmc));
+			host->flags &= ~SDHCI_USE_64_BIT_DMA;
+		}
+	}
+
+	/* 32-bit mask as default & fallback */
+	if (ret) {
+		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret)
+			pr_warn("%s: Failed to set 32-bit DMA mask.\n",
+				mmc_hostname(mmc));
+	}
+
+	return ret;
+}
+
 int sdhci_add_host(struct sdhci_host *host)
 {
 	struct mmc_host *mmc;
@@ -2932,13 +2962,17 @@ int sdhci_add_host(struct sdhci_host *host)
 		host->flags |= SDHCI_USE_64_BIT_DMA;
 
 	if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) {
-		if (host->ops->enable_dma) {
-			if (host->ops->enable_dma(host)) {
-				pr_warn("%s: No suitable DMA available - falling back to PIO\n",
-					mmc_hostname(mmc));
-				host->flags &=
-					~(SDHCI_USE_SDMA | SDHCI_USE_ADMA);
-			}
+		ret = sdhci_set_dma_mask(host);
+
+		if (!ret && host->ops->enable_dma)
+			ret = host->ops->enable_dma(host);
+
+		if (ret) {
+			pr_warn("%s: No suitable DMA available - falling back to PIO\n",
+				mmc_hostname(mmc));
+			host->flags &= ~(SDHCI_USE_SDMA | SDHCI_USE_ADMA);
+
+			ret = 0;
 		}
 	}
 
-- 
2.7.2

[toc] | [next] | [standalone]

#1358726

On 07/03/16 04:07, Alexandre Courbot wrote:
> Set the DMA mask in sdhci_add_host() after we determined the
> capabilities of the device. 64-bit devices in particular are given the
> proper mask that ensures bounce buffers are not used.
> 
> Also disable DMA if no proper DMA mask can be set, as the DMA-API
> documentation specifies.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/mmc/host/sdhci.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index fd9139947fa3..920e1c996280 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2857,6 +2857,36 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
>  
>  EXPORT_SYMBOL_GPL(sdhci_alloc_host);
>  
> +static int sdhci_set_dma_mask(struct sdhci_host *host)
> +{
> +	struct mmc_host *mmc = host->mmc;
> +	struct device *dev = mmc_dev(mmc);
> +	int ret = -EINVAL;
> +
> +	if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA)
> +		host->flags &= ~SDHCI_USE_64_BIT_DMA;
> +
> +	/* Try 64-bit mask if hardware is capable  of it */
> +	if (host->flags & SDHCI_USE_64_BIT_DMA) {
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +		if (ret) {
> +			pr_warn("%s: Failed to set 64-bit DMA mask.\n",
> +				mmc_hostname(mmc));
> +			host->flags &= ~SDHCI_USE_64_BIT_DMA;
> +		}
> +	}
> +
> +	/* 32-bit mask as default & fallback */
> +	if (ret) {
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));

What happens if device enumeration (e.g. of_dma_configure) has already set a
more restrictive DMA mask?

> +		if (ret)
> +			pr_warn("%s: Failed to set 32-bit DMA mask.\n",
> +				mmc_hostname(mmc));
> +	}
> +
> +	return ret;
> +}
> +
>  int sdhci_add_host(struct sdhci_host *host)
>  {
>  	struct mmc_host *mmc;
> @@ -2932,13 +2962,17 @@ int sdhci_add_host(struct sdhci_host *host)
>  		host->flags |= SDHCI_USE_64_BIT_DMA;
>  
>  	if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) {
> -		if (host->ops->enable_dma) {
> -			if (host->ops->enable_dma(host)) {
> -				pr_warn("%s: No suitable DMA available - falling back to PIO\n",
> -					mmc_hostname(mmc));
> -				host->flags &=
> -					~(SDHCI_USE_SDMA | SDHCI_USE_ADMA);
> -			}
> +		ret = sdhci_set_dma_mask(host);
> +
> +		if (!ret && host->ops->enable_dma)
> +			ret = host->ops->enable_dma(host);
> +
> +		if (ret) {
> +			pr_warn("%s: No suitable DMA available - falling back to PIO\n",
> +				mmc_hostname(mmc));
> +			host->flags &= ~(SDHCI_USE_SDMA | SDHCI_USE_ADMA);
> +
> +			ret = 0;
>  		}
>  	}
>  
> 

[toc] | [prev] | [next] | [standalone]

#1358757

On Wednesday 16 March 2016 10:43:33 Adrian Hunter wrote:
> > +
> > +     /* 32-bit mask as default & fallback */
> > +     if (ret) {
> > +             ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> 
> What happens if device enumeration (e.g. of_dma_configure) has already set a
> more restrictive DMA mask?
> 
> 

In this case, dma_set_mask_and_coherent() is supposed to check the
bus properties settings again and fail dma_set_mask_and_coherent().

We currently don't do that on ARM, which is a bug.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1358769

On 16/03/16 11:07, Arnd Bergmann wrote:
> On Wednesday 16 March 2016 10:43:33 Adrian Hunter wrote:
>>> +
>>> +     /* 32-bit mask as default & fallback */
>>> +     if (ret) {
>>> +             ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>>
>> What happens if device enumeration (e.g. of_dma_configure) has already set a
>> more restrictive DMA mask?
>>
>>
> 
> In this case, dma_set_mask_and_coherent() is supposed to check the
> bus properties settings again and fail dma_set_mask_and_coherent().

So the logic this patch introduces will disable DMA in that case.  Would it
be better just to leave the DMA mask alone (as it does now for most sdhci
drivers) in the 32-bit case?

[toc] | [prev] | [next] | [standalone]

#1358789

On Wednesday 16 March 2016 11:26:48 Adrian Hunter wrote:
> On 16/03/16 11:07, Arnd Bergmann wrote:
> > On Wednesday 16 March 2016 10:43:33 Adrian Hunter wrote:
> >>> +
> >>> +     /* 32-bit mask as default & fallback */
> >>> +     if (ret) {
> >>> +             ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> >>
> >> What happens if device enumeration (e.g. of_dma_configure) has already set a
> >> more restrictive DMA mask?
> >>
> >>
> > 
> > In this case, dma_set_mask_and_coherent() is supposed to check the
> > bus properties settings again and fail dma_set_mask_and_coherent().
> 
> So the logic this patch introduces will disable DMA in that case.  Would it
> be better just to leave the DMA mask alone (as it does now for most sdhci
> drivers) in the 32-bit case?

It depends to some degree on the specific capabilities of the system.

Basically when the driver asks for a 32-bit mask, we have to check if that
is actually possible, and there are a couple of possible outcomes:

- If the bus is less than 32-bit wide but the RAM is small enough to
  to fit within the addressable range of the bus, the
  dma_set_mask_and_coherent() should succeed

- If the RAM is larger than what the bus can address, but swiotlb
  is configured and the swiotlb bounce buffer is addressable by
  the bus, dma_set_mask_and_coherent() should also succeed

- If there is no swiotlb and there is RAM that fits into the 32-bit
  mask but that is not addressable by the bus, the
  dma_set_mask_and_coherent() should fail, and the driver should not
  use DMA.

- Similarly, if swiotlb is enabled, but its bounce buffer is not
  reachable by the bus, the call needs to fail and the driver must
  not use DMA.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web