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

[Thierry Reding]

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@xxxxxxxxxx>
> Reviewed-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: PGP signature