[PATCH 1/2] mmc: t210: Add autocal and tap/trim updates for SDMMC1/3

Tom Warren TWarren at nvidia.com
Wed Mar 18 17:28:30 CET 2020


[Tom] see below

-----Original Message-----
From: Jaehoon Chung <jh80.chung at samsung.com> 
Sent: Tuesday, March 17, 2020 4:48 PM
To: Tom Warren <TWarren at nvidia.com>; u-boot at lists.denx.de
Subject: Re: [PATCH 1/2] mmc: t210: Add autocal and tap/trim updates for SDMMC1/3

External email: Use caution opening links or attachments


On 3/17/20 6:59 AM, twarren at nvidia.com wrote:
> From: Tom Warren <twarren at nvidia.com>
>
> As per the T210 TRM, when running at 3.3v, the SDMMC1 tap/trim and 
> autocal values need to be set to condition the signals correctly 
> before talking to the SD-card. This is the same as what's being done 
> in CBoot, but it gets reset when the SDMMC1 HW is soft-reset during SD 
> driver init, so needs to be repeated here. Also set autocal and 
> tap/trim for SDMMC3, although no T210 boards use it for SD-card at this time.
>
> Signed-off-by: Tom Warren <twarren at nvidia.com>
> ---
>  arch/arm/include/asm/arch-tegra/tegra_mmc.h | 20 +++++--
>  drivers/mmc/tegra_mmc.c                     | 84 ++++++++++++++++++++++++++---
>  2 files changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-tegra/tegra_mmc.h 
> b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> index a2b6f63..a8bfa46 100644
> --- a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> +++ b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
> @@ -2,7 +2,7 @@
>  /*
>   * (C) Copyright 2009 SAMSUNG Electronics
>   * Minkyu Kang <mk7.kang at samsung.com>
> - * Portions Copyright (C) 2011-2012 NVIDIA Corporation
> + * Portions Copyright (C) 2011-2012,2019 NVIDIA Corporation
>   */
>
>  #ifndef __TEGRA_MMC_H_
> @@ -52,7 +52,7 @@ struct tegra_mmc {
>       unsigned char   admaerr;        /* offset 54h */
>       unsigned char   res4[3];        /* RESERVED, offset 55h-57h */
>       unsigned long   admaaddr;       /* offset 58h-5Fh */
> -     unsigned char   res5[0xa0];     /* RESERVED, offset 60h-FBh */
> +     unsigned char   res5[0x9c];     /* RESERVED, offset 60h-FBh */

Is comment right? "RESERVED, offset 60h-FBh"..
[Tom] yes, the comment was/is correct, it's the number of reserved bytes that was wrong - 0x9C is correct.

>       unsigned short  slotintstatus;  /* offset FCh */
>       unsigned short  hcver;          /* HOST Version */
>       unsigned int    venclkctl;      /* _VENDOR_CLOCK_CNTRL_0,    100h */
> @@ -127,11 +127,23 @@ struct tegra_mmc {
>
>  #define TEGRA_MMC_NORINTSIGEN_XFER_COMPLETE                  (1 << 1)
>
> -/* SDMMC1/3 settings from section 24.6 of T30 TRM */
> +/* SDMMC1/3 settings from SDMMCx Initialization Sequence of TRM */
>  #define MEMCOMP_PADCTRL_VREF 7
> -#define AUTO_CAL_ENABLED     (1 << 29)
> +#define AUTO_CAL_ENABLE              (1 << 29)
> +#if defined(CONFIG_TEGRA210)
> +#define AUTO_CAL_ACTIVE              (1 << 31)
> +#define AUTO_CAL_START               (1 << 31)
> +#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
>
>  #endif       /* __ASSEMBLY__ */
>  #endif       /* __TEGRA_MMC_H_ */
> diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c index 
> f022e93..a4bd679 100644
> --- a/drivers/mmc/tegra_mmc.c
> +++ b/drivers/mmc/tegra_mmc.c
> @@ -3,7 +3,7 @@
>   * (C) Copyright 2009 SAMSUNG Electronics
>   * Minkyu Kang <mk7.kang at samsung.com>
>   * Jaehoon Chung <jh80.chung at samsung.com>
> - * Portions Copyright 2011-2016 NVIDIA Corporation
> + * Portions Copyright 2011-2019 NVIDIA Corporation
>   */
>
>  #include <bouncebuf.h>
> @@ -15,6 +15,9 @@
>  #include <asm/io.h>
>  #include <asm/arch-tegra/tegra_mmc.h>  #include <linux/err.h>
> +#if defined(CONFIG_TEGRA210)
> +#include <asm/arch/clock.h>
> +#endif
>
>  struct tegra_mmc_plat {
>       struct mmc_config cfg;
> @@ -30,6 +33,7 @@ struct tegra_mmc_priv {
>       struct gpio_desc wp_gpio;       /* Write Protect GPIO */
>       unsigned int version;   /* SDHCI spec. version */
>       unsigned int clock;     /* Current clock (MHz) */
> +     int mmc_id;             /* peripheral id */
>  };
>
>  static void tegra_mmc_set_power(struct tegra_mmc_priv *priv, @@ 
> -446,16 +450,19 @@ static int tegra_mmc_set_ios(struct udevice *dev)
>
>  static void tegra_mmc_pad_init(struct tegra_mmc_priv *priv)  { -#if 
> defined(CONFIG_TEGRA30)
> +#if defined(CONFIG_TEGRA30) || defined(CONFIG_TEGRA210)
>       u32 val;
> +     u16 clk_con;
> +     int timeout;
> +     int id = priv->mmc_id;
>
> -     debug("%s: sdmmc address = %08x\n", __func__, (unsigned int)priv->reg);
> +     debug("%s: sdmmc address = %p, id = %d\n", __func__,
> +             priv->reg, id);
>
>       /* Set the pad drive strength for SDMMC1 or 3 only */
> -     if (priv->reg != (void *)0x78000000 &&
> -         priv->reg != (void *)0x78000400) {
> +     if (id != PERIPH_ID_SDMMC1 && id != PERIPH_ID_SDMMC3) {
>               debug("%s: settings are only valid for SDMMC1/SDMMC3!\n",
> -                   __func__);
> +                     __func__);
>               return;
>       }
>
> @@ -464,11 +471,65 @@ static void tegra_mmc_pad_init(struct tegra_mmc_priv *priv)
>       val |= MEMCOMP_PADCTRL_VREF;
>       writel(val, &priv->reg->sdmemcmppadctl);
>
> +     /* Disable SD Clock Enable before running auto-cal as per TRM */
> +     clk_con = readw(&priv->reg->clkcon);
> +     debug("%s: CLOCK_CONTROL = 0x%04X\n", __func__, clk_con);
> +     clk_con &= ~TEGRA_MMC_CLKCON_SD_CLOCK_ENABLE;
> +     writew(clk_con, &priv->reg->clkcon);
> +
>       val = readl(&priv->reg->autocalcfg);
>       val &= 0xFFFF0000;
> -     val |= AUTO_CAL_PU_OFFSET | AUTO_CAL_PD_OFFSET | AUTO_CAL_ENABLED;
> +     val |= AUTO_CAL_PU_OFFSET | AUTO_CAL_PD_OFFSET;
>       writel(val, &priv->reg->autocalcfg); -#endif
> +     val |= AUTO_CAL_START | AUTO_CAL_ENABLE;hen
> +     writel(val, &priv->reg->autocalcfg);

PU_OFFSET | PD_OFFSET is written to autocalcfg register.
And AUTO_CAL_START | AUTO_CAL_ENABLE is written to autocalcfg again.
Does it do different behavior when together with above values is set?
[Tom] According to the HW team, yes, you need to write the offsets, then write the START/ENABLE bits.

> +     debug("%s: AUTO_CAL_CFG = 0x%08X\n", __func__, val);
> +     udelay(1);
> +     timeout = 100;                          /* 10 mSec max (100*100uS) */
> +     do {
> +             val = readl(&priv->reg->autocalsts);
> +             udelay(100);
> +     } while ((val & AUTO_CAL_ACTIVE) && --timeout);
> +     val = readl(&priv->reg->autocalsts);

Why read value at here? for just debugging message?
[Tom] yes, AFAIR it was only for the debug printf below.

> +     debug("%s: Final AUTO_CAL_STATUS = 0x%08X, timeout = %d\n",
> +           __func__, val, timeout);
> +
> +     /* Re-enable SD Clock Enable when auto-cal is done */
> +     clk_con |= TEGRA_MMC_CLKCON_SD_CLOCK_ENABLE;
> +     writew(clk_con, &priv->reg->clkcon);
> +     clk_con = readw(&priv->reg->clkcon);
> +     debug("%s: final CLOCK_CONTROL = 0x%04X\n", __func__, clk_con);
> +
> +     if (timeout == 0) {
> +             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 */

I want not to use magic number like 4. we don't know what means 4, 3, 2...
[Tom] The X1 TRM uses these numbers, w/o much explanation, unfortunately. They're taken from the tap/trim tables in section 32.7.6 of the X1 TRM, page 2615. See also the VENDOR_CLOCK_CNTRL_0 register, section 32.8.2.

> +             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 */
>  }
>
>  static void tegra_mmc_reset(struct tegra_mmc_priv *priv, struct mmc 
> *mmc) @@ -514,6 +575,13 @@ static int tegra_mmc_init(struct udevice *dev)
>       unsigned int mask;
>       debug(" tegra_mmc_init called\n");
>
> +#if defined(CONFIG_TEGRA210)
> +     priv->mmc_id = clock_decode_periph_id(dev);
> +     if (priv->mmc_id == PERIPH_ID_NONE) {
> +             printf("%s: Missing/invalid peripheral ID\n", __func__);
> +             return -EINVAL;
> +     }
> +#endif
>       tegra_mmc_reset(priv, mmc);
>
>  #if defined(CONFIG_TEGRA124_MMC_DISABLE_EXT_LOOPBACK)
>


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------


More information about the U-Boot mailing list