From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Babic Date: Thu, 22 Sep 2011 16:00:38 +0200 Subject: [U-Boot] [PATCH 2/2] EfikaSB: Add preliminary EfikaSB support In-Reply-To: <201109221429.32951.marek.vasut@gmail.com> References: <1316428941-8957-1-git-send-email-marek.vasut@gmail.com> <1316428941-8957-3-git-send-email-marek.vasut@gmail.com> <4E7B0361.8000309@denx.de> <201109221429.32951.marek.vasut@gmail.com> Message-ID: <4E7B3F86.3000008@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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. 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. >>> >>> +#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. Best regards, Stefano Babic -- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de =====================================================================