[U-Boot] [PATCH 3/9 v2] mmc: Add Marvell Xenon SDHCI controller driver

Jaehoon Chung jh80.chung at samsung.com
Mon Jan 23 04:52:41 CET 2017


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 <xigu at marvell.com> with minor changes ported from
> the Linux driver which is written by Ziji Hu <huziji at 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 <sr at denx.de>
> Cc: Jaehoon Chung <jh80.chung at samsung.com>
> Cc: Masahiro Yamada <yamada.masahiro at 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 <xigu at marvell.com>
> + * Date:	2016-8-24
> + *
> + * Included parts of the Linux driver version which was written by:
> + * Hu Ziji <huziji at marvell.com>
> + *
> + * Ported to from Marvell 2015.01 to mainline U-Boot 2017.01:
> + * Stefan Roese <sr at 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),
> +};
> 



More information about the U-Boot mailing list