On Sunday 02 September 2018 11:19 PM, Alexander Graf wrote: > > >> Am 02.09.2018 um 18:04 schrieb Vagrant Cascadian <[email protected]>: >> >>> On 2018-09-02, Alexander Graf <[email protected]> wrote: >>>> On 02.08.18 23:31, Dr. Philipp Tomsich wrote: >>>> >>>>> On 2 Aug 2018, at 22:36, Simon Glass <[email protected]> wrote: >>>>> >>>>> On 26 July 2018 at 07:59, Philipp Tomsich >>>>> <[email protected] >>>>> <mailto:[email protected]>> wrote: >>>>>> Even on 32bit systems a full 4GB of DRAM may be installed and reported >>>>>> by the DRAM controller. Whether these 4GB are larger available >>>>>> depends on the size/configuration of address decoding windows and >>>>>> architectural features (e.g. consider a hypothetical architecture that >>>>>> uses dedicated instructions to access the 'memory-mapped' peripheral >>>>>> IO ranges). In U-Boot, the available DRAM, as reported by the >>>>>> device-model is independent of the accessible DRAM (i.e. adjusted top >>>>>> and effective size). >>>>>> >>>>>> This was first reported against the RK3288, which may have 4GB DRAM >>>>>> attached, but will have a small (e.g. 128MB) window not accessible due >>>>>> to address decoding limitations. >>>>>> >>>>>> The solution is to increase the types used for storing the ram_size to >>>>>> have at least 33 bits even on 32bit systems... i.e. we need to use a >>>>>> u64 for these always (this was previously only the case for >>>>>> PHYS_64BIT, which will have unwanted side-effects for 32bit systems >>>>>> and would require more intrusive changes). >>>>>> >>>>>> This commit changes the size-field in 'struct ram' and the ram_size in >>>>>> 'gd' to be a u64. >>>>>> >>>>>> Reported-by: Marty E. Plummer <[email protected]> >>>>>> Signed-off-by: Philipp Tomsich <[email protected]> >>>>>> --- >>>>>> >>>>>> include/asm-generic/global_data.h | 2 +- >>>>>> include/ram.h | 9 ++++++++- >>>>>> 2 files changed, 9 insertions(+), 2 deletions(-) >>>>>> >>>>> >>>>> Reviewed-by: Simon Glass <[email protected] <mailto:[email protected]>> >>>>> >>>>>> diff --git a/include/asm-generic/global_data.h >>>>>> b/include/asm-generic/global_data.h >>>>>> index 0fd4900..f3d687c 100644 >>>>>> --- a/include/asm-generic/global_data.h >>>>>> +++ b/include/asm-generic/global_data.h >>>>>> @@ -55,7 +55,7 @@ typedef struct global_data { >>>>>> unsigned long ram_base; /* Base address of RAM used by >>>>>> U-Boot */ >>>>>> unsigned long ram_top; /* Top address of RAM used by >>>>>> U-Boot */ >>>>>> unsigned long relocaddr; /* Start address of U-Boot in RAM >>>>>> */ >>>>>> - phys_size_t ram_size; /* RAM size */ >>>>>> + u64 ram_size; /* RAM size */ >>>>>> unsigned long mon_len; /* monitor len */ >>>>>> unsigned long irq_sp; /* irq stack pointer */ >>>>>> unsigned long start_addr_sp; /* start_addr_stackpointer */ >>>>>> diff --git a/include/ram.h b/include/ram.h >>>>>> index 67e22d7..1fe024f 100644 >>>>>> --- a/include/ram.h >>>>>> +++ b/include/ram.h >>>>>> @@ -9,7 +9,14 @@ >>>>>> >>>>>> struct ram_info { >>>>>> phys_addr_t base; >>>>>> - size_t size; >>>>>> + /* >>>>>> + * We use a 64bit type for the size to correctly handle 32bit >>>>>> + * systems with 4GB of DRAM (which would overflow a 32bit type >>>>>> + * and read back as 0). For 64bit systems, we assume (for now) >>>>> >>>>> forever :-) >>>>> >>>>>> + * that they will always have less than 2^65 byte of DRAM >>>>>> + * installed. -- prt >>>>> >>>>> Is the prt your signature? I suggest dropping it. >>>> >>>> In fact it is. But I’ll need to rewrite the entire comment anyway for the >>>> next >>>> version of this series as there’s even more functions and places that the >>>> memory size is stored in... >>>> >>>>> >>>>>> + */ >>>>>> + u64 size; >>> >>> With LPAE available in all modern ARM cores, shouldn't phys_addr_t just >>> be u64? And then we'd probably want to use that throughout the code, right? >> >> Quite a few currently supported boards do not support LPAE, e.g. imx6. > > What I'm trying to say is that we probably want to make phys_addr_t be u64 > when CONFIG_LPAE is set.
That's right. Enabling PHYS_64BIT should be sufficient. Based on this phys_addr_t should be set to u32 or u64. arm already does that[1]. [1] http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/types.h;h=9af7353f0866f05dbe298a603d52d90e9c8e6d28;hb=HEAD Thanks and regards, Lokesh _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
