linux-kernel - Re: [PATCH 15/40] mmc: tegra: Power on the calibration pad

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180809125226.GW21639@ulmo>
Date:   Thu, 9 Aug 2018 14:52:26 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Aapo Vienamo <avienamo@...dia.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Mikko Perttunen <mperttunen@...dia.com>,
        Stefan Agner <stefan@...er.ch>, devicetree@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mmc@...r.kernel.org
Subject: Re: [PATCH 15/40] mmc: tegra: Power on the calibration pad

On Wed, Aug 01, 2018 at 07:32:05PM +0300, Aapo Vienamo wrote:
> Automatic pad drive strength calibration is performed on a separate pad
> identical to the ones used for driving the actual bus. Power on the
> calibration pad during the calibration procedure and power it off
> afterwards to save power.
> 
> Signed-off-by: Aapo Vienamo <avienamo@...dia.com>
> Reviewed-by: Mikko Perttunen <mperttunen@...dia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 53c035b9..9e22fec 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -54,6 +54,7 @@
>  #define SDHCI_TEGRA_SDMEM_COMP_PADCTRL			0x1e0
>  #define SDHCI_TEGRA_SDMEM_COMP_PADCTRL_VREF_SEL_MASK	0x0000000f
>  #define SDHCI_TEGRA_SDMEM_COMP_PADCTRL_VREF_SEL_VAL	0x7
> +#define SDHCI_TEGRA_SDMEM_COMP_PADCTRL_E_INPUT_E_PWRD	BIT(31)
>  
>  #define SDHCI_TEGRA_AUTO_CAL_STATUS			0x1ec
>  #define SDHCI_TEGRA_AUTO_CAL_ACTIVE			BIT(31)
> @@ -240,11 +241,32 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
>  	tegra_host->ddr_signaling = false;
>  }
>  
> +static void tegra_sdhci_configure_cal_pad(struct sdhci_host *host, bool enable)
> +{
> +	u32 reg;
> +
> +	/*
> +	 * Enable or disable the additional I/O pad used by the drive strength
> +	 * calibration process.
> +	 */
> +	reg = sdhci_readl(host, SDHCI_TEGRA_SDMEM_COMP_PADCTRL);
> +	if (enable) {
> +		reg |= SDHCI_TEGRA_SDMEM_COMP_PADCTRL_E_INPUT_E_PWRD;
> +		sdhci_writel(host, reg, SDHCI_TEGRA_SDMEM_COMP_PADCTRL);
> +		udelay(1);
> +	} else {
> +		reg &= ~SDHCI_TEGRA_SDMEM_COMP_PADCTRL_E_INPUT_E_PWRD;
> +		sdhci_writel(host, reg, SDHCI_TEGRA_SDMEM_COMP_PADCTRL);
> +	}

Might be worth extracting the common write as well. This would of course
mean that we either have to conditionalize the udelay() again, or we
just keep it in both cases. Doesn't really hurt to wait a little on
disabling, right?

Either way is fine with me. Though it would be nice if we could make the
udelay() go away and use a sleeping variant if possible. This isn't run
very often, so even if we overshoot by a couple of microseconds it isn't
that bad. 1 us is pretty short, but I've been burnt by busy loops a
couple of times, so I like to avoid them at all costs.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)