[U-Boot] [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()

[U-Boot] [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()

[Sam Protsenko]

mmc_wait_dat0() expects timeout argument to be in usec units. But some
overlying functions operate on timeout in msec units. Convert timeout
from msec to usec when passing it to mmc_wait_dat0().

This fixes 'avb' commands on BeagleBoard X15, because next chain was
failing:

    get_partition() -> mmc_switch_part() -> __mmc_switch() ->
    mmc_wait_dat0()

when passing incorrect timeout from __mmc_switch() to mmc_wait_dat0().

Fixes: bb98b8c5c06a ("mmc: During a switch, poll on dat0 if available and check the final status")
Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
 drivers/mmc/mmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index eecc7d687e..e247730ff2 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -235,7 +235,7 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout)
 	unsigned int status;
 	int err;
 
-	err = mmc_wait_dat0(mmc, 1, timeout);
+	err = mmc_wait_dat0(mmc, 1, timeout * 1000);
 	if (err != -ENOSYS)
 		return err;
 
@@ -778,7 +778,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 	start = get_timer(0);
 
 	/* poll dat0 for rdy/buys status */
-	ret = mmc_wait_dat0(mmc, 1, timeout);
+	ret = mmc_wait_dat0(mmc, 1, timeout * 1000);
 	if (ret && ret != -ENOSYS)
 		return ret;
 
-- 
2.20.1

[U-Boot] [PATCH 2/2] mmc: Rename timeout parameters for clarification

[Sam Protsenko]

It's quite hard to figure out time units for various function that have
timeout parameters. This leads to possible errors when one forgets to
convert ms to us, for example. Let's rename those parameters
correspondingly to 'timeout_us' and 'timeout_ms' to prevent such issues
further.

While at it, add time units info as comments to struct mmc fields.

This commit doesn't change the behavior, only renames parameters names.
Buildman should report no changes at all.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
 drivers/mmc/mmc-uclass.c   |  8 ++++----
 drivers/mmc/mmc.c          | 24 ++++++++++++------------
 drivers/mmc/mmc_write.c    |  8 ++++----
 drivers/mmc/omap_hsmmc.c   |  6 +++---
 drivers/mmc/renesas-sdhi.c |  7 ++++---
 include/mmc.h              | 12 ++++++------
 6 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 551007905c..f740bae3c7 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -47,18 +47,18 @@ int mmc_set_ios(struct mmc *mmc)
 	return dm_mmc_set_ios(mmc->dev);
 }
 
-int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout)
+int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us)
 {
 	struct dm_mmc_ops *ops = mmc_get_ops(dev);
 
 	if (!ops->wait_dat0)
 		return -ENOSYS;
-	return ops->wait_dat0(dev, state, timeout);
+	return ops->wait_dat0(dev, state, timeout_us);
 }
 
-int mmc_wait_dat0(struct mmc *mmc, int state, int timeout)
+int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us)
 {
-	return dm_mmc_wait_dat0(mmc->dev, state, timeout);
+	return dm_mmc_wait_dat0(mmc->dev, state, timeout_us);
 }
 
 int dm_mmc_get_wp(struct udevice *dev)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index e247730ff2..c8f71cd0c1 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -31,7 +31,7 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps);
 
 #if !CONFIG_IS_ENABLED(DM_MMC)
 
-static int mmc_wait_dat0(struct mmc *mmc, int state, int timeout)
+static int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us)
 {
 	return -ENOSYS;
 }
@@ -230,12 +230,12 @@ int mmc_send_status(struct mmc *mmc, unsigned int *status)
 	return -ECOMM;
 }
 
-int mmc_poll_for_busy(struct mmc *mmc, int timeout)
+int mmc_poll_for_busy(struct mmc *mmc, int timeout_ms)
 {
 	unsigned int status;
 	int err;
 
-	err = mmc_wait_dat0(mmc, 1, timeout * 1000);
+	err = mmc_wait_dat0(mmc, 1, timeout_ms * 1000);
 	if (err != -ENOSYS)
 		return err;
 
@@ -256,13 +256,13 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout)
 			return -ECOMM;
 		}
 
-		if (timeout-- <= 0)
+		if (timeout_ms-- <= 0)
 			break;
 
 		udelay(1000);
 	}
 
-	if (timeout <= 0) {
+	if (timeout_ms <= 0) {
 #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 		pr_err("Timeout waiting card ready\n");
 #endif
@@ -750,17 +750,17 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 {
 	unsigned int status, start;
 	struct mmc_cmd cmd;
-	int timeout = DEFAULT_CMD6_TIMEOUT_MS;
+	int timeout_ms = DEFAULT_CMD6_TIMEOUT_MS;
 	bool is_part_switch = (set == EXT_CSD_CMD_SET_NORMAL) &&
 			      (index == EXT_CSD_PART_CONF);
 	int retries = 3;
 	int ret;
 
 	if (mmc->gen_cmd6_time)
-		timeout = mmc->gen_cmd6_time * 10;
+		timeout_ms = mmc->gen_cmd6_time * 10;
 
 	if (is_part_switch  && mmc->part_switch_time)
-		timeout = mmc->part_switch_time * 10;
+		timeout_ms = mmc->part_switch_time * 10;
 
 	cmd.cmdidx = MMC_CMD_SWITCH;
 	cmd.resp_type = MMC_RSP_R1b;
@@ -778,7 +778,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 	start = get_timer(0);
 
 	/* poll dat0 for rdy/buys status */
-	ret = mmc_wait_dat0(mmc, 1, timeout * 1000);
+	ret = mmc_wait_dat0(mmc, 1, timeout_ms * 1000);
 	if (ret && ret != -ENOSYS)
 		return ret;
 
@@ -788,11 +788,11 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 	 * stated timeout to be sufficient.
 	 */
 	if (ret == -ENOSYS && !send_status)
-		mdelay(timeout);
+		mdelay(timeout_ms);
 
 	/* Finally wait until the card is ready or indicates a failure
 	 * to switch. It doesn't hurt to use CMD13 here even if send_status
-	 * is false, because by now (after 'timeout' ms) the bus should be
+	 * is false, because by now (after 'timeout_ms' ms) the bus should be
 	 * reliable.
 	 */
 	do {
@@ -806,7 +806,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 		if (!ret && (status & MMC_STATUS_RDY_FOR_DATA))
 			return 0;
 		udelay(100);
-	} while (get_timer(start) < timeout);
+	} while (get_timer(start) < timeout_ms);
 
 	return -ETIMEDOUT;
 }
diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
index 02648b0f50..b52ff9f3bc 100644
--- a/drivers/mmc/mmc_write.c
+++ b/drivers/mmc/mmc_write.c
@@ -79,7 +79,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
 	u32 start_rem, blkcnt_rem;
 	struct mmc *mmc = find_mmc_device(dev_num);
 	lbaint_t blk = 0, blk_r = 0;
-	int timeout = 1000;
+	int timeout_ms = 1000;
 
 	if (!mmc)
 		return -1;
@@ -119,7 +119,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
 		blk += blk_r;
 
 		/* Waiting for the ready status */
-		if (mmc_poll_for_busy(mmc, timeout))
+		if (mmc_poll_for_busy(mmc, timeout_ms))
 			return 0;
 	}
 
@@ -131,7 +131,7 @@ static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
 {
 	struct mmc_cmd cmd;
 	struct mmc_data data;
-	int timeout = 1000;
+	int timeout_ms = 1000;
 
 	if ((start + blkcnt) > mmc_get_blk_desc(mmc)->lba) {
 		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
@@ -177,7 +177,7 @@ static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
 	}
 
 	/* Waiting for the ready status */
-	if (mmc_poll_for_busy(mmc, timeout))
+	if (mmc_poll_for_busy(mmc, timeout_ms))
 		return 0;
 
 	return blkcnt;
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index 3ea7f4e173..bade129aea 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -430,7 +430,7 @@ static void omap_hsmmc_conf_bus_power(struct mmc *mmc, uint signal_voltage)
 	writel(ac12, &mmc_base->ac12);
 }
 
-static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int timeout)
+static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int timeout_us)
 {
 	int ret = -ETIMEDOUT;
 	u32 con;
@@ -442,8 +442,8 @@ static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int timeout)
 	con = readl(&mmc_base->con);
 	writel(con | CON_CLKEXTFREE | CON_PADEN, &mmc_base->con);
 
-	timeout = DIV_ROUND_UP(timeout, 10); /* check every 10 us. */
-	while (timeout--)	{
+	timeout_us = DIV_ROUND_UP(timeout_us, 10); /* check every 10 us. */
+	while (timeout_us--) {
 		dat0_high = !!(readl(&mmc_base->pstate) & PSTATE_DLEV_DAT0);
 		if (dat0_high == target_dat0_high) {
 			ret = 0;
diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c
index 7c53aa221e..0cb65b480d 100644
--- a/drivers/mmc/renesas-sdhi.c
+++ b/drivers/mmc/renesas-sdhi.c
@@ -499,15 +499,16 @@ static int renesas_sdhi_set_ios(struct udevice *dev)
 }
 
 #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
-static int renesas_sdhi_wait_dat0(struct udevice *dev, int state, int timeout)
+static int renesas_sdhi_wait_dat0(struct udevice *dev, int state,
+				  int timeout_us)
 {
 	int ret = -ETIMEDOUT;
 	bool dat0_high;
 	bool target_dat0_high = !!state;
 	struct tmio_sd_priv *priv = dev_get_priv(dev);
 
-	timeout = DIV_ROUND_UP(timeout, 10); /* check every 10 us. */
-	while (timeout--) {
+	timeout_us = DIV_ROUND_UP(timeout_us, 10); /* check every 10 us. */
+	while (timeout_us--) {
 		dat0_high = !!(tmio_sd_readl(priv, TMIO_SD_INFO2) & TMIO_SD_INFO2_DAT0);
 		if (dat0_high == target_dat0_high) {
 			ret = 0;
diff --git a/include/mmc.h b/include/mmc.h
index 46422f41a4..686ba00656 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -457,10 +457,10 @@ struct dm_mmc_ops {
 	 *
 	 * @dev:	Device to check
 	 * @state:	target state
-	 * @timeout:	timeout in us
+	 * @timeout_us:	timeout in us
 	 * @return 0 if dat0 is in the target state, -ve on error
 	 */
-	int (*wait_dat0)(struct udevice *dev, int state, int timeout);
+	int (*wait_dat0)(struct udevice *dev, int state, int timeout_us);
 
 #if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
 	/* set_enhanced_strobe() - set HS400 enhanced strobe */
@@ -476,14 +476,14 @@ int dm_mmc_set_ios(struct udevice *dev);
 int dm_mmc_get_cd(struct udevice *dev);
 int dm_mmc_get_wp(struct udevice *dev);
 int dm_mmc_execute_tuning(struct udevice *dev, uint opcode);
-int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout);
+int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us);
 
 /* Transition functions for compatibility */
 int mmc_set_ios(struct mmc *mmc);
 int mmc_getcd(struct mmc *mmc);
 int mmc_getwp(struct mmc *mmc);
 int mmc_execute_tuning(struct mmc *mmc, uint opcode);
-int mmc_wait_dat0(struct mmc *mmc, int state, int timeout);
+int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us);
 int mmc_set_enhanced_strobe(struct mmc *mmc);
 
 #else
@@ -602,8 +602,8 @@ struct mmc {
 	u8 part_attr;
 	u8 wr_rel_set;
 	u8 part_config;
-	u8 gen_cmd6_time;
-	u8 part_switch_time;
+	u8 gen_cmd6_time;	/* units: 10 ms */
+	u8 part_switch_time;	/* units: 10 ms */
 	uint tran_speed;
 	uint legacy_speed; /* speed for the legacy mode provided by the card */
 	uint read_bl_len;
-- 
2.20.1

[U-Boot] [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()

[Eugeniu Rosca]

Hi Sam,

CC: Peng

On Wed, Aug 14, 2019 at 10:52:50PM +0300, Sam Protsenko wrote:
> mmc_wait_dat0() expects timeout argument to be in usec units.

I agree, based on the documentation of wait_dat0() from commit:
https://gitlab.denx.de/u-boot/u-boot/commit/c10b85d6c25f9#5a47a9a1803c0a873c9ec4b91ce244822405d1ec_413_436

> But some
> overlying functions operate on timeout in msec units.

That seems to be also true. The two functions touched in this commit
(mmc_poll_for_busy, __mmc_switch) seem to originate from Linux, where
they clearly accept MS timeout granularity [1-2].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=716bdb8953c7c
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f90d2e4035d45

> Convert timeout
> from msec to usec when passing it to mmc_wait_dat0().

I've reviewed all the callers of mmc_wait_dat0() and I couldn't find any
other occurrences of passing MS values to this function.

I've run 'ext2load mmc 0:1 0x48000000 Image' several times on
H3-Salvator-X w/o noticing any issues.

I agree with the idea of s/timeout/timeout_{us,ms}/ in the second patch
from this series.

> 
> This fixes 'avb' commands on BeagleBoard X15, because next chain was
> failing:
> 
>     get_partition() -> mmc_switch_part() -> __mmc_switch() ->
>     mmc_wait_dat0()
> 
> when passing incorrect timeout from __mmc_switch() to mmc_wait_dat0().
> 
> Fixes: bb98b8c5c06a ("mmc: During a switch, poll on dat0 if available and check the final status")
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>

Reviewed-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>
Tested-by: Eugeniu Rosca <rosca.eugeniu@gmail.com>

-- 
Best Regards,
Eugeniu.

[U-Boot] [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()

[Peng Fan]

> Subject: [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()
> 
> mmc_wait_dat0() expects timeout argument to be in usec units. But some
> overlying functions operate on timeout in msec units. Convert timeout from
> msec to usec when passing it to mmc_wait_dat0().
> 
> This fixes 'avb' commands on BeagleBoard X15, because next chain was
> failing:
> 
>     get_partition() -> mmc_switch_part() -> __mmc_switch() ->
>     mmc_wait_dat0()
> 
> when passing incorrect timeout from __mmc_switch() to mmc_wait_dat0().
> 
> Fixes: bb98b8c5c06a ("mmc: During a switch, poll on dat0 if available and
> check the final status")
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
>  drivers/mmc/mmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> eecc7d687e..e247730ff2 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -235,7 +235,7 @@ int mmc_poll_for_busy(struct mmc *mmc, int
> timeout)
>  	unsigned int status;
>  	int err;
> 
> -	err = mmc_wait_dat0(mmc, 1, timeout);
> +	err = mmc_wait_dat0(mmc, 1, timeout * 1000);
>  	if (err != -ENOSYS)
>  		return err;
> 
> @@ -778,7 +778,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set,
> u8 index, u8 value,
>  	start = get_timer(0);
> 
>  	/* poll dat0 for rdy/buys status */
> -	ret = mmc_wait_dat0(mmc, 1, timeout);
> +	ret = mmc_wait_dat0(mmc, 1, timeout * 1000);
>  	if (ret && ret != -ENOSYS)
>  		return ret;
> 

Reviewed-by: Peng Fan <peng.fan@nxp.com>

> --
> 2.20.1

[U-Boot] [PATCH 2/2] mmc: Rename timeout parameters for clarification

[Peng Fan]

> Subject: [PATCH 2/2] mmc: Rename timeout parameters for clarification
> 
> It's quite hard to figure out time units for various function that have timeout
> parameters. This leads to possible errors when one forgets to convert ms to
> us, for example. Let's rename those parameters correspondingly to
> 'timeout_us' and 'timeout_ms' to prevent such issues further.
> 
> While at it, add time units info as comments to struct mmc fields.
> 
> This commit doesn't change the behavior, only renames parameters names.
> Buildman should report no changes at all.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
>  drivers/mmc/mmc-uclass.c   |  8 ++++----
>  drivers/mmc/mmc.c          | 24 ++++++++++++------------
>  drivers/mmc/mmc_write.c    |  8 ++++----
>  drivers/mmc/omap_hsmmc.c   |  6 +++---
>  drivers/mmc/renesas-sdhi.c |  7 ++++---
>  include/mmc.h              | 12 ++++++------
>  6 files changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index
> 551007905c..f740bae3c7 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -47,18 +47,18 @@ int mmc_set_ios(struct mmc *mmc)
>  	return dm_mmc_set_ios(mmc->dev);
>  }
> 
> -int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout)
> +int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us)
>  {
>  	struct dm_mmc_ops *ops = mmc_get_ops(dev);
> 
>  	if (!ops->wait_dat0)
>  		return -ENOSYS;
> -	return ops->wait_dat0(dev, state, timeout);
> +	return ops->wait_dat0(dev, state, timeout_us);
>  }
> 
> -int mmc_wait_dat0(struct mmc *mmc, int state, int timeout)
> +int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us)
>  {
> -	return dm_mmc_wait_dat0(mmc->dev, state, timeout);
> +	return dm_mmc_wait_dat0(mmc->dev, state, timeout_us);
>  }
> 
>  int dm_mmc_get_wp(struct udevice *dev)
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> e247730ff2..c8f71cd0c1 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -31,7 +31,7 @@ static int mmc_select_mode_and_width(struct mmc
> *mmc, uint card_caps);
> 
>  #if !CONFIG_IS_ENABLED(DM_MMC)
> 
> -static int mmc_wait_dat0(struct mmc *mmc, int state, int timeout)
> +static int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us)
>  {
>  	return -ENOSYS;
>  }
> @@ -230,12 +230,12 @@ int mmc_send_status(struct mmc *mmc, unsigned
> int *status)
>  	return -ECOMM;
>  }
> 
> -int mmc_poll_for_busy(struct mmc *mmc, int timeout)
> +int mmc_poll_for_busy(struct mmc *mmc, int timeout_ms)
>  {
>  	unsigned int status;
>  	int err;
> 
> -	err = mmc_wait_dat0(mmc, 1, timeout * 1000);
> +	err = mmc_wait_dat0(mmc, 1, timeout_ms * 1000);
>  	if (err != -ENOSYS)
>  		return err;
> 
> @@ -256,13 +256,13 @@ int mmc_poll_for_busy(struct mmc *mmc, int
> timeout)
>  			return -ECOMM;
>  		}
> 
> -		if (timeout-- <= 0)
> +		if (timeout_ms-- <= 0)
>  			break;
> 
>  		udelay(1000);
>  	}
> 
> -	if (timeout <= 0) {
> +	if (timeout_ms <= 0) {
>  #if !defined(CONFIG_SPL_BUILD) ||
> defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>  		pr_err("Timeout waiting card ready\n");  #endif @@ -750,17
> +750,17 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8
> value,  {
>  	unsigned int status, start;
>  	struct mmc_cmd cmd;
> -	int timeout = DEFAULT_CMD6_TIMEOUT_MS;
> +	int timeout_ms = DEFAULT_CMD6_TIMEOUT_MS;
>  	bool is_part_switch = (set == EXT_CSD_CMD_SET_NORMAL) &&
>  			      (index == EXT_CSD_PART_CONF);
>  	int retries = 3;
>  	int ret;
> 
>  	if (mmc->gen_cmd6_time)
> -		timeout = mmc->gen_cmd6_time * 10;
> +		timeout_ms = mmc->gen_cmd6_time * 10;
> 
>  	if (is_part_switch  && mmc->part_switch_time)
> -		timeout = mmc->part_switch_time * 10;
> +		timeout_ms = mmc->part_switch_time * 10;
> 
>  	cmd.cmdidx = MMC_CMD_SWITCH;
>  	cmd.resp_type = MMC_RSP_R1b;
> @@ -778,7 +778,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set,
> u8 index, u8 value,
>  	start = get_timer(0);
> 
>  	/* poll dat0 for rdy/buys status */
> -	ret = mmc_wait_dat0(mmc, 1, timeout * 1000);
> +	ret = mmc_wait_dat0(mmc, 1, timeout_ms * 1000);
>  	if (ret && ret != -ENOSYS)
>  		return ret;
> 
> @@ -788,11 +788,11 @@ static int __mmc_switch(struct mmc *mmc, u8 set,
> u8 index, u8 value,
>  	 * stated timeout to be sufficient.
>  	 */
>  	if (ret == -ENOSYS && !send_status)
> -		mdelay(timeout);
> +		mdelay(timeout_ms);
> 
>  	/* Finally wait until the card is ready or indicates a failure
>  	 * to switch. It doesn't hurt to use CMD13 here even if send_status
> -	 * is false, because by now (after 'timeout' ms) the bus should be
> +	 * is false, because by now (after 'timeout_ms' ms) the bus should be
>  	 * reliable.
>  	 */
>  	do {
> @@ -806,7 +806,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set,
> u8 index, u8 value,
>  		if (!ret && (status & MMC_STATUS_RDY_FOR_DATA))
>  			return 0;
>  		udelay(100);
> -	} while (get_timer(start) < timeout);
> +	} while (get_timer(start) < timeout_ms);
> 
>  	return -ETIMEDOUT;
>  }
> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c index
> 02648b0f50..b52ff9f3bc 100644
> --- a/drivers/mmc/mmc_write.c
> +++ b/drivers/mmc/mmc_write.c
> @@ -79,7 +79,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t
> start, lbaint_t blkcnt)
>  	u32 start_rem, blkcnt_rem;
>  	struct mmc *mmc = find_mmc_device(dev_num);
>  	lbaint_t blk = 0, blk_r = 0;
> -	int timeout = 1000;
> +	int timeout_ms = 1000;
> 
>  	if (!mmc)
>  		return -1;
> @@ -119,7 +119,7 @@ ulong mmc_berase(struct blk_desc *block_dev,
> lbaint_t start, lbaint_t blkcnt)
>  		blk += blk_r;
> 
>  		/* Waiting for the ready status */
> -		if (mmc_poll_for_busy(mmc, timeout))
> +		if (mmc_poll_for_busy(mmc, timeout_ms))
>  			return 0;
>  	}
> 
> @@ -131,7 +131,7 @@ static ulong mmc_write_blocks(struct mmc *mmc,
> lbaint_t start,  {
>  	struct mmc_cmd cmd;
>  	struct mmc_data data;
> -	int timeout = 1000;
> +	int timeout_ms = 1000;
> 
>  	if ((start + blkcnt) > mmc_get_blk_desc(mmc)->lba) {
>  		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
> @@ -177,7 +177,7 @@ static ulong mmc_write_blocks(struct mmc *mmc,
> lbaint_t start,
>  	}
> 
>  	/* Waiting for the ready status */
> -	if (mmc_poll_for_busy(mmc, timeout))
> +	if (mmc_poll_for_busy(mmc, timeout_ms))
>  		return 0;
> 
>  	return blkcnt;
> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> index 3ea7f4e173..bade129aea 100644
> --- a/drivers/mmc/omap_hsmmc.c
> +++ b/drivers/mmc/omap_hsmmc.c
> @@ -430,7 +430,7 @@ static void omap_hsmmc_conf_bus_power(struct
> mmc *mmc, uint signal_voltage)
>  	writel(ac12, &mmc_base->ac12);
>  }
> 
> -static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int timeout)
> +static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int
> +timeout_us)
>  {
>  	int ret = -ETIMEDOUT;
>  	u32 con;
> @@ -442,8 +442,8 @@ static int omap_hsmmc_wait_dat0(struct udevice
> *dev, int state, int timeout)
>  	con = readl(&mmc_base->con);
>  	writel(con | CON_CLKEXTFREE | CON_PADEN, &mmc_base->con);
> 
> -	timeout = DIV_ROUND_UP(timeout, 10); /* check every 10 us. */
> -	while (timeout--)	{
> +	timeout_us = DIV_ROUND_UP(timeout_us, 10); /* check every 10 us. */
> +	while (timeout_us--) {
>  		dat0_high = !!(readl(&mmc_base->pstate) & PSTATE_DLEV_DAT0);
>  		if (dat0_high == target_dat0_high) {
>  			ret = 0;
> diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c index
> 7c53aa221e..0cb65b480d 100644
> --- a/drivers/mmc/renesas-sdhi.c
> +++ b/drivers/mmc/renesas-sdhi.c
> @@ -499,15 +499,16 @@ static int renesas_sdhi_set_ios(struct udevice
> *dev)  }
> 
>  #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
> -static int renesas_sdhi_wait_dat0(struct udevice *dev, int state, int timeout)
> +static int renesas_sdhi_wait_dat0(struct udevice *dev, int state,
> +				  int timeout_us)
>  {
>  	int ret = -ETIMEDOUT;
>  	bool dat0_high;
>  	bool target_dat0_high = !!state;
>  	struct tmio_sd_priv *priv = dev_get_priv(dev);
> 
> -	timeout = DIV_ROUND_UP(timeout, 10); /* check every 10 us. */
> -	while (timeout--) {
> +	timeout_us = DIV_ROUND_UP(timeout_us, 10); /* check every 10 us. */
> +	while (timeout_us--) {
>  		dat0_high = !!(tmio_sd_readl(priv, TMIO_SD_INFO2) &
> TMIO_SD_INFO2_DAT0);
>  		if (dat0_high == target_dat0_high) {
>  			ret = 0;
> diff --git a/include/mmc.h b/include/mmc.h index 46422f41a4..686ba00656
> 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -457,10 +457,10 @@ struct dm_mmc_ops {
>  	 *
>  	 * @dev:	Device to check
>  	 * @state:	target state
> -	 * @timeout:	timeout in us
> +	 * @timeout_us:	timeout in us
>  	 * @return 0 if dat0 is in the target state, -ve on error
>  	 */
> -	int (*wait_dat0)(struct udevice *dev, int state, int timeout);
> +	int (*wait_dat0)(struct udevice *dev, int state, int timeout_us);
> 
>  #if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
>  	/* set_enhanced_strobe() - set HS400 enhanced strobe */ @@ -476,14
> +476,14 @@ int dm_mmc_set_ios(struct udevice *dev);  int
> dm_mmc_get_cd(struct udevice *dev);  int dm_mmc_get_wp(struct udevice
> *dev);  int dm_mmc_execute_tuning(struct udevice *dev, uint opcode); -int
> dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout);
> +int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us);
> 
>  /* Transition functions for compatibility */  int mmc_set_ios(struct mmc
> *mmc);  int mmc_getcd(struct mmc *mmc);  int mmc_getwp(struct mmc
> *mmc);  int mmc_execute_tuning(struct mmc *mmc, uint opcode); -int
> mmc_wait_dat0(struct mmc *mmc, int state, int timeout);
> +int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us);
>  int mmc_set_enhanced_strobe(struct mmc *mmc);
> 
>  #else
> @@ -602,8 +602,8 @@ struct mmc {
>  	u8 part_attr;
>  	u8 wr_rel_set;
>  	u8 part_config;
> -	u8 gen_cmd6_time;
> -	u8 part_switch_time;
> +	u8 gen_cmd6_time;	/* units: 10 ms */
> +	u8 part_switch_time;	/* units: 10 ms */
>  	uint tran_speed;
>  	uint legacy_speed; /* speed for the legacy mode provided by the card */
>  	uint read_bl_len;

Reviewed-by: Peng Fan <peng.fan@nxp.com>

> --
> 2.20.1

[U-Boot] [PATCH 2/2] mmc: Rename timeout parameters for clarification

[Igor Opaniuk]

Hi Sam,

On Wed, Aug 14, 2019 at 10:53 PM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
>
> It's quite hard to figure out time units for various function that have
> timeout parameters. This leads to possible errors when one forgets to
> convert ms to us, for example. Let's rename those parameters
> correspondingly to 'timeout_us' and 'timeout_ms' to prevent such issues
> further.
>
> While at it, add time units info as comments to struct mmc fields.
>
> This commit doesn't change the behavior, only renames parameters names.
> Buildman should report no changes at all.
>
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
>  drivers/mmc/mmc-uclass.c   |  8 ++++----
>  drivers/mmc/mmc.c          | 24 ++++++++++++------------
>  drivers/mmc/mmc_write.c    |  8 ++++----
>  drivers/mmc/omap_hsmmc.c   |  6 +++---
>  drivers/mmc/renesas-sdhi.c |  7 ++++---
>  include/mmc.h              | 12 ++++++------
>  6 files changed, 33 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 551007905c..f740bae3c7 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -47,18 +47,18 @@ int mmc_set_ios(struct mmc *mmc)
>         return dm_mmc_set_ios(mmc->dev);
>  }
>
> -int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout)
> +int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us)
>  {
>         struct dm_mmc_ops *ops = mmc_get_ops(dev);
>
>         if (!ops->wait_dat0)
>                 return -ENOSYS;
> -       return ops->wait_dat0(dev, state, timeout);
> +       return ops->wait_dat0(dev, state, timeout_us);
>  }
>
> -int mmc_wait_dat0(struct mmc *mmc, int state, int timeout)
> +int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us)
>  {
> -       return dm_mmc_wait_dat0(mmc->dev, state, timeout);
> +       return dm_mmc_wait_dat0(mmc->dev, state, timeout_us);
>  }
>
>  int dm_mmc_get_wp(struct udevice *dev)
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index e247730ff2..c8f71cd0c1 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -31,7 +31,7 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps);
>
>  #if !CONFIG_IS_ENABLED(DM_MMC)
>
> -static int mmc_wait_dat0(struct mmc *mmc, int state, int timeout)
> +static int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us)
>  {
>         return -ENOSYS;
>  }
> @@ -230,12 +230,12 @@ int mmc_send_status(struct mmc *mmc, unsigned int *status)
>         return -ECOMM;
>  }
>
> -int mmc_poll_for_busy(struct mmc *mmc, int timeout)
> +int mmc_poll_for_busy(struct mmc *mmc, int timeout_ms)
>  {
>         unsigned int status;
>         int err;
>
> -       err = mmc_wait_dat0(mmc, 1, timeout * 1000);
> +       err = mmc_wait_dat0(mmc, 1, timeout_ms * 1000);
>         if (err != -ENOSYS)
>                 return err;
>
> @@ -256,13 +256,13 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout)
>                         return -ECOMM;
>                 }
>
> -               if (timeout-- <= 0)
> +               if (timeout_ms-- <= 0)
>                         break;
>
>                 udelay(1000);
>         }
>
> -       if (timeout <= 0) {
> +       if (timeout_ms <= 0) {
>  #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>                 pr_err("Timeout waiting card ready\n");
>  #endif
> @@ -750,17 +750,17 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
>  {
>         unsigned int status, start;
>         struct mmc_cmd cmd;
> -       int timeout = DEFAULT_CMD6_TIMEOUT_MS;
> +       int timeout_ms = DEFAULT_CMD6_TIMEOUT_MS;
>         bool is_part_switch = (set == EXT_CSD_CMD_SET_NORMAL) &&
>                               (index == EXT_CSD_PART_CONF);
>         int retries = 3;
>         int ret;
>
>         if (mmc->gen_cmd6_time)
> -               timeout = mmc->gen_cmd6_time * 10;
> +               timeout_ms = mmc->gen_cmd6_time * 10;
>
>         if (is_part_switch  && mmc->part_switch_time)
> -               timeout = mmc->part_switch_time * 10;
> +               timeout_ms = mmc->part_switch_time * 10;
>
>         cmd.cmdidx = MMC_CMD_SWITCH;
>         cmd.resp_type = MMC_RSP_R1b;
> @@ -778,7 +778,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
>         start = get_timer(0);
>
>         /* poll dat0 for rdy/buys status */
> -       ret = mmc_wait_dat0(mmc, 1, timeout * 1000);
> +       ret = mmc_wait_dat0(mmc, 1, timeout_ms * 1000);
>         if (ret && ret != -ENOSYS)
>                 return ret;
>
> @@ -788,11 +788,11 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
>          * stated timeout to be sufficient.
>          */
>         if (ret == -ENOSYS && !send_status)
> -               mdelay(timeout);
> +               mdelay(timeout_ms);
>
>         /* Finally wait until the card is ready or indicates a failure
>          * to switch. It doesn't hurt to use CMD13 here even if send_status
> -        * is false, because by now (after 'timeout' ms) the bus should be
> +        * is false, because by now (after 'timeout_ms' ms) the bus should be
>          * reliable.
>          */
>         do {
> @@ -806,7 +806,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
>                 if (!ret && (status & MMC_STATUS_RDY_FOR_DATA))
>                         return 0;
>                 udelay(100);
> -       } while (get_timer(start) < timeout);
> +       } while (get_timer(start) < timeout_ms);
>
>         return -ETIMEDOUT;
>  }
> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
> index 02648b0f50..b52ff9f3bc 100644
> --- a/drivers/mmc/mmc_write.c
> +++ b/drivers/mmc/mmc_write.c
> @@ -79,7 +79,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
>         u32 start_rem, blkcnt_rem;
>         struct mmc *mmc = find_mmc_device(dev_num);
>         lbaint_t blk = 0, blk_r = 0;
> -       int timeout = 1000;
> +       int timeout_ms = 1000;
>
>         if (!mmc)
>                 return -1;
> @@ -119,7 +119,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
>                 blk += blk_r;
>
>                 /* Waiting for the ready status */
> -               if (mmc_poll_for_busy(mmc, timeout))
> +               if (mmc_poll_for_busy(mmc, timeout_ms))
>                         return 0;
>         }
>
> @@ -131,7 +131,7 @@ static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
>  {
>         struct mmc_cmd cmd;
>         struct mmc_data data;
> -       int timeout = 1000;
> +       int timeout_ms = 1000;
>
>         if ((start + blkcnt) > mmc_get_blk_desc(mmc)->lba) {
>                 printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
> @@ -177,7 +177,7 @@ static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
>         }
>
>         /* Waiting for the ready status */
> -       if (mmc_poll_for_busy(mmc, timeout))
> +       if (mmc_poll_for_busy(mmc, timeout_ms))
>                 return 0;
>
>         return blkcnt;
> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> index 3ea7f4e173..bade129aea 100644
> --- a/drivers/mmc/omap_hsmmc.c
> +++ b/drivers/mmc/omap_hsmmc.c
> @@ -430,7 +430,7 @@ static void omap_hsmmc_conf_bus_power(struct mmc *mmc, uint signal_voltage)
>         writel(ac12, &mmc_base->ac12);
>  }
>
> -static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int timeout)
> +static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int timeout_us)
>  {
>         int ret = -ETIMEDOUT;
>         u32 con;
> @@ -442,8 +442,8 @@ static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int timeout)
>         con = readl(&mmc_base->con);
>         writel(con | CON_CLKEXTFREE | CON_PADEN, &mmc_base->con);
>
> -       timeout = DIV_ROUND_UP(timeout, 10); /* check every 10 us. */
> -       while (timeout--)       {
> +       timeout_us = DIV_ROUND_UP(timeout_us, 10); /* check every 10 us. */
> +       while (timeout_us--) {
>                 dat0_high = !!(readl(&mmc_base->pstate) & PSTATE_DLEV_DAT0);
>                 if (dat0_high == target_dat0_high) {
>                         ret = 0;
> diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c
> index 7c53aa221e..0cb65b480d 100644
> --- a/drivers/mmc/renesas-sdhi.c
> +++ b/drivers/mmc/renesas-sdhi.c
> @@ -499,15 +499,16 @@ static int renesas_sdhi_set_ios(struct udevice *dev)
>  }
>
>  #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
> -static int renesas_sdhi_wait_dat0(struct udevice *dev, int state, int timeout)
> +static int renesas_sdhi_wait_dat0(struct udevice *dev, int state,
> +                                 int timeout_us)
>  {
>         int ret = -ETIMEDOUT;
>         bool dat0_high;
>         bool target_dat0_high = !!state;
>         struct tmio_sd_priv *priv = dev_get_priv(dev);
>
> -       timeout = DIV_ROUND_UP(timeout, 10); /* check every 10 us. */
> -       while (timeout--) {
> +       timeout_us = DIV_ROUND_UP(timeout_us, 10); /* check every 10 us. */
> +       while (timeout_us--) {
>                 dat0_high = !!(tmio_sd_readl(priv, TMIO_SD_INFO2) & TMIO_SD_INFO2_DAT0);
>                 if (dat0_high == target_dat0_high) {
>                         ret = 0;
> diff --git a/include/mmc.h b/include/mmc.h
> index 46422f41a4..686ba00656 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -457,10 +457,10 @@ struct dm_mmc_ops {
>          *
>          * @dev:        Device to check
>          * @state:      target state
> -        * @timeout:    timeout in us
> +        * @timeout_us: timeout in us
>          * @return 0 if dat0 is in the target state, -ve on error
>          */
> -       int (*wait_dat0)(struct udevice *dev, int state, int timeout);
> +       int (*wait_dat0)(struct udevice *dev, int state, int timeout_us);
>
>  #if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
>         /* set_enhanced_strobe() - set HS400 enhanced strobe */
> @@ -476,14 +476,14 @@ int dm_mmc_set_ios(struct udevice *dev);
>  int dm_mmc_get_cd(struct udevice *dev);
>  int dm_mmc_get_wp(struct udevice *dev);
>  int dm_mmc_execute_tuning(struct udevice *dev, uint opcode);
> -int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout);
> +int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us);
>
>  /* Transition functions for compatibility */
>  int mmc_set_ios(struct mmc *mmc);
>  int mmc_getcd(struct mmc *mmc);
>  int mmc_getwp(struct mmc *mmc);
>  int mmc_execute_tuning(struct mmc *mmc, uint opcode);
> -int mmc_wait_dat0(struct mmc *mmc, int state, int timeout);
> +int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us);
>  int mmc_set_enhanced_strobe(struct mmc *mmc);
>
>  #else
> @@ -602,8 +602,8 @@ struct mmc {
>         u8 part_attr;
>         u8 wr_rel_set;
>         u8 part_config;
> -       u8 gen_cmd6_time;
> -       u8 part_switch_time;
> +       u8 gen_cmd6_time;       /* units: 10 ms */
> +       u8 part_switch_time;    /* units: 10 ms */
>         uint tran_speed;
>         uint legacy_speed; /* speed for the legacy mode provided by the card */
>         uint read_bl_len;
> --
> 2.20.1
>

Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[U-Boot] [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()

[Igor Opaniuk]

Hi Sam,
So you finally found it :)

On Wed, Aug 14, 2019 at 10:52 PM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
>
> mmc_wait_dat0() expects timeout argument to be in usec units. But some
> overlying functions operate on timeout in msec units. Convert timeout
> from msec to usec when passing it to mmc_wait_dat0().
>
> This fixes 'avb' commands on BeagleBoard X15, because next chain was
> failing:
>
>     get_partition() -> mmc_switch_part() -> __mmc_switch() ->
>     mmc_wait_dat0()
>
> when passing incorrect timeout from __mmc_switch() to mmc_wait_dat0().
>
> Fixes: bb98b8c5c06a ("mmc: During a switch, poll on dat0 if available and check the final status")
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
>  drivers/mmc/mmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index eecc7d687e..e247730ff2 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -235,7 +235,7 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout)
>         unsigned int status;
>         int err;
>
> -       err = mmc_wait_dat0(mmc, 1, timeout);
> +       err = mmc_wait_dat0(mmc, 1, timeout * 1000);
>         if (err != -ENOSYS)
>                 return err;
>
> @@ -778,7 +778,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
>         start = get_timer(0);
>
>         /* poll dat0 for rdy/buys status */
> -       ret = mmc_wait_dat0(mmc, 1, timeout);
> +       ret = mmc_wait_dat0(mmc, 1, timeout * 1000);
>         if (ret && ret != -ENOSYS)
>                 return ret;
>
> --
> 2.20.1
>

Tested-by: Igor Opaniuk <igor.opaniuk@gmail.com>
Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[U-Boot] [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()

[Peng Fan]

> Subject: [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()
> 
> mmc_wait_dat0() expects timeout argument to be in usec units. But some
> overlying functions operate on timeout in msec units. Convert timeout from
> msec to usec when passing it to mmc_wait_dat0().
> 
> This fixes 'avb' commands on BeagleBoard X15, because next chain was
> failing:
> 
>     get_partition() -> mmc_switch_part() -> __mmc_switch() ->
>     mmc_wait_dat0()
> 
> when passing incorrect timeout from __mmc_switch() to mmc_wait_dat0().
> 
> Fixes: bb98b8c5c06a ("mmc: During a switch, poll on dat0 if available and
> check the final status")
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>

Applied to mmc/master.

Thanks,
Peng.

[U-Boot] [PATCH 2/2] mmc: Rename timeout parameters for clarification

[Peng Fan]

> Subject: [PATCH 2/2] mmc: Rename timeout parameters for clarification
> 
> It's quite hard to figure out time units for various function that have timeout
> parameters. This leads to possible errors when one forgets to convert ms to
> us, for example. Let's rename those parameters correspondingly to
> 'timeout_us' and 'timeout_ms' to prevent such issues further.
> 
> While at it, add time units info as comments to struct mmc fields.
> 
> This commit doesn't change the behavior, only renames parameters names.
> Buildman should report no changes at all.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---

Applied to mmc/master.

Thanks,
Peng.

[U-Boot] [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()

[Sam Protsenko]

Hi Peng,

On Tue, Aug 27, 2019 at 10:40 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> > Subject: [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()
> >
> > mmc_wait_dat0() expects timeout argument to be in usec units. But some
> > overlying functions operate on timeout in msec units. Convert timeout from
> > msec to usec when passing it to mmc_wait_dat0().
> >
> > This fixes 'avb' commands on BeagleBoard X15, because next chain was
> > failing:
> >
> >     get_partition() -> mmc_switch_part() -> __mmc_switch() ->
> >     mmc_wait_dat0()
> >
> > when passing incorrect timeout from __mmc_switch() to mmc_wait_dat0().
> >
> > Fixes: bb98b8c5c06a ("mmc: During a switch, poll on dat0 if available and
> > check the final status")
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
>
> Applied to mmc/master.
>

Thanks for taking care of this. Quick question: does it mean that this
patch will find its way to v2019.10? Because this patch is a bug fix
to me (it actually fixes AVB, which doesn't work at all right now).

Thanks!

> Thanks,
> Peng.

[U-Boot] [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()

[Peng Fan]

> Subject: Re: [PATCH 1/2] mmc: Fix timeout values passed to mmc_wait_dat0()
> 
> Hi Peng,
> 
> On Tue, Aug 27, 2019 at 10:40 AM Peng Fan <peng.fan@nxp.com> wrote:
> >
> > > Subject: [PATCH 1/2] mmc: Fix timeout values passed to
> > > mmc_wait_dat0()
> > >
> > > mmc_wait_dat0() expects timeout argument to be in usec units. But
> > > some overlying functions operate on timeout in msec units. Convert
> > > timeout from msec to usec when passing it to mmc_wait_dat0().
> > >
> > > This fixes 'avb' commands on BeagleBoard X15, because next chain was
> > > failing:
> > >
> > >     get_partition() -> mmc_switch_part() -> __mmc_switch() ->
> > >     mmc_wait_dat0()
> > >
> > > when passing incorrect timeout from __mmc_switch() to
> mmc_wait_dat0().
> > >
> > > Fixes: bb98b8c5c06a ("mmc: During a switch, poll on dat0 if
> > > available and check the final status")
> > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> >
> > Applied to mmc/master.
> >
> 
> Thanks for taking care of this. Quick question: does it mean that this patch will
> find its way to v2019.10? Because this patch is a bug fix to me (it actually fixes
> AVB, which doesn't work at all right now).

Yes. It will land in 2019.10, I'll send pull request to Tom later.

Regards,
Peng.

> 
> Thanks!
> 
> > Thanks,
> > Peng.