[U-Boot,v2,1/1] mmc: Add MMC support for stm32h7 Socs

From: Patrice Chotard <patrice.chotard@st.com>

This patch adds SD/MMC support for STM32H7 SoCs.

Here is an extraction of SDMMC main features, embedded in
STM32H7 SoCs.
The SD/MMC block include the following:
 _ Full compliance with MultiMediaCard System Specification
   Version 4.51. Card support for three different databus modes:
   1-bit (default), 4-bit and 8-bit.
 _ Full compatibility with previous versions of MultiMediaCards
   (backward compatibility).
 _ Full compliance with SD memory card specifications version 4.1.
   (SDR104 SDMMC_CK speed limited to maximum allowed IO speed,
    SPI mode and UHS-II mode not supported).
 _ Full compliance with SDIO card specification version 4.0.
   Card support for two different databus modes: 1-bit (default)
   and 4-bit. (SDR104 SDMMC_CK speed limited to maximum allowed IO
   speed, SPI mode and UHS-II mode not supported).
 _ Data transfer up to 208 Mbyte/s for the 8 bit mode.
   (depending maximum allowed IO speed).
 _ Data and command output enable signals to control external
   bidirectional drivers.

The current version of the SDMMC supports only one SD/SDIO/MMC card
at any one time and a stack of MMC Version 4.51 or previous.

Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---

v2: _ add .get_cd() callback support

 drivers/mmc/Kconfig        |   8 +
 drivers/mmc/Makefile       |   1 +
 drivers/mmc/stm32_sdmmc2.c | 619 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 628 insertions(+)
 create mode 100644 drivers/mmc/stm32_sdmmc2.c

Hi,

On 07/19/2017 04:27 PM, patrice.chotard@st.com wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
> 
> This patch adds SD/MMC support for STM32H7 SoCs.
> 
> Here is an extraction of SDMMC main features, embedded in
> STM32H7 SoCs.
> The SD/MMC block include the following:
>  _ Full compliance with MultiMediaCard System Specification
>    Version 4.51. Card support for three different databus modes:
>    1-bit (default), 4-bit and 8-bit.
>  _ Full compatibility with previous versions of MultiMediaCards
>    (backward compatibility).
>  _ Full compliance with SD memory card specifications version 4.1.
>    (SDR104 SDMMC_CK speed limited to maximum allowed IO speed,
>     SPI mode and UHS-II mode not supported).
>  _ Full compliance with SDIO card specification version 4.0.
>    Card support for two different databus modes: 1-bit (default)
>    and 4-bit. (SDR104 SDMMC_CK speed limited to maximum allowed IO
>    speed, SPI mode and UHS-II mode not supported).
>  _ Data transfer up to 208 Mbyte/s for the 8 bit mode.
>    (depending maximum allowed IO speed).
>  _ Data and command output enable signals to control external
>    bidirectional drivers.
> 
> The current version of the SDMMC supports only one SD/SDIO/MMC card
> at any one time and a stack of MMC Version 4.51 or previous.
> 
> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
> 
> v2: _ add .get_cd() callback support
> 
>  drivers/mmc/Kconfig        |   8 +
>  drivers/mmc/Makefile       |   1 +
>  drivers/mmc/stm32_sdmmc2.c | 619 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 628 insertions(+)
>  create mode 100644 drivers/mmc/stm32_sdmmc2.c
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 82b8d75..f2e4c26 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -377,6 +377,14 @@ config GENERIC_ATMEL_MCI
>  	  the SD Memory Card Specification V2.0, the SDIO V2.0 specification
>  	  and CE-ATA V1.1.
>  
> +config STM32_SDMMC2

Why add the SDMMC2? not SDMMC?
Is there a special reason?

> +	bool "STMicroelectronics STM32H7 SD/MMC Host Controller support"
> +	depends on DM_MMC && OF_CONTROL && DM_MMC_OPS
> +	help
> +	  This selects support for the SD/MMC controller on STM32H7 SoCs.
> +	  If you have a board based on such a SoC and with a SD/MMC slot,
> +	  say Y or M here.
> +
>  endif
>  
>  config TEGRA124_MMC_DISABLE_EXT_LOOPBACK
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 2d781c3..2584663 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
>  obj-$(CONFIG_MMC_SANDBOX)		+= sandbox_mmc.o
>  obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o
>  obj-$(CONFIG_SH_SDHI) += sh_sdhi.o
> +obj-$(CONFIG_STM32_SDMMC2) += stm32_sdmmc2.o
>  
>  # SDHCI
>  obj-$(CONFIG_MMC_SDHCI)			+= sdhci.o
> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
> new file mode 100644
> index 0000000..84f96e5
> --- /dev/null
> +++ b/drivers/mmc/stm32_sdmmc2.c
> @@ -0,0 +1,619 @@
> +/*
> + *  Copyright (c) 2017 STMicrelectronics
> + *
> + * SPDX-License-Identifier:	GPL-2.0
> + */
> +
> +#include <common.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <libfdt.h>
> +#include <mmc.h>
> +#include <reset.h>
> +#include <asm/io.h>
> +#include <asm/gpio.h>
> +
> +struct stm32_sdmmc2 {
> +	u32 power;		/* SDMMC power control             [0x00] */
> +	u32 clkcr;		/* SDMMC clock control             [0x04] */
> +	u32 arg;		/* SDMMC argument                  [0x08] */
> +	u32 cmd;		/* SDMMC command                   [0x0C] */
> +	u32 respcmd;		/* SDMMC command response          [0x10] */
> +	u32 resp1;		/* SDMMC response 1                [0x14] */
> +	u32 resp2;		/* SDMMC response 2                [0x18] */
> +	u32 resp3;		/* SDMMC response 3                [0x1C] */
> +	u32 resp4;		/* SDMMC response 4                [0x20] */
> +	u32 dtimer;		/* SDMMC data timer                [0x24] */
> +	u32 dlen;		/* SDMMC data length               [0x28] */
> +	u32 dctrl;		/* SDMMC data control              [0x2C] */
> +	u32 dcount;		/* SDMMC data counter              [0x30] */
> +	u32 sta;		/* SDMMC status                    [0x34] */
> +	u32 icr;		/* SDMMC interrupt clear           [0x38] */
> +	u32 mask;		/* SDMMC mask                      [0x3C] */
> +	u32 acktime;		/* SDMMC Acknowledgment timer      [0x40] */
> +	u32 reserved0[3];	/* Reserved, 0x44 - 0x4C - 0x4C           */
> +	u32 idmactrl;		/* SDMMC DMA control               [0x50] */
> +	u32 idmabsize;		/* SDMMC DMA buffer size           [0x54] */
> +	u32 idmabase0;		/* SDMMC DMA buffer 0 base address [0x58] */
> +	u32 idmabase1;		/* SDMMC DMA buffer 1 base address [0x5C] */
> +	u32 reserved1[8];	/* Reserved, 0x4C-0x7C                    */
> +	u32 fifo;		/* SDMMC data FIFO                 [0x80] */
> +};

This is for register offset, right?

I want to use the defined value..likes "#define SDMMC_POWER_CONTROL 0x00"
(It's my preference.)
I'm not sure but i have remembered that was difficult to debug.

> +
> +struct stm32_sdmmc2_host {
> +	struct stm32_sdmmc2 *base;
> +	struct mmc_config cfg;
> +	struct clk clk;
> +	struct reset_ctl reset_ctl;
> +	struct gpio_desc cd_gpio;
> +	u32 clk_reg_add;
> +	u32 pwr_reg_add;
> +};
> +
> +struct stm32_sdmmc2_ctx {
> +	u32 cache_start;
> +	u32 cache_end;
> +	u32 data_length;
> +	bool dpsm_abort;
> +};
> +
> +/* SDMMC_POWER register */
> +#define SDMMC_POWER_PWRCTRL		GENMASK(1, 0)
> +#define SDMMC_POWER_VSWITCH		BIT(2)
> +#define SDMMC_POWER_VSWITCHEN		BIT(3)
> +#define SDMMC_POWER_DIRPOL		BIT(4)
> +
> +/* SDMMC_CLKCR register */
> +#define SDMMC_CLKCR_CLKDIV		GENMASK(9, 0)
> +#define SDMMC_CLKCR_CLKDIV_MAX		SDMMC_CLKCR_CLKDIV
> +#define SDMMC_CLKCR_PWRSAV		BIT(12)
> +#define SDMMC_CLKCR_WIDBUS_4		BIT(14)
> +#define SDMMC_CLKCR_WIDBUS_8		BIT(15)
> +#define SDMMC_CLKCR_NEGEDGE		BIT(16)
> +#define SDMMC_CLKCR_HWFC_EN		BIT(17)
> +#define SDMMC_CLKCR_DDR			BIT(18)
> +#define SDMMC_CLKCR_BUSSPEED		BIT(19)
> +#define SDMMC_CLKCR_SELCLKRX		GENMASK(21, 20)
> +
> +/* SDMMC_CMD register */
> +#define SDMMC_CMD_CMDINDEX		GENMASK(5, 0)
> +#define SDMMC_CMD_CMDTRANS		BIT(6)
> +#define SDMMC_CMD_CMDSTOP		BIT(7)
> +#define SDMMC_CMD_WAITRESP		GENMASK(9, 8)
> +#define SDMMC_CMD_WAITRESP_0		BIT(8)
> +#define SDMMC_CMD_WAITRESP_1		BIT(9)
> +#define SDMMC_CMD_WAITINT		BIT(10)
> +#define SDMMC_CMD_WAITPEND		BIT(11)
> +#define SDMMC_CMD_CPSMEN		BIT(12)
> +#define SDMMC_CMD_DTHOLD		BIT(13)
> +#define SDMMC_CMD_BOOTMODE		BIT(14)
> +#define SDMMC_CMD_BOOTEN		BIT(15)
> +#define SDMMC_CMD_CMDSUSPEND		BIT(16)
> +
> +/* SDMMC_DCTRL register */
> +#define SDMMC_DCTRL_DTEN		BIT(0)
> +#define SDMMC_DCTRL_DTDIR		BIT(1)
> +#define SDMMC_DCTRL_DTMODE		GENMASK(3, 2)
> +#define SDMMC_DCTRL_DBLOCKSIZE		GENMASK(7, 4)
> +#define SDMMC_DCTRL_DBLOCKSIZE_SHIFT	4
> +#define SDMMC_DCTRL_RWSTART		BIT(8)
> +#define SDMMC_DCTRL_RWSTOP		BIT(9)
> +#define SDMMC_DCTRL_RWMOD		BIT(10)
> +#define SDMMC_DCTRL_SDMMCEN		BIT(11)
> +#define SDMMC_DCTRL_BOOTACKEN		BIT(12)
> +#define SDMMC_DCTRL_FIFORST		BIT(13)
> +
> +/* SDMMC_STA register */
> +#define SDMMC_STA_CCRCFAIL		BIT(0)
> +#define SDMMC_STA_DCRCFAIL		BIT(1)
> +#define SDMMC_STA_CTIMEOUT		BIT(2)
> +#define SDMMC_STA_DTIMEOUT		BIT(3)
> +#define SDMMC_STA_TXUNDERR		BIT(4)
> +#define SDMMC_STA_RXOVERR		BIT(5)
> +#define SDMMC_STA_CMDREND		BIT(6)
> +#define SDMMC_STA_CMDSENT		BIT(7)
> +#define SDMMC_STA_DATAEND		BIT(8)
> +#define SDMMC_STA_DHOLD			BIT(9)
> +#define SDMMC_STA_DBCKEND		BIT(10)
> +#define SDMMC_STA_DABORT		BIT(11)
> +#define SDMMC_STA_DPSMACT		BIT(12)
> +#define SDMMC_STA_CPSMACT		BIT(13)
> +#define SDMMC_STA_TXFIFOHE		BIT(14)
> +#define SDMMC_STA_RXFIFOHF		BIT(15)
> +#define SDMMC_STA_TXFIFOF		BIT(16)
> +#define SDMMC_STA_RXFIFOF		BIT(17)
> +#define SDMMC_STA_TXFIFOE		BIT(18)
> +#define SDMMC_STA_RXFIFOE		BIT(19)
> +#define SDMMC_STA_BUSYD0		BIT(20)
> +#define SDMMC_STA_BUSYD0END		BIT(21)
> +#define SDMMC_STA_SDMMCIT		BIT(22)
> +#define SDMMC_STA_ACKFAIL		BIT(23)
> +#define SDMMC_STA_ACKTIMEOUT		BIT(24)
> +#define SDMMC_STA_VSWEND		BIT(25)
> +#define SDMMC_STA_CKSTOP		BIT(26)
> +#define SDMMC_STA_IDMATE		BIT(27)
> +#define SDMMC_STA_IDMABTC		BIT(28)
> +
> +/* SDMMC_ICR register */
> +#define SDMMC_ICR_CCRCFAILC		BIT(0)
> +#define SDMMC_ICR_DCRCFAILC		BIT(1)
> +#define SDMMC_ICR_CTIMEOUTC		BIT(2)
> +#define SDMMC_ICR_DTIMEOUTC		BIT(3)
> +#define SDMMC_ICR_TXUNDERRC		BIT(4)
> +#define SDMMC_ICR_RXOVERRC		BIT(5)
> +#define SDMMC_ICR_CMDRENDC		BIT(6)
> +#define SDMMC_ICR_CMDSENTC		BIT(7)
> +#define SDMMC_ICR_DATAENDC		BIT(8)
> +#define SDMMC_ICR_DHOLDC		BIT(9)
> +#define SDMMC_ICR_DBCKENDC		BIT(10)
> +#define SDMMC_ICR_DABORTC		BIT(11)
> +#define SDMMC_ICR_BUSYD0ENDC		BIT(21)
> +#define SDMMC_ICR_SDMMCITC		BIT(22)
> +#define SDMMC_ICR_ACKFAILC		BIT(23)
> +#define SDMMC_ICR_ACKTIMEOUTC		BIT(24)
> +#define SDMMC_ICR_VSWENDC		BIT(25)
> +#define SDMMC_ICR_CKSTOPC		BIT(26)
> +#define SDMMC_ICR_IDMATEC		BIT(27)
> +#define SDMMC_ICR_IDMABTCC		BIT(28)
> +#define SDMMC_ICR_STATIC_FLAGS		((GENMASK(28, 21)) | (GENMASK(11, 0)))
> +
> +/* SDMMC_MASK register */
> +#define SDMMC_MASK_CCRCFAILIE		BIT(0)
> +#define SDMMC_MASK_DCRCFAILIE		BIT(1)
> +#define SDMMC_MASK_CTIMEOUTIE		BIT(2)
> +#define SDMMC_MASK_DTIMEOUTIE		BIT(3)
> +#define SDMMC_MASK_TXUNDERRIE		BIT(4)
> +#define SDMMC_MASK_RXOVERRIE		BIT(5)
> +#define SDMMC_MASK_CMDRENDIE		BIT(6)
> +#define SDMMC_MASK_CMDSENTIE		BIT(7)
> +#define SDMMC_MASK_DATAENDIE		BIT(8)
> +#define SDMMC_MASK_DHOLDIE		BIT(9)
> +#define SDMMC_MASK_DBCKENDIE		BIT(10)
> +#define SDMMC_MASK_DABORTIE		BIT(11)
> +#define SDMMC_MASK_TXFIFOHEIE		BIT(14)
> +#define SDMMC_MASK_RXFIFOHFIE		BIT(15)
> +#define SDMMC_MASK_RXFIFOFIE		BIT(17)
> +#define SDMMC_MASK_TXFIFOEIE		BIT(18)
> +#define SDMMC_MASK_BUSYD0ENDIE		BIT(21)
> +#define SDMMC_MASK_SDMMCITIE		BIT(22)
> +#define SDMMC_MASK_ACKFAILIE		BIT(23)
> +#define SDMMC_MASK_ACKTIMEOUTIE		BIT(24)
> +#define SDMMC_MASK_VSWENDIE		BIT(25)
> +#define SDMMC_MASK_CKSTOPIE		BIT(26)
> +#define SDMMC_MASK_IDMABTCIE		BIT(28)
> +
> +/* SDMMC_IDMACTRL register */
> +#define SDMMC_IDMACTRL_IDMAEN		BIT(0)
> +
> +#define SDMMC_CMD_TIMEOUT		0xFFFFFFFF
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static void stm32_sdmmc2_start_data(struct mmc *mmc, struct mmc_data *data,
> +				    struct stm32_sdmmc2_ctx *ctx)
> +{
> +	struct stm32_sdmmc2_host *host = mmc->priv;
> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
> +	u32 data_ctrl, idmabase0;
> +
> +	/* Configure the SDMMC DPSM (Data Path State Machine) */
> +	data_ctrl = (__ilog2(data->blocksize) <<
> +		     SDMMC_DCTRL_DBLOCKSIZE_SHIFT) &
> +		    SDMMC_DCTRL_DBLOCKSIZE;
> +
> +	if (data->flags & MMC_DATA_READ) {
> +		data_ctrl |= SDMMC_DCTRL_DTDIR;
> +		idmabase0 = (u32)data->dest;
> +	} else {
> +		idmabase0 = (u32)data->src;
> +	}
> +
> +	/* Set the SDMMC Data TimeOut value */
> +	writel(SDMMC_CMD_TIMEOUT, &sdmmc_handler->dtimer);
> +
> +	/* Set the SDMMC DataLength value */
> +	writel(ctx->data_length, &sdmmc_handler->dlen);
> +
> +	/* Write to SDMMC DCTRL */
> +	writel(data_ctrl, &sdmmc_handler->dctrl);
> +
> +	/* Cache align */
> +	ctx->cache_start = rounddown(idmabase0, ARCH_DMA_MINALIGN);
> +	ctx->cache_end = roundup(idmabase0 + ctx->data_length,
> +				  ARCH_DMA_MINALIGN);
> +
> +	/*
> +	 * Flush data cache before DMA start (clean and invalidate)
> +	 * Clean also needed for read
> +	 * Avoid issue on buffer not cached-aligned
> +	 */
> +	flush_dcache_range(ctx->cache_start, ctx->cache_end);
> +
> +	/* Enable internal DMA */
> +	writel(idmabase0, &sdmmc_handler->idmabase0);
> +	writel(SDMMC_IDMACTRL_IDMAEN, &sdmmc_handler->idmactrl);
> +}
> +
> +static void stm32_sdmmc2_start_cmd(struct mmc *mmc, struct mmc_cmd *cmd, u32 c)
> +{
> +	struct stm32_sdmmc2_host *host = mmc->priv;
> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
> +
> +	if (readl(&sdmmc_handler->arg) & SDMMC_CMD_CPSMEN)
> +		writel(0, &sdmmc_handler->arg);
> +
> +	c |= cmd->cmdidx | SDMMC_CMD_CPSMEN;
> +	if (cmd->resp_type & MMC_RSP_PRESENT) {
> +		if (cmd->resp_type & MMC_RSP_136)
> +			c |= SDMMC_CMD_WAITRESP;
> +		else if (cmd->resp_type & MMC_RSP_CRC)
> +			c |= SDMMC_CMD_WAITRESP_0;
> +		else
> +			c |= SDMMC_CMD_WAITRESP_1;
> +	}
> +
> +	/* Clear flags */
> +	writel(SDMMC_ICR_STATIC_FLAGS, &sdmmc_handler->icr);
> +
> +	/* Set SDMMC argument value */
> +	writel(cmd->cmdarg, &sdmmc_handler->arg);
> +
> +	/* Set SDMMC command parameters */
> +	writel(c, &sdmmc_handler->cmd);
> +}
> +
> +static int stm32_sdmmc2_end_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> +				struct stm32_sdmmc2_ctx *ctx)
> +{
> +	struct stm32_sdmmc2_host *host = mmc->priv;
> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
> +	u32 mask = SDMMC_STA_CTIMEOUT;
> +	u32 status;
> +
> +	if (cmd->resp_type & MMC_RSP_PRESENT) {
> +		mask |= SDMMC_STA_CMDREND;
> +		if (cmd->resp_type & MMC_RSP_CRC)
> +			mask |= SDMMC_STA_CCRCFAIL;
> +	} else {
> +		mask |= SDMMC_STA_CMDSENT;
> +	}
> +
> +	/* Polling status register */
> +	status = readl(&sdmmc_handler->sta);
> +	while (!(status & mask))
> +		status = readl(&sdmmc_handler->sta);
> +
> +	/* Check status */
> +	if (status & SDMMC_STA_CTIMEOUT) {
> +		debug("%s: error SDMMC_STA_CTIMEOUT (0x%x) for cmd %d\n",
> +		      __func__, status, cmd->cmdidx);
> +		ctx->dpsm_abort = true;
> +		return -ETIMEDOUT;
> +	}
> +
> +	if (status & SDMMC_STA_CCRCFAIL && cmd->resp_type & MMC_RSP_CRC) {
> +		debug("%s: error SDMMC_STA_CCRCFAIL (0x%x) for cmd %d\n",
> +		      __func__, status, cmd->cmdidx);
> +		ctx->dpsm_abort = true;
> +		return -EILSEQ;
> +	}
> +
> +	if (status & SDMMC_STA_CMDREND && cmd->resp_type & MMC_RSP_PRESENT) {
> +		cmd->response[0] = readl(&sdmmc_handler->resp1);
> +		if (cmd->resp_type & MMC_RSP_136) {
> +			cmd->response[1] = readl(&sdmmc_handler->resp2);
> +			cmd->response[2] = readl(&sdmmc_handler->resp3);
> +			cmd->response[3] = readl(&sdmmc_handler->resp4);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int stm32_sdmmc2_end_data(struct mmc *mmc, struct mmc_cmd *cmd,
> +				   struct mmc_data *data,
> +				   struct stm32_sdmmc2_ctx *ctx)
> +{
> +	struct stm32_sdmmc2_host *host = mmc->priv;
> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
> +	u32 mask = SDMMC_STA_DCRCFAIL | SDMMC_STA_DTIMEOUT |
> +		   SDMMC_STA_IDMATE | SDMMC_STA_DATAEND;
> +	u32 status;
> +
> +	if (data->flags & MMC_DATA_READ)
> +		mask |= SDMMC_STA_RXOVERR;
> +	else
> +		mask |= SDMMC_STA_TXUNDERR;
> +
> +	status = readl(&sdmmc_handler->sta);
> +	while (!(status & mask))
> +		status = readl(&sdmmc_handler->sta);
> +
> +	/*
> +	 * Need invalidate the dcache again to avoid any
> +	 * cache-refill during the DMA operations (pre-fetching)
> +	 */
> +	if (data->flags & MMC_DATA_READ)
> +		invalidate_dcache_range(ctx->cache_start, ctx->cache_end);
> +
> +	if (status & SDMMC_STA_DCRCFAIL) {
> +		debug("%s: error SDMMC_STA_DCRCFAIL (0x%x) for cmd %d\n",
> +		      __func__, status, cmd->cmdidx);
> +		if (readl(&sdmmc_handler->dcount))
> +			ctx->dpsm_abort = true;
> +		return -EILSEQ;
> +	}
> +
> +	if (status & SDMMC_STA_DTIMEOUT) {
> +		debug("%s: error SDMMC_STA_DTIMEOUT (0x%x) for cmd %d\n",
> +		      __func__, status, cmd->cmdidx);
> +		ctx->dpsm_abort = true;
> +		return -ETIMEDOUT;
> +	}
> +
> +	if (status & SDMMC_STA_TXUNDERR) {
> +		debug("%s: error SDMMC_STA_TXUNDERR (0x%x) for cmd %d\n",
> +		      __func__, status, cmd->cmdidx);
> +		ctx->dpsm_abort = true;
> +		return -EIO;
> +	}
> +
> +	if (status & SDMMC_STA_RXOVERR) {
> +		debug("%s: error SDMMC_STA_RXOVERR (0x%x) for cmd %d\n",
> +		      __func__, status, cmd->cmdidx);
> +		ctx->dpsm_abort = true;
> +		return -EIO;
> +	}
> +
> +	if (status & SDMMC_STA_IDMATE) {
> +		debug("%s: error SDMMC_STA_IDMATE (0x%x) for cmd %d\n",
> +		      __func__, status, cmd->cmdidx);
> +		ctx->dpsm_abort = true;
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int stm32_sdmmc2_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
> +				 struct mmc_data *data)
> +{
> +	struct mmc *mmc = mmc_get_mmc_dev(dev);
> +	struct stm32_sdmmc2_host *host = mmc->priv;
> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
> +	struct stm32_sdmmc2_ctx ctx;
> +	u32 cmdat = data ? SDMMC_CMD_CMDTRANS : 0;
> +	int ret, retry = 3;
> +
> +retry_cmd:
> +	ctx.data_length = 0;
> +	ctx.dpsm_abort = false;
> +
> +	if (data) {
> +		ctx.data_length = data->blocks * data->blocksize;
> +		stm32_sdmmc2_start_data(mmc, data, &ctx);
> +	}
> +
> +	stm32_sdmmc2_start_cmd(mmc, cmd, cmdat);
> +
> +	debug("%s: send cmd %d data: 0x%x @ 0x%x\n",
> +	      __func__, cmd->cmdidx,
> +	      data ? ctx.data_length : 0, (unsigned int)data);
> +
> +	ret = stm32_sdmmc2_end_cmd(mmc, cmd, &ctx);
> +
> +	if (data && !ret)
> +		ret = stm32_sdmmc2_end_data(mmc, cmd, data, &ctx);
> +
> +	/* Clear flags */
> +	writel(SDMMC_ICR_STATIC_FLAGS, &sdmmc_handler->icr);
> +	if (data)
> +		writel(0x0, &sdmmc_handler->idmactrl);
> +
> +	/*
> +	 * To stop Data Path State Machine, a stop_transmission command
> +	 * shall be send on cmd or data errors.
> +	 */
> +	if (ctx.dpsm_abort && (cmd->cmdidx != MMC_CMD_STOP_TRANSMISSION)) {
> +		struct mmc_cmd stop_cmd;
> +
> +		stop_cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> +		stop_cmd.cmdarg = 0;
> +		stop_cmd.resp_type = MMC_RSP_R1b;
> +
> +		debug("%s: send STOP command to abort dpsm treatments\n",
> +		      __func__);
> +
> +		stm32_sdmmc2_start_cmd(mmc, &stop_cmd, SDMMC_CMD_CMDSTOP);
> +		stm32_sdmmc2_end_cmd(mmc, &stop_cmd, &ctx);
> +
> +		writel(SDMMC_ICR_STATIC_FLAGS, &sdmmc_handler->icr);
> +	}
> +
> +	if ((ret != -ETIMEDOUT) && (ret != 0) && retry) {
> +		printf("%s: cmd %d failed, retrying ...\n",
> +		       __func__, cmd->cmdidx);
> +		retry--;
> +		goto retry_cmd;
> +	}
> +
> +	debug("%s: end for CMD %d, ret = %d\n", __func__, cmd->cmdidx, ret);
> +
> +	return ret;
> +}
> +
> +static void stm32_sdmmc2_pwron(struct stm32_sdmmc2_host *host)
> +{
> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
> +
> +	/* Reset */
> +	reset_assert(&host->reset_ctl);
> +	udelay(2);
> +	reset_deassert(&host->reset_ctl);
> +
> +	udelay(1000);
> +
> +	/* Set Power State to ON */
> +	writel(SDMMC_POWER_PWRCTRL | host->pwr_reg_add, &sdmmc_handler->power);
> +
> +	/*
> +	 * 1ms: required power up waiting time before starting the
> +	 * SD initialization sequence
> +	 */
> +	udelay(1000);
> +}
> +
> +#define IS_RISING_EDGE(reg) (reg & SDMMC_CLKCR_NEGEDGE ? 0 : 1)
> +static int stm32_sdmmc2_set_ios(struct udevice *dev)
> +{
> +	struct mmc *mmc = mmc_get_mmc_dev(dev);
> +	struct stm32_sdmmc2_host *host = mmc->priv;
> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
> +	struct mmc_config *cfg = &host->cfg;
> +	u32 desired = mmc->clock;
> +	u32 sys_clock = clk_get_rate(&host->clk);
> +	u32 clk = 0;
> +
> +	debug("%s: bus_with = %d, clock = %d\n", __func__,
> +	      mmc->bus_width, mmc->clock);
> +
> +	if ((mmc->bus_width == 1) && (desired == cfg->f_min))
> +		stm32_sdmmc2_pwron(host);
> +
> +	/*
> +	 * clk_div = 0 => command and data generated on SDMMCCLK falling edge
> +	 * clk_div > 0 and NEGEDGE = 0 => command and data generated on
> +	 * SDMMCCLK rising edge
> +	 * clk_div > 0 and NEGEDGE = 1 => command and data generated on
> +	 * SDMMCCLK falling edge
> +	 */
> +	if (desired && ((sys_clock > desired) ||
> +			IS_RISING_EDGE(host->clk_reg_add))) {
> +		clk = DIV_ROUND_UP(sys_clock, 2 * desired);
> +		if (clk > SDMMC_CLKCR_CLKDIV_MAX)
> +			clk = SDMMC_CLKCR_CLKDIV_MAX;
> +	}
> +
> +	if (mmc->bus_width == 4)
> +		clk |= SDMMC_CLKCR_WIDBUS_4;
> +	if (mmc->bus_width == 8)
> +		clk |= SDMMC_CLKCR_WIDBUS_8;
> +
> +	writel(clk | host->clk_reg_add, &sdmmc_handler->clkcr);
> +
> +	return 0;
> +}
> +
> +static int stm32_sdmmc2_getcd(struct udevice *dev)
> +{
> +	struct stm32_sdmmc2_host *host = dev_get_priv(dev);
> +
> +	debug("stm32_sdmmc2_getcd called\n");
> +
> +	if (dm_gpio_is_valid(&host->cd_gpio))
> +		return dm_gpio_get_value(&host->cd_gpio);
> +
> +	return 1;
> +}
> +
> +static const struct dm_mmc_ops stm32_sdmmc2_ops = {
> +	.send_cmd = stm32_sdmmc2_send_cmd,
> +	.set_ios = stm32_sdmmc2_set_ios,
> +	.get_cd = stm32_sdmmc2_getcd,
> +};
> +
> +static int stm32_sdmmc2_probe(struct udevice *dev)
> +{
> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +	struct stm32_sdmmc2_host *host = dev_get_priv(dev);
> +	struct mmc_config *cfg = &host->cfg;
> +	struct mmc *mmc;
> +	fdt_addr_t addr;
> +	int ret;
> +
> +	addr = devfdt_get_addr(dev);
> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	host->base = (struct stm32_sdmmc2 *)addr;
> +
> +	if (dev_read_bool(dev, "st,negedge"))
> +		host->clk_reg_add |= SDMMC_CLKCR_NEGEDGE;

why did "or" with host->clk_reg_add?

> +	if (dev_read_bool(dev, "st,dirpol"))
> +		host->pwr_reg_add |= SDMMC_POWER_DIRPOL;

ditto

> +
> +	ret = clk_get_by_index(dev, 0, &host->clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_enable(&host->clk);
> +	if (ret)
> +		goto clk_free;
> +
> +	ret = reset_get_by_index(dev, 0, &host->reset_ctl);
> +	if (ret)
> +		goto clk_disable;
> +
> +	gpio_request_by_name(dev, "cd-gpios", 0, &host->cd_gpio,
> +			     GPIOD_IS_IN);
> +
> +	cfg->f_min = 400000;
> +	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
> +	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
> +	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
> +	cfg->name = "STM32 SD/MMCv2";
> +
> +	cfg->host_caps = 0;
> +	if (cfg->f_max > 25000000)
> +		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
> +
> +	switch (dev_read_u32_default(dev, "bus-width", 1)) {
> +	case 8:
> +		cfg->host_caps |= MMC_MODE_8BIT;
> +	case 4:
> +		cfg->host_caps |= MMC_MODE_4BIT;
> +		break;
> +	case 1:
> +		break;
> +	default:
> +		printf("%s: invalid \"bus-width\" property\n", __func__);
> +		ret = -ENOENT;
> +		goto reset_free;

Maybe default value can be 1..

> +	}
> +
> +	mmc = mmc_create(cfg, host);
> +	if (!mmc) {
> +		ret = -ENOMEM;
> +		goto reset_free;
> +	}
> +
> +	mmc->block_dev.removable = !dev_read_bool(dev, "non-removable");
> +	mmc->dev = dev;
> +	upriv->mmc = mmc;
> +
> +	return 0;
> +
> +reset_free:
> +	reset_free(&host->reset_ctl);
> +clk_disable:
> +	clk_disable(&host->clk);
> +clk_free:
> +	clk_free(&host->clk);
> +
> +	return ret;
> +}
> +
> +static const struct udevice_id stm32_sdmmc2_ids[] = {
> +	{ .compatible = "st,stm32-sdmmc2" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(stm32_sdmmc2) = {
> +	.name = "stm32_sdmmc2",
> +	.id = UCLASS_MMC,
> +	.of_match = stm32_sdmmc2_ids,
> +	.ops = &stm32_sdmmc2_ops,
> +	.probe = stm32_sdmmc2_probe,
> +	.priv_auto_alloc_size = sizeof(struct stm32_sdmmc2_host),
> +};
>

Hi Jaehoon

On 07/19/2017 12:11 PM, Jaehoon Chung wrote:
> Hi,
> 
> On 07/19/2017 04:27 PM, patrice.chotard@st.com wrote:
>> From: Patrice Chotard <patrice.chotard@st.com>
>>
>> This patch adds SD/MMC support for STM32H7 SoCs.
>>
>> Here is an extraction of SDMMC main features, embedded in
>> STM32H7 SoCs.
>> The SD/MMC block include the following:
>>   _ Full compliance with MultiMediaCard System Specification
>>     Version 4.51. Card support for three different databus modes:
>>     1-bit (default), 4-bit and 8-bit.
>>   _ Full compatibility with previous versions of MultiMediaCards
>>     (backward compatibility).
>>   _ Full compliance with SD memory card specifications version 4.1.
>>     (SDR104 SDMMC_CK speed limited to maximum allowed IO speed,
>>      SPI mode and UHS-II mode not supported).
>>   _ Full compliance with SDIO card specification version 4.0.
>>     Card support for two different databus modes: 1-bit (default)
>>     and 4-bit. (SDR104 SDMMC_CK speed limited to maximum allowed IO
>>     speed, SPI mode and UHS-II mode not supported).
>>   _ Data transfer up to 208 Mbyte/s for the 8 bit mode.
>>     (depending maximum allowed IO speed).
>>   _ Data and command output enable signals to control external
>>     bidirectional drivers.
>>
>> The current version of the SDMMC supports only one SD/SDIO/MMC card
>> at any one time and a stack of MMC Version 4.51 or previous.
>>
>> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>> ---
>>
>> v2: _ add .get_cd() callback support
>>
>>   drivers/mmc/Kconfig        |   8 +
>>   drivers/mmc/Makefile       |   1 +
>>   drivers/mmc/stm32_sdmmc2.c | 619 +++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 628 insertions(+)
>>   create mode 100644 drivers/mmc/stm32_sdmmc2.c
>>
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index 82b8d75..f2e4c26 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -377,6 +377,14 @@ config GENERIC_ATMEL_MCI
>>   	  the SD Memory Card Specification V2.0, the SDIO V2.0 specification
>>   	  and CE-ATA V1.1.
>>   
>> +config STM32_SDMMC2
> 
> Why add the SDMMC2? not SDMMC?
> Is there a special reason?

It's simply the IP name

> 
>> +	bool "STMicroelectronics STM32H7 SD/MMC Host Controller support"
>> +	depends on DM_MMC && OF_CONTROL && DM_MMC_OPS
>> +	help
>> +	  This selects support for the SD/MMC controller on STM32H7 SoCs.
>> +	  If you have a board based on such a SoC and with a SD/MMC slot,
>> +	  say Y or M here.
>> +
>>   endif
>>   
>>   config TEGRA124_MMC_DISABLE_EXT_LOOPBACK
>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>> index 2d781c3..2584663 100644
>> --- a/drivers/mmc/Makefile
>> +++ b/drivers/mmc/Makefile
>> @@ -43,6 +43,7 @@ obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
>>   obj-$(CONFIG_MMC_SANDBOX)		+= sandbox_mmc.o
>>   obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o
>>   obj-$(CONFIG_SH_SDHI) += sh_sdhi.o
>> +obj-$(CONFIG_STM32_SDMMC2) += stm32_sdmmc2.o
>>   
>>   # SDHCI
>>   obj-$(CONFIG_MMC_SDHCI)			+= sdhci.o
>> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
>> new file mode 100644
>> index 0000000..84f96e5
>> --- /dev/null
>> +++ b/drivers/mmc/stm32_sdmmc2.c
>> @@ -0,0 +1,619 @@
>> +/*
>> + *  Copyright (c) 2017 STMicrelectronics
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0
>> + */
>> +
>> +#include <common.h>
>> +#include <clk.h>
>> +#include <dm.h>
>> +#include <fdtdec.h>
>> +#include <libfdt.h>
>> +#include <mmc.h>
>> +#include <reset.h>
>> +#include <asm/io.h>
>> +#include <asm/gpio.h>
>> +
>> +struct stm32_sdmmc2 {
>> +	u32 power;		/* SDMMC power control             [0x00] */
>> +	u32 clkcr;		/* SDMMC clock control             [0x04] */
>> +	u32 arg;		/* SDMMC argument                  [0x08] */
>> +	u32 cmd;		/* SDMMC command                   [0x0C] */
>> +	u32 respcmd;		/* SDMMC command response          [0x10] */
>> +	u32 resp1;		/* SDMMC response 1                [0x14] */
>> +	u32 resp2;		/* SDMMC response 2                [0x18] */
>> +	u32 resp3;		/* SDMMC response 3                [0x1C] */
>> +	u32 resp4;		/* SDMMC response 4                [0x20] */
>> +	u32 dtimer;		/* SDMMC data timer                [0x24] */
>> +	u32 dlen;		/* SDMMC data length               [0x28] */
>> +	u32 dctrl;		/* SDMMC data control              [0x2C] */
>> +	u32 dcount;		/* SDMMC data counter              [0x30] */
>> +	u32 sta;		/* SDMMC status                    [0x34] */
>> +	u32 icr;		/* SDMMC interrupt clear           [0x38] */
>> +	u32 mask;		/* SDMMC mask                      [0x3C] */
>> +	u32 acktime;		/* SDMMC Acknowledgment timer      [0x40] */
>> +	u32 reserved0[3];	/* Reserved, 0x44 - 0x4C - 0x4C           */
>> +	u32 idmactrl;		/* SDMMC DMA control               [0x50] */
>> +	u32 idmabsize;		/* SDMMC DMA buffer size           [0x54] */
>> +	u32 idmabase0;		/* SDMMC DMA buffer 0 base address [0x58] */
>> +	u32 idmabase1;		/* SDMMC DMA buffer 1 base address [0x5C] */
>> +	u32 reserved1[8];	/* Reserved, 0x4C-0x7C                    */
>> +	u32 fifo;		/* SDMMC data FIFO                 [0x80] */
>> +};
> 
> This is for register offset, right?

yes

> 
> I want to use the defined value..likes "#define SDMMC_POWER_CONTROL 0x00"
> (It's my preference.)
> I'm not sure but i have remembered that was difficult to debug.

But on http://www.denx.de/wiki/U-Boot/CodingStyle, it's recommended to 
use structures for I/O access, see "Use structures for I/O access " chapter.


> 
>> +
>> +struct stm32_sdmmc2_host {
>> +	struct stm32_sdmmc2 *base;
>> +	struct mmc_config cfg;
>> +	struct clk clk;
>> +	struct reset_ctl reset_ctl;
>> +	struct gpio_desc cd_gpio;
>> +	u32 clk_reg_add;
>> +	u32 pwr_reg_add;
>> +};
>> +
>> +struct stm32_sdmmc2_ctx {
>> +	u32 cache_start;
>> +	u32 cache_end;
>> +	u32 data_length;
>> +	bool dpsm_abort;
>> +};
>> +
>> +/* SDMMC_POWER register */
>> +#define SDMMC_POWER_PWRCTRL		GENMASK(1, 0)
>> +#define SDMMC_POWER_VSWITCH		BIT(2)
>> +#define SDMMC_POWER_VSWITCHEN		BIT(3)
>> +#define SDMMC_POWER_DIRPOL		BIT(4)
>> +
>> +/* SDMMC_CLKCR register */
>> +#define SDMMC_CLKCR_CLKDIV		GENMASK(9, 0)
>> +#define SDMMC_CLKCR_CLKDIV_MAX		SDMMC_CLKCR_CLKDIV
>> +#define SDMMC_CLKCR_PWRSAV		BIT(12)
>> +#define SDMMC_CLKCR_WIDBUS_4		BIT(14)
>> +#define SDMMC_CLKCR_WIDBUS_8		BIT(15)
>> +#define SDMMC_CLKCR_NEGEDGE		BIT(16)
>> +#define SDMMC_CLKCR_HWFC_EN		BIT(17)
>> +#define SDMMC_CLKCR_DDR			BIT(18)
>> +#define SDMMC_CLKCR_BUSSPEED		BIT(19)
>> +#define SDMMC_CLKCR_SELCLKRX		GENMASK(21, 20)
>> +
>> +/* SDMMC_CMD register */
>> +#define SDMMC_CMD_CMDINDEX		GENMASK(5, 0)
>> +#define SDMMC_CMD_CMDTRANS		BIT(6)
>> +#define SDMMC_CMD_CMDSTOP		BIT(7)
>> +#define SDMMC_CMD_WAITRESP		GENMASK(9, 8)
>> +#define SDMMC_CMD_WAITRESP_0		BIT(8)
>> +#define SDMMC_CMD_WAITRESP_1		BIT(9)
>> +#define SDMMC_CMD_WAITINT		BIT(10)
>> +#define SDMMC_CMD_WAITPEND		BIT(11)
>> +#define SDMMC_CMD_CPSMEN		BIT(12)
>> +#define SDMMC_CMD_DTHOLD		BIT(13)
>> +#define SDMMC_CMD_BOOTMODE		BIT(14)
>> +#define SDMMC_CMD_BOOTEN		BIT(15)
>> +#define SDMMC_CMD_CMDSUSPEND		BIT(16)
>> +
>> +/* SDMMC_DCTRL register */
>> +#define SDMMC_DCTRL_DTEN		BIT(0)
>> +#define SDMMC_DCTRL_DTDIR		BIT(1)
>> +#define SDMMC_DCTRL_DTMODE		GENMASK(3, 2)
>> +#define SDMMC_DCTRL_DBLOCKSIZE		GENMASK(7, 4)
>> +#define SDMMC_DCTRL_DBLOCKSIZE_SHIFT	4
>> +#define SDMMC_DCTRL_RWSTART		BIT(8)
>> +#define SDMMC_DCTRL_RWSTOP		BIT(9)
>> +#define SDMMC_DCTRL_RWMOD		BIT(10)
>> +#define SDMMC_DCTRL_SDMMCEN		BIT(11)
>> +#define SDMMC_DCTRL_BOOTACKEN		BIT(12)
>> +#define SDMMC_DCTRL_FIFORST		BIT(13)
>> +
>> +/* SDMMC_STA register */
>> +#define SDMMC_STA_CCRCFAIL		BIT(0)
>> +#define SDMMC_STA_DCRCFAIL		BIT(1)
>> +#define SDMMC_STA_CTIMEOUT		BIT(2)
>> +#define SDMMC_STA_DTIMEOUT		BIT(3)
>> +#define SDMMC_STA_TXUNDERR		BIT(4)
>> +#define SDMMC_STA_RXOVERR		BIT(5)
>> +#define SDMMC_STA_CMDREND		BIT(6)
>> +#define SDMMC_STA_CMDSENT		BIT(7)
>> +#define SDMMC_STA_DATAEND		BIT(8)
>> +#define SDMMC_STA_DHOLD			BIT(9)
>> +#define SDMMC_STA_DBCKEND		BIT(10)
>> +#define SDMMC_STA_DABORT		BIT(11)
>> +#define SDMMC_STA_DPSMACT		BIT(12)
>> +#define SDMMC_STA_CPSMACT		BIT(13)
>> +#define SDMMC_STA_TXFIFOHE		BIT(14)
>> +#define SDMMC_STA_RXFIFOHF		BIT(15)
>> +#define SDMMC_STA_TXFIFOF		BIT(16)
>> +#define SDMMC_STA_RXFIFOF		BIT(17)
>> +#define SDMMC_STA_TXFIFOE		BIT(18)
>> +#define SDMMC_STA_RXFIFOE		BIT(19)
>> +#define SDMMC_STA_BUSYD0		BIT(20)
>> +#define SDMMC_STA_BUSYD0END		BIT(21)
>> +#define SDMMC_STA_SDMMCIT		BIT(22)
>> +#define SDMMC_STA_ACKFAIL		BIT(23)
>> +#define SDMMC_STA_ACKTIMEOUT		BIT(24)
>> +#define SDMMC_STA_VSWEND		BIT(25)
>> +#define SDMMC_STA_CKSTOP		BIT(26)
>> +#define SDMMC_STA_IDMATE		BIT(27)
>> +#define SDMMC_STA_IDMABTC		BIT(28)
>> +
>> +/* SDMMC_ICR register */
>> +#define SDMMC_ICR_CCRCFAILC		BIT(0)
>> +#define SDMMC_ICR_DCRCFAILC		BIT(1)
>> +#define SDMMC_ICR_CTIMEOUTC		BIT(2)
>> +#define SDMMC_ICR_DTIMEOUTC		BIT(3)
>> +#define SDMMC_ICR_TXUNDERRC		BIT(4)
>> +#define SDMMC_ICR_RXOVERRC		BIT(5)
>> +#define SDMMC_ICR_CMDRENDC		BIT(6)
>> +#define SDMMC_ICR_CMDSENTC		BIT(7)
>> +#define SDMMC_ICR_DATAENDC		BIT(8)
>> +#define SDMMC_ICR_DHOLDC		BIT(9)
>> +#define SDMMC_ICR_DBCKENDC		BIT(10)
>> +#define SDMMC_ICR_DABORTC		BIT(11)
>> +#define SDMMC_ICR_BUSYD0ENDC		BIT(21)
>> +#define SDMMC_ICR_SDMMCITC		BIT(22)
>> +#define SDMMC_ICR_ACKFAILC		BIT(23)
>> +#define SDMMC_ICR_ACKTIMEOUTC		BIT(24)
>> +#define SDMMC_ICR_VSWENDC		BIT(25)
>> +#define SDMMC_ICR_CKSTOPC		BIT(26)
>> +#define SDMMC_ICR_IDMATEC		BIT(27)
>> +#define SDMMC_ICR_IDMABTCC		BIT(28)
>> +#define SDMMC_ICR_STATIC_FLAGS		((GENMASK(28, 21)) | (GENMASK(11, 0)))
>> +
>> +/* SDMMC_MASK register */
>> +#define SDMMC_MASK_CCRCFAILIE		BIT(0)
>> +#define SDMMC_MASK_DCRCFAILIE		BIT(1)
>> +#define SDMMC_MASK_CTIMEOUTIE		BIT(2)
>> +#define SDMMC_MASK_DTIMEOUTIE		BIT(3)
>> +#define SDMMC_MASK_TXUNDERRIE		BIT(4)
>> +#define SDMMC_MASK_RXOVERRIE		BIT(5)
>> +#define SDMMC_MASK_CMDRENDIE		BIT(6)
>> +#define SDMMC_MASK_CMDSENTIE		BIT(7)
>> +#define SDMMC_MASK_DATAENDIE		BIT(8)
>> +#define SDMMC_MASK_DHOLDIE		BIT(9)
>> +#define SDMMC_MASK_DBCKENDIE		BIT(10)
>> +#define SDMMC_MASK_DABORTIE		BIT(11)
>> +#define SDMMC_MASK_TXFIFOHEIE		BIT(14)
>> +#define SDMMC_MASK_RXFIFOHFIE		BIT(15)
>> +#define SDMMC_MASK_RXFIFOFIE		BIT(17)
>> +#define SDMMC_MASK_TXFIFOEIE		BIT(18)
>> +#define SDMMC_MASK_BUSYD0ENDIE		BIT(21)
>> +#define SDMMC_MASK_SDMMCITIE		BIT(22)
>> +#define SDMMC_MASK_ACKFAILIE		BIT(23)
>> +#define SDMMC_MASK_ACKTIMEOUTIE		BIT(24)
>> +#define SDMMC_MASK_VSWENDIE		BIT(25)
>> +#define SDMMC_MASK_CKSTOPIE		BIT(26)
>> +#define SDMMC_MASK_IDMABTCIE		BIT(28)
>> +
>> +/* SDMMC_IDMACTRL register */
>> +#define SDMMC_IDMACTRL_IDMAEN		BIT(0)
>> +
>> +#define SDMMC_CMD_TIMEOUT		0xFFFFFFFF
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static void stm32_sdmmc2_start_data(struct mmc *mmc, struct mmc_data *data,
>> +				    struct stm32_sdmmc2_ctx *ctx)
>> +{
>> +	struct stm32_sdmmc2_host *host = mmc->priv;
>> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
>> +	u32 data_ctrl, idmabase0;
>> +
>> +	/* Configure the SDMMC DPSM (Data Path State Machine) */
>> +	data_ctrl = (__ilog2(data->blocksize) <<
>> +		     SDMMC_DCTRL_DBLOCKSIZE_SHIFT) &
>> +		    SDMMC_DCTRL_DBLOCKSIZE;
>> +
>> +	if (data->flags & MMC_DATA_READ) {
>> +		data_ctrl |= SDMMC_DCTRL_DTDIR;
>> +		idmabase0 = (u32)data->dest;
>> +	} else {
>> +		idmabase0 = (u32)data->src;
>> +	}
>> +
>> +	/* Set the SDMMC Data TimeOut value */
>> +	writel(SDMMC_CMD_TIMEOUT, &sdmmc_handler->dtimer);
>> +
>> +	/* Set the SDMMC DataLength value */
>> +	writel(ctx->data_length, &sdmmc_handler->dlen);
>> +
>> +	/* Write to SDMMC DCTRL */
>> +	writel(data_ctrl, &sdmmc_handler->dctrl);
>> +
>> +	/* Cache align */
>> +	ctx->cache_start = rounddown(idmabase0, ARCH_DMA_MINALIGN);
>> +	ctx->cache_end = roundup(idmabase0 + ctx->data_length,
>> +				  ARCH_DMA_MINALIGN);
>> +
>> +	/*
>> +	 * Flush data cache before DMA start (clean and invalidate)
>> +	 * Clean also needed for read
>> +	 * Avoid issue on buffer not cached-aligned
>> +	 */
>> +	flush_dcache_range(ctx->cache_start, ctx->cache_end);
>> +
>> +	/* Enable internal DMA */
>> +	writel(idmabase0, &sdmmc_handler->idmabase0);
>> +	writel(SDMMC_IDMACTRL_IDMAEN, &sdmmc_handler->idmactrl);
>> +}
>> +
>> +static void stm32_sdmmc2_start_cmd(struct mmc *mmc, struct mmc_cmd *cmd, u32 c)
>> +{
>> +	struct stm32_sdmmc2_host *host = mmc->priv;
>> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
>> +
>> +	if (readl(&sdmmc_handler->arg) & SDMMC_CMD_CPSMEN)
>> +		writel(0, &sdmmc_handler->arg);
>> +
>> +	c |= cmd->cmdidx | SDMMC_CMD_CPSMEN;
>> +	if (cmd->resp_type & MMC_RSP_PRESENT) {
>> +		if (cmd->resp_type & MMC_RSP_136)
>> +			c |= SDMMC_CMD_WAITRESP;
>> +		else if (cmd->resp_type & MMC_RSP_CRC)
>> +			c |= SDMMC_CMD_WAITRESP_0;
>> +		else
>> +			c |= SDMMC_CMD_WAITRESP_1;
>> +	}
>> +
>> +	/* Clear flags */
>> +	writel(SDMMC_ICR_STATIC_FLAGS, &sdmmc_handler->icr);
>> +
>> +	/* Set SDMMC argument value */
>> +	writel(cmd->cmdarg, &sdmmc_handler->arg);
>> +
>> +	/* Set SDMMC command parameters */
>> +	writel(c, &sdmmc_handler->cmd);
>> +}
>> +
>> +static int stm32_sdmmc2_end_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>> +				struct stm32_sdmmc2_ctx *ctx)
>> +{
>> +	struct stm32_sdmmc2_host *host = mmc->priv;
>> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
>> +	u32 mask = SDMMC_STA_CTIMEOUT;
>> +	u32 status;
>> +
>> +	if (cmd->resp_type & MMC_RSP_PRESENT) {
>> +		mask |= SDMMC_STA_CMDREND;
>> +		if (cmd->resp_type & MMC_RSP_CRC)
>> +			mask |= SDMMC_STA_CCRCFAIL;
>> +	} else {
>> +		mask |= SDMMC_STA_CMDSENT;
>> +	}
>> +
>> +	/* Polling status register */
>> +	status = readl(&sdmmc_handler->sta);
>> +	while (!(status & mask))
>> +		status = readl(&sdmmc_handler->sta);
>> +
>> +	/* Check status */
>> +	if (status & SDMMC_STA_CTIMEOUT) {
>> +		debug("%s: error SDMMC_STA_CTIMEOUT (0x%x) for cmd %d\n",
>> +		      __func__, status, cmd->cmdidx);
>> +		ctx->dpsm_abort = true;
>> +		return -ETIMEDOUT;
>> +	}
>> +
>> +	if (status & SDMMC_STA_CCRCFAIL && cmd->resp_type & MMC_RSP_CRC) {
>> +		debug("%s: error SDMMC_STA_CCRCFAIL (0x%x) for cmd %d\n",
>> +		      __func__, status, cmd->cmdidx);
>> +		ctx->dpsm_abort = true;
>> +		return -EILSEQ;
>> +	}
>> +
>> +	if (status & SDMMC_STA_CMDREND && cmd->resp_type & MMC_RSP_PRESENT) {
>> +		cmd->response[0] = readl(&sdmmc_handler->resp1);
>> +		if (cmd->resp_type & MMC_RSP_136) {
>> +			cmd->response[1] = readl(&sdmmc_handler->resp2);
>> +			cmd->response[2] = readl(&sdmmc_handler->resp3);
>> +			cmd->response[3] = readl(&sdmmc_handler->resp4);
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_sdmmc2_end_data(struct mmc *mmc, struct mmc_cmd *cmd,
>> +				   struct mmc_data *data,
>> +				   struct stm32_sdmmc2_ctx *ctx)
>> +{
>> +	struct stm32_sdmmc2_host *host = mmc->priv;
>> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
>> +	u32 mask = SDMMC_STA_DCRCFAIL | SDMMC_STA_DTIMEOUT |
>> +		   SDMMC_STA_IDMATE | SDMMC_STA_DATAEND;
>> +	u32 status;
>> +
>> +	if (data->flags & MMC_DATA_READ)
>> +		mask |= SDMMC_STA_RXOVERR;
>> +	else
>> +		mask |= SDMMC_STA_TXUNDERR;
>> +
>> +	status = readl(&sdmmc_handler->sta);
>> +	while (!(status & mask))
>> +		status = readl(&sdmmc_handler->sta);
>> +
>> +	/*
>> +	 * Need invalidate the dcache again to avoid any
>> +	 * cache-refill during the DMA operations (pre-fetching)
>> +	 */
>> +	if (data->flags & MMC_DATA_READ)
>> +		invalidate_dcache_range(ctx->cache_start, ctx->cache_end);
>> +
>> +	if (status & SDMMC_STA_DCRCFAIL) {
>> +		debug("%s: error SDMMC_STA_DCRCFAIL (0x%x) for cmd %d\n",
>> +		      __func__, status, cmd->cmdidx);
>> +		if (readl(&sdmmc_handler->dcount))
>> +			ctx->dpsm_abort = true;
>> +		return -EILSEQ;
>> +	}
>> +
>> +	if (status & SDMMC_STA_DTIMEOUT) {
>> +		debug("%s: error SDMMC_STA_DTIMEOUT (0x%x) for cmd %d\n",
>> +		      __func__, status, cmd->cmdidx);
>> +		ctx->dpsm_abort = true;
>> +		return -ETIMEDOUT;
>> +	}
>> +
>> +	if (status & SDMMC_STA_TXUNDERR) {
>> +		debug("%s: error SDMMC_STA_TXUNDERR (0x%x) for cmd %d\n",
>> +		      __func__, status, cmd->cmdidx);
>> +		ctx->dpsm_abort = true;
>> +		return -EIO;
>> +	}
>> +
>> +	if (status & SDMMC_STA_RXOVERR) {
>> +		debug("%s: error SDMMC_STA_RXOVERR (0x%x) for cmd %d\n",
>> +		      __func__, status, cmd->cmdidx);
>> +		ctx->dpsm_abort = true;
>> +		return -EIO;
>> +	}
>> +
>> +	if (status & SDMMC_STA_IDMATE) {
>> +		debug("%s: error SDMMC_STA_IDMATE (0x%x) for cmd %d\n",
>> +		      __func__, status, cmd->cmdidx);
>> +		ctx->dpsm_abort = true;
>> +		return -EIO;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_sdmmc2_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>> +				 struct mmc_data *data)
>> +{
>> +	struct mmc *mmc = mmc_get_mmc_dev(dev);
>> +	struct stm32_sdmmc2_host *host = mmc->priv;
>> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
>> +	struct stm32_sdmmc2_ctx ctx;
>> +	u32 cmdat = data ? SDMMC_CMD_CMDTRANS : 0;
>> +	int ret, retry = 3;
>> +
>> +retry_cmd:
>> +	ctx.data_length = 0;
>> +	ctx.dpsm_abort = false;
>> +
>> +	if (data) {
>> +		ctx.data_length = data->blocks * data->blocksize;
>> +		stm32_sdmmc2_start_data(mmc, data, &ctx);
>> +	}
>> +
>> +	stm32_sdmmc2_start_cmd(mmc, cmd, cmdat);
>> +
>> +	debug("%s: send cmd %d data: 0x%x @ 0x%x\n",
>> +	      __func__, cmd->cmdidx,
>> +	      data ? ctx.data_length : 0, (unsigned int)data);
>> +
>> +	ret = stm32_sdmmc2_end_cmd(mmc, cmd, &ctx);
>> +
>> +	if (data && !ret)
>> +		ret = stm32_sdmmc2_end_data(mmc, cmd, data, &ctx);
>> +
>> +	/* Clear flags */
>> +	writel(SDMMC_ICR_STATIC_FLAGS, &sdmmc_handler->icr);
>> +	if (data)
>> +		writel(0x0, &sdmmc_handler->idmactrl);
>> +
>> +	/*
>> +	 * To stop Data Path State Machine, a stop_transmission command
>> +	 * shall be send on cmd or data errors.
>> +	 */
>> +	if (ctx.dpsm_abort && (cmd->cmdidx != MMC_CMD_STOP_TRANSMISSION)) {
>> +		struct mmc_cmd stop_cmd;
>> +
>> +		stop_cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
>> +		stop_cmd.cmdarg = 0;
>> +		stop_cmd.resp_type = MMC_RSP_R1b;
>> +
>> +		debug("%s: send STOP command to abort dpsm treatments\n",
>> +		      __func__);
>> +
>> +		stm32_sdmmc2_start_cmd(mmc, &stop_cmd, SDMMC_CMD_CMDSTOP);
>> +		stm32_sdmmc2_end_cmd(mmc, &stop_cmd, &ctx);
>> +
>> +		writel(SDMMC_ICR_STATIC_FLAGS, &sdmmc_handler->icr);
>> +	}
>> +
>> +	if ((ret != -ETIMEDOUT) && (ret != 0) && retry) {
>> +		printf("%s: cmd %d failed, retrying ...\n",
>> +		       __func__, cmd->cmdidx);
>> +		retry--;
>> +		goto retry_cmd;
>> +	}
>> +
>> +	debug("%s: end for CMD %d, ret = %d\n", __func__, cmd->cmdidx, ret);
>> +
>> +	return ret;
>> +}
>> +
>> +static void stm32_sdmmc2_pwron(struct stm32_sdmmc2_host *host)
>> +{
>> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
>> +
>> +	/* Reset */
>> +	reset_assert(&host->reset_ctl);
>> +	udelay(2);
>> +	reset_deassert(&host->reset_ctl);
>> +
>> +	udelay(1000);
>> +
>> +	/* Set Power State to ON */
>> +	writel(SDMMC_POWER_PWRCTRL | host->pwr_reg_add, &sdmmc_handler->power);
>> +
>> +	/*
>> +	 * 1ms: required power up waiting time before starting the
>> +	 * SD initialization sequence
>> +	 */
>> +	udelay(1000);
>> +}
>> +
>> +#define IS_RISING_EDGE(reg) (reg & SDMMC_CLKCR_NEGEDGE ? 0 : 1)
>> +static int stm32_sdmmc2_set_ios(struct udevice *dev)
>> +{
>> +	struct mmc *mmc = mmc_get_mmc_dev(dev);
>> +	struct stm32_sdmmc2_host *host = mmc->priv;
>> +	struct stm32_sdmmc2 *sdmmc_handler = host->base;
>> +	struct mmc_config *cfg = &host->cfg;
>> +	u32 desired = mmc->clock;
>> +	u32 sys_clock = clk_get_rate(&host->clk);
>> +	u32 clk = 0;
>> +
>> +	debug("%s: bus_with = %d, clock = %d\n", __func__,
>> +	      mmc->bus_width, mmc->clock);
>> +
>> +	if ((mmc->bus_width == 1) && (desired == cfg->f_min))
>> +		stm32_sdmmc2_pwron(host);
>> +
>> +	/*
>> +	 * clk_div = 0 => command and data generated on SDMMCCLK falling edge
>> +	 * clk_div > 0 and NEGEDGE = 0 => command and data generated on
>> +	 * SDMMCCLK rising edge
>> +	 * clk_div > 0 and NEGEDGE = 1 => command and data generated on
>> +	 * SDMMCCLK falling edge
>> +	 */
>> +	if (desired && ((sys_clock > desired) ||
>> +			IS_RISING_EDGE(host->clk_reg_add))) {
>> +		clk = DIV_ROUND_UP(sys_clock, 2 * desired);
>> +		if (clk > SDMMC_CLKCR_CLKDIV_MAX)
>> +			clk = SDMMC_CLKCR_CLKDIV_MAX;
>> +	}
>> +
>> +	if (mmc->bus_width == 4)
>> +		clk |= SDMMC_CLKCR_WIDBUS_4;
>> +	if (mmc->bus_width == 8)
>> +		clk |= SDMMC_CLKCR_WIDBUS_8;
>> +
>> +	writel(clk | host->clk_reg_add, &sdmmc_handler->clkcr);
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_sdmmc2_getcd(struct udevice *dev)
>> +{
>> +	struct stm32_sdmmc2_host *host = dev_get_priv(dev);
>> +
>> +	debug("stm32_sdmmc2_getcd called\n");
>> +
>> +	if (dm_gpio_is_valid(&host->cd_gpio))
>> +		return dm_gpio_get_value(&host->cd_gpio);
>> +
>> +	return 1;
>> +}
>> +
>> +static const struct dm_mmc_ops stm32_sdmmc2_ops = {
>> +	.send_cmd = stm32_sdmmc2_send_cmd,
>> +	.set_ios = stm32_sdmmc2_set_ios,
>> +	.get_cd = stm32_sdmmc2_getcd,
>> +};
>> +
>> +static int stm32_sdmmc2_probe(struct udevice *dev)
>> +{
>> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>> +	struct stm32_sdmmc2_host *host = dev_get_priv(dev);
>> +	struct mmc_config *cfg = &host->cfg;
>> +	struct mmc *mmc;
>> +	fdt_addr_t addr;
>> +	int ret;
>> +
>> +	addr = devfdt_get_addr(dev);
>> +	if (addr == FDT_ADDR_T_NONE)
>> +		return -EINVAL;
>> +
>> +	host->base = (struct stm32_sdmmc2 *)addr;
>> +
>> +	if (dev_read_bool(dev, "st,negedge"))
>> +		host->clk_reg_add |= SDMMC_CLKCR_NEGEDGE;
> 
> why did "or" with host->clk_reg_add?

Ok i think i understood your remark, the clk_reg_add name is confusing.

clk_reg_add is a mask that will be applied during stm32_sdmmc2_set_ios()
in the clock control register (clkcr).

I will rename this field from clk_reg_add to clk_reg_msk

> 
>> +	if (dev_read_bool(dev, "st,dirpol"))
>> +		host->pwr_reg_add |= SDMMC_POWER_DIRPOL;
> 
> ditto

ok, same reason than for clk_reg_add, it will be renamed pwr_reg_msk

> 
>> +
>> +	ret = clk_get_by_index(dev, 0, &host->clk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = clk_enable(&host->clk);
>> +	if (ret)
>> +		goto clk_free;
>> +
>> +	ret = reset_get_by_index(dev, 0, &host->reset_ctl);
>> +	if (ret)
>> +		goto clk_disable;
>> +
>> +	gpio_request_by_name(dev, "cd-gpios", 0, &host->cd_gpio,
>> +			     GPIOD_IS_IN);
>> +
>> +	cfg->f_min = 400000;
>> +	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
>> +	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
>> +	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
>> +	cfg->name = "STM32 SD/MMCv2";
>> +
>> +	cfg->host_caps = 0;
>> +	if (cfg->f_max > 25000000)
>> +		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
>> +
>> +	switch (dev_read_u32_default(dev, "bus-width", 1)) {
>> +	case 8:
>> +		cfg->host_caps |= MMC_MODE_8BIT;
>> +	case 4:
>> +		cfg->host_caps |= MMC_MODE_4BIT;
>> +		break;
>> +	case 1:
>> +		break;
>> +	default:
>> +		printf("%s: invalid \"bus-width\" property\n", __func__);
>> +		ret = -ENOENT;
>> +		goto reset_free;
> 
> Maybe default value can be 1..

Default value is already set to 1 in case the property "bus-width" is 
not present in DT.

The "default" case is just a protection in case of "bus-width" property 
is set with other values than 1, 4 or 8.

> 
>> +	}
>> +
>> +	mmc = mmc_create(cfg, host);
>> +	if (!mmc) {
>> +		ret = -ENOMEM;
>> +		goto reset_free;
>> +	}
>> +
>> +	mmc->block_dev.removable = !dev_read_bool(dev, "non-removable");
>> +	mmc->dev = dev;
>> +	upriv->mmc = mmc;
>> +
>> +	return 0;
>> +
>> +reset_free:
>> +	reset_free(&host->reset_ctl);
>> +clk_disable:
>> +	clk_disable(&host->clk);
>> +clk_free:
>> +	clk_free(&host->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct udevice_id stm32_sdmmc2_ids[] = {
>> +	{ .compatible = "st,stm32-sdmmc2" },
>> +	{ }
>> +};
>> +
>> +U_BOOT_DRIVER(stm32_sdmmc2) = {
>> +	.name = "stm32_sdmmc2",
>> +	.id = UCLASS_MMC,
>> +	.of_match = stm32_sdmmc2_ids,
>> +	.ops = &stm32_sdmmc2_ops,
>> +	.probe = stm32_sdmmc2_probe,
>> +	.priv_auto_alloc_size = sizeof(struct stm32_sdmmc2_host),
>> +};
>>
>

Hi Patrice

On 07/19/2017 09:20 PM, Patrice CHOTARD wrote:
> Hi Jaehoon
> 
> On 07/19/2017 12:11 PM, Jaehoon Chung wrote:
>> Hi,
>>
>> On 07/19/2017 04:27 PM, patrice.chotard@st.com wrote:
>>> From: Patrice Chotard <patrice.chotard@st.com>
>>>
>>> This patch adds SD/MMC support for STM32H7 SoCs.
>>>
>>> Here is an extraction of SDMMC main features, embedded in
>>> STM32H7 SoCs.
>>> The SD/MMC block include the following:
>>>   _ Full compliance with MultiMediaCard System Specification
>>>     Version 4.51. Card support for three different databus modes:
>>>     1-bit (default), 4-bit and 8-bit.
>>>   _ Full compatibility with previous versions of MultiMediaCards
>>>     (backward compatibility).
>>>   _ Full compliance with SD memory card specifications version 4.1.
>>>     (SDR104 SDMMC_CK speed limited to maximum allowed IO speed,
>>>      SPI mode and UHS-II mode not supported).
>>>   _ Full compliance with SDIO card specification version 4.0.
>>>     Card support for two different databus modes: 1-bit (default)
>>>     and 4-bit. (SDR104 SDMMC_CK speed limited to maximum allowed IO
>>>     speed, SPI mode and UHS-II mode not supported).
>>>   _ Data transfer up to 208 Mbyte/s for the 8 bit mode.
>>>     (depending maximum allowed IO speed).
>>>   _ Data and command output enable signals to control external
>>>     bidirectional drivers.
>>>
>>> The current version of the SDMMC supports only one SD/SDIO/MMC card
>>> at any one time and a stack of MMC Version 4.51 or previous.
>>>
>>> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
>>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>>> ---
>>>
>>> v2: _ add .get_cd() callback support
>>>
>>>   drivers/mmc/Kconfig        |   8 +
>>>   drivers/mmc/Makefile       |   1 +
>>>   drivers/mmc/stm32_sdmmc2.c | 619 +++++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 628 insertions(+)
>>>   create mode 100644 drivers/mmc/stm32_sdmmc2.c
>>>
>>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>> index 82b8d75..f2e4c26 100644
>>> --- a/drivers/mmc/Kconfig
>>> +++ b/drivers/mmc/Kconfig
>>> @@ -377,6 +377,14 @@ config GENERIC_ATMEL_MCI
>>>   	  the SD Memory Card Specification V2.0, the SDIO V2.0 specification
>>>   	  and CE-ATA V1.1.
>>>   
>>> +config STM32_SDMMC2
>>
>> Why add the SDMMC2? not SDMMC?
>> Is there a special reason?
> 
> It's simply the IP name
> 
>>
>>> +	bool "STMicroelectronics STM32H7 SD/MMC Host Controller support"
>>> +	depends on DM_MMC && OF_CONTROL && DM_MMC_OPS
>>> +	help
>>> +	  This selects support for the SD/MMC controller on STM32H7 SoCs.
>>> +	  If you have a board based on such a SoC and with a SD/MMC slot,
>>> +	  say Y or M here.
>>> +
>>>   endif
>>>   
>>>   config TEGRA124_MMC_DISABLE_EXT_LOOPBACK
>>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>>> index 2d781c3..2584663 100644
>>> --- a/drivers/mmc/Makefile
>>> +++ b/drivers/mmc/Makefile
>>> @@ -43,6 +43,7 @@ obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
>>>   obj-$(CONFIG_MMC_SANDBOX)		+= sandbox_mmc.o
>>>   obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o
>>>   obj-$(CONFIG_SH_SDHI) += sh_sdhi.o
>>> +obj-$(CONFIG_STM32_SDMMC2) += stm32_sdmmc2.o
>>>   
>>>   # SDHCI
>>>   obj-$(CONFIG_MMC_SDHCI)			+= sdhci.o
>>> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
>>> new file mode 100644
>>> index 0000000..84f96e5
>>> --- /dev/null
>>> +++ b/drivers/mmc/stm32_sdmmc2.c
>>> @@ -0,0 +1,619 @@
>>> +/*
>>> + *  Copyright (c) 2017 STMicrelectronics
>>> + *
>>> + * SPDX-License-Identifier:	GPL-2.0
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <clk.h>
>>> +#include <dm.h>
>>> +#include <fdtdec.h>
>>> +#include <libfdt.h>
>>> +#include <mmc.h>
>>> +#include <reset.h>
>>> +#include <asm/io.h>
>>> +#include <asm/gpio.h>
>>> +
>>> +struct stm32_sdmmc2 {
>>> +	u32 power;		/* SDMMC power control             [0x00] */
>>> +	u32 clkcr;		/* SDMMC clock control             [0x04] */
>>> +	u32 arg;		/* SDMMC argument                  [0x08] */
>>> +	u32 cmd;		/* SDMMC command                   [0x0C] */
>>> +	u32 respcmd;		/* SDMMC command response          [0x10] */
>>> +	u32 resp1;		/* SDMMC response 1                [0x14] */
>>> +	u32 resp2;		/* SDMMC response 2                [0x18] */
>>> +	u32 resp3;		/* SDMMC response 3                [0x1C] */
>>> +	u32 resp4;		/* SDMMC response 4                [0x20] */
>>> +	u32 dtimer;		/* SDMMC data timer                [0x24] */
>>> +	u32 dlen;		/* SDMMC data length               [0x28] */
>>> +	u32 dctrl;		/* SDMMC data control              [0x2C] */
>>> +	u32 dcount;		/* SDMMC data counter              [0x30] */
>>> +	u32 sta;		/* SDMMC status                    [0x34] */
>>> +	u32 icr;		/* SDMMC interrupt clear           [0x38] */
>>> +	u32 mask;		/* SDMMC mask                      [0x3C] */
>>> +	u32 acktime;		/* SDMMC Acknowledgment timer      [0x40] */
>>> +	u32 reserved0[3];	/* Reserved, 0x44 - 0x4C - 0x4C           */
>>> +	u32 idmactrl;		/* SDMMC DMA control               [0x50] */
>>> +	u32 idmabsize;		/* SDMMC DMA buffer size           [0x54] */
>>> +	u32 idmabase0;		/* SDMMC DMA buffer 0 base address [0x58] */
>>> +	u32 idmabase1;		/* SDMMC DMA buffer 1 base address [0x5C] */
>>> +	u32 reserved1[8];	/* Reserved, 0x4C-0x7C                    */
>>> +	u32 fifo;		/* SDMMC data FIFO                 [0x80] */
>>> +};
>>
>> This is for register offset, right?
> 
> yes
> 
>>
>> I want to use the defined value..likes "#define SDMMC_POWER_CONTROL 0x00"
>> (It's my preference.)
>> I'm not sure but i have remembered that was difficult to debug.
> 
> But on http://www.denx.de/wiki/U-Boot/CodingStyle, it's recommended to 
> use structures for I/O access, see "Use structures for I/O access " chapter.

It's recommended, not mandatory. 
Already used the "define" style in mmc subsystem on u-boot.
e.g) sdhci/dwmmc controller etc...

I think it's no problem to use "define" style..
In my opinion, it's more complex than define..

For example, some controllers can be changed the register offset according to IP version.
Then we needs to make new structure for it.

And if someone calculated wrong offset, then it's possible that all register should be accessed to wrong offset.
That's why it's my preference.

> 
> 
>>
>>> +
>>> +struct stm32_sdmmc2_host {
>>> +	struct stm32_sdmmc2 *base;
>>> +	struct mmc_config cfg;
>>> +	struct clk clk;
>>> +	struct reset_ctl reset_ctl;
>>> +	struct gpio_desc cd_gpio;
>>> +	u32 clk_reg_add;
>>> +	u32 pwr_reg_add;
>>> +};
>>> +

[..snip..]

>>> +		cfg->host_caps |= MMC_MODE_4BIT;
>>> +		break;
>>> +	case 1:
>>> +		break;
>>> +	default:
>>> +		printf("%s: invalid \"bus-width\" property\n", __func__);
>>> +		ret = -ENOENT;
>>> +		goto reset_free;
>>
>> Maybe default value can be 1..
> 
> Default value is already set to 1 in case the property "bus-width" is 
> not present in DT.
> 
> The "default" case is just a protection in case of "bus-width" property 
> is set with other values than 1, 4 or 8.

Then i think it can be just displayed wrong dt property value, not returned error
and host controller and card can be worked with 1bit buswidth..
how about? :)

Best Regards,
Jaehoon Chung

> 
>>
>>> +	}
>>> +
>>> +	mmc = mmc_create(cfg, host);
>>> +	if (!mmc) {
>>> +		ret = -ENOMEM;
>>> +		goto reset_free;
>>> +	}
>>> +
>>> +	mmc->block_dev.removable = !dev_read_bool(dev, "non-removable");
>>> +	mmc->dev = dev;
>>> +	upriv->mmc = mmc;
>>> +
>>> +	return 0;
>>> +
>>> +reset_free:
>>> +	reset_free(&host->reset_ctl);
>>> +clk_disable:
>>> +	clk_disable(&host->clk);
>>> +clk_free:
>>> +	clk_free(&host->clk);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static const struct udevice_id stm32_sdmmc2_ids[] = {
>>> +	{ .compatible = "st,stm32-sdmmc2" },
>>> +	{ }
>>> +};
>>> +
>>> +U_BOOT_DRIVER(stm32_sdmmc2) = {
>>> +	.name = "stm32_sdmmc2",
>>> +	.id = UCLASS_MMC,
>>> +	.of_match = stm32_sdmmc2_ids,
>>> +	.ops = &stm32_sdmmc2_ops,
>>> +	.probe = stm32_sdmmc2_probe,
>>> +	.priv_auto_alloc_size = sizeof(struct stm32_sdmmc2_host),
>>> +};
>>>
>>
> 
>

Hi Jaehoon

On 07/20/2017 06:22 AM, Jaehoon Chung wrote:
> Hi Patrice
> 
> On 07/19/2017 09:20 PM, Patrice CHOTARD wrote:
>> Hi Jaehoon
>>
>> On 07/19/2017 12:11 PM, Jaehoon Chung wrote:
>>> Hi,
>>>
>>> On 07/19/2017 04:27 PM, patrice.chotard@st.com wrote:
>>>> From: Patrice Chotard <patrice.chotard@st.com>
>>>>
>>>> This patch adds SD/MMC support for STM32H7 SoCs.
>>>>
>>>> Here is an extraction of SDMMC main features, embedded in
>>>> STM32H7 SoCs.
>>>> The SD/MMC block include the following:
>>>>    _ Full compliance with MultiMediaCard System Specification
>>>>      Version 4.51. Card support for three different databus modes:
>>>>      1-bit (default), 4-bit and 8-bit.
>>>>    _ Full compatibility with previous versions of MultiMediaCards
>>>>      (backward compatibility).
>>>>    _ Full compliance with SD memory card specifications version 4.1.
>>>>      (SDR104 SDMMC_CK speed limited to maximum allowed IO speed,
>>>>       SPI mode and UHS-II mode not supported).
>>>>    _ Full compliance with SDIO card specification version 4.0.
>>>>      Card support for two different databus modes: 1-bit (default)
>>>>      and 4-bit. (SDR104 SDMMC_CK speed limited to maximum allowed IO
>>>>      speed, SPI mode and UHS-II mode not supported).
>>>>    _ Data transfer up to 208 Mbyte/s for the 8 bit mode.
>>>>      (depending maximum allowed IO speed).
>>>>    _ Data and command output enable signals to control external
>>>>      bidirectional drivers.
>>>>
>>>> The current version of the SDMMC supports only one SD/SDIO/MMC card
>>>> at any one time and a stack of MMC Version 4.51 or previous.
>>>>
>>>> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
>>>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>>>> ---
>>>>
>>>> v2: _ add .get_cd() callback support
>>>>
>>>>    drivers/mmc/Kconfig        |   8 +
>>>>    drivers/mmc/Makefile       |   1 +
>>>>    drivers/mmc/stm32_sdmmc2.c | 619 +++++++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 628 insertions(+)
>>>>    create mode 100644 drivers/mmc/stm32_sdmmc2.c
>>>>
>>>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>>> index 82b8d75..f2e4c26 100644
>>>> --- a/drivers/mmc/Kconfig
>>>> +++ b/drivers/mmc/Kconfig
>>>> @@ -377,6 +377,14 @@ config GENERIC_ATMEL_MCI
>>>>    	  the SD Memory Card Specification V2.0, the SDIO V2.0 specification
>>>>    	  and CE-ATA V1.1.
>>>>    
>>>> +config STM32_SDMMC2
>>>
>>> Why add the SDMMC2? not SDMMC?
>>> Is there a special reason?
>>
>> It's simply the IP name
>>
>>>
>>>> +	bool "STMicroelectronics STM32H7 SD/MMC Host Controller support"
>>>> +	depends on DM_MMC && OF_CONTROL && DM_MMC_OPS
>>>> +	help
>>>> +	  This selects support for the SD/MMC controller on STM32H7 SoCs.
>>>> +	  If you have a board based on such a SoC and with a SD/MMC slot,
>>>> +	  say Y or M here.
>>>> +
>>>>    endif
>>>>    
>>>>    config TEGRA124_MMC_DISABLE_EXT_LOOPBACK
>>>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>>>> index 2d781c3..2584663 100644
>>>> --- a/drivers/mmc/Makefile
>>>> +++ b/drivers/mmc/Makefile
>>>> @@ -43,6 +43,7 @@ obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
>>>>    obj-$(CONFIG_MMC_SANDBOX)		+= sandbox_mmc.o
>>>>    obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o
>>>>    obj-$(CONFIG_SH_SDHI) += sh_sdhi.o
>>>> +obj-$(CONFIG_STM32_SDMMC2) += stm32_sdmmc2.o
>>>>    
>>>>    # SDHCI
>>>>    obj-$(CONFIG_MMC_SDHCI)			+= sdhci.o
>>>> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
>>>> new file mode 100644
>>>> index 0000000..84f96e5
>>>> --- /dev/null
>>>> +++ b/drivers/mmc/stm32_sdmmc2.c
>>>> @@ -0,0 +1,619 @@
>>>> +/*
>>>> + *  Copyright (c) 2017 STMicrelectronics
>>>> + *
>>>> + * SPDX-License-Identifier:	GPL-2.0
>>>> + */
>>>> +
>>>> +#include <common.h>
>>>> +#include <clk.h>
>>>> +#include <dm.h>
>>>> +#include <fdtdec.h>
>>>> +#include <libfdt.h>
>>>> +#include <mmc.h>
>>>> +#include <reset.h>
>>>> +#include <asm/io.h>
>>>> +#include <asm/gpio.h>
>>>> +
>>>> +struct stm32_sdmmc2 {
>>>> +	u32 power;		/* SDMMC power control             [0x00] */
>>>> +	u32 clkcr;		/* SDMMC clock control             [0x04] */
>>>> +	u32 arg;		/* SDMMC argument                  [0x08] */
>>>> +	u32 cmd;		/* SDMMC command                   [0x0C] */
>>>> +	u32 respcmd;		/* SDMMC command response          [0x10] */
>>>> +	u32 resp1;		/* SDMMC response 1                [0x14] */
>>>> +	u32 resp2;		/* SDMMC response 2                [0x18] */
>>>> +	u32 resp3;		/* SDMMC response 3                [0x1C] */
>>>> +	u32 resp4;		/* SDMMC response 4                [0x20] */
>>>> +	u32 dtimer;		/* SDMMC data timer                [0x24] */
>>>> +	u32 dlen;		/* SDMMC data length               [0x28] */
>>>> +	u32 dctrl;		/* SDMMC data control              [0x2C] */
>>>> +	u32 dcount;		/* SDMMC data counter              [0x30] */
>>>> +	u32 sta;		/* SDMMC status                    [0x34] */
>>>> +	u32 icr;		/* SDMMC interrupt clear           [0x38] */
>>>> +	u32 mask;		/* SDMMC mask                      [0x3C] */
>>>> +	u32 acktime;		/* SDMMC Acknowledgment timer      [0x40] */
>>>> +	u32 reserved0[3];	/* Reserved, 0x44 - 0x4C - 0x4C           */
>>>> +	u32 idmactrl;		/* SDMMC DMA control               [0x50] */
>>>> +	u32 idmabsize;		/* SDMMC DMA buffer size           [0x54] */
>>>> +	u32 idmabase0;		/* SDMMC DMA buffer 0 base address [0x58] */
>>>> +	u32 idmabase1;		/* SDMMC DMA buffer 1 base address [0x5C] */
>>>> +	u32 reserved1[8];	/* Reserved, 0x4C-0x7C                    */
>>>> +	u32 fifo;		/* SDMMC data FIFO                 [0x80] */
>>>> +};
>>>
>>> This is for register offset, right?
>>
>> yes
>>
>>>
>>> I want to use the defined value..likes "#define SDMMC_POWER_CONTROL 0x00"
>>> (It's my preference.)
>>> I'm not sure but i have remembered that was difficult to debug.
>>
>> But on http://www.denx.de/wiki/U-Boot/CodingStyle, it's recommended to
>> use structures for I/O access, see "Use structures for I/O access " chapter.
> 
> It's recommended, not mandatory.
> Already used the "define" style in mmc subsystem on u-boot.
> e.g) sdhci/dwmmc controller etc...
> 
> I think it's no problem to use "define" style..
> In my opinion, it's more complex than define..
> 
> For example, some controllers can be changed the register offset according to IP version.
> Then we needs to make new structure for it.
> 
> And if someone calculated wrong offset, then it's possible that all register should be accessed to wrong offset.
> That's why it's my preference.

Ok i will convert this driver with registers offset.

> 
>>
>>
>>>
>>>> +
>>>> +struct stm32_sdmmc2_host {
>>>> +	struct stm32_sdmmc2 *base;
>>>> +	struct mmc_config cfg;
>>>> +	struct clk clk;
>>>> +	struct reset_ctl reset_ctl;
>>>> +	struct gpio_desc cd_gpio;
>>>> +	u32 clk_reg_add;
>>>> +	u32 pwr_reg_add;
>>>> +};
>>>> +
> 
> [..snip..]
> 
>>>> +		cfg->host_caps |= MMC_MODE_4BIT;
>>>> +		break;
>>>> +	case 1:
>>>> +		break;
>>>> +	default:
>>>> +		printf("%s: invalid \"bus-width\" property\n", __func__);
>>>> +		ret = -ENOENT;
>>>> +		goto reset_free;
>>>
>>> Maybe default value can be 1..
>>
>> Default value is already set to 1 in case the property "bus-width" is
>> not present in DT.
>>
>> The "default" case is just a protection in case of "bus-width" property
>> is set with other values than 1, 4 or 8.
> 
> Then i think it can be just displayed wrong dt property value, not returned error
> and host controller and card can be worked with 1bit buswidth..
> how about? :)

Agree, i will fix it

Thanks

Patrice

> 
> Best Regards,
> Jaehoon Chung
> 
>>
>>>
>>>> +	}
>>>> +
>>>> +	mmc = mmc_create(cfg, host);
>>>> +	if (!mmc) {
>>>> +		ret = -ENOMEM;
>>>> +		goto reset_free;
>>>> +	}
>>>> +
>>>> +	mmc->block_dev.removable = !dev_read_bool(dev, "non-removable");
>>>> +	mmc->dev = dev;
>>>> +	upriv->mmc = mmc;
>>>> +
>>>> +	return 0;
>>>> +
>>>> +reset_free:
>>>> +	reset_free(&host->reset_ctl);
>>>> +clk_disable:
>>>> +	clk_disable(&host->clk);
>>>> +clk_free:
>>>> +	clk_free(&host->clk);
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +static const struct udevice_id stm32_sdmmc2_ids[] = {
>>>> +	{ .compatible = "st,stm32-sdmmc2" },
>>>> +	{ }
>>>> +};
>>>> +
>>>> +U_BOOT_DRIVER(stm32_sdmmc2) = {
>>>> +	.name = "stm32_sdmmc2",
>>>> +	.id = UCLASS_MMC,
>>>> +	.of_match = stm32_sdmmc2_ids,
>>>> +	.ops = &stm32_sdmmc2_ops,
>>>> +	.probe = stm32_sdmmc2_probe,
>>>> +	.priv_auto_alloc_size = sizeof(struct stm32_sdmmc2_host),
>>>> +};
>>>>
>>>
>>
>>
>