mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH master 1/2] mci: imx-esdhc: restore longer timeouts for idle
@ 2025-07-02 10:59 Ahmad Fatoum
  2025-07-02 10:59 ` [PATCH master 2/2] mci: sdhci: fix too short timeout in sdhci_wait_idle_data Ahmad Fatoum
  2025-07-02 12:27 ` [PATCH master 1/2] mci: imx-esdhc: restore longer timeouts for idle Sascha Hauer
  0 siblings, 2 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2025-07-02 10:59 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Commit 02986964885c ("mci: imx-esdhc: implement esdhc_poll using
sdhci_read32_poll_timeout") introduces sdhci_compute_timeout() which
takes a default timeout, but that default timeout is ignored in the
function.

Commit bbecd0b7bb7e ("mci: sdhci: add support for struct mci_data::timeout_ns")
then makes use of this function and with this breaks the polling for DATA0
line by reducing the original 2.5s timeout to SDHCI_CMD_DEFAULT_BUSY_TIMEOUT_NS
which is 10ms.

With this writing to the card times out during a MMC_CMD_STOP_TRANSMISSION
command, observed on i.MX6ul.

All these timeouts print errors and are not expected to trigger in
normal operation, therefore just make the hardcoded timeouts in the
driver the new minimum.

Fixes: 02986964885c ("mci: imx-esdhc: implement esdhc_poll using sdhci_read32_poll_timeout")
Fixes: bbecd0b7bb7e ("mci: sdhci: add support for struct mci_data::timeout_ns")
Reported-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/imx-esdhc-common.c | 6 +++---
 drivers/mci/sdhci.c            | 2 +-
 drivers/mci/sdhci.h            | 4 +---
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/mci/imx-esdhc-common.c b/drivers/mci/imx-esdhc-common.c
index 228f4a9b03fe..66f3edc670e0 100644
--- a/drivers/mci/imx-esdhc-common.c
+++ b/drivers/mci/imx-esdhc-common.c
@@ -394,7 +394,7 @@ int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
 		 */
 		ret = esdhc_poll(host, SDHCI_PRESENT_STATE, val,
 				 val & PRSSTAT_DAT0,
-				 sdhci_compute_timeout(cmd, NULL, 2500 * MSECOND));
+				 max_t(u64, sdhci_compute_timeout(cmd, NULL), 2500 * MSECOND));
 		if (ret) {
 			dev_err(host->dev, "timeout PRSSTAT_DAT0\n");
 			goto undo_setup_data;
@@ -419,7 +419,7 @@ int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
 	/* Wait for the bus to be idle */
 	ret = esdhc_poll(host, SDHCI_PRESENT_STATE, val,
 			 (val & (SDHCI_CMD_INHIBIT_CMD | SDHCI_CMD_INHIBIT_DATA)) == 0,
-			 sdhci_compute_timeout(cmd, data, SECOND));
+			 max_t(u64, sdhci_compute_timeout(cmd, data), SECOND));
 	if (ret) {
 		dev_err(host->dev, "timeout 2\n");
 		return -ETIMEDOUT;
@@ -427,7 +427,7 @@ int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
 
 	ret = esdhc_poll(host, SDHCI_PRESENT_STATE, val,
 			 (val & SDHCI_DATA_LINE_ACTIVE) == 0,
-			 sdhci_compute_timeout(cmd, NULL, 100 * MSECOND));
+			 max_t(u64, sdhci_compute_timeout(cmd, NULL), 100 * MSECOND));
 	if (ret) {
 		dev_err(host->dev, "timeout 3\n");
 		return -ETIMEDOUT;
diff --git a/drivers/mci/sdhci.c b/drivers/mci/sdhci.c
index 17847a13ed5f..9bf886dfe3f4 100644
--- a/drivers/mci/sdhci.c
+++ b/drivers/mci/sdhci.c
@@ -836,7 +836,7 @@ int sdhci_wait_idle(struct sdhci *host, struct mci_cmd *cmd, struct mci_data *da
 		    mmc_op_tuning(cmd->cmdidx)))
 		mask &= ~SDHCI_CMD_INHIBIT_DATA;
 
-	timeout_ns = sdhci_compute_timeout(cmd, data, SDHCI_CMD_DEFAULT_BUSY_TIMEOUT_NS);
+	timeout_ns = sdhci_compute_timeout(cmd, data);
 
 	ret = wait_on_timeout(timeout_ns,
 			!(sdhci_read32(host, SDHCI_PRESENT_STATE) & mask));
diff --git a/drivers/mci/sdhci.h b/drivers/mci/sdhci.h
index 25d257b23145..ec82b1b8ff9a 100644
--- a/drivers/mci/sdhci.h
+++ b/drivers/mci/sdhci.h
@@ -372,12 +372,10 @@ void sdhci_set_bus_width(struct sdhci *host, int width);
  * sdhci_compute_timeout() - compute suitable timeout for operation
  * @cmd: MCI command being sent, can be NULL
  * @data: MCI data being sent, can be NULL
- * @default_timeout: fallback value
  *
  * Return: the number of nanoseconds to wait.
  */
-static inline ktime_t sdhci_compute_timeout(struct mci_cmd *cmd, struct mci_data *data,
-					    ktime_t default_timeout)
+static inline ktime_t sdhci_compute_timeout(struct mci_cmd *cmd, struct mci_data *data)
 {
 	if (data && data->timeout_ns != 0)
 		return data->timeout_ns;
-- 
2.39.5




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH master 2/2] mci: sdhci: fix too short timeout in sdhci_wait_idle_data
  2025-07-02 10:59 [PATCH master 1/2] mci: imx-esdhc: restore longer timeouts for idle Ahmad Fatoum
@ 2025-07-02 10:59 ` Ahmad Fatoum
  2025-07-02 12:27 ` [PATCH master 1/2] mci: imx-esdhc: restore longer timeouts for idle Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2025-07-02 10:59 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

cmd::busy_timeout is in milliseconds like its Linux counterpart and was
correctly used as such, when it was first added.
Switching the function to use SDHCI_CMD_DEFAULT_BUSY_TIMEOUT_NS
instead of SDHCI_CMD_DEFAULT_BUSY_TIMEOUT_MS mixed up the units and
probably reintroduced the regression that commit 8242c8e6fbef
("mci: sdhci: use the busy_timeout value in the sdhci_wait_idle functions")
originally fixed.

Fixes: bbecd0b7bb7e ("mci: sdhci: add support for struct mci_data::timeout_ns")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/sdhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mci/sdhci.c b/drivers/mci/sdhci.c
index 9bf886dfe3f4..38a108adb1a8 100644
--- a/drivers/mci/sdhci.c
+++ b/drivers/mci/sdhci.c
@@ -852,7 +852,7 @@ int sdhci_wait_idle(struct sdhci *host, struct mci_cmd *cmd, struct mci_data *da
 int sdhci_wait_idle_data(struct sdhci *host, struct mci_cmd *cmd)
 {
 	u32 mask;
-	unsigned timeout_ns;
+	ktime_t timeout_ns;
 	int ret;
 
 	mask = SDHCI_CMD_INHIBIT_CMD | SDHCI_CMD_INHIBIT_DATA;
@@ -863,7 +863,7 @@ int sdhci_wait_idle_data(struct sdhci *host, struct mci_cmd *cmd)
 		mask &= ~SDHCI_CMD_INHIBIT_DATA;
 
 	if (cmd && cmd->busy_timeout != 0)
-		timeout_ns = cmd->busy_timeout;
+		timeout_ns = ms_to_ktime(cmd->busy_timeout);
 
 	ret = wait_on_timeout(timeout_ns,
 			!(sdhci_read32(host, SDHCI_PRESENT_STATE) & mask));
-- 
2.39.5




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH master 1/2] mci: imx-esdhc: restore longer timeouts for idle
  2025-07-02 10:59 [PATCH master 1/2] mci: imx-esdhc: restore longer timeouts for idle Ahmad Fatoum
  2025-07-02 10:59 ` [PATCH master 2/2] mci: sdhci: fix too short timeout in sdhci_wait_idle_data Ahmad Fatoum
@ 2025-07-02 12:27 ` Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Sascha Hauer @ 2025-07-02 12:27 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum


On Wed, 02 Jul 2025 12:59:46 +0200, Ahmad Fatoum wrote:
> Commit 02986964885c ("mci: imx-esdhc: implement esdhc_poll using
> sdhci_read32_poll_timeout") introduces sdhci_compute_timeout() which
> takes a default timeout, but that default timeout is ignored in the
> function.
> 
> Commit bbecd0b7bb7e ("mci: sdhci: add support for struct mci_data::timeout_ns")
> then makes use of this function and with this breaks the polling for DATA0
> line by reducing the original 2.5s timeout to SDHCI_CMD_DEFAULT_BUSY_TIMEOUT_NS
> which is 10ms.
> 
> [...]

Applied, thanks!

[1/2] mci: imx-esdhc: restore longer timeouts for idle
      https://git.pengutronix.de/cgit/barebox/commit/?id=e44d8628b582 (link may not be stable)
[2/2] mci: sdhci: fix too short timeout in sdhci_wait_idle_data
      https://git.pengutronix.de/cgit/barebox/commit/?id=848a56aa1559 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-02 12:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-02 10:59 [PATCH master 1/2] mci: imx-esdhc: restore longer timeouts for idle Ahmad Fatoum
2025-07-02 10:59 ` [PATCH master 2/2] mci: sdhci: fix too short timeout in sdhci_wait_idle_data Ahmad Fatoum
2025-07-02 12:27 ` [PATCH master 1/2] mci: imx-esdhc: restore longer timeouts for idle Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox