All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.