From: Stefano Babic <sbabic@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/2] EfikaSB: Add preliminary EfikaSB support
Date: Thu, 22 Sep 2011 16:00:38 +0200 [thread overview]
Message-ID: <4E7B3F86.3000008@denx.de> (raw)
In-Reply-To: <201109221429.32951.marek.vasut@gmail.com>
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
=====================================================================
next prev parent reply other threads:[~2011-09-22 14:00 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-19 10:42 [U-Boot] [PATCH 0/2] Add EfikaSB support Marek Vasut
2011-09-19 10:42 ` [U-Boot] [PATCH 1/2] EfikaMX: Add imximage config for Efika SB Marek Vasut
2011-09-19 13:19 ` Stefano Babic
2011-09-19 18:57 ` [U-Boot] [PATCH 1/2 V2] " Marek Vasut
2011-09-25 19:52 ` Marek Vasut
2011-09-26 10:53 ` Stefano Babic
2011-09-19 10:42 ` [U-Boot] [PATCH 2/2] EfikaSB: Add preliminary EfikaSB support Marek Vasut
2011-09-22 9:44 ` Stefano Babic
2011-09-22 12:29 ` Marek Vasut
2011-09-22 14:00 ` Stefano Babic [this message]
2011-09-22 15:52 ` Marek Vasut
2011-09-25 19:55 ` [U-Boot] [PATCH 2/2 V2] " Marek Vasut
2011-09-26 10:53 ` Stefano Babic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E7B3F86.3000008@denx.de \
--to=sbabic@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox