mmc: Add Marvell Xenon SDHCI controller driver
PATCH v2 3/9 mmc: Add Marvell Xenon SDHCI controller driver
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 <[email protected]> with minor changes ported from the Linux driver which is written by Ziji Hu <[email protected]>.
Signed-off-by Stefan Roese <[email protected]>
Cc Jaehoon Chung <[email protected]>
Cc Masahiro Yamada <[email protected]>
---
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
drivers/mmc/Kconfig +11 -0
@@ -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
drivers/mmc/Makefile +1 -0
@@ -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
drivers/mmc/xenon_sdhci.c new file +589 -0
589 lines not on this page · raw message
Signature · 1 line
2.11.0
PATCH v2 3/9 mmc: Add Marvell Xenon SDHCI controller driver
Hi Stefan,
Quoted 8 lines · Stefan Roese
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 <[email protected]> with minor changes ported from the Linux driver which is written by Ziji Hu <[email protected]>.
Why this is [PATCH 3/9]? Sorry. i didn't find the other patches. Could you share the information for other patches?
Quoted 55 lines
Signed-off-by Stefan Roese <[email protected]>Cc Jaehoon Chung <[email protected]>Cc Masahiro Yamada <[email protected]>---
v2:
- RenamedMMC_XENON_SDHCItoMMC_SDHCI_XENONas requested by Masahiro for the new consistant MMC namingdrivers/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.cdrivers/mmc/Kconfig +11
-0@@ -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 TEGRAdrivers/mmc/Makefile +1
-0@@ -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.odrivers/mmc/xenon_sdhci.c new file +0
-0
drivers/mmc/xenon_sdhci.c line 4
Quoted 4 lines
drivers/mmc/xenon_sdhci.c new file +4
-0@@ -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?
Quoted 25 lines
drivers/mmc/xenon_sdhci.c new file +25
-0+* +* Author: Victor Gu <[email protected]> +* Date: 2016-8-24 +* +* Included parts of the Linux driver version which was written by: +* Hu Ziji <[email protected]> +* +* Ported to from Marvell 2015.01 to mainline U-Boot 2017.01: +* Stefan Roese <[email protected]> +* +* 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
drivers/mmc/xenon_sdhci.c line 39
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+#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?
drivers/mmc/xenon_sdhci.c line 49
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-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?
drivers/mmc/xenon_sdhci.c line 51
Quoted 2 lines
drivers/mmc/xenon_sdhci.c new file +2
-0+#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?
Quoted 26 lines
drivers/mmc/xenon_sdhci.c new file +26
-0+ +#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
drivers/mmc/xenon_sdhci.c line 87
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+#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?
drivers/mmc/xenon_sdhci.c line 90
Quoted 3 lines
drivers/mmc/xenon_sdhci.c new file +3
-0+ +#define DEFAULT_SDCLK_FREQ 400000 +#define LOWEST_SDCLK_FREQ 100000
Doesn't use these values.
drivers/mmc/xenon_sdhci.c line 102
Quoted 12 lines
drivers/mmc/xenon_sdhci.c new file +12
-0+ +/* 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.
Quoted 14 lines
drivers/mmc/xenon_sdhci.c new file +14
-0+#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)
drivers/mmc/xenon_sdhci.c line 126
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+ +#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.
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+ +#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)
drivers/mmc/xenon_sdhci.c line 146
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+#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?
Quoted 7 lines
drivers/mmc/xenon_sdhci.c new file +7
-0+ +#define SDHCI_RETUNE_EVT_INTSIG 0x00001000 + +/* Hyperion only have one slot 0 */ +#define XENON_MMC_SLOT_ID_HYPERION 0 + +#define MMC_TIMING_LEGACY 0
drivers/mmc/xenon_sdhci.c line 163
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-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.
Quoted 21 lines
drivers/mmc/xenon_sdhci.c new file +21
-0+ +#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;
drivers/mmc/xenon_sdhci.c line 194
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+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?
Quoted 49 lines
drivers/mmc/xenon_sdhci.c new file +49
-0+ +/* 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 ;
drivers/mmc/xenon_sdhci.c line 253
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+ 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?
Quoted 107 lines
drivers/mmc/xenon_sdhci.c new file +107
-0+ +/* 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 ); +} +
drivers/mmc/xenon_sdhci.c line 370
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+/* 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?
Quoted 4 lines
drivers/mmc/xenon_sdhci.c new file +4
-0+sdhci_writel (host, var,SDHC_SYS_OP_CTRL ); +} + +/* Enable Parallel Transfer Mode */
drivers/mmc/xenon_sdhci.c line 384
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+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.
Quoted 60 lines
drivers/mmc/xenon_sdhci.c new file +60
-0+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 ;
drivers/mmc/xenon_sdhci.c line 454
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+ } + }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.
drivers/mmc/xenon_sdhci.c line 462
Quoted 8 lines
drivers/mmc/xenon_sdhci.c new file +8
-0+ +/* 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?
Quoted 30 lines
drivers/mmc/xenon_sdhci.c new file +30
-0+}; + +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)
drivers/mmc/xenon_sdhci.c line 502
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+*/ +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.
Quoted 16 lines
drivers/mmc/xenon_sdhci.c new file +16
-0+ +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;
drivers/mmc/xenon_sdhci.c line 528
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+ + 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".
drivers/mmc/xenon_sdhci.c line 533
Quoted 5 lines
drivers/mmc/xenon_sdhci.c new file +5
-0+ +/* 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.
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+ +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; +
drivers/mmc/xenon_sdhci.c line 553
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+ 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
Quoted 37 lines
drivers/mmc/xenon_sdhci.c new file +36
-0+ 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 ), +};
PATCH v2 3/9 mmc: Add Marvell Xenon SDHCI controller driver
Hi Jaehoon,
Quoted 5 lines · Jaehoon Chung
Quoted 4 lines · Stefan Roese
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.
Quoted 7 lines
Quoted 4 lines
History:
This driver is ported from the Marvell U-Boot version 2015.01 which is written by Victor Gu <[email protected]> with minor changes ported from the Linux driver which is written by Ziji Hu <[email protected]>.Why this is [PATCH 3/9]? Sorry. i didn't find the other patches. Could you share the information for other patches?
This is v2 of this patchset. Since only this patch 3/9 has some changes in v2, I didn't send the complete patchset again. Here the links to the other patches. You are cc'ed for patches 1...3 as they change code in the mmc directory:
http://patchwork.ozlabs.org/patch/716972/
http://patchwork.ozlabs.org/patch/716970/
http://patchwork.ozlabs.org/patch/716967/
http://patchwork.ozlabs.org/patch/716969/
http://patchwork.ozlabs.org/patch/716965/
http://patchwork.ozlabs.org/patch/716964/
http://patchwork.ozlabs.org/patch/716968/
http://patchwork.ozlabs.org/patch/716968/
Quoted 47 lines
Quoted 47 lines
Signed-off-by Stefan Roese <[email protected]>Cc Jaehoon Chung <[email protected]>Cc Masahiro Yamada <[email protected]>---
v2:
- RenamedMMC_XENON_SDHCItoMMC_SDHCI_XENONas requested by Masahiro for the new consistant MMC namingdrivers/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.cdrivers/mmc/Kconfig +11
-0@@ -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 TEGRAdrivers/mmc/Makefile +1
-0@@ -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
Quoted 14 lines
drivers/mmc/xenon_sdhci.c line 4
Quoted 12 lines
drivers/mmc/Makefile +0
-0obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o obj-$(CONFIG_MMC_UNIPHIER) += uniphier-sd.odrivers/mmc/xenon_sdhci.c new file +4
-0@@ -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?
Thats the copyright notice from the Marvell code. Marvell might change this if appropriate, but for now I would like to keep it.
Quoted 29 lines
Quoted 29 lines
drivers/mmc/xenon_sdhci.c new file +29
-0+* +* Author: Victor Gu <[email protected]> +* Date: 2016-8-24 +* +* Included parts of the Linux driver version which was written by: +* Hu Ziji <[email protected]> +* +* Ported to from Marvell 2015.01 to mainline U-Boot 2017.01: +* Stefan Roese <[email protected]> +* +* 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
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 39
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+#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 0Does it need to shift with 0?
Its not needed of course, but it makes to code more consistent for my taste. And please note that the Linux driver also uses these shifts, so we are more in-line with the Linux driver by using it this way.
Quoted 4 lines
Quoted 4 lines
drivers/mmc/xenon_sdhci.c new file +4
-0+ +#define SDHC_SYS_EXT_OP_CTRL 0x010C +#define MASK_CMD_CONFLICT_ERROR BIT(8) +
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 49
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+#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?
Not used in this driver currently. I'll remove it in v3.
Quoted 4 lines
drivers/mmc/xenon_sdhci.c line 51
Quoted 2 lines
drivers/mmc/xenon_sdhci.c new file +2
-0+#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?
Removed in v3.
Quoted 30 lines
Quoted 30 lines
drivers/mmc/xenon_sdhci.c new file +30
-0+ +#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 +
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 87
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+/* Tuning Parameter */ +#define TMR_RETUN_NO_PRESENT 0xf +#define XENON_MAX_TUN_COUNT 0xb +#define DEF_TUNING_COUNT 0x9 + +#define MMC_TIMING_FAKE 0xFFWhat is fake?
Not used, to removed in v3.
Quoted 5 lines
drivers/mmc/xenon_sdhci.c line 90
Quoted 3 lines
drivers/mmc/xenon_sdhci.c new file +3
-0+ +#define DEFAULT_SDCLK_FREQ 400000 +#define LOWEST_SDCLK_FREQ 100000Doesn't use these values.
Yes, thanks. Will remove.
Quoted 7 lines
Quoted 7 lines
drivers/mmc/xenon_sdhci.c new file +7
-0+ +/* 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
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 102
Quoted 5 lines
drivers/mmc/xenon_sdhci.c new file +5
-0+#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 theBIT()API about other bit operation.
Will do.
Quoted 18 lines
Quoted 18 lines
drivers/mmc/xenon_sdhci.c new file +18
-0+#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
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 126
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+#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.
Yes.
Quoted 34 lines
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+ +#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)drivers/mmc/xenon_sdhci.c line 146
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+#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 0x5a54also doesn't use this. why put this?
Quoted 11 lines
drivers/mmc/xenon_sdhci.c new file +11
-0+ +#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
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 163
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+#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 10Why do you need to this? even HS200/400 don't support on u-boot.
Its taken from the Linux driver. As you might know, the SDHCI core saves the current timing in this "ios->timing" variable. I was trying to emulate this in the Xenon driver so long as the U-Boot infrastructure does not implement it this way. I could remove the currently unused timing defines for now but would prefer to keep them.
Quoted 25 lines
Quoted 25 lines
drivers/mmc/xenon_sdhci.c new file +25
-0+ +#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)
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 194
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+{ +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?
I personally prefer this coding style, one variable in one line. So I would like to keep this.
Quoted 53 lines
Quoted 53 lines
drivers/mmc/xenon_sdhci.c new file +53
-0+ +/* 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 */
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 253
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+ clock =100000 ; + } + +/* Get the wait time in unit of ms */ + wait = wait / clock; + wait++;Where does "wait" variable use?
Good catch, thanks. Something that got mixed up while rebasing. Will get fixed in v3.
Quoted 111 lines
Quoted 111 lines
drivers/mmc/xenon_sdhci.c new file +111
-0+ +/* 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;
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 370
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+ + 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?
Its "(0x1 << slot)" masks the current slot. I can change this to use a macro, like:
#define SLOT_MASK(slot) (1 << slot)
var |= SLOT_MASK(slot) << SLOT_ENABLE_SHIFT;
Would you prefer this?
Quoted 82 lines
Quoted 4 lines
drivers/mmc/xenon_sdhci.c new file +4
-0+sdhci_writel (host, var,SDHC_SYS_OP_CTRL ); +} + +/* Enable Parallel Transfer Mode */ drivers/mmc/xenon_sdhci.c line 384
Quoted 10 lines
drivers/mmc/xenon_sdhci.c new file +10
-0+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.
Quoted 65 lines
drivers/mmc/xenon_sdhci.c new file +65
-0+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 ;
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 454
Quoted 5 lines
drivers/mmc/xenon_sdhci.c new file +5
-0+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.
As mentioned above, this driver is partly based on the Linux driver and this one uses this timing variable. Please correct me if I am wrong, but AFAICT we currently have no such "timing" variable in the core U-Boot SDHCI code.
Quoted 10 lines
drivers/mmc/xenon_sdhci.c line 462
Quoted 8 lines
drivers/mmc/xenon_sdhci.c new file +8
-0+ +/* 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_postWhere is ".set_ios_post" define in
sdhci_opsstructure?
Please see patch 2/9:
http://patchwork.ozlabs.org/patch/716970/
Quoted 34 lines
Quoted 34 lines
drivers/mmc/xenon_sdhci.c new file +34
-0+}; + +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); +
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 502
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+ 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_capsis assigned to some value.
I'll take a look at it.
Quoted 21 lines
Quoted 21 lines
drivers/mmc/xenon_sdhci.c new file +21
-0+ +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; +
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 528
Quoted 5 lines
drivers/mmc/xenon_sdhci.c new file +5
-0+/* 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".
Will remove this parameter.
Quoted 7 lines
drivers/mmc/xenon_sdhci.c line 533
Quoted 5 lines
drivers/mmc/xenon_sdhci.c new file +5
-0+ +/* 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.
Copied from the Linux driver. So it more in-line with this driver this way.
Quoted 14 lines
Quoted 14 lines
drivers/mmc/xenon_sdhci.c new file +14
-0+ +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" ))
Quoted 8 lines
drivers/mmc/xenon_sdhci.c line 553
Quoted 6 lines
drivers/mmc/xenon_sdhci.c new file +6
-0+ 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.
Will do.
Thanks for the review,
Stefan