[U-Boot] [PATCH 10/14] sunxi: Move await_completion dram helper to dram.h

[Siarhei Siamashka <siarhei.siamashka@gmail.com>]

On Tue, 16 Dec 2014 21:31:35 +0100
Hans de Goede <hdegoede@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@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