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

[Stephen Carlson]

Jaehoon,

readx_poll_timeout expands to read_poll_timeout which accepts the signature:

read_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)

sdhci_readl requires two arguments, host and SHCI_PRESENT_STATE, which 
cannot both be provided to that macro in the addr parameter. One potential 
workaround would be to declare a static helper function to call 
sdhci_readl with a constant second parameter, but this proposal would 
increase function call overhead and stack usage. Is it worth changing for 
the readability improvement?

Thanks,

Stephen Carlson

-----Original Message-----
From: Jaehoon Chung <jh80.chung at samsung.com>
Sent: Friday, August 27, 2021 10:14 PM
To: stcarlso at linux.microsoft.com; u-boot at lists.denx.de
Cc: Peng Fan <peng.fan at nxp.com>
Subject: Re: [PATCH 2/2] drivers: mmc: Add wait_dat0 support for sdhci 
driver

On 8/18/21 4:46 AM, stcarlso at linux.microsoft.com wrote:
> From: Stephen Carlson <stcarlso at 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 at 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)
>