Hi,
On 04/16/2015 11:09 AM, Ian Campbell wrote:
On Thu, 2015-04-16 at 09:27 +0200, Hans de Goede wrote:
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@gmail.com
Based on Allwinner dram init code from the a33 bsp: https://github.com/allwinner-zh/bootloader/blob/master/basic_loader/bsp/bsp_...
Initial u-boot port by Vishnu Patekar, major cleanup / rewrite by Hans de Goede.
Signed-off-by: Vishnu Patekar vishnupatekar0510@gmail.com Signed-off-by: Hans de Goede hdegoede@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 ?
Anywhere where we know the meanings ;-) If that's nowhere then the magic numbers are fine (which is what my final paragraph was trying to say, but not very clearly).
Ok, so I've gone over the entire dram init code another time, and I've not been able to find a single place where I can add a define with a meaningful name to replace the magic values.
So no v2 for this patch, can I get an ack for v1 ?
Regards,
Hans