On 11/08/2016 09:03 AM, york....@nxp.com wrote: > 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....@nxp.com>; u-boot@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. >
Another thought, if you are concerned about reusing the code for workaround, how about you put the common code into util.c, or even add an errata.c file? York _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
