[U-Boot] [PATCH 3/3] rockchip: fix incorrect detection of ram size

[Dr. Philipp Tomsich]

> On 7 May 2018, at 04:34, Marty E. Plummer <hanetzer at 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 <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) {
>> 
>> 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 at lists.denx.de <mailto:U-Boot at lists.denx.de>
> https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>