sunxi: Make DRAM_ODT_EN Kconfig setting a bool

PATCH Superseded Hans de Goede 3 participants 7 messages May 19, 2015

Subsystems (2)

Arm (32-bit) Arm/Allwinner sunXi SoC support

lore

Share & tools

lore

Hans de Goede

author

2015-05-19 12:56 UTC #0 lore raw

PATCH v3 1/1 sunxi: Make DRAM_ODT_EN Kconfig setting a bool

Make DRAM_ODT_EN Kconfig setting a bool, add a separate DRAM_ODT_CORRECTION setting for A23 SoCs and use DRAM_ODT_EN Kconfig everywhere instead of only in dram_sun4i.c and hardcoding odt_en elsewhere.

Note this commit makes no functional changes for existing boards, its purpose is to allow changing the odt_en value on future A33 boards.

Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:
-Use existing DRAM_ODT_EN Kconfig block moving it out of the
#if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I block it was in
Changes in v3:
-Complete rewrite of the patch

---
 arch/arm/cpu/armv7/sunxi/dram_sun4i.c            |  4 ++--
 arch/arm/cpu/armv7/sunxi/dram_sun8i_a23.c        | 19 +++++++++----------
 arch/arm/cpu/armv7/sunxi/dram_sun8i_a33.c        |  4 ++--
 arch/arm/include/asm/arch-sunxi/dram_sun4i.h     |  3 +--
 arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h |  1 +
 board/sunxi/Kconfig                              | 22 +++++++++++++++-------
 board/sunxi/dram_sun4i_auto.c                    |  3 ++-
 board/sunxi/dram_sun5i_auto.c                    |  3 ++-
 8 files changed, 34 insertions(+), 25 deletions(-)

arch/arm/cpu/armv7/sunxi/dram_sun4i.c +2 -2

index c736fa3..f7b4915 100644
@@ -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);
 }
 
 static unsigned long dramc_init_helper(struct dram_para *para)

arch/arm/cpu/armv7/sunxi/dram_sun8i_a23.c +9 ~~-10~~

index 3d7964d..165c052 100644
@@ -26,12 +26,14 @@
 #include <asm/arch/clock.h>
 #include <asm/arch/dram.h>
 #include <asm/arch/prcm.h>
+#include <linux/kconfig.h>
 
 static const struct dram_para dram_para = {
 	.clock = CONFIG_DRAM_CLK,
 	.type = 3,
 	.zq = CONFIG_DRAM_ZQ,
-	.odt_en = 1,
+	.odt_en = IS_ENABLED(CONFIG_DRAM_ODT_EN),
+	.odt_correction = CONFIG_DRAM_ODT_CORRECTION,
 	.para1 = 0, /* not used (only used when tpr13 bit 31 is set */
 	.para2 = 0, /* not used (only used when tpr13 bit 31 is set */
 	.mr0 = 6736,

@@ -97,7 +99,6 @@ static void mctl_init(u32 *bus_width)
 		(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
 	struct sunxi_mctl_phy_reg * const mctl_phy =
 		(struct sunxi_mctl_phy_reg *)SUNXI_DRAM_PHY0_BASE;
-	int correction;
 
 	if (dram_para.tpr13 & 0x20)
 		writel(0x40b, &mctl_phy->dcr);

@@ -138,7 +139,7 @@ static void mctl_init(u32 *bus_width)
 
 	writel(0x01000081, &mctl_phy->dtcr);
 
-	if (dram_para.clock <= 240 || !(dram_para.odt_en & 0x01)) {
+	if (dram_para.clock <= 240 || !dram_para.odt_en) {
 		clrbits_le32(&mctl_phy->dx0gcr, 0x600);
 		clrbits_le32(&mctl_phy->dx1gcr, 0x600);
 	}

@@ -251,13 +252,11 @@ static void mctl_init(u32 *bus_width)
 	} else
 		*bus_width = 16;
 
-	correction = (dram_para.odt_en >> 8) & 0xff;
-	if (correction) {
-		if (dram_para.odt_en & 0x80000000)
-			correction = -correction;
-
-		mctl_apply_odt_correction(&mctl_phy->dx0lcdlr1, correction);
-		mctl_apply_odt_correction(&mctl_phy->dx1lcdlr1, correction);
+	if (dram_para.odt_correction) {
+		mctl_apply_odt_correction(&mctl_phy->dx0lcdlr1,
+					  dram_para.odt_correction);
+		mctl_apply_odt_correction(&mctl_phy->dx1lcdlr1,
+					  dram_para.odt_correction);
 	}
 
 	mctl_await_completion(&mctl_ctl->statr, 0x01, 0x01);

arch/arm/cpu/armv7/sunxi/dram_sun8i_a33.c +2 -2

index 979bb3c..ebba438 100644
@@ -14,12 +14,12 @@
 #include <asm/arch/clock.h>
 #include <asm/arch/dram.h>
 #include <asm/arch/prcm.h>
+#include <linux/kconfig.h>
 
 /* PLL runs at 2x dram-clk, controller runs at PLL / 4 (dram-clk / 2) */
 #define DRAM_CLK_MUL 2
 #define DRAM_CLK_DIV 4
 #define DRAM_SIGMA_DELTA_ENABLE 1
-#define DRAM_ODT_EN 0
 
 struct dram_para {
 	u8 cs1;

@@ -215,7 +215,7 @@ static int mctl_channel_init(struct dram_para *para)
 	clrbits_le32(&mctl_ctl->pgcr0, 0x3f << 0);
 
 	/* Set ODT */
-	if ((CONFIG_DRAM_CLK > 400) && DRAM_ODT_EN) {
+	if ((CONFIG_DRAM_CLK > 400) && IS_ENABLED(CONFIG_DRAM_ODT_EN)) {
 		setbits_le32(DXnGCR0(0), 0x3 << 9);
 		setbits_le32(DXnGCR0(1), 0x3 << 9);
 	} else {

arch/arm/include/asm/arch-sunxi/dram_sun4i.h +1 -2

index 40c385a..139e336 100644
@@ -164,8 +164,7 @@ struct dram_para {
 
 #define DRAM_ZQSR_ZDONE (1 << 31) /* ZQ calibration completion flag */
 
-#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))
 
 #define DRAM_MR_BURST_LENGTH(n) (((n) & 0x7) << 0)
 #define DRAM_MR_BURST_LENGTH_MASK DRAM_MR_BURST_LENGTH(0x7)

arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h +1 -0

index 06adee2..316a333 100644
@@ -19,6 +19,7 @@ struct dram_para {
 	u32 type;
 	u32 zq;
 	u32 odt_en;
+	s32 odt_correction;
 	u32 para1;
 	u32 para2;
 	u32 mr0;

board/sunxi/Kconfig +15 -7

index 7368d4d..3012346 100644
@@ -96,6 +96,13 @@ config DRAM_ZQ
 	---help---
 	Set the dram zq value.
 
+config DRAM_ODT_EN
+	bool "sunxi dram odt enable"
+	default n if !MACH_SUN8I_A23
+	default y if MACH_SUN8I_A23
+	---help---
+	Select this to enable dram odt (on die termination).
+
 if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I
 config DRAM_EMR1
 	int "sunxi dram emr1 value"

@@ -104,13 +111,6 @@ config DRAM_EMR1
 	---help---
 	Set the dram controller emr1 value.
 
-config DRAM_ODT_EN
-	int "sunxi dram odt_en value"
-	default 0
-	---help---
-	Set the dram controller odt_en parameter. This can be used to
-	enable/disable the ODT feature.
-
 config DRAM_TPR3
 	hex "sunxi dram tpr3 value"
 	default 0

@@ -171,6 +171,14 @@ endchoice
 
 endif
 
+if MACH_SUN8I_A23
+config DRAM_ODT_CORRECTION
+	int "sunxi dram odt correction value"
+	default 0
+	---help---
+	Set the dram odt correction value (range -255 - 255).
+endif
+
 config SYS_CLK_FREQ
 	default 912000000 if MACH_SUN7I
 	default 1008000000 if MACH_SUN4I || MACH_SUN5I || MACH_SUN6I || MACH_SUN8I

board/sunxi/dram_sun4i_auto.c +2 -1

index 09e0c9a..149bb51 100644
@@ -1,5 +1,6 @@
 #include <common.h>
 #include <asm/arch/dram.h>
+#include <linux/kconfig.h>
 
 static struct dram_para dram_para = {
 	.clock = CONFIG_DRAM_CLK,

@@ -9,7 +10,7 @@ static struct dram_para dram_para = {
 	.io_width = 0,
 	.bus_width = 0,
 	.zq = CONFIG_DRAM_ZQ,
-	.odt_en = CONFIG_DRAM_ODT_EN,
+	.odt_en = IS_ENABLED(CONFIG_DRAM_ODT_EN),
 	.size = 0,
 #ifdef CONFIG_DRAM_TIMINGS_VENDOR_MAGIC
 	.cas = 6,

board/sunxi/dram_sun5i_auto.c +2 -1

index 660b18e..596a206 100644
@@ -2,6 +2,7 @@
 
 #include <common.h>
 #include <asm/arch/dram.h>
+#include <linux/kconfig.h>
 
 static struct dram_para dram_para = {
 	.clock = CONFIG_DRAM_CLK,

@@ -12,7 +13,7 @@ static struct dram_para dram_para = {
 	.io_width = 0,
 	.bus_width = 0,
 	.zq = CONFIG_DRAM_ZQ,
-	.odt_en = CONFIG_DRAM_ODT_EN,
+	.odt_en = IS_ENABLED(CONFIG_DRAM_ODT_EN),
 	.size = 0,
 #ifdef CONFIG_DRAM_TIMINGS_VENDOR_MAGIC
 	.cas = 9,

Signature · 1 line

2.4.1

Ian Campbell

2015-05-19 14:13 UTC #1 lore raw

PATCH v3 1/1 sunxi: Make DRAM_ODT_EN Kconfig setting a bool

Quoted 14 lines · Hans de Goede

arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -1

index c736fa3..f7b4915 100644
@@ -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;

arch/arm/cpu/armv7/sunxi/dram_sun4i.c static void mctl_set_impedance(u32 zq, u32 odt_en) · line 559

Quoted 5 lines

arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -1

@@ -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).

Whereas this...

arch/arm/cpu/armv7/sunxi/dram_sun4i.c static void mctl_set_impedance(u32 zq, u32 odt_en) · line 560

Quoted 3 lines

arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -2

-#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,@least with the in-tree defconfigs, odt_en was always 0 for sun4i anyway, but this is a surprising change I think.

DRAM_IOCR_ODT_EN_MASK (which did use 0x3) seems to have been unused. in tree.

arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h struct dram_para { · line 21

Quoted 8 lines

arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h +0 -0

index 06adee2..316a333 100644
@@ -19,6 +19,7 @@ struct dram_para {
 	u32 type;
 	u32 zq;
 	u32 odt_en;

Make this (and maybe others) an actual bool?

Keeping it a u32 might make hex editing easier though?

Ian.

Siarhei Siamashka

2015-05-19 15:28 UTC #2 lore raw

PATCH v3 1/1 sunxi: Make DRAM_ODT_EN Kconfig setting a bool

Quoted 17 lines · On Tue, 19 May 2015 15:13:18 +0100 Ian Campbell <[email protected]>…

Quoted 16 lines · Hans de Goede

arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -1

index c736fa3..f7b4915 100644
@@ -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);

Quoted 6 lines

arch/arm/cpu/armv7/sunxi/dram_sun4i.c static void mctl_set_impedance(u32 zq, u32 odt_en) · line 559
Quoted 3 lines
arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -1
 	/* 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.

Quoted 10 lines

Whereas this...
arch/arm/cpu/armv7/sunxi/dram_sun4i.c static void mctl_set_impedance(u32 zq, u32 odt_en) · line 560
Quoted 3 lines
arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -2
-#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

Signature · 2 lines

Best regards,
Siarhei Siamashka

Hans de Goede

author

2015-05-19 16:39 UTC #3 lore raw

PATCH v3 1/1 sunxi: Make DRAM_ODT_EN Kconfig setting a bool

In reply to Ian Campbell

Hi,

Quoted 26 lines · Ian Campbell

Quoted 14 lines · Hans de Goede
arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -1
index c736fa3..f7b4915 100644
@@ -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;
arch/arm/cpu/armv7/sunxi/dram_sun4i.c static void mctl_set_impedance(u32 zq, u32 odt_en) · line 559
Quoted 5 lines
arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -1
@@ -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).
Whereas this...

Quoted 8 lines

arch/arm/cpu/armv7/sunxi/dram_sun4i.c static void mctl_set_impedance(u32 zq, u32 odt_en) · line 560
Quoted 3 lines
arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -2
-#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.

This is deliberate, as Siarhei explained in his reply to v2 the 2 bits we are setting here enable odt for the DQ resp. DQS lines, having them enabled for one but not the other does not really make sense.

I'll amend the commit message to explicitly mention this.

Regards,

Hans

Quoted 19 lines

DRAM_IOCR_ODT_EN_MASK (which did use 0x3) seems to have been unused. in tree.
arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h struct dram_para { · line 21
Quoted 8 lines
arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h +0 -0
index 06adee2..316a333 100644
@@ -19,6 +19,7 @@ struct dram_para {
  	u32 type;
  	u32 zq;
  	u32 odt_en;
Make this (and maybe others) an actual bool?
Keeping it a u32 might make hex editing easier though?
Ian.

Ian Campbell

2015-05-19 18:41 UTC #4 lore raw

PATCH v3 1/1 sunxi: Make DRAM_ODT_EN Kconfig setting a bool

Quoted 35 lines · Hans de Goede

Hi,
Quoted 32 lines · Ian Campbell
Quoted 14 lines · Hans de Goede
arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -1
index c736fa3..f7b4915 100644
@@ -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;
arch/arm/cpu/armv7/sunxi/dram_sun4i.c static void mctl_set_impedance(u32 zq, u32 odt_en) · line 559
Quoted 5 lines
arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -1
@@ -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).
Whereas this...
arch/arm/cpu/armv7/sunxi/dram_sun4i.c static void mctl_set_impedance(u32 zq, u32 odt_en) · line 560
Quoted 3 lines
arch/arm/cpu/armv7/sunxi/dram_sun4i.c +1 -2
-#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.

Quoted 8 lines

Quoted 2 lines
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.
This is deliberate, as Siarhei explained in his reply to v2 the 2 bits we are setting here enable odt for the DQ resp. DQS lines, having them enabled for one but not the other does not really make sense.
I'll amend the commit message to explicitly mention this.

Thanks.

Ian.

Siarhei Siamashka

2015-05-19 14:54 UTC #5 lore raw

PATCH v3 1/1 sunxi: Make DRAM_ODT_EN Kconfig setting a bool

In reply to Hans de Goede

Quoted 8 lines · On Tue, 19 May 2015 14:56:39 +0200 Hans de Goede <[email protected]> wrote:

Make DRAM_ODT_EN Kconfig setting a bool, add a separate DRAM_ODT_CORRECTION setting for A23 SoCs and use DRAM_ODT_EN Kconfig everywhere instead of only in dram_sun4i.c and hardcoding odt_en elsewhere.
Note this commit makes no functional changes for existing boards, its purpose is to allow changing the odt_en value on future A33 boards.
Signed-off-by Hans de Goede <[email protected]>

The sun4i part is fine.

Regarding the A23 part, the only nitpick from me is the newly added CONFIG_DRAM_ODT_CORRECTION option. The description does not seem to be very informative in Kconfig:

board/sunxi/Kconfig endchoice · line 180

Quoted 7 lines

board/sunxi/Kconfig +7 -0

+if MACH_SUN8I_A23
+config DRAM_ODT_CORRECTION
+	int "sunxi dram odt correction value"
+	default 0
+	---help---
+	Set the dram odt correction value (range -255 - 255).
+endif

Since the right correction value is extracted from the FEX file (or where are we expected to get it from?), a short instruction about converting the 'dram_odt_en' parameter from FEX into the DRAM_ODT_CORRECTION option for U-Boot would be quite useful here.

Other than this, looks good and

Acked-by Siarhei Siamashka <[email protected]>

Signature · 2 lines

Best regards,
Siarhei Siamashka

Hans de Goede

author

2015-05-19 16:50 UTC #6 lore raw

PATCH v3 1/1 sunxi: Make DRAM_ODT_EN Kconfig setting a bool

Hi,

Quoted 18 lines · Siarhei Siamashka

Quoted 8 lines · On Tue, 19 May 2015 14:56:39 +0200 Hans de Goede <[email protected]> wrote:
Make DRAM_ODT_EN Kconfig setting a bool, add a separate DRAM_ODT_CORRECTION setting for A23 SoCs and use DRAM_ODT_EN Kconfig everywhere instead of only in dram_sun4i.c and hardcoding odt_en elsewhere.
Note this commit makes no functional changes for existing boards, its purpose is to allow changing the odt_en value on future A33 boards.
Signed-off-by Hans de Goede <[email protected]>
The sun4i part is fine.
Regarding the A23 part, the only nitpick from me is the newly added CONFIG_DRAM_ODT_CORRECTION option. The description does not seem to be very informative in Kconfig:

Quoted 12 lines

board/sunxi/Kconfig endchoice · line 180
Quoted 7 lines
board/sunxi/Kconfig +7 -0
+if MACH_SUN8I_A23
+config DRAM_ODT_CORRECTION
+	int "sunxi dram odt correction value"
+	default 0
+	---help---
+	Set the dram odt correction value (range -255 - 255).
+endif
Since the right correction value is extracted from the FEX file (or where are we expected to get it from?), a short instruction about converting the 'dram_odt_en' parameter from FEX into the DRAM_ODT_CORRECTION option for U-Boot would be quite useful here.

Thanks for the review, adding a blurb to the Kconfig help on how to get the correction value from a fex file is a good idea, I've added such a blurb to the version in my personal tree.

Regards,

Hans