From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Date: Mon, 19 Dec 2016 10:53:32 +0000 Subject: [U-Boot] [PATCH v3 16/26] sunxi: H3: add DRAM controller single bit delay support In-Reply-To: <20161219095739.flo5qlfoecthu5db@lukather> References: <1482112216-12983-1-git-send-email-andre.przywara@arm.com> <1482112216-12983-17-git-send-email-andre.przywara@arm.com> <20161219095739.flo5qlfoecthu5db@lukather> Message-ID: <5efeb267-2f26-8dd3-5763-3fb9b9c8cdd8@arm.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 19/12/16 09:57, Maxime Ripard wrote: > On Mon, Dec 19, 2016 at 01:50:06AM +0000, Andre Przywara wrote: >> From: Jens Kuske >> >> So far the DRAM driver for the H3 SoC (and apparently boot0/libdram as >> well) only applied coarse delay line settings, with one delay value for >> all the data lines in each byte lane and one value for the control lines. >> >> Instead of setting the delays for whole bytes only allow setting it for >> each individual bit. Also add support for address/command lane delays. >> >> For the purpose of this patch the rules for the existing coarse settings >> were just applied to the new scheme, so the actual register writes don't >> change for the H3. Other SoCs will utilize this feature later properly. >> >> With a stock GCC 5.3.0 this increases the dram_sun8i_h3.o code size from >> 2296 to 2344 Bytes. >> >> [Andre: move delay parameters into macros to ease later sharing, use >> defines for numbers of delay registers, extend commit message] >> >> Signed-off-by: Jens Kuske >> Signed-off-by: Andre Przywara > > I said it earlier, but some comments on these new fields would really > be welcome to document the structure and what values they're supposed > to hold. I guess you know as much as I do on this topic. Apparently there are delays to compensate for differing trace lengths on the PCB, one for every bit line. On the 32-bit DRAM controller these are grouped in four groups of 8 bits each (hence byte lane). Please keep in mind that there is no easily available documentation, some parts are just copying what boot0/libdram does. As we don't know the exact meaning of these fields, I prefer to not add any guesses here. I was hoping that the defines I added in this version would shed some light on this? I am pretty sure of their meaning, but that's as far as it goes. For instance I have no idea what the units are, for cycles they are quite big. So frankly I don't really know what to add here still. I can elaborate on how to get these actual values from an existing boot0/libdram, but that would be more of a documentation patch than actual code. Cheers, Andre.