[PATCH 1/2] include: linux/iopoll.h: add optional PBL support

[PATCH 1/2] include: linux/iopoll.h: add optional PBL support

[Ahmad Fatoum]

While we don't have generic clocksource support in PBL, there is no
reason to not enable read_poll_timeout for use on platforms that have
their own get_time_ns.

Allow these platforms to use read_poll_timeout by defining two macros:
read_pol_is_timeout and read_poll_get_time_ns().

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/iopoll.h | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
index 9350a3d05007..36a2541eb4ce 100644
--- a/include/linux/iopoll.h
+++ b/include/linux/iopoll.h
@@ -12,6 +12,18 @@
 #include <clock.h>
 #include <pbl.h>
 
+#if IN_PROPER
+# define read_poll_get_time_ns()	get_time_ns()
+# define read_poll_is_timeout(s, t)	is_timeout(s, t)
+#else
+# ifndef read_poll_get_time_ns
+#  define read_poll_get_time_ns()	0
+# endif
+# ifndef read_poll_is_timeout
+#  define read_poll_is_timeout(s, t)	((void)s, (void)t, 0)
+# endif
+#endif
+
 /**
  * read_poll_timeout - Periodically poll an address until a condition is met or a timeout occurs
  * @op: accessor function (takes @addr as its only argument)
@@ -31,15 +43,13 @@
  */
 #define read_poll_timeout(op, val, cond, timeout_us, args...)	\
 ({ \
-	uint64_t start = 0; \
-	if (IN_PROPER && (timeout_us) != 0) \
-		start = get_time_ns(); \
+	uint64_t start = timeout_us ? read_poll_get_time_ns() : 0; \
 	for (;;) { \
 		(val) = op(args); \
 		if (cond) \
 			break; \
-		if (IN_PROPER && (timeout_us) != 0 && \
-		    is_timeout(start, ((timeout_us) * USECOND))) { \
+		if ((timeout_us) != 0 && \
+		    read_poll_is_timeout(start, ((timeout_us) * USECOND))) { \
 			(val) = op(args); \
 			break; \
 		} \
-- 
2.39.5

[PATCH 2/2] mci: imx-esdhc: implement esdhc_poll using sdhci_read32_poll_timeout

[Ahmad Fatoum]

sdhci_read32_poll_timeout addresses the same use case as esdhc_poll,
but is better, because it allows more expressive conditions than
those allowed by esdhc_match32.

In return, esdhc_poll has an implementation for PBL, which
sdhci_read32_poll_timeout lacks.

Previous commit add support for plugging custom get_time_ns() and
is_timeout() implementations when outside barebox proper, so let's make
use of that and reimplement esdhc_poll in terms of
sdhci_read32_poll_timeout.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/imx-esdhc-common.c | 60 +++++-----------------------------
 drivers/mci/imx-esdhc.c        | 15 ++++-----
 drivers/mci/imx-esdhc.h        | 32 ++++++++++++++++--
 3 files changed, 45 insertions(+), 62 deletions(-)

diff --git a/drivers/mci/imx-esdhc-common.c b/drivers/mci/imx-esdhc-common.c
index 3d938891436b..0237719a74ec 100644
--- a/drivers/mci/imx-esdhc-common.c
+++ b/drivers/mci/imx-esdhc-common.c
@@ -78,53 +78,11 @@ static int esdhc_setup_data(struct fsl_esdhc_host *host, struct mci_data *data,
 	return 0;
 }
 
-static bool esdhc_match32(struct fsl_esdhc_host *host, unsigned int off,
-			  unsigned int mask, unsigned int val)
-{
-	const unsigned int reg = sdhci_read32(&host->sdhci, off) & mask;
-
-	return reg == val;
-}
-
-#ifdef __PBL__
-/*
- * Stubs to make timeout logic below work in PBL
- */
-
-#define get_time_ns()		0
-/*
- * Use time in us (approx) as a busy counter timeout value
- */
-#define is_timeout(s, t)	((s)++ > ((t) / 1024))
-
-static void __udelay(int us)
-{
-	volatile int i;
-
-	for (i = 0; i < us * 4; i++);
-}
-
-#define udelay(n)	__udelay(n)
-#undef  dev_err
-#undef  dev_dbg
-#define dev_err(d, ...)	pr_err(__VA_ARGS__)
-#define dev_dbg(d, ...)	pr_debug(__VA_ARGS__)
-
-#endif
-
-int esdhc_poll(struct fsl_esdhc_host *host, unsigned int off,
-	       unsigned int mask, unsigned int val,
-	       uint64_t timeout)
-{
-	return wait_on_timeout(timeout,
-			       esdhc_match32(host, off, mask, val));
-}
-
 int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
 		     struct mci_data *data)
 {
 	u32	xfertyp, mixctrl, command;
-	u32	irqstat;
+	u32	val, irqstat;
 	dma_addr_t dma = SDHCI_NO_DMA;
 	int ret;
 
@@ -163,8 +121,8 @@ int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
 		      command << 16 | xfertyp);
 
 	/* Wait for the command to complete */
-	ret = esdhc_poll(host, SDHCI_INT_STATUS,
-			 SDHCI_INT_CMD_COMPLETE, SDHCI_INT_CMD_COMPLETE,
+	ret = esdhc_poll(host, SDHCI_INT_STATUS, val,
+			 val & SDHCI_INT_CMD_COMPLETE,
 			 100 * MSECOND);
 	if (ret) {
 		dev_dbg(host->dev, "timeout 1\n");
@@ -185,8 +143,8 @@ int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
 		 * Poll on DATA0 line for cmd with busy signal for
 		 * timout / 10 usec since DLA polling can be insecure.
 		 */
-		ret = esdhc_poll(host, SDHCI_PRESENT_STATE,
-				 PRSSTAT_DAT0, PRSSTAT_DAT0,
+		ret = esdhc_poll(host, SDHCI_PRESENT_STATE, val,
+				 val & PRSSTAT_DAT0,
 				 2500 * MSECOND);
 		if (ret) {
 			dev_err(host->dev, "timeout PRSSTAT_DAT0\n");
@@ -210,16 +168,16 @@ int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
 	sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, -1);
 
 	/* Wait for the bus to be idle */
-	ret = esdhc_poll(host, SDHCI_PRESENT_STATE,
-			 SDHCI_CMD_INHIBIT_CMD | SDHCI_CMD_INHIBIT_DATA, 0,
+	ret = esdhc_poll(host, SDHCI_PRESENT_STATE, val,
+			 (val & (SDHCI_CMD_INHIBIT_CMD | SDHCI_CMD_INHIBIT_DATA)) == 0,
 			 SECOND);
 	if (ret) {
 		dev_err(host->dev, "timeout 2\n");
 		return -ETIMEDOUT;
 	}
 
-	ret = esdhc_poll(host, SDHCI_PRESENT_STATE,
-			 SDHCI_DATA_LINE_ACTIVE, 0,
+	ret = esdhc_poll(host, SDHCI_PRESENT_STATE, val,
+			 (val & SDHCI_DATA_LINE_ACTIVE) == 0,
 			 100 * MSECOND);
 	if (ret) {
 		dev_err(host->dev, "timeout 3\n");
diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c
index 54309aec2f74..e2794f8eff79 100644
--- a/drivers/mci/imx-esdhc.c
+++ b/drivers/mci/imx-esdhc.c
@@ -49,7 +49,7 @@ static void set_sysctl(struct mci_host *mci, u32 clock, bool ddr)
 	int div, pre_div, ddr_pre_div = 1;
 	struct fsl_esdhc_host *host = to_fsl_esdhc(mci);
 	int sdhc_clk = clk_get_rate(host->clk);
-	u32 clk;
+	u32 val, clk;
 	unsigned long  cur_clock;
 
 	if (esdhc_is_usdhc(host) && ddr)
@@ -92,8 +92,8 @@ static void set_sysctl(struct mci_host *mci, u32 clock, bool ddr)
 	esdhc_clrsetbits32(host, SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
 			SYSCTL_CLOCK_MASK, clk);
 
-	esdhc_poll(host, SDHCI_PRESENT_STATE,
-		   PRSSTAT_SDSTB, PRSSTAT_SDSTB,
+	esdhc_poll(host, SDHCI_PRESENT_STATE, val,
+		   val & PRSSTAT_SDSTB,
 		   10 * MSECOND);
 
 	clk = SYSCTL_PEREN | SYSCTL_CKEN | SYSCTL_INITA;
@@ -101,8 +101,8 @@ static void set_sysctl(struct mci_host *mci, u32 clock, bool ddr)
 	esdhc_setbits32(host, SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
 			clk);
 
-	esdhc_poll(host, SDHCI_CLOCK_CONTROL,
-		   SYSCTL_INITA, SYSCTL_INITA,
+	esdhc_poll(host, SDHCI_CLOCK_CONTROL, val,
+		   val & SYSCTL_INITA,
 		   10 * MSECOND);
 }
 
@@ -217,9 +217,8 @@ static int esdhc_reset(struct fsl_esdhc_host *host)
 	}
 
 	/* hardware clears the bit when it is done */
-	if (esdhc_poll(host,
-		       SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
-		       SYSCTL_RSTA, 0, 100 * MSECOND)) {
+	if (esdhc_poll(host, SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
+		       val, (val & SYSCTL_RSTA) == 0, 100 * MSECOND)) {
 		dev_err(host->dev, "Reset never completed.\n");
 		return -EIO;
 	}
diff --git a/drivers/mci/imx-esdhc.h b/drivers/mci/imx-esdhc.h
index eff556f2ff79..da680353d122 100644
--- a/drivers/mci/imx-esdhc.h
+++ b/drivers/mci/imx-esdhc.h
@@ -10,6 +10,9 @@
 #include <errno.h>
 #include <asm/byteorder.h>
 #include <linux/bitfield.h>
+#include <linux/math.h>
+
+#include "sdhci.h"
 
 #define SYSCTL_INITA		0x08000000
 #define SYSCTL_TIMEOUT_MASK	0x000f0000
@@ -163,10 +166,33 @@ esdhc_setbits32(struct fsl_esdhc_host *host, unsigned int reg,
 }
 
 void esdhc_populate_sdhci(struct fsl_esdhc_host *host);
-int esdhc_poll(struct fsl_esdhc_host *host, unsigned int off,
-	       unsigned int mask, unsigned int val,
-	       uint64_t timeout);
 int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd,
 		     struct mci_data *data);
 
+#ifdef __PBL__
+#undef	read_poll_get_time_ns
+#define read_poll_get_time_ns()		0
+/* Use time in us (approx) as a busy counter timeout value */
+#undef	read_poll_is_timeout
+#define read_poll_is_timeout(s, t)	((s)++ > ((t) / 1024))
+
+static inline void __udelay(int us)
+{
+	volatile int i;
+
+	for (i = 0; i < us * 4; i++);
+}
+
+#define udelay(n)	__udelay(n)
+#undef  dev_err
+#undef  dev_dbg
+#define dev_err(d, ...)	pr_err(__VA_ARGS__)
+#define dev_dbg(d, ...)	pr_debug(__VA_ARGS__)
+
+#endif
+
+#define esdhc_poll(host, reg, val, cond, timeout_ns)	\
+	sdhci_read32_poll_timeout(&host->sdhci, reg, val, cond, \
+				  DIV_ROUND_UP(timeout_ns, 1000))
+
 #endif  /* __FSL_ESDHC_H__ */
-- 
2.39.5

[PATCH] fixup! mci: imx-esdhc: implement esdhc_poll using sdhci_read32_poll_timeout

[Ahmad Fatoum]

It's unlikely that timeout_us would expand to something inducing
breakage here, but better safe than sorry.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

---
 include/linux/iopoll.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
index 36a2541eb4ce..b259109078e4 100644
--- a/include/linux/iopoll.h
+++ b/include/linux/iopoll.h
@@ -43,7 +43,7 @@
  */
 #define read_poll_timeout(op, val, cond, timeout_us, args...)	\
 ({ \
-	uint64_t start = timeout_us ? read_poll_get_time_ns() : 0; \
+	uint64_t start = (timeout_us) ? read_poll_get_time_ns() : 0; \
 	for (;;) { \
 		(val) = op(args); \
 		if (cond) \
-- 
2.39.5

Re: (subset) [PATCH] fixup! mci: imx-esdhc: implement esdhc_poll using sdhci_read32_poll_timeout

[Sascha Hauer]

On Wed, 08 Jan 2025 12:08:21 +0100, Ahmad Fatoum wrote:
> It's unlikely that timeout_us would expand to something inducing
> breakage here, but better safe than sorry.
> 
> 

Applied, thanks!

[1/1] fixup! mci: imx-esdhc: implement esdhc_poll using sdhci_read32_poll_timeout
      https://git.pengutronix.de/cgit/barebox/commit/?id=885930ff7875 (link may not be stable)

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

Re: [PATCH 1/2] include: linux/iopoll.h: add optional PBL support

[Sascha Hauer]

On Wed, 08 Jan 2025 12:04:19 +0100, Ahmad Fatoum wrote:
> While we don't have generic clocksource support in PBL, there is no
> reason to not enable read_poll_timeout for use on platforms that have
> their own get_time_ns.
> 
> Allow these platforms to use read_poll_timeout by defining two macros:
> read_pol_is_timeout and read_poll_get_time_ns().
> 
> [...]

Applied, thanks!

[1/2] include: linux/iopoll.h: add optional PBL support
      https://git.pengutronix.de/cgit/barebox/commit/?id=e94a110c8766 (link may not be stable)
[2/2] mci: imx-esdhc: implement esdhc_poll using sdhci_read32_poll_timeout
      https://git.pengutronix.de/cgit/barebox/commit/?id=5f91d2c53b03 (link may not be stable)

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

Re: [PATCH 1/2] include: linux/iopoll.h: add optional PBL support

[Sascha Hauer]

On Wed, Jan 08, 2025 at 12:04:19PM +0100, Ahmad Fatoum wrote:
> While we don't have generic clocksource support in PBL, there is no
> reason to not enable read_poll_timeout for use on platforms that have
> their own get_time_ns.
> 
> Allow these platforms to use read_poll_timeout by defining two macros:
> read_pol_is_timeout and read_poll_get_time_ns().
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  include/linux/iopoll.h | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)

This causes some breakage in -next, see https://github.com/barebox/barebox/actions/runs/12673405620/job/35319748177

I reverted this for now.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |