On Sat, Aug 19, 2023 at 06:35:01PM +0300, Svyatoslav Ryhel wrote:
> Default-tap and default-trim values are used for eMMC setup
> mostly on T114+ devices. As for now, those values are hardcoded
> for T210 and ignored for all other Tegra generations. Fix this
> by passing tap and trim values from dts.
>
> Tested-by: Svyatoslav Ryhel <clamo...@gmail.com> # ASUS TF701T
> Signed-off-by: Svyatoslav Ryhel <clamo...@gmail.com>
> ---
> arch/arm/include/asm/arch-tegra/tegra_mmc.h | 17 ++++----
> drivers/mmc/tegra_mmc.c | 46 ++++++++++-----------
> 2 files changed, 30 insertions(+), 33 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> index d6a55764ba..750c7d809e 100644
> --- a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> +++ b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> @@ -128,21 +128,22 @@ struct tegra_mmc {
>
> /* SDMMC1/3 settings from SDMMCx Initialization Sequence of TRM */
> #define MEMCOMP_PADCTRL_VREF 7
> -#define AUTO_CAL_ENABLE (1 << 29)
> -#define AUTO_CAL_ACTIVE (1 << 31)
> -#define AUTO_CAL_START (1 << 31)
> +#define AUTO_CAL_ENABLE BIT(29)
> +#define AUTO_CAL_ACTIVE BIT(31)
> +#define AUTO_CAL_START BIT(31)
> +
> #if defined(CONFIG_TEGRA210)
> #define AUTO_CAL_PD_OFFSET (0x7D << 8)
> #define AUTO_CAL_PU_OFFSET (0 << 0)
> -#define IO_TRIM_BYPASS_MASK (1 << 2)
> -#define TRIM_VAL_SHIFT 24
> -#define TRIM_VAL_MASK (0x1F << TRIM_VAL_SHIFT)
> -#define TAP_VAL_SHIFT 16
> -#define TAP_VAL_MASK (0xFF << TAP_VAL_SHIFT)
> #else
> #define AUTO_CAL_PD_OFFSET (0x70 << 8)
> #define AUTO_CAL_PU_OFFSET (0x62 << 0)
> #endif
>
> +#define TRIM_VAL_SHIFT 24
> +#define TRIM_VAL_MASK (0x1F << TRIM_VAL_SHIFT)
> +#define TAP_VAL_SHIFT 16
> +#define TAP_VAL_MASK (0xFF << TAP_VAL_SHIFT)
> +
> #endif /* __ASSEMBLY__ */
> #endif /* __TEGRA_MMC_H_ */
> diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
> index f76fee3ea0..7627800261 100644
> --- a/drivers/mmc/tegra_mmc.c
> +++ b/drivers/mmc/tegra_mmc.c
> @@ -37,6 +37,9 @@ struct tegra_mmc_priv {
> unsigned int version; /* SDHCI spec. version */
> unsigned int clock; /* Current clock (MHz) */
> int mmc_id; /* peripheral id */
> +
> + u32 tap_value;
> + u32 trim_value;
> };
>
> static void tegra_mmc_set_power(struct tegra_mmc_priv *priv,
> @@ -526,31 +529,6 @@ static void tegra_mmc_pad_init(struct tegra_mmc_priv
> *priv)
> printf("%s: Warning: Autocal timed out!\n", __func__);
> /* TBD: Set CFG2TMC_SDMMC1_PAD_CAL_DRV* regs here */
> }
> -
> -#if defined(CONFIG_TEGRA210)
> - u32 tap_value, trim_value;
> -
> - /* Set tap/trim values for SDMMC1/3 @ <48MHz here */
> - val = readl(&priv->reg->venspictl); /* aka VENDOR_SYS_SW_CNTL */
> - val &= IO_TRIM_BYPASS_MASK;
> - if (id == PERIPH_ID_SDMMC1) {
> - tap_value = 4; /* default */
> - if (val)
> - tap_value = 3;
> - trim_value = 2;
> - } else { /* SDMMC3 */
> - tap_value = 3;
> - trim_value = 3;
> - }
> -
> - val = readl(&priv->reg->venclkctl);
> - val &= ~TRIM_VAL_MASK;
> - val |= (trim_value << TRIM_VAL_SHIFT);
> - val &= ~TAP_VAL_MASK;
> - val |= (tap_value << TAP_VAL_SHIFT);
> - writel(val, &priv->reg->venclkctl);
> - debug("%s: VENDOR_CLOCK_CNTRL = 0x%08X\n", __func__, val);
> -#endif /* T210 */
> #endif /* T30/T210 */
> }
>
> @@ -588,6 +566,21 @@ static void tegra_mmc_reset(struct tegra_mmc_priv *priv,
> struct mmc *mmc)
>
> /* Make sure SDIO pads are set up */
> tegra_mmc_pad_init(priv);
> +
> + if (priv->tap_value || priv->trim_value) {
I think 0 is a valid value for both tap and trim, so you want to be able
to write that. I suggest getting rid of the conditional and always
writing these values and rely on defaults to make sure that a good value
is always programmed.
Alternatively if you really only want to program these when they've been
specified, use an extra variable (or something like -1 as a default
value) to discriminate.
The former is superior, in my opinion, because it also allows to avoid
to regression.
Thierry
> + u32 val;
> +
> + val = readl(&priv->reg->venclkctl);
> +
> + val &= ~TRIM_VAL_MASK;
> + val |= (priv->trim_value << TRIM_VAL_SHIFT);
> +
> + val &= ~TAP_VAL_MASK;
> + val |= (priv->tap_value << TAP_VAL_SHIFT);
> +
> + writel(val, &priv->reg->venclkctl);
> + debug("%s: VENDOR_CLOCK_CNTRL = 0x%08X\n", __func__, val);
> + }
> }
>
> static int tegra_mmc_init(struct udevice *dev)
> @@ -742,6 +735,9 @@ static int tegra_mmc_probe(struct udevice *dev)
> if (dm_gpio_is_valid(&priv->pwr_gpio))
> dm_gpio_set_value(&priv->pwr_gpio, 1);
>
> + priv->tap_value = dev_read_u32_default(dev, "nvidia,default-tap", 0);
> + priv->trim_value = dev_read_u32_default(dev, "nvidia,default-trim", 0);
> +
> upriv->mmc = &plat->mmc;
>
> return tegra_mmc_init(dev);
> --
> 2.39.2
>
signature.asc
Description: PGP signature
