[U-Boot] [PATCH 06/10] sunxi: Add a33 dram init code

[Hans de Goede]

Hi,

On 15-04-15 21:56, Ian Campbell wrote:
> On Tue, 2015-04-14 at 18:06 +0200, Hans de Goede wrote:
>> From: Vishnu Patekar <vishnupatekar0510 at gmail.com>
>>
>> Based on Allwinner dram init code from the a33 bsp:
>> https://github.com/allwinner-zh/bootloader/blob/master/basic_loader/bsp/bsp_for_a33/init_dram/mctl_hal.c
>>
>> Initial u-boot port by Vishnu Patekar, major cleanup / rewrite by
>> Hans de Goede.
>>
>> Signed-off-by: Vishnu Patekar <vishnupatekar0510 at gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>
>> +	/* Set dram timing */
>> +	reg_val = (twtp << 24) | (tfaw << 16) | (trasmax << 8) | (tras << 0);
>> +	writel(reg_val, &mctl_ctl->dramtmg0);
>> +	reg_val = (txp << 16) | (trtp << 8) | (trc << 0);
>> +	writel(reg_val, &mctl_ctl->dramtmg1);
>> +	reg_val = (tcwl << 24) | (tcl << 16) | (trd2wr << 8) | (twr2rd << 0);
>> +	writel(reg_val, &mctl_ctl->dramtmg2);
>> +	reg_val = (tmrw << 16) | (tmrd << 12) | (tmod << 0);
>> +	writel(reg_val, &mctl_ctl->dramtmg3);
>> +	reg_val = (trcd << 24) | (tccd << 16) | (trrd << 8) | (trp << 0);
>> +	writel(reg_val, &mctl_ctl->dramtmg4);
>> +	reg_val = (tcksrx << 24) | (tcksre << 16) | (tckesr << 8) | (tcke << 0);
>> +	writel(reg_val, &mctl_ctl->dramtmg5);
>
> There's a lot of magic numbers here (and in the following code),
> although in this particular context (with the named var) unless they are
> the same elsewhere I'm not sure #defines would improve things much, but
> I think some of the other stuff likely would.
>
> Assuming you have any idea what the bits are, I suppose that per usual
> we don't really know because -ENODOC?

Right the problem here is -ENODOC, the magic values come from the allwinnner
code and in the cases where we do not have named variables (as we do in
the above blurb) we've no clue what we're doing really, so I think adding
defines there will only obfuscate things.

Can you give a short description of the cases where you believe that
adding defines would be a good idea ?

Regards,

Hans