CC Philipp and Simon due maintainership (you may want to use get_maintainer.pl
in the future)
and Kever as the original author of the file.
> On 06.05.2018, at 16:25, Marty E. Plummer <hanet...@startmail.com> wrote:
>
> Taken from coreboot's src/soc/rockchip/rk3288/sdram.c
>
> Without this change, my u-boot build for the asus c201 chromebook (4GiB)
> is incorrectly detected as 0 Bytes of ram.
>
> Signed-off-by: Marty E. Plummer <hanet...@startmail.com>
> ---
> arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------
> 1 file changed, 37 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/sdram_common.c
> b/arch/arm/mach-rockchip/sdram_common.c
> index 76dbdc8715..a9c9f970a4 100644
> --- a/arch/arm/mach-rockchip/sdram_common.c
> +++ b/arch/arm/mach-rockchip/sdram_common.c
> @@ -10,6 +10,8 @@
> #include <asm/io.h>
> #include <asm/arch/sdram_common.h>
> #include <dm/uclass-internal.h>
> +#include <linux/kernel.h>
> +#include <linux/sizes.h>
Is adding these headers really necessary?
common.h already includes kernel.h and sizes.h (with some redirections of
config.h)
> DECLARE_GLOBAL_DATA_PTR;
> size_t rockchip_sdram_size(phys_addr_t reg)
> @@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> size_t size_mb = 0;
> u32 ch;
>
> - u32 sys_reg = readl(reg);
> - u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
> - & SYS_REG_NUM_CH_MASK);
> + if (!size_mb) {
Is this really required? I don’t see a way that size_mb will already be set at
this point.
But it blows up your diff that it takes quite a while to see that your only
real change is
the size_mb = min(...) part at the end.
> - debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
> - for (ch = 0; ch < ch_num; ch++) {
> - rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
> - SYS_REG_RANK_MASK);
> - col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
> - bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
> - cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
> - SYS_REG_CS0_ROW_MASK);
> - cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
> - SYS_REG_CS1_ROW_MASK);
> - bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
> - SYS_REG_BW_MASK));
> - row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
> - SYS_REG_ROW_3_4_MASK;
> + u32 sys_reg = readl(reg);
> + u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
> + & SYS_REG_NUM_CH_MASK);
>
> - chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
> + debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
> + for (ch = 0; ch < ch_num; ch++) {
> + rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
> + SYS_REG_RANK_MASK);
> + col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) &
> SYS_REG_COL_MASK);
> + bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) &
> SYS_REG_BK_MASK);
> + cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
> + SYS_REG_CS0_ROW_MASK);
> + cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
> + SYS_REG_CS1_ROW_MASK);
> + bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
> + SYS_REG_BW_MASK));
> + row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
> + SYS_REG_ROW_3_4_MASK;
>
> - if (rank > 1)
> - chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
> - if (row_3_4)
> - chipsize_mb = chipsize_mb * 3 / 4;
> - size_mb += chipsize_mb;
> - debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
> - rank, col, bk, cs0_row, bw, row_3_4);
> + chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
> +
> + if (rank > 1)
> + chipsize_mb += chipsize_mb >> (cs0_row -
> cs1_row);
> + if (row_3_4)
> + chipsize_mb = chipsize_mb * 3 / 4;
> + size_mb += chipsize_mb;
> + debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4
> %d\n",
> + rank, col, bk, cs0_row, bw, row_3_4);
> + }
> +
> + /*
> + * we use the 0x00000000~0xfeffffff space
> + * since 0xff000000~0xffffffff is soc register space
> + * so we reserve it
> + */
That’s not true for all Rockchip SoCs. On the RK3399 for example the upper
limit
is 0xf8000000. Even on the RK3288 DRAM address range is actually
0x00000000-0xfe00000
> + size_mb = min(size_mb, 0xff000000/SZ_1M);
> }
Should be 0xfe000000 (4G-32MB) for RK3288.
But there is already a define for that, SDRAM_MAX_SIZE is defined in
rkxxx_common.h. Using that one would help to avoid possible breakage
of other Rockchip SoCs.
> return (size_t)size_mb << 20;
> --
> 2.17.0
Regards,
Klaus
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
