[PATCH 0/2] drivers: mmc: Add wait_dat0 support for more devices

[PATCH 0/2] drivers: mmc: Add wait_dat0 support for more devices

[stcarlso]

From: Stephen Carlson <stcarlso@linux.microsoft.com>

This patch adds eMMC driver support for polling the DAT0 data connection
on the Freescale LX devices and SDHCI compatible devices.

Polling until the card reports it is no longer busy increases eMMC
driver performance over the default behavior, which is to wait for the
worst case timeout specified by the card upon each mmc_switch operation.

This change has been tested on the Broadcom NS3 series (SDHCI) and
LX2160A series (Freescale/NXP eSDHC). U-Boot boot time savings depend on
the eMMC memory used, but can range from 100 to 500 ms or more.

Stephen Carlson (2):
  drivers: mmc: Add wait_dat0 support for Freescale eSDHC driver
  drivers: mmc: Add wait_dat0 support for sdhci driver

 drivers/mmc/fsl_esdhc.c | 16 ++++++++++++++++
 drivers/mmc/sdhci.c     | 20 ++++++++++++++++++++
 include/sdhci.h         |  2 ++
 3 files changed, 38 insertions(+)

-- 
2.17.1

[PATCH 1/2] drivers: mmc: Add wait_dat0 support for Freescale eSDHC driver

[stcarlso]

From: Stephen Carlson <stcarlso@linux.microsoft.com>

Adds an implementation of the wait_dat0 MMC operation for the Freescale
eSHDC driver, allowing the driver to continue when the card is ready
rather than waiting for the worst case time on each MMC switch operation.

Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
---
 drivers/mmc/fsl_esdhc.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index 1d98fa65c4..f74289a677 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -27,6 +27,7 @@
 #include <dm/device_compat.h>
 #include <linux/bitops.h>
 #include <linux/delay.h>
+#include <linux/iopoll.h>
 #include <linux/dma-mapping.h>
 #include <sdhci.h>
 
@@ -1145,6 +1146,20 @@ int fsl_esdhc_hs400_prepare_ddr(struct udevice *dev)
 	return 0;
 }
 
+static int fsl_esdhc_wait_dat0(struct udevice *dev, int state,
+			       int timeout_us)
+{
+	int ret;
+	u32 tmp;
+	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
+	struct fsl_esdhc *regs = priv->esdhc_regs;
+
+	ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp,
+				 !!(tmp & PRSSTAT_DAT0) == !!state,
+				 timeout_us);
+	return ret;
+}
+
 static const struct dm_mmc_ops fsl_esdhc_ops = {
 	.get_cd		= fsl_esdhc_get_cd,
 	.send_cmd	= fsl_esdhc_send_cmd,
@@ -1154,6 +1169,7 @@ static const struct dm_mmc_ops fsl_esdhc_ops = {
 #endif
 	.reinit = fsl_esdhc_reinit,
 	.hs400_prepare_ddr = fsl_esdhc_hs400_prepare_ddr,
+	.wait_dat0 = fsl_esdhc_wait_dat0,
 };
 
 static const struct udevice_id fsl_esdhc_ids[] = {
-- 
2.17.1

Re: [PATCH 1/2] drivers: mmc: Add wait_dat0 support for Freescale eSDHC driver

[Jaehoon Chung]

On 8/18/21 4:46 AM, stcarlso@linux.microsoft.com wrote:
> From: Stephen Carlson <stcarlso@linux.microsoft.com>
> 
> Adds an implementation of the wait_dat0 MMC operation for the Freescale
> eSHDC driver, allowing the driver to continue when the card is ready
> rather than waiting for the worst case time on each MMC switch operation.
> 
> Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
> ---
>  drivers/mmc/fsl_esdhc.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 1d98fa65c4..f74289a677 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -27,6 +27,7 @@
>  #include <dm/device_compat.h>
>  #include <linux/bitops.h>
>  #include <linux/delay.h>
> +#include <linux/iopoll.h>
>  #include <linux/dma-mapping.h>
>  #include <sdhci.h>
>  
> @@ -1145,6 +1146,20 @@ int fsl_esdhc_hs400_prepare_ddr(struct udevice *dev)
>  	return 0;
>  }
>  
> +static int fsl_esdhc_wait_dat0(struct udevice *dev, int state,
> +			       int timeout_us)
> +{
> +	int ret;

nitpick, doesn't need to use the ret variable.

> +	u32 tmp;
> +	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
> +	struct fsl_esdhc *regs = priv->esdhc_regs;
> +
> +	ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp,
> +				 !!(tmp & PRSSTAT_DAT0) == !!state,
> +				 timeout_us);
> +	return ret;

return readx_poll_timeout(); ?

Other things look good to me.

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> +}
> +
>  static const struct dm_mmc_ops fsl_esdhc_ops = {
>  	.get_cd		= fsl_esdhc_get_cd,
>  	.send_cmd	= fsl_esdhc_send_cmd,
> @@ -1154,6 +1169,7 @@ static const struct dm_mmc_ops fsl_esdhc_ops = {
>  #endif
>  	.reinit = fsl_esdhc_reinit,
>  	.hs400_prepare_ddr = fsl_esdhc_hs400_prepare_ddr,
> +	.wait_dat0 = fsl_esdhc_wait_dat0,
>  };
>  
>  static const struct udevice_id fsl_esdhc_ids[] = {
> 

[PATCH 2/2] drivers: mmc: Add wait_dat0 support for sdhci driver

[stcarlso]

From: Stephen Carlson <stcarlso@linux.microsoft.com>

Adds an implementation of the wait_dat0 MMC operation for the DM SDHCI
driver, allowing the driver to continue when the card is ready rather
than waiting for the worst case time on each MMC switch operation.

Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
---
 drivers/mmc/sdhci.c | 20 ++++++++++++++++++++
 include/sdhci.h     |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index eea4701d8a..bb55e00ef5 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -775,6 +775,25 @@ static int sdhci_get_cd(struct udevice *dev)
 		return value;
 }
 
+static int sdhci_wait_dat0(struct udevice *dev, int state,
+			   int timeout_us)
+{
+	int tmp;
+	struct mmc *mmc = mmc_get_mmc_dev(dev);
+	struct sdhci_host *host = mmc->priv;
+	unsigned long timeout = timer_get_us() + timeout_us;
+
+	// readx_poll_timeout is unsuitable because sdhci_readl accepts
+	// two arguments
+	do {
+		tmp = sdhci_readl(host, SDHCI_PRESENT_STATE);
+		if (!!(tmp & SDHCI_DATA_0_LVL_MASK) == !!state)
+			return 0;
+	} while (!timeout_us || !time_after(timer_get_us(), timeout));
+
+	return -ETIMEDOUT;
+}
+
 const struct dm_mmc_ops sdhci_ops = {
 	.send_cmd	= sdhci_send_command,
 	.set_ios	= sdhci_set_ios,
@@ -783,6 +802,7 @@ const struct dm_mmc_ops sdhci_ops = {
 #ifdef MMC_SUPPORTS_TUNING
 	.execute_tuning	= sdhci_execute_tuning,
 #endif
+	.wait_dat0	= sdhci_wait_dat0,
 };
 #else
 static const struct mmc_ops sdhci_ops = {
diff --git a/include/sdhci.h b/include/sdhci.h
index 0ae9471ad7..dd4eb41442 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -65,6 +65,8 @@
 #define  SDHCI_CARD_STATE_STABLE	BIT(17)
 #define  SDHCI_CARD_DETECT_PIN_LEVEL	BIT(18)
 #define  SDHCI_WRITE_PROTECT	BIT(19)
+#define  SDHCI_DATA_LVL_MASK	0x00F00000
+#define   SDHCI_DATA_0_LVL_MASK BIT(20)
 
 #define SDHCI_HOST_CONTROL	0x28
 #define  SDHCI_CTRL_LED		BIT(0)
-- 
2.17.1

Re: [PATCH 2/2] drivers: mmc: Add wait_dat0 support for sdhci driver

[Jaehoon Chung]

On 8/18/21 4:46 AM, stcarlso@linux.microsoft.com wrote:
> From: Stephen Carlson <stcarlso@linux.microsoft.com>
> 
> Adds an implementation of the wait_dat0 MMC operation for the DM SDHCI
> driver, allowing the driver to continue when the card is ready rather
> than waiting for the worst case time on each MMC switch operation.
> 
> Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
> ---
>  drivers/mmc/sdhci.c | 20 ++++++++++++++++++++
>  include/sdhci.h     |  2 ++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index eea4701d8a..bb55e00ef5 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -775,6 +775,25 @@ static int sdhci_get_cd(struct udevice *dev)
>  		return value;
>  }
>  
> +static int sdhci_wait_dat0(struct udevice *dev, int state,
> +			   int timeout_us)
> +{
> +	int tmp;
> +	struct mmc *mmc = mmc_get_mmc_dev(dev);
> +	struct sdhci_host *host = mmc->priv;
> +	unsigned long timeout = timer_get_us() + timeout_us;
> +
> +	// readx_poll_timeout is unsuitable because sdhci_readl accepts
> +	// two arguments

Removed the comment or use "/* */" instead of "//"
And i didn't understand what's unsuitable?

Best Regards,
Jaehoon Chung

> +	do {
> +		tmp = sdhci_readl(host, SDHCI_PRESENT_STATE);
> +		if (!!(tmp & SDHCI_DATA_0_LVL_MASK) == !!state)
> +			return 0;
> +	} while (!timeout_us || !time_after(timer_get_us(), timeout));
> +
> +	return -ETIMEDOUT;
> +}
> +
>  const struct dm_mmc_ops sdhci_ops = {
>  	.send_cmd	= sdhci_send_command,
>  	.set_ios	= sdhci_set_ios,
> @@ -783,6 +802,7 @@ const struct dm_mmc_ops sdhci_ops = {
>  #ifdef MMC_SUPPORTS_TUNING
>  	.execute_tuning	= sdhci_execute_tuning,
>  #endif
> +	.wait_dat0	= sdhci_wait_dat0,
>  };
>  #else
>  static const struct mmc_ops sdhci_ops = {
> diff --git a/include/sdhci.h b/include/sdhci.h
> index 0ae9471ad7..dd4eb41442 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -65,6 +65,8 @@
>  #define  SDHCI_CARD_STATE_STABLE	BIT(17)
>  #define  SDHCI_CARD_DETECT_PIN_LEVEL	BIT(18)
>  #define  SDHCI_WRITE_PROTECT	BIT(19)
> +#define  SDHCI_DATA_LVL_MASK	0x00F00000
> +#define   SDHCI_DATA_0_LVL_MASK BIT(20)
>  
>  #define SDHCI_HOST_CONTROL	0x28
>  #define  SDHCI_CTRL_LED		BIT(0)
>