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

Igor Opaniuk igor.opaniuk at gmail.com
Mon Aug 19 08:48:31 UTC 2019


Hi Sam,

On Wed, Aug 14, 2019 at 10:53 PM Sam Protsenko
<semen.protsenko at 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 at 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 at 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


More information about the U-Boot mailing list