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

[York Sun]

On 11/11/2013 10:49 AM, Scott Wood wrote:
> On Fri, 2013-11-08 at 20:25 -0600, sun york-R58495 wrote:
>> 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.
> 
> My point is that the type of DDR that is present is hardware
> description, just as much as the type of DDR controller.  "User config"
> means things users can choose purely in software, without a
> corresponding hardware change.
> 

This is a board-level configuration, not SoC level. Should we use
CONFIG_SYS_ as well? If so, we need to come up with a new name, such as
CONFIG_SYS_FSL_DDR_MEM_GEN3.

York