Re: [PATCH 1/2] iopoll: Introduce memory-mapped IO polling macros

[Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>]

Hi Mitch, Matt,

On Thu, Sep 11, 2014 at 07:30:43PM +0100, Mitchel Humpherys wrote:
> From: Matt Wagantall <mattw-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> 
> It is sometimes necessary to poll a memory-mapped register until its
> value satisfies some condition. Introduce a family of convenience macros
> that do this. Tight-loop and sleeping versions are provided with and
> without timeouts.

[...]

> +/**
> + * readl_poll_timeout_noirq - Periodically poll an address until a condition is met or a timeout occurs
> + * @addr: Address to poll
> + * @val: Variable to read the value into
> + * @cond: Break condition (usually involving @val)
> + * @max_reads: Maximum number of reads before giving up
> + * @time_between_us: Time to udelay() between successive reads
> + *
> + * Returns 0 on success and -ETIMEDOUT upon a timeout.
> + */
> +#define readl_poll_timeout_noirq(addr, val, cond, max_reads, time_between_us) \
> +({ \
> +	int count; \
> +	for (count = (max_reads); count > 0; count--) { \
> +		(val) = readl(addr); \
> +		if (cond) \
> +			break; \
> +		udelay(time_between_us); \
> +	} \
> +	(cond) ? 0 : -ETIMEDOUT; \
> +})

I think I'd just name this one readl_poll_timeout_atomic, then drop the
helper macros you define later on. Drivers should be able to figure out the
parameters they want pretty easily and it makes it more explicit imo.

Anyway, the rest of the patch looks fine.

Will