[Buildroot] [PATCH] at91bootstrap3: new package

Simon Dawson spdawson at gmail.com
Tue Jul 31 17:41:09 UTC 2012


Hi Arnout. Thanks for the detailed comments.

On 31 July 2012 15:57, Arnout Vandecappelle <arnout at mind.be> wrote:
>  First comment: in the commit message you could mention why
> this is a new package rather than a bump of at91bootstrap
> (perhaps quoting Thomas).

Okay. Will do so.

>  It's actually in board/<processor>/<board>_defconfig, no?

Yes, quite right.

>> +We have to get JUMP_ADDR consistant with what is used by u-boot
>
>
>  While your at it: spelling correction consistant -> consistent.

Okay.

>  Just for clarity, I'd put a --- and/or some whitespace between the S-o-b
> and the diff.

Okay.
>> +AT91BOOTSTRAP3_DEFCONFIG = \
>> +       $(call qstrip,$(BR2_TARGET_AT91BOOTSTRAP3_DEFCONFIG))
>> +AT91BOOTSTRAP3_CUSTOM_CONFIG_FILE = \
>> +       $(call qstrip,$(BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_CONFIG_FILE))
>
>
>  AFAICS these are used only once so defining a variable is unnecessary.

In fact, these variables are all used more than once in the makefile.
I've no objection to not using the variables though, if you think it
makes things cleaner.

>  I smell a refactoring opportunity here, because this pattern is used in
> a few bootloaders and in the kernel...

Yes, agreed. A job for another day.

Simon.



More information about the buildroot mailing list