From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bo Shen Date: Thu, 14 Nov 2013 14:40:19 +0800 Subject: [U-Boot] [PATCH v3 5/6] arm: atmel: add ddr2 initialization function In-Reply-To: <5283788D.60508@gmail.com> References: <1383715757-20170-1-git-send-email-voice.shen@atmel.com> <1383715757-20170-6-git-send-email-voice.shen@atmel.com> <5283788D.60508@gmail.com> Message-ID: <52847053.7060705@atmel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Andreas, On 11/13/2013 09:03 PM, Andreas Bie?mann wrote: > Hi Bo, > > On 11/06/2013 06:29 AM, Bo Shen wrote: >> The MPDDRC supports different type of SDRAM >> This patch add ddr2 initialization function >> >> Signed-off-by: Bo Shen >> >> --- >> Changes in v3: >> - Move to at91 common folder >> >> Changes in v2: >> - None >> >> arch/arm/cpu/at91-common/Makefile | 32 +++++++ >> arch/arm/cpu/at91-common/mpddrc.c | 123 +++++++++++++++++++++++++ >> arch/arm/include/asm/arch-at91/atmel_mpddrc.h | 111 ++++++++++++++++++++++ >> spl/Makefile | 4 + >> 4 files changed, 270 insertions(+) >> create mode 100644 arch/arm/cpu/at91-common/Makefile >> create mode 100644 arch/arm/cpu/at91-common/mpddrc.c >> create mode 100644 arch/arm/include/asm/arch-at91/atmel_mpddrc.h >> >> diff --git a/arch/arm/cpu/at91-common/Makefile b/arch/arm/cpu/at91-common/Makefile >> new file mode 100644 >> index 0000000..6f1a9e6 >> --- /dev/null >> +++ b/arch/arm/cpu/at91-common/Makefile >> @@ -0,0 +1,32 @@ >> +# >> +# (C) Copyright 2000-2008 >> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de. >> +# >> +# (C) Copyright 2013 Atmel Corporation >> +# Bo Shen >> +# >> +# SPDX-License-Identifier: GPL-2.0+ >> +# >> + >> +include $(TOPDIR)/config.mk >> + >> +LIB = $(obj)libat91-common.o >> + >> +COBJS-$(CONFIG_SPL_BUILD) += mpddrc.o >> + >> +SRCS := $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) >> +OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y)) >> + >> +all: $(obj).depend $(LIB) >> + >> +$(LIB): $(OBJS) >> + $(call cmd_link_o_target, $(OBJS)) >> + >> +######################################################################### >> + >> +# defines $(obj).depend target >> +include $(SRCTREE)/rules.mk >> + >> +sinclude $(obj).depend >> + >> +######################################################################### > > please rewrite in KBuild style. OK, I will use KBuild style in next version. >> diff --git a/arch/arm/cpu/at91-common/mpddrc.c b/arch/arm/cpu/at91-common/mpddrc.c >> new file mode 100644 >> index 0000000..1abde1e >> --- /dev/null >> +++ b/arch/arm/cpu/at91-common/mpddrc.c >> @@ -0,0 +1,123 @@ >> +/* >> + * Copyright (C) 2013 Atmel Corporation >> + * Bo Shen >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include >> +#include >> +#include >> + >> +static void atmel_mpddr_op(int mode, u32 ram_address) > > static inline, could give the compiler a hint to optimize here. I am not sure whether the write(0, ram_address) will be optimized. Because this must excute later than write mode to let it work. >> +{ >> + struct atmel_mpddr *mpddr = (struct atmel_mpddr *)ATMEL_BASE_MPDDRC; >> + >> + writel(mode, &mpddr->mr); >> + writel(0, ram_address); >> +} >> + >> +int ddr2_init(u32 ram_address, struct atmel_mpddr *mpddr_value) > > could you please constify mpddr_value for the very same reason? OK. >> +{ >> + struct atmel_mpddr *mpddr = (struct atmel_mpddr *)ATMEL_BASE_MPDDRC; >> + u32 ba_off, cr; >> + >> + /* Compute bank offset according to NC in configuration register */ >> + ba_off = (mpddr_value->cr & ATMEL_MPDDRC_CR_NC_MASK) + 9; >> + if (!(mpddr_value->cr & ATMEL_MPDDRC_CR_DECOD_INTERLEAVED)) >> + ba_off += ((mpddr->cr & ATMEL_MPDDRC_CR_NR_MASK) >> 2) + 11; >> + >> + ba_off += (mpddr_value->mdr & ATMEL_MPDDRC_MDR_DBW_MASK) ? 1 : 2; >> + >> + /* Program the memory device type into the memory device register */ >> + writel(mpddr_value->mdr, &mpddr->mdr); >> + >> + /* Program the configuration register */ >> + writel(mpddr_value->cr, &mpddr->cr); >> + >> + /* Program the timing register */ >> + writel(mpddr_value->tp0r, &mpddr->tp0r); >> + writel(mpddr_value->tp1r, &mpddr->tp1r); >> + writel(mpddr_value->tp2r, &mpddr->tp2r); >> + >> + /* Issue a NOP command */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_NOP_CMD, ram_address); >> + >> + /* A 200 us is provided to precede any signal toggle */ >> + udelay(200); >> + >> + /* Issue a NOP command */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_NOP_CMD, ram_address); >> + >> + /* Issue an all banks precharge command */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_PRCGALL_CMD, ram_address); >> + >> + /* Issue an extended mode register set(EMRS2) to choose operation */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_EXT_LMR_CMD, >> + ram_address + (0x2 << ba_off)); >> + >> + /* Issue an extended mode register set(EMRS3) to set EMSR to 0 */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_EXT_LMR_CMD, >> + ram_address + (0x3 << ba_off)); >> + >> + /* >> + * Issue an extended mode register set(EMRS1) to enable DLL and >> + * program D.I.C (output driver impedance control) >> + */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_EXT_LMR_CMD, >> + ram_address + (0x1 << ba_off)); >> + >> + /* Enable DLL reset */ >> + cr = readl(&mpddr->cr); >> + writel(cr | ATMEL_MPDDRC_CR_EN_RESET_DLL, &mpddr->cr); >> + >> + /* A mode register set(MRS) cycle is issued to reset DLL */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_LMR_CMD, ram_address); >> + >> + /* Issue an all banks precharge command */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_PRCGALL_CMD, ram_address); >> + >> + /* Two auto-refresh (CBR) cycles are provided */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_RFSH_CMD, ram_address); >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_RFSH_CMD, ram_address); >> + >> + /* Disable DLL reset */ >> + cr = readl(&mpddr->cr); >> + writel(cr & (~ATMEL_MPDDRC_CR_EN_RESET_DLL), &mpddr->cr); >> + >> + /* A mode register set (MRS) cycle is issued to disable DLL reset */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_LMR_CMD, ram_address); >> + >> + /* Set OCD calibration in defautl state */ > > typo default thanks. >> + cr = readl(&mpddr->cr); >> + writel(cr | ATMEL_MPDDRC_CR_OCD_DEFAULT, &mpddr->cr); >> + >> + /* >> + * An extended mode register set (EMRS1) cycle is issued >> + * to OCD default value >> + */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_EXT_LMR_CMD, >> + ram_address + (0x1 << ba_off)); >> + >> + /* OCD calibration mode exit */ >> + cr = readl(&mpddr->cr); >> + writel(cr & (~ATMEL_MPDDRC_CR_OCD_DEFAULT), &mpddr->cr); >> + >> + /* >> + * An extended mode register set (EMRS1) cycle is issued >> + * to enable OCD exit >> + */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_EXT_LMR_CMD, >> + ram_address + (0x1 << ba_off)); >> + >> + /* A nornal mode command is provided */ >> + atmel_mpddr_op(ATMEL_MPDDRC_MR_NORMAL_CMD, ram_address); >> + >> + /* Perform a write access to any DDR2-SDRAM address */ >> + writel(0, ram_address); >> + >> + /* Write the refresh rate */ >> + writel(mpddr_value->rtr, &mpddr->rtr); >> + > > I haven't checked that sequence deeply, I trust in you that it is correct ;) > >> + return 0; >> +} >> diff --git a/arch/arm/include/asm/arch-at91/atmel_mpddrc.h b/arch/arm/include/asm/arch-at91/atmel_mpddrc.h >> new file mode 100644 >> index 0000000..abd8b68 >> --- /dev/null >> +++ b/arch/arm/include/asm/arch-at91/atmel_mpddrc.h >> @@ -0,0 +1,111 @@ >> +/* >> + * Copyright (C) 2013 Atmel Corporation >> + * Bo Shen >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#ifndef __ATMEL_MPDDRC_H__ >> +#define __ATMEL_MPDDRC_H__ >> + >> +struct atmel_mpddr { >> + u32 mr; >> + u32 rtr; >> + u32 cr; >> + u32 tp0r; > > Datasheet names them tprX Actually, this name is Timing Parameter 0 Register, Timing Parameter 1 Register. >> + u32 tp1r; >> + u32 tp2r; >> + u32 reserved[2]; >> + u32 mdr; > > Datasheet names it just md. All other registers with "r" suffix, so, add r to this register name too. > I think it would be good to also add the other register positions or at > least mention that these the only one needed currently (in a comment > right here in the struct). OK, I will add comments here. >> +}; >> + >> +int ddr2_init(unsigned int ram_address, >> + struct atmel_mpddr *mpddr); >> + >> +/* bit field in mode register */ >> +#define ATMEL_MPDDRC_MR_NORMAL_CMD 0x0 >> +#define ATMEL_MPDDRC_MR_NOP_CMD 0x1 >> +#define ATMEL_MPDDRC_MR_PRCGALL_CMD 0x2 >> +#define ATMEL_MPDDRC_MR_LMR_CMD 0x3 >> +#define ATMEL_MPDDRC_MR_RFSH_CMD 0x4 >> +#define ATMEL_MPDDRC_MR_EXT_LMR_CMD 0x5 >> +#define ATMEL_MPDDRC_MR_DEEP_CMD 0x6 >> +#define ATMEL_MPDDRC_MR_LPDDR2_CMD 0x7 > > Could you please drop the tabs between 'define' and the definition. OK >> + >> +/* bit field in configuration register */ >> +#define ATMEL_MPDDRC_CR_NC_MASK 0x3 >> +#define ATMEL_MPDDRC_CR_NC_COL_9 0x0 >> +#define ATMEL_MPDDRC_CR_NC_COL_10 0x1 >> +#define ATMEL_MPDDRC_CR_NC_COL_11 0x2 >> +#define ATMEL_MPDDRC_CR_NC_COL_12 0x3 >> +#define ATMEL_MPDDRC_CR_NR_MASK (0x3 << 2) >> +#define ATMEL_MPDDRC_CR_NR_ROW_11 (0x0 << 2) >> +#define ATMEL_MPDDRC_CR_NR_ROW_12 (0x1 << 2) >> +#define ATMEL_MPDDRC_CR_NR_ROW_13 (0x2 << 2) >> +#define ATMEL_MPDDRC_CR_NR_ROW_14 (0x3 << 2) >> +#define ATMEL_MPDDRC_CR_CAS (0x7 << 4) >> +#define ATMEL_MPDDRC_CR_CAS_2 (0x2 << 4) >> +#define ATMEL_MPDDRC_CR_CAS_3 (0x3 << 4) >> +#define ATMEL_MPDDRC_CR_CAS_4 (0x4 << 4) >> +#define ATMEL_MPDDRC_CR_CAS_5 (0x5 << 4) >> +#define ATMEL_MPDDRC_CR_CAS_6 (0x6 << 4) >> +#define ATMEL_MPDDRC_CR_EN_RESET_DLL (0x1 << 7) >> +#define ATMEL_MPDDRC_CR_DIC_DS (0x1 << 8) >> +#define ATMEL_MPDDRC_CR_DIS_DLL (0x1 << 9) >> +#define ATMEL_MPDDRC_CR_OCD_DEFAULT (0x7 << 12) >> +#define ATMEL_MPDDRC_CR_EN_ENRDM (0x1 << 17) >> +#define ATMEL_MPDDRC_CR_NB_8BANKS (0x1 << 20) >> +#define ATMEL_MPDDRC_CR_DIS_NDQS (0x1 << 21) >> +#define ATMEL_MPDDRC_CR_DECOD_INTERLEAVED (0x1 << 22) >> +#define ATMEL_MPDDRC_CR_UNAL_SUPPORTED (0x1 << 23) > > Some of the defines have the strict scheme name> with being ATMEL_MPDDRC, CR here and the > respective field names from datasheet. Some however have some more > information like UNAL_SUPPORTED or DECOD_INTERLEAVED (those two are Ok I > think, but we could discuss if we like to have the strict scheme or > leave some space). But EN_ENRDM is definitely too much ;) > Has anyone a opinion about the strict scheme? > > Regarding EN_ENRDM I think using ENRDM_ON would be better Agree. > >> + >> +/* bit field in timing parameter 0 register */ >> +#define ATMEL_MPDDRC_TP0R_TRAS_OFFSET 0 >> +#define ATMEL_MPDDRC_TP0R_TRAS_MASK 0xf >> +#define ATMEL_MPDDRC_TP0R_TRCD_OFFSET 4 >> +#define ATMEL_MPDDRC_TP0R_TRCD_MASK 0xf >> +#define ATMEL_MPDDRC_TP0R_TWR_OFFSET 8 >> +#define ATMEL_MPDDRC_TP0R_TWR_MASK 0xf >> +#define ATMEL_MPDDRC_TP0R_TRC_OFFSET 12 >> +#define ATMEL_MPDDRC_TP0R_TRC_MASK 0xf >> +#define ATMEL_MPDDRC_TP0R_TRP_OFFSET 16 >> +#define ATMEL_MPDDRC_TP0R_TRP_MASK 0xf >> +#define ATMEL_MPDDRC_TP0R_TRRD_OFFSET 20 >> +#define ATMEL_MPDDRC_TP0R_TRRD_MASK 0xf >> +#define ATMEL_MPDDRC_TP0R_TWTR_OFFSET 24 >> +#define ATMEL_MPDDRC_TP0R_TWTR_MASK 0x7 >> +#define ATMEL_MPDDRC_TP0R_RDC_WRRD_OFFSET 27 >> +#define ATMEL_MPDDRC_TP0R_RDC_WRRD_MASK 0x1 >> +#define ATMEL_MPDDRC_TP0R_TMRD_OFFSET 28 >> +#define ATMEL_MPDDRC_TP0R_TMRD_MASK 0xf >> + >> +/* bit field in timing parameter 1 register */ >> +#define ATMEL_MPDDRC_TP1R_TRFC_OFFSET 0 >> +#define ATMEL_MPDDRC_TP1R_TRFC_MASK 0x7f >> +#define ATMEL_MPDDRC_TP1R_TXSNR_OFFSET 8 >> +#define ATMEL_MPDDRC_TP1R_TXSNR_MASK 0xff >> +#define ATMEL_MPDDRC_TP1R_TXSRD_OFFSET 16 >> +#define ATMEL_MPDDRC_TP1R_TXSRD_MASK 0xff >> +#define ATMEL_MPDDRC_TP1R_TXP_OFFSET 24 >> +#define ATMEL_MPDDRC_TP1R_TXP_MASK 0xf >> + >> +/* bit field in timing parameter 2 register */ >> +#define ATMEL_MPDDRC_TP2R_TXARD_OFFSET 0 >> +#define ATMEL_MPDDRC_TP2R_TXARD_MASK 0xf >> +#define ATMEL_MPDDRC_TP2R_TXARDS_OFFSET 4 >> +#define ATMEL_MPDDRC_TP2R_TXARDS_MASK 0xf >> +#define ATMEL_MPDDRC_TP2R_TRPA_OFFSET 8 >> +#define ATMEL_MPDDRC_TP2R_TRPA_MASK 0xf >> +#define ATMEL_MPDDRC_TP2R_TRTP_OFFSET 12 >> +#define ATMEL_MPDDRC_TP2R_TRTP_MASK 0x7 >> +#define ATMEL_MPDDRC_TP2R_TFAW_OFFSET 16 >> +#define ATMEL_MPDDRC_TP2R_TFAW_MASK 0xf >> + >> +/* bit field in Memory Device Register */ >> +#define ATMEL_MPDDRC_MDR_LPDDR_SDRAM 0x3 >> +#define ATMEL_MPDDRC_MDR_DDR2_SDRAM 0x6 >> +#define ATMEL_MPDDRC_MDR_DBW_MASK (0x1 << 4) >> +#define ATMEL_MPDDRC_MDR_DBW_32BITS (0x0 << 4) >> +#define ATMEL_MPDDRC_MDR_DBW_16BITS (0x1 << 4) >> + >> +#endif >> diff --git a/spl/Makefile b/spl/Makefile >> index b366ac2..6dd1e5d 100644 >> --- a/spl/Makefile >> +++ b/spl/Makefile >> @@ -123,6 +123,10 @@ ifeq ($(SOC),exynos) >> LIBS-y += $(CPUDIR)/s5p-common/libs5p-common.o >> endif >> >> +ifeq ($(SOC),at91) >> +LIBS-y += arch/$(ARCH)/cpu/at91-common/libat91-common.o >> +endif > > The libat91-common.o should be built in any case. The mpddrc.o should > only be included for SPL build (that mentioned Heiko in another mail). OK. >> + >> # Add GCC lib >> ifeq ("$(USE_PRIVATE_LIBGCC)", "yes") >> PLATFORM_LIBGCC = $(SPLTREE)/arch/$(ARCH)/lib/libgcc.o >> > > Best regards > > Andreas Bie?mann > Best Regards, Bo Shen