> On 7 May 2018, at 04:34, Marty E. Plummer <hanet...@startmail.com> wrote: > > On Mon, May 07, 2018 at 10:20:55AM +0800, Kever Yang wrote: >> Hi Marty, >> >> >> On 05/06/2018 10:25 PM, Marty E. Plummer 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. >> >> I know the root cause for this issue, and I have a local patch for it. >> The rk3288 is 32bit, and 4GB size is just out of range, so we need to before >> the max size before return with '<<20'. Sorry for forgot to send it out. >> >>> >>> 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> >>> >>> 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) { >> >> I don't understand this and follow up changes, we don't really need it, >> isn't it? >> I think don't need the changes before here. > Yeah, that was just another level of indentation for the if (!size_mb) > guard, but I've reworked the patch to not do that as it was pointed out > that since size_mb is initialized to 0 prior. >>> + /* >>> + * 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); >> >> This is what we really need, as Klaus point out, we need to use >> SDRAM_MAX_SIZE >> instead of hard code. > Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, > build and boot tested on my hardware.
In that case you just masked the problem but didn’t solve it: assuming size_mb is size_t (I’ll assume this is 64bit, but did not check), then your 4GB is 0x1_0000_0000 ) which overflows to 0x0 when converted to a u32. In other words: we need to figure out where the truncation occurs (image what happens if a new 32bit processor with LPAE comes out…). >> >> Thanks, >> - Kever >>> } >>> >>> return (size_t)size_mb << 20; >> >> > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de <mailto:U-Boot@lists.denx.de> > https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot> _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot