Hi Simon,
Thanks for your review, I would like to take all the comments but one below, because I think the change only get a lot of work to do but have no any help on understand the source code, because each of shift/mask operation has comment for it.
I have spend a lot of time on make different controller base addresses from dts and gather them into one structure, which we think it does not make any helpful on source code itself but only met some bug on of-platdata for address decode.
There are hundreds of ctrl/pi/phy register in this dram controller, I don't think it's a good idea to add MASK/SHIFT MACRO definition for all of them since comments are already there.
I will send my patch V2 without mask/shift change, but updates by your other comments.
Thanks, - Kever
On 01/13/2017 10:18 AM, Simon Glass wrote:
+{
u32 *denali_phy = ddr_publ_regs->denali_phy;if (freq <= 125*MHz) {/* phy_sw_master_mode_X PHY_86/214/342/470 4bits offset_8 */setbits_le32(&denali_phy[86], (0x3 << 2) << 8);setbits_le32(&denali_phy[214], (0x3 << 2) << 8);setbits_le32(&denali_phy[342], (0x3 << 2) << 8);setbits_le32(&denali_phy[470], (0x3 << 2) << 8);These should really be defined as SHIFT and MASK values.