[U-Boot] [Patch v1 1/6] Driver/DDR: Moving Freescale DDR driver to a common driver

[york sun]

On Nov 8, 2013, at 4:48 PM, Scott Wood wrote:

> On Wed, 2013-10-30 at 19:07 -0700, York Sun wrote:
>> 
>> +		CONFIG_SYS_FSL_DDR
>> +		Freescale DDR driver in use. This type of DDR controller is
>> +		found in mpc83xx, mpc85xx, mpc86xx as well as some ARM core
>> +		SoCs.
>> +
>> +		CONFIG_SYS_FSL_DDR_ADDR
>> +		Freescale DDR memory-mapped register base.
>> +
>> +		CONFIG_SYS_FSL_DDR_EMU
>> +		Specify emulator support for DDR. Some DDR features such as
>> +		deskew training are not available.
>> +
>> +		CONFIG_SYS_FSL_DDR_PPC_GEN1
>> +		Freescale DDR1 controller.
>> +
>> +		CONFIG_SYS_FSL_DDR_PPC_GEN2
>> +		Freescale DDR2 controller.
>> +
>> +		CONFIG_SYS_FSL_DDR_PPC_GEN3
>> +		Freescale DDR3 controller.
> 
> Why is there "PPC" in the name if this is to be common for PPC and ARM?
> The description is more generic than the config symbol...

This patch moves the DDR driver out of powerpc. I should deal with the common part for ARM and PPC later. Good point.

> 
>> +		CONFIG_FSL_DDR1
>> +		User config to use DDR1. It can be enabled for SoCs with
>> +		Freescale DDR1 or DDR2 controllers.
>> +
>> +		CONFIG_FSL_DDR2
>> +		User config to use DDR2. It can be eanbeld for SoCs with
>> +		Freescale DDR2 or DDR3 controllers.
>> +
>> +		CONFIG_FSL_DDR3
>> +		User config to use DDR3. It can be enabled for SoCs with
>> +		Freescale DDR3 controllers.
> 
> How is this user config, rather than a description of the type of DDR
> that is present?

The DDR controller may support more than one type of memory. DDR2 controllers support both DDR1 and DDR2, and some DDR3 controllers support both DDR2 and DDR3. It's user's option to choose which type of DDR devices to use. The driver needs to deal with them differently. You may argue the driver should detect them and choose the algorithm differently. But that will increase the code size considerably. It makes less sense to do so because once the board is designed, it is either this or that.

> 
>> diff --git a/arch/powerpc/include/asm/config_mpc86xx.h b/arch/powerpc/include/asm/config_mpc86xx.h
>> index 694b110..4f9b225 100644
>> --- a/arch/powerpc/include/asm/config_mpc86xx.h
>> +++ b/arch/powerpc/include/asm/config_mpc86xx.h
>> @@ -7,6 +7,8 @@
>> #ifndef _ASM_MPC86xx_CONFIG_H_
>> #define _ASM_MPC86xx_CONFIG_H_
>> 
>> +#define CONFIG_SYS_FSL_DDR_86XX
> 
> This doesn't qualify as one of the "PPC_GEN" controllers?
> 
The common part is shared for all controllers. The final writing register function is different. I didn't try to merge 86xx with 85xx, because 85xx is messy, with a lot of workaround. And most importantly, 86xx driver stay with DDR2 while 85xx driver mainly supports DDR3.

York