Re: Re: [PATCH 2/2] drivers: mmc: Add wait_dat0 support for sdhci driver
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
Sent: Friday, August 27, 2021 10:14 PM
To: stcar...@linux.microsoft.com; u-boot@lists.denx.de
Cc: Peng Fan
Subject: Re: [PATCH 2/2] drivers: mmc: Add wait_dat0 support for sdhci
driver
On 8/18/21 4:46 AM, stcar...@linux.microsoft.com wrote:
From: Stephen Carlson
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
---
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 0x00F0
+#define SDHCI_DATA_0_LVL_MASK BIT(20)
#define SDHCI_HOST_CONTROL 0x28
#define SDHCI_CTRL_LEDBIT(0)
Re: [PATCH 2/2] drivers: mmc: Add wait_dat0 support for sdhci driver
On 8/18/21 4:46 AM, stcar...@linux.microsoft.com wrote:
> From: Stephen Carlson
>
> 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
> ---
> 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 0x00F0
> +#define SDHCI_DATA_0_LVL_MASK BIT(20)
>
> #define SDHCI_HOST_CONTROL 0x28
> #define SDHCI_CTRL_LED BIT(0)
>
[PATCH 2/2] drivers: mmc: Add wait_dat0 support for sdhci driver
From: Stephen Carlson
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
---
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
+ 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 0x00F0
+#define SDHCI_DATA_0_LVL_MASK BIT(20)
#define SDHCI_HOST_CONTROL 0x28
#define SDHCI_CTRL_LEDBIT(0)
--
2.17.1
