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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 12-12-14 21:24, Siarhei Siamashka wrote:
> On Sun, 23 Nov 2014 14:43:14 +0100
> Hans de Goede <hdegoede@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?

I do not really have a private communication channel, as Simos Xenitellis
who has been working on getting beter collaboration between Allwinner and the
linux-sunxi community has mentioned in the
"[linux-sunxi] Introductions and Allwinner documentation update"
thread, kevin at allwinnertech.com and shuge at allwinnertech.com are available to
ask questions we may have and I've been doing that.

Note that I had multiple questions about the sun6i DRAM controller, and
the info in this commit is all I got unfortunately they are not able to
share anything wrt the DRAM controller.

>
> 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.

AFAIK there are no plans to provide any documentation, or any info at all,
as said I had multiple questions and the info in this commit is all the
info I got.

The A23 dram support I'm about to post is also all my own work with no
help from Allwinner.

>
>> 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.

A valid point, unfortunately I only have A31 based top set boxes and those
do not do suspend / resume AFAIK...

But definitely something to keep in mind.

Regards,

Hans


>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Acked-by: Ian Campbell <ijc@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.
>
>