| Message ID | 20160831162324.15480-1-thierry.reding@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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 --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 },