| Message ID | 1416154613-8506-1-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Ian Campbell |
| Headers | show |
On Sun, 2014-11-16 at 17:16 +0100, Hans de Goede wrote: > +config DRAM_ZQ > + int "sun6i dram zq value" if EXPERT > + default 123 == 0x7b, which differs from the previous default of 0x78 (not by much, but still) > diff --git a/configs/Colombus_defconfig b/configs/Colombus_defconfig > index bef568d..5e245a3 100644 > --- a/configs/Colombus_defconfig > +++ b/configs/Colombus_defconfig > @@ -4,3 +4,5 @@ CONFIG_FDTFILE="sun6i-a31-colombus.dtb" > +S:CONFIG_ARCH_SUNXI=y > +S:CONFIG_MACH_SUN6I=y > +S:CONFIG_TARGET_COLOMBUS=y > ++S:CONFIG_DRAM_CLK=288 > ++S:CONFIG_DRAM_ZQ=379 Do these actually take affect if CONFIG_EXPERT=n? Perhaps CONFIG_TARGET_COLOMBUS should select DRAM_CLK == 288 and DRAM_CLK should be a hidden option? Ian.
Hi, On 11/17/2014 07:54 PM, Ian Campbell wrote: > On Sun, 2014-11-16 at 17:16 +0100, Hans de Goede wrote: > >> +config DRAM_ZQ >> + int "sun6i dram zq value" if EXPERT >> + default 123 > > == 0x7b, which differs from the previous default of 0x78 (not by much, > but still) Right, this is deliberate as I've gotten access to more boards, 123 seems a better default (this is also what most A10 / A13 / A20 boards use). > >> diff --git a/configs/Colombus_defconfig b/configs/Colombus_defconfig >> index bef568d..5e245a3 100644 >> --- a/configs/Colombus_defconfig >> +++ b/configs/Colombus_defconfig >> @@ -4,3 +4,5 @@ CONFIG_FDTFILE="sun6i-a31-colombus.dtb" >> +S:CONFIG_ARCH_SUNXI=y >> +S:CONFIG_MACH_SUN6I=y >> +S:CONFIG_TARGET_COLOMBUS=y >> ++S:CONFIG_DRAM_CLK=288 >> ++S:CONFIG_DRAM_ZQ=379 > > Do these actually take affect if CONFIG_EXPERT=n? Ugh, I just checked, and no they don't. > Perhaps CONFIG_TARGET_COLOMBUS should select DRAM_CLK == 288 and > DRAM_CLK should be a hidden option? I would prefer to keep these inside the defconfig's so that for adding a new board only a defconfig needs to be added. Currently the only CONFIG_TARGET_BOARD we actually use in u-boot is the bananapi gmac tx clk delay tweak, and I would actually like to keep things that way. Actually thinking more about this, I think that that should be turned into a GMAC specific Kconfig option and all the CONFIG_TARGET_BOARD stuff should be dropped, as it is just a drag causing extra maintenance going forward. So is there a way to not show a choice to the user unless EXPERT mode is set, and still override it from Kconfig ? If that is not (easily) possible, then I guess we will just need to not make these "if EXPERT". Regards, Hans
On Tue, 2014-11-18 at 12:32 +0100, Hans de Goede wrote: > Hi, > > On 11/17/2014 07:54 PM, Ian Campbell wrote: > > On Sun, 2014-11-16 at 17:16 +0100, Hans de Goede wrote: > > > >> +config DRAM_ZQ > >> + int "sun6i dram zq value" if EXPERT > >> + default 123 > > > > == 0x7b, which differs from the previous default of 0x78 (not by much, > > but still) > > Right, this is deliberate as I've gotten access to more boards, 123 seems > a better default (this is also what most A10 / A13 / A20 boards use). Please mention this in the commit log. > >> diff --git a/configs/Colombus_defconfig b/configs/Colombus_defconfig > >> index bef568d..5e245a3 100644 > >> --- a/configs/Colombus_defconfig > >> +++ b/configs/Colombus_defconfig > >> @@ -4,3 +4,5 @@ CONFIG_FDTFILE="sun6i-a31-colombus.dtb" > >> +S:CONFIG_ARCH_SUNXI=y > >> +S:CONFIG_MACH_SUN6I=y > >> +S:CONFIG_TARGET_COLOMBUS=y > >> ++S:CONFIG_DRAM_CLK=288 > >> ++S:CONFIG_DRAM_ZQ=379 > > > > Do these actually take affect if CONFIG_EXPERT=n? > > Ugh, I just checked, and no they don't. > > > Perhaps CONFIG_TARGET_COLOMBUS should select DRAM_CLK == 288 and > > DRAM_CLK should be a hidden option? > > I would prefer to keep these inside the defconfig's so that for > adding a new board only a defconfig needs to be added. > > Currently the only CONFIG_TARGET_BOARD we actually use in u-boot is > the bananapi gmac tx clk delay tweak, and I would actually like > to keep things that way. Actually thinking more about this, I > think that that should be turned into a GMAC specific Kconfig option > and all the CONFIG_TARGET_BOARD stuff should be dropped, as it is > just a drag causing extra maintenance going forward. Hrm, I'm not sure yet which way I fall on this question. > So is there a way to not show a choice to the user unless EXPERT > mode is set, and still override it from Kconfig ? Not AFAIK, but I'm not expert (no pun intended) in Kconfig. > If that is not (easily) possible, then I guess we will just need > to not make these "if EXPERT". Probably :-(
Hi, On 11/18/2014 03:07 PM, Ian Campbell wrote: > On Tue, 2014-11-18 at 12:32 +0100, Hans de Goede wrote: >> Hi, >> >> On 11/17/2014 07:54 PM, Ian Campbell wrote: >>> On Sun, 2014-11-16 at 17:16 +0100, Hans de Goede wrote: >>> >>>> +config DRAM_ZQ >>>> + int "sun6i dram zq value" if EXPERT >>>> + default 123 >>> >>> == 0x7b, which differs from the previous default of 0x78 (not by much, >>> but still) >> >> Right, this is deliberate as I've gotten access to more boards, 123 seems >> a better default (this is also what most A10 / A13 / A20 boards use). > > Please mention this in the commit log. Fixed in my personal tree. >>>> diff --git a/configs/Colombus_defconfig b/configs/Colombus_defconfig >>>> index bef568d..5e245a3 100644 >>>> --- a/configs/Colombus_defconfig >>>> +++ b/configs/Colombus_defconfig >>>> @@ -4,3 +4,5 @@ CONFIG_FDTFILE="sun6i-a31-colombus.dtb" >>>> +S:CONFIG_ARCH_SUNXI=y >>>> +S:CONFIG_MACH_SUN6I=y >>>> +S:CONFIG_TARGET_COLOMBUS=y >>>> ++S:CONFIG_DRAM_CLK=288 >>>> ++S:CONFIG_DRAM_ZQ=379 >>> >>> Do these actually take affect if CONFIG_EXPERT=n? >> >> Ugh, I just checked, and no they don't. >> >>> Perhaps CONFIG_TARGET_COLOMBUS should select DRAM_CLK == 288 and >>> DRAM_CLK should be a hidden option? >> >> I would prefer to keep these inside the defconfig's so that for >> adding a new board only a defconfig needs to be added. >> >> Currently the only CONFIG_TARGET_BOARD we actually use in u-boot is >> the bananapi gmac tx clk delay tweak, and I would actually like >> to keep things that way. Actually thinking more about this, I >> think that that should be turned into a GMAC specific Kconfig option >> and all the CONFIG_TARGET_BOARD stuff should be dropped, as it is >> just a drag causing extra maintenance going forward. > > Hrm, I'm not sure yet which way I fall on this question. Ok, so how do you feel no that you've slept a night on it ? May main reason for wanting to get rid of this is that it does not buy us much, unless we start adding things like select FOO in there as you suggested, but that seems less transparent then having all the board specific settings in the _defconfig. To me the ideal scenario is that adding a new board is just adding a new _defconfig, and likewise figuring out what board specific settings are done for a certain board also is just a manner of inspecting the _defconfig, and not 2 - 3 different files. If we decide we want everything in the _defconfig, and thus CONFIG_TARGET_BOARD does not select / enable anything, iow is essentially unused, then we might just as well drop it. >> So is there a way to not show a choice to the user unless EXPERT >> mode is set, and still override it from Kconfig ? > > Not AFAIK, but I'm not expert (no pun intended) in Kconfig. > >> If that is not (easily) possible, then I guess we will just need >> to not make these "if EXPERT". > > Probably :-( Ok, so I've dropped the "if EXPERT" from the version in my personal tree. Regards, Hans
diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c index 10a6241..30439dc 100644 --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c @@ -17,9 +17,7 @@ #include <asm/arch/dram.h> #include <asm/arch/prcm.h> -/* DRAM clk & zq defaults, maybe turn these into Kconfig options ? */ -#define DRAM_CLK_DEFAULT 312000000 -#define DRAM_ZQ_DEFAULT 0x78 +#define DRAM_CLK (CONFIG_DRAM_CLK * 1000000) struct dram_sun6i_para { u8 bus_width; @@ -48,7 +46,7 @@ static void mctl_sys_init(void) (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; const int dram_clk_div = 2; - clock_set_pll5(DRAM_CLK_DEFAULT * dram_clk_div); + clock_set_pll5(DRAM_CLK * dram_clk_div); clrsetbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_DIV0_MASK, CCM_DRAMCLK_CFG_DIV0(dram_clk_div) | CCM_DRAMCLK_CFG_RST | @@ -173,7 +171,7 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para) await_completion(&mctl_phy->pgsr, 0x03, 0x03); - writel(DRAM_ZQ_DEFAULT, &mctl_phy->zq0cr1); + writel(CONFIG_DRAM_ZQ, &mctl_phy->zq0cr1); setbits_le32(&mctl_phy->pir, MCTL_PIR_CLEAR_STATUS); writel(MCTL_PIR_STEP1, &mctl_phy->pir); @@ -219,9 +217,9 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para) await_completion(&mctl_ctl->sstat, 0x07, 0x01); /* Set number of clks per micro-second */ - writel(DRAM_CLK_DEFAULT / 1000000, &mctl_ctl->togcnt1u); + writel(DRAM_CLK / 1000000, &mctl_ctl->togcnt1u); /* Set number of clks per 100 nano-seconds */ - writel(DRAM_CLK_DEFAULT / 10000000, &mctl_ctl->togcnt100n); + writel(DRAM_CLK / 10000000, &mctl_ctl->togcnt100n); /* Set memory timing registers */ writel(MCTL_TREFI, &mctl_ctl->trefi); writel(MCTL_TMRD, &mctl_ctl->tmrd); diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 246cd9a..352f04e 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -32,6 +32,23 @@ config MACH_SUN8I endchoice +if MACH_SUN6I + +config DRAM_CLK + int "sun6i dram clock speed" if EXPERT + default 312 + ---help--- + Set the dram clock speed, valid range 240 - 480, must be a multiple + of 24. + +config DRAM_ZQ + int "sun6i dram zq value" if EXPERT + default 123 + ---help--- + Set the dram zq value. + +endif + config SYS_CONFIG_NAME string default "sun4i" if MACH_SUN4I diff --git a/configs/Colombus_defconfig b/configs/Colombus_defconfig index bef568d..5e245a3 100644 --- a/configs/Colombus_defconfig +++ b/configs/Colombus_defconfig @@ -4,3 +4,5 @@ CONFIG_FDTFILE="sun6i-a31-colombus.dtb" +S:CONFIG_ARCH_SUNXI=y +S:CONFIG_MACH_SUN6I=y +S:CONFIG_TARGET_COLOMBUS=y ++S:CONFIG_DRAM_CLK=288 ++S:CONFIG_DRAM_ZQ=379 diff --git a/configs/Mele_M9_defconfig b/configs/Mele_M9_defconfig index f46439f..7e50888 100644 --- a/configs/Mele_M9_defconfig +++ b/configs/Mele_M9_defconfig @@ -5,6 +5,8 @@ CONFIG_FDTFILE="sun6i-a31-m9.dtb" +S:CONFIG_ARCH_SUNXI=y +S:CONFIG_MACH_SUN6I=y +S:CONFIG_TARGET_MELE_M9=y ++S:CONFIG_DRAM_CLK=312 ++S:CONFIG_DRAM_ZQ=120 # Ethernet phy power +S:CONFIG_AXP221_DLDO1_VOLT=3300 # USB hub power
It turns out that there is a too large spread between boards to handle this with a default value, turn this into Kconfig options, and set the values the factory images are using for the Colombus and Mele_M9 boards. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 12 +++++------- board/sunxi/Kconfig | 17 +++++++++++++++++ configs/Colombus_defconfig | 2 ++ configs/Mele_M9_defconfig | 2 ++ 4 files changed, 26 insertions(+), 7 deletions(-)