* [PATCH v3 1/3] mmc: sdhci: Set DMA mask when adding host
2016-03-04 10:38 [PATCH v3 0/3] mmc: sdhci: Set DMA mask properly Alexandre Courbot
@ 2016-03-04 10:38 ` Alexandre Courbot
2016-03-04 15:57 ` Arnd Bergmann
2016-03-04 10:38 ` [PATCH v3 2/3] mmc: sdhci-acpi: Remove enable_dma() hook Alexandre Courbot
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Alexandre Courbot @ 2016-03-04 10:38 UTC (permalink / raw)
To: Ulf Hansson, Adrian Hunter, Arnd Bergmann
Cc: linux-mmc, linux-kernel, gnurou, 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 | 46 +++++++++++++++++++++++++++++++++++++++-------
1 file changed, 39 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fd9139947fa3..00fb45ba6f39 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2857,6 +2857,34 @@ 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));
+ }
+
+ /* 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 +2960,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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/3] mmc: sdhci: Set DMA mask when adding host
2016-03-04 10:38 ` [PATCH v3 1/3] mmc: sdhci: Set DMA mask when adding host Alexandre Courbot
@ 2016-03-04 15:57 ` Arnd Bergmann
2016-03-07 2:06 ` Alexandre Courbot
0 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2016-03-04 15:57 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Ulf Hansson, Adrian Hunter, linux-mmc, linux-kernel, gnurou
On Friday 04 March 2016 19:38:43 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 | 46 +++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index fd9139947fa3..00fb45ba6f39 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2857,6 +2857,34 @@ 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));
> + }
> +
I think you need to disable the SDHCI_USE_64_BIT_DMA flag when
dma_set_mask_and_coherent() fails here. Otherwise looks good.
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/3] mmc: sdhci: Set DMA mask when adding host
2016-03-04 15:57 ` Arnd Bergmann
@ 2016-03-07 2:06 ` Alexandre Courbot
0 siblings, 0 replies; 9+ messages in thread
From: Alexandre Courbot @ 2016-03-07 2:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Alexandre Courbot, Ulf Hansson, Adrian Hunter, linux-mmc,
Linux Kernel Mailing List
On Sat, Mar 5, 2016 at 12:57 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 04 March 2016 19:38:43 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 | 46 +++++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 39 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index fd9139947fa3..00fb45ba6f39 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -2857,6 +2857,34 @@ 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));
>> + }
>> +
>
> I think you need to disable the SDHCI_USE_64_BIT_DMA flag when
> dma_set_mask_and_coherent() fails here. Otherwise looks good.
Ah, you're right, thanks. v4 is on the way.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] mmc: sdhci-acpi: Remove enable_dma() hook
2016-03-04 10:38 [PATCH v3 0/3] mmc: sdhci: Set DMA mask properly Alexandre Courbot
2016-03-04 10:38 ` [PATCH v3 1/3] mmc: sdhci: Set DMA mask when adding host Alexandre Courbot
@ 2016-03-04 10:38 ` Alexandre Courbot
2016-03-04 15:57 ` Arnd Bergmann
2016-03-04 10:38 ` [PATCH v3 3/3] mmc: sdhci-pci: Do not set DMA mask in enable_dma() Alexandre Courbot
2016-03-04 15:59 ` [PATCH v3 0/3] mmc: sdhci: Set DMA mask properly Arnd Bergmann
3 siblings, 1 reply; 9+ messages in thread
From: Alexandre Courbot @ 2016-03-04 10:38 UTC (permalink / raw)
To: Ulf Hansson, Adrian Hunter, Arnd Bergmann
Cc: linux-mmc, linux-kernel, gnurou, 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>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 3/3] mmc: sdhci-pci: Do not set DMA mask in enable_dma()
2016-03-04 10:38 [PATCH v3 0/3] mmc: sdhci: Set DMA mask properly Alexandre Courbot
2016-03-04 10:38 ` [PATCH v3 1/3] mmc: sdhci: Set DMA mask when adding host Alexandre Courbot
2016-03-04 10:38 ` [PATCH v3 2/3] mmc: sdhci-acpi: Remove enable_dma() hook Alexandre Courbot
@ 2016-03-04 10:38 ` Alexandre Courbot
2016-03-04 15:58 ` Arnd Bergmann
2016-03-04 15:59 ` [PATCH v3 0/3] mmc: sdhci: Set DMA mask properly Arnd Bergmann
3 siblings, 1 reply; 9+ messages in thread
From: Alexandre Courbot @ 2016-03-04 10:38 UTC (permalink / raw)
To: Ulf Hansson, Adrian Hunter, Arnd Bergmann
Cc: linux-mmc, linux-kernel, gnurou, 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>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 0/3] mmc: sdhci: Set DMA mask properly
2016-03-04 10:38 [PATCH v3 0/3] mmc: sdhci: Set DMA mask properly Alexandre Courbot
` (2 preceding siblings ...)
2016-03-04 10:38 ` [PATCH v3 3/3] mmc: sdhci-pci: Do not set DMA mask in enable_dma() Alexandre Courbot
@ 2016-03-04 15:59 ` Arnd Bergmann
3 siblings, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2016-03-04 15:59 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Ulf Hansson, Adrian Hunter, linux-mmc, linux-kernel, gnurou
On Friday 04 March 2016 19:38:42 Alexandre Courbot wrote:
> Well that's far from the two-liner I had in mind to fix our bounce-buffers
> problem with Tegra, but it feels correct. Testing for ACPI and PCI would be
> appreciated (I am rather confident that ACPI will be ok, but the PCI part
> should be reviewed by someone who knows better).
>
> 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.
>
> Thanks to Arnd for the insightful discussion that led to this.
Looks great overall, much simpler and saner in the new version. I found
one small bug, which appears to have been preexisting, but you moved
it to a different file now.
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread