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

From: Thierry Reding
Date: Thu Aug 09 2018 - 08:52:33 EST


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