On Tue, Jul 08, 2025 at 04:16:34PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> While read_poll_timeout() & co. were originally introduced just
> for simple I/O usage scenarios they have since been generalized to
> be useful in more cases.
>
> However the interface is very cumbersome to use in the general case.
> Attempt to make it more flexible by combining the 'op', 'var' and
> 'args' parameter into just a single 'op' that the caller can fully
> specify.
>
> For example i915 has one case where one might currently
> have to write something like:
> ret = read_poll_timeout(drm_dp_dpcd_read_byte, err,
> err || (status & mask),
> 0 * 1000, 200 * 1000, false,
> aux, DP_FEC_STATUS, &status);
> which is practically illegible, but with the adjusted macro
> we do:
> ret = poll_timeout_us(err = drm_dp_dpcd_read_byte(aux, DP_FEC_STATUS,
> &status),
> err || (status & mask),
> 0 * 1000, 200 * 1000, false);
> which much easier to understand.
>
> One could even combine the 'op' and 'cond' parameters into
> one, but that might make the caller a bit too unwieldly with
> assignments and checks being done on the same statement.
>
> This makes poll_timeout_us() closer to the i915 __wait_for()
> macro, with the main difference being that __wait_for() uses
> expenential backoff as opposed to the fixed polling interval
> used by poll_timeout_us(). Eventually we might be able to switch
> (at least most of) i915 to use poll_timeout_us().
>
> v2: Fix typos (Jani)
> Fix delay_us docs for poll_timeout_us_atomic() (Jani)
>
> Cc: Lucas De Marchi <lucas.demar...@intel.com>
> Cc: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahman...@intel.com>
> Cc: Imre Deak <imre.d...@intel.com>
> Cc: David Laight <david.laight.li...@gmail.com>
> Cc: Geert Uytterhoeven <geert+rene...@glider.be>
> Cc: Matt Wagantall <ma...@codeaurora.org>
> Cc: Dejin Zheng <zhengdej...@gmail.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: intel...@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Reviewed-by: Jani Nikula <jani.nik...@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
> include/linux/iopoll.h | 110 +++++++++++++++++++++++++++++------------
> 1 file changed, 78 insertions(+), 32 deletions(-)
Any thoughs how we should get this stuff in? Jani will need it for
some i915 stuff once he returns from vacation, so I could just push
it into drm-intel-next...
Are people OK with that, or is there a better tree that could pick
this up?
>
> diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
> index 91324c331a4b..440aca5b4b59 100644
> --- a/include/linux/iopoll.h
> +++ b/include/linux/iopoll.h
> @@ -14,41 +14,38 @@
> #include <linux/io.h>
>
> /**
> - * read_poll_timeout - Periodically poll an address until a condition is
> - * met or a timeout occurs
> - * @op: accessor function (takes @args as its arguments)
> - * @val: Variable to read the value into
> - * @cond: Break condition (usually involving @val)
> - * @sleep_us: Maximum time to sleep between reads in us (0 tight-loops).
> Please
> - * read usleep_range() function description for details and
> + * poll_timeout_us - Periodically poll and perform an operation until
> + * a condition is met or a timeout occurs
> + *
> + * @op: Operation
> + * @cond: Break condition
> + * @sleep_us: Maximum time to sleep between operations in us (0 tight-loops).
> + * Please read usleep_range() function description for details and
> * limitations.
> * @timeout_us: Timeout in us, 0 means never timeout
> - * @sleep_before_read: if it is true, sleep @sleep_us before read.
> - * @args: arguments for @op poll
> + * @sleep_before_op: if it is true, sleep @sleep_us before operation.
> *
> * When available, you'll probably want to use one of the specialized
> * macros defined below rather than this macro directly.
> *
> - * Returns: 0 on success and -ETIMEDOUT upon a timeout. In either
> - * case, the last read value at @args is stored in @val. Must not
> + * Returns: 0 on success and -ETIMEDOUT upon a timeout. Must not
> * be called from atomic context if sleep_us or timeout_us are used.
> */
> -#define read_poll_timeout(op, val, cond, sleep_us, timeout_us, \
> - sleep_before_read, args...) \
> +#define poll_timeout_us(op, cond, sleep_us, timeout_us, sleep_before_op) \
> ({ \
> u64 __timeout_us = (timeout_us); \
> unsigned long __sleep_us = (sleep_us); \
> ktime_t __timeout = ktime_add_us(ktime_get(), __timeout_us); \
> might_sleep_if((__sleep_us) != 0); \
> - if (sleep_before_read && __sleep_us) \
> + if ((sleep_before_op) && __sleep_us) \
> usleep_range((__sleep_us >> 2) + 1, __sleep_us); \
> for (;;) { \
> - (val) = op(args); \
> + op; \
> if (cond) \
> break; \
> if (__timeout_us && \
> ktime_compare(ktime_get(), __timeout) > 0) { \
> - (val) = op(args); \
> + op; \
> break; \
> } \
> if (__sleep_us) \
> @@ -59,17 +56,16 @@
> })
>
> /**
> - * read_poll_timeout_atomic - Periodically poll an address until a condition
> is
> - * met or a timeout occurs
> - * @op: accessor function (takes @args as its arguments)
> - * @val: Variable to read the value into
> - * @cond: Break condition (usually involving @val)
> - * @delay_us: Time to udelay between reads in us (0 tight-loops). Please
> - * read udelay() function description for details and
> + * poll_timeout_us_atomic - Periodically poll and perform an operation until
> + * a condition is met or a timeout occurs
> + *
> + * @op: Operation
> + * @cond: Break condition
> + * @delay_us: Time to udelay between operations in us (0 tight-loops).
> + * Please read udelay() function description for details and
> * limitations.
> * @timeout_us: Timeout in us, 0 means never timeout
> - * @delay_before_read: if it is true, delay @delay_us before read.
> - * @args: arguments for @op poll
> + * @delay_before_op: if it is true, delay @delay_us before operation.
> *
> * This macro does not rely on timekeeping. Hence it is safe to call even
> when
> * timekeeping is suspended, at the expense of an underestimation of wall
> clock
> @@ -78,27 +74,26 @@
> * When available, you'll probably want to use one of the specialized
> * macros defined below rather than this macro directly.
> *
> - * Returns: 0 on success and -ETIMEDOUT upon a timeout. In either
> - * case, the last read value at @args is stored in @val.
> + * Returns: 0 on success and -ETIMEDOUT upon a timeout.
> */
> -#define read_poll_timeout_atomic(op, val, cond, delay_us, timeout_us, \
> - delay_before_read, args...) \
> +#define poll_timeout_us_atomic(op, cond, delay_us, timeout_us, \
> + delay_before_op) \
> ({ \
> u64 __timeout_us = (timeout_us); \
> s64 __left_ns = __timeout_us * NSEC_PER_USEC; \
> unsigned long __delay_us = (delay_us); \
> u64 __delay_ns = __delay_us * NSEC_PER_USEC; \
> - if (delay_before_read && __delay_us) { \
> + if ((delay_before_op) && __delay_us) { \
> udelay(__delay_us); \
> if (__timeout_us) \
> __left_ns -= __delay_ns; \
> } \
> for (;;) { \
> - (val) = op(args); \
> + op; \
> if (cond) \
> break; \
> if (__timeout_us && __left_ns < 0) { \
> - (val) = op(args); \
> + op; \
> break; \
> } \
> if (__delay_us) { \
> @@ -113,6 +108,57 @@
> (cond) ? 0 : -ETIMEDOUT; \
> })
>
> +/**
> + * read_poll_timeout - Periodically poll an address until a condition is
> + * met or a timeout occurs
> + * @op: accessor function (takes @args as its arguments)
> + * @val: Variable to read the value into
> + * @cond: Break condition (usually involving @val)
> + * @sleep_us: Maximum time to sleep between reads in us (0 tight-loops).
> Please
> + * read usleep_range() function description for details and
> + * limitations.
> + * @timeout_us: Timeout in us, 0 means never timeout
> + * @sleep_before_read: if it is true, sleep @sleep_us before read.
> + * @args: arguments for @op poll
> + *
> + * When available, you'll probably want to use one of the specialized
> + * macros defined below rather than this macro directly.
> + *
> + * Returns: 0 on success and -ETIMEDOUT upon a timeout. In either
> + * case, the last read value at @args is stored in @val. Must not
> + * be called from atomic context if sleep_us or timeout_us are used.
> + */
> +#define read_poll_timeout(op, val, cond, sleep_us, timeout_us, \
> + sleep_before_read, args...) \
> + poll_timeout_us((val) = op(args), cond, sleep_us, timeout_us,
> sleep_before_read)
> +
> +/**
> + * read_poll_timeout_atomic - Periodically poll an address until a condition
> is
> + * met or a timeout occurs
> + * @op: accessor function (takes @args as its arguments)
> + * @val: Variable to read the value into
> + * @cond: Break condition (usually involving @val)
> + * @delay_us: Time to udelay between reads in us (0 tight-loops). Please
> + * read udelay() function description for details and
> + * limitations.
> + * @timeout_us: Timeout in us, 0 means never timeout
> + * @delay_before_read: if it is true, delay @delay_us before read.
> + * @args: arguments for @op poll
> + *
> + * This macro does not rely on timekeeping. Hence it is safe to call even
> when
> + * timekeeping is suspended, at the expense of an underestimation of wall
> clock
> + * time, which is rather minimal with a non-zero delay_us.
> + *
> + * When available, you'll probably want to use one of the specialized
> + * macros defined below rather than this macro directly.
> + *
> + * Returns: 0 on success and -ETIMEDOUT upon a timeout. In either
> + * case, the last read value at @args is stored in @val.
> + */
> +#define read_poll_timeout_atomic(op, val, cond, sleep_us, timeout_us, \
> + sleep_before_read, args...) \
> + poll_timeout_us_atomic((val) = op(args), cond, sleep_us, timeout_us,
> sleep_before_read)
> +
> /**
> * readx_poll_timeout - Periodically poll an address until a condition is
> met or a timeout occurs
> * @op: accessor function (takes @addr as its only argument)
> --
> 2.49.0
--
Ville Syrjälä
Intel
