Grant,

On Wednesday 21 May 2008, Grant Erickson wrote:
> This patch (Part 1 of 2):

Again, please find some comments below.

<snip>

> diff --git a/cpu/ppc4xx/start.S b/cpu/ppc4xx/start.S
> index a513b45..e43da7b 100644
> --- a/cpu/ppc4xx/start.S
> +++ b/cpu/ppc4xx/start.S
> @@ -3,6 +3,8 @@
>   *  Copyright (C) 1999       Magnus Damm <kieraypc01.p.y.kie.era.ericsson.se>
>   *  Copyright (C) 2000,2001,2002 Wolfgang Denk <[EMAIL PROTECTED]>
>   *  Copyright (C) 2007 Stefan Roese <[EMAIL PROTECTED]>, DENX Software
> Engineering + *  Copyright (c) 2008 Nuovation System Designs, LLC
> + *    Grant Erickson <[EMAIL PROTECTED]>
>   *
>   * See file CREDITS for list of people who contributed to this
>   * project.
> @@ -79,34 +81,100 @@
>  # if (CFG_INIT_DCACHE_CS == 0)
>  #  define PBxAP pb0ap
>  #  define PBxCR pb0cr
> +#  if (defined(CFG_EBC_PB0AP) && defined(CFG_EBC_PB0CR))
> +#   define PBxAP_VAL CFG_EBC_PB0AP
> +#   define PBxCR_VAL CFG_EBC_PB0CR
> +#  endif
>  # endif
>  # if (CFG_INIT_DCACHE_CS == 1)
>  #  define PBxAP pb1ap
>  #  define PBxCR pb1cr
> +#  if (defined(CFG_EBC_PB1AP) && defined(CFG_EBC_PB1CR))
> +#   define PBxAP_VAL CFG_EBC_PB1AP
> +#   define PBxCR_VAL CFG_EBC_PB1CR
> +#  endif
>  # endif
>  # if (CFG_INIT_DCACHE_CS == 2)
>  #  define PBxAP pb2ap
>  #  define PBxCR pb2cr
> +#  if (defined(CFG_EBC_PB2AP) && defined(CFG_EBC_PB2CR))
> +#   define PBxAP_VAL CFG_EBC_PB2AP
> +#   define PBxCR_VAL CFG_EBC_PB2CR
> +#  endif
>  # endif
>  # if (CFG_INIT_DCACHE_CS == 3)
>  #  define PBxAP pb3ap
>  #  define PBxCR pb3cr
> +#  if (defined(CFG_EBC_PB3AP) && defined(CFG_EBC_PB3CR))
> +#   define PBxAP_VAL CFG_EBC_PB3AP
> +#   define PBxCR_VAL CFG_EBC_PB3CR
> +#  endif
>  # endif
>  # if (CFG_INIT_DCACHE_CS == 4)
>  #  define PBxAP pb4ap
>  #  define PBxCR pb4cr
> +#  if (defined(CFG_EBC_PB4AP) && defined(CFG_EBC_PB4CR))
> +#   define PBxAP_VAL CFG_EBC_PB4AP
> +#   define PBxCR_VAL CFG_EBC_PB4CR
> +#  endif
>  # endif
>  # if (CFG_INIT_DCACHE_CS == 5)
>  #  define PBxAP pb5ap
>  #  define PBxCR pb5cr
> +#  if (defined(CFG_EBC_PB5AP) && defined(CFG_EBC_PB5CR))
> +#   define PBxAP_VAL CFG_EBC_PB5AP
> +#   define PBxCR_VAL CFG_EBC_PB5CR
> +#  endif
>  # endif
>  # if (CFG_INIT_DCACHE_CS == 6)
>  #  define PBxAP pb6ap
>  #  define PBxCR pb6cr
> +#  if (defined(CFG_EBC_PB6AP) && defined(CFG_EBC_PB6CR))
> +#   define PBxAP_VAL CFG_EBC_PB6AP
> +#   define PBxCR_VAL CFG_EBC_PB6CR
> +#  endif
>  # endif
>  # if (CFG_INIT_DCACHE_CS == 7)
>  #  define PBxAP pb7ap
>  #  define PBxCR pb7cr
> +#  if (defined(CFG_EBC_PB7AP) && defined(CFG_EBC_PB7CR))
> +#   define PBxAP_VAL CFG_EBC_PB7AP
> +#   define PBxCR_VAL CFG_EBC_PB7CR
> +#  endif
> +# endif
> +# ifndef PBxAP_VAL
> +#  define PBxAP_VAL  0
> +# endif
> +# ifndef PBxCR_VAL
> +#  define PBxCR_VAL  0
> +# endif
> +/*
> + * Memory Bank x (nothingness) initialization CFG_INIT_RAM_ADDR + 64 MiB
> + * used as temporary stack pointer for the primordial stack
> + */
> +# ifndef CFG_INIT_DCACHE_PBxAR
> +#  define CFG_INIT_DCACHE_PBxAR      (EBC_BXAP_BME_DISABLED                  
> | \
> +                              EBC_BXAP_TWT_ENCODE(7)                 | \
> +                              EBC_BXAP_BCE_DISABLE                   | \
> +                              EBC_BXAP_BCT_2TRANS                    | \
> +                              EBC_BXAP_CSN_ENCODE(0)                 | \
> +                              EBC_BXAP_OEN_ENCODE(0)                 | \
> +                              EBC_BXAP_WBN_ENCODE(0)                 | \
> +                              EBC_BXAP_WBF_ENCODE(0)                 | \
> +                              EBC_BXAP_TH_ENCODE(2)                  | \
> +                              EBC_BXAP_RE_DISABLED                   | \
> +                              EBC_BXAP_SOR_NONDELAYED                | \
> +                              EBC_BXAP_BEM_WRITEONLY                 | \
> +                              EBC_BXAP_PEN_DISABLED)
> +# endif /* CFG_INIT_DCACHE_PBxAR */
> +# ifndef CFG_INIT_DCACHE_PBxCR
> +#  define CFG_INIT_DCACHE_PBxCR      (EBC_BXCR_BAS_ENCODE(CFG_INIT_RAM_ADDR) 
> |
> \ +                            EBC_BXCR_BS_64MB                       | \
> +                              EBC_BXCR_BU_RW                         | \
> +                              EBC_BXCR_BW_16BIT)
> +# endif /* CFG_INIT_DCACHE_PBxCR */
> +# ifndef CFG_INIT_RAM_PATTERN
> +#  define CFG_INIT_RAM_PATTERN       0xDEADDEAD
>  # endif
>  #endif /* CFG_INIT_DCACHE_CS */
>
> @@ -840,15 +908,25 @@ _start:
>       /* make sure above stores all comlete before going on */
>       sync
>
> -     
> /*-----------------------------------------------------------------------
> */ -  /* Enable two 128MB cachable regions. */
> -     
> /*-----------------------------------------------------------------------
> */ -  addis   r1,r0,0xc000
> -     addi    r1,r1,0x0001
> +     /*---------------------------------------------------------------------
> +      * Enable two 128MB cachable instruction regions at CFG_SDRAM_BASE
> +      * and another 128MB cacheable instruction region covering NOR flash
> +      * at CFG_FLASH_BASE. Disbale all cacheable data regions.

"Disable" instead of "Disbale".

> +      *------------------------------------------------------------------ */
> +
> +#define ICSACRVAL    (PPC_128MB_SACR_VALUE(CFG_SDRAM_BASE + (  0 << 20)) | \
> +                      PPC_128MB_SACR_VALUE(CFG_SDRAM_BASE + (128 << 20)) | \
> +                      PPC_128MB_SACR_VALUE(CFG_FLASH_BASE))

I don't like these #defines within the code. Please move them to the top of 
the file.

> +     lis     r1, [EMAIL PROTECTED]
> +     ori     r1, r1, [EMAIL PROTECTED]
>       mticcr  r1                      /* instruction cache */
> +     isync
>
> -     addis   r1,r0,0x0000
> -     addi    r1,r1,0x0000
> +#define DCSACRVAL    0x00000000

Same here.

> +     lis     r1, [EMAIL PROTECTED]
> +     ori     r1, r1, [EMAIL PROTECTED]
>       mtdccr  r1                      /* data cache */
>
>       addis   r1,r0,[EMAIL PROTECTED]
> @@ -902,16 +980,25 @@ _start:
>       bl      invalidate_icache
>       bl      invalidate_dcache
>
> -     
> /*-----------------------------------------------------------------------
> */ -  /* Enable two 128MB cachable regions. */
> -     
> /*-----------------------------------------------------------------------
> */ -  lis     r4,0xc000
> -     ori     r4,r4,0x0001
> +     /*---------------------------------------------------------------------
> +      * Enable two 128MB cachable instruction regions at CFG_SDRAM_BASE
> +      * and another 128MB cacheable instruction region covering NOR flash
> +      * at CFG_FLASH_BASE. Disbale all cacheable data regions.
> +      *------------------------------------------------------------------ */
> +
> +#define ICSACRVAL    (PPC_128MB_SACR_VALUE(CFG_SDRAM_BASE + (  0 << 20)) | \
> +                      PPC_128MB_SACR_VALUE(CFG_SDRAM_BASE + (128 << 20)) | \
> +                      PPC_128MB_SACR_VALUE(CFG_FLASH_BASE))

And now it's defined twice. Please remove.

> +     lis     r4, [EMAIL PROTECTED]
> +     ori     r4, r4, [EMAIL PROTECTED]
>       mticcr  r4                      /* instruction cache */
>       isync
>
> -     lis     r4,0x0000
> -     ori     r4,r4,0x0000
> +#define DCSACRVAL    0x00000000

Here too.

Please fix and resubmit. Thanks.

Best regards,
Stefan

=====================================================================
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: [EMAIL PROTECTED]
=====================================================================

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users

Reply via email to