Re: [U-Boot] [PATCH 2/2] mmc: Rename timeout parameters for clarification
> 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 > --- Applied to mmc/master. Thanks, Peng. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] mmc: Rename timeout parameters for clarification
Hi Sam,
On Wed, Aug 14, 2019 at 10:53 PM Sam Protsenko
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
> ---
> 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'
Re: [U-Boot] [PATCH 2/2] mmc: Rename timeout parameters for clarification
> 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
> ---
> 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
[U-Boot] [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
---
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))
