[U-Boot] [PATCH 0/9] mx23: Make DDR initialization stable
Prior to this series running 'memtester' utility in Linux on a mx23evk always resulted in many errors during stress testing, if the kernel is loaded via U-boot.
Running the same test and loading the kernel via FSL bootlets resulted on zero errors.
Adjust U-boot so that it can also pass the 'memtester' stress test.
After this series was applied, no more DDR errors were observed on a mx23evk.
From: Fabio Estevam fabio.estevam@freescale.com
Currently when accessing an element of mxs_pinctrl_regs structure we do:
&pinctrl_regs->hw_pinctrl_emi_ds_ctrl_set
The 'hw_pinctrl' prefix is a redundant information as we already know that we are accessing a pinctrl register.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +- arch/arm/include/asm/arch-mxs/regs-pinctrl.h | 168 +++++++++++++------------- 2 files changed, 85 insertions(+), 85 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 4950490..1c509d6 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -294,7 +294,7 @@ static void mx28_mem_init(void)
/* Set DDR2 mode */ writel(PINCTRL_EMI_DS_CTRL_DDR_MODE_DDR2, - &pinctrl_regs->hw_pinctrl_emi_ds_ctrl_set); + &pinctrl_regs->emi_ds_ctrl_set);
/* * Configure the DRAM registers diff --git a/arch/arm/include/asm/arch-mxs/regs-pinctrl.h b/arch/arm/include/asm/arch-mxs/regs-pinctrl.h index 191093b..c3a8700 100644 --- a/arch/arm/include/asm/arch-mxs/regs-pinctrl.h +++ b/arch/arm/include/asm/arch-mxs/regs-pinctrl.h @@ -30,129 +30,129 @@
#ifndef __ASSEMBLY__ struct mxs_pinctrl_regs { - mxs_reg_32(hw_pinctrl_ctrl) /* 0x0 */ + mxs_reg_32(ctrl) /* 0x0 */
uint32_t reserved1[60];
- mxs_reg_32(hw_pinctrl_muxsel0) /* 0x100 */ - mxs_reg_32(hw_pinctrl_muxsel1) /* 0x110 */ - mxs_reg_32(hw_pinctrl_muxsel2) /* 0x120 */ - mxs_reg_32(hw_pinctrl_muxsel3) /* 0x130 */ - mxs_reg_32(hw_pinctrl_muxsel4) /* 0x140 */ - mxs_reg_32(hw_pinctrl_muxsel5) /* 0x150 */ - mxs_reg_32(hw_pinctrl_muxsel6) /* 0x160 */ - mxs_reg_32(hw_pinctrl_muxsel7) /* 0x170 */ - mxs_reg_32(hw_pinctrl_muxsel8) /* 0x180 */ - mxs_reg_32(hw_pinctrl_muxsel9) /* 0x190 */ - mxs_reg_32(hw_pinctrl_muxsel10) /* 0x1a0 */ - mxs_reg_32(hw_pinctrl_muxsel11) /* 0x1b0 */ - mxs_reg_32(hw_pinctrl_muxsel12) /* 0x1c0 */ - mxs_reg_32(hw_pinctrl_muxsel13) /* 0x1d0 */ + mxs_reg_32(muxsel0) /* 0x100 */ + mxs_reg_32(muxsel1) /* 0x110 */ + mxs_reg_32(muxsel2) /* 0x120 */ + mxs_reg_32(muxsel3) /* 0x130 */ + mxs_reg_32(muxsel4) /* 0x140 */ + mxs_reg_32(muxsel5) /* 0x150 */ + mxs_reg_32(muxsel6) /* 0x160 */ + mxs_reg_32(muxsel7) /* 0x170 */ + mxs_reg_32(muxsel8) /* 0x180 */ + mxs_reg_32(muxsel9) /* 0x190 */ + mxs_reg_32(muxsel10) /* 0x1a0 */ + mxs_reg_32(muxsel11) /* 0x1b0 */ + mxs_reg_32(muxsel12) /* 0x1c0 */ + mxs_reg_32(muxsel13) /* 0x1d0 */
uint32_t reserved2[72];
- mxs_reg_32(hw_pinctrl_drive0) /* 0x300 */ - mxs_reg_32(hw_pinctrl_drive1) /* 0x310 */ - mxs_reg_32(hw_pinctrl_drive2) /* 0x320 */ - mxs_reg_32(hw_pinctrl_drive3) /* 0x330 */ - mxs_reg_32(hw_pinctrl_drive4) /* 0x340 */ - mxs_reg_32(hw_pinctrl_drive5) /* 0x350 */ - mxs_reg_32(hw_pinctrl_drive6) /* 0x360 */ - mxs_reg_32(hw_pinctrl_drive7) /* 0x370 */ - mxs_reg_32(hw_pinctrl_drive8) /* 0x380 */ - mxs_reg_32(hw_pinctrl_drive9) /* 0x390 */ - mxs_reg_32(hw_pinctrl_drive10) /* 0x3a0 */ - mxs_reg_32(hw_pinctrl_drive11) /* 0x3b0 */ - mxs_reg_32(hw_pinctrl_drive12) /* 0x3c0 */ - mxs_reg_32(hw_pinctrl_drive13) /* 0x3d0 */ - mxs_reg_32(hw_pinctrl_drive14) /* 0x3e0 */ - mxs_reg_32(hw_pinctrl_drive15) /* 0x3f0 */ - mxs_reg_32(hw_pinctrl_drive16) /* 0x400 */ - mxs_reg_32(hw_pinctrl_drive17) /* 0x410 */ - mxs_reg_32(hw_pinctrl_drive18) /* 0x420 */ - mxs_reg_32(hw_pinctrl_drive19) /* 0x430 */ + mxs_reg_32(drive0) /* 0x300 */ + mxs_reg_32(drive1) /* 0x310 */ + mxs_reg_32(drive2) /* 0x320 */ + mxs_reg_32(drive3) /* 0x330 */ + mxs_reg_32(drive4) /* 0x340 */ + mxs_reg_32(drive5) /* 0x350 */ + mxs_reg_32(drive6) /* 0x360 */ + mxs_reg_32(drive7) /* 0x370 */ + mxs_reg_32(drive8) /* 0x380 */ + mxs_reg_32(drive9) /* 0x390 */ + mxs_reg_32(drive10) /* 0x3a0 */ + mxs_reg_32(drive11) /* 0x3b0 */ + mxs_reg_32(drive12) /* 0x3c0 */ + mxs_reg_32(drive13) /* 0x3d0 */ + mxs_reg_32(drive14) /* 0x3e0 */ + mxs_reg_32(drive15) /* 0x3f0 */ + mxs_reg_32(drive16) /* 0x400 */ + mxs_reg_32(drive17) /* 0x410 */ + mxs_reg_32(drive18) /* 0x420 */ + mxs_reg_32(drive19) /* 0x430 */
uint32_t reserved3[112];
- mxs_reg_32(hw_pinctrl_pull0) /* 0x600 */ - mxs_reg_32(hw_pinctrl_pull1) /* 0x610 */ - mxs_reg_32(hw_pinctrl_pull2) /* 0x620 */ - mxs_reg_32(hw_pinctrl_pull3) /* 0x630 */ - mxs_reg_32(hw_pinctrl_pull4) /* 0x640 */ - mxs_reg_32(hw_pinctrl_pull5) /* 0x650 */ - mxs_reg_32(hw_pinctrl_pull6) /* 0x660 */ + mxs_reg_32(pull0) /* 0x600 */ + mxs_reg_32(pull1) /* 0x610 */ + mxs_reg_32(pull2) /* 0x620 */ + mxs_reg_32(pull3) /* 0x630 */ + mxs_reg_32(pull4) /* 0x640 */ + mxs_reg_32(pull5) /* 0x650 */ + mxs_reg_32(pull6) /* 0x660 */
uint32_t reserved4[36];
- mxs_reg_32(hw_pinctrl_dout0) /* 0x700 */ - mxs_reg_32(hw_pinctrl_dout1) /* 0x710 */ - mxs_reg_32(hw_pinctrl_dout2) /* 0x720 */ - mxs_reg_32(hw_pinctrl_dout3) /* 0x730 */ - mxs_reg_32(hw_pinctrl_dout4) /* 0x740 */ + mxs_reg_32(dout0) /* 0x700 */ + mxs_reg_32(dout1) /* 0x710 */ + mxs_reg_32(dout2) /* 0x720 */ + mxs_reg_32(dout3) /* 0x730 */ + mxs_reg_32(dout4) /* 0x740 */
uint32_t reserved5[108];
- mxs_reg_32(hw_pinctrl_din0) /* 0x900 */ - mxs_reg_32(hw_pinctrl_din1) /* 0x910 */ - mxs_reg_32(hw_pinctrl_din2) /* 0x920 */ - mxs_reg_32(hw_pinctrl_din3) /* 0x930 */ - mxs_reg_32(hw_pinctrl_din4) /* 0x940 */ + mxs_reg_32(din0) /* 0x900 */ + mxs_reg_32(din1) /* 0x910 */ + mxs_reg_32(din2) /* 0x920 */ + mxs_reg_32(din3) /* 0x930 */ + mxs_reg_32(din4) /* 0x940 */
uint32_t reserved6[108];
- mxs_reg_32(hw_pinctrl_doe0) /* 0xb00 */ - mxs_reg_32(hw_pinctrl_doe1) /* 0xb10 */ - mxs_reg_32(hw_pinctrl_doe2) /* 0xb20 */ - mxs_reg_32(hw_pinctrl_doe3) /* 0xb30 */ - mxs_reg_32(hw_pinctrl_doe4) /* 0xb40 */ + mxs_reg_32(doe0) /* 0xb00 */ + mxs_reg_32(doe1) /* 0xb10 */ + mxs_reg_32(doe2) /* 0xb20 */ + mxs_reg_32(doe3) /* 0xb30 */ + mxs_reg_32(doe4) /* 0xb40 */
uint32_t reserved7[300];
- mxs_reg_32(hw_pinctrl_pin2irq0) /* 0x1000 */ - mxs_reg_32(hw_pinctrl_pin2irq1) /* 0x1010 */ - mxs_reg_32(hw_pinctrl_pin2irq2) /* 0x1020 */ - mxs_reg_32(hw_pinctrl_pin2irq3) /* 0x1030 */ - mxs_reg_32(hw_pinctrl_pin2irq4) /* 0x1040 */ + mxs_reg_32(pin2irq0) /* 0x1000 */ + mxs_reg_32(pin2irq1) /* 0x1010 */ + mxs_reg_32(pin2irq2) /* 0x1020 */ + mxs_reg_32(pin2irq3) /* 0x1030 */ + mxs_reg_32(pin2irq4) /* 0x1040 */
uint32_t reserved8[44];
- mxs_reg_32(hw_pinctrl_irqen0) /* 0x1100 */ - mxs_reg_32(hw_pinctrl_irqen1) /* 0x1110 */ - mxs_reg_32(hw_pinctrl_irqen2) /* 0x1120 */ - mxs_reg_32(hw_pinctrl_irqen3) /* 0x1130 */ - mxs_reg_32(hw_pinctrl_irqen4) /* 0x1140 */ + mxs_reg_32(irqen0) /* 0x1100 */ + mxs_reg_32(irqen1) /* 0x1110 */ + mxs_reg_32(irqen2) /* 0x1120 */ + mxs_reg_32(irqen3) /* 0x1130 */ + mxs_reg_32(irqen4) /* 0x1140 */
uint32_t reserved9[44];
- mxs_reg_32(hw_pinctrl_irqlevel0) /* 0x1200 */ - mxs_reg_32(hw_pinctrl_irqlevel1) /* 0x1210 */ - mxs_reg_32(hw_pinctrl_irqlevel2) /* 0x1220 */ - mxs_reg_32(hw_pinctrl_irqlevel3) /* 0x1230 */ - mxs_reg_32(hw_pinctrl_irqlevel4) /* 0x1240 */ + mxs_reg_32(irqlevel0) /* 0x1200 */ + mxs_reg_32(irqlevel1) /* 0x1210 */ + mxs_reg_32(irqlevel2) /* 0x1220 */ + mxs_reg_32(irqlevel3) /* 0x1230 */ + mxs_reg_32(irqlevel4) /* 0x1240 */
uint32_t reserved10[44];
- mxs_reg_32(hw_pinctrl_irqpol0) /* 0x1300 */ - mxs_reg_32(hw_pinctrl_irqpol1) /* 0x1310 */ - mxs_reg_32(hw_pinctrl_irqpol2) /* 0x1320 */ - mxs_reg_32(hw_pinctrl_irqpol3) /* 0x1330 */ - mxs_reg_32(hw_pinctrl_irqpol4) /* 0x1340 */ + mxs_reg_32(irqpol0) /* 0x1300 */ + mxs_reg_32(irqpol1) /* 0x1310 */ + mxs_reg_32(irqpol2) /* 0x1320 */ + mxs_reg_32(irqpol3) /* 0x1330 */ + mxs_reg_32(irqpol4) /* 0x1340 */
uint32_t reserved11[44];
- mxs_reg_32(hw_pinctrl_irqstat0) /* 0x1400 */ - mxs_reg_32(hw_pinctrl_irqstat1) /* 0x1410 */ - mxs_reg_32(hw_pinctrl_irqstat2) /* 0x1420 */ - mxs_reg_32(hw_pinctrl_irqstat3) /* 0x1430 */ - mxs_reg_32(hw_pinctrl_irqstat4) /* 0x1440 */ + mxs_reg_32(irqstat0) /* 0x1400 */ + mxs_reg_32(irqstat1) /* 0x1410 */ + mxs_reg_32(irqstat2) /* 0x1420 */ + mxs_reg_32(irqstat3) /* 0x1430 */ + mxs_reg_32(irqstat4) /* 0x1440 */
uint32_t reserved12[380];
- mxs_reg_32(hw_pinctrl_emi_odt_ctrl) /* 0x1a40 */ + mxs_reg_32(emi_odt_ctrl) /* 0x1a40 */
uint32_t reserved13[76];
- mxs_reg_32(hw_pinctrl_emi_ds_ctrl) /* 0x1b80 */ + mxs_reg_32(emi_ds_ctrl) /* 0x1b80 */ }; #endif
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
Currently when accessing an element of mxs_pinctrl_regs structure we do:
&pinctrl_regs->hw_pinctrl_emi_ds_ctrl_set
The 'hw_pinctrl' prefix is a redundant information as we already know that we are accessing a pinctrl register.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Now this single file is inconsistent with the rest of the register header files.
Best regards, Marek Vasut
On Wed, May 1, 2013 at 8:26 PM, Marek Vasut marex@denx.de wrote:
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
Currently when accessing an element of mxs_pinctrl_regs structure we do:
&pinctrl_regs->hw_pinctrl_emi_ds_ctrl_set
The 'hw_pinctrl' prefix is a redundant information as we already know that we are accessing a pinctrl register.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Now this single file is inconsistent with the rest of the register header files.
I can change the other header files as well.
Dear Fabio Estevam,
On Wed, May 1, 2013 at 8:26 PM, Marek Vasut marex@denx.de wrote:
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
Currently when accessing an element of mxs_pinctrl_regs structure we do:
&pinctrl_regs->hw_pinctrl_emi_ds_ctrl_set
The 'hw_pinctrl' prefix is a redundant information as we already know that we are accessing a pinctrl register.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Now this single file is inconsistent with the rest of the register header files.
I can change the other header files as well.
Yes, but a separate patchset can do that. Let's keep it consistent please.
Best regards, Marek Vasut
From: Fabio Estevam fabio.estevam@freescale.com
Provide a mxs_pinctrl_regs structure for mx23.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- arch/arm/include/asm/arch-mxs/regs-pinctrl.h | 68 ++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)
diff --git a/arch/arm/include/asm/arch-mxs/regs-pinctrl.h b/arch/arm/include/asm/arch-mxs/regs-pinctrl.h index c3a8700..fe7e910 100644 --- a/arch/arm/include/asm/arch-mxs/regs-pinctrl.h +++ b/arch/arm/include/asm/arch-mxs/regs-pinctrl.h @@ -29,6 +29,7 @@ #include <asm/imx-common/regs-common.h>
#ifndef __ASSEMBLY__ +#if defined(CONFIG_MX28) struct mxs_pinctrl_regs { mxs_reg_32(ctrl) /* 0x0 */
@@ -154,6 +155,73 @@ struct mxs_pinctrl_regs {
mxs_reg_32(emi_ds_ctrl) /* 0x1b80 */ }; +#elif defined(CONFIG_MX23) +struct mxs_pinctrl_regs { + mxs_reg_32(ctrl) /* 0x0 */ + int reserved1[0x3c]; + mxs_reg_32(muxsel0) /* 0x100 */ + mxs_reg_32(muxsel1) /* 0x110 */ + mxs_reg_32(muxsel2) /* 0x120 */ + mxs_reg_32(muxsel3) /* 0x130 */ + mxs_reg_32(muxsel4) /* 0x140 */ + mxs_reg_32(muxsel5) /* 0x150 */ + mxs_reg_32(muxsel6) /* 0x160 */ + mxs_reg_32(muxsel7) /* 0x170 */ + int reserved2[0x20]; + mxs_reg_32(drive0) /* 0x200 */ + mxs_reg_32(drive1) /* 0x210 */ + mxs_reg_32(drive2) /* 0x220 */ + mxs_reg_32(drive3) /* 0x230 */ + mxs_reg_32(drive4) /* 0x240 */ + mxs_reg_32(drive5) /* 0x250 */ + mxs_reg_32(drive6) /* 0x260 */ + mxs_reg_32(drive7) /* 0x270 */ + mxs_reg_32(drive8) /* 0x280 */ + mxs_reg_32(drive9) /* 0x290 */ + mxs_reg_32(drive10) /* 0x2a0 */ + mxs_reg_32(drive11) /* 0x2b0 */ + mxs_reg_32(drive12) /* 0x2c0 */ + mxs_reg_32(drive13) /* 0x2d0 */ + mxs_reg_32(drive14) /* 0x2e0 */ + int reserved3[0x44]; + mxs_reg_32(pull0) /* 0x400 */ + mxs_reg_32(pull1) /* 0x410 */ + mxs_reg_32(pull2) /* 0x420 */ + mxs_reg_32(pull3) /* 0x430 */ + int reserved4[0x30]; + mxs_reg_32(dout0) /* 0x500 */ + mxs_reg_32(dout1) /* 0x510 */ + mxs_reg_32(dout2) /* 0x520 */ + int reserved5[0x34]; + mxs_reg_32(din0) /* 0x600 */ + mxs_reg_32(din1) /* 0x610 */ + mxs_reg_32(din2) /* 0x620 */ + int reserved6[0x34]; + mxs_reg_32(doe0) /* 0x700 */ + mxs_reg_32(doe1) /* 0x710 */ + mxs_reg_32(doe2) /* 0x720 */ + int reserved7[0x34]; + mxs_reg_32(pin2irq0) /* 0x800 */ + mxs_reg_32(pin2irq1) /* 0x810 */ + mxs_reg_32(pin2irq2) /* 0x820 */ + int reserved8[0x34]; + mxs_reg_32(irqen0) /* 0x900 */ + mxs_reg_32(irqen1) /* 0x910 */ + mxs_reg_32(irqen2) /* 0x920 */ + int reserved9[0x34]; + mxs_reg_32(irqlevel0) /* 0xa00 */ + mxs_reg_32(irqlevel1) /* 0xa10 */ + mxs_reg_32(irqlevel2) /* 0xa20 */ + int reserved10[0x34]; + mxs_reg_32(irqpol0) /* 0xb00 */ + mxs_reg_32(irqpol1) /* 0xb10 */ + mxs_reg_32(irqpol2) /* 0xb20 */ + int reserved11[0x34]; + mxs_reg_32(irqstat0) /* 0xc00 */ + mxs_reg_32(irqstat1) /* 0xc10 */ + mxs_reg_32(irqstat2) /* 0xc20 */ +}; +#endif #endif
#define PINCTRL_CTRL_SFTRST (1 << 31)
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
Provide a mxs_pinctrl_regs structure for mx23.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
So the pinctrl structure was always wrong on mx23? Otavio?
Best regards, Marek Vasut
On Wed, May 1, 2013 at 8:27 PM, Marek Vasut marex@denx.de wrote:
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
Provide a mxs_pinctrl_regs structure for mx23.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
So the pinctrl structure was always wrong on mx23? Otavio?
pinctrl structure was never used on mx23 in U-boot.
Dear Fabio Estevam,
On Wed, May 1, 2013 at 8:27 PM, Marek Vasut marex@denx.de wrote:
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
Provide a mxs_pinctrl_regs structure for mx23.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
So the pinctrl structure was always wrong on mx23? Otavio?
pinctrl structure was never used on mx23 in U-boot.
How come? Doesn't the board/...mx23.../spl_boot.c use that? Or iomux.c in FSL's parlance.
Best regards, Marek Vasut
On Wed, May 1, 2013 at 9:13 PM, Marek Vasut marex@denx.de wrote:
How come? Doesn't the board/...mx23.../spl_boot.c use that? Or iomux.c in FSL's parlance.
I meant mxs_pinctrl_regs structure is only referenced for mx28 currently:
#ifdef CONFIG_MX28 static void mx28_mem_init(void) { struct mxs_pinctrl_regs *pinctrl_regs = (struct mxs_pinctrl_regs *)MXS_PINCTRL_BASE;
/* Set DDR2 mode */ writel(PINCTRL_EMI_DS_CTRL_DDR_MODE_DDR2, &pinctrl_regs->hw_pinctrl_emi_ds_ctrl_set);
Dear Fabio Estevam,
On Wed, May 1, 2013 at 9:13 PM, Marek Vasut marex@denx.de wrote:
How come? Doesn't the board/...mx23.../spl_boot.c use that? Or iomux.c in FSL's parlance.
I meant mxs_pinctrl_regs structure is only referenced for mx28 currently:
#ifdef CONFIG_MX28 static void mx28_mem_init(void) { struct mxs_pinctrl_regs *pinctrl_regs = (struct mxs_pinctrl_regs *)MXS_PINCTRL_BASE;
/* Set DDR2 mode */ writel(PINCTRL_EMI_DS_CTRL_DDR_MODE_DDR2, &pinctrl_regs->hw_pinctrl_emi_ds_ctrl_set);
Ah! Good point, sorry for the prodding then.
Best regards, Marek Vasut
From: Fabio Estevam fabio.estevam@freescale.com
Registers HW_PINCTRL_DRIVE9, HW_PINCTRL_DRIVE10, HW_PINCTRL_DRIVE11, HW_PINCTRL_DRIVE12, HW_PINCTRL_DRIVE13 and HW_PINCTRL_DRIVE14 control the drive strength and the voltage selection for the DDR pins.
The reset values of the voltage selection pins are '1', which is marked as 'reserved' in the mx23 reference manual.
Clear these bits for proper operation of DDR.
Also change MUX_CONFIG_EMI, which results in better stability and match the bootlets code from Freescale.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx23evk/spl_boot.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/board/freescale/mx23evk/spl_boot.c b/board/freescale/mx23evk/spl_boot.c index b6f4e7e..035a29c 100644 --- a/board/freescale/mx23evk/spl_boot.c +++ b/board/freescale/mx23evk/spl_boot.c @@ -26,7 +26,7 @@ #include <asm/arch/sys_proto.h>
#define MUX_CONFIG_SSP1 (MXS_PAD_3V3 | MXS_PAD_8MA | MXS_PAD_PULLUP) -#define MUX_CONFIG_EMI (MXS_PAD_3V3 | MXS_PAD_16MA | MXS_PAD_PULLUP) +#define MUX_CONFIG_EMI MXS_PAD_12MA
const iomux_cfg_t iomux_setup[] = { /* DUART */ @@ -110,5 +110,15 @@ void mxs_adjust_memory_params(uint32_t *dram_vals)
void board_init_ll(void) { + struct mxs_pinctrl_regs *pinctrl = + (struct mxs_pinctrl_regs *)MXS_PINCTRL_BASE; + /* Clear the voltage bits for EMI pins as the reset value is invalid */ + writel(0, &pinctrl->drive9); + writel(0, &pinctrl->drive10); + writel(0, &pinctrl->drive11); + writel(0, &pinctrl->drive12); + writel(0, &pinctrl->drive13); + writel(0, &pinctrl->drive14); + writel(0, &pinctrl->drive9); mxs_common_spl_init(iomux_setup, ARRAY_SIZE(iomux_setup)); }
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
Registers HW_PINCTRL_DRIVE9, HW_PINCTRL_DRIVE10, HW_PINCTRL_DRIVE11, HW_PINCTRL_DRIVE12, HW_PINCTRL_DRIVE13 and HW_PINCTRL_DRIVE14 control the drive strength and the voltage selection for the DDR pins.
The reset values of the voltage selection pins are '1', which is marked as 'reserved' in the mx23 reference manual.
Clear these bits for proper operation of DDR.
Also change MUX_CONFIG_EMI, which results in better stability and match the bootlets code from Freescale.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Uh, should the pinctrl/iomux stuff not set these up properly?
board/freescale/mx23evk/spl_boot.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/board/freescale/mx23evk/spl_boot.c b/board/freescale/mx23evk/spl_boot.c index b6f4e7e..035a29c 100644 --- a/board/freescale/mx23evk/spl_boot.c +++ b/board/freescale/mx23evk/spl_boot.c @@ -26,7 +26,7 @@ #include <asm/arch/sys_proto.h>
#define MUX_CONFIG_SSP1 (MXS_PAD_3V3 | MXS_PAD_8MA | MXS_PAD_PULLUP) -#define MUX_CONFIG_EMI (MXS_PAD_3V3 | MXS_PAD_16MA | MXS_PAD_PULLUP) +#define MUX_CONFIG_EMI MXS_PAD_12MA
const iomux_cfg_t iomux_setup[] = { /* DUART */ @@ -110,5 +110,15 @@ void mxs_adjust_memory_params(uint32_t *dram_vals)
void board_init_ll(void) {
- struct mxs_pinctrl_regs *pinctrl =
(struct mxs_pinctrl_regs *)MXS_PINCTRL_BASE;- /* Clear the voltage bits for EMI pins as the reset value is invalid */
- writel(0, &pinctrl->drive9);
- writel(0, &pinctrl->drive10);
- writel(0, &pinctrl->drive11);
- writel(0, &pinctrl->drive12);
- writel(0, &pinctrl->drive13);
- writel(0, &pinctrl->drive14);
- writel(0, &pinctrl->drive9); mxs_common_spl_init(iomux_setup, ARRAY_SIZE(iomux_setup));
}
Best regards, Marek Vasut
On Wed, May 1, 2013 at 8:28 PM, Marek Vasut marex@denx.de wrote:
Uh, should the pinctrl/iomux stuff not set these up properly?
This is mx23 and EMI pins specific, so I think it is OK to do it in the board file.
Dear Fabio Estevam,
On Wed, May 1, 2013 at 8:28 PM, Marek Vasut marex@denx.de wrote:
Uh, should the pinctrl/iomux stuff not set these up properly?
This is mx23 and EMI pins specific, so I think it is OK to do it in the board file.
Yes, but should the iomux_setup[] configure that already?
Best regards, Marek Vasut
From: Fabio Estevam fabio.estevam@freescale.com
Registers HW_PINCTRL_DRIVE9, HW_PINCTRL_DRIVE10, HW_PINCTRL_DRIVE11, HW_PINCTRL_DRIVE12, HW_PINCTRL_DRIVE13 and HW_PINCTRL_DRIVE14 control the drive strength and the voltage selection for the DDR pins.
The reset values of the voltage selection pins are '1', which is marked as 'reserved' in the mx23 reference manual.
Clear these bits for proper operation of DDR.
Also change MUX_CONFIG_EMI, which results in better stability and match the bootlets code from Freescale.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/olimex/mx23_olinuxino/spl_boot.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/board/olimex/mx23_olinuxino/spl_boot.c b/board/olimex/mx23_olinuxino/spl_boot.c index a96c293..5a43677 100644 --- a/board/olimex/mx23_olinuxino/spl_boot.c +++ b/board/olimex/mx23_olinuxino/spl_boot.c @@ -30,7 +30,7 @@ #include <asm/arch/sys_proto.h>
#define MUX_CONFIG_EMI (MXS_PAD_3V3 | MXS_PAD_16MA | MXS_PAD_PULLUP) -#define MUX_CONFIG_SSP (MXS_PAD_3V3 | MXS_PAD_8MA | MXS_PAD_PULLUP) +#define MUX_CONFIG_SSP MXS_PAD_12MA
const iomux_cfg_t iomux_setup[] = { /* DUART */ @@ -103,5 +103,16 @@ const iomux_cfg_t iomux_setup[] = {
void board_init_ll(void) { + struct mxs_pinctrl_regs *pinctrl = + (struct mxs_pinctrl_regs *)MXS_PINCTRL_BASE; + + /* Clear the voltage bits for EMI pins as the reset value is invalid */ + writel(0, &pinctrl->drive9); + writel(0, &pinctrl->drive10); + writel(0, &pinctrl->drive11); + writel(0, &pinctrl->drive12); + writel(0, &pinctrl->drive13); + writel(0, &pinctrl->drive14); + writel(0, &pinctrl->drive9); mxs_common_spl_init(iomux_setup, ARRAY_SIZE(iomux_setup)); }
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
Registers HW_PINCTRL_DRIVE9, HW_PINCTRL_DRIVE10, HW_PINCTRL_DRIVE11, HW_PINCTRL_DRIVE12, HW_PINCTRL_DRIVE13 and HW_PINCTRL_DRIVE14 control the drive strength and the voltage selection for the DDR pins.
The reset values of the voltage selection pins are '1', which is marked as 'reserved' in the mx23 reference manual.
Clear these bits for proper operation of DDR.
Also change MUX_CONFIG_EMI, which results in better stability and match the bootlets code from Freescale.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Lost of duplicated code here.
Best regards, Marek Vasut
On Wed, May 1, 2013 at 8:28 PM, Marek Vasut marex@denx.de wrote:
Lost of duplicated code here.
What do you suggest?
Dear Fabio Estevam,
On Wed, May 1, 2013 at 8:28 PM, Marek Vasut marex@denx.de wrote:
Lost of duplicated code here.
What do you suggest?
Stuff it into common init sequence, but only if it really is necessary to wipe those registers and iomux_setup[] table doesn't do it for some reason.
But then, if iomux_setup[] table doesn't configure them, there's a deeper problem.
Best regards, Marek Vasut
On Wed, May 1, 2013 at 9:14 PM, Marek Vasut marex@denx.de wrote:
Stuff it into common init sequence, but only if it really is necessary to wipe those registers and iomux_setup[] table doesn't do it for some reason.
But then, if iomux_setup[] table doesn't configure them, there's a deeper problem.
Right, I managed to fix this as you suggested (via iomux_setup).
Will send it in v2.
Thanks for reviewing it.
From: Fabio Estevam fabio.estevam@freescale.com
Disable the internal pad keepers to match the code from FSL bootlets and provide better stability.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx23evk/spl_boot.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/board/freescale/mx23evk/spl_boot.c b/board/freescale/mx23evk/spl_boot.c index 035a29c..143aaed 100644 --- a/board/freescale/mx23evk/spl_boot.c +++ b/board/freescale/mx23evk/spl_boot.c @@ -120,5 +120,9 @@ void board_init_ll(void) writel(0, &pinctrl->drive13); writel(0, &pinctrl->drive14); writel(0, &pinctrl->drive9); + + /* Disable the internal pad keepers */ + writel(0x3ffff, &pinctrl->pull3); + mxs_common_spl_init(iomux_setup, ARRAY_SIZE(iomux_setup)); }
From: Fabio Estevam fabio.estevam@freescale.com
Disable the internal pad keepers to match the code from FSL bootlets and provide better stability.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/olimex/mx23_olinuxino/spl_boot.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/board/olimex/mx23_olinuxino/spl_boot.c b/board/olimex/mx23_olinuxino/spl_boot.c index 5a43677..65a9f41 100644 --- a/board/olimex/mx23_olinuxino/spl_boot.c +++ b/board/olimex/mx23_olinuxino/spl_boot.c @@ -114,5 +114,9 @@ void board_init_ll(void) writel(0, &pinctrl->drive13); writel(0, &pinctrl->drive14); writel(0, &pinctrl->drive9); + + /* Disable the internal pad keepers */ + writel(0x3ffff, &pinctrl->pull3); + mxs_common_spl_init(iomux_setup, ARRAY_SIZE(iomux_setup)); }
From: Fabio Estevam fabio.estevam@freescale.com
There is no need to write to DRAM_CTL8 register prior to nitialize_dram_values().
Fix a comment related to writing to DRAM_CTL8.
Also, DRAM_CTL18 register on mx23 does not contain DRAM init complete bit, so remove this setting as this is also not done by FSL bootlets code.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 1c509d6..cde883d 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -262,12 +262,9 @@ static void mx23_mem_init(void) * Configure the DRAM registers */
- /* Clear START and SREFRESH bit from DRAM_CTL8 */ - clrbits_le32(MXS_DRAM_BASE + 0x20, (1 << 16) | (1 << 8)); - initialize_dram_values();
- /* Set START bit in DRAM_CTL16 */ + /* Set START bit in DRAM_CTL8 */ setbits_le32(MXS_DRAM_BASE + 0x20, 1 << 16);
clrbits_le32(MXS_DRAM_BASE + 0x40, 1 << 17); @@ -279,10 +276,6 @@ static void mx23_mem_init(void)
setbits_le32(MXS_DRAM_BASE + 0x40, 1 << 19); setbits_le32(MXS_DRAM_BASE + 0x40, 1 << 11); - - /* Wait for bit 10 (DRAM init complete) in DRAM_CTL18 */ - while (!(readl(MXS_DRAM_BASE + 0x48) & (1 << 10))) - ; } #endif
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
There is no need to write to DRAM_CTL8 register prior to nitialize_dram_values().
Fix a comment related to writing to DRAM_CTL8.
Also, DRAM_CTL18 register on mx23 does not contain DRAM init complete bit, so remove this setting as this is also not done by FSL bootlets code.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 1c509d6..cde883d 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -262,12 +262,9 @@ static void mx23_mem_init(void) * Configure the DRAM registers */
- /* Clear START and SREFRESH bit from DRAM_CTL8 */
- clrbits_le32(MXS_DRAM_BASE + 0x20, (1 << 16) | (1 << 8));
Do you not need to stop the DRAM if it's already running? Like after a software reset?
initialize_dram_values();
- /* Set START bit in DRAM_CTL16 */
/* Set START bit in DRAM_CTL8 */ setbits_le32(MXS_DRAM_BASE + 0x20, 1 << 16);
clrbits_le32(MXS_DRAM_BASE + 0x40, 1 << 17);
@@ -279,10 +276,6 @@ static void mx23_mem_init(void)
setbits_le32(MXS_DRAM_BASE + 0x40, 1 << 19); setbits_le32(MXS_DRAM_BASE + 0x40, 1 << 11);
- /* Wait for bit 10 (DRAM init complete) in DRAM_CTL18 */
- while (!(readl(MXS_DRAM_BASE + 0x48) & (1 << 10)))
;} #endif
Best regards, Marek Vasut
On Wed, May 1, 2013 at 8:29 PM, Marek Vasut marex@denx.de wrote:
Do you not need to stop the DRAM if it's already running? Like after a software reset?
I don't think this is needed. FSL bootlets code does not do this.
Dear Fabio Estevam,
On Wed, May 1, 2013 at 8:29 PM, Marek Vasut marex@denx.de wrote:
Do you not need to stop the DRAM if it's already running? Like after a software reset?
I don't think this is needed. FSL bootlets code does not do this.
You'll end up messing with configuration registers on already-running RAM. That doesn't really make sense and I suspect it can result in some weird behavior.
Best regards, Marek Vasut
On Wed, May 1, 2013 at 9:15 PM, Marek Vasut marex@denx.de wrote:
You'll end up messing with configuration registers on already-running RAM. That
HW_DRAM_CTL08, bit 16 (START) is 0 (inactive) after a reset, so no need to turn off the RAM.
From: Fabio Estevam fabio.estevam@freescale.com
HW_DRAM_CTL27, HW_DRAM_CTL28 and HW_DRAM_CTL35 are not initialized as per FSL bootlets code.
mx23 Reference Manual mark HW_DRAM_CTL27 and HW_DRAM_CTL28 as "reserved".
HW_DRAM_CTL8 is setup as the last element.
So skip the initialization of these DRAM_CTL registers.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index cde883d..f500851 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -116,9 +116,12 @@ static void initialize_dram_values(void)
mxs_adjust_memory_params(dram_vals);
- for (i = 0; i < ARRAY_SIZE(dram_vals); i++) - writel(dram_vals[i], MXS_DRAM_BASE + (4 * i)); - + for (i = 0; i < ARRAY_SIZE(dram_vals); i++) { +#ifdef CONFIG_MX23 + if (!(i == 8 || i == 27 || i == 28 || i == 35)) +#endif + writel(dram_vals[i], MXS_DRAM_BASE + (4 * i)); + } #ifdef CONFIG_MX23 /* * Enable tRAS lockout in HW_DRAM_CTL08 ; it must be the last
Dear Fabio Estevam,
From: Fabio Estevam fabio.estevam@freescale.com
HW_DRAM_CTL27, HW_DRAM_CTL28 and HW_DRAM_CTL35 are not initialized as per FSL bootlets code.
mx23 Reference Manual mark HW_DRAM_CTL27 and HW_DRAM_CTL28 as "reserved".
HW_DRAM_CTL8 is setup as the last element.
So skip the initialization of these DRAM_CTL registers.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index cde883d..f500851 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -116,9 +116,12 @@ static void initialize_dram_values(void)
mxs_adjust_memory_params(dram_vals);
- for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
- for (i = 0; i < ARRAY_SIZE(dram_vals); i++) {
+#ifdef CONFIG_MX23
if (!(i == 8 || i == 27 || i == 28 || i == 35))
Stick a "continue" keyword here so it's contained instead of such a semi- complete "if" clause please.
+#endif
writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));- }
#ifdef CONFIG_MX23 /* * Enable tRAS lockout in HW_DRAM_CTL08 ; it must be the last
Best regards, Marek Vasut
On 01/05/2013 23:44, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
HW_DRAM_CTL27, HW_DRAM_CTL28 and HW_DRAM_CTL35 are not initialized as per FSL bootlets code.
mx23 Reference Manual mark HW_DRAM_CTL27 and HW_DRAM_CTL28 as "reserved".
HW_DRAM_CTL8 is setup as the last element.
So skip the initialization of these DRAM_CTL registers.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Hi Fabio,
arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index cde883d..f500851 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -116,9 +116,12 @@ static void initialize_dram_values(void)
mxs_adjust_memory_params(dram_vals);
- for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
- for (i = 0; i < ARRAY_SIZE(dram_vals); i++) {
+#ifdef CONFIG_MX23
if (!(i == 8 || i == 27 || i == 28 || i == 35))+#endif
I will suggest you add also a comment here telling that the registers are reserved. I know you explain this in commit message, but people look directly into the code and it will be easire to understand why you do that.
Best regards, Stefano
From: Fabio Estevam fabio.estevam@freescale.com
FSL bootlets code set the PORT_PRIORITY_ORDER field of register HW_EMI_CTRL as 0x2, which means:
PORT0231 = 0x02 Priority Order: AXI0, AHB2, AHB3, AHB1
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index f500851..68b9587 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -274,7 +274,7 @@ static void mx23_mem_init(void) early_delay(20000);
/* Adjust EMI port priority. */ - clrsetbits_le32(0x80020000, 0x1f << 16, 0x8); + clrsetbits_le32(0x80020000, 0x1f << 16, 0x2); early_delay(20000);
setbits_le32(MXS_DRAM_BASE + 0x40, 1 << 19);
participants (3)
-
Fabio Estevam -
Marek Vasut -
Stefano Babic