public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
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
=====================================================================

  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