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

[PATCH V4 0/7] 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.

Changes from V3:
* Addressed comments from Adrian:
- set "Host Control 2" when enabled v4 mode, and do the same for reset-for-all in sdhci_do_reset();
- Moved "v4_mode" above to the private;
- Change the subject of patch 2/7;
- Use %pad to pirnt dma_addr_t;
- Adjusted to not clear SDHCI_USE_SDMA for v4 mode in sdhci_setup_host();
- Changed the function name to sdhci_can_64bit_dma() instead of sdhci_use_64bit_dma;
- Adjusted to write SDHCI_CTRL_64BIT_ADDR when we decide to use 64-bit DMA in V4 mode,
  rather than check the register to decide if use 64-bit DMA;
- Added a comments for using dma_zalloc_coherent() to replace dma_alloc_coherent();
- Added SDHCI_SPEC_420;
- Set 16-bit block count register to zero conditionally when using 32-bit block count;
- Adjusted to Use 32-bit block count register only for v4.10 v4 mode;
- Added the rules used for AUTO CMD23/12 to AUTO CMD as well;
- Moved the selection of Host Control 2 register CMD23 Enable to sdhci_init() from Spreadtrum's driver;
- Used usleep_range() to replace udelay();
- Added the checks for clk_prepare_enable().
* Added comments for Spreadtrum's specific changes to the register SDHCI_SOFTWARE_RESET;

Changes from V2:
* Addressed comments from Adrian:
- Added sdhci_enable_v4_mode() for enabling v4 mode instead of determining by reading from registers;
- Added support for 64-bit SDMA address in v4 mode;
- Dropped the changes of ADMA2 data aglinment;
- Added support for "Auto Cmd Auto Select".
* Rebased on v4.18-rc2.
* Dealt with a few issues in sdhci-sprd:
- Save return value of mmc_of_parse();
- Add checking for clk_prepare_enable();
- Use BIT() macro instead.

Changes from v1:
* Addressed comments from Ulf:
 - Add dt-bindings for Spreadtrum sdhci;
 - Use assigned-clocks* DT bindings to set default source of sdio clock;
 - Removed unuseful print;
 - Removed two functions which are not used;
 - Add back the missing pm_runtime_put_autosuspend() after adding sdhci host.

* Changed Spreadtrum sdhci driver name to sdhci-sprd.

Chunyan Zhang (7):
  mmc: sdhci: add sd host v4 mode
  mmc: sdhci: Change SDMA address register for v4 mode
  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 Auto CMD Auto Select support
  mmc: sdhci-sprd: added Spreadtrum's initial host controller
  dt-bindings: sdhci-sprd: Add bindings for the sdhci-sprd controller

 .../devicetree/bindings/mmc/sdhci-sprd.txt         |  41 ++
 drivers/mmc/host/Kconfig                           |  13 +
 drivers/mmc/host/Makefile                          |   1 +
 drivers/mmc/host/sdhci-sprd.c                      | 455 +++++++++++++++++++++
 drivers/mmc/host/sdhci.c                           | 223 +++++++---
 drivers/mmc/host/sdhci.h                           |  23 +-
 6 files changed, 708 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
 create mode 100644 drivers/mmc/host/sdhci-sprd.c

-- 
2.7.4

[PATCH V4 1/7] 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 introduced an interface to enable
v4 mode.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1c828e0..cab5350 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -123,6 +123,30 @@ EXPORT_SYMBOL_GPL(sdhci_dumpregs);
  *                                                                           *
 \*****************************************************************************/
 
+static void sdhci_do_enable_v4_mode(struct sdhci_host *host)
+{
+	u16 ctrl2;
+
+	ctrl2 = sdhci_readb(host, SDHCI_HOST_CONTROL2);
+	if (ctrl2 & SDHCI_CTRL_V4_MODE)
+		return;
+
+	ctrl2 |= SDHCI_CTRL_V4_MODE;
+	sdhci_writeb(host, ctrl2, SDHCI_HOST_CONTROL);
+}
+
+/*
+ * Vendor's Host Controller which supports v4 mode can call
+ * this function to enable v4 mode before calling
+ * __sdhci_add_host().
+ */
+void sdhci_enable_v4_mode(struct sdhci_host *host)
+{
+	host->v4_mode = true;
+	sdhci_do_enable_v4_mode(host);
+}
+EXPORT_SYMBOL_GPL(sdhci_enable_v4_mode);
+
 static inline bool sdhci_data_line_cmd(struct mmc_command *cmd)
 {
 	return cmd->data || cmd->flags & MMC_RSP_BUSY;
@@ -224,6 +248,10 @@ static void sdhci_do_reset(struct sdhci_host *host, u8 mask)
 
 		/* Resetting the controller clears many */
 		host->preset_enabled = false;
+
+		if (host->v4_mode)
+			sdhci_do_enable_v4_mode(host);
+
 	}
 }
 
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 23966f8..519d939 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
@@ -565,6 +566,9 @@ struct sdhci_host {
 
 	u64			data_timeout;
 
+	/* Host Version 4 Enable */
+	bool			v4_mode;
+
 	unsigned long private[0] ____cacheline_aligned;
 };
 
@@ -747,5 +751,6 @@ bool sdhci_cqe_irq(struct sdhci_host *host, u32 intmask, int *cmd_error,
 		   int *data_error);
 
 void sdhci_dumpregs(struct sdhci_host *host);
+void sdhci_enable_v4_mode(struct sdhci_host *host);
 
 #endif /* __SDHCI_HW_H */
-- 
2.7.4

Re: [PATCH V4 1/7] mmc: sdhci: add sd host v4 mode

[Adrian Hunter]

On 23/07/18 13:08, Chunyan Zhang wrote:
> 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 introduced an interface to enable
> v4 mode.
> 
> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
> ---
>  drivers/mmc/host/sdhci.c | 28 ++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci.h |  5 +++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1c828e0..cab5350 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -123,6 +123,30 @@ EXPORT_SYMBOL_GPL(sdhci_dumpregs);
>   *                                                                           *
>  \*****************************************************************************/
>  
> +static void sdhci_do_enable_v4_mode(struct sdhci_host *host)
> +{
> +	u16 ctrl2;
> +
> +	ctrl2 = sdhci_readb(host, SDHCI_HOST_CONTROL2);
> +	if (ctrl2 & SDHCI_CTRL_V4_MODE)
> +		return;
> +
> +	ctrl2 |= SDHCI_CTRL_V4_MODE;
> +	sdhci_writeb(host, ctrl2, SDHCI_HOST_CONTROL);
> +}
> +
> +/*
> + * Vendor's Host Controller which supports v4 mode can call
> + * this function to enable v4 mode before calling
> + * __sdhci_add_host().
> + */
> +void sdhci_enable_v4_mode(struct sdhci_host *host)
> +{
> +	host->v4_mode = true;
> +	sdhci_do_enable_v4_mode(host);
> +}
> +EXPORT_SYMBOL_GPL(sdhci_enable_v4_mode);
> +
>  static inline bool sdhci_data_line_cmd(struct mmc_command *cmd)
>  {
>  	return cmd->data || cmd->flags & MMC_RSP_BUSY;
> @@ -224,6 +248,10 @@ static void sdhci_do_reset(struct sdhci_host *host, u8 mask)
>  
>  		/* Resetting the controller clears many */
>  		host->preset_enabled = false;
> +
> +		if (host->v4_mode)
> +			sdhci_do_enable_v4_mode(host);

Instead of sdhci_do_reset() I would rather add this to sdhci_init() and
__sdhci_read_caps()

> +
>  	}
>  }
>  
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 23966f8..519d939 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
> @@ -565,6 +566,9 @@ struct sdhci_host {
>  
>  	u64			data_timeout;
>  
> +	/* Host Version 4 Enable */
> +	bool			v4_mode;

Let's put this with the other bools i.e. after bool irq_wake_enabled;

> +
>  	unsigned long private[0] ____cacheline_aligned;
>  };
>  
> @@ -747,5 +751,6 @@ bool sdhci_cqe_irq(struct sdhci_host *host, u32 intmask, int *cmd_error,
>  		   int *data_error);
>  
>  void sdhci_dumpregs(struct sdhci_host *host);
> +void sdhci_enable_v4_mode(struct sdhci_host *host);
>  
>  #endif /* __SDHCI_HW_H */
> 

[PATCH V4 2/7] mmc: sdhci: Change SDMA address register for v4 mode

[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 | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index cab5350..9cb17c0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -729,7 +729,7 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
 	}
 }
 
-static u32 sdhci_sdma_address(struct sdhci_host *host)
+static dma_addr_t sdhci_sdma_address(struct sdhci_host *host)
 {
 	if (host->bounce_buffer)
 		return host->bounce_addr;
@@ -737,6 +737,18 @@ static u32 sdhci_sdma_address(struct sdhci_host *host)
 		return sg_dma_address(host->data->sg);
 }
 
+static void sdhci_set_sdma_addr(struct sdhci_host *host, dma_addr_t addr)
+{
+	if (host->v4_mode) {
+		sdhci_writel(host, addr, SDHCI_ADMA_ADDRESS);
+		if (host->flags & SDHCI_USE_64_BIT_DMA)
+			sdhci_writel(host, (u64)addr >> 32, SDHCI_ADMA_ADDRESS_HI);
+	} else {
+		sdhci_writel(host, addr, SDHCI_DMA_ADDRESS);
+	}
+
+}
+
 static unsigned int sdhci_target_timeout(struct sdhci_host *host,
 					 struct mmc_command *cmd,
 					 struct mmc_data *data)
@@ -996,8 +1008,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 					     SDHCI_ADMA_ADDRESS_HI);
 		} else {
 			WARN_ON(sg_cnt != 1);
-			sdhci_writel(host, sdhci_sdma_address(host),
-				     SDHCI_DMA_ADDRESS);
+			sdhci_set_sdma_addr(host, sdhci_sdma_address(host));
 		}
 	}
 
@@ -2824,7 +2835,7 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 		 * some controllers are faulty, don't trust them.
 		 */
 		if (intmask & SDHCI_INT_DMA_END) {
-			u32 dmastart, dmanow;
+			dma_addr_t dmastart, dmanow;
 
 			dmastart = sdhci_sdma_address(host);
 			dmanow = dmastart + host->data->bytes_xfered;
@@ -2832,12 +2843,12 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 			 * Force update to the next DMA block boundary.
 			 */
 			dmanow = (dmanow &
-				~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1)) +
+				~((dma_addr_t)SDHCI_DEFAULT_BOUNDARY_SIZE - 1)) +
 				SDHCI_DEFAULT_BOUNDARY_SIZE;
 			host->data->bytes_xfered = dmanow - dmastart;
-			DBG("DMA base 0x%08x, transferred 0x%06x bytes, next 0x%08x\n",
+			DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
 			    dmastart, host->data->bytes_xfered, dmanow);
-			sdhci_writel(host, dmanow, SDHCI_DMA_ADDRESS);
+			sdhci_set_sdma_addr(host, dmanow);
 		}
 
 		if (intmask & SDHCI_INT_DATA_END) {
@@ -3581,8 +3592,8 @@ int sdhci_setup_host(struct sdhci_host *host)
 		}
 	}
 
-	/* SDMA does not support 64-bit DMA */
-	if (host->flags & SDHCI_USE_64_BIT_DMA)
+	/* SDMA does not support 64-bit DMA if v4 mode not set */
+	if ((host->flags & SDHCI_USE_64_BIT_DMA) && !host->v4_mode)
 		host->flags &= ~SDHCI_USE_SDMA;
 
 	if (host->flags & SDHCI_USE_ADMA) {
-- 
2.7.4

Re: [PATCH V4 2/7] mmc: sdhci: Change SDMA address register for v4 mode

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6553 bytes --]

Hi Chunyan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on ulf.hansson-mmc/next]
[also build test WARNING on v4.18-rc6 next-20180723]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chunyan-Zhang/mmc-add-support-for-sdhci-4-0/20180724-045328
base:   git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git next
config: arm-exynos_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from include/linux/delay.h:22,
                    from drivers/mmc/host/sdhci.c:16:
   drivers/mmc/host/sdhci.c: In function 'sdhci_data_irq':
>> drivers/mmc/host/sdhci.c:43:11: warning: format '%p' expects argument of type 'void *', but argument 4 has type 'dma_addr_t {aka unsigned int}' [-Wformat=]
     pr_debug("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
              ^
   include/linux/printk.h:288:21: note: in definition of macro 'pr_fmt'
    #define pr_fmt(fmt) fmt
                        ^~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers/mmc/host/sdhci.c:43:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
     ^~~~~~~~
   drivers/mmc/host/sdhci.c:2849:4: note: in expansion of macro 'DBG'
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
       ^~~
   drivers/mmc/host/sdhci.c:2849:19: note: format string is defined here
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
                     ~^
                     %d
   In file included from include/linux/kernel.h:14:0,
                    from include/linux/delay.h:22,
                    from drivers/mmc/host/sdhci.c:16:
   drivers/mmc/host/sdhci.c:43:11: warning: format '%p' expects argument of type 'void *', but argument 6 has type 'dma_addr_t {aka unsigned int}' [-Wformat=]
     pr_debug("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
              ^
   include/linux/printk.h:288:21: note: in definition of macro 'pr_fmt'
    #define pr_fmt(fmt) fmt
                        ^~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers/mmc/host/sdhci.c:43:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
     ^~~~~~~~
   drivers/mmc/host/sdhci.c:2849:4: note: in expansion of macro 'DBG'
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
       ^~~
   drivers/mmc/host/sdhci.c:2849:56: note: format string is defined here
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
                                                          ~^
                                                          %d

vim +43 drivers/mmc/host/sdhci.c

d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24 @16  #include <linux/delay.h>
5a436cc0a drivers/mmc/host/sdhci.c Adrian Hunter         2017-03-20  17  #include <linux/ktime.h>
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  18  #include <linux/highmem.h>
b8c86fc5d drivers/mmc/host/sdhci.c Pierre Ossman         2008-03-18  19  #include <linux/io.h>
88b476797 drivers/mmc/host/sdhci.c Paul Gortmaker        2011-07-03  20  #include <linux/module.h>
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  21  #include <linux/dma-mapping.h>
5a0e3ad6a drivers/mmc/host/sdhci.c Tejun Heo             2010-03-24  22  #include <linux/slab.h>
117636092 drivers/mmc/host/sdhci.c Ralf Baechle          2007-10-23  23  #include <linux/scatterlist.h>
bd9b90279 drivers/mmc/host/sdhci.c Linus Walleij         2018-01-29  24  #include <linux/sizes.h>
250dcd114 drivers/mmc/host/sdhci.c Ulf Hansson           2017-11-27  25  #include <linux/swiotlb.h>
9bea3c850 drivers/mmc/host/sdhci.c Marek Szyprowski      2010-08-10  26  #include <linux/regulator/consumer.h>
66fd8ad51 drivers/mmc/host/sdhci.c Adrian Hunter         2011-10-03  27  #include <linux/pm_runtime.h>
92e0c44b9 drivers/mmc/host/sdhci.c Zach Brown            2016-11-02  28  #include <linux/of.h>
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  29  
2f730fec8 drivers/mmc/host/sdhci.c Pierre Ossman         2008-03-17  30  #include <linux/leds.h>
2f730fec8 drivers/mmc/host/sdhci.c Pierre Ossman         2008-03-17  31  
22113efd0 drivers/mmc/host/sdhci.c Aries Lee             2010-12-15  32  #include <linux/mmc/mmc.h>
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  33  #include <linux/mmc/host.h>
473b095a7 drivers/mmc/host/sdhci.c Aaron Lu              2012-07-03  34  #include <linux/mmc/card.h>
85cc1c331 drivers/mmc/host/sdhci.c Corneliu Doban        2015-02-09  35  #include <linux/mmc/sdio.h>
bec9d4e59 drivers/mmc/host/sdhci.c Guennadi Liakhovetski 2012-09-17  36  #include <linux/mmc/slot-gpio.h>
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  37  
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  38  #include "sdhci.h"
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  39  
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  40  #define DRIVER_NAME "sdhci"
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  41  
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  42  #define DBG(f, x...) \
f421865d5 drivers/mmc/host/sdhci.c Adrian Hunter         2017-03-20 @43  	pr_debug("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
d129bceb1 drivers/mmc/sdhci.c      Pierre Ossman         2006-03-24  44  

:::::: The code at line 43 was first introduced by commit
:::::: f421865d5b4ce57013040fb1700edceb43a14b42 mmc: sdhci: Improve debug print format

:::::: TO: Adrian Hunter <adrian.hunter@intel.com>
:::::: CC: Ulf Hansson <ulf.hansson@linaro.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28339 bytes --]

Re: [PATCH V4 2/7] mmc: sdhci: Change SDMA address register for v4 mode

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7231 bytes --]

Hi Chunyan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on ulf.hansson-mmc/next]
[also build test WARNING on v4.18-rc6 next-20180723]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chunyan-Zhang/mmc-add-support-for-sdhci-4-0/20180724-045328
base:   git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git next
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from include/linux/delay.h:22,
                    from drivers/mmc//host/sdhci.c:16:
   drivers/mmc//host/sdhci.c: In function 'sdhci_data_irq':
>> include/linux/kern_levels.h:5:18: warning: format '%p' expects argument of type 'void *', but argument 3 has type 'dma_addr_t {aka unsigned int}' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:136:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/mmc//host/sdhci.c:43:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
     ^~~~~~~~
   drivers/mmc//host/sdhci.c:2849:4: note: in expansion of macro 'DBG'
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
       ^~~
   drivers/mmc//host/sdhci.c:2849:19: note: format string is defined here
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
                     ~^
                     %d
   In file included from include/linux/kernel.h:14:0,
                    from include/linux/delay.h:22,
                    from drivers/mmc//host/sdhci.c:16:
   include/linux/kern_levels.h:5:18: warning: format '%p' expects argument of type 'void *', but argument 5 has type 'dma_addr_t {aka unsigned int}' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:136:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/mmc//host/sdhci.c:43:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
     ^~~~~~~~
   drivers/mmc//host/sdhci.c:2849:4: note: in expansion of macro 'DBG'
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
       ^~~
   drivers/mmc//host/sdhci.c:2849:56: note: format string is defined here
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
                                                          ~^
                                                          %d
--
   In file included from include/linux/kernel.h:14:0,
                    from include/linux/delay.h:22,
                    from drivers/mmc/host/sdhci.c:16:
   drivers/mmc/host/sdhci.c: In function 'sdhci_data_irq':
>> include/linux/kern_levels.h:5:18: warning: format '%p' expects argument of type 'void *', but argument 3 has type 'dma_addr_t {aka unsigned int}' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:136:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/mmc/host/sdhci.c:43:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
     ^~~~~~~~
   drivers/mmc/host/sdhci.c:2849:4: note: in expansion of macro 'DBG'
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
       ^~~
   drivers/mmc/host/sdhci.c:2849:19: note: format string is defined here
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
                     ~^
                     %d
   In file included from include/linux/kernel.h:14:0,
                    from include/linux/delay.h:22,
                    from drivers/mmc/host/sdhci.c:16:
   include/linux/kern_levels.h:5:18: warning: format '%p' expects argument of type 'void *', but argument 5 has type 'dma_addr_t {aka unsigned int}' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:136:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/mmc/host/sdhci.c:43:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
     ^~~~~~~~
   drivers/mmc/host/sdhci.c:2849:4: note: in expansion of macro 'DBG'
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
       ^~~
   drivers/mmc/host/sdhci.c:2849:56: note: format string is defined here
       DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
                                                          ~^
                                                          %d

vim +5 include/linux/kern_levels.h

314ba352 Joe Perches 2012-07-30  4  
04d2c8c8 Joe Perches 2012-07-30 @5  #define KERN_SOH	"\001"		/* ASCII Start Of Header */
04d2c8c8 Joe Perches 2012-07-30  6  #define KERN_SOH_ASCII	'\001'
04d2c8c8 Joe Perches 2012-07-30  7  

:::::: The code at line 5 was first introduced by commit
:::::: 04d2c8c83d0e3ac5f78aeede51babb3236200112 printk: convert the format for KERN_<LEVEL> to a 2 byte pattern

:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43902 bytes --]

[PATCH V4 2/7] mmc: sdhci: Change SDMA address register for v4 mode

[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 | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index cab5350..b7ad8e5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -729,7 +729,7 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
 	}
 }
 
-static u32 sdhci_sdma_address(struct sdhci_host *host)
+static dma_addr_t sdhci_sdma_address(struct sdhci_host *host)
 {
 	if (host->bounce_buffer)
 		return host->bounce_addr;
@@ -737,6 +737,18 @@ static u32 sdhci_sdma_address(struct sdhci_host *host)
 		return sg_dma_address(host->data->sg);
 }
 
+static void sdhci_set_sdma_addr(struct sdhci_host *host, dma_addr_t addr)
+{
+	if (host->v4_mode) {
+		sdhci_writel(host, addr, SDHCI_ADMA_ADDRESS);
+		if (host->flags & SDHCI_USE_64_BIT_DMA)
+			sdhci_writel(host, (u64)addr >> 32, SDHCI_ADMA_ADDRESS_HI);
+	} else {
+		sdhci_writel(host, addr, SDHCI_DMA_ADDRESS);
+	}
+
+}
+
 static unsigned int sdhci_target_timeout(struct sdhci_host *host,
 					 struct mmc_command *cmd,
 					 struct mmc_data *data)
@@ -996,8 +1008,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 					     SDHCI_ADMA_ADDRESS_HI);
 		} else {
 			WARN_ON(sg_cnt != 1);
-			sdhci_writel(host, sdhci_sdma_address(host),
-				     SDHCI_DMA_ADDRESS);
+			sdhci_set_sdma_addr(host, sdhci_sdma_address(host));
 		}
 	}
 
@@ -2824,7 +2835,7 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 		 * some controllers are faulty, don't trust them.
 		 */
 		if (intmask & SDHCI_INT_DMA_END) {
-			u32 dmastart, dmanow;
+			dma_addr_t dmastart, dmanow;
 
 			dmastart = sdhci_sdma_address(host);
 			dmanow = dmastart + host->data->bytes_xfered;
@@ -2832,12 +2843,12 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 			 * Force update to the next DMA block boundary.
 			 */
 			dmanow = (dmanow &
-				~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1)) +
+				~((dma_addr_t)SDHCI_DEFAULT_BOUNDARY_SIZE - 1)) +
 				SDHCI_DEFAULT_BOUNDARY_SIZE;
 			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);
+			DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
+			    &dmastart, host->data->bytes_xfered, &dmanow);
+			sdhci_set_sdma_addr(host, dmanow);
 		}
 
 		if (intmask & SDHCI_INT_DATA_END) {
@@ -3581,8 +3592,8 @@ int sdhci_setup_host(struct sdhci_host *host)
 		}
 	}
 
-	/* SDMA does not support 64-bit DMA */
-	if (host->flags & SDHCI_USE_64_BIT_DMA)
+	/* SDMA does not support 64-bit DMA if v4 mode not set */
+	if ((host->flags & SDHCI_USE_64_BIT_DMA) && !host->v4_mode)
 		host->flags &= ~SDHCI_USE_SDMA;
 
 	if (host->flags & SDHCI_USE_ADMA) {
-- 
2.7.4

Re: [PATCH V4 2/7] mmc: sdhci: Change SDMA address register for v4 mode

[Adrian Hunter]

On 24/07/18 05:47, Chunyan Zhang wrote:
> 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 | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index cab5350..b7ad8e5 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -729,7 +729,7 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
>  	}
>  }
>  
> -static u32 sdhci_sdma_address(struct sdhci_host *host)
> +static dma_addr_t sdhci_sdma_address(struct sdhci_host *host)
>  {
>  	if (host->bounce_buffer)
>  		return host->bounce_addr;
> @@ -737,6 +737,18 @@ static u32 sdhci_sdma_address(struct sdhci_host *host)
>  		return sg_dma_address(host->data->sg);
>  }
>  
> +static void sdhci_set_sdma_addr(struct sdhci_host *host, dma_addr_t addr)
> +{
> +	if (host->v4_mode) {
> +		sdhci_writel(host, addr, SDHCI_ADMA_ADDRESS);
> +		if (host->flags & SDHCI_USE_64_BIT_DMA)
> +			sdhci_writel(host, (u64)addr >> 32, SDHCI_ADMA_ADDRESS_HI);
> +	} else {
> +		sdhci_writel(host, addr, SDHCI_DMA_ADDRESS);
> +	}
> +

Please remove this blank line

> +}
> +
>  static unsigned int sdhci_target_timeout(struct sdhci_host *host,
>  					 struct mmc_command *cmd,
>  					 struct mmc_data *data)
> @@ -996,8 +1008,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
>  					     SDHCI_ADMA_ADDRESS_HI);
>  		} else {
>  			WARN_ON(sg_cnt != 1);
> -			sdhci_writel(host, sdhci_sdma_address(host),
> -				     SDHCI_DMA_ADDRESS);
> +			sdhci_set_sdma_addr(host, sdhci_sdma_address(host));
>  		}
>  	}
>  
> @@ -2824,7 +2835,7 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
>  		 * some controllers are faulty, don't trust them.
>  		 */
>  		if (intmask & SDHCI_INT_DMA_END) {
> -			u32 dmastart, dmanow;
> +			dma_addr_t dmastart, dmanow;
>  
>  			dmastart = sdhci_sdma_address(host);
>  			dmanow = dmastart + host->data->bytes_xfered;
> @@ -2832,12 +2843,12 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
>  			 * Force update to the next DMA block boundary.
>  			 */
>  			dmanow = (dmanow &
> -				~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1)) +
> +				~((dma_addr_t)SDHCI_DEFAULT_BOUNDARY_SIZE - 1)) +
>  				SDHCI_DEFAULT_BOUNDARY_SIZE;
>  			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);
> +			DBG("DMA base %pad, transferred 0x%06x bytes, next %pad\n",
> +			    &dmastart, host->data->bytes_xfered, &dmanow);
> +			sdhci_set_sdma_addr(host, dmanow);
>  		}
>  
>  		if (intmask & SDHCI_INT_DATA_END) {
> @@ -3581,8 +3592,8 @@ int sdhci_setup_host(struct sdhci_host *host)
>  		}
>  	}
>  
> -	/* SDMA does not support 64-bit DMA */
> -	if (host->flags & SDHCI_USE_64_BIT_DMA)
> +	/* SDMA does not support 64-bit DMA if v4 mode not set */
> +	if ((host->flags & SDHCI_USE_64_BIT_DMA) && !host->v4_mode)
>  		host->flags &= ~SDHCI_USE_SDMA;
>  
>  	if (host->flags & SDHCI_USE_ADMA) {
> 

[PATCH V4 3/7] 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 | 90 ++++++++++++++++++++++++++++++++++--------------
 drivers/mmc/host/sdhci.h | 15 ++++++--
 2 files changed, 78 insertions(+), 27 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9cb17c0..ce71afa 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -271,6 +271,46 @@ static void sdhci_set_default_irqs(struct sdhci_host *host)
 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
 }
 
+static void sdhci_config_dma(struct sdhci_host *host)
+{
+	u8 ctrl;
+	u16 ctrl2;
+
+	if (host->version < SDHCI_SPEC_200)
+		return;
+
+	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+
+	/*
+	 * Always adjust the DMA selection as some controllers
+	 * (e.g. JMicron) can't do PIO properly when the selection
+	 * is ADMA.
+	 */
+	ctrl &= ~SDHCI_CTRL_DMA_MASK;
+	if ((host->flags & SDHCI_REQ_USE_DMA) &&
+	    (host->flags & SDHCI_USE_ADMA))
+		ctrl |= SDHCI_CTRL_ADMA32;
+
+	if (host->flags & SDHCI_USE_64_BIT_DMA) {
+		/*
+		 * If v4 mode, all supported DMA can be 64-bit addressing if
+		 * controller supports 64-bit system address, otherwise only
+		 * ADMA can support 64-bit addressing.
+		 */
+		if (host->v4_mode) {
+			ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+			ctrl2 |= SDHCI_CTRL_64BIT_ADDR;
+			sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
+		} else {
+			if ((host->flags & SDHCI_REQ_USE_DMA) &&
+			    (host->flags & SDHCI_USE_ADMA))
+				ctrl |= SDHCI_CTRL_ADMA64;
+		}
+	}
+
+	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+}
+
 static void sdhci_init(struct sdhci_host *host, int soft)
 {
 	struct mmc_host *mmc = host->mmc;
@@ -916,7 +956,6 @@ 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;
 	struct mmc_data *data = cmd->data;
 
 	host->data_timeout = 0;
@@ -1012,25 +1051,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 		}
 	}
 
-	/*
-	 * Always adjust the DMA selection as some controllers
-	 * (e.g. JMicron) can't do PIO properly when the selection
-	 * is ADMA.
-	 */
-	if (host->version >= SDHCI_SPEC_200) {
-		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
-		ctrl &= ~SDHCI_CTRL_DMA_MASK;
-		if ((host->flags & SDHCI_REQ_USE_DMA) &&
-			(host->flags & SDHCI_USE_ADMA)) {
-			if (host->flags & SDHCI_USE_64_BIT_DMA)
-				ctrl |= SDHCI_CTRL_ADMA64;
-			else
-				ctrl |= SDHCI_CTRL_ADMA32;
-		} else {
-			ctrl |= SDHCI_CTRL_SDMA;
-		}
-		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
-	}
+	sdhci_config_dma(host);
 
 	if (!(host->flags & SDHCI_REQ_USE_DMA)) {
 		int flags;
@@ -3503,6 +3524,19 @@ static int sdhci_allocate_bounce_buffer(struct sdhci_host *host)
 	return 0;
 }
 
+static inline bool sdhci_can_64bit_dma(struct sdhci_host *host)
+{
+	/*
+	 * 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.
+	 */
+	if (host->version >= SDHCI_SPEC_410 && host->v4_mode)
+		return host->caps & SDHCI_CAN_64BIT_V4;
+
+	return host->caps & SDHCI_CAN_64BIT;
+}
+
 int sdhci_setup_host(struct sdhci_host *host)
 {
 	struct mmc_host *mmc;
@@ -3539,7 +3573,7 @@ int sdhci_setup_host(struct sdhci_host *host)
 
 	override_timeout_clk = host->timeout_clk;
 
-	if (host->version > SDHCI_SPEC_300) {
+	if (host->version > SDHCI_SPEC_420) {
 		pr_err("%s: Unknown controller version (%d). You may experience problems.\n",
 		       mmc_hostname(mmc), host->version);
 	}
@@ -3574,7 +3608,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_can_64bit_dma(host))
 		host->flags |= SDHCI_USE_64_BIT_DMA;
 
 	if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) {
@@ -3608,8 +3642,8 @@ 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;
@@ -3617,7 +3651,13 @@ int sdhci_setup_host(struct sdhci_host *host)
 		}
 
 		host->align_buffer_sz = SDHCI_MAX_SEGS * SDHCI_ADMA2_ALIGN;
-		buf = dma_alloc_coherent(mmc_dev(mmc), host->align_buffer_sz +
+		/*
+		 * Host Controller Version 4.00 or later can support 128-bit
+		 * and 96-bit Descriptor for 64-bit addressing mode. 128-bit
+		 * Descriptor is for v4 mode, and high 32-bit of it is reserved
+		 * according to the specification v4.10.
+		 */
+		buf = dma_zalloc_coherent(mmc_dev(mmc), host->align_buffer_sz +
 					 host->adma_table_sz, &dma, GFP_KERNEL);
 		if (!buf) {
 			pr_warn("%s: Unable to allocate ADMA buffers - falling back to standard DMA\n",
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 519d939..23318ff 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
@@ -205,6 +206,7 @@
 #define  SDHCI_CAN_VDD_330	0x01000000
 #define  SDHCI_CAN_VDD_300	0x02000000
 #define  SDHCI_CAN_VDD_180	0x04000000
+#define  SDHCI_CAN_64BIT_V4	0x08000000
 #define  SDHCI_CAN_64BIT	0x10000000
 
 #define  SDHCI_SUPPORT_SDR50	0x00000001
@@ -271,6 +273,9 @@
 #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
+#define   SDHCI_SPEC_420	5
 
 /*
  * End of controller registers.
@@ -306,8 +311,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

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

[Adrian Hunter]

On 23/07/18 13:08, Chunyan Zhang wrote:
> 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 | 90 ++++++++++++++++++++++++++++++++++--------------
>  drivers/mmc/host/sdhci.h | 15 ++++++--
>  2 files changed, 78 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 9cb17c0..ce71afa 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -271,6 +271,46 @@ static void sdhci_set_default_irqs(struct sdhci_host *host)
>  	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>  }
>  
> +static void sdhci_config_dma(struct sdhci_host *host)
> +{
> +	u8 ctrl;
> +	u16 ctrl2;
> +
> +	if (host->version < SDHCI_SPEC_200)
> +		return;
> +
> +	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> +
> +	/*
> +	 * Always adjust the DMA selection as some controllers
> +	 * (e.g. JMicron) can't do PIO properly when the selection
> +	 * is ADMA.
> +	 */
> +	ctrl &= ~SDHCI_CTRL_DMA_MASK;
> +	if ((host->flags & SDHCI_REQ_USE_DMA) &&

	if (!(host->flags & SDHCI_REQ_USE_DMA))
		goto out;

> +	    (host->flags & SDHCI_USE_ADMA))
> +		ctrl |= SDHCI_CTRL_ADMA32;

	/* Note if DMA Select is zero then SDMA is selected */
	if (host->flags & SDHCI_USE_ADMA)
		ctrl |= SDHCI_CTRL_ADMA32;

> +
> +	if (host->flags & SDHCI_USE_64_BIT_DMA) {
> +		/*
> +		 * If v4 mode, all supported DMA can be 64-bit addressing if
> +		 * controller supports 64-bit system address, otherwise only
> +		 * ADMA can support 64-bit addressing.
> +		 */
> +		if (host->v4_mode) {
> +			ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +			ctrl2 |= SDHCI_CTRL_64BIT_ADDR;
> +			sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
> +		} else {
> +			if ((host->flags & SDHCI_REQ_USE_DMA) &&

The 'else' and 'if' should be together i.e.

		} else if (host->flags & SDHCI_USE_ADMA) {
			/*
			 * Don't need to undo SDHCI_CTRL_ADMA32 in order to set
			 * SDHCI_CTRL_ADMA64.
			 */
			ctrl |= SDHCI_CTRL_ADMA64;
		}

> +			    (host->flags & SDHCI_USE_ADMA))
> +				ctrl |= SDHCI_CTRL_ADMA64;
> +		}
> +	}
> +

out:

> +	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> +}
> +
>  static void sdhci_init(struct sdhci_host *host, int soft)
>  {
>  	struct mmc_host *mmc = host->mmc;
> @@ -916,7 +956,6 @@ 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;
>  	struct mmc_data *data = cmd->data;
>  
>  	host->data_timeout = 0;
> @@ -1012,25 +1051,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
>  		}
>  	}
>  
> -	/*
> -	 * Always adjust the DMA selection as some controllers
> -	 * (e.g. JMicron) can't do PIO properly when the selection
> -	 * is ADMA.
> -	 */
> -	if (host->version >= SDHCI_SPEC_200) {
> -		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> -		ctrl &= ~SDHCI_CTRL_DMA_MASK;
> -		if ((host->flags & SDHCI_REQ_USE_DMA) &&
> -			(host->flags & SDHCI_USE_ADMA)) {
> -			if (host->flags & SDHCI_USE_64_BIT_DMA)
> -				ctrl |= SDHCI_CTRL_ADMA64;
> -			else
> -				ctrl |= SDHCI_CTRL_ADMA32;
> -		} else {
> -			ctrl |= SDHCI_CTRL_SDMA;
> -		}
> -		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> -	}
> +	sdhci_config_dma(host);
>  
>  	if (!(host->flags & SDHCI_REQ_USE_DMA)) {
>  		int flags;
> @@ -3503,6 +3524,19 @@ static int sdhci_allocate_bounce_buffer(struct sdhci_host *host)
>  	return 0;
>  }
>  
> +static inline bool sdhci_can_64bit_dma(struct sdhci_host *host)
> +{
> +	/*
> +	 * 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.
> +	 */
> +	if (host->version >= SDHCI_SPEC_410 && host->v4_mode)
> +		return host->caps & SDHCI_CAN_64BIT_V4;
> +
> +	return host->caps & SDHCI_CAN_64BIT;
> +}
> +
>  int sdhci_setup_host(struct sdhci_host *host)
>  {
>  	struct mmc_host *mmc;
> @@ -3539,7 +3573,7 @@ int sdhci_setup_host(struct sdhci_host *host)
>  
>  	override_timeout_clk = host->timeout_clk;
>  
> -	if (host->version > SDHCI_SPEC_300) {
> +	if (host->version > SDHCI_SPEC_420) {

Please make this and the addition of the SDHCI_SPEC_4xx defines
a separate patch.

>  		pr_err("%s: Unknown controller version (%d). You may experience problems.\n",
>  		       mmc_hostname(mmc), host->version);
>  	}
> @@ -3574,7 +3608,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_can_64bit_dma(host))
>  		host->flags |= SDHCI_USE_64_BIT_DMA;
>  
>  	if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) {
> @@ -3608,8 +3642,8 @@ 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;
> @@ -3617,7 +3651,13 @@ int sdhci_setup_host(struct sdhci_host *host)
>  		}
>  
>  		host->align_buffer_sz = SDHCI_MAX_SEGS * SDHCI_ADMA2_ALIGN;
> -		buf = dma_alloc_coherent(mmc_dev(mmc), host->align_buffer_sz +
> +		/*
> +		 * Host Controller Version 4.00 or later can support 128-bit
> +		 * and 96-bit Descriptor for 64-bit addressing mode. 128-bit
> +		 * Descriptor is for v4 mode, and high 32-bit of it is reserved
> +		 * according to the specification v4.10.
> +		 */

But the point is zalloc() lets us skip writing the reserved bits.
How about:

		/*
		 * Use zalloc to zero the reserved high 32-bits of 128-bit
		 * descriptors so that they never need to be written.
		 */

> +		buf = dma_zalloc_coherent(mmc_dev(mmc), host->align_buffer_sz +
>  					 host->adma_table_sz, &dma, GFP_KERNEL);
>  		if (!buf) {
>  			pr_warn("%s: Unable to allocate ADMA buffers - falling back to standard DMA\n",
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 519d939..23318ff 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
> @@ -205,6 +206,7 @@
>  #define  SDHCI_CAN_VDD_330	0x01000000
>  #define  SDHCI_CAN_VDD_300	0x02000000
>  #define  SDHCI_CAN_VDD_180	0x04000000
> +#define  SDHCI_CAN_64BIT_V4	0x08000000
>  #define  SDHCI_CAN_64BIT	0x10000000
>  
>  #define  SDHCI_SUPPORT_SDR50	0x00000001
> @@ -271,6 +273,9 @@
>  #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
> +#define   SDHCI_SPEC_420	5
>  
>  /*
>   * End of controller registers.
> @@ -306,8 +311,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
> 

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

[Chunyan Zhang]

Host Controller Version 4.10 re-defines SDMA System Address register
as 32-bit Block Count for v4 mode, and SDMA uses ADMA System
Address register (05Fh-058h) instead if v4 mode is enabled. Also
when using 32-bit block count, 16-bit block count register need
to be set to zero.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ce71afa..5acea3d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -956,6 +956,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)
 {
+	u32 reg;
 	struct mmc_data *data = cmd->data;
 
 	host->data_timeout = 0;
@@ -1070,7 +1071,19 @@ 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);
+
+	/*
+	 * For Version 4.10 onwards, if v4 mode is enabled, 16-bit Block Count
+	 * register need to be set to zero, 32-bit Block Count register would
+	 * be selected.
+	 */
+	if (host->version >= SDHCI_SPEC_410 && host->v4_mode) {
+		if (sdhci_readw(host, SDHCI_BLOCK_COUNT))
+			sdhci_writew(host, 0, SDHCI_BLOCK_COUNT);
+		sdhci_writew(host, data->blocks, SDHCI_32BIT_BLK_CNT);
+	} else {
+		sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
+	}
 }
 
 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 23318ff..81aae07 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 V4 4/7] mmc: sdhci: add 32-bit block count support for v4 mode

[Chunyan Zhang]

Host Controller Version 4.10 re-defines SDMA System Address register
as 32-bit Block Count for v4 mode, and SDMA uses ADMA System
Address register (05Fh-058h) instead if v4 mode is enabled. Also
when using 32-bit block count, 16-bit block count register need
to be set to zero.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 920d8ec..c272a2b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1070,7 +1070,19 @@ 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);
+
+	/*
+	 * For Version 4.10 onwards, if v4 mode is enabled, 16-bit Block Count
+	 * register need to be set to zero, 32-bit Block Count register would
+	 * be selected.
+	 */
+	if (host->version >= SDHCI_SPEC_410 && host->v4_mode) {
+		if (sdhci_readw(host, SDHCI_BLOCK_COUNT))
+			sdhci_writew(host, 0, SDHCI_BLOCK_COUNT);
+		sdhci_writew(host, data->blocks, SDHCI_32BIT_BLK_CNT);
+	} else {
+		sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
+	}
 }
 
 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 23318ff..81aae07 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

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

[Adrian Hunter]

On 24/07/18 05:51, Chunyan Zhang wrote:
> Host Controller Version 4.10 re-defines SDMA System Address register
> as 32-bit Block Count for v4 mode, and SDMA uses ADMA System
> Address register (05Fh-058h) instead if v4 mode is enabled. Also
> when using 32-bit block count, 16-bit block count register need
> to be set to zero.
> 
> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
> ---
>  drivers/mmc/host/sdhci.c | 14 +++++++++++++-
>  drivers/mmc/host/sdhci.h |  1 +
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 920d8ec..c272a2b 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1070,7 +1070,19 @@ 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);
> +
> +	/*
> +	 * For Version 4.10 onwards, if v4 mode is enabled, 16-bit Block Count
> +	 * register need to be set to zero, 32-bit Block Count register would
> +	 * be selected.
> +	 */
> +	if (host->version >= SDHCI_SPEC_410 && host->v4_mode) {
> +		if (sdhci_readw(host, SDHCI_BLOCK_COUNT))
> +			sdhci_writew(host, 0, SDHCI_BLOCK_COUNT);
> +		sdhci_writew(host, data->blocks, SDHCI_32BIT_BLK_CNT);

So this is also SDHCI_ARGUMENT2 which is why there is a conflict with
auto-CMD23.  We need to write SDHCI_32BIT_BLK_CNT only once, but also cater
for eMMC which uses the CMD23 argument for more than just block count.

> +	} else {
> +		sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
> +	}
>  }
>  
>  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 23318ff..81aae07 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))
> 

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

[Chunyan Zhang]

Hi Adrian,

On 30 July 2018 at 21:05, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 24/07/18 05:51, Chunyan Zhang wrote:
>> Host Controller Version 4.10 re-defines SDMA System Address register
>> as 32-bit Block Count for v4 mode, and SDMA uses ADMA System
>> Address register (05Fh-058h) instead if v4 mode is enabled. Also
>> when using 32-bit block count, 16-bit block count register need
>> to be set to zero.
>>
>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>> ---
>>  drivers/mmc/host/sdhci.c | 14 +++++++++++++-
>>  drivers/mmc/host/sdhci.h |  1 +
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 920d8ec..c272a2b 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1070,7 +1070,19 @@ 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);
>> +
>> +     /*
>> +      * For Version 4.10 onwards, if v4 mode is enabled, 16-bit Block Count
>> +      * register need to be set to zero, 32-bit Block Count register would
>> +      * be selected.
>> +      */
>> +     if (host->version >= SDHCI_SPEC_410 && host->v4_mode) {
>> +             if (sdhci_readw(host, SDHCI_BLOCK_COUNT))
>> +                     sdhci_writew(host, 0, SDHCI_BLOCK_COUNT);
>> +             sdhci_writew(host, data->blocks, SDHCI_32BIT_BLK_CNT);
>
> So this is also SDHCI_ARGUMENT2 which is why there is a conflict with
> auto-CMD23.  We need to write SDHCI_32BIT_BLK_CNT only once, but also cater
> for eMMC which uses the CMD23 argument for more than just block count.
>

What you would suggest on how should we change here?

I've double checked with the hardware fellow within the company, the
sd host controller v4 (on our platform at least) would  use this
register as block count only, that's saying that it would not deal
with the flags (i.e. reliable write and data tag) of CMD23.

Thanks,
Chunyan

>> +     } else {
>> +             sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
>> +     }
>>  }
>>
>>  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 23318ff..81aae07 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))
>>
>

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

[Adrian Hunter]

On 07/08/18 04:58, Jason Wu (吴霁爽) wrote:
>  
> 
> According the information of following picture, in this case I think, we
> just have 2 choice in sdio v4.1:
> 
> 1 do not define SDHCI_AUTO_CMD23: argument of cmd23 will define in register
> 0x8(Argument register)

That would only need to be done for requests that use the upper bits. i.e.
Perhaps hook host->mmc_host_ops->request() and check if sbc argument is ok.
Then toggle SDHCI_AUTO_CMD23 accordingly.

> 
> 2 or define SDHCI_USE_ADMA: block count of cmd18/28 will be get in “Command
> Descriptor for SD Mod”of ADMA. And block cnt register will be used as
> parameter of auto cmd23.
> 
> -----Original Message-----
> From: Chunyan Zhang [mailto:zhang.lyra@gmail.com]
> Sent: Monday, August 06, 2018 7:29 PM
> To: Adrian Hunter
> Cc: Chunyan Zhang; Ulf Hansson; linux-mmc@vger.kernel.org; Linux Kernel
> Mailing List; Orson Zhai; Baolin Wang; Billows Wu (武洪涛); Jason Wu (吴霁爽)
> Subject: Re: [PATCH V4 4/7] mmc: sdhci: add 32-bit block count support for
> v4 mode
> 
>  
> 
> Hi Adrian,
> 
>  
> 
> On 30 July 2018 at 21:05, Adrian Hunter <adrian.hunter@intel.com
> <mailto:adrian.hunter@intel.com>> wrote:
> 
>> On 24/07/18 05:51, Chunyan Zhang wrote:
> 
>>> Host Controller Version 4.10 re-defines SDMA System Address register
> 
>>> as 32-bit Block Count for v4 mode, and SDMA uses ADMA System Address
> 
>>> register (05Fh-058h) instead if v4 mode is enabled. Also when using
> 
>>> 32-bit block count, 16-bit block count register need to be set to
> 
>>> zero.
> 
>>> 
> 
>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org <mailto:zhang.chunyan@linaro.org>>
> 
>>> ---
> 
>>>  drivers/mmc/host/sdhci.c | 14 +++++++++++++- 
> 
>>> drivers/mmc/host/sdhci.h |  1 +
> 
>>>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
>>> 
> 
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> 
>>> index 920d8ec..c272a2b 100644
> 
>>> --- a/drivers/mmc/host/sdhci.c
> 
>>> +++ b/drivers/mmc/host/sdhci.c
> 
>>> @@ -1070,7 +1070,19 @@ 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);
> 
>>> +
> 
>>> +     /*
> 
>>> +      * For Version 4.10 onwards, if v4 mode is enabled, 16-bit Block Count
> 
>>> +      * register need to be set to zero, 32-bit Block Count register would
> 
>>> +      * be selected.
> 
>>> +      */
> 
>>> +     if (host->version >= SDHCI_SPEC_410 && host->v4_mode) {
> 
>>> +             if (sdhci_readw(host, SDHCI_BLOCK_COUNT))
> 
>>> +                     sdhci_writew(host, 0, SDHCI_BLOCK_COUNT);
> 
>>> +             sdhci_writew(host, data->blocks, SDHCI_32BIT_BLK_CNT);
> 
>> 
> 
>> So this is also SDHCI_ARGUMENT2 which is why there is a conflict with
> 
>> auto-CMD23.  We need to write SDHCI_32BIT_BLK_CNT only once, but also
> 
>> cater for eMMC which uses the CMD23 argument for more than just block count.
> 
>> 
> 
>  
> 
> What you would suggest on how should we change here?
> 
>  
> 
> I've double checked with the hardware fellow within the company, the sd host
> controller v4 (on our platform at least) would  use this register as block
> count only, that's saying that it would not deal with the flags (i.e.
> reliable write and data tag) of CMD23.
> 
>  
> 
> Thanks,
> 
> Chunyan
> 
>  
> 
>>> +     } else {
> 
>>> +             sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
> 
>>> +     }
> 
>>>  }
> 
>>> 
> 
>>>  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
> 
>>> 23318ff..81aae07 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))
> 
>>> 
> 
>> 
> 

[PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Chunyan Zhang]

As SD Host Controller Specification v4.10 documents:
Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
Selection of Auto CMD depends on setting of CMD23 Enable in the Host
Control 2 register which indicates whether card supports CMD23. If CMD23
Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
recommended rather than use of Auto CMD12 Enable or Auto CMD23
Enable.

This patch add this new mode support.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5acea3d..5c60590 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
 	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 }
 
+static void sdhci_enable_cmd23(struct sdhci_host *host)
+{
+	u16 ctrl2;
+
+	/*
+	 * This is used along with "Auto CMD Auto Select" feature,
+	 * which is introduced from v4.10, if card supports CMD23,
+	 * Auto CMD23 should be used instead of Auto CMD12.
+	 */
+	if (host->version >= SDHCI_SPEC_410 &&
+	    (host->mmc->caps & MMC_CAP_CMD23)) {
+		ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		ctrl2 |= SDHCI_CMD23_ENABLE;
+		sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
+	}
+}
+
 static void sdhci_init(struct sdhci_host *host, int soft)
 {
 	struct mmc_host *mmc = host->mmc;
@@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
 		host->clock = 0;
 		mmc->ops->set_ios(mmc, &mmc->ios);
 	}
+
+	sdhci_enable_cmd23(host);
 }
 
 static void sdhci_reinit(struct sdhci_host *host)
@@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
 	       !mrq->cap_cmd_during_tfr;
 }
 
+static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
+					 struct mmc_command *cmd,
+					 u16 *mode)
+{
+	bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
+			 (cmd->opcode != SD_IO_RW_EXTENDED);
+	bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
+
+	/*
+	 * In case of Version 4.10 or later, use of 'Auto CMD Auto
+	 * Select' is recommended rather than use of 'Auto CMD12
+	 * Enable' or 'Auto CMD23 Enable'.
+	 */
+	if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
+		*mode |= SDHCI_TRNS_AUTO_SEL;
+		return;
+	}
+
+	/*
+	 * If we are sending CMD23, CMD12 never gets sent
+	 * on successful completion (so no Auto-CMD12).
+	 */
+	if (use_cmd12) {
+		*mode |= SDHCI_TRNS_AUTO_CMD12;
+	} else if (use_cmd23) {
+		*mode |= SDHCI_TRNS_AUTO_CMD23;
+		sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
+	}
+}
+
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	struct mmc_command *cmd)
 {
@@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 
 	if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
 		mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
-		/*
-		 * If we are sending CMD23, CMD12 never gets sent
-		 * on successful completion (so no Auto-CMD12).
-		 */
-		if (sdhci_auto_cmd12(host, cmd->mrq) &&
-		    (cmd->opcode != SD_IO_RW_EXTENDED))
-			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_auto_cmd_select(host, cmd, &mode);
 	}
 
 	if (data->flags & MMC_DATA_READ)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 81aae07..a8f4ec2 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -42,6 +42,7 @@
 #define  SDHCI_TRNS_BLK_CNT_EN	0x02
 #define  SDHCI_TRNS_AUTO_CMD12	0x04
 #define  SDHCI_TRNS_AUTO_CMD23	0x08
+#define  SDHCI_TRNS_AUTO_SEL	0x0C
 #define  SDHCI_TRNS_READ	0x10
 #define  SDHCI_TRNS_MULTI	0x20
 
@@ -185,6 +186,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

Re: [PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Adrian Hunter]

On 23/07/18 13:08, Chunyan Zhang wrote:
> As SD Host Controller Specification v4.10 documents:
> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
> Selection of Auto CMD depends on setting of CMD23 Enable in the Host
> Control 2 register which indicates whether card supports CMD23. If CMD23
> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
> recommended rather than use of Auto CMD12 Enable or Auto CMD23
> Enable.
> 
> This patch add this new mode support.
> 
> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
> ---
>  drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++---------
>  drivers/mmc/host/sdhci.h |  2 ++
>  2 files changed, 52 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 5acea3d..5c60590 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
>  	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>  }
>  
> +static void sdhci_enable_cmd23(struct sdhci_host *host)
> +{
> +	u16 ctrl2;
> +
> +	/*
> +	 * This is used along with "Auto CMD Auto Select" feature,
> +	 * which is introduced from v4.10, if card supports CMD23,
> +	 * Auto CMD23 should be used instead of Auto CMD12.
> +	 */
> +	if (host->version >= SDHCI_SPEC_410 &&
> +	    (host->mmc->caps & MMC_CAP_CMD23)) {

That is the host capability.  It needs to be the card capability.

> +		ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +		ctrl2 |= SDHCI_CMD23_ENABLE;
> +		sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
> +	}
> +}
> +
>  static void sdhci_init(struct sdhci_host *host, int soft)
>  {
>  	struct mmc_host *mmc = host->mmc;
> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
>  		host->clock = 0;
>  		mmc->ops->set_ios(mmc, &mmc->ios);
>  	}
> +
> +	sdhci_enable_cmd23(host);
>  }
>  
>  static void sdhci_reinit(struct sdhci_host *host)
> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>  	       !mrq->cap_cmd_during_tfr;
>  }
>  
> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
> +					 struct mmc_command *cmd,
> +					 u16 *mode)
> +{
> +	bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
> +			 (cmd->opcode != SD_IO_RW_EXTENDED);
> +	bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
> +
> +	/*
> +	 * In case of Version 4.10 or later, use of 'Auto CMD Auto
> +	 * Select' is recommended rather than use of 'Auto CMD12
> +	 * Enable' or 'Auto CMD23 Enable'.
> +	 */
> +	if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
> +		*mode |= SDHCI_TRNS_AUTO_SEL;

As noted in patch 4, the CMD23 argument is not just the block count for eMMC.

> +		return;
> +	}
> +
> +	/*
> +	 * If we are sending CMD23, CMD12 never gets sent
> +	 * on successful completion (so no Auto-CMD12).
> +	 */
> +	if (use_cmd12) {
> +		*mode |= SDHCI_TRNS_AUTO_CMD12;
> +	} else if (use_cmd23) {
> +		*mode |= SDHCI_TRNS_AUTO_CMD23;
> +		sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
> +	}
> +}
> +
>  static void sdhci_set_transfer_mode(struct sdhci_host *host,
>  	struct mmc_command *cmd)
>  {
> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>  
>  	if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>  		mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
> -		/*
> -		 * If we are sending CMD23, CMD12 never gets sent
> -		 * on successful completion (so no Auto-CMD12).
> -		 */
> -		if (sdhci_auto_cmd12(host, cmd->mrq) &&
> -		    (cmd->opcode != SD_IO_RW_EXTENDED))
> -			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_auto_cmd_select(host, cmd, &mode);
>  	}
>  
>  	if (data->flags & MMC_DATA_READ)
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 81aae07..a8f4ec2 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -42,6 +42,7 @@
>  #define  SDHCI_TRNS_BLK_CNT_EN	0x02
>  #define  SDHCI_TRNS_AUTO_CMD12	0x04
>  #define  SDHCI_TRNS_AUTO_CMD23	0x08
> +#define  SDHCI_TRNS_AUTO_SEL	0x0C
>  #define  SDHCI_TRNS_READ	0x10
>  #define  SDHCI_TRNS_MULTI	0x20
>  
> @@ -185,6 +186,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
> 

Re: [PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Chunyan Zhang]

Hi Adrian,

On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 23/07/18 13:08, Chunyan Zhang wrote:
>> As SD Host Controller Specification v4.10 documents:
>> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
>> Selection of Auto CMD depends on setting of CMD23 Enable in the Host
>> Control 2 register which indicates whether card supports CMD23. If CMD23
>> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
>> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
>> recommended rather than use of Auto CMD12 Enable or Auto CMD23
>> Enable.
>>
>> This patch add this new mode support.
>>
>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>> ---
>>  drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++---------
>>  drivers/mmc/host/sdhci.h |  2 ++
>>  2 files changed, 52 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 5acea3d..5c60590 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
>>       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>  }
>>
>> +static void sdhci_enable_cmd23(struct sdhci_host *host)
>> +{
>> +     u16 ctrl2;
>> +
>> +     /*
>> +      * This is used along with "Auto CMD Auto Select" feature,
>> +      * which is introduced from v4.10, if card supports CMD23,
>> +      * Auto CMD23 should be used instead of Auto CMD12.
>> +      */
>> +     if (host->version >= SDHCI_SPEC_410 &&
>> +         (host->mmc->caps & MMC_CAP_CMD23)) {
>
> That is the host capability.  It needs to be the card capability.

Could you please elaborate the issue?

I thought we're setting for host here.  Do you mean we need to see the
card capability?

>
>> +             ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> +             ctrl2 |= SDHCI_CMD23_ENABLE;
>> +             sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
>> +     }
>> +}
>> +
>>  static void sdhci_init(struct sdhci_host *host, int soft)
>>  {
>>       struct mmc_host *mmc = host->mmc;
>> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
>>               host->clock = 0;
>>               mmc->ops->set_ios(mmc, &mmc->ios);
>>       }
>> +
>> +     sdhci_enable_cmd23(host);
>>  }
>>
>>  static void sdhci_reinit(struct sdhci_host *host)
>> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>>              !mrq->cap_cmd_during_tfr;
>>  }
>>
>> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
>> +                                      struct mmc_command *cmd,
>> +                                      u16 *mode)
>> +{
>> +     bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
>> +                      (cmd->opcode != SD_IO_RW_EXTENDED);
>> +     bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
>> +
>> +     /*
>> +      * In case of Version 4.10 or later, use of 'Auto CMD Auto
>> +      * Select' is recommended rather than use of 'Auto CMD12
>> +      * Enable' or 'Auto CMD23 Enable'.
>> +      */
>> +     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
>> +             *mode |= SDHCI_TRNS_AUTO_SEL;
>
> As noted in patch 4, the CMD23 argument is not just the block count for eMMC.

Probably I haven't got your point...

From what I understand, auto_sel mode doesn't need argument2. Doesn't
this work for eMMC?

The test platform I used was just eMMC only, haven't found problem.

Thanks,
Chunyan

>
>> +             return;
>> +     }
>> +
>> +     /*
>> +      * If we are sending CMD23, CMD12 never gets sent
>> +      * on successful completion (so no Auto-CMD12).
>> +      */
>> +     if (use_cmd12) {
>> +             *mode |= SDHCI_TRNS_AUTO_CMD12;
>> +     } else if (use_cmd23) {
>> +             *mode |= SDHCI_TRNS_AUTO_CMD23;
>> +             sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
>> +     }
>> +}
>> +
>>  static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>       struct mmc_command *cmd)
>>  {
>> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>
>>       if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>>               mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
>> -             /*
>> -              * If we are sending CMD23, CMD12 never gets sent
>> -              * on successful completion (so no Auto-CMD12).
>> -              */
>> -             if (sdhci_auto_cmd12(host, cmd->mrq) &&
>> -                 (cmd->opcode != SD_IO_RW_EXTENDED))
>> -                     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_auto_cmd_select(host, cmd, &mode);
>>       }
>>
>>       if (data->flags & MMC_DATA_READ)
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 81aae07..a8f4ec2 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -42,6 +42,7 @@
>>  #define  SDHCI_TRNS_BLK_CNT_EN       0x02
>>  #define  SDHCI_TRNS_AUTO_CMD12       0x04
>>  #define  SDHCI_TRNS_AUTO_CMD23       0x08
>> +#define  SDHCI_TRNS_AUTO_SEL 0x0C
>>  #define  SDHCI_TRNS_READ     0x10
>>  #define  SDHCI_TRNS_MULTI    0x20
>>
>> @@ -185,6 +186,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
>>
>

Re: [PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Adrian Hunter]

On 31/07/18 10:04, Chunyan Zhang wrote:
> Hi Adrian,
> 
> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 23/07/18 13:08, Chunyan Zhang wrote:
>>> As SD Host Controller Specification v4.10 documents:
>>> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
>>> Selection of Auto CMD depends on setting of CMD23 Enable in the Host
>>> Control 2 register which indicates whether card supports CMD23. If CMD23
>>> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
>>> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
>>> recommended rather than use of Auto CMD12 Enable or Auto CMD23
>>> Enable.
>>>
>>> This patch add this new mode support.
>>>
>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++---------
>>>  drivers/mmc/host/sdhci.h |  2 ++
>>>  2 files changed, 52 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 5acea3d..5c60590 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
>>>       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>>  }
>>>
>>> +static void sdhci_enable_cmd23(struct sdhci_host *host)
>>> +{
>>> +     u16 ctrl2;
>>> +
>>> +     /*
>>> +      * This is used along with "Auto CMD Auto Select" feature,
>>> +      * which is introduced from v4.10, if card supports CMD23,
>>> +      * Auto CMD23 should be used instead of Auto CMD12.
>>> +      */
>>> +     if (host->version >= SDHCI_SPEC_410 &&
>>> +         (host->mmc->caps & MMC_CAP_CMD23)) {
>>
>> That is the host capability.  It needs to be the card capability.
> 
> Could you please elaborate the issue?
> 
> I thought we're setting for host here.  Do you mean we need to see the
> card capability?

Yes. SDHCI_TRNS_AUTO_SEL selects auto-CMD23 or auto-CMD12 based on this
setting, so this must reflect the card's capability.

> 
>>
>>> +             ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>> +             ctrl2 |= SDHCI_CMD23_ENABLE;
>>> +             sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
>>> +     }
>>> +}
>>> +
>>>  static void sdhci_init(struct sdhci_host *host, int soft)
>>>  {
>>>       struct mmc_host *mmc = host->mmc;
>>> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
>>>               host->clock = 0;
>>>               mmc->ops->set_ios(mmc, &mmc->ios);
>>>       }
>>> +
>>> +     sdhci_enable_cmd23(host);
>>>  }
>>>
>>>  static void sdhci_reinit(struct sdhci_host *host)
>>> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>>>              !mrq->cap_cmd_during_tfr;
>>>  }
>>>
>>> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
>>> +                                      struct mmc_command *cmd,
>>> +                                      u16 *mode)
>>> +{
>>> +     bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
>>> +                      (cmd->opcode != SD_IO_RW_EXTENDED);
>>> +     bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
>>> +
>>> +     /*
>>> +      * In case of Version 4.10 or later, use of 'Auto CMD Auto
>>> +      * Select' is recommended rather than use of 'Auto CMD12
>>> +      * Enable' or 'Auto CMD23 Enable'.
>>> +      */
>>> +     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
>>> +             *mode |= SDHCI_TRNS_AUTO_SEL;
>>
>> As noted in patch 4, the CMD23 argument is not just the block count for eMMC.
> 
> Probably I haven't got your point...
> 
>>From what I understand, auto_sel mode doesn't need argument2. Doesn't
> this work for eMMC?

CMD23 always needs an argument

> 
> The test platform I used was just eMMC only, haven't found problem.

Because the bits that are missing from the CMD23 argument (reliable write,
context id, etc) will not make the command fail.

> 
> Thanks,
> Chunyan
> 
>>
>>> +             return;
>>> +     }
>>> +
>>> +     /*
>>> +      * If we are sending CMD23, CMD12 never gets sent
>>> +      * on successful completion (so no Auto-CMD12).
>>> +      */
>>> +     if (use_cmd12) {
>>> +             *mode |= SDHCI_TRNS_AUTO_CMD12;
>>> +     } else if (use_cmd23) {
>>> +             *mode |= SDHCI_TRNS_AUTO_CMD23;
>>> +             sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
>>> +     }
>>> +}
>>> +
>>>  static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>       struct mmc_command *cmd)
>>>  {
>>> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>
>>>       if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>>>               mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
>>> -             /*
>>> -              * If we are sending CMD23, CMD12 never gets sent
>>> -              * on successful completion (so no Auto-CMD12).
>>> -              */
>>> -             if (sdhci_auto_cmd12(host, cmd->mrq) &&
>>> -                 (cmd->opcode != SD_IO_RW_EXTENDED))
>>> -                     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_auto_cmd_select(host, cmd, &mode);
>>>       }
>>>
>>>       if (data->flags & MMC_DATA_READ)
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index 81aae07..a8f4ec2 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -42,6 +42,7 @@
>>>  #define  SDHCI_TRNS_BLK_CNT_EN       0x02
>>>  #define  SDHCI_TRNS_AUTO_CMD12       0x04
>>>  #define  SDHCI_TRNS_AUTO_CMD23       0x08
>>> +#define  SDHCI_TRNS_AUTO_SEL 0x0C
>>>  #define  SDHCI_TRNS_READ     0x10
>>>  #define  SDHCI_TRNS_MULTI    0x20
>>>
>>> @@ -185,6 +186,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
>>>
>>
> 

Re: [PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Chunyan Zhang]

On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 31/07/18 10:04, Chunyan Zhang wrote:
>> Hi Adrian,
>>
>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 23/07/18 13:08, Chunyan Zhang wrote:
>>>> As SD Host Controller Specification v4.10 documents:
>>>> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
>>>> Selection of Auto CMD depends on setting of CMD23 Enable in the Host
>>>> Control 2 register which indicates whether card supports CMD23. If CMD23
>>>> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
>>>> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
>>>> recommended rather than use of Auto CMD12 Enable or Auto CMD23
>>>> Enable.
>>>>
>>>> This patch add this new mode support.
>>>>
>>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>> ---
>>>>  drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++---------
>>>>  drivers/mmc/host/sdhci.h |  2 ++
>>>>  2 files changed, 52 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index 5acea3d..5c60590 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
>>>>       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>>>  }
>>>>
>>>> +static void sdhci_enable_cmd23(struct sdhci_host *host)
>>>> +{
>>>> +     u16 ctrl2;
>>>> +
>>>> +     /*
>>>> +      * This is used along with "Auto CMD Auto Select" feature,
>>>> +      * which is introduced from v4.10, if card supports CMD23,
>>>> +      * Auto CMD23 should be used instead of Auto CMD12.
>>>> +      */
>>>> +     if (host->version >= SDHCI_SPEC_410 &&
>>>> +         (host->mmc->caps & MMC_CAP_CMD23)) {
>>>
>>> That is the host capability.  It needs to be the card capability.
>>
>> Could you please elaborate the issue?
>>
>> I thought we're setting for host here.  Do you mean we need to see the
>> card capability?
>
> Yes. SDHCI_TRNS_AUTO_SEL selects auto-CMD23 or auto-CMD12 based on this
> setting, so this must reflect the card's capability.

Got it, but how to know if the card supports CMD23?

>
>>
>>>
>>>> +             ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>>> +             ctrl2 |= SDHCI_CMD23_ENABLE;
>>>> +             sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
>>>> +     }
>>>> +}
>>>> +
>>>>  static void sdhci_init(struct sdhci_host *host, int soft)
>>>>  {
>>>>       struct mmc_host *mmc = host->mmc;
>>>> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
>>>>               host->clock = 0;
>>>>               mmc->ops->set_ios(mmc, &mmc->ios);
>>>>       }
>>>> +
>>>> +     sdhci_enable_cmd23(host);
>>>>  }
>>>>
>>>>  static void sdhci_reinit(struct sdhci_host *host)
>>>> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>>>>              !mrq->cap_cmd_during_tfr;
>>>>  }
>>>>
>>>> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
>>>> +                                      struct mmc_command *cmd,
>>>> +                                      u16 *mode)
>>>> +{
>>>> +     bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
>>>> +                      (cmd->opcode != SD_IO_RW_EXTENDED);
>>>> +     bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
>>>> +
>>>> +     /*
>>>> +      * In case of Version 4.10 or later, use of 'Auto CMD Auto
>>>> +      * Select' is recommended rather than use of 'Auto CMD12
>>>> +      * Enable' or 'Auto CMD23 Enable'.
>>>> +      */
>>>> +     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
>>>> +             *mode |= SDHCI_TRNS_AUTO_SEL;
>>>
>>> As noted in patch 4, the CMD23 argument is not just the block count for eMMC.
>>
>> Probably I haven't got your point...
>>
>>>From what I understand, auto_sel mode doesn't need argument2. Doesn't
>> this work for eMMC?
>
> CMD23 always needs an argument

But setting argument for CMD23 will cover the block count value we set
before, that will lead mounting emmc to fail.

>
>>
>> The test platform I used was just eMMC only, haven't found problem.
>
> Because the bits that are missing from the CMD23 argument (reliable write,
> context id, etc) will not make the command fail.
>
>>
>> Thanks,
>> Chunyan
>>
>>>
>>>> +             return;
>>>> +     }
>>>> +
>>>> +     /*
>>>> +      * If we are sending CMD23, CMD12 never gets sent
>>>> +      * on successful completion (so no Auto-CMD12).
>>>> +      */
>>>> +     if (use_cmd12) {
>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD12;
>>>> +     } else if (use_cmd23) {
>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD23;
>>>> +             sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
>>>> +     }
>>>> +}
>>>> +
>>>>  static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>       struct mmc_command *cmd)
>>>>  {
>>>> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>
>>>>       if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>>>>               mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
>>>> -             /*
>>>> -              * If we are sending CMD23, CMD12 never gets sent
>>>> -              * on successful completion (so no Auto-CMD12).
>>>> -              */
>>>> -             if (sdhci_auto_cmd12(host, cmd->mrq) &&
>>>> -                 (cmd->opcode != SD_IO_RW_EXTENDED))
>>>> -                     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_auto_cmd_select(host, cmd, &mode);
>>>>       }
>>>>
>>>>       if (data->flags & MMC_DATA_READ)
>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>> index 81aae07..a8f4ec2 100644
>>>> --- a/drivers/mmc/host/sdhci.h
>>>> +++ b/drivers/mmc/host/sdhci.h
>>>> @@ -42,6 +42,7 @@
>>>>  #define  SDHCI_TRNS_BLK_CNT_EN       0x02
>>>>  #define  SDHCI_TRNS_AUTO_CMD12       0x04
>>>>  #define  SDHCI_TRNS_AUTO_CMD23       0x08
>>>> +#define  SDHCI_TRNS_AUTO_SEL 0x0C
>>>>  #define  SDHCI_TRNS_READ     0x10
>>>>  #define  SDHCI_TRNS_MULTI    0x20
>>>>
>>>> @@ -185,6 +186,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
>>>>
>>>
>>
>

Re: [PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Adrian Hunter]

On 31/07/18 11:36, Chunyan Zhang wrote:
> On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 31/07/18 10:04, Chunyan Zhang wrote:
>>> Hi Adrian,
>>>
>>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> On 23/07/18 13:08, Chunyan Zhang wrote:
>>>>> As SD Host Controller Specification v4.10 documents:
>>>>> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
>>>>> Selection of Auto CMD depends on setting of CMD23 Enable in the Host
>>>>> Control 2 register which indicates whether card supports CMD23. If CMD23
>>>>> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
>>>>> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
>>>>> recommended rather than use of Auto CMD12 Enable or Auto CMD23
>>>>> Enable.
>>>>>
>>>>> This patch add this new mode support.
>>>>>
>>>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>>> ---
>>>>>  drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++---------
>>>>>  drivers/mmc/host/sdhci.h |  2 ++
>>>>>  2 files changed, 52 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>>> index 5acea3d..5c60590 100644
>>>>> --- a/drivers/mmc/host/sdhci.c
>>>>> +++ b/drivers/mmc/host/sdhci.c
>>>>> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
>>>>>       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>>>>  }
>>>>>
>>>>> +static void sdhci_enable_cmd23(struct sdhci_host *host)
>>>>> +{
>>>>> +     u16 ctrl2;
>>>>> +
>>>>> +     /*
>>>>> +      * This is used along with "Auto CMD Auto Select" feature,
>>>>> +      * which is introduced from v4.10, if card supports CMD23,
>>>>> +      * Auto CMD23 should be used instead of Auto CMD12.
>>>>> +      */
>>>>> +     if (host->version >= SDHCI_SPEC_410 &&
>>>>> +         (host->mmc->caps & MMC_CAP_CMD23)) {
>>>>
>>>> That is the host capability.  It needs to be the card capability.
>>>
>>> Could you please elaborate the issue?
>>>
>>> I thought we're setting for host here.  Do you mean we need to see the
>>> card capability?
>>
>> Yes. SDHCI_TRNS_AUTO_SEL selects auto-CMD23 or auto-CMD12 based on this
>> setting, so this must reflect the card's capability.
> 
> Got it, but how to know if the card supports CMD23?

At the moment the only way of knowing is if a request is received with mrq->sbc

> 
>>
>>>
>>>>
>>>>> +             ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>>>> +             ctrl2 |= SDHCI_CMD23_ENABLE;
>>>>> +             sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
>>>>> +     }
>>>>> +}
>>>>> +
>>>>>  static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>  {
>>>>>       struct mmc_host *mmc = host->mmc;
>>>>> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>               host->clock = 0;
>>>>>               mmc->ops->set_ios(mmc, &mmc->ios);
>>>>>       }
>>>>> +
>>>>> +     sdhci_enable_cmd23(host);
>>>>>  }
>>>>>
>>>>>  static void sdhci_reinit(struct sdhci_host *host)
>>>>> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>>>>>              !mrq->cap_cmd_during_tfr;
>>>>>  }
>>>>>
>>>>> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
>>>>> +                                      struct mmc_command *cmd,
>>>>> +                                      u16 *mode)
>>>>> +{
>>>>> +     bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>> +                      (cmd->opcode != SD_IO_RW_EXTENDED);
>>>>> +     bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
>>>>> +
>>>>> +     /*
>>>>> +      * In case of Version 4.10 or later, use of 'Auto CMD Auto
>>>>> +      * Select' is recommended rather than use of 'Auto CMD12
>>>>> +      * Enable' or 'Auto CMD23 Enable'.
>>>>> +      */
>>>>> +     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
>>>>> +             *mode |= SDHCI_TRNS_AUTO_SEL;
>>>>
>>>> As noted in patch 4, the CMD23 argument is not just the block count for eMMC.
>>>
>>> Probably I haven't got your point...
>>>
>>> >From what I understand, auto_sel mode doesn't need argument2. Doesn't
>>> this work for eMMC?
>>
>> CMD23 always needs an argument
> 
> But setting argument for CMD23 will cover the block count value we set
> before, that will lead mounting emmc to fail.

Does it fail because it contains cmd->mrq->sbc->arg or does it fail because
it gets written twice?

> 
>>
>>>
>>> The test platform I used was just eMMC only, haven't found problem.
>>
>> Because the bits that are missing from the CMD23 argument (reliable write,
>> context id, etc) will not make the command fail.
>>
>>>
>>> Thanks,
>>> Chunyan
>>>
>>>>
>>>>> +             return;
>>>>> +     }
>>>>> +
>>>>> +     /*
>>>>> +      * If we are sending CMD23, CMD12 never gets sent
>>>>> +      * on successful completion (so no Auto-CMD12).
>>>>> +      */
>>>>> +     if (use_cmd12) {
>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD12;
>>>>> +     } else if (use_cmd23) {
>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD23;
>>>>> +             sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
>>>>> +     }
>>>>> +}
>>>>> +
>>>>>  static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>       struct mmc_command *cmd)
>>>>>  {
>>>>> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>
>>>>>       if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>>>>>               mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
>>>>> -             /*
>>>>> -              * If we are sending CMD23, CMD12 never gets sent
>>>>> -              * on successful completion (so no Auto-CMD12).
>>>>> -              */
>>>>> -             if (sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>> -                 (cmd->opcode != SD_IO_RW_EXTENDED))
>>>>> -                     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_auto_cmd_select(host, cmd, &mode);
>>>>>       }
>>>>>
>>>>>       if (data->flags & MMC_DATA_READ)
>>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>>> index 81aae07..a8f4ec2 100644
>>>>> --- a/drivers/mmc/host/sdhci.h
>>>>> +++ b/drivers/mmc/host/sdhci.h
>>>>> @@ -42,6 +42,7 @@
>>>>>  #define  SDHCI_TRNS_BLK_CNT_EN       0x02
>>>>>  #define  SDHCI_TRNS_AUTO_CMD12       0x04
>>>>>  #define  SDHCI_TRNS_AUTO_CMD23       0x08
>>>>> +#define  SDHCI_TRNS_AUTO_SEL 0x0C
>>>>>  #define  SDHCI_TRNS_READ     0x10
>>>>>  #define  SDHCI_TRNS_MULTI    0x20
>>>>>
>>>>> @@ -185,6 +186,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
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Chunyan Zhang]

On 31 July 2018 at 16:56, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 31/07/18 11:36, Chunyan Zhang wrote:
>> On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 31/07/18 10:04, Chunyan Zhang wrote:
>>>> Hi Adrian,
>>>>
>>>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> On 23/07/18 13:08, Chunyan Zhang wrote:
>>>>>> As SD Host Controller Specification v4.10 documents:
>>>>>> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
>>>>>> Selection of Auto CMD depends on setting of CMD23 Enable in the Host
>>>>>> Control 2 register which indicates whether card supports CMD23. If CMD23
>>>>>> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
>>>>>> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
>>>>>> recommended rather than use of Auto CMD12 Enable or Auto CMD23
>>>>>> Enable.
>>>>>>
>>>>>> This patch add this new mode support.
>>>>>>
>>>>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>>>> ---
>>>>>>  drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++---------
>>>>>>  drivers/mmc/host/sdhci.h |  2 ++
>>>>>>  2 files changed, 52 insertions(+), 11 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>>>> index 5acea3d..5c60590 100644
>>>>>> --- a/drivers/mmc/host/sdhci.c
>>>>>> +++ b/drivers/mmc/host/sdhci.c
>>>>>> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
>>>>>>       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>>>>>  }
>>>>>>
>>>>>> +static void sdhci_enable_cmd23(struct sdhci_host *host)
>>>>>> +{
>>>>>> +     u16 ctrl2;
>>>>>> +
>>>>>> +     /*
>>>>>> +      * This is used along with "Auto CMD Auto Select" feature,
>>>>>> +      * which is introduced from v4.10, if card supports CMD23,
>>>>>> +      * Auto CMD23 should be used instead of Auto CMD12.
>>>>>> +      */
>>>>>> +     if (host->version >= SDHCI_SPEC_410 &&
>>>>>> +         (host->mmc->caps & MMC_CAP_CMD23)) {
>>>>>
>>>>> That is the host capability.  It needs to be the card capability.
>>>>
>>>> Could you please elaborate the issue?
>>>>
>>>> I thought we're setting for host here.  Do you mean we need to see the
>>>> card capability?
>>>
>>> Yes. SDHCI_TRNS_AUTO_SEL selects auto-CMD23 or auto-CMD12 based on this
>>> setting, so this must reflect the card's capability.
>>
>> Got it, but how to know if the card supports CMD23?
>
> At the moment the only way of knowing is if a request is received with mrq->sbc
>
>>
>>>
>>>>
>>>>>
>>>>>> +             ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>>>>> +             ctrl2 |= SDHCI_CMD23_ENABLE;
>>>>>> +             sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
>>>>>> +     }
>>>>>> +}
>>>>>> +
>>>>>>  static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>>  {
>>>>>>       struct mmc_host *mmc = host->mmc;
>>>>>> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>>               host->clock = 0;
>>>>>>               mmc->ops->set_ios(mmc, &mmc->ios);
>>>>>>       }
>>>>>> +
>>>>>> +     sdhci_enable_cmd23(host);
>>>>>>  }
>>>>>>
>>>>>>  static void sdhci_reinit(struct sdhci_host *host)
>>>>>> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>>>>>>              !mrq->cap_cmd_during_tfr;
>>>>>>  }
>>>>>>
>>>>>> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
>>>>>> +                                      struct mmc_command *cmd,
>>>>>> +                                      u16 *mode)
>>>>>> +{
>>>>>> +     bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>>> +                      (cmd->opcode != SD_IO_RW_EXTENDED);
>>>>>> +     bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
>>>>>> +
>>>>>> +     /*
>>>>>> +      * In case of Version 4.10 or later, use of 'Auto CMD Auto
>>>>>> +      * Select' is recommended rather than use of 'Auto CMD12
>>>>>> +      * Enable' or 'Auto CMD23 Enable'.
>>>>>> +      */
>>>>>> +     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
>>>>>> +             *mode |= SDHCI_TRNS_AUTO_SEL;
>>>>>
>>>>> As noted in patch 4, the CMD23 argument is not just the block count for eMMC.
>>>>
>>>> Probably I haven't got your point...
>>>>
>>>> >From what I understand, auto_sel mode doesn't need argument2. Doesn't
>>>> this work for eMMC?
>>>
>>> CMD23 always needs an argument
>>
>> But setting argument for CMD23 will cover the block count value we set
>> before, that will lead mounting emmc to fail.
>
> Does it fail because it contains cmd->mrq->sbc->arg or does it fail because
> it gets written twice?

Seems that the argument is too big compared with block count, hardware
think it is a super block count.

More details of the error information:
https://www.irccloud.com/pastebin/uYlVEUsP/

>
>>
>>>
>>>>
>>>> The test platform I used was just eMMC only, haven't found problem.
>>>
>>> Because the bits that are missing from the CMD23 argument (reliable write,
>>> context id, etc) will not make the command fail.
>>>
>>>>
>>>> Thanks,
>>>> Chunyan
>>>>
>>>>>
>>>>>> +             return;
>>>>>> +     }
>>>>>> +
>>>>>> +     /*
>>>>>> +      * If we are sending CMD23, CMD12 never gets sent
>>>>>> +      * on successful completion (so no Auto-CMD12).
>>>>>> +      */
>>>>>> +     if (use_cmd12) {
>>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD12;
>>>>>> +     } else if (use_cmd23) {
>>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD23;
>>>>>> +             sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
>>>>>> +     }
>>>>>> +}
>>>>>> +
>>>>>>  static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>>       struct mmc_command *cmd)
>>>>>>  {
>>>>>> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>>
>>>>>>       if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>>>>>>               mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
>>>>>> -             /*
>>>>>> -              * If we are sending CMD23, CMD12 never gets sent
>>>>>> -              * on successful completion (so no Auto-CMD12).
>>>>>> -              */
>>>>>> -             if (sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>>> -                 (cmd->opcode != SD_IO_RW_EXTENDED))
>>>>>> -                     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_auto_cmd_select(host, cmd, &mode);
>>>>>>       }
>>>>>>
>>>>>>       if (data->flags & MMC_DATA_READ)
>>>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>>>> index 81aae07..a8f4ec2 100644
>>>>>> --- a/drivers/mmc/host/sdhci.h
>>>>>> +++ b/drivers/mmc/host/sdhci.h
>>>>>> @@ -42,6 +42,7 @@
>>>>>>  #define  SDHCI_TRNS_BLK_CNT_EN       0x02
>>>>>>  #define  SDHCI_TRNS_AUTO_CMD12       0x04
>>>>>>  #define  SDHCI_TRNS_AUTO_CMD23       0x08
>>>>>> +#define  SDHCI_TRNS_AUTO_SEL 0x0C
>>>>>>  #define  SDHCI_TRNS_READ     0x10
>>>>>>  #define  SDHCI_TRNS_MULTI    0x20
>>>>>>
>>>>>> @@ -185,6 +186,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
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Adrian Hunter]

On 31/07/18 12:20, Chunyan Zhang wrote:
> On 31 July 2018 at 16:56, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 31/07/18 11:36, Chunyan Zhang wrote:
>>> On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> On 31/07/18 10:04, Chunyan Zhang wrote:
>>>>> Hi Adrian,
>>>>>
>>>>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>>> On 23/07/18 13:08, Chunyan Zhang wrote:
>>>>>>> As SD Host Controller Specification v4.10 documents:
>>>>>>> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
>>>>>>> Selection of Auto CMD depends on setting of CMD23 Enable in the Host
>>>>>>> Control 2 register which indicates whether card supports CMD23. If CMD23
>>>>>>> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
>>>>>>> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
>>>>>>> recommended rather than use of Auto CMD12 Enable or Auto CMD23
>>>>>>> Enable.
>>>>>>>
>>>>>>> This patch add this new mode support.
>>>>>>>
>>>>>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>>>>> ---
>>>>>>>  drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++---------
>>>>>>>  drivers/mmc/host/sdhci.h |  2 ++
>>>>>>>  2 files changed, 52 insertions(+), 11 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>>>>> index 5acea3d..5c60590 100644
>>>>>>> --- a/drivers/mmc/host/sdhci.c
>>>>>>> +++ b/drivers/mmc/host/sdhci.c
>>>>>>> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
>>>>>>>       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>>>>>>  }
>>>>>>>
>>>>>>> +static void sdhci_enable_cmd23(struct sdhci_host *host)
>>>>>>> +{
>>>>>>> +     u16 ctrl2;
>>>>>>> +
>>>>>>> +     /*
>>>>>>> +      * This is used along with "Auto CMD Auto Select" feature,
>>>>>>> +      * which is introduced from v4.10, if card supports CMD23,
>>>>>>> +      * Auto CMD23 should be used instead of Auto CMD12.
>>>>>>> +      */
>>>>>>> +     if (host->version >= SDHCI_SPEC_410 &&
>>>>>>> +         (host->mmc->caps & MMC_CAP_CMD23)) {
>>>>>>
>>>>>> That is the host capability.  It needs to be the card capability.
>>>>>
>>>>> Could you please elaborate the issue?
>>>>>
>>>>> I thought we're setting for host here.  Do you mean we need to see the
>>>>> card capability?
>>>>
>>>> Yes. SDHCI_TRNS_AUTO_SEL selects auto-CMD23 or auto-CMD12 based on this
>>>> setting, so this must reflect the card's capability.
>>>
>>> Got it, but how to know if the card supports CMD23?
>>
>> At the moment the only way of knowing is if a request is received with mrq->sbc
>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>>> +             ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>>>>>> +             ctrl2 |= SDHCI_CMD23_ENABLE;
>>>>>>> +             sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
>>>>>>> +     }
>>>>>>> +}
>>>>>>> +
>>>>>>>  static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>>>  {
>>>>>>>       struct mmc_host *mmc = host->mmc;
>>>>>>> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>>>               host->clock = 0;
>>>>>>>               mmc->ops->set_ios(mmc, &mmc->ios);
>>>>>>>       }
>>>>>>> +
>>>>>>> +     sdhci_enable_cmd23(host);
>>>>>>>  }
>>>>>>>
>>>>>>>  static void sdhci_reinit(struct sdhci_host *host)
>>>>>>> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>>>>>>>              !mrq->cap_cmd_during_tfr;
>>>>>>>  }
>>>>>>>
>>>>>>> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
>>>>>>> +                                      struct mmc_command *cmd,
>>>>>>> +                                      u16 *mode)
>>>>>>> +{
>>>>>>> +     bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>>>> +                      (cmd->opcode != SD_IO_RW_EXTENDED);
>>>>>>> +     bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
>>>>>>> +
>>>>>>> +     /*
>>>>>>> +      * In case of Version 4.10 or later, use of 'Auto CMD Auto
>>>>>>> +      * Select' is recommended rather than use of 'Auto CMD12
>>>>>>> +      * Enable' or 'Auto CMD23 Enable'.
>>>>>>> +      */
>>>>>>> +     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
>>>>>>> +             *mode |= SDHCI_TRNS_AUTO_SEL;
>>>>>>
>>>>>> As noted in patch 4, the CMD23 argument is not just the block count for eMMC.
>>>>>
>>>>> Probably I haven't got your point...
>>>>>
>>>>> >From what I understand, auto_sel mode doesn't need argument2. Doesn't
>>>>> this work for eMMC?
>>>>
>>>> CMD23 always needs an argument
>>>
>>> But setting argument for CMD23 will cover the block count value we set
>>> before, that will lead mounting emmc to fail.
>>
>> Does it fail because it contains cmd->mrq->sbc->arg or does it fail because
>> it gets written twice?
> 
> Seems that the argument is too big compared with block count, hardware
> think it is a super block count.
> 
> More details of the error information:
> https://www.irccloud.com/pastebin/uYlVEUsP/

Does it work with auto-CMD23 instead of auto-CMD-auto-select?
Does it work if the 16-bit block count register is used?

Obviously, if we can't pass the CMD23 argument correctly then we can't use
auto-CMD23.

> 
>>
>>>
>>>>
>>>>>
>>>>> The test platform I used was just eMMC only, haven't found problem.
>>>>
>>>> Because the bits that are missing from the CMD23 argument (reliable write,
>>>> context id, etc) will not make the command fail.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Chunyan
>>>>>
>>>>>>
>>>>>>> +             return;
>>>>>>> +     }
>>>>>>> +
>>>>>>> +     /*
>>>>>>> +      * If we are sending CMD23, CMD12 never gets sent
>>>>>>> +      * on successful completion (so no Auto-CMD12).
>>>>>>> +      */
>>>>>>> +     if (use_cmd12) {
>>>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD12;
>>>>>>> +     } else if (use_cmd23) {
>>>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD23;
>>>>>>> +             sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
>>>>>>> +     }
>>>>>>> +}
>>>>>>> +
>>>>>>>  static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>>>       struct mmc_command *cmd)
>>>>>>>  {
>>>>>>> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>>>
>>>>>>>       if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>>>>>>>               mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
>>>>>>> -             /*
>>>>>>> -              * If we are sending CMD23, CMD12 never gets sent
>>>>>>> -              * on successful completion (so no Auto-CMD12).
>>>>>>> -              */
>>>>>>> -             if (sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>>>> -                 (cmd->opcode != SD_IO_RW_EXTENDED))
>>>>>>> -                     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_auto_cmd_select(host, cmd, &mode);
>>>>>>>       }
>>>>>>>
>>>>>>>       if (data->flags & MMC_DATA_READ)
>>>>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>>>>> index 81aae07..a8f4ec2 100644
>>>>>>> --- a/drivers/mmc/host/sdhci.h
>>>>>>> +++ b/drivers/mmc/host/sdhci.h
>>>>>>> @@ -42,6 +42,7 @@
>>>>>>>  #define  SDHCI_TRNS_BLK_CNT_EN       0x02
>>>>>>>  #define  SDHCI_TRNS_AUTO_CMD12       0x04
>>>>>>>  #define  SDHCI_TRNS_AUTO_CMD23       0x08
>>>>>>> +#define  SDHCI_TRNS_AUTO_SEL 0x0C
>>>>>>>  #define  SDHCI_TRNS_READ     0x10
>>>>>>>  #define  SDHCI_TRNS_MULTI    0x20
>>>>>>>
>>>>>>> @@ -185,6 +186,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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Chunyan Zhang]

On 31 July 2018 at 17:36, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 31/07/18 12:20, Chunyan Zhang wrote:
>> On 31 July 2018 at 16:56, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 31/07/18 11:36, Chunyan Zhang wrote:
>>>> On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> On 31/07/18 10:04, Chunyan Zhang wrote:
>>>>>> Hi Adrian,
>>>>>>
>>>>>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>>>> On 23/07/18 13:08, Chunyan Zhang wrote:
>>>>>>>> As SD Host Controller Specification v4.10 documents:
>>>>>>>> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
>>>>>>>> Selection of Auto CMD depends on setting of CMD23 Enable in the Host
>>>>>>>> Control 2 register which indicates whether card supports CMD23. If CMD23
>>>>>>>> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
>>>>>>>> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
>>>>>>>> recommended rather than use of Auto CMD12 Enable or Auto CMD23
>>>>>>>> Enable.
>>>>>>>>
>>>>>>>> This patch add this new mode support.
>>>>>>>>
>>>>>>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>>>>>> ---
>>>>>>>>  drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++---------
>>>>>>>>  drivers/mmc/host/sdhci.h |  2 ++
>>>>>>>>  2 files changed, 52 insertions(+), 11 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>>>>>> index 5acea3d..5c60590 100644
>>>>>>>> --- a/drivers/mmc/host/sdhci.c
>>>>>>>> +++ b/drivers/mmc/host/sdhci.c
>>>>>>>> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
>>>>>>>>       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>>>>>>>  }
>>>>>>>>
>>>>>>>> +static void sdhci_enable_cmd23(struct sdhci_host *host)
>>>>>>>> +{
>>>>>>>> +     u16 ctrl2;
>>>>>>>> +
>>>>>>>> +     /*
>>>>>>>> +      * This is used along with "Auto CMD Auto Select" feature,
>>>>>>>> +      * which is introduced from v4.10, if card supports CMD23,
>>>>>>>> +      * Auto CMD23 should be used instead of Auto CMD12.
>>>>>>>> +      */
>>>>>>>> +     if (host->version >= SDHCI_SPEC_410 &&
>>>>>>>> +         (host->mmc->caps & MMC_CAP_CMD23)) {
>>>>>>>
>>>>>>> That is the host capability.  It needs to be the card capability.
>>>>>>
>>>>>> Could you please elaborate the issue?
>>>>>>
>>>>>> I thought we're setting for host here.  Do you mean we need to see the
>>>>>> card capability?
>>>>>
>>>>> Yes. SDHCI_TRNS_AUTO_SEL selects auto-CMD23 or auto-CMD12 based on this
>>>>> setting, so this must reflect the card's capability.
>>>>
>>>> Got it, but how to know if the card supports CMD23?
>>>
>>> At the moment the only way of knowing is if a request is received with mrq->sbc
>>>
>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +             ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>>>>>>> +             ctrl2 |= SDHCI_CMD23_ENABLE;
>>>>>>>> +             sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
>>>>>>>> +     }
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>  static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>>>>  {
>>>>>>>>       struct mmc_host *mmc = host->mmc;
>>>>>>>> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>>>>               host->clock = 0;
>>>>>>>>               mmc->ops->set_ios(mmc, &mmc->ios);
>>>>>>>>       }
>>>>>>>> +
>>>>>>>> +     sdhci_enable_cmd23(host);
>>>>>>>>  }
>>>>>>>>
>>>>>>>>  static void sdhci_reinit(struct sdhci_host *host)
>>>>>>>> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>>>>>>>>              !mrq->cap_cmd_during_tfr;
>>>>>>>>  }
>>>>>>>>
>>>>>>>> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
>>>>>>>> +                                      struct mmc_command *cmd,
>>>>>>>> +                                      u16 *mode)
>>>>>>>> +{
>>>>>>>> +     bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>>>>> +                      (cmd->opcode != SD_IO_RW_EXTENDED);
>>>>>>>> +     bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
>>>>>>>> +
>>>>>>>> +     /*
>>>>>>>> +      * In case of Version 4.10 or later, use of 'Auto CMD Auto
>>>>>>>> +      * Select' is recommended rather than use of 'Auto CMD12
>>>>>>>> +      * Enable' or 'Auto CMD23 Enable'.
>>>>>>>> +      */
>>>>>>>> +     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
>>>>>>>> +             *mode |= SDHCI_TRNS_AUTO_SEL;
>>>>>>>
>>>>>>> As noted in patch 4, the CMD23 argument is not just the block count for eMMC.
>>>>>>
>>>>>> Probably I haven't got your point...
>>>>>>
>>>>>> >From what I understand, auto_sel mode doesn't need argument2. Doesn't
>>>>>> this work for eMMC?
>>>>>
>>>>> CMD23 always needs an argument
>>>>
>>>> But setting argument for CMD23 will cover the block count value we set
>>>> before, that will lead mounting emmc to fail.
>>>
>>> Does it fail because it contains cmd->mrq->sbc->arg or does it fail because
>>> it gets written twice?
>>
>> Seems that the argument is too big compared with block count, hardware
>> think it is a super block count.
>>
>> More details of the error information:
>> https://www.irccloud.com/pastebin/uYlVEUsP/
>
> Does it work with auto-CMD23 instead of auto-CMD-auto-select?

No, so long as SDHCI_32BIT_BLK_CNT was set with cmd->mrq->sbc->arg
which is a big value, it would report error "I/O error while writing
superblock" when mounting emmc.

> Does it work if the 16-bit block count register is used?

16-bit block count register is Read Only on my board :-(

>
> Obviously, if we can't pass the CMD23 argument correctly then we can't use
> auto-CMD23.

On my board, if argument2 was limited to the maximum of 65535, then
everything works, otherwise I would see an error "EXT4-fs
(mmcblk0p28): I/O error while writing superblock".  Haven't found the
root cause. I will continue to look at the problem, any comments would
be appreciated.

Thanks,
Chunyan

>
>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>> The test platform I used was just eMMC only, haven't found problem.
>>>>>
>>>>> Because the bits that are missing from the CMD23 argument (reliable write,
>>>>> context id, etc) will not make the command fail.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Chunyan
>>>>>>
>>>>>>>
>>>>>>>> +             return;
>>>>>>>> +     }
>>>>>>>> +
>>>>>>>> +     /*
>>>>>>>> +      * If we are sending CMD23, CMD12 never gets sent
>>>>>>>> +      * on successful completion (so no Auto-CMD12).
>>>>>>>> +      */
>>>>>>>> +     if (use_cmd12) {
>>>>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD12;
>>>>>>>> +     } else if (use_cmd23) {
>>>>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD23;
>>>>>>>> +             sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
>>>>>>>> +     }
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>  static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>>>>       struct mmc_command *cmd)
>>>>>>>>  {
>>>>>>>> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>>>>
>>>>>>>>       if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>>>>>>>>               mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
>>>>>>>> -             /*
>>>>>>>> -              * If we are sending CMD23, CMD12 never gets sent
>>>>>>>> -              * on successful completion (so no Auto-CMD12).
>>>>>>>> -              */
>>>>>>>> -             if (sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>>>>> -                 (cmd->opcode != SD_IO_RW_EXTENDED))
>>>>>>>> -                     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_auto_cmd_select(host, cmd, &mode);
>>>>>>>>       }
>>>>>>>>
>>>>>>>>       if (data->flags & MMC_DATA_READ)
>>>>>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>>>>>> index 81aae07..a8f4ec2 100644
>>>>>>>> --- a/drivers/mmc/host/sdhci.h
>>>>>>>> +++ b/drivers/mmc/host/sdhci.h
>>>>>>>> @@ -42,6 +42,7 @@
>>>>>>>>  #define  SDHCI_TRNS_BLK_CNT_EN       0x02
>>>>>>>>  #define  SDHCI_TRNS_AUTO_CMD12       0x04
>>>>>>>>  #define  SDHCI_TRNS_AUTO_CMD23       0x08
>>>>>>>> +#define  SDHCI_TRNS_AUTO_SEL 0x0C
>>>>>>>>  #define  SDHCI_TRNS_READ     0x10
>>>>>>>>  #define  SDHCI_TRNS_MULTI    0x20
>>>>>>>>
>>>>>>>> @@ -185,6 +186,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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [PATCH V4 5/7] mmc: sdhci: add Auto CMD Auto Select support

[Chunyan Zhang]

On 31 July 2018 at 16:56, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 31/07/18 11:36, Chunyan Zhang wrote:
>> On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 31/07/18 10:04, Chunyan Zhang wrote:
>>>> Hi Adrian,
>>>>
>>>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> On 23/07/18 13:08, Chunyan Zhang wrote:
>>>>>> As SD Host Controller Specification v4.10 documents:
>>>>>> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode.
>>>>>> Selection of Auto CMD depends on setting of CMD23 Enable in the Host
>>>>>> Control 2 register which indicates whether card supports CMD23. If CMD23
>>>>>> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is
>>>>>> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is
>>>>>> recommended rather than use of Auto CMD12 Enable or Auto CMD23
>>>>>> Enable.
>>>>>>
>>>>>> This patch add this new mode support.
>>>>>>
>>>>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>>>> ---
>>>>>>  drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++---------
>>>>>>  drivers/mmc/host/sdhci.h |  2 ++
>>>>>>  2 files changed, 52 insertions(+), 11 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>>>> index 5acea3d..5c60590 100644
>>>>>> --- a/drivers/mmc/host/sdhci.c
>>>>>> +++ b/drivers/mmc/host/sdhci.c
>>>>>> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host)
>>>>>>       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>>>>>  }
>>>>>>
>>>>>> +static void sdhci_enable_cmd23(struct sdhci_host *host)
>>>>>> +{
>>>>>> +     u16 ctrl2;
>>>>>> +
>>>>>> +     /*
>>>>>> +      * This is used along with "Auto CMD Auto Select" feature,
>>>>>> +      * which is introduced from v4.10, if card supports CMD23,
>>>>>> +      * Auto CMD23 should be used instead of Auto CMD12.
>>>>>> +      */
>>>>>> +     if (host->version >= SDHCI_SPEC_410 &&
>>>>>> +         (host->mmc->caps & MMC_CAP_CMD23)) {
>>>>>
>>>>> That is the host capability.  It needs to be the card capability.
>>>>
>>>> Could you please elaborate the issue?
>>>>
>>>> I thought we're setting for host here.  Do you mean we need to see the
>>>> card capability?
>>>
>>> Yes. SDHCI_TRNS_AUTO_SEL selects auto-CMD23 or auto-CMD12 based on this
>>> setting, so this must reflect the card's capability.
>>
>> Got it, but how to know if the card supports CMD23?
>
> At the moment the only way of knowing is if a request is received with mrq->sbc

Ok. I will move this setting to sdhci_auto_cmd_select() for use_cmd23 is true.

>
>>
>>>
>>>>
>>>>>
>>>>>> +             ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>>>>> +             ctrl2 |= SDHCI_CMD23_ENABLE;
>>>>>> +             sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2);
>>>>>> +     }
>>>>>> +}
>>>>>> +
>>>>>>  static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>>  {
>>>>>>       struct mmc_host *mmc = host->mmc;
>>>>>> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft)
>>>>>>               host->clock = 0;
>>>>>>               mmc->ops->set_ios(mmc, &mmc->ios);
>>>>>>       }
>>>>>> +
>>>>>> +     sdhci_enable_cmd23(host);
>>>>>>  }
>>>>>>
>>>>>>  static void sdhci_reinit(struct sdhci_host *host)
>>>>>> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>>>>>>              !mrq->cap_cmd_during_tfr;
>>>>>>  }
>>>>>>
>>>>>> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
>>>>>> +                                      struct mmc_command *cmd,
>>>>>> +                                      u16 *mode)
>>>>>> +{
>>>>>> +     bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>>> +                      (cmd->opcode != SD_IO_RW_EXTENDED);
>>>>>> +     bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
>>>>>> +
>>>>>> +     /*
>>>>>> +      * In case of Version 4.10 or later, use of 'Auto CMD Auto
>>>>>> +      * Select' is recommended rather than use of 'Auto CMD12
>>>>>> +      * Enable' or 'Auto CMD23 Enable'.
>>>>>> +      */
>>>>>> +     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
>>>>>> +             *mode |= SDHCI_TRNS_AUTO_SEL;
>>>>>
>>>>> As noted in patch 4, the CMD23 argument is not just the block count for eMMC.
>>>>
>>>> Probably I haven't got your point...
>>>>
>>>> >From what I understand, auto_sel mode doesn't need argument2. Doesn't
>>>> this work for eMMC?
>>>
>>> CMD23 always needs an argument
>>
>> But setting argument for CMD23 will cover the block count value we set
>> before, that will lead mounting emmc to fail.
>
> Does it fail because it contains cmd->mrq->sbc->arg or does it fail because
> it gets written twice?
>
>>
>>>
>>>>
>>>> The test platform I used was just eMMC only, haven't found problem.
>>>
>>> Because the bits that are missing from the CMD23 argument (reliable write,
>>> context id, etc) will not make the command fail.
>>>
>>>>
>>>> Thanks,
>>>> Chunyan
>>>>
>>>>>
>>>>>> +             return;
>>>>>> +     }
>>>>>> +
>>>>>> +     /*
>>>>>> +      * If we are sending CMD23, CMD12 never gets sent
>>>>>> +      * on successful completion (so no Auto-CMD12).
>>>>>> +      */
>>>>>> +     if (use_cmd12) {
>>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD12;
>>>>>> +     } else if (use_cmd23) {
>>>>>> +             *mode |= SDHCI_TRNS_AUTO_CMD23;
>>>>>> +             sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
>>>>>> +     }
>>>>>> +}
>>>>>> +
>>>>>>  static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>>       struct mmc_command *cmd)
>>>>>>  {
>>>>>> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
>>>>>>
>>>>>>       if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>>>>>>               mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
>>>>>> -             /*
>>>>>> -              * If we are sending CMD23, CMD12 never gets sent
>>>>>> -              * on successful completion (so no Auto-CMD12).
>>>>>> -              */
>>>>>> -             if (sdhci_auto_cmd12(host, cmd->mrq) &&
>>>>>> -                 (cmd->opcode != SD_IO_RW_EXTENDED))
>>>>>> -                     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_auto_cmd_select(host, cmd, &mode);
>>>>>>       }
>>>>>>
>>>>>>       if (data->flags & MMC_DATA_READ)
>>>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>>>> index 81aae07..a8f4ec2 100644
>>>>>> --- a/drivers/mmc/host/sdhci.h
>>>>>> +++ b/drivers/mmc/host/sdhci.h
>>>>>> @@ -42,6 +42,7 @@
>>>>>>  #define  SDHCI_TRNS_BLK_CNT_EN       0x02
>>>>>>  #define  SDHCI_TRNS_AUTO_CMD12       0x04
>>>>>>  #define  SDHCI_TRNS_AUTO_CMD23       0x08
>>>>>> +#define  SDHCI_TRNS_AUTO_SEL 0x0C
>>>>>>  #define  SDHCI_TRNS_READ     0x10
>>>>>>  #define  SDHCI_TRNS_MULTI    0x20
>>>>>>
>>>>>> @@ -185,6 +186,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
>>>>>>
>>>>>
>>>>
>>>
>>
>

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

[Chunyan Zhang]

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

This patch adds the initial support of Secure Digital Host Controller
Interface compliant controller found in some latest Spreadtrum chipsets.
This patch has been tested on the version of SPRD-R11 controller.

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

With this driver, R11 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.c | 455 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 469 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-sprd.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 0581c19..c5424dc 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -581,6 +581,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
+	tristate "Spreadtrum SDIO host Controller"
+	depends on ARCH_SPRD
+	depends on MMC_SDHCI_PLTFM
+	select MMC_SDHCI_IO_ACCESSORS
+	help
+	  This selects the SDIO Host Controller in Spreadtrum
+	  SoCs, this driver supports R11(IP version: R11P0).
+
+	  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 85dc132..b0b6802 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_ST)		+= sdhci-st.o
 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_SDHCI_SPRD)		+= sdhci-sprd.o
 obj-$(CONFIG_MMC_CQHCI)			+= cqhci.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
new file mode 100644
index 0000000..b467ed5
--- /dev/null
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -0,0 +1,455 @@
+// 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		BIT(5)
+#define  SDHCI_SPRD_BIT_CMD_DLY_INV		BIT(13)
+#define  SDHCI_SPRD_BIT_POSRD_DLY_INV		BIT(21)
+#define  SDHCI_SPRD_BIT_NEGRD_DLY_INV		BIT(29)
+
+#define SDHCI_SPRD_REG_32_BUSY_POSI		0x250
+#define  SDHCI_SPRD_BIT_OUTR_CLK_AUTO_EN	BIT(25)
+#define  SDHCI_SPRD_BIT_INNR_CLK_AUTO_EN	BIT(24)
+
+#define SDHCI_SPRD_REG_DEBOUNCE		0x28C
+#define  SDHCI_SPRD_BIT_DLL_BAK		BIT(0)
+#define  SDHCI_SPRD_BIT_DLL_VAL		BIT(1)
+
+#define  SDHCI_SPRD_INT_SIGNAL_MASK	0x1B7F410B
+
+/* SDHCI_HOST_CONTROL2 */
+#define  SDHCI_SPRD_CTRL_HS200		0x0005
+#define  SDHCI_SPRD_CTRL_HS400		0x0006
+
+/*
+ * According to the standard specification, BIT(3) of SDHCI_SOFTWARE_RESET is
+ * reserved, and only used on Spreadtrum's design, the hardware cannot work
+ * if this bit is cleared.
+ * 1 : normal work
+ * 0 : hardware reset
+ */
+#define  SDHCI_HW_RESET_CARD		BIT(3)
+
+#define SDHCI_SPRD_MAX_CUR		0xFFFFFF
+#define SDHCI_SPRD_CLK_MAX_DIV		1023
+
+#define SDHCI_SPRD_CLK_DEF_RATE		26000000
+
+struct sdhci_sprd_host {
+	u32 version;
+	struct clk *clk_sdio;
+	struct clk *clk_enable;
+	u32 base_rate;
+};
+
+#define TO_SPRD_HOST(host) sdhci_pltfm_priv(sdhci_priv(host))
+
+static void sdhci_sprd_init_config(struct sdhci_host *host)
+{
+	u16 val;
+
+	/* 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;
+
+	writel_relaxed(val, host->ioaddr + reg);
+}
+
+static inline void sdhci_sprd_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	/*
+	 * Since BIT(3) of SDHCI_SOFTWARE_RESET is reserved according to the
+	 * standard specification, sdhci_reset() write this register directly
+	 * without checking other reserved bits, that will clear BIT(3) which
+	 * is defined as hardware reset on Spreadtrum's platform and clearing
+	 * it by mistake will lead the card not work. So here we need to work
+	 * around it.
+	 */
+	if (unlikely(reg == SDHCI_SOFTWARE_RESET)) {
+		if (readb_relaxed(host->ioaddr + reg) & SDHCI_HW_RESET_CARD)
+			val |= SDHCI_HW_RESET_CARD;
+	}
+
+	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_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_writeb() API here since it is redirected to
+	 * sdhci_sprd_writeb() in which we have a workaround for
+	 * SDHCI_SOFTWARE_RESET which would make bit SDHCI_HW_RESET_CARD can
+	 * not be cleared.
+	 */
+	val = readb_relaxed(host->ioaddr + SDHCI_SOFTWARE_RESET);
+	val &= ~SDHCI_HW_RESET_CARD;
+	writeb_relaxed(val, host->ioaddr + SDHCI_SOFTWARE_RESET);
+	/* wait for 10 us */
+	usleep_range(10, 20);
+
+	val |= SDHCI_HW_RESET_CARD;
+	writeb_relaxed(val, host->ioaddr + SDHCI_SOFTWARE_RESET);
+	usleep_range(300, 500);
+}
+
+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_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;
+	struct clk *clk;
+	int ret = 0;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_sprd_pdata, sizeof(*sprd_host));
+	if (IS_ERR(host))
+		return PTR_ERR(host);
+
+	host->dma_mask = DMA_BIT_MASK(64);
+	pdev->dev.dma_mask = &host->dma_mask;
+
+	host->mmc->caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
+		MMC_CAP_ERASE | MMC_CAP_CMD23;
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		goto pltfm_free;
+
+	sprd_host = TO_SPRD_HOST(host);
+
+	clk = devm_clk_get(&pdev->dev, "sdio");
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		goto pltfm_free;
+	}
+	sprd_host->clk_sdio = clk;
+	sprd_host->base_rate = clk_get_rate(sprd_host->clk_sdio);
+	if (!sprd_host->base_rate)
+		sprd_host->base_rate = SDHCI_SPRD_CLK_DEF_RATE;
+
+	clk = devm_clk_get(&pdev->dev, "enable");
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		goto pltfm_free;
+	}
+	sprd_host->clk_enable = clk;
+
+	ret = clk_prepare_enable(sprd_host->clk_sdio);
+	if (ret)
+		goto pltfm_free;
+
+	clk_prepare_enable(sprd_host->clk_enable);
+	if (ret)
+		goto clk_disable;
+
+	sdhci_sprd_init_config(host);
+	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);
+
+	sdhci_enable_v4_mode(host);
+	ret = sdhci_add_host(host);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add mmc host: %d\n", ret);
+		goto pm_runtime_disable;
+	}
+
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
+
+	return 0;
+
+pm_runtime_disable:
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_set_suspended(&pdev->dev);
+
+	clk_disable_unprepare(sprd_host->clk_enable);
+
+clk_disable:
+	clk_disable_unprepare(sprd_host->clk_sdio);
+
+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,sdhci-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);
+	int ret;
+
+	ret = clk_prepare_enable(sprd_host->clk_enable);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(sprd_host->clk_sdio);
+	if (ret) {
+		clk_disable_unprepare(sprd_host->clk_enable);
+		return ret;
+	}
+
+	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

[PATCH V4 7/7] dt-bindings: sdhci-sprd: Add bindings for the sdhci-sprd controller

[Chunyan Zhang]

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

This patch adds the device-tree binding documentation for Spreadtrum
SDHCI driver.

Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
---
 .../devicetree/bindings/mmc/sdhci-sprd.txt         | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-sprd.txt

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
new file mode 100644
index 0000000..45c9978
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
@@ -0,0 +1,41 @@
+* Spreadtrum SDHCI controller (sdhci-sprd)
+
+The Secure Digital (SD) Host controller on Spreadtrum SoCs provides an interface
+for MMC, SD and SDIO types of cards.
+
+This file documents differences between the core properties in mmc.txt
+and the properties used by the sdhci-sprd driver.
+
+Required properties:
+- compatible: Should contain "sprd,sdhci-r11".
+- reg: physical base address of the controller and length.
+- interrupts: Interrupts used by the SDHCI controller.
+- clocks: Should contain phandle for the clock feeding the SDHCI controller
+- clock-names: Should contain the following:
+	"sdio" - SDIO source clock (required)
+	"enable" - gate clock which used for enabling/disabling the device (required)
+
+Optional properties:
+- assigned-clocks: the same with "sdio" clock
+- assigned-clock-parents: the default parent of "sdio" clock
+
+Examples:
+
+sdio0: sdio@20600000 {
+	compatible  = "sprd,sdhci-r11";
+	reg = <0 0x20600000 0 0x1000>;
+	interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+
+	clock-names = "sdio", "enable";
+	clocks = <&ap_clk CLK_EMMC_2X>,
+		 <&apahb_gate CLK_EMMC_EB>;
+	assigned-clocks = <&ap_clk CLK_EMMC_2X>;
+	assigned-clock-parents = <&rpll CLK_RPLL_390M>;
+
+	bus-width = <8>;
+	non-removable;
+	no-sdio;
+	no-sd;
+	cap-mmc-hw-reset;
+	status = "okay";
+};
-- 
2.7.4