Re: [U-Boot] [PATCH 2/2] sunxi: Make dram odt-en configurable through Kconfig for A33 based boards
Hi, On 14-05-15 20:41, Ian Campbell wrote: On Thu, 2015-05-14 at 18:55 +0200, Hans de Goede wrote: +config DRAM_ODT_EN + int sunxi dram odt enable + default 0 + ---help--- + Set this to 1 to enable dram odt (on die termination) Why is this an int rather than a bool? Because it is used directly as an int in the code, otherwise I need to add #ifdef-ery. I guess I could make it a bool and use IS_ENABLED(), but that does lead to slightly less readable code IMHO. I'm afraid I think IS_ENABLED is the way to go though, it's the lesser of two evils compared with using an int for a boolean option. Ok, so while trying to fix this I noticed that there already is a definition of CONFIG_DRAM_ODT_EN in board/sunxi/Kconfig inside a #if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I block, and that one actually really is an int (it goes from 0-3). so I'm going to leave this as an int, and use the existing option moving it outside of the #if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I block. v2 coming up. Regards, Hans ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] sunxi: Make dram odt-en configurable through Kconfig for A33 based boards
On Thu, 2015-05-14 at 18:55 +0200, Hans de Goede wrote: +config DRAM_ODT_EN + int sunxi dram odt enable + default 0 + ---help--- + Set this to 1 to enable dram odt (on die termination) Why is this an int rather than a bool? Because it is used directly as an int in the code, otherwise I need to add #ifdef-ery. I guess I could make it a bool and use IS_ENABLED(), but that does lead to slightly less readable code IMHO. I'm afraid I think IS_ENABLED is the way to go though, it's the lesser of two evils compared with using an int for a boolean option. Ian. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] sunxi: Make dram odt-en configurable through Kconfig for A33 based boards
Hi, On 05/13/2015 09:26 PM, Ian Campbell wrote: On Wed, 2015-05-13 at 17:09 +0200, Hans de Goede wrote: diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 940b6c7..d4ae6c7 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -95,6 +95,14 @@ config DRAM_ZQ ---help--- Set the dram zq value. +if MACH_SUN8I_A33 Shouldn't this be a depends on MACH_SUN8I_A33 in the entry itself? I see we use if a lot in this file, is there a reason for that or just how it has been done? No special reason, just copy paste from elsewhere. In any case using if here is at least consistent so no strong objection on that grounds. +config DRAM_ODT_EN + int sunxi dram odt enable + default 0 + ---help--- + Set this to 1 to enable dram odt (on die termination) Why is this an int rather than a bool? Because it is used directly as an int in the code, otherwise I need to add #ifdef-ery. I guess I could make it a bool and use IS_ENABLED(), but that does lead to slightly less readable code IMHO. Regards, Hans ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/2] sunxi: Make dram odt-en configurable through Kconfig for A33 based boards
Some A33 based boards use odt, while others do not, so make this configurable.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
arch/arm/cpu/armv7/sunxi/dram_sun8i_a33.c | 3 +--
board/sunxi/Kconfig | 8
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun8i_a33.c
b/arch/arm/cpu/armv7/sunxi/dram_sun8i_a33.c
index 979bb3c..a7a0a2e 100644
--- a/arch/arm/cpu/armv7/sunxi/dram_sun8i_a33.c
+++ b/arch/arm/cpu/armv7/sunxi/dram_sun8i_a33.c
@@ -19,7 +19,6 @@
#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 +214,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) CONFIG_DRAM_ODT_EN) {
setbits_le32(DXnGCR0(0), 0x3 9);
setbits_le32(DXnGCR0(1), 0x3 9);
} else {
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 940b6c7..d4ae6c7 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -95,6 +95,14 @@ config DRAM_ZQ
---help---
Set the dram zq value.
+if MACH_SUN8I_A33
+config DRAM_ODT_EN
+ int sunxi dram odt enable
+ default 0
+ ---help---
+ Set this to 1 to enable dram odt (on die termination)
+endif
+
if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I
config DRAM_EMR1
int sunxi dram emr1 value
--
2.4.0
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] sunxi: Make dram odt-en configurable through Kconfig for A33 based boards
On Wed, 2015-05-13 at 17:09 +0200, Hans de Goede wrote: diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 940b6c7..d4ae6c7 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -95,6 +95,14 @@ config DRAM_ZQ ---help--- Set the dram zq value. +if MACH_SUN8I_A33 Shouldn't this be a depends on MACH_SUN8I_A33 in the entry itself? I see we use if a lot in this file, is there a reason for that or just how it has been done? In any case using if here is at least consistent so no strong objection on that grounds. +config DRAM_ODT_EN + int sunxi dram odt enable + default 0 + ---help--- + Set this to 1 to enable dram odt (on die termination) Why is this an int rather than a bool? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
