[PATCH V2 0/5] mmc: add stm32 sdmmc controller

[PATCH V2 0/5] mmc: add stm32 sdmmc controller

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch serie adds support of stm32 SDMMC controller.
stm32h7 is the first SoC to use stm32 SDMMC controller
(previous SoC had pl180 controller).

The SDMMC features 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 compliance with SD memory card specifications version 4.1.
 SDR104 speed limited to maximum allowed I/O speed, SPI mode and
 UHS-II mode not supported.
-Full compliance with SDIO card specification version 4.0.

change V2:
-dt-bindings: properties name
st,dir-output-high; st,neg-edge; st,use-ckin
-dt-bindings: node example s/sdmmc@/mmc@/g
-removes return of enable/disable_imask functions
-adds dev_warn if clk divisor is too short.
-replaces overprotection of n_elem by fail of dma_map_sg
-removes shortcut host->base
-Use device_property_present

Ludovic Barre (5):
  dt-bindings: mmc: document the stm32 sdmmc bindings
  mmc: add stm32 sdmmc controller driver
  ARM: dts: stm32: add sdmmc support for stm32h743
  ARM: dts: stm32: add sdmmc1 support for stm32h743i-eval
  ARM: configs: stm32: add mmc and ext2/3/4 support

 .../devicetree/bindings/mmc/st,stm32-sdmmc.txt     |  35 +
 arch/arm/boot/dts/stm32h743-pinctrl.dtsi           |  26 +
 arch/arm/boot/dts/stm32h743.dtsi                   |  26 +
 arch/arm/boot/dts/stm32h743i-eval.dts              |  11 +
 arch/arm/configs/stm32_defconfig                   |   4 +-
 drivers/mmc/host/Kconfig                           |   8 +
 drivers/mmc/host/Makefile                          |   1 +
 drivers/mmc/host/stm32-sdmmc.c                     | 708 +++++++++++++++++++++
 drivers/mmc/host/stm32-sdmmc.h                     | 220 +++++++
 9 files changed, 1038 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/st,stm32-sdmmc.txt
 create mode 100644 drivers/mmc/host/stm32-sdmmc.c
 create mode 100644 drivers/mmc/host/stm32-sdmmc.h

-- 
2.7.4

[PATCH V2 1/5] dt-bindings: mmc: document the stm32 sdmmc bindings

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

Document the binding for stm32 sdmmc controller.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 .../devicetree/bindings/mmc/st,stm32-sdmmc.txt     | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/st,stm32-sdmmc.txt

diff --git a/Documentation/devicetree/bindings/mmc/st,stm32-sdmmc.txt b/Documentation/devicetree/bindings/mmc/st,stm32-sdmmc.txt
new file mode 100644
index 0000000..74d0bff
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/st,stm32-sdmmc.txt
@@ -0,0 +1,35 @@
+* STMicroelectronics STM32 SDMMC controller
+
+The highspeed MMC host controller on STM32 soc family
+provides an interface for MMC, SD and SDIO types of memory cards.
+
+This file documents differences between the core properties described
+by mmc.txt and the properties used by the sdmmc driver.
+
+Required properties:
+ - compatible: Should be "st,stm32h7-sdmmc"
+ - reg: mmc controller base registers
+ - interrupts: Should contain the interrupt number
+ - clocks: Should contain phandle for the clock feeding the controller
+ - resets: Should contain phandle for the reset feeding the controller
+
+Optional property:
+- st,dir-output-high: Allow to select direction polarity of external voltage
+  transceiver (which manage data and command direction).
+  if set: Voltage transceiver IOs are driven as output when direction signals are high,
+  else: Voltage transceiver IOs are driven as output when direction signals are low.
+- st,neg-edge: generate data & command on sdmmc clock falling edge
+- st,use-ckin: use sdmmc_ckin pin from an external driver to sample
+  the receive data (example: with voltage switch transceiver).
+
+Example:
+	sdmmc1: mmc@52007000 {
+		compatible = "st,stm32h7-sdmmc";
+		reg = <0x52007000 0x1000>;
+		interrupts = <49>;
+		clocks = <&rcc SDMMC1_CK>;
+		resets = <&rcc SDMMC1_R>;
+		bus-width = <4>;
+		cap-sd-highspeed;
+		cap-mmc-highspeed;
+	};
-- 
2.7.4

Re: [PATCH V2 1/5] dt-bindings: mmc: document the stm32 sdmmc bindings

[Rob Herring]

On Wed, Feb 28, 2018 at 04:47:20PM +0100, Ludovic Barre wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
> 
> Document the binding for stm32 sdmmc controller.
> 
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
>  .../devicetree/bindings/mmc/st,stm32-sdmmc.txt     | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/st,stm32-sdmmc.txt

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH V2 2/5] mmc: add stm32 sdmmc controller driver

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds support for stm32 SDMMC controller.
The SDMMC controller provides an interface for SD,SDIO
cards and MMC devices.

The SDMMC features 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 compliance with SD memory card specifications version 4.1.
 SDR104 speed limited to maximum allowed I/O speed, SPI mode and
 UHS-II mode not supported.
-Full compliance with SDIO card specification version 4.0.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/Kconfig       |   8 +
 drivers/mmc/host/Makefile      |   1 +
 drivers/mmc/host/stm32-sdmmc.c | 708 +++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/stm32-sdmmc.h | 220 +++++++++++++
 4 files changed, 937 insertions(+)
 create mode 100644 drivers/mmc/host/stm32-sdmmc.c
 create mode 100644 drivers/mmc/host/stm32-sdmmc.h

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 620c2d9..13c4d32 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -938,3 +938,11 @@ config MMC_SDHCI_OMAP
 	  If you have a controller with this interface, say Y or M here.
 
 	  If unsure, say N.
+
+config MMC_STM32_SDMMC
+	tristate "STMicroelectronics STM32 SD/MMC Host Controller support"
+	depends on ARCH_STM32 && OF
+	help
+	  This selects support for the SD/MMC controller on STM32 SoCs.
+	  If you have a board based on such a SoC and with a SD/MMC slot,
+	  say Y or M here.
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 84cd138..13f601f 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_MMC_SUNXI)		+= sunxi-mmc.o
 obj-$(CONFIG_MMC_USDHI6ROL0)	+= usdhi6rol0.o
 obj-$(CONFIG_MMC_TOSHIBA_PCI)	+= toshsd.o
 obj-$(CONFIG_MMC_BCM2835)	+= bcm2835.o
+obj-$(CONFIG_MMC_STM32_SDMMC)	+= stm32-sdmmc.o
 
 obj-$(CONFIG_MMC_REALTEK_PCI)	+= rtsx_pci_sdmmc.o
 obj-$(CONFIG_MMC_REALTEK_USB)	+= rtsx_usb_sdmmc.o
diff --git a/drivers/mmc/host/stm32-sdmmc.c b/drivers/mmc/host/stm32-sdmmc.c
new file mode 100644
index 0000000..8705e25
--- /dev/null
+++ b/drivers/mmc/host/stm32-sdmmc.c
@@ -0,0 +1,708 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
+ * Author: Ludovic Barre <ludovic.barre@st.com> for STMicroelectronics.
+ */
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/slot-gpio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#include "stm32-sdmmc.h"
+
+#define DRIVER_NAME "stm32-sdmmc"
+
+#ifdef CONFIG_DEBUG_FS
+static int stm32_sdmmc_stat_show(struct seq_file *s, void *v)
+{
+	struct sdmmc_host *host = s->private;
+	struct sdmmc_stat *stat = &host->stat;
+
+	seq_puts(s, "\033[1;34mstm32 sdmmc statistic\033[0m\n");
+	seq_printf(s, "%-20s:%d\n", "sdmmc ck", host->sdmmc_ck);
+	seq_printf(s, "%-20s:%ld\n", "nb request", stat->n_req);
+	seq_printf(s, "%-20s:%ld\n", "nb data req", stat->n_datareq);
+	seq_printf(s, "%-20s:%ld\n", "nb cmd timeout", stat->n_ctimeout);
+	seq_printf(s, "%-20s:%ld\n", "nb cmd crcfail", stat->n_ccrcfail);
+	seq_printf(s, "%-20s:%ld\n", "nb dat timeout", stat->n_dtimeout);
+	seq_printf(s, "%-20s:%ld\n", "nb dat crcfail", stat->n_dcrcfail);
+	seq_printf(s, "%-20s:%ld\n", "nb rx overrun", stat->n_rxoverrun);
+	seq_printf(s, "%-20s:%ld\n", "nb tx underrun", stat->n_txunderrun);
+
+	return 0;
+}
+
+static ssize_t stm32_sdmmc_stat_reset(struct file *filp,
+				      const char __user *ubuf,
+				      size_t count, loff_t *ppos)
+{
+	struct seq_file *seqf = filp->private_data;
+	struct sdmmc_host *host = seqf->private;
+
+	mutex_lock(&seqf->lock);
+	memset(&host->stat, 0, sizeof(host->stat));
+	mutex_unlock(&seqf->lock);
+
+	return count;
+}
+
+static int stm32_sdmmc_stat_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, stm32_sdmmc_stat_show, inode->i_private);
+}
+
+static const struct file_operations stm32_sdmmc_stat_fops = {
+	.owner		= THIS_MODULE,
+	.open		= stm32_sdmmc_stat_open,
+	.read		= seq_read,
+	.write		= stm32_sdmmc_stat_reset,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static void stm32_sdmmc_stat_init(struct sdmmc_host *host)
+{
+	struct mmc_host	*mmc = host->mmc;
+	struct dentry *root;
+
+	root = mmc->debugfs_root;
+	if (!root)
+		return;
+
+	if (!debugfs_create_file("stat", 0600, root, host,
+				 &stm32_sdmmc_stat_fops))
+		dev_err(mmc_dev(host->mmc), "failed to initialize debugfs\n");
+}
+
+#define STAT_INC(stat) ((stat)++)
+#else
+static void stm32_sdmmc_stat_init(struct sdmmc_host *host)
+{
+}
+
+#define STAT_INC(stat)
+#endif
+
+static inline void enable_imask(struct sdmmc_host *host, u32 imask)
+{
+	u32 newmask;
+
+	newmask = readl_relaxed(host->base + SDMMC_MASKR);
+	newmask |= imask;
+
+	dev_vdbg(mmc_dev(host->mmc), "mask:%#x\n", newmask);
+
+	writel_relaxed(newmask, host->base + SDMMC_MASKR);
+}
+
+static inline void disable_imask(struct sdmmc_host *host, u32 imask)
+{
+	u32 newmask;
+
+	newmask = readl_relaxed(host->base + SDMMC_MASKR);
+	newmask &= ~imask;
+
+	dev_vdbg(mmc_dev(host->mmc), "mask:%#x\n", newmask);
+
+	writel_relaxed(newmask, host->base + SDMMC_MASKR);
+}
+
+static inline void clear_imask(struct sdmmc_host *host)
+{
+	u32 mask = readl_relaxed(host->base + SDMMC_MASKR);
+
+	/* preserve the SDIO IRQ mask state */
+	mask &= MASKR_SDIOITIE;
+
+	dev_vdbg(mmc_dev(host->mmc), "mask:%#x\n", mask);
+
+	writel_relaxed(mask, host->base + SDMMC_MASKR);
+}
+
+static int stm32_sdmmc_card_busy(struct mmc_host *mmc)
+{
+	struct sdmmc_host *host = mmc_priv(mmc);
+	unsigned long flags;
+	u32 status;
+
+	spin_lock_irqsave(&host->lock, flags);
+	status = readl_relaxed(host->base + SDMMC_STAR);
+	spin_unlock_irqrestore(&host->lock, flags);
+
+	return !!(status & STAR_BUSYD0);
+}
+
+static void stm32_sdmmc_request_end(struct sdmmc_host *host,
+				    struct mmc_request *mrq)
+{
+	writel_relaxed(0, host->base + SDMMC_CMDR);
+	writel_relaxed(ICR_STATIC_FLAG, host->base + SDMMC_ICR);
+
+	host->mrq = NULL;
+	host->cmd = NULL;
+	host->data = NULL;
+
+	clear_imask(host);
+
+	mmc_request_done(host->mmc, mrq);
+}
+
+static void stm32_sdmmc_pwroff(struct sdmmc_host *host)
+{
+	/* Only a reset could disable sdmmc */
+	reset_control_assert(host->rst);
+	udelay(2);
+	reset_control_deassert(host->rst);
+
+	/*
+	 * Set the SDMMC in Power-cycle state. This will make that the
+	 * SDMMC_D[7:0], SDMMC_CMD and SDMMC_CK are driven low,
+	 * to prevent the Card from being powered through the signal lines.
+	 */
+	writel_relaxed(POWERCTRL_CYC | host->pwr_reg_add,
+		       host->base + SDMMC_POWER);
+}
+
+static void stm32_sdmmc_pwron(struct sdmmc_host *host)
+{
+	/*
+	 * After a power-cycle state, we must set the SDMMC in Power-off.
+	 * The SDMMC_D[7:0], SDMMC_CMD and SDMMC_CK are driven high.
+	 * Then we can set the SDMMC to Power-on state
+	 */
+	writel_relaxed(POWERCTRL_OFF | host->pwr_reg_add,
+		       host->base + SDMMC_POWER);
+	mdelay(1);
+	writel_relaxed(POWERCTRL_ON | host->pwr_reg_add,
+		       host->base + SDMMC_POWER);
+}
+
+static void stm32_sdmmc_set_clkreg(struct sdmmc_host *host, struct mmc_ios *ios)
+{
+	u32 desired = ios->clock;
+	u32 clk = 0;
+
+	/*
+	 * sdmmc_ck = sdmmcclk/(2*clkdiv)
+	 * clkdiv 0 => bypass
+	 */
+	if (desired) {
+		if (desired >= host->sdmmcclk) {
+			clk = 0;
+			host->sdmmc_ck = host->sdmmcclk;
+		} else {
+			clk = DIV_ROUND_UP(host->sdmmcclk, 2 * desired);
+			if (clk > CLKCR_CLKDIV_MAX) {
+				clk = CLKCR_CLKDIV_MAX;
+				dev_warn(mmc_dev(host->mmc),
+					 "divisor too short, sd clk over clocked\n");
+			}
+
+			host->sdmmc_ck = host->sdmmcclk / (2 * clk);
+		}
+	}
+
+	if (ios->bus_width == MMC_BUS_WIDTH_4)
+		clk |= CLKCR_WIDBUS_4;
+	if (ios->bus_width == MMC_BUS_WIDTH_8)
+		clk |= CLKCR_WIDBUS_8;
+
+	clk |= CLKCR_HWFC_EN;
+
+	writel_relaxed(clk | host->clk_reg_add, host->base + SDMMC_CLKCR);
+}
+
+static void stm32_sdmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	struct sdmmc_host *host = mmc_priv(mmc);
+
+	stm32_sdmmc_set_clkreg(host, ios);
+
+	switch (ios->power_mode) {
+	case MMC_POWER_OFF:
+		if (!IS_ERR(mmc->supply.vmmc))
+			mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+		stm32_sdmmc_pwroff(host);
+		return;
+	case MMC_POWER_UP:
+		if (!IS_ERR(mmc->supply.vmmc))
+			mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, ios->vdd);
+		break;
+	case MMC_POWER_ON:
+		stm32_sdmmc_pwron(host);
+		break;
+	}
+}
+
+static int stm32_sdmmc_validate_data(struct sdmmc_host *host,
+				     struct mmc_data *data, int cookie)
+{
+	int n_elem;
+
+	if (!data || data->host_cookie == COOKIE_PRE_MAPPED)
+		return 0;
+
+	if (!is_power_of_2(data->blksz)) {
+		dev_err(mmc_dev(host->mmc),
+			"unsupported block size (%d bytes)\n", data->blksz);
+		return -EINVAL;
+	}
+
+	if (data->sg->offset & 3 || data->sg->length & 3) {
+		dev_err(mmc_dev(host->mmc),
+			"unaligned scatterlist: ofst:%x length:%d\n",
+			data->sg->offset, data->sg->length);
+		return -EINVAL;
+	}
+
+	n_elem = dma_map_sg(mmc_dev(host->mmc),
+			    data->sg,
+			    data->sg_len,
+			    mmc_get_dma_dir(data));
+
+	if (!n_elem) {
+		dev_err(mmc_dev(host->mmc), "dma_map_sg failed\n");
+		return -EINVAL;
+	}
+
+	data->host_cookie = cookie;
+
+	return 0;
+}
+
+static void stm32_sdmmc_start_data(struct sdmmc_host *host,
+				   struct mmc_data *data)
+{
+	u32 datactrl, timeout, imask, idmactrl;
+	unsigned long long clks;
+
+	dev_dbg(mmc_dev(host->mmc), "blksz %d blks %d flags %08x\n",
+		data->blksz, data->blocks, data->flags);
+
+	STAT_INC(host->stat.n_datareq);
+	host->data = data;
+	host->size = data->blksz * data->blocks;
+	data->bytes_xfered = 0;
+
+	clks = (unsigned long long)data->timeout_ns * host->sdmmc_ck;
+	do_div(clks, NSEC_PER_SEC);
+	timeout = data->timeout_clks + (unsigned int)clks;
+
+	writel_relaxed(timeout, host->base + SDMMC_DTIMER);
+	writel_relaxed(host->size, host->base + SDMMC_DLENR);
+
+	datactrl = FIELD_PREP(DCTRLR_DBLOCKSIZE_MASK, ilog2(data->blksz));
+
+	if (data->flags & MMC_DATA_READ) {
+		datactrl |= DCTRLR_DTDIR;
+		imask = MASKR_RXOVERRIE;
+	} else {
+		imask = MASKR_TXUNDERRIE;
+	}
+
+	if (host->mmc->card && mmc_card_sdio(host->mmc->card))
+		datactrl |= DCTRLR_SDIOEN | DCTRLR_DTMODE_SDIO;
+
+	idmactrl = IDMACTRLR_IDMAEN;
+
+	writel_relaxed(sg_dma_address(data->sg),
+		       host->base + SDMMC_IDMABASE0R);
+	writel_relaxed(idmactrl, host->base + SDMMC_IDMACTRLR);
+
+	imask |= MASKR_DATAENDIE | MASKR_DTIMEOUTIE | MASKR_DCRCFAILIE;
+	enable_imask(host, imask);
+
+	writel_relaxed(datactrl, host->base + SDMMC_DCTRLR);
+}
+
+static void stm32_sdmmc_start_cmd(struct sdmmc_host *host,
+				  struct mmc_command *cmd, u32 c)
+{
+	u32 imsk;
+
+	dev_dbg(mmc_dev(host->mmc), "op %u arg %08x flags %08x\n",
+		cmd->opcode, cmd->arg, cmd->flags);
+
+	STAT_INC(host->stat.n_req);
+
+	if (readl_relaxed(host->base + SDMMC_CMDR) & CMDR_CPSMEM)
+		writel_relaxed(0, host->base + SDMMC_CMDR);
+
+	c |= cmd->opcode | CMDR_CPSMEM;
+	if (cmd->flags & MMC_RSP_PRESENT) {
+		imsk = MASKR_CMDRENDIE | MASKR_CTIMEOUTIE;
+		if (cmd->flags & MMC_RSP_CRC)
+			imsk |= MASKR_CCRCFAILIE;
+
+		if (cmd->flags & MMC_RSP_136)
+			c |= CMDR_WAITRESP_LRSP_CRC;
+		else if (cmd->flags & MMC_RSP_CRC)
+			c |= CMDR_WAITRESP_SRSP_CRC;
+		else
+			c |= CMDR_WAITRESP_SRSP;
+	} else {
+		c &= ~CMDR_WAITRESP_MASK;
+		imsk = MASKR_CMDSENTIE;
+	}
+
+	host->cmd = cmd;
+
+	enable_imask(host, imsk);
+
+	writel_relaxed(cmd->arg, host->base + SDMMC_ARGR);
+	writel_relaxed(c, host->base + SDMMC_CMDR);
+}
+
+static void stm32_sdmmc_cmd_irq(struct sdmmc_host *host, u32 status)
+{
+	struct mmc_command *cmd = host->cmd;
+
+	if (!cmd)
+		return;
+
+	host->cmd = NULL;
+
+	if (status & STAR_CTIMEOUT) {
+		STAT_INC(host->stat.n_ctimeout);
+		cmd->error = -ETIMEDOUT;
+		host->dpsm_abort = true;
+	} else if (status & STAR_CCRCFAIL && cmd->flags & MMC_RSP_CRC) {
+		STAT_INC(host->stat.n_ccrcfail);
+		cmd->error = -EILSEQ;
+		host->dpsm_abort = true;
+	} else if (status & STAR_CMDREND && cmd->flags & MMC_RSP_PRESENT) {
+		cmd->resp[0] = readl_relaxed(host->base + SDMMC_RESP1R);
+		cmd->resp[1] = readl_relaxed(host->base + SDMMC_RESP2R);
+		cmd->resp[2] = readl_relaxed(host->base + SDMMC_RESP3R);
+		cmd->resp[3] = readl_relaxed(host->base + SDMMC_RESP4R);
+	}
+
+	if (!host->data)
+		stm32_sdmmc_request_end(host, host->mrq);
+}
+
+static void stm32_sdmmc_data_irq(struct sdmmc_host *host, u32 status)
+{
+	struct mmc_data	*data = host->data;
+	struct mmc_command *stop = &host->stop_abort;
+
+	if (!data)
+		return;
+
+	if (status & STAR_DCRCFAIL) {
+		STAT_INC(host->stat.n_dcrcfail);
+		data->error = -EILSEQ;
+		if (readl_relaxed(host->base + SDMMC_DCNTR))
+			host->dpsm_abort = true;
+	} else if (status & STAR_DTIMEOUT) {
+		STAT_INC(host->stat.n_dtimeout);
+		data->error = -ETIMEDOUT;
+		host->dpsm_abort = true;
+	} else if (status & STAR_TXUNDERR) {
+		STAT_INC(host->stat.n_txunderrun);
+		data->error = -EIO;
+		host->dpsm_abort = true;
+	} else if (status & STAR_RXOVERR) {
+		STAT_INC(host->stat.n_rxoverrun);
+		data->error = -EIO;
+		host->dpsm_abort = true;
+	}
+
+	if (status & STAR_DATAEND || data->error || host->dpsm_abort) {
+		host->data = NULL;
+
+		writel_relaxed(0, host->base + SDMMC_IDMACTRLR);
+
+		if (!data->error)
+			data->bytes_xfered = data->blocks * data->blksz;
+
+		/*
+		 * To stop Data Path State Machine, a stop_transmission command
+		 * shall be send on cmd or data errors of single, multi,
+		 * pre-defined block and stream request.
+		 */
+		if (host->dpsm_abort && !data->stop) {
+			memset(stop, 0, sizeof(struct mmc_command));
+			stop->opcode = MMC_STOP_TRANSMISSION;
+			stop->arg = 0;
+			stop->flags = MMC_RSP_R1B | MMC_CMD_AC;
+			data->stop = stop;
+		}
+
+		disable_imask(host, MASKR_RXOVERRIE | MASKR_TXUNDERRIE
+			      | MASKR_DCRCFAILIE | MASKR_DATAENDIE
+			      | MASKR_DTIMEOUTIE);
+
+		if (!data->stop)
+			stm32_sdmmc_request_end(host, data->mrq);
+		else
+			stm32_sdmmc_start_cmd(host, data->stop, CMDR_CMDSTOP);
+	}
+}
+
+static irqreturn_t stm32_sdmmc_irq(int irq, void *dev_id)
+{
+	struct sdmmc_host *host = dev_id;
+	u32 status;
+
+	spin_lock(&host->lock);
+
+	status = readl_relaxed(host->base + SDMMC_STAR);
+	dev_dbg(mmc_dev(host->mmc), "irq sta:%#x\n", status);
+	writel_relaxed(status & ICR_STATIC_FLAG, host->base + SDMMC_ICR);
+
+	stm32_sdmmc_cmd_irq(host, status);
+	stm32_sdmmc_data_irq(host, status);
+
+	spin_unlock(&host->lock);
+
+	return IRQ_HANDLED;
+}
+
+static void stm32_sdmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	struct sdmmc_host *host = mmc_priv(mmc);
+	struct mmc_data *data = mrq->data;
+
+	if (!data)
+		return;
+
+	/* This data might be unmapped at this time */
+	data->host_cookie = COOKIE_UNMAPPED;
+
+	if (!stm32_sdmmc_validate_data(host, mrq->data, COOKIE_PRE_MAPPED))
+		data->host_cookie = COOKIE_UNMAPPED;
+}
+
+static void stm32_sdmmc_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
+				 int err)
+{
+	struct sdmmc_host *host = mmc_priv(mmc);
+	struct mmc_data *data = mrq->data;
+
+	if (!data)
+		return;
+
+	if (data->host_cookie != COOKIE_UNMAPPED)
+		dma_unmap_sg(mmc_dev(host->mmc),
+			     data->sg,
+			     data->sg_len,
+			     mmc_get_dma_dir(data));
+
+	data->host_cookie = COOKIE_UNMAPPED;
+}
+
+static void stm32_sdmmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	unsigned int cmdat = 0;
+	struct sdmmc_host *host = mmc_priv(mmc);
+	unsigned long flags;
+
+	mrq->cmd->error = stm32_sdmmc_validate_data(host, mrq->data,
+						    COOKIE_MAPPED);
+	if (mrq->cmd->error) {
+		mmc_request_done(mmc, mrq);
+		return;
+	}
+
+	spin_lock_irqsave(&host->lock, flags);
+
+	host->mrq = mrq;
+
+	if (mrq->data) {
+		host->dpsm_abort = false;
+		stm32_sdmmc_start_data(host, mrq->data);
+		cmdat |= CMDR_CMDTRANS;
+	}
+
+	stm32_sdmmc_start_cmd(host, mrq->cmd, cmdat);
+
+	spin_unlock_irqrestore(&host->lock, flags);
+}
+
+static struct mmc_host_ops stm32_sdmmc_ops = {
+	.request	= stm32_sdmmc_request,
+	.pre_req	= stm32_sdmmc_pre_req,
+	.post_req	= stm32_sdmmc_post_req,
+	.set_ios	= stm32_sdmmc_set_ios,
+	.get_cd		= mmc_gpio_get_cd,
+	.card_busy	= stm32_sdmmc_card_busy,
+};
+
+static const struct of_device_id stm32_sdmmc_match[] = {
+	{ .compatible = "st,stm32h7-sdmmc",},
+	{},
+};
+MODULE_DEVICE_TABLE(of, stm32_sdmmc_match);
+
+static int stm32_sdmmc_of_parse(struct mmc_host *mmc)
+{
+	struct sdmmc_host *host = mmc_priv(mmc);
+	int ret = mmc_of_parse(mmc);
+
+	if (ret)
+		return ret;
+
+	if (device_property_present(mmc_dev(mmc),"st,neg-edge"))
+		host->clk_reg_add |= CLKCR_NEGEDGE;
+	if (device_property_present(mmc_dev(mmc), "st,dir-output-high"))
+		host->pwr_reg_add |= POWER_DIRPOL;
+	if (device_property_present(mmc_dev(mmc), "st,use-ckin"))
+		host->clk_reg_add |= CLKCR_SELCLKRX_CKIN;
+
+	return 0;
+}
+
+static int stm32_sdmmc_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct sdmmc_host *host;
+	struct mmc_host *mmc;
+	struct resource *res;
+	int irq, ret;
+
+	if (!np) {
+		dev_err(&pdev->dev, "No DT found\n");
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return -EINVAL;
+
+	mmc = mmc_alloc_host(sizeof(struct sdmmc_host), &pdev->dev);
+	if (!mmc)
+		return -ENOMEM;
+
+	host = mmc_priv(mmc);
+	host->mmc = mmc;
+	platform_set_drvdata(pdev, mmc);
+
+	host->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(host->base)) {
+		ret = PTR_ERR(host->base);
+		goto host_free;
+	}
+
+	writel_relaxed(0, host->base + SDMMC_MASKR);
+	writel_relaxed(~0UL, host->base + SDMMC_ICR);
+
+	ret = devm_request_irq(&pdev->dev, irq, stm32_sdmmc_irq, IRQF_SHARED,
+			       DRIVER_NAME " (cmd)", host);
+	if (ret)
+		goto host_free;
+
+	host->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(host->clk)) {
+		ret = PTR_ERR(host->clk);
+		goto host_free;
+	}
+
+	ret = clk_prepare_enable(host->clk);
+	if (ret)
+		goto host_free;
+
+	host->sdmmcclk = clk_get_rate(host->clk);
+	mmc->f_min = DIV_ROUND_UP(host->sdmmcclk, 2 * CLKCR_CLKDIV_MAX);
+	mmc->f_max = host->sdmmcclk;
+
+	ret = stm32_sdmmc_of_parse(mmc);
+	if (ret)
+		goto clk_disable;
+
+	host->rst = devm_reset_control_get(&pdev->dev, NULL);
+	if (IS_ERR(host->rst)) {
+		ret = PTR_ERR(host->rst);
+		goto clk_disable;
+	}
+
+	stm32_sdmmc_pwroff(host);
+
+	/* Get regulators and the supported OCR mask */
+	ret = mmc_regulator_get_supply(mmc);
+	if (ret == -EPROBE_DEFER)
+		goto clk_disable;
+
+	if (!mmc->ocr_avail)
+		mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+
+	mmc->ops = &stm32_sdmmc_ops;
+
+	/* IDMA cannot do scatter lists */
+	mmc->max_segs = 1;
+	mmc->max_req_size = DLENR_DATALENGHT_MAX;
+	mmc->max_seg_size = mmc->max_req_size;
+	mmc->max_blk_size = 1 << DCTRLR_DBLOCKSIZE_MAX;
+
+	/*
+	 * Limit the number of blocks transferred so that we don't overflow
+	 * the maximum request size.
+	 */
+	mmc->max_blk_count = mmc->max_req_size >> DCTRLR_DBLOCKSIZE_MAX;
+
+	spin_lock_init(&host->lock);
+
+	ret = mmc_add_host(mmc);
+	if (ret)
+		goto clk_disable;
+
+	stm32_sdmmc_stat_init(host);
+
+	host->ip_ver = readl_relaxed(host->base + SDMMC_IPVR);
+	dev_info(&pdev->dev, "%s: rev:%ld.%ld irq:%d\n",
+		 mmc_hostname(mmc),
+		 FIELD_GET(IPVR_MAJREV_MASK, host->ip_ver),
+		 FIELD_GET(IPVR_MINREV_MASK, host->ip_ver), irq);
+
+	return 0;
+
+clk_disable:
+	clk_disable_unprepare(host->clk);
+host_free:
+	mmc_free_host(mmc);
+	return ret;
+}
+
+static int stm32_sdmmc_remove(struct platform_device *pdev)
+{
+	struct mmc_host *mmc = platform_get_drvdata(pdev);
+	struct sdmmc_host *host = mmc_priv(mmc);
+
+	/* Debugfs stuff is cleaned up by mmc core */
+	mmc_remove_host(mmc);
+	clk_disable_unprepare(host->clk);
+	mmc_free_host(mmc);
+
+	return 0;
+}
+
+static struct platform_driver stm32_sdmmc_driver = {
+	.probe		= stm32_sdmmc_probe,
+	.remove		= stm32_sdmmc_remove,
+	.driver	= {
+		.name	= DRIVER_NAME,
+		.of_match_table = stm32_sdmmc_match,
+	},
+};
+
+module_platform_driver(stm32_sdmmc_driver);
+
+MODULE_DESCRIPTION("STMicroelectronics STM32 MMC/SD Card Interface driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Ludovic Barre <ludovic.barre@st.com>");
diff --git a/drivers/mmc/host/stm32-sdmmc.h b/drivers/mmc/host/stm32-sdmmc.h
new file mode 100644
index 0000000..e39578e
--- /dev/null
+++ b/drivers/mmc/host/stm32-sdmmc.h
@@ -0,0 +1,220 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
+ * Author: Ludovic Barre <ludovic.barre@st.com> for STMicroelectronics.
+ */
+#define SDMMC_POWER			0x000
+#define POWERCTRL_MASK			GENMASK(1, 0)
+#define POWERCTRL_OFF			0x00
+#define POWERCTRL_CYC			0x02
+#define POWERCTRL_ON			0x03
+#define POWER_VSWITCH			BIT(2)
+#define POWER_VSWITCHEN			BIT(3)
+#define POWER_DIRPOL			BIT(4)
+
+#define SDMMC_CLKCR			0x004
+#define CLKCR_CLKDIV_MASK		GENMASK(9, 0)
+#define CLKCR_CLKDIV_MAX		CLKCR_CLKDIV_MASK
+#define CLKCR_PWRSAV			BIT(12)
+#define CLKCR_WIDBUS_4			BIT(14)
+#define CLKCR_WIDBUS_8			BIT(15)
+#define CLKCR_NEGEDGE			BIT(16)
+#define CLKCR_HWFC_EN			BIT(17)
+#define CLKCR_DDR			BIT(18)
+#define CLKCR_BUSSPEED			BIT(19)
+#define CLKCR_SELCLKRX_MASK		GENMASK(21, 20)
+#define CLKCR_SELCLKRX_CK		(0 << 20)
+#define CLKCR_SELCLKRX_CKIN		(1 << 20)
+#define CLKCR_SELCLKRX_FBCK		(2 << 20)
+
+#define SDMMC_ARGR			0x008
+
+#define SDMMC_CMDR			0x00c
+#define CMDR_CMDTRANS			BIT(6)
+#define CMDR_CMDSTOP			BIT(7)
+#define CMDR_WAITRESP_MASK		GENMASK(9, 8)
+#define CMDR_WAITRESP_NORSP		(0 << 8)
+#define CMDR_WAITRESP_SRSP_CRC		(1 << 8)
+#define CMDR_WAITRESP_SRSP		(2 << 8)
+#define CMDR_WAITRESP_LRSP_CRC		(3 << 8)
+#define CMDR_WAITINT			BIT(10)
+#define CMDR_WAITPEND			BIT(11)
+#define CMDR_CPSMEM			BIT(12)
+#define CMDR_DTHOLD			BIT(13)
+#define CMDR_BOOTMODE			BIT(14)
+#define CMDR_BOOTEN			BIT(15)
+#define CMDR_CMDSUSPEND			BIT(16)
+
+#define SDMMC_RESPCMDR			0x010
+#define SDMMC_RESP1R			0x014
+#define SDMMC_RESP2R			0x018
+#define SDMMC_RESP3R			0x01c
+#define SDMMC_RESP4R			0x020
+
+#define SDMMC_DTIMER			0x024
+
+#define SDMMC_DLENR			0x028
+#define DLENR_DATALENGHT_MASK		GENMASK(24, 0)
+#define DLENR_DATALENGHT_MAX		DLENR_DATALENGHT_MASK
+
+#define SDMMC_DCTRLR			0x02c
+#define DCTRLR_DTEN			BIT(0)
+#define DCTRLR_DTDIR			BIT(1)
+#define DCTRLR_DTMODE_MASK		GENMASK(3, 2)
+#define DCTRLR_DTMODE_BLOCK		(0 << 2)
+#define DCTRLR_DTMODE_SDIO		(1 << 2)
+#define DCTRLR_DTMODE_MMC		(2 << 2)
+#define DCTRLR_DBLOCKSIZE_MASK		GENMASK(7, 4)
+#define DCTRLR_DBLOCKSIZE_MAX		14
+#define DCTRLR_RWSTART			BIT(8)
+#define DCTRLR_RWSTOP			BIT(9)
+#define DCTRLR_RWMOD			BIT(10)
+#define DCTRLR_SDIOEN			BIT(11)
+#define DCTRLR_BOOTACKEN		BIT(12)
+#define DCTRLR_FIFORST			BIT(13)
+
+#define SDMMC_DCNTR			0x030
+
+#define SDMMC_STAR			0x034
+#define STAR_CCRCFAIL			BIT(0)
+#define STAR_DCRCFAIL			BIT(1)
+#define STAR_CTIMEOUT			BIT(2)
+#define STAR_DTIMEOUT			BIT(3)
+#define STAR_TXUNDERR			BIT(4)
+#define STAR_RXOVERR			BIT(5)
+#define STAR_CMDREND			BIT(6)
+#define STAR_CMDSENT			BIT(7)
+#define STAR_DATAEND			BIT(8)
+#define STAR_DHOLD			BIT(9)
+#define STAR_DBCKEND			BIT(10)
+#define STAR_DABORT			BIT(11)
+#define STAR_DPSMACT			BIT(12)
+#define STAR_CPSMACT			BIT(13)
+#define STAR_TXFIFOHE			BIT(14)
+#define STAR_TXFIFOHF			BIT(15)
+#define STAR_TXFIFOF			BIT(16)
+#define STAR_RXFIFOF			BIT(17)
+#define STAR_TXFIFOE			BIT(18)
+#define STAR_RXFIFOE			BIT(19)
+#define STAR_BUSYD0			BIT(20)
+#define STAR_BUSYD0END			BIT(21)
+#define STAR_SDIOIT			BIT(22)
+#define STAR_ACKFAIL			BIT(23)
+#define STAR_ACKTIMEOUT			BIT(24)
+#define STAR_VSWEND			BIT(25)
+#define STAR_CKSTOP			BIT(26)
+#define STAR_IDMATE			BIT(27)
+#define STAR_IDMABTC			BIT(28)
+
+#define SDMMC_ICR			0x038
+#define ICR_CCRCFAILC			BIT(0)
+#define ICR_DCRCFAILC			BIT(1)
+#define ICR_CTIMEOUTC			BIT(2)
+#define ICR_DTIMEOUTC			BIT(3)
+#define ICR_TXUNDERRC			BIT(4)
+#define ICR_RXOVERRC			BIT(5)
+#define ICR_CMDRENDC			BIT(6)
+#define ICR_CMDSENTC			BIT(7)
+#define ICR_DATAENDC			BIT(8)
+#define ICR_DHOLDC			BIT(9)
+#define ICR_DBCKENDC			BIT(10)
+#define ICR_DABORTC			BIT(11)
+#define ICR_BUSYD0ENDC			BIT(21)
+#define ICR_SDIOITC			BIT(22)
+#define ICR_ACKFAILC			BIT(23)
+#define ICR_ACKTIMEOUTC			BIT(24)
+#define ICR_VSWENDC			BIT(25)
+#define ICR_CKSTOPC			BIT(26)
+#define ICR_IDMATEC			BIT(27)
+#define ICR_IDMABTCC			BIT(28)
+#define ICR_STATIC_FLAG			((GENMASK(28, 21)) | (GENMASK(11, 0)))
+
+#define SDMMC_MASKR			0x03c
+#define MASKR_CCRCFAILIE		BIT(0)
+#define MASKR_DCRCFAILIE		BIT(1)
+#define MASKR_CTIMEOUTIE		BIT(2)
+#define MASKR_DTIMEOUTIE		BIT(3)
+#define MASKR_TXUNDERRIE		BIT(4)
+#define MASKR_RXOVERRIE			BIT(5)
+#define MASKR_CMDRENDIE			BIT(6)
+#define MASKR_CMDSENTIE			BIT(7)
+#define MASKR_DATAENDIE			BIT(8)
+#define MASKR_DHOLDIE			BIT(9)
+#define MASKR_DBCKENDIE			BIT(10)
+#define MASKR_DABORTIE			BIT(11)
+#define MASKR_TXFIFOHEIE		BIT(14)
+#define MASKR_RXFIFOHFIE		BIT(15)
+#define MASKR_RXFIFOFIE			BIT(17)
+#define MASKR_TXFIFOEIE			BIT(18)
+#define MASKR_BUSYD0ENDIE		BIT(21)
+#define MASKR_SDIOITIE			BIT(22)
+#define MASKR_ACKFAILIE			BIT(23)
+#define MASKR_ACKTIMEOUTIE		BIT(24)
+#define MASKR_VSWENDIE			BIT(25)
+#define MASKR_CKSTOPIE			BIT(26)
+#define MASKR_IDMABTCIE			BIT(28)
+
+#define SDMMC_ACKTIMER			0x040
+#define ACKTIMER_ACKTIME_MASK		GENMASK(24, 0)
+
+#define SDMMC_FIFOR			0x080
+
+#define SDMMC_IDMACTRLR			0x050
+#define IDMACTRLR_IDMAEN		BIT(0)
+#define IDMACTRLR_IDMABMODE		BIT(1)
+#define IDMACTRLR_IDMABACT		BIT(2)
+
+#define SDMMC_IDMABSIZER		0x054
+#define IDMABSIZER_IDMABNDT_MASK	GENMASK(12, 5)
+
+#define SDMMC_IDMABASE0R		0x058
+#define SDMMC_IDMABASE1R		0x05c
+
+#define SDMMC_IPVR			0x3fc
+#define IPVR_MINREV_MASK		GENMASK(3, 0)
+#define IPVR_MAJREV_MASK		GENMASK(7, 4)
+
+enum stm32_sdmmc_cookie {
+	COOKIE_UNMAPPED,
+	COOKIE_PRE_MAPPED,	/* mapped by pre_req() of stm32 */
+	COOKIE_MAPPED,		/* mapped by prepare_data() of stm32 */
+};
+
+struct sdmmc_stat {
+	unsigned long		n_req;
+	unsigned long		n_datareq;
+	unsigned long		n_ctimeout;
+	unsigned long		n_ccrcfail;
+	unsigned long		n_dtimeout;
+	unsigned long		n_dcrcfail;
+	unsigned long		n_txunderrun;
+	unsigned long		n_rxoverrun;
+	unsigned long		nb_dma_err;
+};
+
+struct sdmmc_host {
+	void __iomem		*base;
+	struct mmc_host		*mmc;
+	struct clk		*clk;
+	struct reset_control	*rst;
+
+	u32			clk_reg_add;
+	u32			pwr_reg_add;
+
+	struct mmc_request	*mrq;
+	struct mmc_command	*cmd;
+	struct mmc_data		*data;
+	struct mmc_command	stop_abort;
+	bool			dpsm_abort;
+
+	/* protect host registers access */
+	spinlock_t		lock;
+
+	unsigned int		sdmmcclk;
+	unsigned int		sdmmc_ck;
+
+	u32			size;
+
+	u32			ip_ver;
+	struct sdmmc_stat	stat;
+};
-- 
2.7.4

[PATCH V2 3/5] ARM: dts: stm32: add sdmmc support for stm32h743

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds sdmmc support for stm32h743.
2×SD/SDIO/MMC interfaces (up to 125 MHz)

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 arch/arm/boot/dts/stm32h743.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/stm32h743.dtsi b/arch/arm/boot/dts/stm32h743.dtsi
index bbfcbac..5e85538 100644
--- a/arch/arm/boot/dts/stm32h743.dtsi
+++ b/arch/arm/boot/dts/stm32h743.dtsi
@@ -217,6 +217,19 @@
 			};
 		};
 
+		sdmmc2: sdmmc@48022400 {
+			compatible = "st,stm32h7-sdmmc";
+			reg = <0x48022400 0x400>;
+			reg-names = "sdmmc";
+			interrupts = <124>;
+			clocks = <&rcc SDMMC2_CK>;
+			resets = <&rcc STM32H7_AHB2_RESET(SDMMC2)>;
+			cap-sd-highspeed;
+			cap-mmc-highspeed;
+			max-frequency = <125000000>;
+			status = "disabled";
+		};
+
 		mdma1: dma@52000000 {
 			compatible = "st,stm32h7-mdma";
 			reg = <0x52000000 0x1000>;
@@ -227,6 +240,19 @@
 			dma-requests = <32>;
 		};
 
+		sdmmc1: sdmmc@52007000 {
+			compatible = "st,stm32h7-sdmmc";
+			reg = <0x52007000 0x1000>;
+			reg-names = "sdmmc";
+			interrupts = <49>;
+			clocks = <&rcc SDMMC1_CK>;
+			resets = <&rcc STM32H7_AHB3_RESET(SDMMC1)>;
+			cap-sd-highspeed;
+			cap-mmc-highspeed;
+			max-frequency = <125000000>;
+			status = "disabled";
+		};
+
 		lptimer2: timer@58002400 {
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
2.7.4

Re: [PATCH V2 3/5] ARM: dts: stm32: add sdmmc support for stm32h743

[Rob Herring]

On Wed, Feb 28, 2018 at 04:47:22PM +0100, Ludovic Barre wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
> 
> This patch adds sdmmc support for stm32h743.
> 2×SD/SDIO/MMC interfaces (up to 125 MHz)
> 
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
>  arch/arm/boot/dts/stm32h743.dtsi | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/stm32h743.dtsi b/arch/arm/boot/dts/stm32h743.dtsi
> index bbfcbac..5e85538 100644
> --- a/arch/arm/boot/dts/stm32h743.dtsi
> +++ b/arch/arm/boot/dts/stm32h743.dtsi
> @@ -217,6 +217,19 @@
>  			};
>  		};
>  
> +		sdmmc2: sdmmc@48022400 {

Like I said in the binding example:

mmc@...

> +			compatible = "st,stm32h7-sdmmc";
> +			reg = <0x48022400 0x400>;
> +			reg-names = "sdmmc";

Not in the binding definition (And don't add it, -names for a single 
item is pointless).

> +			interrupts = <124>;
> +			clocks = <&rcc SDMMC2_CK>;
> +			resets = <&rcc STM32H7_AHB2_RESET(SDMMC2)>;
> +			cap-sd-highspeed;
> +			cap-mmc-highspeed;
> +			max-frequency = <125000000>;
> +			status = "disabled";
> +		};
> +
>  		mdma1: dma@52000000 {
>  			compatible = "st,stm32h7-mdma";
>  			reg = <0x52000000 0x1000>;
> @@ -227,6 +240,19 @@
>  			dma-requests = <32>;
>  		};
>  
> +		sdmmc1: sdmmc@52007000 {
> +			compatible = "st,stm32h7-sdmmc";
> +			reg = <0x52007000 0x1000>;
> +			reg-names = "sdmmc";
> +			interrupts = <49>;
> +			clocks = <&rcc SDMMC1_CK>;
> +			resets = <&rcc STM32H7_AHB3_RESET(SDMMC1)>;
> +			cap-sd-highspeed;
> +			cap-mmc-highspeed;
> +			max-frequency = <125000000>;
> +			status = "disabled";
> +		};
> +
>  		lptimer2: timer@58002400 {
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> -- 
> 2.7.4
> 

[PATCH V2 4/5] ARM: dts: stm32: add sdmmc1 support for stm32h743i-eval

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds support of micro sd card connected to sdmmc1.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 arch/arm/boot/dts/stm32h743-pinctrl.dtsi | 26 ++++++++++++++++++++++++++
 arch/arm/boot/dts/stm32h743i-eval.dts    | 11 +++++++++++
 2 files changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/stm32h743-pinctrl.dtsi b/arch/arm/boot/dts/stm32h743-pinctrl.dtsi
index 65c1cd0..ef17192 100644
--- a/arch/arm/boot/dts/stm32h743-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stm32h743-pinctrl.dtsi
@@ -164,6 +164,32 @@
 					bias-disable;
 				};
 			};
+
+			sdmmc1_b4_pins_a: sdmmc1-b4@0 {
+				pins {
+					pinmux = <STM32_PINMUX('C', 8, AF12)>, /* SDMMC1_D0 */
+						 <STM32_PINMUX('C', 9, AF12)>, /* SDMMC1_D1 */
+						 <STM32_PINMUX('C', 10, AF12)>, /* SDMMC1_D2 */
+						 <STM32_PINMUX('C', 11, AF12)>, /* SDMMC1_D3 */
+						 <STM32_PINMUX('C', 12, AF12)>, /* SDMMC1_CK */
+						 <STM32_PINMUX('D', 2, AF12)>; /* SDMMC1_CMD */
+					slew-rate = <3>;
+					drive-push-pull;
+					bias-disable;
+				};
+			};
+
+			sdmmc1_dir_pins_a: sdmmc1-dir@0 {
+				pins {
+					pinmux = <STM32_PINMUX('C', 6, AF8)>, /* SDMMC1_D0DIR */
+						 <STM32_PINMUX('C', 7, AF8)>, /* SDMMC1_D123DIR */
+						 <STM32_PINMUX('B', 9, AF7)>, /* SDMMC1_CDIR */
+						 <STM32_PINMUX('B', 8, AF7)>; /* SDMMC1_CKIN */
+					slew-rate = <3>;
+					drive-push-pull;
+					bias-pull-up;
+				};
+			};
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/stm32h743i-eval.dts b/arch/arm/boot/dts/stm32h743i-eval.dts
index c6ceda8..e1dbfaf 100644
--- a/arch/arm/boot/dts/stm32h743i-eval.dts
+++ b/arch/arm/boot/dts/stm32h743i-eval.dts
@@ -98,6 +98,17 @@
 	clock-frequency = <25000000>;
 };
 
+&sdmmc1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc1_b4_pins_a &sdmmc1_dir_pins_a>;
+	broken-cd;
+	st,dir-output-high;
+	st,neg-edge;
+	st,use-ckin;
+	bus-width = <4>;
+	status = "okay";
+};
+
 &usart1 {
 	pinctrl-0 = <&usart1_pins>;
 	pinctrl-names = "default";
-- 
2.7.4

[PATCH V2 5/5] ARM: configs: stm32: add mmc and ext2/3/4 support

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds support of
-mmc framework and stm32 sdmmc controller
-ext2/3/4 filesystem
-LBDAF to support huge file (needed for ext4)

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 arch/arm/configs/stm32_defconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
index 91014ea..2573355 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -15,7 +15,6 @@ CONFIG_EMBEDDED=y
 # CONFIG_VM_EVENT_COUNTERS is not set
 # CONFIG_SLUB_DEBUG is not set
 CONFIG_MODULES=y
-# CONFIG_LBDAF is not set
 # CONFIG_BLK_DEV_BSG is not set
 # CONFIG_IOSCHED_DEADLINE is not set
 # CONFIG_IOSCHED_CFQ is not set
@@ -57,6 +56,8 @@ CONFIG_MFD_STMPE=y
 CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 # CONFIG_USB_SUPPORT is not set
+CONFIG_MMC=y
+CONFIG_MMC_STM32_SDMMC=y
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
 CONFIG_LEDS_GPIO=y
@@ -71,6 +72,7 @@ CONFIG_STM32_MDMA=y
 CONFIG_IIO=y
 CONFIG_STM32_ADC_CORE=y
 CONFIG_STM32_ADC=y
+CONFIG_EXT3_FS=y
 # CONFIG_FILE_LOCKING is not set
 # CONFIG_DNOTIFY is not set
 # CONFIG_INOTIFY_USER is not set
-- 
2.7.4

Re: [PATCH V2 0/5] mmc: add stm32 sdmmc controller

[Ulf Hansson]

Hi Ludovic,

On 28 February 2018 at 16:47, Ludovic Barre <ludovic.Barre@st.com> wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> This patch serie adds support of stm32 SDMMC controller.
> stm32h7 is the first SoC to use stm32 SDMMC controller
> (previous SoC had pl180 controller).

I am a not convinced this isn't a new improved variant of the pl180.
According to register layout and the code you submitted in patch2,
there are great similarities to pl180 and the mmci driver.

In other words, it more sounds like an improved variant of pl180,
hence I am thinking that you should try to extend the mmci driver
instead. Could you please have a look at that, unless you can specify
more exactly why that isn't suitable?

>
> The SDMMC features 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 compliance with SD memory card specifications version 4.1.
>  SDR104 speed limited to maximum allowed I/O speed, SPI mode and
>  UHS-II mode not supported.
> -Full compliance with SDIO card specification version 4.0.
>
> change V2:
> -dt-bindings: properties name
> st,dir-output-high; st,neg-edge; st,use-ckin
> -dt-bindings: node example s/sdmmc@/mmc@/g
> -removes return of enable/disable_imask functions
> -adds dev_warn if clk divisor is too short.
> -replaces overprotection of n_elem by fail of dma_map_sg
> -removes shortcut host->base
> -Use device_property_present
>
> Ludovic Barre (5):
>   dt-bindings: mmc: document the stm32 sdmmc bindings
>   mmc: add stm32 sdmmc controller driver
>   ARM: dts: stm32: add sdmmc support for stm32h743
>   ARM: dts: stm32: add sdmmc1 support for stm32h743i-eval
>   ARM: configs: stm32: add mmc and ext2/3/4 support
>
>  .../devicetree/bindings/mmc/st,stm32-sdmmc.txt     |  35 +
>  arch/arm/boot/dts/stm32h743-pinctrl.dtsi           |  26 +
>  arch/arm/boot/dts/stm32h743.dtsi                   |  26 +
>  arch/arm/boot/dts/stm32h743i-eval.dts              |  11 +
>  arch/arm/configs/stm32_defconfig                   |   4 +-
>  drivers/mmc/host/Kconfig                           |   8 +
>  drivers/mmc/host/Makefile                          |   1 +
>  drivers/mmc/host/stm32-sdmmc.c                     | 708 +++++++++++++++++++++
>  drivers/mmc/host/stm32-sdmmc.h                     | 220 +++++++
>  9 files changed, 1038 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/st,stm32-sdmmc.txt
>  create mode 100644 drivers/mmc/host/stm32-sdmmc.c
>  create mode 100644 drivers/mmc/host/stm32-sdmmc.h
>
> --
> 2.7.4
>

Kind regards
Uffe

Re: [PATCH V2 0/5] mmc: add stm32 sdmmc controller

[Ludovic BARRE]

Hi Ulf

On 03/01/2018 10:06 AM, Ulf Hansson wrote:
> Hi Ludovic,
> 
> On 28 February 2018 at 16:47, Ludovic Barre <ludovic.Barre@st.com> wrote:
>> From: Ludovic Barre <ludovic.barre@st.com>
>>
>> This patch serie adds support of stm32 SDMMC controller.
>> stm32h7 is the first SoC to use stm32 SDMMC controller
>> (previous SoC had pl180 controller).
> 
> I am a not convinced this isn't a new improved variant of the pl180.
> According to register layout and the code you submitted in patch2,
> there are great similarities to pl180 and the mmci driver.

In fact, ST designers which created stm32-sdmmc hardware block from 
scratch are the same which have done the modifications on pl180 variant 
(stm32 legacy f4, f7).
So some registers or bits name seem identical (or strongly inspirited) 
but the engine and features are different.

You could find the datasheet of STM32H7x3 on:
http://www.st.com/content/ccc/resource/technical/document/reference_manual/group0/c9/a3/76/fa/55/46/45/fa/DM00314099/files/DM00314099.pdf/jcr:content/translations/en.DM00314099.pdf

Chapters: 55 Secure digital input/output MultiMediaCard interface
(SDMMC)

This hardware block has own roadmap and some features are already in the 
pipe for next SoC.

For code design: like I also worked on pl180 in the past :-)
my code is inspirited of this driver.

BR
Ludo

> 
> In other words, it more sounds like an improved variant of pl180,
> hence I am thinking that you should try to extend the mmci driver
> instead. Could you please have a look at that, unless you can specify
> more exactly why that isn't suitable?
> 
>>
>> The SDMMC features 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 compliance with SD memory card specifications version 4.1.
>>   SDR104 speed limited to maximum allowed I/O speed, SPI mode and
>>   UHS-II mode not supported.
>> -Full compliance with SDIO card specification version 4.0.
>>
>> change V2:
>> -dt-bindings: properties name
>> st,dir-output-high; st,neg-edge; st,use-ckin
>> -dt-bindings: node example s/sdmmc@/mmc@/g
>> -removes return of enable/disable_imask functions
>> -adds dev_warn if clk divisor is too short.
>> -replaces overprotection of n_elem by fail of dma_map_sg
>> -removes shortcut host->base
>> -Use device_property_present
>>
>> Ludovic Barre (5):
>>    dt-bindings: mmc: document the stm32 sdmmc bindings
>>    mmc: add stm32 sdmmc controller driver
>>    ARM: dts: stm32: add sdmmc support for stm32h743
>>    ARM: dts: stm32: add sdmmc1 support for stm32h743i-eval
>>    ARM: configs: stm32: add mmc and ext2/3/4 support
>>
>>   .../devicetree/bindings/mmc/st,stm32-sdmmc.txt     |  35 +
>>   arch/arm/boot/dts/stm32h743-pinctrl.dtsi           |  26 +
>>   arch/arm/boot/dts/stm32h743.dtsi                   |  26 +
>>   arch/arm/boot/dts/stm32h743i-eval.dts              |  11 +
>>   arch/arm/configs/stm32_defconfig                   |   4 +-
>>   drivers/mmc/host/Kconfig                           |   8 +
>>   drivers/mmc/host/Makefile                          |   1 +
>>   drivers/mmc/host/stm32-sdmmc.c                     | 708 +++++++++++++++++++++
>>   drivers/mmc/host/stm32-sdmmc.h                     | 220 +++++++
>>   9 files changed, 1038 insertions(+), 1 deletion(-)
>>   create mode 100644 Documentation/devicetree/bindings/mmc/st,stm32-sdmmc.txt
>>   create mode 100644 drivers/mmc/host/stm32-sdmmc.c
>>   create mode 100644 drivers/mmc/host/stm32-sdmmc.h
>>
>> --
>> 2.7.4
>>
> 
> Kind regards
> Uffe
> 

Re: [PATCH V2 0/5] mmc: add stm32 sdmmc controller

[Ulf Hansson]

On 1 March 2018 at 10:57, Ludovic BARRE <ludovic.barre@st.com> wrote:
> Hi Ulf
>
> On 03/01/2018 10:06 AM, Ulf Hansson wrote:
>>
>> Hi Ludovic,
>>
>> On 28 February 2018 at 16:47, Ludovic Barre <ludovic.Barre@st.com> wrote:
>>>
>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>
>>> This patch serie adds support of stm32 SDMMC controller.
>>> stm32h7 is the first SoC to use stm32 SDMMC controller
>>> (previous SoC had pl180 controller).
>>
>>
>> I am a not convinced this isn't a new improved variant of the pl180.
>> According to register layout and the code you submitted in patch2,
>> there are great similarities to pl180 and the mmci driver.
>
>
> In fact, ST designers which created stm32-sdmmc hardware block from scratch
> are the same which have done the modifications on pl180 variant (stm32
> legacy f4, f7).
> So some registers or bits name seem identical (or strongly inspirited) but
> the engine and features are different.

Well, in that case, I assume the driver would also need work
differently, but when looking at the code in patch2 this doesn't seem
to be the case.

>
> You could find the datasheet of STM32H7x3 on:
> http://www.st.com/content/ccc/resource/technical/document/reference_manual/group0/c9/a3/76/fa/55/46/45/fa/DM00314099/files/DM00314099.pdf/jcr:content/translations/en.DM00314099.pdf
>
> Chapters: 55 Secure digital input/output MultiMediaCard interface
> (SDMMC)

Thanks for sharing this. However this confirms my view, it looks
exactly as a new improved mmci variant. :-)

>
> This hardware block has own roadmap and some features are already in the
> pipe for next SoC.

That's fine. I don't have a problem extending the mmci driver, even
several times, as to cope with new revisions.

>
> For code design: like I also worked on pl180 in the past :-)
> my code is inspirited of this driver.

Right, that may explain things a bit.

However, besides a re-name of the registers, I really think that the
code execution, dealing with IRQs etc, is very similar to the mmci
driver. Isn't it?

So, I think it's at least worth to give it a go with the mmci driver
first, to see if we can get it to work.

I guess you understand why I am pushing!? This is about maintenance -
and I really want to avoid having a yet another driver to maintain,
unless we can extend an existing one.

[...]

Kind regards
Uffe

Re: [PATCH V2 0/5] mmc: add stm32 sdmmc controller

[Ludovic BARRE]

On 03/01/2018 11:44 AM, Ulf Hansson wrote:
> On 1 March 2018 at 10:57, Ludovic BARRE <ludovic.barre@st.com> wrote:
>> Hi Ulf
>>
>> On 03/01/2018 10:06 AM, Ulf Hansson wrote:
>>>
>>> Hi Ludovic,
>>>
>>> On 28 February 2018 at 16:47, Ludovic Barre <ludovic.Barre@st.com> wrote:
>>>>
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> This patch serie adds support of stm32 SDMMC controller.
>>>> stm32h7 is the first SoC to use stm32 SDMMC controller
>>>> (previous SoC had pl180 controller).
>>>
>>>
>>> I am a not convinced this isn't a new improved variant of the pl180.
>>> According to register layout and the code you submitted in patch2,
>>> there are great similarities to pl180 and the mmci driver.
>>
>>
>> In fact, ST designers which created stm32-sdmmc hardware block from scratch
>> are the same which have done the modifications on pl180 variant (stm32
>> legacy f4, f7).
>> So some registers or bits name seem identical (or strongly inspirited) but
>> the engine and features are different.
> 
> Well, in that case, I assume the driver would also need work
> differently, but when looking at the code in patch2 this doesn't seem
> to be the case.
> 

I understand why you push to avoid drivers multiplication. But I'm
scared to squash 2 different hardware block which are their own roadmap
in the same drivers. I fear that it complicates the features management 
and maintenance of this driver.

there are some difference:
-the stm32-sdmmc use an internal dma (stm32-sdmmc is master on the bus), 
-idma alignment constraint.
-idma transfer mode single buffer, double buffer... (future evolution)
-need a command to stop transmission for data state machine
-same bit naming could have offset, mask width or set in different register.
=> I will try to synthesis register difference in a document
-voltage switch sequence
-delay block: calibration, tunning (sdr104)
-reset line required
-the same feature have not the same impact, example ddr mode
stm32:no bypass clk, activated in clk register
pl180: clkreg_neg_edge_enable, activated in datactrl register,
...

stm32-sdmmc is just at begining of its life cycle. Each revision of this 
block increases the difference with pl180. Today, I've just push the 
minimal driver to start a request, but I've not yet send all features of 
this revision like sdio, sdr104 support. After this revision there 
already are 2 other revisions in the pipe (at short term).

I am Out of Office with limited access to my e-mail till 2018 march 12th
I'll try to think about it on ski slope. euh, in fact no just ski and 
enjoy ;-)

BR
Ludo

>>
>> You could find the datasheet of STM32H7x3 on:
>> http://www.st.com/content/ccc/resource/technical/document/reference_manual/group0/c9/a3/76/fa/55/46/45/fa/DM00314099/files/DM00314099.pdf/jcr:content/translations/en.DM00314099.pdf
>>
>> Chapters: 55 Secure digital input/output MultiMediaCard interface
>> (SDMMC)
> 
> Thanks for sharing this. However this confirms my view, it looks
> exactly as a new improved mmci variant. :-)
> 
>>
>> This hardware block has own roadmap and some features are already in the
>> pipe for next SoC.
> 
> That's fine. I don't have a problem extending the mmci driver, even
> several times, as to cope with new revisions.
> 
>>
>> For code design: like I also worked on pl180 in the past :-)
>> my code is inspirited of this driver.
> 
> Right, that may explain things a bit.
> 
> However, besides a re-name of the registers, I really think that the
> code execution, dealing with IRQs etc, is very similar to the mmci
> driver. Isn't it?
> 
> So, I think it's at least worth to give it a go with the mmci driver
> first, to see if we can get it to work.
> 
> I guess you understand why I am pushing!? This is about maintenance -
> and I really want to avoid having a yet another driver to maintain,
> unless we can extend an existing one.
> 
> [...]
> 
> Kind regards
> Uffe
> 

Re: [PATCH V2 0/5] mmc: add stm32 sdmmc controller

[Ludovic BARRE]

On 03/01/2018 11:44 AM, Ulf Hansson wrote:
> On 1 March 2018 at 10:57, Ludovic BARRE <ludovic.barre@st.com> wrote:
>> Hi Ulf
>>
>> On 03/01/2018 10:06 AM, Ulf Hansson wrote:
>>>
>>> Hi Ludovic,
>>>
>>> On 28 February 2018 at 16:47, Ludovic Barre <ludovic.Barre@st.com> wrote:
>>>>
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> This patch serie adds support of stm32 SDMMC controller.
>>>> stm32h7 is the first SoC to use stm32 SDMMC controller
>>>> (previous SoC had pl180 controller).
>>>
>>>
>>> I am a not convinced this isn't a new improved variant of the pl180.
>>> According to register layout and the code you submitted in patch2,
>>> there are great similarities to pl180 and the mmci driver.
>>
>>
>> In fact, ST designers which created stm32-sdmmc hardware block from scratch
>> are the same which have done the modifications on pl180 variant (stm32
>> legacy f4, f7).
>> So some registers or bits name seem identical (or strongly inspirited) but
>> the engine and features are different.
> 
> Well, in that case, I assume the driver would also need work
> differently, but when looking at the code in patch2 this doesn't seem
> to be the case.
> 

I understand why you push to avoid drivers multiplication. But I'm
scared to squash 2 different hardware block which are their own roadmap
in the same drivers. I fear that it complicates the features management 
and maintenance of this driver.

there are some difference:
-the stm32-sdmmc use an internal dma (stm32-sdmmc is master on the bus), 
-idma alignment constraint.
-idma transfer mode single buffer, double buffer... (future evolution)
-need a command to stop transmission for data state machine
-same bit naming could have offset, mask width or set in different register.
=> I will try to synthesis register difference in a document
-voltage switch sequence
-delay block: calibration, tunning (sdr104)
-reset line required
-the same feature have not the same impact, example ddr mode
stm32:no bypass clk, activated in clk register
pl180: clkreg_neg_edge_enable, activated in datactrl register,
...

stm32-sdmmc is just at begining of its life cycle. Each revision of this 
block increases the difference with pl180. Today, I've just push the 
minimal driver to start a request, but I've not yet send all features of 
this revision like sdio, sdr104 support. After this revision there 
already are 2 other revisions in the pipe (at short term).

I am Out of Office with limited access to my e-mail till 2018 march 12th
I'll try to think about it on ski slope. euh, in fact no just ski and 
enjoy ;-)

BR
Ludo

>>
>> You could find the datasheet of STM32H7x3 on:
>> http://www.st.com/content/ccc/resource/technical/document/reference_manual/group0/c9/a3/76/fa/55/46/45/fa/DM00314099/files/DM00314099.pdf/jcr:content/translations/en.DM00314099.pdf
>>
>> Chapters: 55 Secure digital input/output MultiMediaCard interface
>> (SDMMC)
> 
> Thanks for sharing this. However this confirms my view, it looks
> exactly as a new improved mmci variant. :-)
> 
>>
>> This hardware block has own roadmap and some features are already in the
>> pipe for next SoC.
> 
> That's fine. I don't have a problem extending the mmci driver, even
> several times, as to cope with new revisions.
> 
>>
>> For code design: like I also worked on pl180 in the past :-)
>> my code is inspirited of this driver.
> 
> Right, that may explain things a bit.
> 
> However, besides a re-name of the registers, I really think that the
> code execution, dealing with IRQs etc, is very similar to the mmci
> driver. Isn't it?
> 
> So, I think it's at least worth to give it a go with the mmci driver
> first, to see if we can get it to work.
> 
> I guess you understand why I am pushing!? This is about maintenance -
> and I really want to avoid having a yet another driver to maintain,
> unless we can extend an existing one.
> 
> [...]
> 
> Kind regards
> Uffe
> 

Re: [PATCH V2 0/5] mmc: add stm32 sdmmc controller

[Ulf Hansson]

On 2 March 2018 at 09:51, Ludovic BARRE <ludovic.barre@st.com> wrote:
>
>
> On 03/01/2018 11:44 AM, Ulf Hansson wrote:
>>
>> On 1 March 2018 at 10:57, Ludovic BARRE <ludovic.barre@st.com> wrote:
>>>
>>> Hi Ulf
>>>
>>> On 03/01/2018 10:06 AM, Ulf Hansson wrote:
>>>>
>>>>
>>>> Hi Ludovic,
>>>>
>>>> On 28 February 2018 at 16:47, Ludovic Barre <ludovic.Barre@st.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>>
>>>>> This patch serie adds support of stm32 SDMMC controller.
>>>>> stm32h7 is the first SoC to use stm32 SDMMC controller
>>>>> (previous SoC had pl180 controller).
>>>>
>>>>
>>>>
>>>> I am a not convinced this isn't a new improved variant of the pl180.
>>>> According to register layout and the code you submitted in patch2,
>>>> there are great similarities to pl180 and the mmci driver.
>>>
>>>
>>>
>>> In fact, ST designers which created stm32-sdmmc hardware block from
>>> scratch
>>> are the same which have done the modifications on pl180 variant (stm32
>>> legacy f4, f7).
>>> So some registers or bits name seem identical (or strongly inspirited)
>>> but
>>> the engine and features are different.
>>
>>
>> Well, in that case, I assume the driver would also need work
>> differently, but when looking at the code in patch2 this doesn't seem
>> to be the case.
>>
>
> I understand why you push to avoid drivers multiplication. But I'm
> scared to squash 2 different hardware block which are their own roadmap
> in the same drivers. I fear that it complicates the features management and
> maintenance of this driver.

It may require more work for you to upstream the code you need. You
may even have to re-factor existing code in mmci before you can add
your specific parts on top.

However, whether it gets complicated or not, I think much depends on
the quality of the code changes we make. Both sdhci and dw_mmc gives
an idea of how we can deal with variant differences, in particular
when variant changes are a bit bigger.

In the mmci case, so far the controller that differs the most, is the
qcom variant, which also has the built-in DMA support, similar to your
new ST variant. Perhaps there is some code we can re-use around that.

>
> there are some difference:
> -the stm32-sdmmc use an internal dma (stm32-sdmmc is master on the bus),
> -idma alignment constraint.
> -idma transfer mode single buffer, double buffer... (future evolution)
> -need a command to stop transmission for data state machine
> -same bit naming could have offset, mask width or set in different register.
> => I will try to synthesis register difference in a document
> -voltage switch sequence
> -delay block: calibration, tunning (sdr104)
> -reset line required
> -the same feature have not the same impact, example ddr mode
> stm32:no bypass clk, activated in clk register
> pl180: clkreg_neg_edge_enable, activated in datactrl register,

Doesn't sound like this couldn't be done, yes there are differences
but not that much.

Sure we may need re-work the core driver mmci code, maybe convert it
to set of library function instead and invent a couple mmci specific
callbacks. As I said, sdhci and dw_mmc already does it, so I am sure
it can be done.

> ...
>
> stm32-sdmmc is just at begining of its life cycle. Each revision of this
> block increases the difference with pl180. Today, I've just push the minimal

That's fine, this happens all the time to sdhci and dw_mmc variants.
Again, if it works for them, it should work for mmci.

> driver to start a request, but I've not yet send all features of this
> revision like sdio, sdr104 support. After this revision there already are 2
> other revisions in the pipe (at short term).
>
> I am Out of Office with limited access to my e-mail till 2018 march 12th
> I'll try to think about it on ski slope. euh, in fact no just ski and enjoy
> ;-)

Enjoy you skiing and let's talk more when you get back!

[...]

Kind regards
Uffe