[U-Boot] [V3 1/3] sunxi: Fix A33 memory initialization
From: Michael Trimarchi michael@amarulasolutions.com
While the exact problem is not known, based on discussion between Philipp Tomsich and André Przywara it is guessed that exit self-refresh timing is not set with correct value. There may be implicit enter or exit Self-Refresh anywhere as part of some training phase.
In ZynqMP register guide [1], which is close to the various Allwinner DRAM controllers, tXSDLL is bits [14:8], while the non-DLL tXS is bits [6:0]: Self refresh exit delay. So it could be safely increased and it only affects the time after the self-refresh “exit”, which happens only after (re-)initialisation.
There was no document for cpu in question so based on oscilloscope readings [2][3] and observed result by comparing allwinner architecture. So set it same as Allwinner H5 silicon.
Before this patch, failure rate of was 7%.
This was tested on A33 allwinner cpu, dual rank connection connected with two MT41K512M16HA-125:A memory model. Memory is configured as DDR3 1.5V
[1] https://www.xilinx.com/html_docs/registers/ug1087/ddrc___dramtmg8.html [2] https://ibb.co/R70zmyS [3] https://ibb.co/HVVCGQ8
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Shyam Saini shyam.saini@amarulasolutions.com --- Changelogs: V1->V2: adjust commit message V2->V3: Adjust commit message based on V2 Discussion --- arch/arm/mach-sunxi/dram_sun8i_a33.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a33.c b/arch/arm/mach-sunxi/dram_sun8i_a33.c index 1da2727f9874..5da01922bfaf 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_a33.c +++ b/arch/arm/mach-sunxi/dram_sun8i_a33.c @@ -152,7 +152,7 @@ static void auto_set_timing_para(struct dram_para *para) reg_val &= ~(0xff << 8); reg_val &= ~(0xff << 0); reg_val |= (0x33 << 8); - reg_val |= (0x8 << 0); + reg_val |= (0x10 << 0); writel(reg_val, &mctl_ctl->dramtmg8); /* Set phy interface time */ reg_val = (0x2 << 24) | (t_rdata_en << 16) | (0x1 << 8)
From: Michael Trimarchi michael@amarulasolutions.com
Change the size create a glitch in the clken signal on second bank. According to the ddr manual the clken need to be sent accros the reset signal coming the cpu. The rank is calculated just before this function is called and the mctl_set_cr should not change this value anymore
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Shyam Saini shyam.saini@amarulasolutions.com --- Changelogs: V1->V2: Adjust commit description V2->V3: None --- arch/arm/mach-sunxi/dram_sun8i_a33.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a33.c b/arch/arm/mach-sunxi/dram_sun8i_a33.c index 5da01922bfaf..63e18f17d0db 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_a33.c +++ b/arch/arm/mach-sunxi/dram_sun8i_a33.c @@ -45,14 +45,12 @@ static void mctl_set_cr(struct dram_para *para)
static void auto_detect_dram_size(struct dram_para *para) { - u8 orig_rank = para->rank; int rows, columns;
/* Row detect */ para->page_size = 512; para->seq = 1; para->rows = 16; - para->rank = 1; mctl_set_cr(para); for (rows = 11 ; rows < 16 ; rows++) { if (mctl_mem_matches(1 << (rows + 9))) /* row-column */ @@ -69,7 +67,6 @@ static void auto_detect_dram_size(struct dram_para *para) }
para->seq = 0; - para->rank = orig_rank; para->rows = rows; para->page_size = 1 << columns; mctl_set_cr(para);
From: Michael Trimarchi michael@amarulasolutions.com
This will improve code readabilty
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Shyam Saini shyam.saini@amarulasolutions.com --- Changelogs: V1->V2: none V2->V3: Fix use of clrsetbits_le32 and setbits_le32 functions V3->V4: Rebase to original series's patch 2 and 3 --- arch/arm/mach-sunxi/dram_sun8i_a33.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a33.c b/arch/arm/mach-sunxi/dram_sun8i_a33.c index 63e18f17d0db..9fe4c88bd87f 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_a33.c +++ b/arch/arm/mach-sunxi/dram_sun8i_a33.c @@ -145,11 +145,8 @@ static void auto_set_timing_para(struct dram_para *para) reg_val = (tcksrx << 24) | (tcksre << 16) | (tckesr << 8) | (tcke << 0); writel(reg_val, &mctl_ctl->dramtmg5); /* Set two rank timing and exit self-refresh timing */ - reg_val = readl(&mctl_ctl->dramtmg8); - reg_val &= ~(0xff << 8); - reg_val &= ~(0xff << 0); - reg_val |= (0x33 << 8); - reg_val |= (0x10 << 0); + clrsetbits_le32(&mctl_ctl->dramtmg8, (0xff << 8) | (0xff << 0)); + setbits_le32(&mctl_ctl->dramtmg8, (0x33 << 8) | (0x10 << 0)); writel(reg_val, &mctl_ctl->dramtmg8); /* Set phy interface time */ reg_val = (0x2 << 24) | (t_rdata_en << 16) | (0x1 << 8)
On Mon, 18 Mar 2019 15:17:47 +0530 Shyam Saini shyam.saini@amarulasolutions.com wrote:
Hi,
From: Michael Trimarchi michael@amarulasolutions.com
This will improve code readabilty
Somehow this patch looks horribly wrong, it doesn't even compile. See below. Are you sure you sent the right version?
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Shyam Saini shyam.saini@amarulasolutions.com
Changelogs: V1->V2: none V2->V3: Fix use of clrsetbits_le32 and setbits_le32 functions V3->V4: Rebase to original series's patch 2 and 3
arch/arm/mach-sunxi/dram_sun8i_a33.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a33.c b/arch/arm/mach-sunxi/dram_sun8i_a33.c index 63e18f17d0db..9fe4c88bd87f 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_a33.c +++ b/arch/arm/mach-sunxi/dram_sun8i_a33.c @@ -145,11 +145,8 @@ static void auto_set_timing_para(struct dram_para *para) reg_val = (tcksrx << 24) | (tcksre << 16) | (tckesr << 8) | (tcke << 0); writel(reg_val, &mctl_ctl->dramtmg5); /* Set two rank timing and exit self-refresh timing */
- reg_val = readl(&mctl_ctl->dramtmg8);
- reg_val &= ~(0xff << 8);
- reg_val &= ~(0xff << 0);
- reg_val |= (0x33 << 8);
- reg_val |= (0x10 << 0);
- clrsetbits_le32(&mctl_ctl->dramtmg8, (0xff << 8) | (0xff << 0));
- setbits_le32(&mctl_ctl->dramtmg8, (0x33 << 8) | (0x10 << 0));
That should just be one call to clrsetbits_le32().
writel(reg_val, &mctl_ctl->dramtmg8);
And you need to remove this line, of course, otherwise you write the value of dramtmg5 into dramtmg8.
Cheers, Andre.
/* Set phy interface time */ reg_val = (0x2 << 24) | (t_rdata_en << 16) | (0x1 << 8)
Hi
On Mon, Mar 18, 2019 at 4:56 PM Andre Przywara andre.przywara@arm.com wrote:
On Mon, 18 Mar 2019 15:17:47 +0530 Shyam Saini shyam.saini@amarulasolutions.com wrote:
Hi,
From: Michael Trimarchi michael@amarulasolutions.com
This will improve code readabilty
Somehow this patch looks horribly wrong, it doesn't even compile. See below. Are you sure you sent the right version?
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Shyam Saini shyam.saini@amarulasolutions.com
Changelogs: V1->V2: none V2->V3: Fix use of clrsetbits_le32 and setbits_le32 functions V3->V4: Rebase to original series's patch 2 and 3
arch/arm/mach-sunxi/dram_sun8i_a33.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a33.c b/arch/arm/mach-sunxi/dram_sun8i_a33.c index 63e18f17d0db..9fe4c88bd87f 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_a33.c +++ b/arch/arm/mach-sunxi/dram_sun8i_a33.c @@ -145,11 +145,8 @@ static void auto_set_timing_para(struct dram_para *para) reg_val = (tcksrx << 24) | (tcksre << 16) | (tckesr << 8) | (tcke << 0); writel(reg_val, &mctl_ctl->dramtmg5); /* Set two rank timing and exit self-refresh timing */
reg_val = readl(&mctl_ctl->dramtmg8);reg_val &= ~(0xff << 8);reg_val &= ~(0xff << 0);reg_val |= (0x33 << 8);reg_val |= (0x10 << 0);
clrsetbits_le32(&mctl_ctl->dramtmg8, (0xff << 8) | (0xff << 0));setbits_le32(&mctl_ctl->dramtmg8, (0x33 << 8) | (0x10 << 0));That should just be one call to clrsetbits_le32().
writel(reg_val, &mctl_ctl->dramtmg8);And you need to remove this line, of course, otherwise you write the value of dramtmg5 into dramtmg8.
Agree, I prefer the patchset original order and it should work
Thank Andre
Michael
Cheers, Andre.
/* Set phy interface time */ reg_val = (0x2 << 24) | (t_rdata_en << 16) | (0x1 << 8)
Hi Philippe and Andre'
On Mon, Mar 18, 2019 at 10:48 AM Shyam Saini shyam.saini@amarulasolutions.com wrote:
From: Michael Trimarchi michael@amarulasolutions.com
While the exact problem is not known, based on discussion between Philipp Tomsich and André Przywara it is guessed that exit self-refresh timing is not set with correct value. There may be implicit enter or exit Self-Refresh anywhere as part of some training phase.
In ZynqMP register guide [1], which is close to the various Allwinner DRAM controllers, tXSDLL is bits [14:8], while the non-DLL tXS is bits [6:0]: Self refresh exit delay. So it could be safely increased and it only affects the time after the self-refresh “exit”, which happens only after (re-)initialisation.
There was no document for cpu in question so based on oscilloscope readings [2][3] and observed result by comparing allwinner architecture. So set it same as Allwinner H5 silicon.
Before this patch, failure rate of was 7%.
This was tested on A33 allwinner cpu, dual rank connection connected with two MT41K512M16HA-125:A memory model. Memory is configured as DDR3 1.5V
Are you happy with this description?
Michael
[1] https://www.xilinx.com/html_docs/registers/ug1087/ddrc___dramtmg8.html [2] https://ibb.co/R70zmyS [3] https://ibb.co/HVVCGQ8
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Shyam Saini shyam.saini@amarulasolutions.com
Changelogs: V1->V2: adjust commit message V2->V3: Adjust commit message based on V2 Discussion
arch/arm/mach-sunxi/dram_sun8i_a33.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a33.c b/arch/arm/mach-sunxi/dram_sun8i_a33.c index 1da2727f9874..5da01922bfaf 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_a33.c +++ b/arch/arm/mach-sunxi/dram_sun8i_a33.c @@ -152,7 +152,7 @@ static void auto_set_timing_para(struct dram_para *para) reg_val &= ~(0xff << 8); reg_val &= ~(0xff << 0); reg_val |= (0x33 << 8);
reg_val |= (0x8 << 0);
reg_val |= (0x10 << 0); writel(reg_val, &mctl_ctl->dramtmg8); /* Set phy interface time */ reg_val = (0x2 << 24) | (t_rdata_en << 16) | (0x1 << 8)-- 2.11.0
On Mon, Mar 18, 2019 at 3:20 PM Shyam Saini shyam.saini@amarulasolutions.com wrote:
From: Michael Trimarchi michael@amarulasolutions.com
While the exact problem is not known, based on discussion between Philipp Tomsich and André Przywara it is guessed that exit self-refresh timing is not set with correct value. There may be implicit enter or exit Self-Refresh anywhere as part of some training phase.
In ZynqMP register guide [1], which is close to the various Allwinner DRAM controllers, tXSDLL is bits [14:8], while the non-DLL tXS is bits [6:0]: Self refresh exit delay. So it could be safely increased and it only affects the time after the self-refresh “exit”, which happens only after (re-)initialisation.
There was no document for cpu in question so based on oscilloscope readings [2][3] and observed result by comparing allwinner architecture. So set it same as Allwinner H5 silicon.
Before this patch, failure rate of was 7%.
This was tested on A33 allwinner cpu, dual rank connection connected with two MT41K512M16HA-125:A memory model. Memory is configured as DDR3 1.5V
[1] https://www.xilinx.com/html_docs/registers/ug1087/ddrc___dramtmg8.html [2] https://ibb.co/R70zmyS [3] https://ibb.co/HVVCGQ8
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Shyam Saini shyam.saini@amarulasolutions.com
Changelogs: V1->V2: adjust commit message V2->V3: Adjust commit message based on V2 Discussion
Acked-by: Jagan Teki jagan@openedev.com
On Thu, Mar 28, 2019 at 4:55 PM Jagan Teki jagan@amarulasolutions.com wrote:
On Mon, Mar 18, 2019 at 3:20 PM Shyam Saini shyam.saini@amarulasolutions.com wrote:
From: Michael Trimarchi michael@amarulasolutions.com
While the exact problem is not known, based on discussion between Philipp Tomsich and André Przywara it is guessed that exit self-refresh timing is not set with correct value. There may be implicit enter or exit Self-Refresh anywhere as part of some training phase.
In ZynqMP register guide [1], which is close to the various Allwinner DRAM controllers, tXSDLL is bits [14:8], while the non-DLL tXS is bits [6:0]: Self refresh exit delay. So it could be safely increased and it only affects the time after the self-refresh “exit”, which happens only after (re-)initialisation.
There was no document for cpu in question so based on oscilloscope readings [2][3] and observed result by comparing allwinner architecture. So set it same as Allwinner H5 silicon.
Before this patch, failure rate of was 7%.
This was tested on A33 allwinner cpu, dual rank connection connected with two MT41K512M16HA-125:A memory model. Memory is configured as DDR3 1.5V
[1] https://www.xilinx.com/html_docs/registers/ug1087/ddrc___dramtmg8.html [2] https://ibb.co/R70zmyS [3] https://ibb.co/HVVCGQ8
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Shyam Saini shyam.saini@amarulasolutions.com
Changelogs: V1->V2: adjust commit message V2->V3: Adjust commit message based on V2 Discussion
Acked-by: Jagan Teki jagan@openedev.com
Applied to u-boot-sunxi/master
participants (4)
-
Andre Przywara -
Jagan Teki -
Michael Nazzareno Trimarchi -
Shyam Saini