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@samsung.com Sent: Friday, August 27, 2021 10:14 PM To: stcarlso@linux.microsoft.com; u-boot@lists.denx.de Cc: Peng Fan peng.fan@nxp.com Subject: Re: [PATCH 2/2] drivers: mmc: Add wait_dat0 support for sdhci driver

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)