Re: [PATCH 3/3] rockchip: fix incorrect detection of ram size

["Marty E. Plummer" <hanetzer@startmail.com>]

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@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.
> 
> Thanks,
> - Kever
> >  	}
> >  
> >  	return (size_t)size_mb << 20;
> 
> 
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot