| Message ID | 1416750195-25318-5-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Ian Campbell |
| Headers | show |
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? 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@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.
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@allwinnertech.com and shuge@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. > >
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); writel((MCTL_TDINIT1 << 19) | MCTL_TDINIT0, &mctl_phy->ptr1); writel((MCTL_TDINIT3 << 17) | MCTL_TDINIT2, &mctl_phy->ptr2);