From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Thu, 25 Dec 2014 23:55:07 +0100 Subject: [U-Boot] [PATCH] sunxi: Fix buggy sun6i/sun8i DRAM size detection logic In-Reply-To: <20141225181342.4b29b782@i7> References: <1419440297-29503-1-git-send-email-siarhei.siamashka@gmail.com> <20141224193933.49fe67d4@i7> <20141225040753.3c4df3b6@i7> <549BE37F.1020004@redhat.com> <20141225181342.4b29b782@i7> Message-ID: <549C95CB.6080509@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 25-12-14 17:13, Siarhei Siamashka wrote: > On Thu, 25 Dec 2014 11:14:23 +0100 > Hans de Goede wrote: > >> Hi, >> >> On 25-12-14 03:07, Siarhei Siamashka wrote: >>> On Wed, 24 Dec 2014 19:39:33 +0200 >>> Siarhei Siamashka wrote: >>> >>>> On Wed, 24 Dec 2014 18:58:17 +0200 >>>> Siarhei Siamashka wrote: >>>> >>>>> After reboot, reset or even short power off, DRAM typically retains >>>>> the old stale data for some period of time (for this type of memory, >>>>> the bits of data are stored in slowly discharging capacitors). >>>>> >>>>> The current sun6i/sun8i DRAM size detection logic, which is >>>>> inherited from the Allwinner code, relies on using a large magic >>>>> signature with the hope that it is unique enough and unlikely to >>>>> ever accidentally match this leftover garbage data in RAM. But >>>>> this approach is inherently unsafe, as can be demonstrated using >>>>> the following test program: >>>>> >>>>> /***** A testcase for reproducing the problem ******/ >>>>> #include >>>>> #include >>>>> #include >>>>> >>>>> void main(int argc, char *argv[]) >>>>> { >>>>> size_t size, i; >>>>> uint32_t *buf; >>>>> /* Allocate the buffer */ >>>>> if (argc < 2 || !(size = (size_t)atoi(argv[1]) * 1048576) || >>>>> !(buf = malloc(size))) { >>>>> printf("Need buffer size in MiB as a cmdline argument\n"); >>>>> exit(1); >>>>> } >>>>> /* Fill it with the Allwinner DRAM "magic" values */ >>>>> for (i = 0; i < size / 4; i++) >>>>> buf[i] = 0xaa55aa55 + ((uintptr_t)&buf[i] / 4) % 64; >>>>> /* Try to reboot */ >>>>> system("reboot"); >>>>> /* And wait */ >>>>> for (;;) {} >>>>> } >>>>> /***************************************************/ >>>>> >>>>> If this test program is run on the device (giving it a large >>>>> chunk of memory), then the DRAM size detection logic in u-boot >>>>> gets confused after reboot and fails to initialize DRAM properly. >>>>> >>>>> A better approach is not to rely on luck and abstain from making >>>>> any assumptions about the properties of the leftover garbage >>>>> data in RAM. Instead just use a more reliable code for testing >>>>> whether two different addresses refer to the same memory location. >>>>> >>>>> Signed-off-by: Siarhei Siamashka >> >> Thanks for the patch, it looks good to me, I'll give it a test on my >> own sun6i and sun8i boards when I find some time to do so. > > Thanks. > >>>>> --- >>>>> >>>>> Done only very limited testing on MSI Primo81 tablet (Allwinner A31s), >>>>> which is currently a rather impractical device for doing any sun6i code >>>>> development due to having no access to the serial console, USB or other >>>>> convenient ways to interact with the device. >>> >>> Got a serial console on my tablet via a MicroSD breakout board. So I'm >>> retracting this statement :-) >> >> Nice. Note that my personal u-boot git repo has lcd support in the sunxi-wip >> branch, so that we can have at least u-boot output messages on tablets, but >> I guess that FEL + console via microsd breakout works too. > > HDMI works too. Too bad that neither LCD nor HDMI can handle input. But > at least the UART console can. I was hoping to just get WLAN working as > the next step in order to use it instead of the UART console for > communicating with the device. > > The MSI Primo81 is quite a nice tablet: > * has a HDMI connector (can run a real desktop Linux system on a HDMI > monitor, that is if we get USB OTG working) > * has access to the hardware FEL button (this makes the device truly > unbrickable) > * has a sturdy slim metal frame, but a downside is that it may be > more difficult/dangerous to disassemble > * has an IPS LCD display with good colors and viewing angles but > unfortunately relatively low resolution (1024x768) > * The current price seems to be around or under 100 EUR on amazon.de > and other places. > >> If you can point me to a fex file for your board I can add a defconfig for >> your tablet with hopefully working LCD support. > > If you really insist on doing this yourself, then you can find the fex > file in the sunxi-boards git repository: > https://github.com/linux-sunxi/sunxi-boards/blob/master/sys_config/a31s/msi_primo81.fex > > In any case, this tablet is not going to be really usable until the > mainline kernel gets USB OTG support. Which gives us some weeks/months > to take care of the necessary dts/defconfig files. Ack, I offered to create the defconfig since the alternative was to write a short howto on the VIDEO_LCD_MODE Kconfig, but since I've done that now anyways I'll happily wait for you to provide a defconfig. >>> And indeed, the DRAM parameters get incorrectly detected after running >>> the test program (the system fails to boot later on): >> >> Bummer, does this happen with both the old and the new memcmp functions ? > > Everything works fine after the "sunxi: Fix buggy sun6i/sun8i DRAM size > detection logic" patch is applied. Sorry for not stating this clearly > enough. Ah, ok, that is good to know. > Without the patch and after running the test program, I just see that > the system fails to boot. Typically with the HDMI not coming up at all. > The UART console allows to see why it fails and makes debugging > possible. Ack. >> I'll send you a private mail with a statically linked util I've written >> called mmio-dump, which can dump any hardware address from within android, >> assuming you've an adb shell as root on your tablet, you can then copy >> this to /system/bin and use it to dump the dram controller registers, >> compare with what u-boot selects and see where we're getting things wrong. >> >> Usage is e.g.: >> >> mmio-dump 0x01c62000 64 > > We are not getting anything that behaves obviously wrong, so I would > postpone this check for a later date. > > I had root on this tablet for a short while (just for extracting fex), > but not anymore. Gaining root in Android is a pain on this tablet. And > one rooting package that works is flagged as a trojan on virustotal.com > (maybe a bogus report, but I don't want to take any risks). Since I'm > actually using this tablet as an Android tablet, trying to root it > again only for running mmio-dump would cause way too much disruption > (backing up and restoring everything). > > But I'm glad that you are actually using mmio dumping yourself. You did > not sound like a big fan of it earlier, this had me seriously worried: > http://lists.denx.de/pipermail/u-boot/2014-December/199429.html Oh, no I do use mmio dumping frequently to check various things, esp. to compare the dram setup between what boot0 / u-boot does. I just do not see much use in a meminfo like tool for a31 / a23 for end users, since for double checking all register matter, and for a simple bringup the info in the fex is enough. Regards, Hans