linux-kernel - Re: [PATCH V2 2/2] mmc: tegra: Only advertise UHS modes if IO regulator is present

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Sat, 23 Jul 2016 11:38:08 +0200
From:	Ulf Hansson <ulf.hansson@...aro.org>
To:	Jon Hunter <jonathanh@...dia.com>
Cc:	Adrian Hunter <adrian.hunter@...el.com>,
	Stephen Warren <swarren@...dotorg.org>,
	Thierry Reding <thierry.reding@...il.com>,
	Alexandre Courbot <gnurou@...il.com>,
	linux-mmc <linux-mmc@...r.kernel.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 2/2] mmc: tegra: Only advertise UHS modes if IO
 regulator is present

On 12 July 2016 at 15:53, Jon Hunter <jonathanh@...dia.com> wrote:
> To support UHS modes for Tegra an external regulator must be present
> to adjust the IO voltage accordingly. Even if the regulator is not
> present but the host supports the UHS modes and the device supports the
> UHS modes, then we will attempt to switch to a high-speed mode. Without
> an external regulator, Tegra will fail to switch to the high-speed
> mode.
>
> It has been found that with some SD cards, that once it has been switch
> to operate at a high-speed mode, all subsequent commands issues to the
> card will fail and so it will not be possible to switch back to a non
> high-speed mode and so the SD card initialisation will fail.
>
> The SDHCI core does not require that the host have an external regulator
> when switching to UHS modes and therefore, the Tegra SDHCI host
> controller should only advertise the UHS modes as being supported if the
> regulator for the IO voltage is present. Fortunately, Tegra has a vendor
> specific register which can be used to control which modes are
> advertised via the SDHCI_CAPABILITIES register. Hence, if there is no IO
> voltage regulator available for the Tegra SDHCI host, then don't
> advertise the UHS modes.
>
> Note that if the regulator is not available, we also don't advertise that
> the SDHCI is compatible with v3.0 of the SDHCI specification because
> this will read the SDHCI_CAPABILITIES_1 register which will enable other
> UHS modes.
>
> This fixes commit 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes") which
> enables UHS mode without checking if the board can support them.
>
> Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
>
> Signed-off-by: Jon Hunter <jonathanh@...dia.com>

Thanks, applied for next!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-tegra.c | 49 +++++++++++++++++++++++++-----------------
>  1 file changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index bcc0de47fe7e..bd1199825f9f 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -148,28 +148,37 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
>                 return;
>
>         misc_ctrl = sdhci_readl(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
> -       /* Erratum: Enable SDHCI spec v3.00 support */
> -       if (soc_data->nvquirks & NVQUIRK_ENABLE_SDHCI_SPEC_300)
> -               misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300;
> -       /* Advertise UHS modes as supported by host */
> -       if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR50)
> -               misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR50;
> -       else
> -               misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR50;
> -       if (soc_data->nvquirks & NVQUIRK_ENABLE_DDR50)
> -               misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_DDR50;
> -       else
> -               misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_DDR50;
> -       if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR104)
> -               misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR104;
> -       else
> -               misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR104;
> -       sdhci_writel(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
> -
>         clk_ctrl = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
> +
> +       misc_ctrl &= ~(SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300 |
> +                      SDHCI_MISC_CTRL_ENABLE_SDR50 |
> +                      SDHCI_MISC_CTRL_ENABLE_DDR50 |
> +                      SDHCI_MISC_CTRL_ENABLE_SDR104);
> +
>         clk_ctrl &= ~SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE;
> -       if (soc_data->nvquirks & SDHCI_MISC_CTRL_ENABLE_SDR50)
> -               clk_ctrl |= SDHCI_CLOCK_CTRL_SDR50_TUNING_OVERRIDE;
> +
> +       /*
> +        * If the board does not define a regulator for the SDHCI
> +        * IO voltage, then don't advertise support for UHS modes
> +        * even if the device supports it because the IO voltage
> +        * cannot be configured.
> +        */
> +       if (!IS_ERR(host->mmc->supply.vqmmc)) {
> +               /* Erratum: Enable SDHCI spec v3.00 support */
> +               if (soc_data->nvquirks & NVQUIRK_ENABLE_SDHCI_SPEC_300)
> +                       misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300;
> +               /* Advertise UHS modes as supported by host */
> +               if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR50)
> +                       misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR50;
> +               if (soc_data->nvquirks & NVQUIRK_ENABLE_DDR50)
> +                       misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_DDR50;
> +               if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR104)
> +                       misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR104;
> +               if (soc_data->nvquirks & SDHCI_MISC_CTRL_ENABLE_SDR50)
> +                       clk_ctrl |= SDHCI_CLOCK_CTRL_SDR50_TUNING_OVERRIDE;
> +       }
> +
> +       sdhci_writel(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
>         sdhci_writel(host, clk_ctrl, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
>
>         if (soc_data->nvquirks & NVQUIRK_HAS_PADCALIB)
> --
> 2.1.4
>