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