Re: [U-Boot] [PATCH 10/14] sunxi: Move await_completion dram helper to dram.h
On Fri, 19 Dec 2014 17:42:58 +0100 Hans de Goede hdego...@redhat.com wrote: Hi, On 19-12-14 11:06, Siarhei Siamashka wrote: 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 textdata bss dec hex filename 1731 0 01731 6c3 $ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o textdata bss dec hex filename 1841 0 01841 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 textdata bss dec hex filename 1763 0 01763 6e3 $ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o textdata bss dec hex filename 1983 0 01983 7bf Ah, thanks for catching that, the size increase does not seem to be a problem right now, but it certainly is something to keep in mind. Could we perhaps just introduce something like a new source file dram_common.c or even dram.c? Sounds like a good idea, patches welcome. I would definitely appreciate if we could have avoided having this unnecessary code size increase in the first place. But if we run out of the code size space, then the oversized static inline functions in the headers are always relatively easy to identify and eliminate in one go. That's only a minor annoyance and I'm not going to waste time on pointless debates. 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. Sounds like another good idea, patches welcome. -- Best regards, Siarhei Siamashka ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 10/14] sunxi: Move await_completion dram helper to dram.h
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
textdata bss dec hex filename
1731 0 01731 6c3
$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
textdata bss dec hex filename
1841 0 01841 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
textdata bss dec hex filename
1763 0 01763 6e3
$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
textdata bss dec hex filename
1983 0 01983 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() + 100;
-
- 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() + 100;
-
- 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 |
Re: [U-Boot] [PATCH 10/14] sunxi: Move await_completion dram helper to dram.h
Hi, On 19-12-14 11:06, Siarhei Siamashka wrote: 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 textdata bss dec hex filename 1731 0 01731 6c3 $ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o textdata bss dec hex filename 1841 0 01841 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 textdata bss dec hex filename 1763 0 01763 6e3 $ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o textdata bss dec hex filename 1983 0 01983 7bf Ah, thanks for catching that, the size increase does not seem to be a problem right now, but it certainly is something to keep in mind. Could we perhaps just introduce something like a new source file dram_common.c or even dram.c? Sounds like a good idea, patches welcome. 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. Sounds like another good idea, patches welcome. Regards, Hans ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 10/14] sunxi: Move await_completion dram helper to dram.h
On Tue, 2014-12-16 at 21:31 +0100, Hans de Goede 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 Acked-by: Ian Campbell i...@hellion.org.uk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 10/14] sunxi: Move await_completion dram helper to dram.h
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
---
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() + 100;
-
- 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() + 100;
-
- 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);
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 / 100, mctl_ctl-togcnt1u);
