On Tue, 19 May 2015 15:13:18 +0100 Ian Campbell ijc+uboot@hellion.org.uk wrote:
On Tue, 2015-05-19 at 14:56 +0200, Hans de Goede wrote:
diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun4i.c b/arch/arm/cpu/armv7/sunxi/dram_sun4i.c index c736fa3..f7b4915 100644 --- a/arch/arm/cpu/armv7/sunxi/dram_sun4i.c +++ b/arch/arm/cpu/armv7/sunxi/dram_sun4i.c @@ -508,7 +508,7 @@ static void mctl_ddr3_initialize(void) /*
- Perform impedance calibration on the DRAM controller side of the wire.
*/ -static void mctl_set_impedance(u32 zq, u32 odt_en) +static void mctl_set_impedance(u32 zq, bool odt_en) { struct sunxi_dram_reg *dram = (struct sunxi_dram_reg *)SUNXI_DRAMC_BASE; u32 reg_val; @@ -556,7 +556,7 @@ static void mctl_set_impedance(u32 zq, u32 odt_en) clrbits_le32(&dram->zqcr0, DRAM_ZQCR0_ZCAL);
/* Set I/O configure register */
- writel(DRAM_IOCR_ODT_EN(odt_en), &dram->iocr);
- writel(DRAM_IOCR_ODT_EN, &dram->iocr);
I think at this point previously odt_en would always be 0x1 here (0x0 having been short circuited earlier).
In fact, only 0 and 3 were the realistic practical choices. The commit message says that no functional changes are introduced for the existing boards. And this is true because these boards all use 0 by default.
But setting this option to 3 (which enables ODT for both DQ and DQS lines), combined with picking good ZQ settings, can do wonders and significantly improve reliability at high DRAM clock speeds. And this is not just an "armchair expert" opinion :-) The 'highspeedtruck' branch specifically took advantage of this configuration knob: http://lists.denx.de/pipermail/u-boot/2014-July/183981.html If I understand it correctly, you do have a Cubietruck board, right? So if you are really interested, then you should be able to easily verify this yourself.
With a bit of ZQ tuning, I also had DRAM successfully running at 648 MHz on my A13-OLinuXino-MICRO. And Adam Sampson could reach 648 MHz DRAM clock speeds on his two LinkSprite pcDuino3 Nano boards too.
Whereas this...
-#define DRAM_IOCR_ODT_EN(n) ((((n) & 0x3) << 30) | ((n) & 0x3) << 0) -#define DRAM_IOCR_ODT_EN_MASK DRAM_IOCR_ODT_EN(0x3) +#define DRAM_IOCR_ODT_EN ((3 << 30) | (3 << 0))
... now behaves as if it were always 0x3.
AFAICT up until now, at least with the in-tree defconfigs, odt_en was always 0 for sun4i anyway, but this is a surprising change I think.
Do you mean that you want a better description of this change in the commit message?
I myself can only welcome the change of this option to the boolean type, because this way we avoid any need to answer quite a predictable question "why 3 and not 1 or 2?" at: http://linux-sunxi.org/A10_DRAM_Controller_Calibration#Finding_good_impedanc...