[PATCH 0/6] mmc: add support for sdhci 4.0

[PATCH 0/6] mmc: add support for sdhci 4.0

[Chunyan Zhang]

>From the SD host controller version 4.0 on, SDHCI implementation either
is version 3 compatible or version 4 mode. This patch-set covers those
changes which are common for SDHCI 4.0 version, regardless of whether
they are used with SD or eMMC storage devices.

This patchset also added a new sdhci driver for Spreadtrum's controller
which supports v4.0 mode.

This patchset has been tested on Spreadtrum's mobile phone, emmc can be
initialized, mounted, read and written, with these changes for common
sdhci framework and sdhci-sprd driver.

Chunyan Zhang (6):
  mmc: sdhci: add sd host v4 mode
  mmc: sdhci: made changes for System Address register of SDMA
  mmc: sdhci: add ADMA2 64-bit addressing support for V4 mode
  mmc: sdhci: add 32-bit block count support for v4 mode
  mmc: sdhci: add CMD23 support for v4 mode
  mmc: host: sdhci-sprd: added Spreadtrum's host controller R11

 drivers/mmc/host/Kconfig          |  13 ++
 drivers/mmc/host/Makefile         |   1 +
 drivers/mmc/host/sdhci-sprd-r11.c | 472 ++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.c          |  85 +++++--
 drivers/mmc/host/sdhci.h          |  31 ++-
 5 files changed, 580 insertions(+), 22 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-sprd-r11.c

-- 
2.7.4

[PATCH 1/6] mmc: sdhci: add sd host v4 mode

[Chunyan Zhang]

For SD host controller version 4.00 or later ones, there're two
modes of implementation - Version 3.00 compatible mode or
Version 4 mode.  This patch introduces a flag to record this.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
 drivers/mmc/host/sdhci.c | 6 ++++++
 drivers/mmc/host/sdhci.h | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2ededa7f..cf5695f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3302,6 +3302,12 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
 	v = ver ? *ver : sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (v & SDHCI_SPEC_VER_MASK) >> SDHCI_SPEC_VER_SHIFT;
 
+	if (host->version >= SDHCI_SPEC_400) {
+		if (sdhci_readw(host, SDHCI_HOST_CONTROL2) &
+			SDHCI_CTRL_V4_MODE)
+			host->v4_mode = true;
+	}
+
 	if (host->quirks & SDHCI_QUIRK_MISSING_CAPS)
 		return;
 
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c95b0a4..128b0ba 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -184,6 +184,7 @@
 #define   SDHCI_CTRL_DRV_TYPE_D		0x0030
 #define  SDHCI_CTRL_EXEC_TUNING		0x0040
 #define  SDHCI_CTRL_TUNED_CLK		0x0080
+#define  SDHCI_CTRL_V4_MODE		0x1000
 #define  SDHCI_CTRL_PRESET_VAL_ENABLE	0x8000
 
 #define SDHCI_CAPABILITIES	0x40
@@ -270,6 +271,8 @@
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
 #define   SDHCI_SPEC_300	2
+#define   SDHCI_SPEC_400	3
+#define   SDHCI_SPEC_410	4
 
 /*
  * End of controller registers.
@@ -551,6 +554,9 @@ struct sdhci_host {
 	u32			sdma_boundary;
 
 	unsigned long private[0] ____cacheline_aligned;
+
+	/* Host Version 4 Enable */
+	bool			v4_mode;
 };
 
 struct sdhci_ops {
-- 
2.7.4

[PATCH 2/6] mmc: sdhci: made changes for System Address register of SDMA

[Chunyan Zhang]

According to the SD host controller specification version 4.10, when
Host Version 4 is enabled, SDMA uses ADMA System Address register
(05Fh-058h) instead of using SDMA System Address register to
support both 32-bit and 64-bit addressing.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
 drivers/mmc/host/sdhci.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index cf5695f..f57201f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -805,6 +805,7 @@ static void sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 ctrl;
+	u32 reg;
 	struct mmc_data *data = cmd->data;
 
 	if (sdhci_data_line_cmd(cmd))
@@ -894,8 +895,10 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 					     SDHCI_ADMA_ADDRESS_HI);
 		} else {
 			WARN_ON(sg_cnt != 1);
+			reg = host->v4_mode ? SDHCI_ADMA_ADDRESS :
+				SDHCI_DMA_ADDRESS;
 			sdhci_writel(host, sdhci_sdma_address(host),
-				     SDHCI_DMA_ADDRESS);
+				     reg);
 		}
 	}
 
@@ -2721,6 +2724,7 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 		 */
 		if (intmask & SDHCI_INT_DMA_END) {
 			u32 dmastart, dmanow;
+			u32 reg;
 
 			dmastart = sdhci_sdma_address(host);
 			dmanow = dmastart + host->data->bytes_xfered;
@@ -2733,7 +2737,9 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 			host->data->bytes_xfered = dmanow - dmastart;
 			DBG("DMA base 0x%08x, transferred 0x%06x bytes, next 0x%08x\n",
 			    dmastart, host->data->bytes_xfered, dmanow);
-			sdhci_writel(host, dmanow, SDHCI_DMA_ADDRESS);
+			reg = host->v4_mode ? SDHCI_ADMA_ADDRESS :
+				SDHCI_DMA_ADDRESS;
+			sdhci_writel(host, dmanow, reg);
 		}
 
 		if (intmask & SDHCI_INT_DATA_END) {
-- 
2.7.4

[PATCH 3/6] mmc: sdhci: add ADMA2 64-bit addressing support for V4 mode

[Chunyan Zhang]

ADMA2 64-bit addressing support is divided into V3 mode and V4 mode.
So there are two kinds of descriptors for ADMA2 64-bit addressing
i.e. 96-bit Descriptor for V3 mode, and 128-bit Descriptor for V4
mode. 128-bit Descriptor is aligned to 8-byte.

For V4 mode, ADMA2 64-bit addressing is enabled via Host Control 2
register.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
 drivers/mmc/host/sdhci.c | 50 +++++++++++++++++++++++++++++++++++-------------
 drivers/mmc/host/sdhci.h | 23 +++++++++++++++++-----
 2 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f57201f..5d3b0d8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -585,6 +585,8 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
 	void *desc, *align;
 	char *buffer;
 	int len, offset, i;
+	unsigned int adma2_align = SDHCI_ADMA2_ALIGN(host);
+	unsigned int adma2_mask = SDHCI_ADMA2_MASK(host);
 
 	/*
 	 * The spec does not specify endianness of descriptor table.
@@ -608,8 +610,8 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
 		 * buffer for the (up to three) bytes that screw up the
 		 * alignment.
 		 */
-		offset = (SDHCI_ADMA2_ALIGN - (addr & SDHCI_ADMA2_MASK)) &
-			 SDHCI_ADMA2_MASK;
+		offset = (adma2_align - (addr & adma2_align)) &
+			 adma2_mask;
 		if (offset) {
 			if (data->flags & MMC_DATA_WRITE) {
 				buffer = sdhci_kmap_atomic(sg, &flags);
@@ -623,8 +625,8 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
 
 			BUG_ON(offset > 65536);
 
-			align += SDHCI_ADMA2_ALIGN;
-			align_addr += SDHCI_ADMA2_ALIGN;
+			align += adma2_align;
+			align_addr += adma2_align;
 
 			desc += host->desc_sz;
 
@@ -668,13 +670,15 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
 	void *align;
 	char *buffer;
 	unsigned long flags;
+	unsigned int adma2_align = SDHCI_ADMA2_ALIGN(host);
+	unsigned int adma2_mask = SDHCI_ADMA2_MASK(host);
 
 	if (data->flags & MMC_DATA_READ) {
 		bool has_unaligned = false;
 
 		/* Do a quick scan of the SG list for any unaligned mappings */
 		for_each_sg(data->sg, sg, host->sg_count, i)
-			if (sg_dma_address(sg) & SDHCI_ADMA2_MASK) {
+			if (sg_dma_address(sg) & adma2_mask) {
 				has_unaligned = true;
 				break;
 			}
@@ -686,15 +690,15 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
 			align = host->align_buffer;
 
 			for_each_sg(data->sg, sg, host->sg_count, i) {
-				if (sg_dma_address(sg) & SDHCI_ADMA2_MASK) {
-					size = SDHCI_ADMA2_ALIGN -
-					       (sg_dma_address(sg) & SDHCI_ADMA2_MASK);
+				if (sg_dma_address(sg) & adma2_mask) {
+					size = adma2_align -
+					       (sg_dma_address(sg) & adma2_mask);
 
 					buffer = sdhci_kmap_atomic(sg, &flags);
 					memcpy(buffer, align, size);
 					sdhci_kunmap_atomic(buffer, &flags);
 
-					align += SDHCI_ADMA2_ALIGN;
+					align += adma2_align;
 				}
 			}
 		}
@@ -3400,6 +3404,26 @@ static int sdhci_allocate_bounce_buffer(struct sdhci_host *host)
 	return 0;
 }
 
+static inline bool sdhci_use_64bit_dma(struct sdhci_host *host)
+{
+	u32 addr64bit_en;
+
+	/*
+	 * According to SD Host Controller spec v4.10, bit[27] added from
+	 * version 4.10 in Capabilities Register is used as 64-bit System
+	 * Address support for V4 mode, 64-bit DMA Addressing for V4 mode
+	 * is enabled only if 64-bit Addressing =1 in the Host Control 2
+	 * register.
+	 */
+	if (host->version == SDHCI_SPEC_410 && host->v4_mode) {
+		addr64bit_en = (sdhci_readw(host, SDHCI_HOST_CONTROL2) &
+				SDHCI_CTRL_64BIT_ADDR);
+		return addr64bit_en && (host->caps & SDHCI_CAN_64BIT_V4);
+	}
+
+	return host->caps & SDHCI_CAN_64BIT;
+}
+
 int sdhci_setup_host(struct sdhci_host *host)
 {
 	struct mmc_host *mmc;
@@ -3471,7 +3495,7 @@ int sdhci_setup_host(struct sdhci_host *host)
 	 * SDHCI_QUIRK2_BROKEN_64_BIT_DMA must be left to the drivers to
 	 * implement.
 	 */
-	if (host->caps & SDHCI_CAN_64BIT)
+	if (sdhci_use_64bit_dma(host))
 		host->flags |= SDHCI_USE_64_BIT_DMA;
 
 	if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) {
@@ -3505,15 +3529,15 @@ int sdhci_setup_host(struct sdhci_host *host)
 		 */
 		if (host->flags & SDHCI_USE_64_BIT_DMA) {
 			host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) *
-					      SDHCI_ADMA2_64_DESC_SZ;
-			host->desc_sz = SDHCI_ADMA2_64_DESC_SZ;
+					      SDHCI_ADMA2_64_DESC_SZ(host);
+			host->desc_sz = SDHCI_ADMA2_64_DESC_SZ(host);
 		} else {
 			host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) *
 					      SDHCI_ADMA2_32_DESC_SZ;
 			host->desc_sz = SDHCI_ADMA2_32_DESC_SZ;
 		}
 
-		host->align_buffer_sz = SDHCI_MAX_SEGS * SDHCI_ADMA2_ALIGN;
+		host->align_buffer_sz = SDHCI_MAX_SEGS * SDHCI_ADMA2_ALIGN(host);
 		buf = dma_alloc_coherent(mmc_dev(mmc), host->align_buffer_sz +
 					 host->adma_table_sz, &dma, GFP_KERNEL);
 		if (!buf) {
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 128b0ba..820a863 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -185,6 +185,7 @@
 #define  SDHCI_CTRL_EXEC_TUNING		0x0040
 #define  SDHCI_CTRL_TUNED_CLK		0x0080
 #define  SDHCI_CTRL_V4_MODE		0x1000
+#define  SDHCI_CTRL_64BIT_ADDR		0x2000
 #define  SDHCI_CTRL_PRESET_VAL_ENABLE	0x8000
 
 #define SDHCI_CAPABILITIES	0x40
@@ -206,6 +207,7 @@
 #define  SDHCI_CAN_VDD_300	0x02000000
 #define  SDHCI_CAN_VDD_180	0x04000000
 #define  SDHCI_CAN_64BIT	0x10000000
+#define  SDHCI_CAN_64BIT_V4	0x8000000
 
 #define  SDHCI_SUPPORT_SDR50	0x00000001
 #define  SDHCI_SUPPORT_SDR104	0x00000002
@@ -297,9 +299,14 @@ struct sdhci_adma2_32_desc {
 	__le32	addr;
 }  __packed __aligned(4);
 
-/* ADMA2 data alignment */
-#define SDHCI_ADMA2_ALIGN	4
-#define SDHCI_ADMA2_MASK	(SDHCI_ADMA2_ALIGN - 1)
+/*
+ * ADMA2 data alignment
+ * According to SD Host Controller spec v4.10, if Host Version 4 Enable is set
+ * in the Host Control 2 register, 128-bit Descriptor will be selected which
+ * shall be aligned 8-byte address boundary.
+ */
+#define SDHCI_ADMA2_ALIGN(host)	((host)->v4_mode ? 8 : 4)
+#define SDHCI_ADMA2_MASK(host)	(SDHCI_ADMA2_ALIGN(host) - 1)
 
 /*
  * ADMA2 descriptor alignment.  Some controllers (e.g. Intel) require 8 byte
@@ -308,8 +315,14 @@ struct sdhci_adma2_32_desc {
  */
 #define SDHCI_ADMA2_DESC_ALIGN	8
 
-/* ADMA2 64-bit DMA descriptor size */
-#define SDHCI_ADMA2_64_DESC_SZ	12
+/*
+ * ADMA2 64-bit DMA descriptor size
+ * According to SD Host Controller spec v4.10, there are two kinds of
+ * descriptors for 64-bit addressing mode: 96-bit Descriptor and 128-bit
+ * Descriptor, if Host Version 4 Enable is set in the Host Control 2
+ * register, 128-bit Descriptor will be selected.
+ */
+#define SDHCI_ADMA2_64_DESC_SZ(host)	((host)->v4_mode ? 16 : 12)
 
 /*
  * ADMA2 64-bit descriptor. Note 12-byte descriptor can't always be 8-byte
-- 
2.7.4

[PATCH 4/6] mmc: sdhci: add 32-bit block count support for v4 mode

[Chunyan Zhang]

When Host Version 4 is enabled, SDMA System Address register is
re-defined as 32-bit Block Count, and SDMA uses ADMA System
Address register (05Fh-058h) instead.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
 drivers/mmc/host/sdhci.c | 3 ++-
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5d3b0d8..b8ee124 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -943,7 +943,8 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 	/* Set the DMA boundary value and block size */
 	sdhci_writew(host, SDHCI_MAKE_BLKSZ(host->sdma_boundary, data->blksz),
 		     SDHCI_BLOCK_SIZE);
-	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
+	reg = host->v4_mode ? SDHCI_32BIT_BLK_CNT : SDHCI_BLOCK_COUNT;
+	sdhci_writew(host, data->blocks, reg);
 }
 
 static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 820a863..1e84539 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -28,6 +28,7 @@
 
 #define SDHCI_DMA_ADDRESS	0x00
 #define SDHCI_ARGUMENT2		SDHCI_DMA_ADDRESS
+#define SDHCI_32BIT_BLK_CNT	SDHCI_DMA_ADDRESS
 
 #define SDHCI_BLOCK_SIZE	0x04
 #define  SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | (blksz & 0xFFF))
-- 
2.7.4

[PATCH 5/6] mmc: sdhci: add CMD23 support for v4 mode

[Chunyan Zhang]

Host Driver Version 4.10 adds a new bit in Host Control 2 Register
for selecting Auto CMD23 or Auto CMD12 for ADMA3 data transfer.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
 drivers/mmc/host/sdhci.c | 16 +++++++++++++++-
 drivers/mmc/host/sdhci.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b8ee124..3b2af7e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -954,6 +954,20 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
 	       !mrq->cap_cmd_during_tfr;
 }
 
+static inline void sdhci_set_auto_cmd23(struct sdhci_host *host,
+					struct mmc_command *cmd)
+{
+	u16 ctrl2;
+
+	if (host->v4_mode) {
+		ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		ctrl2 |= SDHCI_CMD23_ENABLE;
+		sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
+	} else {
+		sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
+	}
+}
+
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	struct mmc_command *cmd)
 {
@@ -989,7 +1003,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 			mode |= SDHCI_TRNS_AUTO_CMD12;
 		else if (cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23)) {
 			mode |= SDHCI_TRNS_AUTO_CMD23;
-			sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
+			sdhci_set_auto_cmd23(host, cmd);
 		}
 	}
 
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 1e84539..d5e1c10 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -185,6 +185,7 @@
 #define   SDHCI_CTRL_DRV_TYPE_D		0x0030
 #define  SDHCI_CTRL_EXEC_TUNING		0x0040
 #define  SDHCI_CTRL_TUNED_CLK		0x0080
+#define  SDHCI_CMD23_ENABLE		0x0800
 #define  SDHCI_CTRL_V4_MODE		0x1000
 #define  SDHCI_CTRL_64BIT_ADDR		0x2000
 #define  SDHCI_CTRL_PRESET_VAL_ENABLE	0x8000
-- 
2.7.4

[PATCH 6/6] mmc: host: sdhci-sprd: added Spreadtrum's host controller R11

[Chunyan Zhang]

From: Chunyan Zhang <chunyan.zhang@spreadtrum.com>

This patch adds the initial support of Secure Digital Host Controller
Interface compliant controller - R11 found in some latest Spreadtrum
chipsets.

R11 is a variant based on SD v4.0 specification.

With this driver, mmc can be initialized, can be mounted, read and
written.

Original-by: Billows Wu <billows.wu@spreadtrum.com>
Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
---
 drivers/mmc/host/Kconfig          |  13 ++
 drivers/mmc/host/Makefile         |   1 +
 drivers/mmc/host/sdhci-sprd-r11.c | 472 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 486 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-sprd-r11.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 9589f9c..563baf5 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -584,6 +584,19 @@ config MMC_SDRICOH_CS
 	  To compile this driver as a module, choose M here: the
 	  module will be called sdricoh_cs.
 
+config MMC_SDHCI_SPRD_R11
+	tristate "Spreadtrum SDIO host Controller(IP version: R11P0)"
+	depends on ARCH_SPRD
+	depends on MMC_SDHCI_PLTFM
+	select MMC_SDHCI_IO_ACCESSORS
+	help
+	  This selects the SDIO Host Controller(R11P0) in Spreadtrum
+	  SoCs.
+
+	  If you have a controller with this interface, say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_TMIO_CORE
 	tristate
 
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 6aead24..417394d 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)	+= sdhci-pic32.o
 obj-$(CONFIG_MMC_SDHCI_BRCMSTB)		+= sdhci-brcmstb.o
 obj-$(CONFIG_MMC_SDHCI_OMAP)		+= sdhci-omap.o
 obj-$(CONFIG_MMC_CQHCI)			+= cqhci.o
+obj-$(CONFIG_MMC_SDHCI_SPRD_R11)	+= sdhci-sprd-r11.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
 	CFLAGS-cb710-mmc	+= -DDEBUG
diff --git a/drivers/mmc/host/sdhci-sprd-r11.c b/drivers/mmc/host/sdhci-sprd-r11.c
new file mode 100644
index 0000000..fcd093a5
--- /dev/null
+++ b/drivers/mmc/host/sdhci-sprd-r11.c
@@ -0,0 +1,472 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Secure Digital Host Controller
+//
+// Copyright (C) 2018 Spreadtrum, Inc.
+// Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
+
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/highmem.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#include "sdhci-pltfm.h"
+
+#define SDHCI_SPRD_REG_32_DLL_DLY_OFFSET	0x208
+#define  SDHCIBSPRD_IT_WR_DLY_INV		(1 << 5)
+#define  SDHCI_SPRD_BIT_CMD_DLY_INV		(1 << 13)
+#define  SDHCI_SPRD_BIT_POSRD_DLY_INV		(1 << 21)
+#define  SDHCI_SPRD_BIT_NEGRD_DLY_INV		(1 << 29)
+
+#define SDHCI_SPRD_REG_32_BUSY_POSI		0x250
+#define  SDHCI_SPRD_BIT_OUTR_CLK_AUTO_EN	(1 << 25)
+#define  SDHCI_SPRD_BIT_INNR_CLK_AUTO_EN	(1 << 24)
+
+#define SDHCI_SPRD_REG_DEBOUNCE		0x28C
+#define  SDHCI_SPRD_BIT_DLL_BAK		(1 << 0)
+#define  SDHCI_SPRD_BIT_DLL_VAL		(1 << 1)
+
+#define  SDHCI_SPRD_INT_SIGNAL_MASK	0x1B7F410B
+
+/* SDHCI_HOST_CONTROL2 */
+#define  SDHCI_SPRD_CTRL_HS200		0x0005
+#define  SDHCI_SPRD_CTRL_HS400		0x0006
+
+/* SDHCI_SOFTWARE_RESET */
+#define  SDHCI_HW_RESET_CARD		0x8 /* For Spreadtrum's design */
+
+#define SDHCI_SPRD_MAX_CUR		1020
+#define SDHCI_SPRD_CLK_MAX_DIV		0x3FF
+
+struct sdhci_sprd_host {
+	u32 version;
+	struct clk *clk_sdio;
+	struct clk *clk_source;
+	struct clk *clk_enable;
+	u32 base_rate;
+};
+
+#define TO_SPRD_HOST(host) sdhci_pltfm_priv(sdhci_priv(host))
+
+static int sdhci_sprd_get_dt_resource(struct platform_device *pdev,
+		struct sdhci_sprd_host *sprd_host)
+{
+	int ret = 0;
+	struct clk *clk;
+
+	clk = devm_clk_get(&pdev->dev, "sdio");
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		dev_warn(&pdev->dev, "Failed to get sdio clock (%d)\n", ret);
+		goto out;
+	}
+	sprd_host->clk_sdio = clk;
+
+	clk = devm_clk_get(&pdev->dev, "source");
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		dev_warn(&pdev->dev, "Failed to get source clock (%d)\n", ret);
+		goto out;
+	}
+	sprd_host->clk_source = clk;
+
+	clk_set_parent(sprd_host->clk_sdio, sprd_host->clk_source);
+	sprd_host->base_rate = clk_get_rate(sprd_host->clk_source);
+	if (!sprd_host->base_rate) {
+		sprd_host->base_rate = 26000000;
+		dev_warn(&pdev->dev, "The source clock rate is 0\n");
+	}
+
+	clk = devm_clk_get(&pdev->dev, "enable");
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		dev_warn(&pdev->dev, "Failed to get gate clock (%d)\n", ret);
+		goto out;
+	}
+	sprd_host->clk_enable = clk;
+
+out:
+	return ret;
+}
+
+static void sdhci_sprd_set_mmc_struct(struct platform_device *pdev,
+				struct mmc_host *mmc)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	mmc->caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
+		MMC_CAP_ERASE | MMC_CAP_CMD23;
+
+	mmc_of_parse(mmc);
+	mmc_of_parse_voltage(np, &host->ocr_mask);
+
+	mmc->ocr_avail = 0x40000;
+	mmc->ocr_avail_sdio = mmc->ocr_avail;
+	mmc->ocr_avail_sd = mmc->ocr_avail;
+	mmc->ocr_avail_mmc = mmc->ocr_avail;
+
+	mmc->max_current_330 = SDHCI_SPRD_MAX_CUR;
+	mmc->max_current_300 = SDHCI_SPRD_MAX_CUR;
+	mmc->max_current_180 = SDHCI_SPRD_MAX_CUR;
+
+	host->dma_mask = DMA_BIT_MASK(64);
+	mmc_dev(host->mmc)->dma_mask = &host->dma_mask;
+}
+
+static void sdhci_sprd_init_config(struct sdhci_host *host)
+{
+	u16 val;
+
+	/* set 64-bit addressing modes */
+	val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+	val |= SDHCI_CTRL_64BIT_ADDR;
+	sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
+
+	/* set dll backup mode */
+	val = sdhci_readl(host, SDHCI_SPRD_REG_DEBOUNCE);
+	val |= SDHCI_SPRD_BIT_DLL_BAK | SDHCI_SPRD_BIT_DLL_VAL;
+	sdhci_writel(host, val, SDHCI_SPRD_REG_DEBOUNCE);
+}
+
+static inline u32 sdhci_sprd_readl(struct sdhci_host *host, int reg)
+{
+	if (unlikely(reg == SDHCI_MAX_CURRENT))
+		return SDHCI_SPRD_MAX_CUR;
+
+	return readl_relaxed(host->ioaddr + reg);
+}
+
+static inline void sdhci_sprd_writel(struct sdhci_host *host, u32 val, int reg)
+{
+	/* SDHCI_MAX_CURRENT is reserved on Spreadtrum's platform */
+	if (unlikely(reg == SDHCI_MAX_CURRENT))
+		return;
+
+	if (unlikely(reg == SDHCI_SIGNAL_ENABLE || reg == SDHCI_INT_ENABLE))
+		val = val & SDHCI_SPRD_INT_SIGNAL_MASK;
+
+	return writel_relaxed(val, host->ioaddr + reg);
+}
+
+static inline void sdhci_sprd_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	if (unlikely(reg == SDHCI_SOFTWARE_RESET)) {
+		if (readb_relaxed(host->ioaddr + reg) & SDHCI_HW_RESET_CARD)
+			val |= SDHCI_HW_RESET_CARD;
+	}
+
+	return writeb_relaxed(val, host->ioaddr + reg);
+}
+
+static inline void sdhci_sprd_sd_clk_off(struct sdhci_host *host)
+{
+	u16 ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+
+	ctrl &= (~SDHCI_CLOCK_CARD_EN);
+	sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL);
+}
+
+static inline void
+sdhci_sprd_set_dll_invert(struct sdhci_host *host, u32 mask, bool en)
+{
+	u32 dll_dly_offset;
+
+	dll_dly_offset = sdhci_readl(host, SDHCI_SPRD_REG_32_DLL_DLY_OFFSET);
+	if (en)
+		dll_dly_offset |= mask;
+	else
+		dll_dly_offset &= ~mask;
+	sdhci_writel(host, dll_dly_offset, SDHCI_SPRD_REG_32_DLL_DLY_OFFSET);
+}
+
+static inline u32 sdhci_sprd_calc_div(u32 base_clk, u32 clk)
+{
+	u32 div;
+
+	/* select 2x clock source */
+	if (base_clk <= clk * 2)
+		return 0;
+
+	div = (u32) (base_clk / (clk * 2));
+
+	if ((base_clk / div) > (clk * 2))
+		div++;
+
+	if (div > SDHCI_SPRD_CLK_MAX_DIV)
+		div = SDHCI_SPRD_CLK_MAX_DIV;
+
+	if (div % 2)
+		div = (div + 1) / 2;
+	else
+		div = div / 2;
+
+	return div;
+}
+
+static inline void _sdhci_sprd_set_clock(struct sdhci_host *host,
+					unsigned int clk)
+{
+	struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
+	u32 div, val, mask;
+
+	div = sdhci_sprd_calc_div(sprd_host->base_rate, clk);
+
+	clk |= ((div & 0x300) >> 2) | ((div & 0xFF) << 8);
+	sdhci_enable_clk(host, clk);
+
+	/* enable auto gate sdhc_enable_auto_gate */
+	val = sdhci_readl(host, SDHCI_SPRD_REG_32_BUSY_POSI);
+	mask = SDHCI_SPRD_BIT_OUTR_CLK_AUTO_EN |
+	       SDHCI_SPRD_BIT_INNR_CLK_AUTO_EN;
+	if (mask != (val & mask)) {
+		val |= mask;
+		sdhci_writel(host, val, SDHCI_SPRD_REG_32_BUSY_POSI);
+	}
+}
+
+static void sdhci_sprd_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	bool en = false;
+
+	if (clock == 0) {
+		sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
+	} else if (clock != host->clock) {
+		sdhci_sprd_sd_clk_off(host);
+		_sdhci_sprd_set_clock(host, clock);
+
+		if (clock <= 400000)
+			en = true;
+		sdhci_sprd_set_dll_invert(host, SDHCI_SPRD_BIT_CMD_DLY_INV |
+					  SDHCI_SPRD_BIT_POSRD_DLY_INV, en);
+	} else {
+		_sdhci_sprd_set_clock(host, clock);
+	}
+
+}
+
+static unsigned int sdhci_sprd_get_max_clock(struct sdhci_host *host)
+{
+	struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
+
+	return clk_round_rate(sprd_host->clk_sdio, ULONG_MAX);
+}
+
+static unsigned int sdhci_sprd_get_min_clock(struct sdhci_host *host)
+{
+	return 400000;
+}
+
+static void sdhci_sprd_reset(struct sdhci_host *host, u8 mask)
+{
+	sdhci_reset(host, mask);
+}
+
+static void sdhci_sprd_set_uhs_signaling(struct sdhci_host *host,
+					 unsigned int timing)
+{
+	u16 ctrl_2;
+
+	if (timing == host->timing)
+		return;
+
+	ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+	/* Select Bus Speed Mode for host */
+	ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
+	switch (timing) {
+	case MMC_TIMING_UHS_SDR12:
+		ctrl_2 = SDHCI_CTRL_UHS_SDR12;
+		break;
+	case MMC_TIMING_MMC_HS:
+	case MMC_TIMING_SD_HS:
+	case MMC_TIMING_UHS_SDR25:
+		ctrl_2 = SDHCI_CTRL_UHS_SDR25;
+		break;
+	case MMC_TIMING_UHS_SDR50:
+		ctrl_2 = SDHCI_CTRL_UHS_SDR50;
+		break;
+	case MMC_TIMING_UHS_SDR104:
+		ctrl_2 = SDHCI_CTRL_UHS_SDR104;
+		break;
+	case MMC_TIMING_UHS_DDR50:
+	case MMC_TIMING_MMC_DDR52:
+		ctrl_2 = SDHCI_CTRL_UHS_DDR50;
+		break;
+	case MMC_TIMING_MMC_HS200:
+		ctrl_2 = SDHCI_SPRD_CTRL_HS200;
+		break;
+	case MMC_TIMING_MMC_HS400:
+		ctrl_2 = SDHCI_SPRD_CTRL_HS400;
+		break;
+	default:
+		break;
+	}
+
+	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+}
+
+static void sdhci_sprd_hw_reset(struct sdhci_host *host)
+{
+	int val;
+
+	/* Note: don't use sdhci_readb/writeb() API here */
+	val = readb_relaxed(host->ioaddr + SDHCI_SOFTWARE_RESET);
+	val &= ~SDHCI_HW_RESET_CARD;
+	writeb_relaxed(val, host->ioaddr + SDHCI_SOFTWARE_RESET);
+	udelay(10);
+
+	val |= SDHCI_HW_RESET_CARD;
+	writeb_relaxed(val, host->ioaddr + SDHCI_SOFTWARE_RESET);
+	udelay(300);
+}
+
+static struct sdhci_ops sdhci_sprd_ops = {
+	.read_l = sdhci_sprd_readl,
+	.write_l = sdhci_sprd_writel,
+	.write_b = sdhci_sprd_writeb,
+	.set_clock = sdhci_sprd_set_clock,
+	.get_max_clock = sdhci_sprd_get_max_clock,
+	.get_min_clock = sdhci_sprd_get_min_clock,
+	.set_bus_width = sdhci_set_bus_width,
+	.reset = sdhci_sprd_reset,
+	.set_uhs_signaling = sdhci_sprd_set_uhs_signaling,
+	.hw_reset = sdhci_sprd_hw_reset,
+};
+
+static const struct sdhci_pltfm_data sdhci_sprd_pdata = {
+	.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
+	.quirks2 = SDHCI_QUIRK2_BROKEN_HS200,
+	.ops = &sdhci_sprd_ops,
+};
+
+static int sdhci_sprd_probe(struct platform_device *pdev)
+{
+	struct sdhci_host *host;
+	struct sdhci_sprd_host *sprd_host;
+	int ret = 0;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_sprd_pdata, sizeof(*sprd_host));
+	if (IS_ERR(host))
+		return PTR_ERR(host);
+
+	sprd_host = TO_SPRD_HOST(host);
+
+	ret = sdhci_sprd_get_dt_resource(pdev, sprd_host);
+	if (ret)
+		goto pltfm_free;
+
+	clk_prepare_enable(sprd_host->clk_sdio);
+	clk_prepare_enable(sprd_host->clk_enable);
+
+	sdhci_sprd_init_config(host);
+
+	sdhci_sprd_set_mmc_struct(pdev, host->mmc);
+
+	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
+	sprd_host->version = ((host->version & SDHCI_VENDOR_VER_MASK) >>
+			       SDHCI_VENDOR_VER_SHIFT);
+
+	pm_runtime_get_noresume(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_suspend_ignore_children(&pdev->dev, 1);
+
+	ret = sdhci_add_host(host);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add mmc host: %d\n", ret);
+		goto pm_runtime_disable;
+	}
+
+	return 0;
+
+pm_runtime_disable:
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_set_suspended(&pdev->dev);
+
+	clk_disable_unprepare(sprd_host->clk_sdio);
+	clk_disable_unprepare(sprd_host->clk_enable);
+
+pltfm_free:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static int sdhci_sprd_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
+	struct mmc_host *mmc = host->mmc;
+
+	mmc_remove_host(mmc);
+	clk_disable_unprepare(sprd_host->clk_sdio);
+	clk_disable_unprepare(sprd_host->clk_enable);
+
+	mmc_free_host(mmc);
+
+	return 0;
+}
+
+static const struct of_device_id sdhci_sprd_of_match[] = {
+	{ .compatible = "sprd,sdhc-r11", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sdhci_sprd_of_match);
+
+#ifdef CONFIG_PM
+static int sdhci_sprd_runtime_suspend(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
+
+	sdhci_runtime_suspend_host(host);
+
+	clk_disable_unprepare(sprd_host->clk_sdio);
+	clk_disable_unprepare(sprd_host->clk_enable);
+
+	return 0;
+}
+
+static int sdhci_sprd_runtime_resume(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
+
+	clk_prepare_enable(sprd_host->clk_enable);
+	clk_prepare_enable(sprd_host->clk_sdio);
+
+	sdhci_runtime_resume_host(host);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops sdhci_sprd_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(sdhci_sprd_runtime_suspend,
+			   sdhci_sprd_runtime_resume, NULL)
+};
+
+static struct platform_driver sdhci_sprd_driver = {
+	.probe = sdhci_sprd_probe,
+	.remove = sdhci_sprd_remove,
+	.driver = {
+		.name = "sdhci_sprd_r11",
+		.of_match_table = of_match_ptr(sdhci_sprd_of_match),
+		.pm = &sdhci_sprd_pm_ops,
+	},
+};
+module_platform_driver(sdhci_sprd_driver);
+
+MODULE_DESCRIPTION("Spreadtrum sdio host controller r11 driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:sdhci-sprd-r11");
-- 
2.7.4

Re: [PATCH 6/6] mmc: host: sdhci-sprd: added Spreadtrum's host controller R11

[Ulf Hansson]

On 8 June 2018 at 10:18, Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
> From: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>
> This patch adds the initial support of Secure Digital Host Controller
> Interface compliant controller - R11 found in some latest Spreadtrum
> chipsets.
>
> R11 is a variant based on SD v4.0 specification.
>
> With this driver, mmc can be initialized, can be mounted, read and
> written.
>
> Original-by: Billows Wu <billows.wu@spreadtrum.com>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
> ---
>  drivers/mmc/host/Kconfig          |  13 ++
>  drivers/mmc/host/Makefile         |   1 +
>  drivers/mmc/host/sdhci-sprd-r11.c | 472 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 486 insertions(+)
>  create mode 100644 drivers/mmc/host/sdhci-sprd-r11.c

This is a DT based driver. Please add a separate patch describing the
corresponding bindings and compatibles.

[...]

> +static int sdhci_sprd_get_dt_resource(struct platform_device *pdev,
> +               struct sdhci_sprd_host *sprd_host)
> +{
> +       int ret = 0;
> +       struct clk *clk;
> +
> +       clk = devm_clk_get(&pdev->dev, "sdio");
> +       if (IS_ERR(clk)) {
> +               ret = PTR_ERR(clk);
> +               dev_warn(&pdev->dev, "Failed to get sdio clock (%d)\n", ret);
> +               goto out;
> +       }
> +       sprd_host->clk_sdio = clk;
> +
> +       clk = devm_clk_get(&pdev->dev, "source");
> +       if (IS_ERR(clk)) {
> +               ret = PTR_ERR(clk);
> +               dev_warn(&pdev->dev, "Failed to get source clock (%d)\n", ret);
> +               goto out;
> +       }
> +       sprd_host->clk_source = clk;
> +
> +       clk_set_parent(sprd_host->clk_sdio, sprd_host->clk_source);
> +       sprd_host->base_rate = clk_get_rate(sprd_host->clk_source);
> +       if (!sprd_host->base_rate) {
> +               sprd_host->base_rate = 26000000;
> +               dev_warn(&pdev->dev, "The source clock rate is 0\n");
> +       }
> +

The above can be managed by the assigned-clock* DT bindings. Please
have a look at:

Documentation/devicetree/bindings/clock/clock-bindings.txt
drivers/clk/clk-conf.c

> +       clk = devm_clk_get(&pdev->dev, "enable");
> +       if (IS_ERR(clk)) {
> +               ret = PTR_ERR(clk);
> +               dev_warn(&pdev->dev, "Failed to get gate clock (%d)\n", ret);

The printed name of the clock doesn't match the name used in
devm_clk_get() call.

BTW, I think devm_clk_get() already prints some information when it
fails to lookup a clock. Isn't that sufficient?

> +               goto out;
> +       }
> +       sprd_host->clk_enable = clk;
> +
> +out:
> +       return ret;
> +}
> +
> +static void sdhci_sprd_set_mmc_struct(struct platform_device *pdev,
> +                               struct mmc_host *mmc)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct sdhci_host *host = mmc_priv(mmc);
> +
> +       mmc->caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
> +               MMC_CAP_ERASE | MMC_CAP_CMD23;
> +
> +       mmc_of_parse(mmc);
> +       mmc_of_parse_voltage(np, &host->ocr_mask);

mmc_of_parse_voltage() is intended to be used for controllers that
internally manages the powered to the card. Is that really the case?

I assume you have external regulator(s) to manage that, no?

> +
> +       mmc->ocr_avail = 0x40000;
> +       mmc->ocr_avail_sdio = mmc->ocr_avail;
> +       mmc->ocr_avail_sd = mmc->ocr_avail;
> +       mmc->ocr_avail_mmc = mmc->ocr_avail;

If there is external regulators used, all the above can go away. In
either case, at least the *_sdio, *_sd, *_mmc can go away.

> +
> +       mmc->max_current_330 = SDHCI_SPRD_MAX_CUR;
> +       mmc->max_current_300 = SDHCI_SPRD_MAX_CUR;
> +       mmc->max_current_180 = SDHCI_SPRD_MAX_CUR;

This should probably also be fetched used an external regulator and
sdhci already manages that.

> +
> +       host->dma_mask = DMA_BIT_MASK(64);
> +       mmc_dev(host->mmc)->dma_mask = &host->dma_mask;
> +}

[...]

> +
> +static unsigned int sdhci_sprd_get_min_clock(struct sdhci_host *host)
> +{
> +       return 400000;
> +}

Isn't there a more straightforward way to assign the minimum clock
rate? Do you really need to use a callback?

> +
> +static void sdhci_sprd_reset(struct sdhci_host *host, u8 mask)
> +{
> +       sdhci_reset(host, mask);
> +}

Looks like an unnecessary wrapper function.

[...]

> +static int sdhci_sprd_probe(struct platform_device *pdev)
> +{
> +       struct sdhci_host *host;
> +       struct sdhci_sprd_host *sprd_host;
> +       int ret = 0;
> +
> +       host = sdhci_pltfm_init(pdev, &sdhci_sprd_pdata, sizeof(*sprd_host));
> +       if (IS_ERR(host))
> +               return PTR_ERR(host);
> +
> +       sprd_host = TO_SPRD_HOST(host);
> +
> +       ret = sdhci_sprd_get_dt_resource(pdev, sprd_host);
> +       if (ret)
> +               goto pltfm_free;
> +
> +       clk_prepare_enable(sprd_host->clk_sdio);
> +       clk_prepare_enable(sprd_host->clk_enable);
> +
> +       sdhci_sprd_init_config(host);
> +
> +       sdhci_sprd_set_mmc_struct(pdev, host->mmc);
> +
> +       host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
> +       sprd_host->version = ((host->version & SDHCI_VENDOR_VER_MASK) >>
> +                              SDHCI_VENDOR_VER_SHIFT);
> +
> +       pm_runtime_get_noresume(&pdev->dev);
> +       pm_runtime_set_active(&pdev->dev);
> +       pm_runtime_enable(&pdev->dev);
> +       pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> +       pm_runtime_use_autosuspend(&pdev->dev);
> +       pm_suspend_ignore_children(&pdev->dev, 1);
> +
> +       ret = sdhci_add_host(host);
> +       if (ret) {
> +               dev_err(&pdev->dev, "failed to add mmc host: %d\n", ret);
> +               goto pm_runtime_disable;
> +       }
> +

Looks like there is a missing call to pm_runtime_put() here, no?

Unless it's intentional to prevent runtime suspend, for whatever reasons!?

> +       return 0;
> +
> +pm_runtime_disable:
> +       pm_runtime_disable(&pdev->dev);
> +       pm_runtime_set_suspended(&pdev->dev);
> +
> +       clk_disable_unprepare(sprd_host->clk_sdio);
> +       clk_disable_unprepare(sprd_host->clk_enable);
> +
> +pltfm_free:
> +       sdhci_pltfm_free(pdev);
> +       return ret;
> +}
> +
> +static int sdhci_sprd_remove(struct platform_device *pdev)
> +{
> +       struct sdhci_host *host = platform_get_drvdata(pdev);
> +       struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
> +       struct mmc_host *mmc = host->mmc;
> +
> +       mmc_remove_host(mmc);
> +       clk_disable_unprepare(sprd_host->clk_sdio);
> +       clk_disable_unprepare(sprd_host->clk_enable);
> +
> +       mmc_free_host(mmc);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id sdhci_sprd_of_match[] = {
> +       { .compatible = "sprd,sdhc-r11", },

As stated, don't forget to document this.

[...]

Kind regards
Uffe

Re: [PATCH 6/6] mmc: host: sdhci-sprd: added Spreadtrum's host controller R11

[Chunyan Zhang]

Hi Ulf,

On 11 June 2018 at 15:15, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 8 June 2018 at 10:18, Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
>> From: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>>
>> This patch adds the initial support of Secure Digital Host Controller
>> Interface compliant controller - R11 found in some latest Spreadtrum
>> chipsets.
>>
>> R11 is a variant based on SD v4.0 specification.
>>
>> With this driver, mmc can be initialized, can be mounted, read and
>> written.
>>
>> Original-by: Billows Wu <billows.wu@spreadtrum.com>
>> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>> ---
>>  drivers/mmc/host/Kconfig          |  13 ++
>>  drivers/mmc/host/Makefile         |   1 +
>>  drivers/mmc/host/sdhci-sprd-r11.c | 472 ++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 486 insertions(+)
>>  create mode 100644 drivers/mmc/host/sdhci-sprd-r11.c
>
> This is a DT based driver. Please add a separate patch describing the
> corresponding bindings and compatibles.

Ok.

>
> [...]
>
>> +static int sdhci_sprd_get_dt_resource(struct platform_device *pdev,
>> +               struct sdhci_sprd_host *sprd_host)
>> +{
>> +       int ret = 0;
>> +       struct clk *clk;
>> +
>> +       clk = devm_clk_get(&pdev->dev, "sdio");
>> +       if (IS_ERR(clk)) {
>> +               ret = PTR_ERR(clk);
>> +               dev_warn(&pdev->dev, "Failed to get sdio clock (%d)\n", ret);
>> +               goto out;
>> +       }
>> +       sprd_host->clk_sdio = clk;
>> +
>> +       clk = devm_clk_get(&pdev->dev, "source");
>> +       if (IS_ERR(clk)) {
>> +               ret = PTR_ERR(clk);
>> +               dev_warn(&pdev->dev, "Failed to get source clock (%d)\n", ret);
>> +               goto out;
>> +       }
>> +       sprd_host->clk_source = clk;
>> +
>> +       clk_set_parent(sprd_host->clk_sdio, sprd_host->clk_source);
>> +       sprd_host->base_rate = clk_get_rate(sprd_host->clk_source);
>> +       if (!sprd_host->base_rate) {
>> +               sprd_host->base_rate = 26000000;
>> +               dev_warn(&pdev->dev, "The source clock rate is 0\n");
>> +       }
>> +
>
> The above can be managed by the assigned-clock* DT bindings. Please
> have a look at:
>

Ah, didn't notice these bindings, managing in this way is indeed better.

> Documentation/devicetree/bindings/clock/clock-bindings.txt
> drivers/clk/clk-conf.c
>
>> +       clk = devm_clk_get(&pdev->dev, "enable");
>> +       if (IS_ERR(clk)) {
>> +               ret = PTR_ERR(clk);
>> +               dev_warn(&pdev->dev, "Failed to get gate clock (%d)\n", ret);
>
> The printed name of the clock doesn't match the name used in
> devm_clk_get() call.
>
> BTW, I think devm_clk_get() already prints some information when it
> fails to lookup a clock. Isn't that sufficient?

Right, will remove this print.

>
>> +               goto out;
>> +       }
>> +       sprd_host->clk_enable = clk;
>> +
>> +out:
>> +       return ret;
>> +}
>> +
>> +static void sdhci_sprd_set_mmc_struct(struct platform_device *pdev,
>> +                               struct mmc_host *mmc)
>> +{
>> +       struct device_node *np = pdev->dev.of_node;
>> +       struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +       mmc->caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
>> +               MMC_CAP_ERASE | MMC_CAP_CMD23;
>> +
>> +       mmc_of_parse(mmc);
>> +       mmc_of_parse_voltage(np, &host->ocr_mask);
>
> mmc_of_parse_voltage() is intended to be used for controllers that
> internally manages the powered to the card. Is that really the case?

I guess it is not, will remove this,

>
> I assume you have external regulator(s) to manage that, no?
>
>> +
>> +       mmc->ocr_avail = 0x40000;
>> +       mmc->ocr_avail_sdio = mmc->ocr_avail;
>> +       mmc->ocr_avail_sd = mmc->ocr_avail;
>> +       mmc->ocr_avail_mmc = mmc->ocr_avail;
>

and this,

> If there is external regulators used, all the above can go away. In
> either case, at least the *_sdio, *_sd, *_mmc can go away.
>
>> +
>> +       mmc->max_current_330 = SDHCI_SPRD_MAX_CUR;
>> +       mmc->max_current_300 = SDHCI_SPRD_MAX_CUR;
>> +       mmc->max_current_180 = SDHCI_SPRD_MAX_CUR;
>

also this :)

> This should probably also be fetched used an external regulator and
> sdhci already manages that.
>
>> +
>> +       host->dma_mask = DMA_BIT_MASK(64);
>> +       mmc_dev(host->mmc)->dma_mask = &host->dma_mask;
>> +}
>
> [...]
>
>> +
>> +static unsigned int sdhci_sprd_get_min_clock(struct sdhci_host *host)
>> +{
>> +       return 400000;
>> +}
>
> Isn't there a more straightforward way to assign the minimum clock
> rate? Do you really need to use a callback?
>

Do you mean setting mmc->f_min directly after sdhci_add_host()?
We would get a different f_min without this callback, since
SDHCI_CLOCK_MUL_MASK in caps1 register is reserved.

>> +
>> +static void sdhci_sprd_reset(struct sdhci_host *host, u8 mask)
>> +{
>> +       sdhci_reset(host, mask);
>> +}
>
> Looks like an unnecessary wrapper function.

Ok, will take out of this wrapper.

>
> [...]
>
>> +static int sdhci_sprd_probe(struct platform_device *pdev)
>> +{
>> +       struct sdhci_host *host;
>> +       struct sdhci_sprd_host *sprd_host;
>> +       int ret = 0;
>> +
>> +       host = sdhci_pltfm_init(pdev, &sdhci_sprd_pdata, sizeof(*sprd_host));
>> +       if (IS_ERR(host))
>> +               return PTR_ERR(host);
>> +
>> +       sprd_host = TO_SPRD_HOST(host);
>> +
>> +       ret = sdhci_sprd_get_dt_resource(pdev, sprd_host);
>> +       if (ret)
>> +               goto pltfm_free;
>> +
>> +       clk_prepare_enable(sprd_host->clk_sdio);
>> +       clk_prepare_enable(sprd_host->clk_enable);
>> +
>> +       sdhci_sprd_init_config(host);
>> +
>> +       sdhci_sprd_set_mmc_struct(pdev, host->mmc);
>> +
>> +       host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
>> +       sprd_host->version = ((host->version & SDHCI_VENDOR_VER_MASK) >>
>> +                              SDHCI_VENDOR_VER_SHIFT);
>> +
>> +       pm_runtime_get_noresume(&pdev->dev);
>> +       pm_runtime_set_active(&pdev->dev);
>> +       pm_runtime_enable(&pdev->dev);
>> +       pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
>> +       pm_runtime_use_autosuspend(&pdev->dev);
>> +       pm_suspend_ignore_children(&pdev->dev, 1);
>> +
>> +       ret = sdhci_add_host(host);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "failed to add mmc host: %d\n", ret);
>> +               goto pm_runtime_disable;
>> +       }
>> +
>
> Looks like there is a missing call to pm_runtime_put() here, no?
>

Yes, will add back.

> Unless it's intentional to prevent runtime suspend, for whatever reasons!?
>
>> +       return 0;
>> +
>> +pm_runtime_disable:
>> +       pm_runtime_disable(&pdev->dev);
>> +       pm_runtime_set_suspended(&pdev->dev);
>> +
>> +       clk_disable_unprepare(sprd_host->clk_sdio);
>> +       clk_disable_unprepare(sprd_host->clk_enable);
>> +
>> +pltfm_free:
>> +       sdhci_pltfm_free(pdev);
>> +       return ret;
>> +}
>> +
>> +static int sdhci_sprd_remove(struct platform_device *pdev)
>> +{
>> +       struct sdhci_host *host = platform_get_drvdata(pdev);
>> +       struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
>> +       struct mmc_host *mmc = host->mmc;
>> +
>> +       mmc_remove_host(mmc);
>> +       clk_disable_unprepare(sprd_host->clk_sdio);
>> +       clk_disable_unprepare(sprd_host->clk_enable);
>> +
>> +       mmc_free_host(mmc);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct of_device_id sdhci_sprd_of_match[] = {
>> +       { .compatible = "sprd,sdhc-r11", },
>
> As stated, don't forget to document this.

Ok.

Thanks for your comments,
Chunyan

>
> [...]
>
> Kind regards
> Uffe