[U-Boot] [PATCH v3] sunxi: Make DRAM_ODT_EN Kconfig setting a bool

[Siarhei Siamashka]

On Tue, 19 May 2015 15:13:18 +0100
Ian Campbell <ijc+uboot at 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_impedance_settings

-- 
Best regards,
Siarhei Siamashka