[U-Boot-Users] [GIT-PULL][Blackfin] Please pull http://www.denx.de/git/u-boot-blackfin.git

[Aubrey Li]

On 3/11/07, Wolfgang Denk <wd at denx.de> wrote:
> In message
>
> You break the feature of using the O=... option on the  make  command
> line, or use the "BUILD_DIR" environment variable. Using this feature
> means  that  all  files  generated  during  this build are put into a
> different directory, so your current source directory tree  will  not
> contain any new, generated files.

Still working on it. so far I found blackfin broke this feature
itself. It doesn't impact any other boards.

>
> > As my latest commit and git push, I think copyright and coding style
> > issues should be fixed. Please let me know if there still has any
> > problems. Thanks a lot.
>
> Argh... I really don't have the time to check and clean up your code.
> You are supposed to do that.
>
> You still have coding style issues at least in the following files:
>
> Trailing white space: cpu/bf533/flush.S, cpu/bf533/init_sdram.S,
> cpu/bf533/init_sdram_bootrom_initblock.S, cpu/bf533/start.S,
> cpu/bf533/traps.c

Fixed.

>
> Indentation not by TABs: include/configs/bf533-stamp.h
>
May I ask where the indentation is not TABs?


>
> A few more comments:
>
> * I object to adding "rm -f $(obj)board/bf*/u-boot.lds" to the top
>   level Makefile. You don't know who might be using a board name of
>   "bf*".

Fixed.

>
> * cpu/bf533/cache.S: please leave labels unindented, i. e. undo things
>   like this:
>
>         -1:
>         +       1:

Fixed.

>
> * cpu/bf533/cache.S: please fix indentation of comments, i. e. undo
>   things like this:
>
>         -       /* Clear the IMC bit , All valid bits in the instruction
>         -        * cache are set to the invalid state
>         -        */
>         -       BITCLR(R7,IMC_P);
>         +/*
>         + * Clear the IMC bit , All valid bits in the instruction
>         + * cache are set to the invalid state
>         + */
>         +       BITCLR(R7, IMC_P);
>

Fixed.

> * Don't remove comments that might be useful, like here:
>
>         -       SSYNC;          /* SSYNC required before writing to IMEM_CONTROL. */
>         +       SSYNC;

Not useful, I think.

>
> * cpu/bf533/serial.c - Please undo your changes to the *  declaration
>   of  DECLARE_GLOBAL_DATA_PTR.  Leave  it  at file global scope as it
>   was, or you will run into problems with more recent  tolchains.  In
>   general, don't change things without need!

Fixed.

>
> * cpu/bf533/serial.c - we recently discussed to  introduce  a  sync()
>   macro definition - please use this generic name instead of directly
>   calling things like __builtin_bfin_ssync()

Fixed.

>
> * include/asm-blackfin/io.h - Why do  you  delete  the  (to  me  good
>   looking)  definition  of  sync()  and  replace  it  with  an  empty
>   (broken?) one?

Fixed.

>
> * include/asm-blackfin/page.h - it seems you delete the definition of
>   BUG() but continue  to use it in PAGE_BUG() ?

There is a BUG() defined in the include/common.h
>
> * include/asm-blackfin/u-boot.h - here you change only  white  space,
>   and  your  don't  make  it  better  - instead, you break formatting
>   (indentation  of  comment  on  bi_baudrate).  Please  don't  change
>   existing files in such a way.

Fixed.

>
> * lib_blackfin/board.c - don't define things like pr_debug(); please
>   use the existing debug() macro instead.,

Fixed.

>
> Please note that above list of files are usually only  examples  -  I
> mention  problems  only  when I run into them the first time; many of
> them are actually present in several files.  Please  check  all  your
> code.

Thanks for your comments.

Best Regards,
-Aubrey