On Mon, May 07, 2018 at 12:19:11AM +0200, Dr. Philipp Tomsich wrote:
>
> > On 6 May 2018, at 16:25, Marty E. Plummer <hanetzer at 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.
>
> Could you elaborate what the change is and what root-cause this addresses (4GB
> reporting as 0 sounds a bit like a 32bit type overflowing)?
> It's really hard to tell from the patch below (which seems to have everything simply
> reformatted to a different indentation)...
>
if (!size_mb) {} wrapping, plus the min code near the end. However,
actual testing on hardware shows this if guard to be unneeded, so I'll
be dropping it. I was just taking what was different from coreboot's
implementation (which I knew to work), but not all was needed it seems.
> >
> > Signed-off-by: Marty E. Plummer <hanetzer at 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>
> >
> > 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) {
>
> Given that there's a "size_mb = 0" just above it, this will always evaluate
> to true...
>
Very true, next patch revision will do away with this if guard, as its
unneeded according to hardware retesting.
> >
> > - 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
> > + */
> > + size_mb = min(size_mb, 0xff000000/SZ_1M);
> > }
> >
> > return (size_t)size_mb << 20;
> > --
> > 2.17.0
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
>
