[PATCH 1/3] qe-uart: modify qe-uart to adapt both powerpc and arm

[Scott Wood]

On Mon, 2014-10-13 at 11:30 -0500, Timur Tabi wrote:
> On Fri, Oct 10, 2014 at 1:05 PM, Scott Wood <scottwood at freescale.com> wrote:
> > There are many changes in here that ought to be separate patches with
> > separate justification.
> >
> > Also, some of the QE changes seem to be reasonable cleanup, but not
> > related to making the code work on ARM.
> 
> I agree with Scott.  This patch already makes significant code
> changes, so you should have one patch that just makes the
> out_be32->iowrite32be changes.  Changes to the QE library should NOT
> be in the same patch as changes to ucc_uart.c.
> 
> In addition, changes like this:
> 
> -       iprop = of_get_property(np, "port-number", NULL);
> -       if (!iprop) {
> +       ret = of_property_read_u32_index(np, "port-number", 0, &val);
> +       if (ret) {
> 
> should be changed to remove the OF dependency.  If you're going to
> replace of_get_property, replace it with device_property_read_u32(),
> to remove the OF dependency.
> 
> >> diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
> >> index dff714d..a932f99 100644
> >> --- a/arch/arm/include/asm/delay.h
> >> +++ b/arch/arm/include/asm/delay.h
> >> @@ -57,6 +57,22 @@ extern void __bad_udelay(void);
> >>                       __const_udelay((n) * UDELAY_MULT)) :            \
> >>         __udelay(n))
> >>
> >> +#define spin_event_timeout(condition, timeout, delay)                          \
> >> +({                                                                             \
> >> +     typeof(condition) __ret;                                               \
> >> +     int i = 0;                                                             \
> >> +     while (!(__ret = (condition)) && (i++ < timeout)) {                    \
> >> +             if (delay)                                                     \
> >> +                     udelay(delay);                                         \
> >> +             else                                                           \
> >> +                     cpu_relax();                                           \
> >> +             udelay(1);                                                     \
> >> +     }                                                                      \
> >
> > This will delay too long if "delay" is used.
> 
> Shouldn't ARM have a version of tb_ticks_since() by now?

There's get_cycles(), but it's not clear to me whether loops_per_jiffy
is OK to use with get_cycles() on 32-bit ARM.  Is avoiding the udelay
worth making this non-generic?

> 
> >> +     if (!__ret)                                                            \
> >> +             __ret = (condition);                                           \
> >> +     __ret;                                                                 \
> >
> > Timur, do you remember why that final "if (!__ret) __ret = (condition);"
> > is needed?
> 
>     powerpc: Fix spin_event_timeout() to be robust over context switches
> 
>     Current implementation of spin_event_timeout can be interrupted by an
>     IRQ or context switch after testing the condition, but before checking
>     the timeout.  This can cause the loop to report a timeout when the
>     condition actually became true in the middle.
> 
>     This patch adds one final check of the condition upon exit of the loop
>     if the last test of the condition was still false.

OK, so this shouldn't be needed in the udelay version, since an
interrupt shouldn't cause a significant difference in the timeout count.

-Scott