Re: [PATCH 1/4] Blackfin: arch patch for 2.6.18

Randy Dunlap
2006-09-27T16:35:52Z

On Wed, 27 Sep 2006 12:25:17 -0400 Robin Getz wrote:

> Arnd wrote:
> >Ok, looks good now. Just a few details that don't impact the
> >functionality:
>
> [snip]
>
> What I committed (to our source) is:
>
> +++ process.c 27 Sep 2006 15:32:46 -0000 1.42
> @@ -101,10 +101,10 @@
> {
> while (!need_resched()) {
> leds_switch(LED_OFF);
> - __asm__("nop;\n\t \
> - nop;\n\t \
> - nop;\n\t \
> - idle;\n\t": : :"cc");
> + local_irq_disable();
> + if (likely( !need_resched()))
> + idle_with_irq_disabled();
> + local_irq_enable();
> leds_switch(LED_ON);
> }
> }
>
> And in system.h, this was added (because this is where all the other
> inlines is which messes with the interrupts are - and irq_flags is already
> defined here)
>
> +++ system.h 27 Sep 2006 15:32:51 -0000 1.24
>
> +#define idle_with_irq_disabled() do { \
> + __asm__ __volatile__ ( \
> + "nop; nop;\n" \
> + ".align 8;\n" \
> + "sti %0; idle;\n" \
> + ::"d" (irq_flags)); \
> +} while (0)
>
> It seems to work OK.
>
> Thanks for your help on this.
>
> I think we have been weeding through everyone's comments, and have most
> things fixed up.

except for coding style nits. E.g., the patch above:
a. uses spaces instead of tabs for indentation
b. has an extra (unwanted) space in:
> + if (likely( !need_resched()))
^

> Are there any other major issues that you can see (that have not been
> pointed out).

---
~Randy