[PATCH v3 1/2] mmc: tegra: Support module reset

[PATCH v3 1/2] mmc: tegra: Support module reset

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The device tree binding for the SDHCI controller found on Tegra SoCs
specifies that a reset control can be provided by the device tree. No
code was ever added to support the module reset, which can cause the
driver to try and access registers from a module that's in reset. On
most Tegra SoC generations doing so would cause a hang.

Note that it's unlikely to see this happen because on most platforms
these resets will have been deasserted by the bootloader. However the
portability can be improved by making sure the driver deasserts the
reset before accessing any registers.

Since resets are synchronous on Tegra SoCs, the platform driver needs
to implement a custom ->remove() callback now to make sure the clock
is disabled after the reset is asserted.

Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Changes in v3:
- use consistent variable names

Changes in v2:
- assert reset in ->probe() error path
- remove unneeded PCI specific quirk

 drivers/mmc/host/sdhci-tegra.c | 43 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 20b6ff5b4af1..9d31ee8988ef 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/reset.h>
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>
@@ -65,6 +66,8 @@ struct sdhci_tegra {
 	struct gpio_desc *power_gpio;
 	bool ddr_signaling;
 	bool pad_calib_required;
+
+	struct reset_control *rst;
 };
 
 static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
@@ -489,6 +492,25 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	clk_prepare_enable(clk);
 	pltfm_host->clk = clk;
 
+	tegra_host->rst = devm_reset_control_get(&pdev->dev, "sdhci");
+	if (IS_ERR(tegra_host->rst)) {
+		rc = PTR_ERR(tegra_host->rst);
+		dev_err(&pdev->dev, "failed to get reset control: %d\n", rc);
+		goto err_rst_get;
+	}
+
+	rc = reset_control_assert(tegra_host->rst);
+	if (rc)
+		goto err_rst_get;
+
+	usleep_range(2000, 4000);
+
+	rc = reset_control_deassert(tegra_host->rst);
+	if (rc)
+		goto err_rst_get;
+
+	usleep_range(2000, 4000);
+
 	rc = sdhci_add_host(host);
 	if (rc)
 		goto err_add_host;
@@ -496,6 +518,8 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	return 0;
 
 err_add_host:
+	reset_control_assert(tegra_host->rst);
+err_rst_get:
 	clk_disable_unprepare(pltfm_host->clk);
 err_clk_get:
 err_power_req:
@@ -504,6 +528,23 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	return rc;
 }
 
+static int sdhci_tegra_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
+
+	sdhci_remove_host(host, 0);
+
+	reset_control_assert(tegra_host->rst);
+	usleep_range(2000, 4000);
+	clk_disable_unprepare(pltfm_host->clk);
+
+	sdhci_pltfm_free(pdev);
+
+	return 0;
+}
+
 static struct platform_driver sdhci_tegra_driver = {
 	.driver		= {
 		.name	= "sdhci-tegra",
@@ -511,7 +552,7 @@ static struct platform_driver sdhci_tegra_driver = {
 		.pm	= &sdhci_pltfm_pmops,
 	},
 	.probe		= sdhci_tegra_probe,
-	.remove		= sdhci_pltfm_unregister,
+	.remove		= sdhci_tegra_remove,
 };
 
 module_platform_driver(sdhci_tegra_driver);
-- 
2.12.0

[PATCH v3 2/2] mmc: tegra: Add Tegra186 support

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The SDHCI controller found on NVIDIA Tegra186 SoCs is very similar to
the one on prior generations of Tegra and can be supported by the same
driver.

Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/mmc/host/sdhci-tegra.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 9d31ee8988ef..7f93079c7a3a 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -434,7 +434,23 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = {
 	.pdata = &sdhci_tegra210_pdata,
 };
 
+static const struct sdhci_pltfm_data sdhci_tegra186_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,
+	.ops  = &tegra114_sdhci_ops,
+};
+
+static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
+	.pdata = &sdhci_tegra186_pdata,
+};
+
 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_tegra124 },
 	{ .compatible = "nvidia,tegra114-sdhci", .data = &soc_data_tegra114 },
-- 
2.12.0

Re: [PATCH v3 1/2] mmc: tegra: Support module reset

[Ulf Hansson]

On 8 March 2017 at 20:00, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The device tree binding for the SDHCI controller found on Tegra SoCs
> specifies that a reset control can be provided by the device tree. No
> code was ever added to support the module reset, which can cause the
> driver to try and access registers from a module that's in reset. On
> most Tegra SoC generations doing so would cause a hang.
>
> Note that it's unlikely to see this happen because on most platforms
> these resets will have been deasserted by the bootloader. However the
> portability can be improved by making sure the driver deasserts the
> reset before accessing any registers.
>
> Since resets are synchronous on Tegra SoCs, the platform driver needs
> to implement a custom ->remove() callback now to make sure the clock
> is disabled after the reset is asserted.
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
> Changes in v3:
> - use consistent variable names
>
> Changes in v2:
> - assert reset in ->probe() error path
> - remove unneeded PCI specific quirk
>
>  drivers/mmc/host/sdhci-tegra.c | 43 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 20b6ff5b4af1..9d31ee8988ef 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -21,6 +21,7 @@
>  #include <linux/io.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/reset.h>
>  #include <linux/mmc/card.h>
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/mmc.h>
> @@ -65,6 +66,8 @@ struct sdhci_tegra {
>         struct gpio_desc *power_gpio;
>         bool ddr_signaling;
>         bool pad_calib_required;
> +
> +       struct reset_control *rst;
>  };
>
>  static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
> @@ -489,6 +492,25 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
>         clk_prepare_enable(clk);
>         pltfm_host->clk = clk;
>
> +       tegra_host->rst = devm_reset_control_get(&pdev->dev, "sdhci");
> +       if (IS_ERR(tegra_host->rst)) {
> +               rc = PTR_ERR(tegra_host->rst);
> +               dev_err(&pdev->dev, "failed to get reset control: %d\n", rc);
> +               goto err_rst_get;
> +       }
> +
> +       rc = reset_control_assert(tegra_host->rst);
> +       if (rc)
> +               goto err_rst_get;
> +
> +       usleep_range(2000, 4000);
> +
> +       rc = reset_control_deassert(tegra_host->rst);
> +       if (rc)
> +               goto err_rst_get;
> +
> +       usleep_range(2000, 4000);
> +
>         rc = sdhci_add_host(host);
>         if (rc)
>                 goto err_add_host;
> @@ -496,6 +518,8 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
>         return 0;
>
>  err_add_host:
> +       reset_control_assert(tegra_host->rst);
> +err_rst_get:
>         clk_disable_unprepare(pltfm_host->clk);
>  err_clk_get:
>  err_power_req:
> @@ -504,6 +528,23 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
>         return rc;
>  }
>
> +static int sdhci_tegra_remove(struct platform_device *pdev)
> +{
> +       struct sdhci_host *host = platform_get_drvdata(pdev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
> +
> +       sdhci_remove_host(host, 0);
> +
> +       reset_control_assert(tegra_host->rst);
> +       usleep_range(2000, 4000);
> +       clk_disable_unprepare(pltfm_host->clk);
> +
> +       sdhci_pltfm_free(pdev);
> +
> +       return 0;
> +}
> +
>  static struct platform_driver sdhci_tegra_driver = {
>         .driver         = {
>                 .name   = "sdhci-tegra",
> @@ -511,7 +552,7 @@ static struct platform_driver sdhci_tegra_driver = {
>                 .pm     = &sdhci_pltfm_pmops,
>         },
>         .probe          = sdhci_tegra_probe,
> -       .remove         = sdhci_pltfm_unregister,
> +       .remove         = sdhci_tegra_remove,
>  };
>
>  module_platform_driver(sdhci_tegra_driver);
> --
> 2.12.0
>

Re: [PATCH v3 2/2] mmc: tegra: Add Tegra186 support

[Ulf Hansson]

On 8 March 2017 at 20:00, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The SDHCI controller found on NVIDIA Tegra186 SoCs is very similar to
> the one on prior generations of Tegra and can be supported by the same
> driver.
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Thanks, applied for next!

However, the new compatible string must be documented. Please send a
patch on top to fix that!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-tegra.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 9d31ee8988ef..7f93079c7a3a 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -434,7 +434,23 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = {
>         .pdata = &sdhci_tegra210_pdata,
>  };
>
> +static const struct sdhci_pltfm_data sdhci_tegra186_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,
> +       .ops  = &tegra114_sdhci_ops,
> +};
> +
> +static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
> +       .pdata = &sdhci_tegra186_pdata,
> +};
> +
>  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_tegra124 },
>         { .compatible = "nvidia,tegra114-sdhci", .data = &soc_data_tegra114 },
> --
> 2.12.0
>