On Fri, 19 Dec 2014 18:05:49 +0100 Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 19-12-14 11:20, Siarhei Siamashka wrote:
On Tue, 16 Dec 2014 21:31:38 +0100 Hans de Goede hdegoede@redhat.com wrote:
Based on the register / dram_para headers from the Allwinner u-boot / linux sources + the init sequences from boot0.
Can the commit message have more detailed information about the precise location of the original Allwinner code, which was used as the reference?
No, it cannot because no code was referenced, I traced the boot0 binary to figure out what was needed to get dram going on the A23.
Thanks for explaining. And good job extracting all the information from there.
In this case, can we have a clear reference to this binary in the commit message? It might be useful to have a look at objdump logs when reviewing your patch.
[...]
+++ b/arch/arm/cpu/armv7/sunxi/dram_sun8i.c @@ -0,0 +1,340 @@ +/*
- Sun8i platform dram controller init.
- (C) Copyright 2014 Hans de Goede hdegoede@redhat.com
Is Allwinner copyright really not necessary here?
- SPDX-License-Identifier: GPL-2.0+
- */
+/*
- Note this code uses a lot of magic hex values, that is because this code
- simply replays the init sequence as done by the Allwinner boot0 code, so
- we do not know what these values mean. There are no symbolic constants for
- these magic values, since we do not know how to name them and making up
- names for them is not useful.
- */
You are well aware of this documentation since a long time ago, right? http://www.ti.com/lit/ug/spruhn7a/spruhn7a.pdf
I'm aware you've used various sources to figure out more about the A10 dram controller, since this DRAM controller seems significantly different I assumed your sources would not apply, it is good to hear that they do.
I wonder how could you have ended up with this strange assumption? When I clearly told you that "It looks like the Allwinner A31 DRAM controller registers are very similar to what is used in RK3288" and provided the links to the relevant sources and documentation back in September: http://lists.denx.de/pipermail/u-boot/2014-September/189199.html
Then reminded yet again about this RK3288 similarity and used the mctl_phy->ptr0 register as an example: http://lists.denx.de/pipermail/u-boot/2014-December/198582.html
When we were discussing the A31 DRAM controller, of course you should have expected the information about A31.
Yes, A23 was not a part of the picture at that time. However comparing its registers with the information from spruhn7a.pdf and various Rockchip source code drops is a very natural thing to try.
So far the best matching pairs (almost 100% identical PHY) are: Allwinner A10 (sun4i) - RK29xx Allwinner A31 (sun6i) - RK3288 Allwinner A23 (sun8i) - TI Keystone2
It's all the same family of DRAM controllers, very likely just different revisions/generations from the same supplier. I don't expect A80 or other Allwinner chips to be any different.
[...]
- else
writel(0x1000040b, &mctl_phy->dcr);The bit 28 is marked as reserved and "Reads return zeros" in the TI Keystone2 documentation. It's a nice chance to test whether this write has any effect and whether it does modify the register.
It modifies the register, so that means that at least that bit does not match the TI Keystone2 documentation.
OK.