diff mbox

mmc: tegra: Mark 64-bit DMA broken on Tegra124

Message ID 20160831162324.15480-1-thierry.reding@gmail.com
State New, archived
Headers show

Commit Message

Thierry Reding Aug. 31, 2016, 4:23 p.m. UTC
From: Thierry Reding <treding@nvidia.com>

According to the TRM, the SD/MMC controller on Tegra124 supports 34-bit
addressing, but testing shows that this doesn't work. On a device which
has more than 2 GiB of RAM and LPAE enabled, buffer allocations can use
addresses above the 32-bit boundary.

One way to work around this would be to enable IOMMU physical to virtual
address translations for the SD/MMC controllers, but that's not easy to
implement without breaking existing use-cases. It's also not obvious why
34-bit addressing doesn't work as advertised. In order to fix this for
existing users, add the SDHCI_QUIRK2_BROKEN_64_BIT_DMA quirk for now.

Reported-by: Paul Kocialkowski <contact@paulk.fr>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Comments

Stephen Warren Aug. 31, 2016, 4:40 p.m. UTC | #1
On 08/31/2016 10:23 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> According to the TRM, the SD/MMC controller on Tegra124 supports 34-bit
> addressing, but testing shows that this doesn't work. On a device which
> has more than 2 GiB of RAM and LPAE enabled, buffer allocations can use
> addresses above the 32-bit boundary.
>
> One way to work around this would be to enable IOMMU physical to virtual
> address translations for the SD/MMC controllers, but that's not easy to
> implement without breaking existing use-cases. It's also not obvious why
> 34-bit addressing doesn't work as advertised. In order to fix this for
> existing users, add the SDHCI_QUIRK2_BROKEN_64_BIT_DMA quirk for now.

Acked-by: Stephen Warren <swarren@nvidia.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lucas Stach Aug. 31, 2016, 7:24 p.m. UTC | #2
Am Mittwoch, den 31.08.2016, 18:23 +0200 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
> 
> According to the TRM, the SD/MMC controller on Tegra124 supports 34-
> bit
> addressing, but testing shows that this doesn't work. On a device
> which
> has more than 2 GiB of RAM and LPAE enabled, buffer allocations can
> use
> addresses above the 32-bit boundary.
> 
> One way to work around this would be to enable IOMMU physical to
> virtual
> address translations for the SD/MMC controllers, but that's not easy
> to
> implement without breaking existing use-cases. It's also not obvious
> why
> 34-bit addressing doesn't work as advertised. In order to fix this
> for
> existing users, add the SDHCI_QUIRK2_BROKEN_64_BIT_DMA quirk for now.
> 
> Reported-by: Paul Kocialkowski <contact@paulk.fr>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-
> tegra.c
> index d89200ee017e..a3d045630d0c 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -394,6 +394,22 @@ static const struct sdhci_tegra_soc_data
> soc_data_tegra114 = {
>  	.pdata = &sdhci_tegra114_pdata,
>  };
>  
> +static const struct sdhci_pltfm_data sdhci_tegra124_pdata = {
> +	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
> +		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> +		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
> +		  SDHCI_QUIRK_NO_HISPD_BIT |
> +		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
> +		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |

IMHO it would be good to have a short comment in the code saying why
this flag is necessary. Not everyone checks the git log before trying
to change those things...

Regards,
Lucas

> +		   SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
> +	.ops  = &tegra114_sdhci_ops,
> +};
> +
> +static const struct sdhci_tegra_soc_data soc_data_tegra124 = {
> +	.pdata = &sdhci_tegra124_pdata,
> +};
> +
>  static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {
>  	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
>  		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> @@ -427,7 +443,7 @@ static const struct sdhci_tegra_soc_data
> soc_data_tegra186 = {
>  static const struct of_device_id sdhci_tegra_dt_match[] = {
>  	{ .compatible = "nvidia,tegra186-sdhci", .data =
> &soc_data_tegra186 },
>  	{ .compatible = "nvidia,tegra210-sdhci", .data =
> &soc_data_tegra210 },
> -	{ .compatible = "nvidia,tegra124-sdhci", .data =
> &soc_data_tegra114 },
> +	{ .compatible = "nvidia,tegra124-sdhci", .data =
> &soc_data_tegra124 },
>  	{ .compatible = "nvidia,tegra114-sdhci", .data =
> &soc_data_tegra114 },
>  	{ .compatible = "nvidia,tegra30-sdhci", .data =
> &soc_data_tegra30 },
>  	{ .compatible = "nvidia,tegra20-sdhci", .data =
> &soc_data_tegra20 },
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Aug. 31, 2016, 7:35 p.m. UTC | #3
On Wednesday, August 31, 2016 6:23:24 PM CEST Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> According to the TRM, the SD/MMC controller on Tegra124 supports 34-bit
> addressing, but testing shows that this doesn't work. On a device which
> has more than 2 GiB of RAM and LPAE enabled, buffer allocations can use
> addresses above the 32-bit boundary.
> 
> One way to work around this would be to enable IOMMU physical to virtual
> address translations for the SD/MMC controllers, but that's not easy to
> implement without breaking existing use-cases. It's also not obvious why
> 34-bit addressing doesn't work as advertised. In order to fix this for
> existing users, add the SDHCI_QUIRK2_BROKEN_64_BIT_DMA quirk for now.
> 
> Reported-by: Paul Kocialkowski <contact@paulk.fr>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---

Acked-by: Arnd Bergmann <arnd@arndb.de>

I also tried to find the code that handles bounce buffers for the attached
devices in the absence of SWIOTLB (which ARM does not use). The mmc block
driver handles this correctly with the blk_queue_bounce_limit() call,
and any SDIO network devices will work as long as they

a) don't advertize NETIF_F_HIGHDMA, and
b) all memory that is out of reach of the SDHCI device is also
   beyond the highmem boundary (this might not be true with
   CONFIG_VMSPLIT_1G)

All other SDIO devices (camera, DVB, bluetooth) might need driver
changes or SWIOTLB support here, but those devices are fairly rare,
and it's possible that all upstream drivers are actually ok.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adrian Hunter Sept. 1, 2016, 6:44 a.m. UTC | #4
On 31/08/16 19:23, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> According to the TRM, the SD/MMC controller on Tegra124 supports 34-bit
> addressing, but testing shows that this doesn't work. On a device which
> has more than 2 GiB of RAM and LPAE enabled, buffer allocations can use
> addresses above the 32-bit boundary.
> 
> One way to work around this would be to enable IOMMU physical to virtual
> address translations for the SD/MMC controllers, but that's not easy to
> implement without breaking existing use-cases. It's also not obvious why
> 34-bit addressing doesn't work as advertised. In order to fix this for
> existing users, add the SDHCI_QUIRK2_BROKEN_64_BIT_DMA quirk for now.
> 
> Reported-by: Paul Kocialkowski <contact@paulk.fr>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Doesn't apply cleanly, but otherwise:

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

> ---
>  drivers/mmc/host/sdhci-tegra.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index d89200ee017e..a3d045630d0c 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -394,6 +394,22 @@ static const struct sdhci_tegra_soc_data soc_data_tegra114 = {
>  	.pdata = &sdhci_tegra114_pdata,
>  };
>  
> +static const struct sdhci_pltfm_data sdhci_tegra124_pdata = {
> +	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
> +		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> +		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
> +		  SDHCI_QUIRK_NO_HISPD_BIT |
> +		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
> +		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> +		   SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
> +	.ops  = &tegra114_sdhci_ops,
> +};
> +
> +static const struct sdhci_tegra_soc_data soc_data_tegra124 = {
> +	.pdata = &sdhci_tegra124_pdata,
> +};
> +
>  static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {
>  	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
>  		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> @@ -427,7 +443,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
>  static const struct of_device_id sdhci_tegra_dt_match[] = {
>  	{ .compatible = "nvidia,tegra186-sdhci", .data = &soc_data_tegra186 },
>  	{ .compatible = "nvidia,tegra210-sdhci", .data = &soc_data_tegra210 },
> -	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra114 },
> +	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra124 },
>  	{ .compatible = "nvidia,tegra114-sdhci", .data = &soc_data_tegra114 },
>  	{ .compatible = "nvidia,tegra30-sdhci", .data = &soc_data_tegra30 },
>  	{ .compatible = "nvidia,tegra20-sdhci", .data = &soc_data_tegra20 },
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Sept. 1, 2016, 7:51 a.m. UTC | #5
On 31 August 2016 at 18:23, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> According to the TRM, the SD/MMC controller on Tegra124 supports 34-bit
> addressing, but testing shows that this doesn't work. On a device which
> has more than 2 GiB of RAM and LPAE enabled, buffer allocations can use
> addresses above the 32-bit boundary.
>
> One way to work around this would be to enable IOMMU physical to virtual
> address translations for the SD/MMC controllers, but that's not easy to
> implement without breaking existing use-cases. It's also not obvious why
> 34-bit addressing doesn't work as advertised. In order to fix this for
> existing users, add the SDHCI_QUIRK2_BROKEN_64_BIT_DMA quirk for now.
>
> Reported-by: Paul Kocialkowski <contact@paulk.fr>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Looks good to me, although could you please address Lucas' comment and
send a re-based version.

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-tegra.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index d89200ee017e..a3d045630d0c 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -394,6 +394,22 @@ static const struct sdhci_tegra_soc_data soc_data_tegra114 = {
>         .pdata = &sdhci_tegra114_pdata,
>  };
>
> +static const struct sdhci_pltfm_data sdhci_tegra124_pdata = {
> +       .quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
> +                 SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> +                 SDHCI_QUIRK_SINGLE_POWER_WRITE |
> +                 SDHCI_QUIRK_NO_HISPD_BIT |
> +                 SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
> +                 SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +       .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> +                  SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
> +       .ops  = &tegra114_sdhci_ops,
> +};
> +
> +static const struct sdhci_tegra_soc_data soc_data_tegra124 = {
> +       .pdata = &sdhci_tegra124_pdata,
> +};
> +
>  static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {
>         .quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
>                   SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> @@ -427,7 +443,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
>  static const struct of_device_id sdhci_tegra_dt_match[] = {
>         { .compatible = "nvidia,tegra186-sdhci", .data = &soc_data_tegra186 },
>         { .compatible = "nvidia,tegra210-sdhci", .data = &soc_data_tegra210 },
> -       { .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra114 },
> +       { .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra124 },
>         { .compatible = "nvidia,tegra114-sdhci", .data = &soc_data_tegra114 },
>         { .compatible = "nvidia,tegra30-sdhci", .data = &soc_data_tegra30 },
>         { .compatible = "nvidia,tegra20-sdhci", .data = &soc_data_tegra20 },
> --
> 2.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index d89200ee017e..a3d045630d0c 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -394,6 +394,22 @@  static const struct sdhci_tegra_soc_data soc_data_tegra114 = {
 	.pdata = &sdhci_tegra114_pdata,
 };
 
+static const struct sdhci_pltfm_data sdhci_tegra124_pdata = {
+	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
+		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
+		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
+		  SDHCI_QUIRK_NO_HISPD_BIT |
+		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+		   SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
+	.ops  = &tegra114_sdhci_ops,
+};
+
+static const struct sdhci_tegra_soc_data soc_data_tegra124 = {
+	.pdata = &sdhci_tegra124_pdata,
+};
+
 static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {
 	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
 		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
@@ -427,7 +443,7 @@  static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
 static const struct of_device_id sdhci_tegra_dt_match[] = {
 	{ .compatible = "nvidia,tegra186-sdhci", .data = &soc_data_tegra186 },
 	{ .compatible = "nvidia,tegra210-sdhci", .data = &soc_data_tegra210 },
-	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra114 },
+	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra124 },
 	{ .compatible = "nvidia,tegra114-sdhci", .data = &soc_data_tegra114 },
 	{ .compatible = "nvidia,tegra30-sdhci", .data = &soc_data_tegra30 },
 	{ .compatible = "nvidia,tegra20-sdhci", .data = &soc_data_tegra20 },