Hi Stefan,
On 01/21/2017 06:20 PM, Stefan Roese wrote:
> This driver implementes platform specific code for the Xenon SDHCI
> controller which is integrated in the Marvell MVEBU Armada 37xx and
> Armada 7k / 8K SoCs.
>
> History:
> This driver is ported from the Marvell U-Boot version 2015.01 which is
> written by Victor Gu <x...@marvell.com> with minor changes ported from
> the Linux driver which is written by Ziji Hu <huz...@marvell.com>.
Why this is [PATCH 3/9]? Sorry. i didn't find the other patches.
Could you share the information for other patches?
>
> Signed-off-by: Stefan Roese <s...@denx.de>
> Cc: Jaehoon Chung <jh80.ch...@samsung.com>
> Cc: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
> v2:
> - Renamed MMC_XENON_SDHCI to MMC_SDHCI_XENON as requested by Masahiro
> for the new consistant MMC naming
>
> drivers/mmc/Kconfig | 11 +
> drivers/mmc/Makefile | 1 +
> drivers/mmc/xenon_sdhci.c | 589
> ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 601 insertions(+)
> create mode 100644 drivers/mmc/xenon_sdhci.c
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 9ed8da39ef..147e52d332 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -287,6 +287,17 @@ config MMC_SDHCI_SPEAR
>
> If unsure, say N.
>
> +config MMC_SDHCI_XENON
> + bool "SDHCI support for the Xenon SDHCI controller"
> + depends on MMC_SDHCI && DM_MMC && OF_CONTROL
> + help
> + Support for Xenon SDHCI host controller on Marvell Armada 3700
> + 7k/8k ARM SoCs platforms
> +
> + If you have a controller with this interface, say Y here.
> +
> + If unsure, say N.
> +
> config MMC_SDHCI_TEGRA
> bool "SDHCI platform support for the Tegra SD/MMC Controller"
> depends on TEGRA
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 4dca09c955..6af7f79ff8 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -68,6 +68,7 @@ obj-$(CONFIG_MMC_SDHCI_MV) += mv_sdhci.o
> obj-$(CONFIG_MMC_SDHCI_S5P) += s5p_sdhci.o
> obj-$(CONFIG_MMC_SDHCI_SPEAR) += spear_sdhci.o
> obj-$(CONFIG_MMC_SDHCI_TEGRA) += tegra_mmc.o
> +obj-$(CONFIG_MMC_SDHCI_XENON) += xenon_sdhci.o
>
> obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
> obj-$(CONFIG_MMC_UNIPHIER) += uniphier-sd.o
> diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c
> new file mode 100644
> index 0000000000..f36b482288
> --- /dev/null
> +++ b/drivers/mmc/xenon_sdhci.c
> @@ -0,0 +1,589 @@
> +/*
> + * Driver for Marvell SOC Platform Group Xenon SDHC as a platform device
> + *
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
2016-2017?
> + *
> + * Author: Victor Gu <x...@marvell.com>
> + * Date: 2016-8-24
> + *
> + * Included parts of the Linux driver version which was written by:
> + * Hu Ziji <huz...@marvell.com>
> + *
> + * Ported to from Marvell 2015.01 to mainline U-Boot 2017.01:
> + * Stefan Roese <s...@denx.de>
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <libfdt.h>
> +#include <malloc.h>
> +#include <sdhci.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* Register Offset of SD Host Controller SOCP self-defined register */
> +#define SDHC_SYS_CFG_INFO 0x0104
> +#define SLOT_TYPE_SDIO_SHIFT 24
> +#define SLOT_TYPE_EMMC_MASK 0xFF
> +#define SLOT_TYPE_EMMC_SHIFT 16
> +#define SLOT_TYPE_SD_SDIO_MMC_MASK 0xFF
> +#define SLOT_TYPE_SD_SDIO_MMC_SHIFT 8
> +#define NR_SUPPORTED_SLOT_MASK 0x7
> +
> +#define SDHC_SYS_OP_CTRL 0x0108
> +#define AUTO_CLKGATE_DISABLE_MASK BIT(20)
> +#define SDCLK_IDLEOFF_ENABLE_SHIFT 8
> +#define SLOT_ENABLE_SHIFT 0
Does it need to shift with 0?
> +
> +#define SDHC_SYS_EXT_OP_CTRL 0x010C
> +#define MASK_CMD_CONFLICT_ERROR BIT(8)
> +
> +#define SDHC_SLOT_OP_STATUS_CTRL 0x0128
> +#define DELAY_90_DEGREE_MASK_EMMC5 BIT(7)
> +#define DELAY_90_DEGREE_SHIFT_EMMC5 7
> +#define EMMC_5_0_PHY_FIXED_DELAY_MASK 0x7F
> +#define EMMC_PHY_FIXED_DELAY_MASK 0xFF
> +#define EMMC_PHY_FIXED_DELAY_WINDOW_MIN
> (EMMC_PHY_FIXED_DELAY_MASK >> 3)
It's strange...0xFF >> 3? is it 0x1F?
> +#define SDH_PHY_FIXED_DELAY_MASK 0x1FF
> +#define SDH_PHY_FIXED_DELAY_WINDOW_MIN
> (SDH_PHY_FIXED_DELAY_MASK >> 4)
Ditto. why shift the 0x1FF?
> +
> +#define TUN_CONSECUTIVE_TIMES_SHIFT 16
> +#define TUN_CONSECUTIVE_TIMES_MASK 0x7
> +#define TUN_CONSECUTIVE_TIMES 0x4
> +#define TUNING_STEP_SHIFT 12
> +#define TUNING_STEP_MASK 0xF
> +#define TUNING_STEP_DIVIDER BIT(6)
> +
> +#define FORCE_SEL_INVERSE_CLK_SHIFT 11
> +
> +#define SDHC_SLOT_FIFO_CTRL 0x012c
> +
> +#define SDHC_SLOT_EMMC_CTRL 0x0130
> +#define ENABLE_DATA_STROBE BIT(24)
> +#define SET_EMMC_RSTN BIT(16)
> +#define DISABLE_RD_DATA_CRC BIT(14)
> +#define DISABLE_CRC_STAT_TOKEN BIT(13)
> +#define EMMC_VCCQ_MASK 0x3
> +#define EMMC_VCCQ_1_8V 0x1
> +#define EMMC_VCCQ_3_3V 0x3
> +
> +#define SDHC_SLOT_RETUNING_REQ_CTRL 0x0144
> +/* retuning compatible */
> +#define RETUNING_COMPATIBLE 0x1
> +
> +#define SDHC_SLOT_EXT_PRESENT_STATE 0x014C
> +#define LOCK_STATE 0x1
> +
> +#define SDHC_SLOT_DLL_CUR_DLY_VAL 0x0150
> +
> +/* Tuning Parameter */
> +#define TMR_RETUN_NO_PRESENT 0xf
> +#define XENON_MAX_TUN_COUNT 0xb
> +#define DEF_TUNING_COUNT 0x9
> +
> +#define MMC_TIMING_FAKE 0xFF
What is fake?
> +
> +#define DEFAULT_SDCLK_FREQ 400000
> +#define LOWEST_SDCLK_FREQ 100000
Doesn't use these values.
> +
> +/* Xenon specific Mode Select value */
> +#define XENON_SDHCI_CTRL_HS200 0x5
> +#define XENON_SDHCI_CTRL_HS400 0x6
> +
> +#define EMMC_PHY_REG_BASE 0x170
> +#define EMMC_PHY_TIMING_ADJUST EMMC_PHY_REG_BASE
> +#define OUTPUT_QSN_PHASE_SELECT (1 << 17)
> +#define SAMPL_INV_QSP_PHASE_SELECT (1 << 18)
> +#define SAMPL_INV_QSP_PHASE_SELECT_SHIFT 18
> +#define EMMC_PHY_SLOW_MODE (1 << 29)
> +#define PHY_INITIALIZAION (1 << 31)
You're using BIT() about some bit operation..but some values don't use it.
Use the BIT() API about other bit operation.
> +#define WAIT_CYCLE_BEFORE_USING_MASK 0xf
> +#define WAIT_CYCLE_BEFORE_USING_SHIFT 12
> +#define FC_SYNC_EN_DURATION_MASK 0xf
> +#define FC_SYNC_EN_DURATION_SHIFT 8
> +#define FC_SYNC_RST_EN_DURATION_MASK 0xf
> +#define FC_SYNC_RST_EN_DURATION_SHIFT 4
> +#define FC_SYNC_RST_DURATION_MASK 0xf
> +#define FC_SYNC_RST_DURATION_SHIFT 0
> +
> +#define EMMC_PHY_FUNC_CONTROL (EMMC_PHY_REG_BASE +
> 0x4)
> +#define DQ_ASYNC_MODE (1 << 4)
> +#define DQ_DDR_MODE_SHIFT 8
> +#define DQ_DDR_MODE_MASK 0xff
> +#define CMD_DDR_MODE (1 << 16)
> +
> +#define EMMC_PHY_PAD_CONTROL (EMMC_PHY_REG_BASE + 0x8)
> +#define REC_EN_SHIFT 24
> +#define REC_EN_MASK 0xf
> +#define FC_DQ_RECEN (1 << 24)
> +#define FC_CMD_RECEN (1 << 25)
> +#define FC_QSP_RECEN (1 << 26)
> +#define FC_QSN_RECEN (1 << 27)
> +#define OEN_QSN (1 << 28)
> +#define AUTO_RECEN_CTRL (1 << 30)
Use BIT() API.
> +
> +#define EMMC_PHY_PAD_CONTROL1 (EMMC_PHY_REG_BASE +
> 0xc)
> +#define EMMC5_1_FC_QSP_PD BIT(9)
> +#define EMMC5_1_FC_QSP_PU BIT(25)
> +#define EMMC5_1_FC_CMD_PD BIT(8)
> +#define EMMC5_1_FC_CMD_PU BIT(24)
> +#define EMMC5_1_FC_DQ_PD 0xff
> +#define EMMC5_1_FC_DQ_PU (0xff << 16)
> +
> +#define EMMC_PHY_PAD_CONTROL2 (EMMC_PHY_REG_BASE +
> 0x10)
> +#define EMMC_PHY_DLL_CONTROL (EMMC_PHY_REG_BASE + 0x14)
> +#define DLL_DELAY_TEST_LOWER_SHIFT 8
> +#define DLL_DELAY_TEST_LOWER_MASK 0xff
> +#define DLL_BYPASS_EN 0x1
> +
> +#define EMMC_LOGIC_TIMING_ADJUST (EMMC_PHY_REG_BASE + 0x18)
> +#define EMMC_LOGIC_TIMING_ADJUST_LOW (EMMC_PHY_REG_BASE + 0x1c)
> +
> +/* Recommend by HW team */
> +#define LOGIC_TIMING_VALUE 0x5a54
also doesn't use this. why put this?
> +
> +#define SDHCI_RETUNE_EVT_INTSIG 0x00001000
> +
> +/* Hyperion only have one slot 0 */
> +#define XENON_MMC_SLOT_ID_HYPERION 0
> +
> +#define MMC_TIMING_LEGACY 0
> +#define MMC_TIMING_MMC_HS 1
> +#define MMC_TIMING_SD_HS 2
> +#define MMC_TIMING_UHS_SDR12 3
> +#define MMC_TIMING_UHS_SDR25 4
> +#define MMC_TIMING_UHS_SDR50 5
> +#define MMC_TIMING_UHS_SDR104 6
> +#define MMC_TIMING_UHS_DDR50 7
> +#define MMC_TIMING_MMC_DDR52 8
> +#define MMC_TIMING_MMC_HS200 9
> +#define MMC_TIMING_MMC_HS400 10
Why do you need to this? even HS200/400 don't support on u-boot.
> +
> +#define XENON_MMC_MAX_CLK 400000000
> +
> +enum soc_pad_ctrl_type {
> + SOC_PAD_SD,
> + SOC_PAD_FIXED_1_8V,
> +};
> +
> +struct xenon_sdhci_plat {
> + struct mmc_config cfg;
> + struct mmc mmc;
> +};
> +
> +struct xenon_sdhci_priv {
> + struct sdhci_host host;
> +
> + u8 timing;
> +
> + unsigned int clock;
> +
> + void *pad_ctrl_reg;
> + int pad_type;
> +};
> +
> +static int xenon_mmc_phy_init(struct sdhci_host *host)
> +{
> + struct xenon_sdhci_priv *priv = host->mmc->priv;
> + u32 clock = priv->clock;
> + u32 wait;
> + u32 time;
> + u32 var;
Make one line. u32 wait, time, var?
> +
> + /* Enable QSP PHASE SELECT */
> + var = sdhci_readl(host, EMMC_PHY_TIMING_ADJUST);
> + var |= SAMPL_INV_QSP_PHASE_SELECT;
> + if ((priv->timing == MMC_TIMING_UHS_SDR50) ||
> + (priv->timing == MMC_TIMING_UHS_SDR25) ||
> + (priv->timing == MMC_TIMING_UHS_SDR12) ||
> + (priv->timing == MMC_TIMING_SD_HS) ||
> + (priv->timing == MMC_TIMING_LEGACY))
> + var |= EMMC_PHY_SLOW_MODE;
> + sdhci_writel(host, var, EMMC_PHY_TIMING_ADJUST);
> +
> + /* Poll for host MMC PHY clock init to be stable */
> + /* Wait up to 10ms */
> + time = 100;
> + while (time--) {
> + var = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> + if (var & SDHCI_CLOCK_INT_STABLE)
> + break;
> +
> + udelay(100);
> + }
> +
> + if (time <= 0) {
> + error("Failed to enable MMC internal clock in time\n");
> + return -ETIMEDOUT;
> + }
> +
> + /* Init PHY */
> + var = sdhci_readl(host, EMMC_PHY_TIMING_ADJUST);
> + var |= PHY_INITIALIZAION;
> + sdhci_writel(host, var, EMMC_PHY_TIMING_ADJUST);
> +
> + /* Add duration of FC_SYNC_RST */
> + wait = (var >> FC_SYNC_RST_DURATION_SHIFT) & FC_SYNC_RST_DURATION_MASK;
> + /* Add interval between FC_SYNC_EN and FC_SYNC_RST */
> + wait += (var >> FC_SYNC_RST_EN_DURATION_SHIFT) &
> + FC_SYNC_RST_EN_DURATION_MASK;
> + /* Add duration of asserting FC_SYNC_EN */
> + wait += (var >> FC_SYNC_EN_DURATION_SHIFT) & FC_SYNC_EN_DURATION_MASK;
> + /* Add duration of waiting for PHY */
> + wait += (var >> WAIT_CYCLE_BEFORE_USING_SHIFT) &
> + WAIT_CYCLE_BEFORE_USING_MASK;
> + /*
> + * According to Moyang, 4 addtional bus clock and 4 AXI bus clock
> + * are required
> + */
> + /* left shift 20 bits */
> + wait += 8;
> + wait <<= 20;
> +
> + if (clock == 0) {
> + /* Use the possibly slowest bus frequency value */
> + clock = 100000;
> + }
> +
> + /* Get the wait time in unit of ms */
> + wait = wait / clock;
> + wait++;
Where does "wait" variable use?
> +
> + /* Poll for host eMMC PHY init to complete */
> + /* Wait up to 10ms */
> + time = 100;
> + while (time--) {
> + var = sdhci_readl(host, EMMC_PHY_TIMING_ADJUST);
> + var &= PHY_INITIALIZAION;
> + if (!var)
> + break;
> +
> + /* wait for host eMMC PHY init to complete */
> + udelay(100);
> + }
> +
> + if (time <= 0) {
> + error("Failed to init MMC PHY in time\n");
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +#define ARMADA_3700_SOC_PAD_1_8V 0x1
> +#define ARMADA_3700_SOC_PAD_3_3V 0x0
> +
> +static void armada_3700_soc_pad_voltage_set(struct sdhci_host *host)
> +{
> + struct xenon_sdhci_priv *priv = host->mmc->priv;
> +
> + if (priv->pad_type == SOC_PAD_FIXED_1_8V)
> + writel(ARMADA_3700_SOC_PAD_1_8V, priv->pad_ctrl_reg);
> + else if (priv->pad_type == SOC_PAD_SD)
> + writel(ARMADA_3700_SOC_PAD_3_3V, priv->pad_ctrl_reg);
> +}
> +
> +static void xenon_mmc_phy_set(struct sdhci_host *host)
> +{
> + struct xenon_sdhci_priv *priv = host->mmc->priv;
> + u32 var;
> +
> + /* Setup pad, set bit[30], bit[28] and bits[26:24] */
> + var = sdhci_readl(host, EMMC_PHY_PAD_CONTROL);
> + var |= AUTO_RECEN_CTRL | OEN_QSN | FC_QSP_RECEN |
> + FC_CMD_RECEN | FC_DQ_RECEN;
> + sdhci_writel(host, var, EMMC_PHY_PAD_CONTROL);
> +
> + /* Set CMD and DQ Pull Up */
> + var = sdhci_readl(host, EMMC_PHY_PAD_CONTROL1);
> + var |= (EMMC5_1_FC_CMD_PU | EMMC5_1_FC_DQ_PU);
> + var &= ~(EMMC5_1_FC_CMD_PD | EMMC5_1_FC_DQ_PD);
> + sdhci_writel(host, var, EMMC_PHY_PAD_CONTROL1);
> +
> + /*
> + * If timing belongs to high speed, set bit[17] of
> + * EMMC_PHY_TIMING_ADJUST register
> + */
> + if ((priv->timing == MMC_TIMING_MMC_HS400) ||
> + (priv->timing == MMC_TIMING_MMC_HS200) ||
> + (priv->timing == MMC_TIMING_UHS_SDR50) ||
> + (priv->timing == MMC_TIMING_UHS_SDR104) ||
> + (priv->timing == MMC_TIMING_UHS_DDR50) ||
> + (priv->timing == MMC_TIMING_UHS_SDR25) ||
> + (priv->timing == MMC_TIMING_MMC_DDR52)) {
> + var = sdhci_readl(host, EMMC_PHY_TIMING_ADJUST);
> + var |= OUTPUT_QSN_PHASE_SELECT;
> + sdhci_writel(host, var, EMMC_PHY_TIMING_ADJUST);
> + }
> +
> + /*
> + * When setting EMMC_PHY_FUNC_CONTROL register,
> + * SD clock should be disabled
> + */
> + var = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> + var &= ~SDHCI_CLOCK_CARD_EN;
> + sdhci_writew(host, var, SDHCI_CLOCK_CONTROL);
> +
> + var = sdhci_readl(host, EMMC_PHY_FUNC_CONTROL);
> + if (host->mmc->ddr_mode) {
> + var |= (DQ_DDR_MODE_MASK << DQ_DDR_MODE_SHIFT) | CMD_DDR_MODE;
> + } else {
> + var &= ~((DQ_DDR_MODE_MASK << DQ_DDR_MODE_SHIFT) |
> + CMD_DDR_MODE);
> + }
> + sdhci_writel(host, var, EMMC_PHY_FUNC_CONTROL);
> +
> + /* Enable bus clock */
> + var = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> + var |= SDHCI_CLOCK_CARD_EN;
> + sdhci_writew(host, var, SDHCI_CLOCK_CONTROL);
> +
> + xenon_mmc_phy_init(host);
> +}
> +
> +/* Enable/Disable the Auto Clock Gating function of this slot */
> +static void xenon_mmc_set_acg(struct sdhci_host *host, bool enable)
> +{
> + u32 var;
> +
> + var = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + if (enable)
> + var &= ~AUTO_CLKGATE_DISABLE_MASK;
> + else
> + var |= AUTO_CLKGATE_DISABLE_MASK;
> +
> + sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable specific slot */
> +static void xenon_mmc_set_slot(struct sdhci_host *host, u8 slot, bool enable)
> +{
> + u32 var;
> +
> + var = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + if (enable)
> + var |= (0x1 << slot) << SLOT_ENABLE_SHIFT;
> + else
> + var &= ~((0x1 << slot) << SLOT_ENABLE_SHIFT);
What is 0x1?
> + sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable Parallel Transfer Mode */
> +static void xenon_mmc_set_parallel_tran(struct sdhci_host *host, u8 slot,
> + bool enable)
> +{
> + u32 var;
> +
> + var = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
> + if (enable)
> + var |= (0x1 << slot);
> + else
> + var &= ~(0x1 << slot);
Ditto.
> + sdhci_writel(host, var, SDHC_SYS_EXT_OP_CTRL);
> +}
> +
> +static void xenon_mmc_set_tuning(struct sdhci_host *host, u8 slot, bool
> enable)
> +{
> + u32 var;
> +
> + /* Set the Re-Tuning Request functionality */
> + var = sdhci_readl(host, SDHC_SLOT_RETUNING_REQ_CTRL);
> + if (enable)
> + var |= RETUNING_COMPATIBLE;
> + else
> + var &= ~RETUNING_COMPATIBLE;
> + sdhci_writel(host, var, SDHC_SLOT_RETUNING_REQ_CTRL);
> +
> + /* Set the Re-tuning Event Signal Enable */
> + var = sdhci_readl(host, SDHCI_SIGNAL_ENABLE);
> + if (enable)
> + var |= SDHCI_RETUNE_EVT_INTSIG;
> + else
> + var &= ~SDHCI_RETUNE_EVT_INTSIG;
> + sdhci_writel(host, var, SDHCI_SIGNAL_ENABLE);
> +}
> +
> +/* Mask command conflict error */
> +static void xenon_mask_cmd_conflict_err(struct sdhci_host *host)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
> + reg |= MASK_CMD_CONFLICT_ERROR;
> + sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
> +}
> +
> +/* Platform specific function for post set_ios configuration */
> +static void xenon_sdhci_set_ios_post(struct sdhci_host *host)
> +{
> + struct xenon_sdhci_priv *priv = host->mmc->priv;
> + uint speed = host->mmc->tran_speed;
> + int pwr_18v = 0;
> +
> + if ((sdhci_readb(host, SDHCI_POWER_CONTROL) & ~SDHCI_POWER_ON) ==
> + SDHCI_POWER_180)
> + pwr_18v = 1;
> +
> + /* Set timing variable according to the configured speed */
> + if (IS_SD(host->mmc)) {
> + /* SD/SDIO */
> + if (pwr_18v) {
> + if (host->mmc->ddr_mode)
> + priv->timing = MMC_TIMING_UHS_DDR50;
> + else if (speed <= 25000000)
> + priv->timing = MMC_TIMING_UHS_SDR25;
> + else
> + priv->timing = MMC_TIMING_UHS_SDR50;
> + } else {
> + if (speed <= 25000000)
> + priv->timing = MMC_TIMING_LEGACY;
> + else
> + priv->timing = MMC_TIMING_SD_HS;
> + }
> + } else {
> + /* eMMC */
> + if (host->mmc->ddr_mode)
> + priv->timing = MMC_TIMING_MMC_DDR52;
> + else if (speed <= 26000000)
> + priv->timing = MMC_TIMING_LEGACY;
> + else
> + priv->timing = MMC_TIMING_MMC_HS;
> + }
Why do you assign to timing in host controller?
MMC Timing is already assigned in mmc core.
> +
> + /* Re-init the PHY */
> + xenon_mmc_phy_set(host);
> +}
> +
> +/* Install a driver specific handler for post set_ios configuration */
> +static const struct sdhci_ops xenon_sdhci_ops = {
> + .set_ios_post = xenon_sdhci_set_ios_post
Where is ".set_ios_post" define in sdhci_ops structure?
> +};
> +
> +static int xenon_sdhci_probe(struct udevice *dev)
> +{
> + struct xenon_sdhci_plat *plat = dev_get_platdata(dev);
> + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> + struct xenon_sdhci_priv *priv = dev_get_priv(dev);
> + struct sdhci_host *host = dev_get_priv(dev);
> + int ret;
> +
> + host->mmc = &plat->mmc;
> + host->mmc->priv = host;
> + host->mmc->dev = dev;
> + upriv->mmc = host->mmc;
> +
> + /* Set quirks */
> + host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_32BIT_DMA_ADDR;
> +
> + /* Set default timing */
> + priv->timing = MMC_TIMING_LEGACY;
> +
> + /* Disable auto clock gating during init */
> + xenon_mmc_set_acg(host, false);
> +
> + /* Enable slot */
> + xenon_mmc_set_slot(host, XENON_MMC_SLOT_ID_HYPERION, true);
> +
> + /*
> + * Set default power on SoC PHY PAD register (currently only
> + * available on the Armada 3700)
> + */
> + if (priv->pad_ctrl_reg)
> + armada_3700_soc_pad_voltage_set(host);
> +
> + ret = sdhci_setup_cfg(&plat->cfg, host, XENON_MMC_MAX_CLK, 0);
> + if (ret)
> + return ret;
> +
> + plat->cfg.host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz |
> + MMC_MODE_DDR_52MHz;
In sdhci_setup_cfg(), host_caps is assigned to some value.
> +
> + switch (fdtdec_get_int(gd->fdt_blob, dev->of_offset, "bus-width", 1)) {
> + case 8:
> + plat->cfg.host_caps |= MMC_MODE_8BIT;
> + break;
> + case 4:
> + plat->cfg.host_caps |= MMC_MODE_4BIT;
> + break;
> + case 1:
> + break;
> + default:
> + printf("Invalid \"bus-width\" value\n");
> + return -EINVAL;
> + }
> +
> + host->ops = &xenon_sdhci_ops;
> +
> + ret = sdhci_probe(dev);
> + if (ret)
> + return ret;
> +
> + /* Enable parallel transfer */
> + xenon_mmc_set_parallel_tran(host, XENON_MMC_SLOT_ID_HYPERION, true);
> +
> + /* Disable tuning functionality of this slot */
> + xenon_mmc_set_tuning(host, XENON_MMC_SLOT_ID_HYPERION, false);
Does it need to pass "false"? I didn't see where xenon_mmc_set_tuning() called
with "true".
> +
> + /* Enable auto clock gating after init */
> + xenon_mmc_set_acg(host, true);
> +
> + xenon_mask_cmd_conflict_err(host);
Why put this function? this function didn't resue anywhere.
> +
> + return ret;
> +}
> +
> +static int xenon_sdhci_ofdata_to_platdata(struct udevice *dev)
> +{
> + struct sdhci_host *host = dev_get_priv(dev);
> + struct xenon_sdhci_priv *priv = dev_get_priv(dev);
> + const char *name;
> +
> + host->name = dev->name;
> + host->ioaddr = (void *)dev_get_addr(dev);
> +
> + if (of_device_is_compatible(dev, "marvell,armada-3700-sdhci"))
> + priv->pad_ctrl_reg = (void *)dev_get_addr_index(dev, 1);
> +
> + name = fdt_getprop(gd->fdt_blob, dev->of_offset, "marvell,pad-type",
> + NULL);
> + if (name) {
> + if (0 == strcmp(name, "sd")) {
use strncmp instead of strcmp.
Best Regards,
Jaehoon Chung
> + priv->pad_type = SOC_PAD_SD;
> + } else if (0 == strcmp(name, "fixed-1-8v")) {
> + priv->pad_type = SOC_PAD_FIXED_1_8V;
> + } else {
> + printf("Unsupported SOC PHY PAD ctrl type %s\n", name);
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int xenon_sdhci_bind(struct udevice *dev)
> +{
> + struct xenon_sdhci_plat *plat = dev_get_platdata(dev);
> +
> + return sdhci_bind(dev, &plat->mmc, &plat->cfg);
> +}
> +
> +static const struct udevice_id xenon_sdhci_ids[] = {
> + { .compatible = "marvell,armada-8k-sdhci",},
> + { .compatible = "marvell,armada-3700-sdhci",},
> + { }
> +};
> +
> +U_BOOT_DRIVER(xenon_sdhci_drv) = {
> + .name = "xenon_sdhci",
> + .id = UCLASS_MMC,
> + .of_match = xenon_sdhci_ids,
> + .ofdata_to_platdata = xenon_sdhci_ofdata_to_platdata,
> + .ops = &sdhci_ops,
> + .bind = xenon_sdhci_bind,
> + .probe = xenon_sdhci_probe,
> + .priv_auto_alloc_size = sizeof(struct xenon_sdhci_priv),
> + .platdata_auto_alloc_size = sizeof(struct xenon_sdhci_plat),
> +};
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
