[U-Boot] [PATCH v2 4/5] sun6i: Drop some "unknown magic" from dram init

[Siarhei Siamashka]

On Sun, 23 Nov 2014 14:43:14 +0100
Hans de Goede <hdegoede at redhat.com> wrote:

Sorry for a late reply. I was not on CC for these patches and don't
properly keep track of the u-boot mailing list activity lately.

> Allwinner tells us that this bit of code is the rtc ram being used to detect
> coming out of "super-standby" mode, and if that is the case, going out of
> self-refresh mode.

If I understand this paragraph correctly, you seem to have a privileged
communication channel with Allwinner. And I wonder if you are keeping
track of this information somehow and willing to share it with the
others?

For sun4i/sun5i/sun7i DRAM controller, we actively used a wiki page:
    http://linux-sunxi.org/A10_DRAM_Controller_Register_Guide
Basically, whenever something new is found about some hardware
register, just do a quick edit to make sure that this information
is not forgotten or lost. This does not really take much time.
Trying to remember something later and searching it in the scattered
old e-mails is usually a bigger waste of time.

The most interesting question is of course whether Allwinner has any
plans to provide real DRAM controller documentation. If they do it,
then we don't have to waste time on finding and documenting all the
relevant information.

> Since we do not support "super-standby" mode, this can be dropped.

This patch seems to have a similar purpose to what we did for
sun4i/sun5i/sun7i dram controller earlier:
    http://git.denx.de/?p=u-boot.git;a=commitdiff;h=f2577967738f923571b7156ad46ef91d9fa8d9f8

A somewhat tricky part about this stuff is that while we don't support
the "super-standby" mode, we still have to somehow deal with it whenever
we actually encounter it in the wild. It is easy to reproduce if you
have an Allwinner tablet with Android firmware (just disable WLAN, let
it sleep for a while, insert an SD card with u-boot and GNU/Linux, try
to press the power button to wake the device from sleep).

For example, on sun5i/sun7i hardware, doing "writel(0x16510000,
&dram->ppwrsctl)" part is important if we want to really discard the
RAM data and boot normally. A failure to do so just transitions the
device into and unbootable state. Moreover, the device may look as
"bricked" to the end user until the RTC battery runs out of juice or
some special action is taken. I think that this scenario is exactly
what was described in the NOTICE section:
    http://cubieboard.org/2014/01/13/upgrade-new-android-for-cubietruckv1-01/

I'm not sure if all the same applies to A31 hardware (does having a
dedicated OpenRISC power management chip change anything?), but just
chopping off the code and hoping for the best might be not the best
action.

> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> Acked-by: Ian Campbell <ijc at hellion.org.uk>
> ---
>  arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c
> index 2ac0b58..5e163cb 100644
> --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c
> +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c
> @@ -140,9 +140,6 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
>  
>  	writel((MCTL_TITMSRST << 18) | (MCTL_TDLLLOCK << 6) | MCTL_TDLLSRST,
>  	       &mctl_phy->ptr0);
> -	/* Unknown magic performed by boot0 */
> -	if ((readl(SUNXI_RTC_BASE + 0x20c) & 3) == 2)
> -		setbits_le32(&mctl_phy->ptr0, 1 << 18);

Regarding the "unknown magic". In fact we already have quite a lot of
information about various obscure parts of the Allwinner DRAM code:
    http://lists.denx.de/pipermail/u-boot/2014-September/189199.html

For example, for this PTR0 PHY register, we have the following
information from the RK3288 header files:

/* PTR0 */
#define tITMSRST(n)          ((n)<<18)
#define tDLLLOCK(n)          ((n)<<6)
#define tDLLSRST(n)          ((n)<<0)

Having the bit fields as named identifiers instead of Allwinner magic
numbers makes the code more readable.

And we also have the "4.41 PHY Timing Register 0 (PTR0)" section in
http://www.ti.com/lit/ug/spruhn7a/spruhn7a.pdf for some general
explanations.

The RK3288 is almost a perfect match, because all the hardware register
addresses and the register names are nearly identical. The TI Keystone2
is a lot more distant relative, so the information in its documentation
is less trustworthy for us.

The actual functionality of these bits still needs to be confirmed in
an experimental way (instead of just making theoretical assumptions and
hoping for the best). But again, wasting time on doing this only makes
sense if Allwinner in fact has no plans to release proper documentation
for the DRAM controllers used in their SoCs.
 
-- 
Best regards,
Siarhei Siamashka