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

[Mitchel Humpherys <mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On Thu, Oct 30 2014 at 05:00:23 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday 30 October 2014 11:41:00 Will Deacon wrote:
>> > +
>> > +#define readl_poll_timeout(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout(readl, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define readl_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout_atomic(readl, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define readb_poll_timeout(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout(readb, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define readb_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout_atomic(readb, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define readw_poll_timeout(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout(readw, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define readw_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout_atomic(readw, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define readq_poll_timeout(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout(readq, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define readq_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout_atomic(readq, addr, val, cond, delay_us, timeout_us)
>
> Sort these by size (b, w, l, q) maybe?

Sure

>
>> > +#define ioread32_poll_timeout(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout(ioread32, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define ioread32_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout_atomic(ioread32, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define ioread32b3_poll_timeout(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout(ioread32b3, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define ioread32b3_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout_atomic(ioread32b3, addr, val, cond, delay_us, timeout_us)
>
> What is ioread32b3?

Looks like it's a... typo!  It was supposed to be ioread32be.

>
>> > +#define inb_poll_timeout(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout(inb, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define inb_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout_atomic(inb, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define inb_p_poll_timeout(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout(inb_p, addr, val, cond, delay_us, timeout_us)
>> > +
>> > +#define inb_p_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
>> > +	readx_poll_timeout_atomic(inb_p, addr, val, cond, delay_us, timeout_us)
>
> I would leave out the _p variants, they are very rarely used anyway.
>
> Looking at the long list, I wonder if we should really define each variant,
> or just expect drivers to call readx_poll_timeout{,_atomic} directly and
> pass whichever accessor they want.

That sounds reasonable although I think we'd at least want to include
the readX family of functions.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project