* [U-Boot] [PATCH v3 1/6] arm: atmel: sama5d3: correct the ID for DBGU and PIT
2013-11-06 5:29 [U-Boot] [PATCH v3 0/6] arm: atmel: sama5d3: enable spl boot from SD card Bo Shen
@ 2013-11-06 5:29 ` Bo Shen
2013-11-13 12:01 ` Andreas Bießmann
2013-11-06 5:29 ` [U-Boot] [PATCH v3 2/6] arm: atmel: sama5d3: correct the error define of DIV Bo Shen
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Bo Shen @ 2013-11-06 5:29 UTC (permalink / raw)
To: u-boot
As the DBGU and PIT has its own ID on sama5d3 SoC, while not share
with SYS ID. So, correct them.
Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
Changes in v3:
- None
Changes in v2:
- None
arch/arm/cpu/armv7/at91/sama5d3_devices.c | 2 +-
arch/arm/cpu/armv7/at91/timer.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/at91/sama5d3_devices.c b/arch/arm/cpu/armv7/at91/sama5d3_devices.c
index 51f0a6d..ebe99d2 100644
--- a/arch/arm/cpu/armv7/at91/sama5d3_devices.c
+++ b/arch/arm/cpu/armv7/at91/sama5d3_devices.c
@@ -82,7 +82,7 @@ void at91_seriald_hw_init(void)
at91_set_a_periph(AT91_PIO_PORTB, 30, 0); /* DRXD */
/* Enable clock */
- at91_periph_clk_enable(ATMEL_ID_SYS);
+ at91_periph_clk_enable(ATMEL_ID_DBGU);
}
#if defined(CONFIG_ATMEL_SPI)
diff --git a/arch/arm/cpu/armv7/at91/timer.c b/arch/arm/cpu/armv7/at91/timer.c
index 3808aed..e3ebfe0 100644
--- a/arch/arm/cpu/armv7/at91/timer.c
+++ b/arch/arm/cpu/armv7/at91/timer.c
@@ -60,7 +60,7 @@ int timer_init(void)
at91_pit_t *pit = (at91_pit_t *)ATMEL_BASE_PIT;
/* Enable PITC Clock */
- at91_periph_clk_enable(ATMEL_ID_SYS);
+ at91_periph_clk_enable(ATMEL_ID_PIT);
/* Enable PITC */
writel(TIMER_LOAD_VAL | AT91_PIT_MR_EN , &pit->mr);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 2/6] arm: atmel: sama5d3: correct the error define of DIV
2013-11-06 5:29 [U-Boot] [PATCH v3 0/6] arm: atmel: sama5d3: enable spl boot from SD card Bo Shen
2013-11-06 5:29 ` [U-Boot] [PATCH v3 1/6] arm: atmel: sama5d3: correct the ID for DBGU and PIT Bo Shen
@ 2013-11-06 5:29 ` Bo Shen
2013-11-13 12:20 ` Andreas Bießmann
2013-11-06 5:29 ` [U-Boot] [PATCH v3 3/6] arm: atmel: sama5d3: the offset of MULA is 18 Bo Shen
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Bo Shen @ 2013-11-06 5:29 UTC (permalink / raw)
To: u-boot
Correct the error define of DIV.
Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
Changes in v3:
- None
Changes in v2:
- None
arch/arm/include/asm/arch-at91/at91_pmc.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/arch-at91/at91_pmc.h b/arch/arm/include/asm/arch-at91/at91_pmc.h
index 003920c..ed5462c 100644
--- a/arch/arm/include/asm/arch-at91/at91_pmc.h
+++ b/arch/arm/include/asm/arch-at91/at91_pmc.h
@@ -124,8 +124,8 @@ typedef struct at91_pmc {
#define AT91_PMC_MCKR_MDIV_MASK 0x00000300
#endif
-#define AT91_PMC_MCKR_PLLADIV_1 0x00001000
-#define AT91_PMC_MCKR_PLLADIV_2 0x00002000
+#define AT91_PMC_MCKR_PLLADIV_1 0x00000000
+#define AT91_PMC_MCKR_PLLADIV_2 0x00001000
#define AT91_PMC_IXR_MOSCS 0x00000001
#define AT91_PMC_IXR_LOCKA 0x00000002
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 2/6] arm: atmel: sama5d3: correct the error define of DIV
2013-11-06 5:29 ` [U-Boot] [PATCH v3 2/6] arm: atmel: sama5d3: correct the error define of DIV Bo Shen
@ 2013-11-13 12:20 ` Andreas Bießmann
2013-11-14 6:31 ` Bo Shen
0 siblings, 1 reply; 21+ messages in thread
From: Andreas Bießmann @ 2013-11-13 12:20 UTC (permalink / raw)
To: u-boot
Hi Bo,
On 11/06/2013 06:29 AM, Bo Shen wrote:
> Correct the error define of DIV.
>
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>
> ---
> Changes in v3:
> - None
>
> Changes in v2:
> - None
>
> arch/arm/include/asm/arch-at91/at91_pmc.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-at91/at91_pmc.h b/arch/arm/include/asm/arch-at91/at91_pmc.h
> index 003920c..ed5462c 100644
> --- a/arch/arm/include/asm/arch-at91/at91_pmc.h
> +++ b/arch/arm/include/asm/arch-at91/at91_pmc.h
> @@ -124,8 +124,8 @@ typedef struct at91_pmc {
> #define AT91_PMC_MCKR_MDIV_MASK 0x00000300
> #endif
>
> -#define AT91_PMC_MCKR_PLLADIV_1 0x00001000
> -#define AT91_PMC_MCKR_PLLADIV_2 0x00002000
> +#define AT91_PMC_MCKR_PLLADIV_1 0x00000000
> +#define AT91_PMC_MCKR_PLLADIV_2 0x00001000
this change will touch pm9261 board. I dunno if your change is correct
for that board. I also wonder why one should set bit position 13 in MCKR
... I can't find any source what that bit could be. However mature
documentation for sam9261 (which is the SoC on pm9261) doesn't name bit
position 12 either.
So I'm fine with your change, but please fix the pm9261 board also (use
the PLLADIV_2 define).
>
> #define AT91_PMC_IXR_MOSCS 0x00000001
> #define AT91_PMC_IXR_LOCKA 0x00000002
>
Best regards
Andreas Bie?mann
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 2/6] arm: atmel: sama5d3: correct the error define of DIV
2013-11-13 12:20 ` Andreas Bießmann
@ 2013-11-14 6:31 ` Bo Shen
0 siblings, 0 replies; 21+ messages in thread
From: Bo Shen @ 2013-11-14 6:31 UTC (permalink / raw)
To: u-boot
Hi Andreas,
On 11/13/2013 08:20 PM, Andreas Bie?mann wrote:
> Hi Bo,
>
> On 11/06/2013 06:29 AM, Bo Shen wrote:
>> Correct the error define of DIV.
>>
>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>>
>> ---
>> Changes in v3:
>> - None
>>
>> Changes in v2:
>> - None
>>
>> arch/arm/include/asm/arch-at91/at91_pmc.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-at91/at91_pmc.h b/arch/arm/include/asm/arch-at91/at91_pmc.h
>> index 003920c..ed5462c 100644
>> --- a/arch/arm/include/asm/arch-at91/at91_pmc.h
>> +++ b/arch/arm/include/asm/arch-at91/at91_pmc.h
>> @@ -124,8 +124,8 @@ typedef struct at91_pmc {
>> #define AT91_PMC_MCKR_MDIV_MASK 0x00000300
>> #endif
>>
>> -#define AT91_PMC_MCKR_PLLADIV_1 0x00001000
>> -#define AT91_PMC_MCKR_PLLADIV_2 0x00002000
>> +#define AT91_PMC_MCKR_PLLADIV_1 0x00000000
>> +#define AT91_PMC_MCKR_PLLADIV_2 0x00001000
>
> this change will touch pm9261 board. I dunno if your change is correct
> for that board. I also wonder why one should set bit position 13 in MCKR
> ... I can't find any source what that bit could be. However mature
> documentation for sam9261 (which is the SoC on pm9261) doesn't name bit
> position 12 either.
>
> So I'm fine with your change, but please fix the pm9261 board also (use
> the PLLADIV_2 define).
Ok, thanks.
>>
>> #define AT91_PMC_IXR_MOSCS 0x00000001
>> #define AT91_PMC_IXR_LOCKA 0x00000002
>>
>
> Best regards
>
> Andreas Bie?mann
>
Best Regards,
Bo Shen
^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v3 3/6] arm: atmel: sama5d3: the offset of MULA is 18
2013-11-06 5:29 [U-Boot] [PATCH v3 0/6] arm: atmel: sama5d3: enable spl boot from SD card Bo Shen
2013-11-06 5:29 ` [U-Boot] [PATCH v3 1/6] arm: atmel: sama5d3: correct the ID for DBGU and PIT Bo Shen
2013-11-06 5:29 ` [U-Boot] [PATCH v3 2/6] arm: atmel: sama5d3: correct the error define of DIV Bo Shen
@ 2013-11-06 5:29 ` Bo Shen
2013-11-13 12:23 ` Andreas Bießmann
2013-11-06 5:29 ` [U-Boot] [PATCH v3 4/6] arm: atmel: sama5d3: early enable PIO peripherals Bo Shen
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Bo Shen @ 2013-11-06 5:29 UTC (permalink / raw)
To: u-boot
The offset of MULA field in PLLA register in sama5d3 is 18,
and the length only 7 bits.
Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
Changes in v3:
- None
Changes in v2:
- None
arch/arm/include/asm/arch-at91/at91_pmc.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/include/asm/arch-at91/at91_pmc.h b/arch/arm/include/asm/arch-at91/at91_pmc.h
index ed5462c..c063213 100644
--- a/arch/arm/include/asm/arch-at91/at91_pmc.h
+++ b/arch/arm/include/asm/arch-at91/at91_pmc.h
@@ -73,7 +73,11 @@ typedef struct at91_pmc {
#define AT91_PMC_PLLXR_DIV(x) (x & 0xFF)
#define AT91_PMC_PLLXR_PLLCOUNT(x) ((x & 0x3F) << 8)
#define AT91_PMC_PLLXR_OUT(x) ((x & 0x03) << 14)
+#ifdef CONFIG_SAMA5D3
+#define AT91_PMC_PLLXR_MUL(x) ((x & 0x7F) << 18)
+#else
#define AT91_PMC_PLLXR_MUL(x) ((x & 0x7FF) << 16)
+#endif
#define AT91_PMC_PLLAR_29 0x20000000
#define AT91_PMC_PLLBR_USBDIV_1 0x00000000
#define AT91_PMC_PLLBR_USBDIV_2 0x10000000
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 4/6] arm: atmel: sama5d3: early enable PIO peripherals
2013-11-06 5:29 [U-Boot] [PATCH v3 0/6] arm: atmel: sama5d3: enable spl boot from SD card Bo Shen
` (2 preceding siblings ...)
2013-11-06 5:29 ` [U-Boot] [PATCH v3 3/6] arm: atmel: sama5d3: the offset of MULA is 18 Bo Shen
@ 2013-11-06 5:29 ` Bo Shen
2013-11-13 12:28 ` Andreas Bießmann
2013-11-06 5:29 ` [U-Boot] [PATCH v3 5/6] arm: atmel: add ddr2 initialization function Bo Shen
2013-11-06 5:29 ` [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card Bo Shen
5 siblings, 1 reply; 21+ messages in thread
From: Bo Shen @ 2013-11-06 5:29 UTC (permalink / raw)
To: u-boot
Enable the PIO peripherals early than other peripherals.
Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
Changes in v3:
- Correct the clock enable code, the ID can not OR
Changes in v2:
- None
board/atmel/sama5d3xek/sama5d3xek.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/board/atmel/sama5d3xek/sama5d3xek.c b/board/atmel/sama5d3xek/sama5d3xek.c
index b0965ef..f245f98 100644
--- a/board/atmel/sama5d3xek/sama5d3xek.c
+++ b/board/atmel/sama5d3xek/sama5d3xek.c
@@ -158,6 +158,12 @@ void lcd_show_board_info(void)
int board_early_init_f(void)
{
+ at91_periph_clk_enable(ATMEL_ID_PIOA);
+ at91_periph_clk_enable(ATMEL_ID_PIOB);
+ at91_periph_clk_enable(ATMEL_ID_PIOC);
+ at91_periph_clk_enable(ATMEL_ID_PIOD);
+ at91_periph_clk_enable(ATMEL_ID_PIOE);
+
at91_seriald_hw_init();
return 0;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 4/6] arm: atmel: sama5d3: early enable PIO peripherals
2013-11-06 5:29 ` [U-Boot] [PATCH v3 4/6] arm: atmel: sama5d3: early enable PIO peripherals Bo Shen
@ 2013-11-13 12:28 ` Andreas Bießmann
0 siblings, 0 replies; 21+ messages in thread
From: Andreas Bießmann @ 2013-11-13 12:28 UTC (permalink / raw)
To: u-boot
Hi Bo,
On 11/06/2013 06:29 AM, Bo Shen wrote:
> Enable the PIO peripherals early than other peripherals.
>
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>
> ---
> Changes in v3:
> - Correct the clock enable code, the ID can not OR
>
> Changes in v2:
> - None
>
> board/atmel/sama5d3xek/sama5d3xek.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/board/atmel/sama5d3xek/sama5d3xek.c b/board/atmel/sama5d3xek/sama5d3xek.c
> index b0965ef..f245f98 100644
> --- a/board/atmel/sama5d3xek/sama5d3xek.c
> +++ b/board/atmel/sama5d3xek/sama5d3xek.c
> @@ -158,6 +158,12 @@ void lcd_show_board_info(void)
>
> int board_early_init_f(void)
I'm still not really sure if _board_ init is the right place ... I feel
more like some _arch_ init.
But I would accept it here for getting the first SPL in at91. We need to
rework that whole arch vs. board related init anyways, so we could
change it then (and insisting on that _now_ would definitely break the
release date).
> {
> + at91_periph_clk_enable(ATMEL_ID_PIOA);
> + at91_periph_clk_enable(ATMEL_ID_PIOB);
> + at91_periph_clk_enable(ATMEL_ID_PIOC);
> + at91_periph_clk_enable(ATMEL_ID_PIOD);
> + at91_periph_clk_enable(ATMEL_ID_PIOE);
> +
> at91_seriald_hw_init();
>
> return 0;
>
To say it clear, for this patch no change requested.
Best regards
Andreas Bie?mann
^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v3 5/6] arm: atmel: add ddr2 initialization function
2013-11-06 5:29 [U-Boot] [PATCH v3 0/6] arm: atmel: sama5d3: enable spl boot from SD card Bo Shen
` (3 preceding siblings ...)
2013-11-06 5:29 ` [U-Boot] [PATCH v3 4/6] arm: atmel: sama5d3: early enable PIO peripherals Bo Shen
@ 2013-11-06 5:29 ` Bo Shen
2013-11-13 13:03 ` Andreas Bießmann
2013-11-06 5:29 ` [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card Bo Shen
5 siblings, 1 reply; 21+ messages in thread
From: Bo Shen @ 2013-11-06 5:29 UTC (permalink / raw)
To: u-boot
The MPDDRC supports different type of SDRAM
This patch add ddr2 initialization function
Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
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 <voice.shen@atmel.com>
+#
+# 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
+
+#########################################################################
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 <voice.shen@atmel.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/atmel_mpddrc.h>
+
+static void atmel_mpddr_op(int mode, u32 ram_address)
+{
+ 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)
+{
+ 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 */
+ 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);
+
+ 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 <voice.shen@atmel.com>
+ *
+ * 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;
+ u32 tp1r;
+ u32 tp2r;
+ u32 reserved[2];
+ u32 mdr;
+};
+
+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
+
+/* 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)
+
+/* 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
+
# Add GCC lib
ifeq ("$(USE_PRIVATE_LIBGCC)", "yes")
PLATFORM_LIBGCC = $(SPLTREE)/arch/$(ARCH)/lib/libgcc.o
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 5/6] arm: atmel: add ddr2 initialization function
2013-11-06 5:29 ` [U-Boot] [PATCH v3 5/6] arm: atmel: add ddr2 initialization function Bo Shen
@ 2013-11-13 13:03 ` Andreas Bießmann
2013-11-14 6:40 ` Bo Shen
0 siblings, 1 reply; 21+ messages in thread
From: Andreas Bießmann @ 2013-11-13 13:03 UTC (permalink / raw)
To: u-boot
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 <voice.shen@atmel.com>
>
> ---
> 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 <voice.shen@atmel.com>
> +#
> +# 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.
> 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 <voice.shen@atmel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/atmel_mpddrc.h>
> +
> +static void atmel_mpddr_op(int mode, u32 ram_address)
static inline, could give the compiler a hint to optimize here.
> +{
> + 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?
> +{
> + 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
> + 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 <voice.shen@atmel.com>
> + *
> + * 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
> + u32 tp1r;
> + u32 tp2r;
> + u32 reserved[2];
> + u32 mdr;
Datasheet names it just md.
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).
> +};
> +
> +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.
> +
> +/* 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 <offset><register name><field
name> with <offset> being ATMEL_MPDDRC, <register name> 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.
> +
> +/* 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).
> +
> # Add GCC lib
> ifeq ("$(USE_PRIVATE_LIBGCC)", "yes")
> PLATFORM_LIBGCC = $(SPLTREE)/arch/$(ARCH)/lib/libgcc.o
>
Best regards
Andreas Bie?mann
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 5/6] arm: atmel: add ddr2 initialization function
2013-11-13 13:03 ` Andreas Bießmann
@ 2013-11-14 6:40 ` Bo Shen
2013-11-14 7:42 ` Andreas Bießmann
0 siblings, 1 reply; 21+ messages in thread
From: Bo Shen @ 2013-11-14 6:40 UTC (permalink / raw)
To: u-boot
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 <voice.shen@atmel.com>
>>
>> ---
>> 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 <voice.shen@atmel.com>
>> +#
>> +# 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 <voice.shen@atmel.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/atmel_mpddrc.h>
>> +
>> +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 <voice.shen@atmel.com>
>> + *
>> + * 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 <offset><register name><field
> name> with <offset> being ATMEL_MPDDRC, <register name> 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
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 5/6] arm: atmel: add ddr2 initialization function
2013-11-14 6:40 ` Bo Shen
@ 2013-11-14 7:42 ` Andreas Bießmann
2013-11-14 10:16 ` Bo Shen
0 siblings, 1 reply; 21+ messages in thread
From: Andreas Bießmann @ 2013-11-14 7:42 UTC (permalink / raw)
To: u-boot
Hi Bo,
On 11/14/2013 07:40 AM, Bo Shen wrote:
> On 11/13/2013 09:03 PM, Andreas Bie?mann wrote:
>> On 11/06/2013 06:29 AM, Bo Shen wrote:
<snip>
>>> +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.
well, I don't mean reordering for optimization here.
The point is the atmel_mpddr_op will get two mostly static values (it is
not fully true cause ram_address will be calculated, but didn't knew
when writing). So when using the function we need to setup two
registers, branch (and do all the stuff involved: saving to the stack
a.s.o.) and write the register content to two different addresses.
If we instruct the compiler to inline atmel_mpddr_op() he could decide
to just take the const value given into ddr2_init to be written to the
relevant address. I think that is optimization too. It could however
increase the text segment by some bytes also, that is to be checked
(cause it would be getting worse and that would be no optimization).
>>> +{
>>> + 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.
I think ram_address should also be declared const.
<snip>
>>> +#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.
Well, I know. But my data sheet names it MPDDRC_TPR0 (the abbreviation
in table 28-13). I think we should use the names in data sheet here. I
often search the data sheet for the given name. mpddr will point to the
correct subsystem in SoC, that is good. But tp0r will find nothing. This
just consumes time when looking up the spec and is IMHO annoying.
>>> + 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.
But the name in data sheet is MPDDRC_MD ...
Best regards
Andreas Bie?mann
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 5/6] arm: atmel: add ddr2 initialization function
2013-11-14 7:42 ` Andreas Bießmann
@ 2013-11-14 10:16 ` Bo Shen
0 siblings, 0 replies; 21+ messages in thread
From: Bo Shen @ 2013-11-14 10:16 UTC (permalink / raw)
To: u-boot
Hi Andreas,
On 11/14/2013 03:42 PM, Andreas Bie?mann wrote:
> Hi Bo,
>
> On 11/14/2013 07:40 AM, Bo Shen wrote:
>> On 11/13/2013 09:03 PM, Andreas Bie?mann wrote:
>>> On 11/06/2013 06:29 AM, Bo Shen wrote:
>
> <snip>
>
>>>> +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.
>
> well, I don't mean reordering for optimization here.
> The point is the atmel_mpddr_op will get two mostly static values (it is
> not fully true cause ram_address will be calculated, but didn't knew
> when writing). So when using the function we need to setup two
> registers, branch (and do all the stuff involved: saving to the stack
> a.s.o.) and write the register content to two different addresses.
>
> If we instruct the compiler to inline atmel_mpddr_op() he could decide
> to just take the const value given into ddr2_init to be written to the
> relevant address. I think that is optimization too. It could however
> increase the text segment by some bytes also, that is to be checked
> (cause it would be getting worse and that would be no optimization).
Got you means, ok, I will test it.
>>>> +{
>>>> + 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.
>
> I think ram_address should also be declared const.
>
> <snip>
>
>>>> +#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.
>
> Well, I know. But my data sheet names it MPDDRC_TPR0 (the abbreviation
> in table 28-13). I think we should use the names in data sheet here. I
> often search the data sheet for the given name. mpddr will point to the
> correct subsystem in SoC, that is good. But tp0r will find nothing. This
> just consumes time when looking up the spec and is IMHO annoying.
>
>>>> + 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.
>
> But the name in data sheet is MPDDRC_MD ...
OK, for register name, I will following up the table.
Thanks.
> Best regards
>
> Andreas Bie?mann
>
Best Regards,
Bo Shen
^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card
2013-11-06 5:29 [U-Boot] [PATCH v3 0/6] arm: atmel: sama5d3: enable spl boot from SD card Bo Shen
` (4 preceding siblings ...)
2013-11-06 5:29 ` [U-Boot] [PATCH v3 5/6] arm: atmel: add ddr2 initialization function Bo Shen
@ 2013-11-06 5:29 ` Bo Shen
2013-11-13 13:34 ` Andreas Bießmann
5 siblings, 1 reply; 21+ messages in thread
From: Bo Shen @ 2013-11-06 5:29 UTC (permalink / raw)
To: u-boot
Enable Atmel sama5d3xek boart spl boot support, which can load u-boot
from SD card with FAT file system.
Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
Changes in v3:
- Move plla and mck configure to spl.c file
Changes in v2:
- Move spl related code to at91-common folder
arch/arm/cpu/armv7/Makefile | 2 +-
arch/arm/cpu/at91-common/Makefile | 1 +
arch/arm/cpu/at91-common/spl.c | 90 ++++++++++++++++++++++++++
arch/arm/cpu/at91-common/u-boot-spl.lds | 50 ++++++++++++++
arch/arm/include/asm/arch-at91/at91_common.h | 4 ++
arch/arm/include/asm/arch-at91/spl.h | 20 ++++++
board/atmel/sama5d3xek/sama5d3xek.c | 82 +++++++++++++++++++++++
include/configs/sama5d3xek.h | 34 ++++++++++
8 files changed, 282 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/cpu/at91-common/spl.c
create mode 100644 arch/arm/cpu/at91-common/u-boot-spl.lds
create mode 100644 arch/arm/include/asm/arch-at91/spl.h
diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
index ee4b021..aea4e8b 100644
--- a/arch/arm/cpu/armv7/Makefile
+++ b/arch/arm/cpu/armv7/Makefile
@@ -16,7 +16,7 @@ COBJS += cache_v7.o
COBJS += cpu.o
COBJS += syslib.o
-ifneq ($(CONFIG_AM43XX)$(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONFIG_MX6)$(CONFIG_TI81XX),)
+ifneq ($(CONFIG_AM43XX)$(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONFIG_MX6)$(CONFIG_TI81XX)$(CONFIG_AT91FAMILY),)
SOBJS += lowlevel_init.o
endif
diff --git a/arch/arm/cpu/at91-common/Makefile b/arch/arm/cpu/at91-common/Makefile
index 6f1a9e6..4377a0b 100644
--- a/arch/arm/cpu/at91-common/Makefile
+++ b/arch/arm/cpu/at91-common/Makefile
@@ -13,6 +13,7 @@ include $(TOPDIR)/config.mk
LIB = $(obj)libat91-common.o
COBJS-$(CONFIG_SPL_BUILD) += mpddrc.o
+COBJS-$(CONFIG_SPL_BUILD) += spl.o
SRCS := $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
diff --git a/arch/arm/cpu/at91-common/spl.c b/arch/arm/cpu/at91-common/spl.c
new file mode 100644
index 0000000..37c0cc4
--- /dev/null
+++ b/arch/arm/cpu/at91-common/spl.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright (C) 2013 Atmel Corporation
+ * Bo Shen <voice.shen@atmel.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/at91_common.h>
+#include <asm/arch/at91_pmc.h>
+#include <asm/arch/at91_wdt.h>
+#include <asm/arch/clk.h>
+#include <spl.h>
+
+static void at91_disable_wdt(void)
+{
+ struct at91_wdt *wdt = (struct at91_wdt *)ATMEL_BASE_WDT;
+
+ writel(AT91_WDT_MR_WDDIS, &wdt->mr);
+}
+
+void at91_plla_init(u32 pllar)
+{
+ struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
+
+ writel(pllar, &pmc->pllar);
+ while (!(readl(&pmc->sr) & (AT91_PMC_LOCKA | AT91_PMC_MCKRDY)))
+ ;
+}
+
+void at91_mck_init(u32 mckr)
+{
+ struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
+ u32 tmp;
+
+ tmp = readl(&pmc->mckr);
+ tmp &= ~(AT91_PMC_MCKR_PRES_MASK |
+ AT91_PMC_MCKR_MDIV_MASK |
+ AT91_PMC_MCKR_PLLADIV_2);
+ tmp |= mckr & (AT91_PMC_MCKR_PRES_MASK |
+ AT91_PMC_MCKR_MDIV_MASK |
+ AT91_PMC_MCKR_PLLADIV_2);
+ writel(tmp, &pmc->mckr);
+
+ while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY))
+ ;
+}
+
+
+u32 spl_boot_device(void)
+{
+#ifdef CONFIG_SYS_USE_MMC
+ return BOOT_DEVICE_MMC1;
+#endif
+ return BOOT_DEVICE_NONE;
+}
+
+u32 spl_boot_mode(void)
+{
+ switch (spl_boot_device()) {
+#ifdef CONFIG_SYS_USE_MMC
+ case BOOT_DEVICE_MMC1:
+ return MMCSD_MODE_FAT;
+ break;
+#endif
+ case BOOT_DEVICE_NONE:
+ default:
+ hang();
+ }
+}
+
+void s_init(void)
+{
+ /* disable watchdog */
+ at91_disable_wdt();
+
+ /* PMC configuration */
+ at91_pmc_init();
+
+ at91_clock_init(CONFIG_SYS_AT91_MAIN_CLOCK);
+
+ timer_init();
+
+ board_early_init_f();
+
+ preloader_console_init();
+
+ mem_init();
+}
diff --git a/arch/arm/cpu/at91-common/u-boot-spl.lds b/arch/arm/cpu/at91-common/u-boot-spl.lds
new file mode 100644
index 0000000..038335d
--- /dev/null
+++ b/arch/arm/cpu/at91-common/u-boot-spl.lds
@@ -0,0 +1,50 @@
+/*
+ * (C) Copyright 2002
+ * Gary Jennejohn, DENX Software Engineering, <garyj@denx.de>
+ *
+ * (C) Copyright 2010
+ * Texas Instruments, <www.ti.com>
+ * Aneesh V <aneesh@ti.com>
+ *
+ * (C) 2013 Atmel Corporation
+ * Bo Shen <voice.shen@atmel.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \
+ LENGTH = CONFIG_SPL_MAX_SIZE }
+MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR, \
+ LENGTH = CONFIG_SPL_BSS_MAX_SIZE }
+
+OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
+OUTPUT_ARCH(arm)
+ENTRY(_start)
+SECTIONS
+{
+ .text :
+ {
+ __start = .;
+ arch/arm/cpu/armv7/start.o (.text*)
+ *(.text*)
+ } >.sram
+
+ . = ALIGN(4);
+ .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram
+
+ . = ALIGN(4);
+ .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram
+
+ . = ALIGN(4);
+ __image_copy_end = .;
+ _end = .;
+
+ .bss :
+ {
+ . = ALIGN(4);
+ __bss_start = .;
+ *(.bss*)
+ . = ALIGN(4);
+ __bss_end = .;
+ } >.sdram
+}
diff --git a/arch/arm/include/asm/arch-at91/at91_common.h b/arch/arm/include/asm/arch-at91/at91_common.h
index abcb97d..3ca4d5b 100644
--- a/arch/arm/include/asm/arch-at91/at91_common.h
+++ b/arch/arm/include/asm/arch-at91/at91_common.h
@@ -22,5 +22,9 @@ void at91_spi1_hw_init(unsigned long cs_mask);
void at91_udp_hw_init(void);
void at91_uhp_hw_init(void);
void at91_lcd_hw_init(void);
+void at91_plla_init(u32 pllar);
+void at91_mck_init(u32 mckr);
+void at91_pmc_init(void);
+void mem_init(void);
#endif /* AT91_COMMON_H */
diff --git a/arch/arm/include/asm/arch-at91/spl.h b/arch/arm/include/asm/arch-at91/spl.h
new file mode 100644
index 0000000..68c5349
--- /dev/null
+++ b/arch/arm/include/asm/arch-at91/spl.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2013 Atmel Corporation
+ * Bo Shen <voice.shen@atmel.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef _ASM_ARCH_SPL_H_
+#define _ASM_ARCH_SPL_H_
+
+enum {
+ BOOT_DEVICE_NONE,
+#ifdef CONFIG_SYS_USE_MMC
+ BOOT_DEVICE_MMC1,
+ BOOT_DEVICE_MMC2,
+ BOOT_DEVICE_MMC2_2,
+#endif
+};
+
+#endif
diff --git a/board/atmel/sama5d3xek/sama5d3xek.c b/board/atmel/sama5d3xek/sama5d3xek.c
index f245f98..846a1b8 100644
--- a/board/atmel/sama5d3xek/sama5d3xek.c
+++ b/board/atmel/sama5d3xek/sama5d3xek.c
@@ -20,6 +20,9 @@
#include <micrel.h>
#include <net.h>
#include <netdev.h>
+#include <spl.h>
+#include <asm/arch/atmel_mpddrc.h>
+#include <asm/arch/at91_wdt.h>
#ifdef CONFIG_USB_GADGET_ATMEL_USBA
#include <asm/arch/atmel_usba_udc.h>
@@ -296,3 +299,82 @@ void spi_cs_deactivate(struct spi_slave *slave)
}
}
#endif /* CONFIG_ATMEL_SPI */
+
+/* SPL */
+#ifdef CONFIG_SPL_BUILD
+void spl_board_init(void)
+{
+#ifdef CONFIG_SYS_USE_MMC
+ sama5d3xek_mci_hw_init();
+#endif
+}
+
+void ddr2_conf(struct atmel_mpddr *ddr2)
+{
+ ddr2->mdr = (ATMEL_MPDDRC_MDR_DBW_32BITS | ATMEL_MPDDRC_MDR_DDR2_SDRAM);
+
+ ddr2->cr = (ATMEL_MPDDRC_CR_NC_COL_10 |
+ ATMEL_MPDDRC_CR_NR_ROW_14 |
+ ATMEL_MPDDRC_CR_CAS_3 |
+ ATMEL_MPDDRC_CR_EN_ENRDM |
+ ATMEL_MPDDRC_CR_NB_8BANKS |
+ ATMEL_MPDDRC_CR_DIS_NDQS |
+ ATMEL_MPDDRC_CR_DECOD_INTERLEAVED |
+ ATMEL_MPDDRC_CR_UNAL_SUPPORTED);
+
+ ddr2->rtr = 0x411;
+
+ ddr2->tp0r = (6 << ATMEL_MPDDRC_TP0R_TRAS_OFFSET |
+ 2 << ATMEL_MPDDRC_TP0R_TRCD_OFFSET |
+ 2 << ATMEL_MPDDRC_TP0R_TWR_OFFSET |
+ 8 << ATMEL_MPDDRC_TP0R_TRC_OFFSET |
+ 2 << ATMEL_MPDDRC_TP0R_TRP_OFFSET |
+ 2 << ATMEL_MPDDRC_TP0R_TRRD_OFFSET |
+ 2 << ATMEL_MPDDRC_TP0R_TWTR_OFFSET |
+ 2 << ATMEL_MPDDRC_TP0R_TMRD_OFFSET);
+
+ ddr2->tp1r = (2 << ATMEL_MPDDRC_TP1R_TXP_OFFSET |
+ 200 << ATMEL_MPDDRC_TP1R_TXSRD_OFFSET |
+ 28 << ATMEL_MPDDRC_TP1R_TXSNR_OFFSET |
+ 26 << ATMEL_MPDDRC_TP1R_TRFC_OFFSET);
+
+ ddr2->tp2r = (7 << ATMEL_MPDDRC_TP2R_TFAW_OFFSET |
+ 2 << ATMEL_MPDDRC_TP2R_TRTP_OFFSET |
+ 2 << ATMEL_MPDDRC_TP2R_TRPA_OFFSET |
+ 7 << ATMEL_MPDDRC_TP2R_TXARDS_OFFSET |
+ 8 << ATMEL_MPDDRC_TP2R_TXARD_OFFSET);
+}
+
+void mem_init(void)
+{
+ struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
+ struct atmel_mpddr ddr2;
+
+ ddr2_conf(&ddr2);
+
+ /* enable MPDDR clock */
+ at91_periph_clk_enable(ATMEL_ID_MPDDRC);
+ writel(0x4, &pmc->scer);
+
+ /* DDRAM2 Controller initialize */
+ ddr2_init(ATMEL_BASE_DDRCS, &ddr2);
+}
+
+void at91_pmc_init(void)
+{
+ struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
+ u32 tmp;
+
+ tmp = AT91_PMC_PLLAR_29 |
+ AT91_PMC_PLLXR_PLLCOUNT(0x3f) |
+ AT91_PMC_PLLXR_MUL(43) |
+ AT91_PMC_PLLXR_DIV(1);
+ at91_plla_init(tmp);
+
+ writel(0x3 << 8, &pmc->pllicpr);
+
+ tmp = AT91_PMC_MCKR_MDIV_4 |
+ AT91_PMC_MCKR_CSS_PLLA;
+ at91_mck_init(tmp);
+}
+#endif
diff --git a/include/configs/sama5d3xek.h b/include/configs/sama5d3xek.h
index 79c0068..9c80e82 100644
--- a/include/configs/sama5d3xek.h
+++ b/include/configs/sama5d3xek.h
@@ -25,7 +25,10 @@
#define CONFIG_AT91FAMILY
#define CONFIG_ARCH_CPU_INIT
+#ifndef CONFIG_SPL_BUILD
#define CONFIG_SKIP_LOWLEVEL_INIT
+#endif
+
#define CONFIG_BOARD_EARLY_INIT_F
#define CONFIG_DISPLAY_CPUINFO
@@ -94,8 +97,12 @@
#define CONFIG_SYS_SDRAM_BASE ATMEL_BASE_DDRCS
#define CONFIG_SYS_SDRAM_SIZE 0x20000000
+#ifdef CONFIG_SPL_BUILD
+#define CONFIG_SYS_INIT_SP_ADDR 0x310000
+#else
#define CONFIG_SYS_INIT_SP_ADDR \
(CONFIG_SYS_SDRAM_BASE + 4 * 1024 - GENERATED_GBL_DATA_SIZE)
+#endif
/* SerialFlash */
#define CONFIG_CMD_SF
@@ -235,4 +242,31 @@
/* Size of malloc() pool */
#define CONFIG_SYS_MALLOC_LEN (1024 * 1024)
+/* SPL */
+#define CONFIG_SPL
+#define CONFIG_SPL_FRAMEWORK
+#define CONFIG_SPL_TEXT_BASE 0x300000
+#define CONFIG_SPL_MAX_SIZE 0x10000
+#define CONFIG_SPL_BSS_START_ADDR 0x20000000
+#define CONFIG_SPL_BSS_MAX_SIZE 0x80000
+#define CONFIG_SYS_SPL_MALLOC_START 0x20080000
+#define CONFIG_SYS_SPL_MALLOC_SIZE 0x80000
+
+#define CONFIG_SPL_LIBCOMMON_SUPPORT
+#define CONFIG_SPL_LIBGENERIC_SUPPORT
+#define CONFIG_SPL_GPIO_SUPPORT
+#define CONFIG_SPL_SERIAL_SUPPORT
+
+#define CONFIG_SPL_BOARD_INIT
+#ifdef CONFIG_SYS_USE_MMC
+#define CONFIG_SPL_LDSCRIPT arch/arm/cpu/at91-common/u-boot-spl.lds
+#define CONFIG_SPL_MMC_SUPPORT
+#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 0x400
+#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0x200
+#define CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION 1
+#define CONFIG_SPL_FAT_LOAD_PAYLOAD_NAME "u-boot.img"
+#define CONFIG_SPL_FAT_SUPPORT
+#define CONFIG_SPL_LIBDISK_SUPPORT
+#endif
+
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card
2013-11-06 5:29 ` [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card Bo Shen
@ 2013-11-13 13:34 ` Andreas Bießmann
2013-11-14 5:52 ` Heiko Schocher
0 siblings, 1 reply; 21+ messages in thread
From: Andreas Bießmann @ 2013-11-13 13:34 UTC (permalink / raw)
To: u-boot
Hi Bo,
On 11/06/2013 06:29 AM, Bo Shen wrote:
> Enable Atmel sama5d3xek boart spl boot support, which can load u-boot
> from SD card with FAT file system.
>
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>
> ---
> Changes in v3:
> - Move plla and mck configure to spl.c file
>
> Changes in v2:
> - Move spl related code to at91-common folder
>
> arch/arm/cpu/armv7/Makefile | 2 +-
> arch/arm/cpu/at91-common/Makefile | 1 +
> arch/arm/cpu/at91-common/spl.c | 90 ++++++++++++++++++++++++++
> arch/arm/cpu/at91-common/u-boot-spl.lds | 50 ++++++++++++++
> arch/arm/include/asm/arch-at91/at91_common.h | 4 ++
> arch/arm/include/asm/arch-at91/spl.h | 20 ++++++
> board/atmel/sama5d3xek/sama5d3xek.c | 82 +++++++++++++++++++++++
> include/configs/sama5d3xek.h | 34 ++++++++++
> 8 files changed, 282 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/cpu/at91-common/spl.c
> create mode 100644 arch/arm/cpu/at91-common/u-boot-spl.lds
> create mode 100644 arch/arm/include/asm/arch-at91/spl.h
>
> diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
> index ee4b021..aea4e8b 100644
> --- a/arch/arm/cpu/armv7/Makefile
> +++ b/arch/arm/cpu/armv7/Makefile
> @@ -16,7 +16,7 @@ COBJS += cache_v7.o
> COBJS += cpu.o
> COBJS += syslib.o
>
> -ifneq ($(CONFIG_AM43XX)$(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONFIG_MX6)$(CONFIG_TI81XX),)
> +ifneq ($(CONFIG_AM43XX)$(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONFIG_MX6)$(CONFIG_TI81XX)$(CONFIG_AT91FAMILY),)
> SOBJS += lowlevel_init.o
> endif
>
> diff --git a/arch/arm/cpu/at91-common/Makefile b/arch/arm/cpu/at91-common/Makefile
> index 6f1a9e6..4377a0b 100644
> --- a/arch/arm/cpu/at91-common/Makefile
> +++ b/arch/arm/cpu/at91-common/Makefile
> @@ -13,6 +13,7 @@ include $(TOPDIR)/config.mk
> LIB = $(obj)libat91-common.o
>
> COBJS-$(CONFIG_SPL_BUILD) += mpddrc.o
> +COBJS-$(CONFIG_SPL_BUILD) += spl.o
>
> SRCS := $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
> OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
> diff --git a/arch/arm/cpu/at91-common/spl.c b/arch/arm/cpu/at91-common/spl.c
> new file mode 100644
> index 0000000..37c0cc4
> --- /dev/null
> +++ b/arch/arm/cpu/at91-common/spl.c
> @@ -0,0 +1,90 @@
> +/*
> + * Copyright (C) 2013 Atmel Corporation
> + * Bo Shen <voice.shen@atmel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/at91_common.h>
> +#include <asm/arch/at91_pmc.h>
> +#include <asm/arch/at91_wdt.h>
> +#include <asm/arch/clk.h>
> +#include <spl.h>
> +
> +static void at91_disable_wdt(void)
Why should we disable the WDT in SPL? I think it would be better to
configure a working timer value than just disable it.
Well it's easy and works, but for the future I think it would be good to
let it run while in SPL and u-boot.
> +{
> + struct at91_wdt *wdt = (struct at91_wdt *)ATMEL_BASE_WDT;
> +
> + writel(AT91_WDT_MR_WDDIS, &wdt->mr);
> +}
> +
> +void at91_plla_init(u32 pllar)
> +{
> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
> +
We should ensure bit 29 to be '1' here.
> + writel(pllar, &pmc->pllar);
> + while (!(readl(&pmc->sr) & (AT91_PMC_LOCKA | AT91_PMC_MCKRDY)))
> + ;
Especially for doing such things it would be best handled by the WDT on
error.
> +}
> +
> +void at91_mck_init(u32 mckr)
> +{
> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
> + u32 tmp;
> +
> + tmp = readl(&pmc->mckr);
> + tmp &= ~(AT91_PMC_MCKR_PRES_MASK |
> + AT91_PMC_MCKR_MDIV_MASK |
> + AT91_PMC_MCKR_PLLADIV_2);
> + tmp |= mckr & (AT91_PMC_MCKR_PRES_MASK |
> + AT91_PMC_MCKR_MDIV_MASK |
> + AT91_PMC_MCKR_PLLADIV_2);
Why gets the at91_mck_init() just some parts of the MCK register (some
fields are preserved here) while the at91_plla_init() just rewrites the
PLLA register?
I think it is not much more than hiding the writel() register access. I
think a better API would be to request some specific frequency and we
calculate the register values with that input.
Please let us discuss this.
> + writel(tmp, &pmc->mckr);
> +
> + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY))
> + ;
> +}
> +
> +
> +u32 spl_boot_device(void)
> +{
> +#ifdef CONFIG_SYS_USE_MMC
> + return BOOT_DEVICE_MMC1;
> +#endif
Isn't there some way to detect the boot source by asking for example the
ROM code? Is there some register that holds that information?
> + return BOOT_DEVICE_NONE;
> +}
> +
> +u32 spl_boot_mode(void)
> +{
> + switch (spl_boot_device()) {
> +#ifdef CONFIG_SYS_USE_MMC
> + case BOOT_DEVICE_MMC1:
> + return MMCSD_MODE_FAT;
> + break;
> +#endif
> + case BOOT_DEVICE_NONE:
> + default:
> + hang();
> + }
> +}
> +
> +void s_init(void)
> +{
> + /* disable watchdog */
> + at91_disable_wdt();
> +
> + /* PMC configuration */
> + at91_pmc_init();
> +
> + at91_clock_init(CONFIG_SYS_AT91_MAIN_CLOCK);
> +
> + timer_init();
> +
> + board_early_init_f();
> +
> + preloader_console_init();
> +
> + mem_init();
> +}
> diff --git a/arch/arm/cpu/at91-common/u-boot-spl.lds b/arch/arm/cpu/at91-common/u-boot-spl.lds
> new file mode 100644
> index 0000000..038335d
> --- /dev/null
> +++ b/arch/arm/cpu/at91-common/u-boot-spl.lds
> @@ -0,0 +1,50 @@
> +/*
> + * (C) Copyright 2002
> + * Gary Jennejohn, DENX Software Engineering, <garyj@denx.de>
> + *
> + * (C) Copyright 2010
> + * Texas Instruments, <www.ti.com>
> + * Aneesh V <aneesh@ti.com>
> + *
> + * (C) 2013 Atmel Corporation
> + * Bo Shen <voice.shen@atmel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \
> + LENGTH = CONFIG_SPL_MAX_SIZE }
> +MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR, \
> + LENGTH = CONFIG_SPL_BSS_MAX_SIZE }
> +
> +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
> +OUTPUT_ARCH(arm)
> +ENTRY(_start)
> +SECTIONS
> +{
> + .text :
> + {
> + __start = .;
> + arch/arm/cpu/armv7/start.o (.text*)
> + *(.text*)
> + } >.sram
> +
> + . = ALIGN(4);
> + .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram
> +
> + . = ALIGN(4);
> + .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram
> +
> + . = ALIGN(4);
> + __image_copy_end = .;
> + _end = .;
> +
> + .bss :
> + {
> + . = ALIGN(4);
> + __bss_start = .;
> + *(.bss*)
> + . = ALIGN(4);
> + __bss_end = .;
> + } >.sdram
> +}
> diff --git a/arch/arm/include/asm/arch-at91/at91_common.h b/arch/arm/include/asm/arch-at91/at91_common.h
> index abcb97d..3ca4d5b 100644
> --- a/arch/arm/include/asm/arch-at91/at91_common.h
> +++ b/arch/arm/include/asm/arch-at91/at91_common.h
> @@ -22,5 +22,9 @@ void at91_spi1_hw_init(unsigned long cs_mask);
> void at91_udp_hw_init(void);
> void at91_uhp_hw_init(void);
> void at91_lcd_hw_init(void);
> +void at91_plla_init(u32 pllar);
> +void at91_mck_init(u32 mckr);
> +void at91_pmc_init(void);
> +void mem_init(void);
>
> #endif /* AT91_COMMON_H */
> diff --git a/arch/arm/include/asm/arch-at91/spl.h b/arch/arm/include/asm/arch-at91/spl.h
> new file mode 100644
> index 0000000..68c5349
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-at91/spl.h
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright (C) 2013 Atmel Corporation
> + * Bo Shen <voice.shen@atmel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef _ASM_ARCH_SPL_H_
> +#define _ASM_ARCH_SPL_H_
> +
> +enum {
> + BOOT_DEVICE_NONE,
> +#ifdef CONFIG_SYS_USE_MMC
> + BOOT_DEVICE_MMC1,
> + BOOT_DEVICE_MMC2,
> + BOOT_DEVICE_MMC2_2,
> +#endif
> +};
> +
> +#endif
> diff --git a/board/atmel/sama5d3xek/sama5d3xek.c b/board/atmel/sama5d3xek/sama5d3xek.c
> index f245f98..846a1b8 100644
> --- a/board/atmel/sama5d3xek/sama5d3xek.c
> +++ b/board/atmel/sama5d3xek/sama5d3xek.c
> @@ -20,6 +20,9 @@
> #include <micrel.h>
> #include <net.h>
> #include <netdev.h>
> +#include <spl.h>
> +#include <asm/arch/atmel_mpddrc.h>
> +#include <asm/arch/at91_wdt.h>
>
> #ifdef CONFIG_USB_GADGET_ATMEL_USBA
> #include <asm/arch/atmel_usba_udc.h>
> @@ -296,3 +299,82 @@ void spi_cs_deactivate(struct spi_slave *slave)
> }
> }
> #endif /* CONFIG_ATMEL_SPI */
> +
> +/* SPL */
> +#ifdef CONFIG_SPL_BUILD
> +void spl_board_init(void)
> +{
> +#ifdef CONFIG_SYS_USE_MMC
> + sama5d3xek_mci_hw_init();
> +#endif
> +}
> +
> +void ddr2_conf(struct atmel_mpddr *ddr2)
> +{
> + ddr2->mdr = (ATMEL_MPDDRC_MDR_DBW_32BITS | ATMEL_MPDDRC_MDR_DDR2_SDRAM);
> +
> + ddr2->cr = (ATMEL_MPDDRC_CR_NC_COL_10 |
> + ATMEL_MPDDRC_CR_NR_ROW_14 |
> + ATMEL_MPDDRC_CR_CAS_3 |
> + ATMEL_MPDDRC_CR_EN_ENRDM |
> + ATMEL_MPDDRC_CR_NB_8BANKS |
> + ATMEL_MPDDRC_CR_DIS_NDQS |
> + ATMEL_MPDDRC_CR_DECOD_INTERLEAVED |
> + ATMEL_MPDDRC_CR_UNAL_SUPPORTED);
> +
> + ddr2->rtr = 0x411;
Please document magic 0x411.
> +
> + ddr2->tp0r = (6 << ATMEL_MPDDRC_TP0R_TRAS_OFFSET |
> + 2 << ATMEL_MPDDRC_TP0R_TRCD_OFFSET |
> + 2 << ATMEL_MPDDRC_TP0R_TWR_OFFSET |
> + 8 << ATMEL_MPDDRC_TP0R_TRC_OFFSET |
> + 2 << ATMEL_MPDDRC_TP0R_TRP_OFFSET |
> + 2 << ATMEL_MPDDRC_TP0R_TRRD_OFFSET |
> + 2 << ATMEL_MPDDRC_TP0R_TWTR_OFFSET |
> + 2 << ATMEL_MPDDRC_TP0R_TMRD_OFFSET);
> +
> + ddr2->tp1r = (2 << ATMEL_MPDDRC_TP1R_TXP_OFFSET |
> + 200 << ATMEL_MPDDRC_TP1R_TXSRD_OFFSET |
> + 28 << ATMEL_MPDDRC_TP1R_TXSNR_OFFSET |
> + 26 << ATMEL_MPDDRC_TP1R_TRFC_OFFSET);
> +
> + ddr2->tp2r = (7 << ATMEL_MPDDRC_TP2R_TFAW_OFFSET |
> + 2 << ATMEL_MPDDRC_TP2R_TRTP_OFFSET |
> + 2 << ATMEL_MPDDRC_TP2R_TRPA_OFFSET |
> + 7 << ATMEL_MPDDRC_TP2R_TXARDS_OFFSET |
> + 8 << ATMEL_MPDDRC_TP2R_TXARD_OFFSET);
> +}
NAK, that is just copying data from ro section to bss section, please
setup a static struct atmel_mpddr with static initialization. I can't
see use cases where this won't work. You could have just more than one
register setup and choose the correct one. Or modify a single instance
before feeding it into ddr2_init().
> +
> +void mem_init(void)
> +{
> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
> + struct atmel_mpddr ddr2;
> +
> + ddr2_conf(&ddr2);
> +
> + /* enable MPDDR clock */
> + at91_periph_clk_enable(ATMEL_ID_MPDDRC);
> + writel(0x4, &pmc->scer);
> +
> + /* DDRAM2 Controller initialize */
> + ddr2_init(ATMEL_BASE_DDRCS, &ddr2);
> +}
> +
> +void at91_pmc_init(void)
> +{
> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
> + u32 tmp;
> +
> + tmp = AT91_PMC_PLLAR_29 |
> + AT91_PMC_PLLXR_PLLCOUNT(0x3f) |
> + AT91_PMC_PLLXR_MUL(43) |
> + AT91_PMC_PLLXR_DIV(1);
> + at91_plla_init(tmp);
That could also be written with
writel(tmp, &pmc->pllar);
Ok, and wait for lock ... You know what I mean? As mentioned above it is
just writing the register, not really much abstraction for that
at91_plla_init() function.
But please do not remove that function. I think it worth to have it in
the current setup.
But I also think we could do it better, for example the board just calls
some function to setup 400MHz (and will then possibly use another
frequency cause the given oscillators could not reach exactely that
frequency).
> +
> + writel(0x3 << 8, &pmc->pllicpr);
Datasheet says PLLICPR->IPLL_PLLA should be written to '0'. Why do you
write it to three? Is there something missing in the spec?
> +
> + tmp = AT91_PMC_MCKR_MDIV_4 |
> + AT91_PMC_MCKR_CSS_PLLA;
> + at91_mck_init(tmp);
> +}
> +#endif
> diff --git a/include/configs/sama5d3xek.h b/include/configs/sama5d3xek.h
> index 79c0068..9c80e82 100644
> --- a/include/configs/sama5d3xek.h
> +++ b/include/configs/sama5d3xek.h
> @@ -25,7 +25,10 @@
> #define CONFIG_AT91FAMILY
> #define CONFIG_ARCH_CPU_INIT
>
> +#ifndef CONFIG_SPL_BUILD
> #define CONFIG_SKIP_LOWLEVEL_INIT
> +#endif
> +
> #define CONFIG_BOARD_EARLY_INIT_F
> #define CONFIG_DISPLAY_CPUINFO
>
> @@ -94,8 +97,12 @@
> #define CONFIG_SYS_SDRAM_BASE ATMEL_BASE_DDRCS
> #define CONFIG_SYS_SDRAM_SIZE 0x20000000
>
> +#ifdef CONFIG_SPL_BUILD
> +#define CONFIG_SYS_INIT_SP_ADDR 0x310000
> +#else
> #define CONFIG_SYS_INIT_SP_ADDR \
> (CONFIG_SYS_SDRAM_BASE + 4 * 1024 - GENERATED_GBL_DATA_SIZE)
> +#endif
>
> /* SerialFlash */
> #define CONFIG_CMD_SF
> @@ -235,4 +242,31 @@
> /* Size of malloc() pool */
> #define CONFIG_SYS_MALLOC_LEN (1024 * 1024)
>
> +/* SPL */
> +#define CONFIG_SPL
> +#define CONFIG_SPL_FRAMEWORK
> +#define CONFIG_SPL_TEXT_BASE 0x300000
> +#define CONFIG_SPL_MAX_SIZE 0x10000
> +#define CONFIG_SPL_BSS_START_ADDR 0x20000000
> +#define CONFIG_SPL_BSS_MAX_SIZE 0x80000
> +#define CONFIG_SYS_SPL_MALLOC_START 0x20080000
> +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x80000
> +
> +#define CONFIG_SPL_LIBCOMMON_SUPPORT
> +#define CONFIG_SPL_LIBGENERIC_SUPPORT
> +#define CONFIG_SPL_GPIO_SUPPORT
> +#define CONFIG_SPL_SERIAL_SUPPORT
> +
> +#define CONFIG_SPL_BOARD_INIT
> +#ifdef CONFIG_SYS_USE_MMC
> +#define CONFIG_SPL_LDSCRIPT arch/arm/cpu/at91-common/u-boot-spl.lds
> +#define CONFIG_SPL_MMC_SUPPORT
> +#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 0x400
> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0x200
> +#define CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION 1
> +#define CONFIG_SPL_FAT_LOAD_PAYLOAD_NAME "u-boot.img"
> +#define CONFIG_SPL_FAT_SUPPORT
> +#define CONFIG_SPL_LIBDISK_SUPPORT
> +#endif
> +
> #endif
>
Looks good to me. The only thing I'd really like to change now is the
ddr setup with a function, the rest should be discussed.
Best regards
Andreas Bie?mann
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card
2013-11-13 13:34 ` Andreas Bießmann
@ 2013-11-14 5:52 ` Heiko Schocher
2013-11-14 6:28 ` Andreas Bießmann
0 siblings, 1 reply; 21+ messages in thread
From: Heiko Schocher @ 2013-11-14 5:52 UTC (permalink / raw)
To: u-boot
Hello Andreas,
Am 13.11.2013 14:34, schrieb Andreas Bie?mann:
> Hi Bo,
>
> On 11/06/2013 06:29 AM, Bo Shen wrote:
>> Enable Atmel sama5d3xek boart spl boot support, which can load u-boot
>> from SD card with FAT file system.
>>
>> Signed-off-by: Bo Shen<voice.shen@atmel.com>
>>
>> ---
>> Changes in v3:
>> - Move plla and mck configure to spl.c file
>>
>> Changes in v2:
>> - Move spl related code to at91-common folder
>>
>> arch/arm/cpu/armv7/Makefile | 2 +-
>> arch/arm/cpu/at91-common/Makefile | 1 +
>> arch/arm/cpu/at91-common/spl.c | 90 ++++++++++++++++++++++++++
>> arch/arm/cpu/at91-common/u-boot-spl.lds | 50 ++++++++++++++
>> arch/arm/include/asm/arch-at91/at91_common.h | 4 ++
>> arch/arm/include/asm/arch-at91/spl.h | 20 ++++++
>> board/atmel/sama5d3xek/sama5d3xek.c | 82 +++++++++++++++++++++++
>> include/configs/sama5d3xek.h | 34 ++++++++++
>> 8 files changed, 282 insertions(+), 1 deletion(-)
>> create mode 100644 arch/arm/cpu/at91-common/spl.c
>> create mode 100644 arch/arm/cpu/at91-common/u-boot-spl.lds
>> create mode 100644 arch/arm/include/asm/arch-at91/spl.h
[...]
>> diff --git a/arch/arm/cpu/at91-common/spl.c b/arch/arm/cpu/at91-common/spl.c
>> new file mode 100644
>> index 0000000..37c0cc4
>> --- /dev/null
>> +++ b/arch/arm/cpu/at91-common/spl.c
>> @@ -0,0 +1,90 @@
>> +/*
>> + * Copyright (C) 2013 Atmel Corporation
>> + * Bo Shen<voice.shen@atmel.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include<common.h>
>> +#include<asm/io.h>
>> +#include<asm/arch/at91_common.h>
>> +#include<asm/arch/at91_pmc.h>
>> +#include<asm/arch/at91_wdt.h>
>> +#include<asm/arch/clk.h>
>> +#include<spl.h>
>> +
>> +static void at91_disable_wdt(void)
>
> Why should we disable the WDT in SPL? I think it would be better to
> configure a working timer value than just disable it.
This is currently done in the at91bootstrap code too...
> Well it's easy and works, but for the future I think it would be good to
> let it run while in SPL and u-boot.
We should have the option to enable/disable it ...
>> +{
>> + struct at91_wdt *wdt = (struct at91_wdt *)ATMEL_BASE_WDT;
>> +
>> + writel(AT91_WDT_MR_WDDIS,&wdt->mr);
>> +}
>> +
>> +void at91_plla_init(u32 pllar)
>> +{
>> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
>> +
>
> We should ensure bit 29 to be '1' here.
What is bit 29 ?
>> + writel(pllar,&pmc->pllar);
>> + while (!(readl(&pmc->sr)& (AT91_PMC_LOCKA | AT91_PMC_MCKRDY)))
>> + ;
>
> Especially for doing such things it would be best handled by the WDT on
> error.
>
>> +}
>> +
>> +void at91_mck_init(u32 mckr)
>> +{
>> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
>> + u32 tmp;
>> +
>> + tmp = readl(&pmc->mckr);
>> + tmp&= ~(AT91_PMC_MCKR_PRES_MASK |
>> + AT91_PMC_MCKR_MDIV_MASK |
>> + AT91_PMC_MCKR_PLLADIV_2);
>> + tmp |= mckr& (AT91_PMC_MCKR_PRES_MASK |
>> + AT91_PMC_MCKR_MDIV_MASK |
>> + AT91_PMC_MCKR_PLLADIV_2);
>
> Why gets the at91_mck_init() just some parts of the MCK register (some
> fields are preserved here) while the at91_plla_init() just rewrites the
> PLLA register?
>
> I think it is not much more than hiding the writel() register access. I
> think a better API would be to request some specific frequency and we
> calculate the register values with that input.
> Please let us discuss this.
Such a function would be nice, indeed. But I would accept this function,
to get SPL code into mainline... (I have the same function ;-)
>> + writel(tmp,&pmc->mckr);
>> +
>> + while (!(readl(&pmc->sr)& AT91_PMC_MCKRDY))
>> + ;
>> +}
>> +
>> +
>> +u32 spl_boot_device(void)
>> +{
>> +#ifdef CONFIG_SYS_USE_MMC
>> + return BOOT_DEVICE_MMC1;
>> +#endif
>
> Isn't there some way to detect the boot source by asking for example the
> ROM code? Is there some register that holds that information?
Yeah, that would be nice if we have such an information. Just got the
information, that on the taurus board is also a SPI dataflash, from
which it can boot. So I have the problem, that I have two boot sources,
and if I want only one spl image, I have to detect from where SPL code
got loaded ...
[...]
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card
2013-11-14 5:52 ` Heiko Schocher
@ 2013-11-14 6:28 ` Andreas Bießmann
2013-11-14 6:53 ` Bo Shen
0 siblings, 1 reply; 21+ messages in thread
From: Andreas Bießmann @ 2013-11-14 6:28 UTC (permalink / raw)
To: u-boot
Hello Heiko,
On 14.11.13 06:52, Heiko Schocher wrote:
> Am 13.11.2013 14:34, schrieb Andreas Bie?mann:
>> Hi Bo,
>>
>> On 11/06/2013 06:29 AM, Bo Shen wrote:
>>> Enable Atmel sama5d3xek boart spl boot support, which can load u-boot
>>> from SD card with FAT file system.
>>>
>>> Signed-off-by: Bo Shen<voice.shen@atmel.com>
>>>
>>> ---
<snip>
>>> +static void at91_disable_wdt(void)
>>
>> Why should we disable the WDT in SPL? I think it would be better to
>> configure a working timer value than just disable it.
>
> This is currently done in the at91bootstrap code too...
I know ...
>
>> Well it's easy and works, but for the future I think it would be good to
>> let it run while in SPL and u-boot.
>
> We should have the option to enable/disable it ...
I just like to mention that this should be changed in near future.
>>> +{
>>> + struct at91_wdt *wdt = (struct at91_wdt *)ATMEL_BASE_WDT;
>>> +
>>> + writel(AT91_WDT_MR_WDDIS,&wdt->mr);
>>> +}
>>> +
>>> +void at91_plla_init(u32 pllar)
>>> +{
>>> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
>>> +
>>
>> We should ensure bit 29 to be '1' here.
>
> What is bit 29 ?
'ONE' ;) Spec says it must be written to '1'
>>> + writel(pllar,&pmc->pllar);
>>> + while (!(readl(&pmc->sr)& (AT91_PMC_LOCKA | AT91_PMC_MCKRDY)))
>>> + ;
>>
>> Especially for doing such things it would be best handled by the WDT on
>> error.
>>
>>> +}
>>> +
>>> +void at91_mck_init(u32 mckr)
>>> +{
>>> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
>>> + u32 tmp;
>>> +
>>> + tmp = readl(&pmc->mckr);
>>> + tmp&= ~(AT91_PMC_MCKR_PRES_MASK |
>>> + AT91_PMC_MCKR_MDIV_MASK |
>>> + AT91_PMC_MCKR_PLLADIV_2);
>>> + tmp |= mckr& (AT91_PMC_MCKR_PRES_MASK |
>>> + AT91_PMC_MCKR_MDIV_MASK |
>>> + AT91_PMC_MCKR_PLLADIV_2);
>>
>> Why gets the at91_mck_init() just some parts of the MCK register (some
>> fields are preserved here) while the at91_plla_init() just rewrites the
>> PLLA register?
Don't you see the error in my sentence here? ;) I thought some parts of
the register content where preserved. The other way round is true, we
need to clean up relevant parts (PRES, MDIV, and PLLADIV Bit(s)). Sorry,
my fault here.
>> I think it is not much more than hiding the writel() register access. I
>> think a better API would be to request some specific frequency and we
>> calculate the register values with that input.
>> Please let us discuss this.
>
> Such a function would be nice, indeed. But I would accept this function,
> to get SPL code into mainline... (I have the same function ;-)
Yea, no problem with this function. I just wonder why the
at91_plla_init() is a plain writel() (plus waiting for lock) and this
one does a bit more (preserve CSS... We could also declare the whole
register content when calling at91_mckr_init(), don't we?
This is no change request in general, but please lets discuss why we not
just write the given register content.
Best regards
Andreas Bie?mann
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card
2013-11-14 6:28 ` Andreas Bießmann
@ 2013-11-14 6:53 ` Bo Shen
2013-11-14 7:49 ` Andreas Bießmann
0 siblings, 1 reply; 21+ messages in thread
From: Bo Shen @ 2013-11-14 6:53 UTC (permalink / raw)
To: u-boot
Hi Andreas,
On 11/14/2013 02:28 PM, Andreas Bie?mann wrote:
> Hello Heiko,
>
> On 14.11.13 06:52, Heiko Schocher wrote:
>> Am 13.11.2013 14:34, schrieb Andreas Bie?mann:
>>> Hi Bo,
>>>
>>> On 11/06/2013 06:29 AM, Bo Shen wrote:
>>>> Enable Atmel sama5d3xek boart spl boot support, which can load u-boot
>>>> from SD card with FAT file system.
>>>>
>>>> Signed-off-by: Bo Shen<voice.shen@atmel.com>
>>>>
>>>> ---
>
> <snip>
>
>>>> +static void at91_disable_wdt(void)
>>>
>>> Why should we disable the WDT in SPL? I think it would be better to
>>> configure a working timer value than just disable it.
>>
>> This is currently done in the at91bootstrap code too...
>
> I know ...
>>
>>> Well it's easy and works, but for the future I think it would be good to
>>> let it run while in SPL and u-boot.
>>
>> We should have the option to enable/disable it ...
The watchdog is one time configurable. If configurabled, can not change
anymore, so we disable it by default.
> I just like to mention that this should be changed in near future.
>
>>>> +{
>>>> + struct at91_wdt *wdt = (struct at91_wdt *)ATMEL_BASE_WDT;
>>>> +
>>>> + writel(AT91_WDT_MR_WDDIS,&wdt->mr);
>>>> +}
>>>> +
>>>> +void at91_plla_init(u32 pllar)
>>>> +{
>>>> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
>>>> +
>>>
>>> We should ensure bit 29 to be '1' here.
>>
>> What is bit 29 ?
>
> 'ONE' ;) Spec says it must be written to '1'
OK, I will add a check here.
>>>> + writel(pllar,&pmc->pllar);
>>>> + while (!(readl(&pmc->sr)& (AT91_PMC_LOCKA | AT91_PMC_MCKRDY)))
>>>> + ;
>>>
>>> Especially for doing such things it would be best handled by the WDT on
>>> error.
As watchdog is disabled, we can not use it here.
>>>> +}
>>>> +
>>>> +void at91_mck_init(u32 mckr)
>>>> +{
>>>> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
>>>> + u32 tmp;
>>>> +
>>>> + tmp = readl(&pmc->mckr);
>>>> + tmp&= ~(AT91_PMC_MCKR_PRES_MASK |
>>>> + AT91_PMC_MCKR_MDIV_MASK |
>>>> + AT91_PMC_MCKR_PLLADIV_2);
>>>> + tmp |= mckr& (AT91_PMC_MCKR_PRES_MASK |
>>>> + AT91_PMC_MCKR_MDIV_MASK |
>>>> + AT91_PMC_MCKR_PLLADIV_2);
>>>
>>> Why gets the at91_mck_init() just some parts of the MCK register (some
>>> fields are preserved here) while the at91_plla_init() just rewrites the
>>> PLLA register?
>
> Don't you see the error in my sentence here? ;) I thought some parts of
> the register content where preserved. The other way round is true, we
> need to clean up relevant parts (PRES, MDIV, and PLLADIV Bit(s)). Sorry,
> my fault here.
So, keep it as is(?)
>>> I think it is not much more than hiding the writel() register access. I
>>> think a better API would be to request some specific frequency and we
>>> calculate the register values with that input.
>>> Please let us discuss this.
>>
>> Such a function would be nice, indeed. But I would accept this function,
>> to get SPL code into mainline... (I have the same function ;-)
>
> Yea, no problem with this function. I just wonder why the
> at91_plla_init() is a plain writel() (plus waiting for lock) and this
> one does a bit more (preserve CSS... We could also declare the whole
> register content when calling at91_mckr_init(), don't we?
at91_plla_init()'s main purpose is to wait the lock.
> This is no change request in general, but please lets discuss why we not
> just write the given register content.
This make we focus on what we need to configure.
Best Regards,
Bo Shen
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card
2013-11-14 6:53 ` Bo Shen
@ 2013-11-14 7:49 ` Andreas Bießmann
0 siblings, 0 replies; 21+ messages in thread
From: Andreas Bießmann @ 2013-11-14 7:49 UTC (permalink / raw)
To: u-boot
Hi Bo,
On 11/14/2013 07:53 AM, Bo Shen wrote:
> On 11/14/2013 02:28 PM, Andreas Bie?mann wrote:
>> On 14.11.13 06:52, Heiko Schocher wrote:
>>> Am 13.11.2013 14:34, schrieb Andreas Bie?mann:
>>>> Hi Bo,
>>>>
>>>> On 11/06/2013 06:29 AM, Bo Shen wrote:
>>>>> Enable Atmel sama5d3xek boart spl boot support, which can load u-boot
>>>>> from SD card with FAT file system.
>>>>>
>>>>> Signed-off-by: Bo Shen<voice.shen@atmel.com>
>>>>>
>>>>> ---
>>
>> <snip>
>>
>>>>> +static void at91_disable_wdt(void)
>>>>
>>>> Why should we disable the WDT in SPL? I think it would be better to
>>>> configure a working timer value than just disable it.
>>>
>>> This is currently done in the at91bootstrap code too...
>>
>> I know ...
>>>
>>>> Well it's easy and works, but for the future I think it would be
>>>> good to
>>>> let it run while in SPL and u-boot.
>>>
>>> We should have the option to enable/disable it ...
>
> The watchdog is one time configurable. If configurabled, can not change
> anymore, so we disable it by default.
But we should give an easy way to enable it. I swear industrial users
require this, so we should at least add a possibility to enable it
easily. Take it as a comment, no need to change it for that patch.
>>>>> +void at91_mck_init(u32 mckr)
>>>>> +{
>>>>> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
>>>>> + u32 tmp;
>>>>> +
>>>>> + tmp = readl(&pmc->mckr);
>>>>> + tmp&= ~(AT91_PMC_MCKR_PRES_MASK |
>>>>> + AT91_PMC_MCKR_MDIV_MASK |
>>>>> + AT91_PMC_MCKR_PLLADIV_2);
>>>>> + tmp |= mckr& (AT91_PMC_MCKR_PRES_MASK |
>>>>> + AT91_PMC_MCKR_MDIV_MASK |
>>>>> + AT91_PMC_MCKR_PLLADIV_2);
>>>>
>>>> Why gets the at91_mck_init() just some parts of the MCK register (some
>>>> fields are preserved here) while the at91_plla_init() just rewrites the
>>>> PLLA register?
>>
>> Don't you see the error in my sentence here? ;) I thought some parts of
>> the register content where preserved. The other way round is true, we
>> need to clean up relevant parts (PRES, MDIV, and PLLADIV Bit(s)). Sorry,
>> my fault here.
>
> So, keep it as is(?)
For now yes ... we need to think about a better solution in the future.
Best regards
Andreas Bie?mann
^ permalink raw reply [flat|nested] 21+ messages in thread