Re: [U-Boot] [PATCH 4/5] sunxi: let sunxi_dram_init return unsigned long long

Wed, 07 Feb 2018 22:57:06 -0800 

在 2018-02-08 08:37，André Przywara 写道：

On 07/02/18 19:35, Icenowy Zheng wrote:

Hi,


As 4GiB capacity is above the range of 32-bit unsigned integer, raise
the return type of sunxi_dram_init() to unsigned long long, thus it can 
hold 4GiB capacity (or maybe more on A80).
Some controllers that are possible to use 4GiB+ memory module are
also changed to calculate its memory capacity in unsigned long long.

Signed-off-by: Icenowy Zheng <icen...@aosc.io>
---
 arch/arm/include/asm/arch-sunxi/dram.h | 2 +-
 arch/arm/mach-sunxi/dram_sun6i.c       | 2 +-
 arch/arm/mach-sunxi/dram_sun8i_a23.c   | 2 +-
 arch/arm/mach-sunxi/dram_sun8i_a33.c   | 2 +-
 arch/arm/mach-sunxi/dram_sun8i_a83t.c  | 2 +-
 arch/arm/mach-sunxi/dram_sun9i.c       | 4 ++--
 arch/arm/mach-sunxi/dram_sunxi_dw.c    | 4 ++--
 board/sunxi/board.c                    | 2 +-
 board/sunxi/dram_sun4i_auto.c          | 2 +-
 board/sunxi/dram_sun5i_auto.c          | 2 +-
 10 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/arm/include/asm/arch-sunxi/dram.h b/arch/arm/include/asm/arch-sunxi/dram.h 
index 80abac95b8..d08b82371d 100644
--- a/arch/arm/include/asm/arch-sunxi/dram.h
+++ b/arch/arm/include/asm/arch-sunxi/dram.h
@@ -32,7 +32,7 @@
 #include <asm/arch/dram_sun4i.h>
 #endif

-unsigned long sunxi_dram_init(void);
+unsigned long long sunxi_dram_init(void);


Since this is explicitly about > 4GB/32 bits, I would suggest we just
use uint64_t here, instead of guessing what long long means.


Seems OK. But is there any sure suffix for uint64_t? For unsigned long
long we can use ULL.



But can't we just change the semantics of sunxi_dram_init() to return
megabytes instead? sun9i already uses this internally, and just blows it 
up in the wrapper. I don't think we have anything with a granularity
smaller than 1MB?
Then we could just leave it at native bit size, and spare poor ARMv7
from struggling with 64 bit arithmetic.


The calculated size is just in byte.

The A80 code first shifted it to MiB in DRAMC_get_dram_size() and then
shifted it back to B in sunxi_dram_init(); however the code that shifts
it to MiB only applies to drivers which have shift operation.
The other target driver, dram_sunxi_dw, uses both bit shifts and multiply 
when calculating the size, and to have 4096MiB value we still need to
calculate the byte value as uint64_t and then shift it to MiB value.
(The multiply value can get as high as 4096, so it's highly possible
that the shift value could be less than 20.)

I think keeping the return value as byte will make the code more clear.



Cheers,
Andre.


 void mctl_await_completion(u32 *reg, u32 mask, u32 val);
 bool mctl_mem_matches(u32 offset);
diff --git a/arch/arm/mach-sunxi/dram_sun6i.c b/arch/arm/mach-sunxi/dram_sun6i.c 
index 5dbbf6186f..bdf52a2c38 100644
--- a/arch/arm/mach-sunxi/dram_sun6i.c
+++ b/arch/arm/mach-sunxi/dram_sun6i.c
@@ -326,7 +326,7 @@ static void mctl_port_cfg(void)
        writel(0x00000307, &mctl_com->mbagcr[5]);
 }

-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
        struct sunxi_mctl_com_reg * const mctl_com =
                (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a23.c b/arch/arm/mach-sunxi/dram_sun8i_a23.c 
index c53671a0e9..169ccff41a 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_a23.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_a23.c
@@ -264,7 +264,7 @@ static void mctl_init(u32 *bus_width)
        writel(0x00000000, &mctl_ctl->rfshctl3);
 }

-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
        struct sunxi_mctl_com_reg * const mctl_com =
                (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a33.c b/arch/arm/mach-sunxi/dram_sun8i_a33.c 
index fa1620cb39..dfbbe6f39c 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_a33.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_a33.c
@@ -325,7 +325,7 @@ static void mctl_sys_init(struct dram_para *para)
        udelay(250);
 }

-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
        struct sunxi_mctl_com_reg * const mctl_com =
                        (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a83t.c b/arch/arm/mach-sunxi/dram_sun8i_a83t.c 
index 55df1b9d54..ec4bccd635 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_a83t.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_a83t.c
@@ -423,7 +423,7 @@ static void mctl_sys_init(struct dram_para *para)
        udelay(250);
 }

-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
        struct sunxi_mctl_com_reg * const mctl_com =
                        (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
diff --git a/arch/arm/mach-sunxi/dram_sun9i.c b/arch/arm/mach-sunxi/dram_sun9i.c 
index 8c681f3541..dcb20f763e 100644
--- a/arch/arm/mach-sunxi/dram_sun9i.c
+++ b/arch/arm/mach-sunxi/dram_sun9i.c
@@ -854,7 +854,7 @@ signed int DRAMC_get_dram_size(void)
        return 1 << dram_size;
 }

-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
        struct sunxi_mctl_com_reg * const mctl_com =
                (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
@@ -957,5 +957,5 @@ unsigned long sunxi_dram_init(void)
        mctl_com_init(&para);

        /* return the proper RAM size */
-       return DRAMC_get_dram_size() << 20;
+       return ((unsigned long long)DRAMC_get_dram_size()) << 20;
 }
diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c b/arch/arm/mach-sunxi/dram_sunxi_dw.c 
index 78b4ffb9c3..3bff1c46cd 100644
--- a/arch/arm/mach-sunxi/dram_sunxi_dw.c
+++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c
@@ -682,7 +682,7 @@ static void mctl_auto_detect_dram_size(uint16_t socid, struct dram_para *para) 
           3,  3,  3,  3,  3,  3,  3,  3,                       \
           3,  3,  3,  3,  2,  0,  0      }

-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
        struct sunxi_mctl_com_reg * const mctl_com =
                        (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
@@ -763,6 +763,6 @@ unsigned long sunxi_dram_init(void)
        mctl_auto_detect_dram_size(socid, &para);
        mctl_set_cr(socid, &para);

-       return (1UL << (para.row_bits + para.bank_bits)) * para.page_size *
+       return (1ULL << (para.row_bits + para.bank_bits)) * para.page_size *
               (para.dual_rank ? 2 : 1);
 }
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 8d707cbac2..5828d47294 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -601,7 +601,7 @@ void sunxi_board_init(void)
 #endif
 #endif
        printf("DRAM:");
-       gd->ram_size = sunxi_dram_init();
+       gd->ram_size = (phys_size_t)sunxi_dram_init();
        printf(" %d MiB\n", (int)(gd->ram_size >> 20));
        if (!gd->ram_size)
                hang();
diff --git a/board/sunxi/dram_sun4i_auto.c b/board/sunxi/dram_sun4i_auto.c 
index 7d4409b51e..293c968f6b 100644
--- a/board/sunxi/dram_sun4i_auto.c
+++ b/board/sunxi/dram_sun4i_auto.c
@@ -29,7 +29,7 @@ static struct dram_para dram_para = {
        .dqs_gating_delay = CONFIG_DRAM_DQS_GATING_DELAY,
 };

-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
        return dramc_init(&dram_para);
 }
diff --git a/board/sunxi/dram_sun5i_auto.c b/board/sunxi/dram_sun5i_auto.c 
index e3fa243267..02e29b215f 100644
--- a/board/sunxi/dram_sun5i_auto.c
+++ b/board/sunxi/dram_sun5i_auto.c
@@ -32,7 +32,7 @@ static struct dram_para dram_para = {
        .dqs_gating_delay = CONFIG_DRAM_DQS_GATING_DELAY,
 };

-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
        return dramc_init(&dram_para);
 }


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to