On Mon, 2014-10-13 at 11:30 -0500, Timur Tabi wrote:
> On Fri, Oct 10, 2014 at 1:05 PM, Scott Wood <scottwood@xxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
