[U-Boot] [PATCH 1/3] fsl/ddr: Revise erratum a009942 and clean related erratum

[york sun]

On 11/08/2016 02:39 AM, Shengzhou Liu wrote:
>
>> -----Original Message-----
>> From: york sun
>> Sent: Tuesday, November 08, 2016 1:04 AM
>> To: Shengzhou Liu <shengzhou.liu at nxp.com>; u-boot at lists.denx.de
>> Subject: Re: [PATCH 1/3] fsl/ddr: Revise erratum a009942 and clean related
>> erratum
>>>
>>> York,
>>>
>>> This change(moving to ctrl_regs.c) has the same effect as read-modify-
>>> write(done in fsl_ddr_gen4.c) before MEM_EN is enabled for DDRC.
>>> As I commented in code with "the POR value of debug_29 register is
>>> zero" for A009942 workaround when moving it to ctrl_regs.c, Actually
>>> only A008378 changes debug_29[8:11] bits to 9 from original POR value 0
>>> before the implementing of A009942, and A009942 overrides debug_29[8:11] set by A008378.
>>> So we can set debug_29 in ctrl_regs.c, it doesn't break anything.
>>
>> Shengzhou,
>>
>> The presumption of POR value is not safe. It is safe for this case for now.
>> You may have other erratum workaround popping up later using the same
>> register, or other registers. PBI can also change registers. This sets an
>> example to preset those registers out the scope of hardware interaction,
>> regardless which controller is in use, or its state.
>>
>
> York
> This change(move to common ctrl_regs.c for reuse on DDR4/DDR3) is only for A009942,
> For other erratum workaround popping up later using the same register, or other registers,
> we still can implement them in fsl_ddr_gen4.c or in other according to actual specific sequence requirement.
> I will add reading value of debug_29 register for A009942 in ctrl_regs.c in next version, which will be safe regardless how POR changes.

You added A009942 and A008378. I don't think this is the right way. You 
break the isolation between calculating the register values and hardware 
interface. If you follow this path, you will add more and more hardware 
interaction into ctrl_regs.c file. Until your change you don't have to 
worry about any hardware condition in this file.

York