[U-Boot] [PATCH] driver/ddr: Fix DDR4 driver for ARM

[U-Boot] [PATCH] driver/ddr: Fix DDR4 driver for ARM

[York Sun]

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

Signed-off-by: York Sun <yorksun@freescale.com>
---
 arch/arm/include/asm/arch-fsl-lsch3/config.h |    4 ++++
 drivers/ddr/fsl/fsl_ddr_gen4.c               |    9 +++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/include/asm/arch-fsl-lsch3/config.h b/arch/arm/include/asm/arch-fsl-lsch3/config.h
index d61a213..60719fd 100644
--- a/arch/arm/include/asm/arch-fsl-lsch3/config.h
+++ b/arch/arm/include/asm/arch-fsl-lsch3/config.h
@@ -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
diff --git a/drivers/ddr/fsl/fsl_ddr_gen4.c b/drivers/ddr/fsl/fsl_ddr_gen4.c
index 7cd878a..04f0c44 100644
--- a/drivers/ddr/fsl/fsl_ddr_gen4.c
+++ b/drivers/ddr/fsl/fsl_ddr_gen4.c
@@ -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++) {
-- 
1.7.9.5

[U-Boot] [PATCH] driver/ddr: Fix DDR4 driver for ARM

[Jon Loeliger]

On Tue, Jun 17, 2014 at 5:07 PM, York Sun <yorksun@freescale.com> wrote:
> Previously the driver was only tested on Power SoCs. Minor fix is needed
> for ARM SoCs.
>
> Signed-off-by: York Sun <yorksun@freescale.com>


Hi York!


> --- a/drivers/ddr/fsl/fsl_ddr_gen4.c
> +++ b/drivers/ddr/fsl/fsl_ddr_gen4.c

> @@ -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++) {

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

[U-Boot] [PATCH] driver/ddr: Fix DDR4 driver for ARM

[York Sun]

On 06/18/2014 07:57 AM, Jon Loeliger wrote:
> On Tue, Jun 17, 2014 at 5:07 PM, York Sun <yorksun@freescale.com> wrote:
>> Previously the driver was only tested on Power SoCs. Minor fix is needed
>> for ARM SoCs.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
> 
> 
> Hi York!
> 
> 
>> --- a/drivers/ddr/fsl/fsl_ddr_gen4.c
>> +++ b/drivers/ddr/fsl/fsl_ddr_gen4.c
> 
>> @@ -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++) {
> 
> 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

[U-Boot] [PATCH] driver/ddr: Fix DDR4 driver for ARM

[York Sun]

On 06/18/2014 07:57 AM, Jon Loeliger wrote:
> On Tue, Jun 17, 2014 at 5:07 PM, York Sun <yorksun@freescale.com> wrote:
>> Previously the driver was only tested on Power SoCs. Minor fix is needed
>> for ARM SoCs.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
> 
> 
> Hi York!
> 
> 
>> --- a/drivers/ddr/fsl/fsl_ddr_gen4.c
>> +++ b/drivers/ddr/fsl/fsl_ddr_gen4.c
> 
>> @@ -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++) {
> 
> 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

[U-Boot] [PATCH] driver/ddr: Fix DDR4 driver for ARM

[York Sun]

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

On 06/19/2014 11:01 AM, Jon Loeliger wrote:
> 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
> 
> On Thu, Jun 19, 2014 at 12:47 PM, York Sun <yorksun@freescale.com> wrote:
>> On 06/18/2014 07:57 AM, Jon Loeliger wrote:
>>> On Tue, Jun 17, 2014 at 5:07 PM, York Sun <yorksun@freescale.com> wrote:
>>>> Previously the driver was only tested on Power SoCs. Minor fix is needed
>>>> for ARM SoCs.
>>>>
>>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>>
>>>
>>> Hi York!
>>>
>>>
>>>> --- a/drivers/ddr/fsl/fsl_ddr_gen4.c
>>>> +++ b/drivers/ddr/fsl/fsl_ddr_gen4.c
>>>
>>>> @@ -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++) {
>>>
>>> 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
>>