From: Stefan Roese <sr@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot-Users] [PATCH 1/2 v2] PPC4xx: Enable Primordial Stack for 40x and Unify ECC Handling
Date: Thu, 22 May 2008 17:37:05 +0200 [thread overview]
Message-ID: <200805221737.05591.sr@denx.de> (raw)
In-Reply-To: <1211405524-22862-1-git-send-email-gerickson@nuovations.com>
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 <wd@denx.de>
> * Copyright (C) 2007 Stefan Roese <sr@denx.de>, DENX Software
> Engineering + * Copyright (c) 2008 Nuovation System Designs, LLC
> + * Grant Erickson <gerickson@nuovations.com>
> *
> * 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, ICSACRVAL at h
> + ori r1, r1, ICSACRVAL at l
> mticcr r1 /* instruction cache */
> + isync
>
> - addis r1,r0,0x0000
> - addi r1,r1,0x0000
> +#define DCSACRVAL 0x00000000
Same here.
> + lis r1, DCSACRVAL at h
> + ori r1, r1, DCSACRVAL at l
> mtdccr r1 /* data cache */
>
> addis r1,r0,CFG_INIT_RAM_ADDR at h
> @@ -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, ICSACRVAL at h
> + ori r4, r4, ICSACRVAL at l
> 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: office at denx.de
=====================================================================
prev parent reply other threads:[~2008-05-22 15:37 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-05-21 21:32 [U-Boot-Users] [PATCH 1/2 v2] PPC4xx: Enable Primordial Stack for 40x and Unify ECC Handling Grant Erickson
2008-05-22 15:37 ` Stefan Roese [this message]
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=200805221737.05591.sr@denx.de \
--to=sr@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.