On Wed, Aug 23, 2023 at 03:02:42PM +0300, Svyatoslav Ryhel wrote: > > > 23 серпня 2023 р. 14:03:25 GMT+03:00, Thierry Reding > <thierry.red...@gmail.com> написав(-ла): > >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. > > I may propose a compromise. Do not set default value at all and when time > comes just check if tap or trim is not error.
Yes, that's essentially a variant of the second option and it should work. Thierry
signature.asc
Description: PGP signature