Skip to contentSkip to conversation

driver/ddr: Fix DDR4 driver for ARM

PATCH New York Sun 2 participants 5 messages Jun 17, 2014 – Jun 19, 2014 1 message not in the archive Arm (32-bit)

lore
Share & tools
lore

York Sun

author
#0 lore raw

PATCH 1/1 driver/ddr: Fix DDR4 driver for ARM

Previously the driver was only tested on Power SoCs. Minor fix is needed for ARM SoCs.

Signed-off-by York Sun <[email protected]>
---
 arch/arm/include/asm/arch-fsl-lsch3/config.h |    4 ++++
 drivers/ddr/fsl/fsl_ddr_gen4.c               |    9 +++++++++
 2 files changed, 13 insertions(+)
arch/arm/include/asm/arch-fsl-lsch3/config.h +4 -0
index d61a213..60719fd 100644
@@ -71,7 +71,11 @@
 /* DDR */
 #define CONFIG_SYS_FSL_DDR_LE
 #define CONFIG_VERY_BIG_RAM
+#ifdef CONFIG_SYS_FSL_DDR4
+#define CONFIG_SYS_FSL_DDRC_GEN4
+#else
 #define CONFIG_SYS_FSL_DDRC_ARM_GEN3	/* Enable Freescale ARM DDR3 driver */
+#endif
 #define CONFIG_SYS_FSL_DDR		/* Freescale DDR driver */
 #define CONFIG_SYS_LS2_DDR_BLOCK1_SIZE	((phys_size_t)2 << 30)
 #define CONFIG_MAX_MEM_MAPPED		CONFIG_SYS_LS2_DDR_BLOCK1_SIZE
drivers/ddr/fsl/fsl_ddr_gen4.c +9 -0
index 7cd878a..04f0c44 100644
@@ -8,6 +8,7 @@
 #include <asm/io.h>
 #include <fsl_ddr_sdram.h>
 #include <asm/processor.h>
+#include <fsl_immap.h>
 #include <fsl_ddr.h>
 
 #if (CONFIG_CHIP_SELECTS_PER_CTRL > 4)

@@ -183,12 +184,20 @@ step2:
 	 * we choose the max, that is 500 us for all of case.
 	 */
 	udelay(500);
+#ifdef CONFIG_PPC
 	asm volatile("sync;isync");
+#else
+	asm volatile("dsb sy;isb");
+#endif
 
 	/* Let the controller go */
 	temp_sdram_cfg = ddr_in32(&ddr->sdram_cfg) & ~SDRAM_CFG_BI;
 	ddr_out32(&ddr->sdram_cfg, temp_sdram_cfg | SDRAM_CFG_MEM_EN);
+#ifdef CONFIG_PPC
 	asm volatile("sync;isync");
+#else
+	asm volatile("dsb sy;isb");
+#endif
 
 	total_gb_size_per_controller = 0;
 	for (i = 0; i < CONFIG_CHIP_SELECTS_PER_CTRL; i++) {
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 driver/ddr: Fix DDR4 driver for ARM

Quoted 4 lines · York Sun

Previously the driver was only tested on Power SoCs. Minor fix is needed for ARM SoCs.

Signed-off-by York Sun <[email protected]>

Hi York!

Quoted 14 lines
drivers/ddr/fsl/fsl_ddr_gen4.c +4 -0
@@ -183,12 +184,20 @@ step2:
         * we choose the max, that is 500 us for all of case.
         */
        udelay(500);
+#ifdef CONFIG_PPC
        asm volatile("sync;isync");
+#else
+       asm volatile("dsb sy;isb");
+#endif
 
        /* Let the controller go */

drivers/ddr/fsl/fsl_ddr_gen4.c step2: · line 203

Quoted 10 lines
drivers/ddr/fsl/fsl_ddr_gen4.c +4 -0
        temp_sdram_cfg = ddr_in32(&ddr->sdram_cfg) & ~SDRAM_CFG_BI;
        ddr_out32(&ddr->sdram_cfg, temp_sdram_cfg | SDRAM_CFG_MEM_EN);
+#ifdef CONFIG_PPC
        asm volatile("sync;isync");
+#else
+       asm volatile("dsb sy;isb");
+#endif
 
        total_gb_size_per_controller = 0;
        for (i = 0; i < CONFIG_CHIP_SELECTS_PER_CTRL; i++) {

This is a great example where we should try to introduce better abstractions in much the same way that Linux has. Specifically, we (U-Boot) collective might work toward some common lower-level abstractions such as a memory_barrier() (and variants), and let those generic names get mapped into architecture-specific implementations via a linked binding. Then this code would not need to change, nor would #ifdefs be needed.

HTH,
jdl

author
#2 lore raw

PATCH 1/1 driver/ddr: Fix DDR4 driver for ARM

Quoted 32 lines · Jon Loeliger
Quoted 4 lines · York Sun

Previously the driver was only tested on Power SoCs. Minor fix is needed for ARM SoCs.

Signed-off-by York Sun <[email protected]>

Hi York!

Quoted 22 lines
drivers/ddr/fsl/fsl_ddr_gen4.c +8 -0
@@ -183,12 +184,20 @@ step2:
         * we choose the max, that is 500 us for all of case.
         */
        udelay(500);
+#ifdef CONFIG_PPC
        asm volatile("sync;isync");
+#else
+       asm volatile("dsb sy;isb");
+#endif
 
        /* Let the controller go */
        temp_sdram_cfg = ddr_in32(&ddr->sdram_cfg) & ~SDRAM_CFG_BI;
        ddr_out32(&ddr->sdram_cfg, temp_sdram_cfg | SDRAM_CFG_MEM_EN);
+#ifdef CONFIG_PPC
        asm volatile("sync;isync");
+#else
+       asm volatile("dsb sy;isb");
+#endif
Quoted 10 lines

drivers/ddr/fsl/fsl_ddr_gen4.c step2: · line 203

Quoted 2 lines
drivers/ddr/fsl/fsl_ddr_gen4.c +0 -0
        total_gb_size_per_controller = 0;
        for (i = 0; i < CONFIG_CHIP_SELECTS_PER_CTRL; i++) {

This is a great example where we should try to introduce better abstractions in much the same way that Linux has. Specifically, we (U-Boot) collective might work toward some common lower-level abstractions such as a memory_barrier() (and variants), and let those generic names get mapped into architecture-specific implementations via a linked binding. Then this code would not need to change, nor would #ifdefs be needed.

Agreed. I will work on that.

York

author
#3 lore raw

PATCH 1/1 driver/ddr: Fix DDR4 driver for ARM

In reply to Jon Loeliger
Quoted 32 lines · Jon Loeliger
Quoted 4 lines · York Sun

Previously the driver was only tested on Power SoCs. Minor fix is needed for ARM SoCs.

Signed-off-by York Sun <[email protected]>

Hi York!

Quoted 22 lines
drivers/ddr/fsl/fsl_ddr_gen4.c +8 -0
@@ -183,12 +184,20 @@ step2:
         * we choose the max, that is 500 us for all of case.
         */
        udelay(500);
+#ifdef CONFIG_PPC
        asm volatile("sync;isync");
+#else
+       asm volatile("dsb sy;isb");
+#endif
 
        /* Let the controller go */
        temp_sdram_cfg = ddr_in32(&ddr->sdram_cfg) & ~SDRAM_CFG_BI;
        ddr_out32(&ddr->sdram_cfg, temp_sdram_cfg | SDRAM_CFG_MEM_EN);
+#ifdef CONFIG_PPC
        asm volatile("sync;isync");
+#else
+       asm volatile("dsb sy;isb");
+#endif
Quoted 10 lines

drivers/ddr/fsl/fsl_ddr_gen4.c step2: · line 203

Quoted 2 lines
drivers/ddr/fsl/fsl_ddr_gen4.c +0 -0
        total_gb_size_per_controller = 0;
        for (i = 0; i < CONFIG_CHIP_SELECTS_PER_CTRL; i++) {

This is a great example where we should try to introduce better abstractions in much the same way that Linux has. Specifically, we (U-Boot) collective might work toward some common lower-level abstractions such as a memory_barrier() (and variants), and let those generic names get mapped into architecture-specific implementations via a linked binding. Then this code would not need to change, nor would #ifdefs be needed.

Jon,

Are you suggesting to pick arch/<arch>/include/asm/barrier.h from Linux, or part of it?

York

A message that is not in the archive (sent off-list or not yet received): CAJgR-BgQuqas549fLbO8_bzsRa+h5S4_4SLvcin4F_hqMC13xg@mail.gmail.com

author
#5 lore raw

PATCH 1/1 driver/ddr: Fix DDR4 driver for ARM

Noted. Maybe we can pull in part of barrier.h into io.h. It is easier but I am not sure if it is the cleanest way to go.

York

Quoted 60 lines · Jon Loeliger

That might be a fine starting point. But I might point out that U-Boot has some notions down this line already too. For example, do a 'git grep wmb' in the U-Boot repo. Also ponder arch/arm/include/asm/io.h too.

HTH,
jdl

Quoted 51 lines · York Sun
Quoted 32 lines · Jon Loeliger
Quoted 4 lines · York Sun

Previously the driver was only tested on Power SoCs. Minor fix is needed for ARM SoCs.

Signed-off-by York Sun <[email protected]>

Hi York!

Quoted 22 lines
drivers/ddr/fsl/fsl_ddr_gen4.c +8 -0
@@ -183,12 +184,20 @@ step2:
         * we choose the max, that is 500 us for all of case.
         */
        udelay(500);
+#ifdef CONFIG_PPC
        asm volatile("sync;isync");
+#else
+       asm volatile("dsb sy;isb");
+#endif
 
        /* Let the controller go */
        temp_sdram_cfg = ddr_in32(&ddr->sdram_cfg) & ~SDRAM_CFG_BI;
        ddr_out32(&ddr->sdram_cfg, temp_sdram_cfg | SDRAM_CFG_MEM_EN);
+#ifdef CONFIG_PPC
        asm volatile("sync;isync");
+#else
+       asm volatile("dsb sy;isb");
+#endif
Quoted 10 lines

drivers/ddr/fsl/fsl_ddr_gen4.c step2: · line 203

Quoted 2 lines
drivers/ddr/fsl/fsl_ddr_gen4.c +0 -0
        total_gb_size_per_controller = 0;
        for (i = 0; i < CONFIG_CHIP_SELECTS_PER_CTRL; i++) {

This is a great example where we should try to introduce better abstractions in much the same way that Linux has. Specifically, we (U-Boot) collective might work toward some common lower-level abstractions such as a memory_barrier() (and variants), and let those generic names get mapped into architecture-specific implementations via a linked binding. Then this code would not need to change, nor would #ifdefs be needed.

Jon,

Are you suggesting to pick arch/<arch>/include/asm/barrier.h from Linux, or part of it?

York