Re: [PATCH v1 1/1] mmc: tegra: get default-tap and default-trim from device tree

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 clamor95@gmail.com # ASUS TF701T Signed-off-by: Svyatoslav Ryhel clamor95@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