On Thursday, September 22, 2011 04:00:38 PM Stefano Babic wrote: > On 09/22/2011 02:29 PM, Marek Vasut wrote: > >> This seems not necessary because CONFIG_MACH_EFIKA* is set at the build > >> time with the option in boards.cfg. With a correct boards.cfg, we cannot > >> get this error. > > > > Well once someone adds another efika, he can forget about it. And there's > > mx53 efika in the works. > > Then there will be a review for the new code. At the moment, this part > behaves as dead code.
Dead code? it's all used, I don't see your point. To me, it's more readable. Hmhm ... > > Do you mean the same board files will be used ? I am not aware about a > board having two different SOCs. Probably (I say probably, we will see > whan the patches for a new board will be sent...) we will have a > different structure, as the MX53 have different setup as the MX51. In > the same way we have now a mx51evk and mx53evk. We'll see ... I don't have the board just yet. > > >>> +#ifndef CONFIG_MACH_EFIKASB > >> > >> It is better to have the check consistent in the file. You mix #ifdef > >> CONFIG_MACH_EFIKAMX with #ifndef CONFIG_MACH_EFIKASB, that is the same. > > > > It expresses the intention much better IMO. And see above -- mx53 efika > > in the works. > > Personally I find confusing if sometimes an #ifdef is used and the next > time #ifndef with the opposite CONFIG is taken, and both part of code > are compiled at the same time. > > >> At the moment, the #ifdef seems redundant. You hard-code the efikasb > >> revision to zero, and then get_efika_rev() is always smaller as > >> EFIKAMX_BOARD_REV_12. What about to introduce a macro such as board_is() > >> to increase readability ? > > > > Yes it would, but it'd also increase code size. > > I let you decide. > > >>> +#else > >>> + gd->bd->bi_arch_number = MACH_TYPE_MX51_EFIKASB; > >>> +#endif > >>> > >>> gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100; > >> > >> Can we use the new rule to set up the MACH-ID ? You can move the #ifdef > >> inside config.h and let common code to set it. > > > > Can we do that in a subsequent patch ? > > Surely, you can add a patch to this patchset. Eventually, yes ... not today though. > > Best regards, > Stefano Babic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
