Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [PATCH 1/4] Blackfin: arch patch for 2.6.18 | Date | Wed, 27 Sep 2006 13:37:52 +0200 |
| |
On Wednesday 27 September 2006 12:04, Aubrey wrote: > inline static void default_idle(void) > { > int flag; > > while (!need_resched()) { > leds_switch(LED_OFF); > local_irq_save(flag); > if ( likely(!need_resched()) { > #if defined (ANOMALY_05000244) && defined (CONFIG_BLKFIN_CACHE) > __asm__("nop; nop;\n"); > #endif > __asm__(".align 64;\n STI %0; IDLE;\n" > : %0 (flag): :"cc"); > } > local_irq_restore(flag); > leds_switch(LED_ON); > } > }====================================== > > Here, according to design, it's not possible that interrupt occurs > between "STI %0"(enable interrupt) and "IDLE". > > __asm__(".align 64; STI %0; IDLE;" : %0 (x): :"cc"); > > Robin can explain more details.
Ok, looks good now. Just a few details that don't impact the functionality:
- Always use 'static inline', not 'inline static', because of C99 - In the kernel, it's more common to use 'asm' than '__asm__'. - It should probably be 'asm volatile', since gcc might notice that the output (flag) is not used anywhere and it can therefore eliminate the asm. - Usually, I recommend using local_irq_disable() instead of local_irq_save(flags) when you know that interrupts are enabled before. It uses one less local variable, which makes it more efficient on some architectures. - I'd insert the two NOPs unconditionally here for better readability.
Arnd <>< - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |