On Tue, 16 Dec 2014 21:31:35 +0100 Hans de Goede <hdego...@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 <hdego...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot