On Mon, Oct 11, 2010 at 6:15 PM, Guzman Lugo, Fernando <fernando.lugo@xxxxxx> wrote: >> +#if defined(CONFIG_TIDSPBRIDGE) || defined(CONFIG_TIDSPBRIDGE_MODULE) >> + >> +static phys_addr_t omap_dsp_mempool_base; >> + >> +static void __init omap_dsp_reserve_mem(struct meminfo *mi) { >> + Â Â phys_addr_t size = CONFIG_TIDSPBRIDGE_MEMPOOL_SIZE; >> + Â Â phys_addr_t addr = ~0; >> + Â Â int i; >> + >> + Â Â if (!size) >> + Â Â Â Â Â Â return; >> + >> + Â Â for (i = mi->nr_banks - 1; i >= 0; i--) >> + Â Â Â Â Â Â if (mi->bank[i].size >= size) { >> + Â Â Â Â Â Â Â Â Â Â mi->bank[i].size -= size; >> + Â Â Â Â Â Â Â Â Â Â addr = mi->bank[i].start + mi->bank[i].size; >> + Â Â Â Â Â Â Â Â Â Â break; >> + Â Â Â Â Â Â } > > Missing {} in "for" lopp. Even tough you are checking for success inside > For loop why check again outside? And also not need to define addr. > What do you think about this: It comes directly from Russell's code: http://article.gmane.org/gmane.linux.kernel/1046606 But I do prefer to add the braces. Your proposed patch is ok, although I prefer to have the important code at the end of the function, and the error check before that. Anyway, Russell has come with a different approach, so I have to try that instead. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html