[U-Boot] [PATCH 10/14] sunxi: Move await_completion dram helper to dram.h
Siarhei Siamashka
siarhei.siamashka at gmail.com
Fri Dec 19 11:06:24 CET 2014
On Tue, 16 Dec 2014 21:31:35 +0100
Hans de Goede <hdegoede at redhat.com> wrote:
> The await_completion helper is already copy pasted between the sun4i and sun6i
> dram code, and we need it for sun8i too, so lets make it an inline helper in
> dram.h, rather then adding yet another copy.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
Making this function "static inline" and placing it into a header file
encourages the compiler to actually inline it. Which is not great for
code size:
== Before the patch and using gcc version 4.8.3 ==
$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun4i.o
text data bss dec hex filename
1731 0 0 1731 6c3
$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
text data bss dec hex filename
1841 0 0 1841 731
== After the patch and using gcc version 4.8.3 ==
$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun4i.o
text data bss dec hex filename
1763 0 0 1763 6e3
$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
text data bss dec hex filename
1983 0 0 1983 7bf
Could we perhaps just introduce something like a new source file
"dram_common.c" or even "dram.c"?
> ---
> arch/arm/cpu/armv7/sunxi/dram_sun4i.c | 17 ++---------------
> arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 31 +++++++++----------------------
> arch/arm/include/asm/arch-sunxi/dram.h | 14 ++++++++++++++
> 3 files changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun4i.c b/arch/arm/cpu/armv7/sunxi/dram_sun4i.c
> index ec8aaa7..c736fa3 100644
> --- a/arch/arm/cpu/armv7/sunxi/dram_sun4i.c
> +++ b/arch/arm/cpu/armv7/sunxi/dram_sun4i.c
> @@ -36,24 +36,11 @@
> #define CPU_CFG_CHIP_REV_B 0x3
>
> /*
> - * Wait up to 1s for value to be set in given part of reg.
> - */
> -static void await_completion(u32 *reg, u32 mask, u32 val)
> -{
> - unsigned long tmo = timer_get_us() + 1000000;
> -
> - while ((readl(reg) & mask) != val) {
> - if (timer_get_us() > tmo)
> - panic("Timeout initialising DRAM\n");
> - }
> -}
> -
> -/*
> * Wait up to 1s for mask to be clear in given reg.
> */
> static inline void await_bits_clear(u32 *reg, u32 mask)
> {
> - await_completion(reg, mask, 0);
> + mctl_await_completion(reg, mask, 0);
> }
>
> /*
> @@ -61,7 +48,7 @@ static inline void await_bits_clear(u32 *reg, u32 mask)
> */
> static inline void await_bits_set(u32 *reg, u32 mask)
> {
> - await_completion(reg, mask, mask);
> + mctl_await_completion(reg, mask, mask);
> }
>
> /*
> diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c
> index a8bbdfd..e1670e5 100644
> --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c
> +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c
> @@ -27,19 +27,6 @@ struct dram_sun6i_para {
> u16 page_size;
> };
>
> -/*
> - * Wait up to 1s for value to be set in given part of reg.
> - */
> -static void await_completion(u32 *reg, u32 mask, u32 val)
> -{
> - unsigned long tmo = timer_get_us() + 1000000;
> -
> - while ((readl(reg) & mask) != val) {
> - if (timer_get_us() > tmo)
> - panic("Timeout initialising DRAM\n");
> - }
> -}
> -
> static void mctl_sys_init(void)
> {
> struct sunxi_ccm_reg * const ccm =
> @@ -51,7 +38,7 @@ static void mctl_sys_init(void)
> clrsetbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_DIV0_MASK,
> CCM_DRAMCLK_CFG_DIV0(dram_clk_div) | CCM_DRAMCLK_CFG_RST |
> CCM_DRAMCLK_CFG_UPD);
> - await_completion(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_UPD, 0);
> + mctl_await_completion(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_UPD, 0);
As an additional observation, moving await_bits_clear/await_bits_set
functions into a common header file and using them in the sun6i dram
code might improve readability.
> writel(MDFS_CLK_DEFAULT, &ccm->mdfs_clk_cfg);
>
> @@ -107,8 +94,8 @@ static bool mctl_rank_detect(u32 *gsr0, int rank)
> const u32 done = MCTL_DX_GSR0_RANK0_TRAIN_DONE << rank;
> const u32 err = MCTL_DX_GSR0_RANK0_TRAIN_ERR << rank;
>
> - await_completion(gsr0, done, done);
> - await_completion(gsr0 + 0x10, done, done);
> + mctl_await_completion(gsr0, done, done);
> + mctl_await_completion(gsr0 + 0x10, done, done);
>
> return !(readl(gsr0) & err) && !(readl(gsr0 + 0x10) & err);
> }
> @@ -129,7 +116,7 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
> }
>
> writel(MCTL_MCMD_NOP, &mctl_ctl->mcmd);
> - await_completion(&mctl_ctl->mcmd, MCTL_MCMD_BUSY, 0);
> + mctl_await_completion(&mctl_ctl->mcmd, MCTL_MCMD_BUSY, 0);
>
> /* PHY initialization */
> writel(MCTL_PGCR, &mctl_phy->pgcr);
> @@ -166,14 +153,14 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
> writel(MCTL_DX_GCR | MCTL_DX_GCR_EN, &mctl_phy->dx2gcr);
> writel(MCTL_DX_GCR | MCTL_DX_GCR_EN, &mctl_phy->dx3gcr);
>
> - await_completion(&mctl_phy->pgsr, 0x03, 0x03);
> + mctl_await_completion(&mctl_phy->pgsr, 0x03, 0x03);
>
> writel(CONFIG_DRAM_ZQ, &mctl_phy->zq0cr1);
>
> setbits_le32(&mctl_phy->pir, MCTL_PIR_CLEAR_STATUS);
> writel(MCTL_PIR_STEP1, &mctl_phy->pir);
> udelay(10);
> - await_completion(&mctl_phy->pgsr, 0x1f, 0x1f);
> + mctl_await_completion(&mctl_phy->pgsr, 0x1f, 0x1f);
>
> /* rank detect */
> if (!mctl_rank_detect(&mctl_phy->dx0gsr0, 1)) {
> @@ -204,14 +191,14 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
> setbits_le32(&mctl_phy->pir, MCTL_PIR_CLEAR_STATUS);
> writel(MCTL_PIR_STEP2, &mctl_phy->pir);
> udelay(10);
> - await_completion(&mctl_phy->pgsr, 0x11, 0x11);
> + mctl_await_completion(&mctl_phy->pgsr, 0x11, 0x11);
>
> if (readl(&mctl_phy->pgsr) & MCTL_PGSR_TRAIN_ERR_MASK)
> panic("Training error initialising DRAM\n");
>
> /* Move to configure state */
> writel(MCTL_SCTL_CONFIG, &mctl_ctl->sctl);
> - await_completion(&mctl_ctl->sstat, 0x07, 0x01);
> + mctl_await_completion(&mctl_ctl->sstat, 0x07, 0x01);
>
> /* Set number of clks per micro-second */
> writel(DRAM_CLK / 1000000, &mctl_ctl->togcnt1u);
> @@ -270,7 +257,7 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
>
> /* Move to access state */
> writel(MCTL_SCTL_ACCESS, &mctl_ctl->sctl);
> - await_completion(&mctl_ctl->sstat, 0x07, 0x03);
> + mctl_await_completion(&mctl_ctl->sstat, 0x07, 0x03);
> }
>
> static void mctl_com_init(struct dram_sun6i_para *para)
> diff --git a/arch/arm/include/asm/arch-sunxi/dram.h b/arch/arm/include/asm/arch-sunxi/dram.h
> index 9072e68..18924f5 100644
> --- a/arch/arm/include/asm/arch-sunxi/dram.h
> +++ b/arch/arm/include/asm/arch-sunxi/dram.h
> @@ -12,6 +12,7 @@
> #ifndef _SUNXI_DRAM_H
> #define _SUNXI_DRAM_H
>
> +#include <asm/io.h>
> #include <linux/types.h>
>
> /* dram regs definition */
> @@ -23,4 +24,17 @@
>
> unsigned long sunxi_dram_init(void);
>
> +/*
> + * Wait up to 1s for value to be set in given part of reg.
> + */
> +static inline void mctl_await_completion(u32 *reg, u32 mask, u32 val)
> +{
> + unsigned long tmo = timer_get_us() + 1000000;
> +
> + while ((readl(reg) & mask) != val) {
> + if (timer_get_us() > tmo)
> + panic("Timeout initialising DRAM\n");
> + }
> +}
> +
> #endif /* _SUNXI_DRAM_H */
--
Best regards,
Siarhei Siamashka
More information about the U-Boot
mailing list