From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Mon, 31 May 2010 16:30:33 +0200 Subject: [U-Boot] [PATCH] arm/pxa: fix and cleanup of pxa_mem_setup macro v2 In-Reply-To: <20100531180249.5d50c04d@laska.campus-ws.pu.ru> References: <20100531180249.5d50c04d@laska.campus-ws.pu.ru> Message-ID: <201005311630.34126.marek.vasut@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dne Po 31. kv?tna 2010 16:02:49 Mikhail Kshevetskiy napsal(a): > WARNING: This macro do not assume the values for K0DB4, KxDB2, KxFREE, > K1RUN, K2RUN and APD bits of CONFIG_SYS_MDREFR_VAL as it was > done early on many pxa platforms. All pxa developers that plan > to use this macro should check the validity of their MDREFR > values. > > v1: > * strict following to section 6.4.10 of Intel PXA27xx Developer's Manual. > * use r7 to store CONFIG_SYS_MDREFR_VAL as r6 is used in pxa_wait_ticks. > > v2: > * rename pxa_mem_setup macro to pxa2xx_mem_setup > * setting of MDREFR[K1RUN] and MDREFR[K2RUN] bits may be optional > * skip certain configuration steps if SDRAM is not present/configured > * improve/fix comments > > Signed-off-by: Mikhail Kshevetskiy > --- > arch/arm/include/asm/arch-pxa/macro.h | 82 > ++++++++++++++++++++++----------- board/vpac270/lowlevel_init.S | > 2 +- > 2 files changed, 56 insertions(+), 28 deletions(-) > > diff --git a/arch/arm/include/asm/arch-pxa/macro.h > b/arch/arm/include/asm/arch-pxa/macro.h index 1f1759b..e2ddfe9 100644 > --- a/arch/arm/include/asm/arch-pxa/macro.h > +++ b/arch/arm/include/asm/arch-pxa/macro.h > @@ -102,11 +102,15 @@ > /* > * This macro sets up the Memory controller of the PXA2xx CPU > * > - * Clobbered regs: r3, r4, r5 > + * Clobbered regs: r3, r4, r5, r6, r7 > + * > + * See section 6.4.10 of Intel PXA2xx Processor Developer's Manual > + * > http://www.marvell.com/products/processors/applications/pxa_family/pxa_27x > _dev_man.pdf */ > -.macro pxa_mem_setup > +.macro pxa2xx_mem_setup > /* This comes handy when setting MDREFR */ > ldr r3, =MEMC_BASE > + ldr r7, =CONFIG_SYS_MDREFR_VAL Push this below. > > /* > * 1) Initialize Asynchronous static memory controller > @@ -149,51 +153,68 @@ > */ > > /* > - * Before accessing MDREFR we need a valid DRI field, so we set > - * this to power on defaults + DRI field. > + * Before accessing MDREFR we need a valid DRI field. > + * Also we must properly configure MDREFR[K0DB2] and MDREFR[K0DB4]. > + * Optionaly we can set MDREFR[KxFREE] bits. > + * So we set MDREFR to power on defaults + (DRI, K0DB2, K0DB4, KxFREE) > + * fields from the config. > + * > + * WARNING: K0DB2 and K0DB4 bits are usually set, while KxFREE bits > + * are usually unset. Not true. Why is such a warning here ? > */ > ldr r5, [r3, #MDREFR_OFFSET] > - bic r5, r5, #0x0ff > - bic r5, r5, #0xf00 /* MDREFR user config with zeroed DRI */ > - > - ldr r4, =CONFIG_SYS_MDREFR_VAL > - mov r6, r4 > - lsl r4, #20 > - lsr r4, #20 /* Get a valid DRI field */ > - > - orr r5, r5, r4 /* MDREFR user config with correct DRI */ > + ldr r4, =( 0xFFF | MDREFR_K0DB4 | MDREFR_K0DB2 | \ > + MDREFR_K0FREE | MDREFR_K1FREE | MDREFR_K2FREE ) Looks good. But then, can't you just write the full custom config here and just adjust the necessary bits? (K0RUN, SLFRSH, APD, E1PIN) ? That might make it even less complicated. > + bic r5, r5, r4 /* clear DRI, K0DB2, K0DB4, KxFREE fields */ > + and r4, r7, r4 > + orr r5, r5, r4 /* use custom DRI, K0DB2, K0DB4, KxFREE */ > > orr r5, #MDREFR_K0RUN > orr r5, #MDREFR_SLFRSH > bic r5, #MDREFR_APD > - bic r5, #MDREFR_E1PIN > + > + /* enable them later, if SDRAM is present */ > + bic r5, #( MDREFR_E1PIN | MDREFR_K1RUN | MDREFR_K2RUN | \ > + MDREFR_K1DB2 | MDREFR_K2DB2 ) > > str r5, [r3, #MDREFR_OFFSET] > - ldr r4, [r3, #MDREFR_OFFSET] > + ldr r5, [r3, #MDREFR_OFFSET] > > /* > * 5) Initialize Synchronous Static Memory (Flash/Peripherals) > */ > > - /* Initialize SXCNFG register. Assert the enable bits. > - * > - * Write SXMRS to cause an MRS command to all enabled banks of > - * synchronous static memory. Note that SXLCR need not be written > - * at this time. > + /* Initialize SXCNFG register to enable synchronous flash memory. > + * While the synchronous flash banks are being configured, the SDRAM > + * banks must be disabled and MDREFR[APD] must be de-asserted. > */ > write32rb (MEMC_BASE + SXCNFG_OFFSET), CONFIG_SYS_SXCNFG_VAL > > /* > - * 6) Initialize SDRAM > + * 6) Initialize SDRAM, > + * If SDRAM present, then MDREFR[K1RUN] and/or MDREFR[K1RUN] bits and/or K2RUN ? > + * must be set. Also we must properly configure MDREFR[K1DB2] and > + * MDREFR[K2DB2] in this case. > + * > + * WARNING: K1DB2 and K2DB2 bits are usually set if SDRAM present > */ > > +#if (CONFIG_SYS_MDREFR_VAL & (MDREFR_K1RUN | MDREFR_K2RUN)) > + and r4, r7, #( MDREFR_K1RUN | MDREFR_K2RUN | \ > + MDREFR_K1DB2 | MDREFR_K2DB2 ) > + ldr r6, [r3, #MDREFR_OFFSET] > + orr r6, r6, r4 > + str r6, [r3, #MDREFR_OFFSET] > + ldr r6, [r3, #MDREFR_OFFSET] > + > bic r6, #MDREFR_SLFRSH > str r6, [r3, #MDREFR_OFFSET] > - ldr r4, [r3, #MDREFR_OFFSET] > + ldr r6, [r3, #MDREFR_OFFSET] > > orr r6, #MDREFR_E1PIN > str r6, [r3, #MDREFR_OFFSET] > - ldr r4, [r3, #MDREFR_OFFSET] > + ldr r6, [r3, #MDREFR_OFFSET] > +#endif This can be done unconditionally. I'd like to implement memory size autodetection and if there was such a condition, it'd have to go away eventually anyway. Also, you put too much into the condition I think. > > /* > * 7) Write MDCNFG with MDCNFG:DEx deasserted (set to 0), to configure > @@ -209,6 +230,7 @@ > str r4, [r3, #MDCNFG_OFFSET] > ldr r4, [r3, #MDCNFG_OFFSET] > > +#if (CONFIG_SYS_MDREFR_VAL & (MDREFR_K1RUN | MDREFR_K2RUN)) > /* Wait for the clock to the SDRAMs to stabilize, 100..200 usec. */ > pxa_wait_ticks 0x300 > > @@ -226,7 +248,7 @@ > .endr > > /* > - * 9) Write MDCNFG with enable bits asserted (MDCNFG:DEx set to 1). > + * 9) Set custom MDCNFG[DEx] bits to enable required SDRAM partitions > */ > > ldr r5, =CONFIG_SYS_MDCNFG_VAL > @@ -238,19 +260,25 @@ > ldr r4, [r3, #MDCNFG_OFFSET] > > /* > - * 10) Write MDMRS. > + * 10) Write to MDMRS register to trigger an MRS command to > + * all enabled banks of SDRAM. For each SDRAM partition pair > + * that has one or both partitions enabled, this forces a pass > + * through the MRS state and a return to NOP. > */ > > ldr r4, =CONFIG_SYS_MDMRS_VAL > str r4, [r3, #MDMRS_OFFSET] > ldr r4, [r3, #MDMRS_OFFSET] > +#endif Remove the conditions please. > > /* > - * 11) Enable APD > + * 11) Optionaly enable auto-power-down by setting MDREFR[APD] > + * > + * WARNING: APD bit is usually set. > */ > > ldr r4, [r3, #MDREFR_OFFSET] > - and r6, r6, #MDREFR_APD > + and r6, r7, #MDREFR_APD > orr r4, r4, r6 > str r4, [r3, #MDREFR_OFFSET] > ldr r4, [r3, #MDREFR_OFFSET] > diff --git a/board/vpac270/lowlevel_init.S b/board/vpac270/lowlevel_init.S > index ec0d12c..a327ebd 100644 > --- a/board/vpac270/lowlevel_init.S > +++ b/board/vpac270/lowlevel_init.S > @@ -32,7 +32,7 @@ > lowlevel_init: > pxa_gpio_setup > pxa_wait_ticks 0x8000 > - pxa_mem_setup > + pxa2xx_mem_setup > pxa_wakeup > pxa_intr_setup > pxa_clock_setup Keep up with it, good work. Cheers