[PATCH v4 0/3] mmc: sdhci: Set DMA mask properly

[PATCH v4 0/3] mmc: sdhci: Set DMA mask properly

[Alexandre Courbot]

64-bit capable devices are supposed to set their own DMA mask. Currently
this does not happen for sdhci devices excepted for the two (sdhci-acpi
and sdhci-pci) that define a enable_dma() hook and do it there. However
this hook is called from several places while DMA mask is supposed to
be set only once ; for instance the sdhci-acpi driver maintains a flag
just to make sure the DMA mask is set only upon the first call of this
hook.

For the vast majority of drivers that do not define a enable_dma() hook, the
default 32-bit DMA mask is used and there is a risk of using unneeded bounce
buffers on hosts capable of 64-bit addressing.

The first patch adds a default DMA mask setting function that is called when
a DMA-capable host is added. It tries to set sane DMA masks according to the
device's reported capabilities.

The addition of this function seems to make the same code in sdhci-acpi and
sdhci-pci redundant, so it is removed from these drivers. On top of making
this series a negative line count, it also removes one usage of the obsolete
pci_set_dma_mask() function.

Changes since v3:
- Unset the SDHCI_USE_64_BIT_DMA flag if setting of 64-bit mask failed
- Carry Acked-bys

Alexandre Courbot (3):
  mmc: sdhci: Set DMA mask when adding host
  mmc: sdhci-acpi: Remove enable_dma() hook
  mmc: sdhci-pci: Do not set DMA mask in enable_dma()

 drivers/mmc/host/sdhci-acpi.c     | 30 ------------------------
 drivers/mmc/host/sdhci-pci-core.c | 15 ------------
 drivers/mmc/host/sdhci.c          | 48 +++++++++++++++++++++++++++++++++------
 3 files changed, 41 insertions(+), 52 deletions(-)

-- 
2.7.2

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

[Alexandre Courbot]

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

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

[Adrian Hunter]

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;
>  		}
>  	}
>  
> 

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

[Arnd Bergmann]

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

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

[Adrian Hunter]

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?

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

[Arnd Bergmann]

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

[PATCH v4 2/3] mmc: sdhci-acpi: Remove enable_dma() hook

[Alexandre Courbot]

This hook was solely used to set the DMA mask, which is now done
by the newly-added sdhci_set_dma_mask() function.

The use of a flag to ensure the mask is only set once is a strong hint
that it should not have been done there anyway.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mmc/host/sdhci-acpi.c | 30 ------------------------------
 1 file changed, 30 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 195ff0853dc8..416da6f3629c 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -75,7 +75,6 @@ struct sdhci_acpi_host {
 	const struct sdhci_acpi_slot	*slot;
 	struct platform_device		*pdev;
 	bool				use_runtime_pm;
-	bool				dma_setup;
 };
 
 static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)
@@ -83,33 +82,6 @@ static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)
 	return c->slot && (c->slot->flags & flag);
 }
 
-static int sdhci_acpi_enable_dma(struct sdhci_host *host)
-{
-	struct sdhci_acpi_host *c = sdhci_priv(host);
-	struct device *dev = &c->pdev->dev;
-	int err = -1;
-
-	if (c->dma_setup)
-		return 0;
-
-	if (host->flags & SDHCI_USE_64_BIT_DMA) {
-		if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA) {
-			host->flags &= ~SDHCI_USE_64_BIT_DMA;
-		} else {
-			err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
-			if (err)
-				dev_warn(dev, "Failed to set 64-bit DMA mask\n");
-		}
-	}
-
-	if (err)
-		err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-
-	c->dma_setup = !err;
-
-	return err;
-}
-
 static void sdhci_acpi_int_hw_reset(struct sdhci_host *host)
 {
 	u8 reg;
@@ -127,7 +99,6 @@ static void sdhci_acpi_int_hw_reset(struct sdhci_host *host)
 
 static const struct sdhci_ops sdhci_acpi_ops_dflt = {
 	.set_clock = sdhci_set_clock,
-	.enable_dma = sdhci_acpi_enable_dma,
 	.set_bus_width = sdhci_set_bus_width,
 	.reset = sdhci_reset,
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
@@ -135,7 +106,6 @@ static const struct sdhci_ops sdhci_acpi_ops_dflt = {
 
 static const struct sdhci_ops sdhci_acpi_ops_int = {
 	.set_clock = sdhci_set_clock,
-	.enable_dma = sdhci_acpi_enable_dma,
 	.set_bus_width = sdhci_set_bus_width,
 	.reset = sdhci_reset,
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
-- 
2.7.2

[PATCH v4 3/3] mmc: sdhci-pci: Do not set DMA mask in enable_dma()

[Alexandre Courbot]

DMA mask will already be set by sdhci_set_dma_mask(), which
is equivalent to the removed code since pci_set_dma_mask()
expands to its DMA-API counterpart.

There should also be no reason to set the DMA mask after probe.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mmc/host/sdhci-pci-core.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index df3b8eced8c4..62aa5d0efcee 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1302,7 +1302,6 @@ static int sdhci_pci_enable_dma(struct sdhci_host *host)
 {
 	struct sdhci_pci_slot *slot;
 	struct pci_dev *pdev;
-	int ret = -1;
 
 	slot = sdhci_priv(host);
 	pdev = slot->chip->pdev;
@@ -1314,20 +1313,6 @@ static int sdhci_pci_enable_dma(struct sdhci_host *host)
 			"doesn't fully claim to support it.\n");
 	}
 
-	if (host->flags & SDHCI_USE_64_BIT_DMA) {
-		if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA) {
-			host->flags &= ~SDHCI_USE_64_BIT_DMA;
-		} else {
-			ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
-			if (ret)
-				dev_warn(&pdev->dev, "Failed to set 64-bit DMA mask\n");
-		}
-	}
-	if (ret)
-		ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
-	if (ret)
-		return ret;
-
 	pci_set_master(pdev);
 
 	return 0;
-- 
2.7.2

Re: [PATCH v4 3/3] mmc: sdhci-pci: Do not set DMA mask in enable_dma()

[Adrian Hunter]

On 07/03/16 04:07, Alexandre Courbot wrote:
> DMA mask will already be set by sdhci_set_dma_mask(), which
> is equivalent to the removed code since pci_set_dma_mask()
> expands to its DMA-API counterpart.
> 
> There should also be no reason to set the DMA mask after probe.

Let's run that by the PCI mailing list just to be sure.  The patches for
reference:

	http://marc.info/?l=linux-mmc&m=145731654328126&w=2
	http://marc.info/?l=linux-mmc&m=145731654328128&w=2

change the sdhci-pci driver to set the DMA mask once during probe instead of
every time during resume.  Is there any reason a PCI device driver might
need to set the DMA mask every time during resume?


> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/mmc/host/sdhci-pci-core.c | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index df3b8eced8c4..62aa5d0efcee 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -1302,7 +1302,6 @@ static int sdhci_pci_enable_dma(struct sdhci_host *host)
>  {
>  	struct sdhci_pci_slot *slot;
>  	struct pci_dev *pdev;
> -	int ret = -1;
>  
>  	slot = sdhci_priv(host);
>  	pdev = slot->chip->pdev;
> @@ -1314,20 +1313,6 @@ static int sdhci_pci_enable_dma(struct sdhci_host *host)
>  			"doesn't fully claim to support it.\n");
>  	}
>  
> -	if (host->flags & SDHCI_USE_64_BIT_DMA) {
> -		if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA) {
> -			host->flags &= ~SDHCI_USE_64_BIT_DMA;
> -		} else {
> -			ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
> -			if (ret)
> -				dev_warn(&pdev->dev, "Failed to set 64-bit DMA mask\n");
> -		}
> -	}
> -	if (ret)
> -		ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> -	if (ret)
> -		return ret;
> -
>  	pci_set_master(pdev);
>  
>  	return 0;
> 

Re: [PATCH v4 3/3] mmc: sdhci-pci: Do not set DMA mask in enable_dma()

[Alexandre Courbot]

On Tue, Mar 8, 2016 at 10:18 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 07/03/16 04:07, Alexandre Courbot wrote:
>> DMA mask will already be set by sdhci_set_dma_mask(), which
>> is equivalent to the removed code since pci_set_dma_mask()
>> expands to its DMA-API counterpart.
>>
>> There should also be no reason to set the DMA mask after probe.
>
> Let's run that by the PCI mailing list just to be sure.  The patches for
> reference:
>
>         http://marc.info/?l=linux-mmc&m=145731654328126&w=2
>         http://marc.info/?l=linux-mmc&m=145731654328128&w=2
>
> change the sdhci-pci driver to set the DMA mask once during probe instead of
> every time during resume.  Is there any reason a PCI device driver might
> need to set the DMA mask every time during resume?

Not seeing much reaction for this patchset. PCI being the only
possible point of contention, can we maybe roll it into -next and see
what happens?

Re: [PATCH v4 3/3] mmc: sdhci-pci: Do not set DMA mask in enable_dma()

[Adrian Hunter]

On 14/03/16 05:15, Alexandre Courbot wrote:
> On Tue, Mar 8, 2016 at 10:18 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 07/03/16 04:07, Alexandre Courbot wrote:
>>> DMA mask will already be set by sdhci_set_dma_mask(), which
>>> is equivalent to the removed code since pci_set_dma_mask()
>>> expands to its DMA-API counterpart.
>>>
>>> There should also be no reason to set the DMA mask after probe.
>>
>> Let's run that by the PCI mailing list just to be sure.  The patches for
>> reference:
>>
>>         http://marc.info/?l=linux-mmc&m=145731654328126&w=2
>>         http://marc.info/?l=linux-mmc&m=145731654328128&w=2
>>
>> change the sdhci-pci driver to set the DMA mask once during probe instead of
>> every time during resume.  Is there any reason a PCI device driver might
>> need to set the DMA mask every time during resume?
> 
> Not seeing much reaction for this patchset. PCI being the only
> possible point of contention, can we maybe roll it into -next and see
> what happens?

+Rafael

Rafael, can you offer any thoughts on this:

PCI drivers that want to use DMA might call pci_set_master() in the pm
resume callback.  Some drivers (like sdhci-pci) also, presumably out of
convenience, set the DMA mask at the same time.  The question is: is it OK
instead to set the DMA mask just once during probe, or is there some other
reason the DMA mask needs to be set every time during resume?

Re: [PATCH v4 3/3] mmc: sdhci-pci: Do not set DMA mask in enable_dma()

[Bjorn Helgaas]

On Mon, Mar 14, 2016 at 03:00:36PM +0200, Adrian Hunter wrote:
> On 14/03/16 05:15, Alexandre Courbot wrote:
> > On Tue, Mar 8, 2016 at 10:18 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> >> On 07/03/16 04:07, Alexandre Courbot wrote:
> >>> DMA mask will already be set by sdhci_set_dma_mask(), which
> >>> is equivalent to the removed code since pci_set_dma_mask()
> >>> expands to its DMA-API counterpart.
> >>>
> >>> There should also be no reason to set the DMA mask after probe.
> >>
> >> Let's run that by the PCI mailing list just to be sure.  The patches for
> >> reference:
> >>
> >>         http://marc.info/?l=linux-mmc&m=145731654328126&w=2
> >>         http://marc.info/?l=linux-mmc&m=145731654328128&w=2
> >>
> >> change the sdhci-pci driver to set the DMA mask once during probe instead of
> >> every time during resume.  Is there any reason a PCI device driver might
> >> need to set the DMA mask every time during resume?
> > 
> > Not seeing much reaction for this patchset. PCI being the only
> > possible point of contention, can we maybe roll it into -next and see
> > what happens?
> 
> +Rafael
> 
> Rafael, can you offer any thoughts on this:
> 
> PCI drivers that want to use DMA might call pci_set_master() in the pm
> resume callback.  Some drivers (like sdhci-pci) also, presumably out of
> convenience, set the DMA mask at the same time.  The question is: is it OK
> instead to set the DMA mask just once during probe, or is there some other
> reason the DMA mask needs to be set every time during resume?

I don't see a reason why the DMA mask would need to be set during
resume.

Bjorn

Re: [PATCH v4 0/3] mmc: sdhci: Set DMA mask properly

[Adrian Hunter]

On 07/03/16 04:07, Alexandre Courbot wrote:
> 64-bit capable devices are supposed to set their own DMA mask. Currently
> this does not happen for sdhci devices excepted for the two (sdhci-acpi
> and sdhci-pci) that define a enable_dma() hook and do it there. However
> this hook is called from several places while DMA mask is supposed to
> be set only once ; for instance the sdhci-acpi driver maintains a flag
> just to make sure the DMA mask is set only upon the first call of this
> hook.
> 
> For the vast majority of drivers that do not define a enable_dma() hook, the
> default 32-bit DMA mask is used and there is a risk of using unneeded bounce
> buffers on hosts capable of 64-bit addressing.
> 
> The first patch adds a default DMA mask setting function that is called when
> a DMA-capable host is added. It tries to set sane DMA masks according to the
> device's reported capabilities.
> 
> The addition of this function seems to make the same code in sdhci-acpi and
> sdhci-pci redundant, so it is removed from these drivers. On top of making
> this series a negative line count, it also removes one usage of the obsolete
> pci_set_dma_mask() function.
> 
> Changes since v3:
> - Unset the SDHCI_USE_64_BIT_DMA flag if setting of 64-bit mask failed
> - Carry Acked-bys
> 
> Alexandre Courbot (3):
>   mmc: sdhci: Set DMA mask when adding host
>   mmc: sdhci-acpi: Remove enable_dma() hook
>   mmc: sdhci-pci: Do not set DMA mask in enable_dma()
> 
>  drivers/mmc/host/sdhci-acpi.c     | 30 ------------------------
>  drivers/mmc/host/sdhci-pci-core.c | 15 ------------
>  drivers/mmc/host/sdhci.c          | 48 +++++++++++++++++++++++++++++++++------
>  3 files changed, 41 insertions(+), 52 deletions(-)
> 

I had a couple of questions which have been answered, so for all 3 patches:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Re: [PATCH v4 0/3] mmc: sdhci: Set DMA mask properly

[Ulf Hansson]

On 7 March 2016 at 03:07, Alexandre Courbot <acourbot@nvidia.com> wrote:
> 64-bit capable devices are supposed to set their own DMA mask. Currently
> this does not happen for sdhci devices excepted for the two (sdhci-acpi
> and sdhci-pci) that define a enable_dma() hook and do it there. However
> this hook is called from several places while DMA mask is supposed to
> be set only once ; for instance the sdhci-acpi driver maintains a flag
> just to make sure the DMA mask is set only upon the first call of this
> hook.
>
> For the vast majority of drivers that do not define a enable_dma() hook, the
> default 32-bit DMA mask is used and there is a risk of using unneeded bounce
> buffers on hosts capable of 64-bit addressing.
>
> The first patch adds a default DMA mask setting function that is called when
> a DMA-capable host is added. It tries to set sane DMA masks according to the
> device's reported capabilities.
>
> The addition of this function seems to make the same code in sdhci-acpi and
> sdhci-pci redundant, so it is removed from these drivers. On top of making
> this series a negative line count, it also removes one usage of the obsolete
> pci_set_dma_mask() function.
>
> Changes since v3:
> - Unset the SDHCI_USE_64_BIT_DMA flag if setting of 64-bit mask failed
> - Carry Acked-bys
>
> Alexandre Courbot (3):
>   mmc: sdhci: Set DMA mask when adding host
>   mmc: sdhci-acpi: Remove enable_dma() hook
>   mmc: sdhci-pci: Do not set DMA mask in enable_dma()
>
>  drivers/mmc/host/sdhci-acpi.c     | 30 ------------------------
>  drivers/mmc/host/sdhci-pci-core.c | 15 ------------
>  drivers/mmc/host/sdhci.c          | 48 +++++++++++++++++++++++++++++++++------
>  3 files changed, 41 insertions(+), 52 deletions(-)
>
> --
> 2.7.2
>

Thanks, applied for next!

Kind regards
Uffe