[U-Boot] [PATCH 0/9] Support 4GB of memory on 32bit systems
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.
This increases the fields holding the RAM size to have at least 33bits (i.e. we use a u64) and fixes the fallout from the change: some casts, the usage of min() and a few printf formats have to be adjusted.
Philipp Tomsich (9): dm: allow 4GB of DRAM on 32bit systems rockchip: support 4GB DRAM on 32bit systems common: include <inttypes.h> always MIPS: use PRIx64 macros for printing ram size rockchip: rk3368: change type of ram-size field for a min()-calculation ram: stm32mp1: use PRIx64 macros for printing ram size board: keymile: add explicit cast to truncate the 64bit ram size field board: cm_fx6: use PRIx64 macros for printing ram size mpc85xx: add casts for ram size in min() calculation.
arch/arm/include/asm/arch-rockchip/sdram_common.h | 2 +- arch/arm/mach-rockchip/sdram_common.c | 8 ++++---- arch/arm/mach-stm32mp/dram_init.c | 2 +- arch/mips/mach-bmips/dram.c | 2 +- arch/powerpc/cpu/mpc85xx/cpu.c | 6 ++++-- board/compulab/cm_fx6/cm_fx6.c | 3 ++- board/keymile/km_arm/km_arm.c | 6 ++++-- drivers/ram/rockchip/dmc-rk3368.c | 2 +- drivers/ram/stm32mp1/stm32mp1_ram.c | 2 +- include/asm-generic/global_data.h | 2 +- include/common.h | 1 + include/ram.h | 9 ++++++++- 12 files changed, 29 insertions(+), 16 deletions(-)
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 hanetzer@startmail.com Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
include/asm-generic/global_data.h | 2 +- include/ram.h | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-)
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) + * that they will always have less than 2^65 byte of DRAM + * installed. -- prt + */ + u64 size; };
struct ram_ops {
On 26 July 2018 at 07:59, Philipp Tomsich philipp.tomsich@theobroma-systems.com 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 hanetzer@startmail.com Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
include/asm-generic/global_data.h | 2 +- include/ram.h | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
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.
*/u64 size;};
struct ram_ops {
2.1.4
Regards, Simon
On 2 Aug 2018, at 22:36, Simon Glass sjg@chromium.org wrote:
On 26 July 2018 at 07:59, Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> 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 hanetzer@startmail.com Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
include/asm-generic/global_data.h | 2 +- include/ram.h | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
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. -- prtIs 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;};
struct ram_ops {
2.1.4
Regards, Simon
On 02.08.18 23:31, Dr. Philipp Tomsich wrote:
On 2 Aug 2018, at 22:36, Simon Glass sjg@chromium.org wrote:
On 26 July 2018 at 07:59, Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> 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 hanetzer@startmail.com Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
include/asm-generic/global_data.h | 2 +- include/ram.h | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
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. -- prtIs 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?
Alex
On 2018-09-02, Alexander Graf agraf@suse.de wrote:
On 02.08.18 23:31, Dr. Philipp Tomsich wrote:
On 2 Aug 2018, at 22:36, Simon Glass sjg@chromium.org wrote:
On 26 July 2018 at 07:59, Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> 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 hanetzer@startmail.com Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
include/asm-generic/global_data.h | 2 +- include/ram.h | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
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. -- prtIs 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.
live well, vagrant
Am 02.09.2018 um 18:04 schrieb Vagrant Cascadian vagrant@debian.org:
On 2018-09-02, Alexander Graf agraf@suse.de wrote:
On 02.08.18 23:31, Dr. Philipp Tomsich wrote:
On 2 Aug 2018, at 22:36, Simon Glass sjg@chromium.org wrote:
On 26 July 2018 at 07:59, Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> 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 hanetzer@startmail.com Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
include/asm-generic/global_data.h | 2 +- include/ram.h | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
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. -- prtIs 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.
Alex
live well, vagrant
On Sunday 02 September 2018 11:19 PM, Alexander Graf wrote:
Am 02.09.2018 um 18:04 schrieb Vagrant Cascadian vagrant@debian.org:
On 2018-09-02, Alexander Graf agraf@suse.de wrote:
On 02.08.18 23:31, Dr. Philipp Tomsich wrote:
On 2 Aug 2018, at 22:36, Simon Glass sjg@chromium.org wrote:
On 26 July 2018 at 07:59, Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> 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 hanetzer@startmail.com Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
include/asm-generic/global_data.h | 2 +- include/ram.h | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
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. -- prtIs 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=9af...
Thanks and regards, Lokesh
Hi,
On 3 September 2018 at 16:29, Lokesh Vutla lokeshvutla@ti.com wrote:
On Sunday 02 September 2018 11:19 PM, Alexander Graf wrote:
Am 02.09.2018 um 18:04 schrieb Vagrant Cascadian vagrant@debian.org:
On 2018-09-02, Alexander Graf agraf@suse.de wrote:
On 02.08.18 23:31, Dr. Philipp Tomsich wrote:
On 2 Aug 2018, at 22:36, Simon Glass sjg@chromium.org wrote:
On 26 July 2018 at 07:59, Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> 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 hanetzer@startmail.com > Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com > --- > > include/asm-generic/global_data.h | 2 +- > include/ram.h | 9 ++++++++- > 2 files changed, 9 insertions(+), 2 deletions(-) >
Reviewed-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
> 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=9af...
Yes I agree. So will this patch be changed?
Regards, Simon
On 14.09.2018, at 12:53, Simon Glass sjg@chromium.org wrote:
Hi,
On 3 September 2018 at 16:29, Lokesh Vutla lokeshvutla@ti.com wrote:
On Sunday 02 September 2018 11:19 PM, Alexander Graf wrote:
Am 02.09.2018 um 18:04 schrieb Vagrant Cascadian vagrant@debian.org:
On 2018-09-02, Alexander Graf agraf@suse.de wrote:
On 02.08.18 23:31, Dr. Philipp Tomsich wrote:
> On 2 Aug 2018, at 22:36, Simon Glass sjg@chromium.org wrote: > > On 26 July 2018 at 07:59, Philipp Tomsich > <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> 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 hanetzer@startmail.com >> Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com >> --- >> >> include/asm-generic/global_data.h | 2 +- >> include/ram.h | 9 ++++++++- >> 2 files changed, 9 insertions(+), 2 deletions(-) >> > > Reviewed-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org> > >> 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=9af...
Yes I agree. So will this patch be changed?
This patch-set needs to be revised for other reasons… However, PHYS_64BIT is not a valid option for some of the affected chips (e.g. if the address space is indeed 32bit and the address-decode is not straightforward).
E.g. Marty Plummer already reported that the RK3288 breaks when enabling PHYS_64BIT.
Thanks, Philipp.
On 03-09-2018 at 19:59 +0530, Lokesh Vutla wrote:
On Sunday 02 September 2018 11:19 PM, Alexander Graf wrote:
Am 02.09.2018 um 18:04 schrieb Vagrant Cascadian vagrant@debian.org:
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].
That will cause warnings from at least two sunxi drivers. I forgot which one the other was, but sunxi_mmc is one of them:
$ git grep -F -e '(u32 *)' drivers/mmc/sunxi_mmc.c drivers/mmc/sunxi_mmc.c: ccu_reg = (u32 *)ofnode_get_addr(args.node); drivers/mmc/sunxi_mmc.c: mmc_config_clk = (u32 *)ofnode_get_addr(args.node);
becase ofnode_get_addr() is declared like so:
include/dm/ofnode.h:phys_addr_t ofnode_get_addr(ofnode node);
The calculation in `rockchip_sdram_size` would overflow for 4GB on 32bit systems (i.e. when PHYS_64BIT is not defined).
This makes the internal variables and the signature prototype use a u64 to ensure that we can represent the 33rd bit (as in '4GB').
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
arch/arm/include/asm/arch-rockchip/sdram_common.h | 2 +- arch/arm/mach-rockchip/sdram_common.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/sdram_common.h b/arch/arm/include/asm/arch-rockchip/sdram_common.h index 671c318..edf5911 100644 --- a/arch/arm/include/asm/arch-rockchip/sdram_common.h +++ b/arch/arm/include/asm/arch-rockchip/sdram_common.h @@ -50,7 +50,7 @@ #define SYS_REG_DBW_MASK 3
/* Get sdram size decode from reg */ -size_t rockchip_sdram_size(phys_addr_t reg); +u64 rockchip_sdram_size(phys_addr_t reg);
/* Called by U-Boot board_init_r for Rockchip SoCs */ int dram_init(void); diff --git a/arch/arm/mach-rockchip/sdram_common.c b/arch/arm/mach-rockchip/sdram_common.c index 650d53e..6bad537 100644 --- a/arch/arm/mach-rockchip/sdram_common.c +++ b/arch/arm/mach-rockchip/sdram_common.c @@ -11,11 +11,11 @@ #include <dm/uclass-internal.h>
DECLARE_GLOBAL_DATA_PTR; -size_t rockchip_sdram_size(phys_addr_t reg) +u64 rockchip_sdram_size(phys_addr_t reg) { u32 rank, col, bk, cs0_row, cs1_row, bw, row_3_4; - size_t chipsize_mb = 0; - size_t size_mb = 0; + u64 chipsize_mb = 0; + u64 size_mb = 0; u32 ch;
u32 sys_reg = readl(reg); @@ -48,7 +48,7 @@ size_t rockchip_sdram_size(phys_addr_t reg) rank, col, bk, cs0_row, bw, row_3_4); }
- return (size_t)size_mb << 20; + return (u64)size_mb << 20; }
int dram_init(void)
On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
The calculation in `rockchip_sdram_size` would overflow for 4GB on 32bit systems (i.e. when PHYS_64BIT is not defined).
This makes the internal variables and the signature prototype use a u64 to ensure that we can represent the 33rd bit (as in '4GB').
Hi Philipp, just to let you know that this is still not working on the veyron jerry chromebook with 4GB I have here (RK3288). The boot stops at:
U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100) Trying to boot from SPI
U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
Model: Google Jerry DRAM: 0 Bytes
I'm still investigating why but for sure there are several points to fix to have a proper debug, see [0].
Also I was wondering if we should also fix get_effective_memsize() and gd->bd->bi_dram[].size
[0]https://gist.github.com/carlocaione/b93cfd9ee71e07fcf68d5c02256ff0fa
Cheers,
On 26 Jul 2018, at 22:05, Carlo Caione carlo@endlessm.com wrote:
On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
The calculation in `rockchip_sdram_size` would overflow for 4GB on 32bit systems (i.e. when PHYS_64BIT is not defined).
This makes the internal variables and the signature prototype use a u64 to ensure that we can represent the 33rd bit (as in '4GB').
Hi Philipp, just to let you know that this is still not working on the veyron jerry chromebook with 4GB I have here (RK3288). The boot stops at:
U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100) Trying to boot from SPI
U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
Model: Google Jerry DRAM: 0 Bytes
I'm still investigating why but for sure there are several points to fix to have a proper debug, see [0].
Also I was wondering if we should also fix get_effective_memsize() and gd->bd->bi_dram[].size
Yes, we should. I missed that one… I only have RK3368 and RK3399 targets to test here, so your testing is very much appreciated.
[0]https://gist.github.com/carlocaione/b93cfd9ee71e07fcf68d5c02256ff0fa
Cheers,
-- Carlo Caione carlo@endlessm.com
On 26 Jul 2018, at 22:05, Carlo Caione carlo@endlessm.com wrote:
On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
The calculation in `rockchip_sdram_size` would overflow for 4GB on 32bit systems (i.e. when PHYS_64BIT is not defined).
This makes the internal variables and the signature prototype use a u64 to ensure that we can represent the 33rd bit (as in '4GB').
Hi Philipp, just to let you know that this is still not working on the veyron jerry chromebook with 4GB I have here (RK3288). The boot stops at:
U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100) Trying to boot from SPI
U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
Model: Google Jerry DRAM: 0 Bytes
I'm still investigating why but for sure there are several points to fix to have a proper debug, see [0].
Also I was wondering if we should also fix get_effective_memsize() and gd->bd->bi_dram[].size
[0]https://gist.github.com/carlocaione/b93cfd9ee71e07fcf68d5c02256ff0fa
Thanks for identifying these two… I’ll pick them up and include in the next series.
Cheers,
-- Carlo Caione carlo@endlessm.com
On 26 Jul 2018, at 22:05, Carlo Caione carlo@endlessm.com wrote:
On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
The calculation in `rockchip_sdram_size` would overflow for 4GB on 32bit systems (i.e. when PHYS_64BIT is not defined).
This makes the internal variables and the signature prototype use a u64 to ensure that we can represent the 33rd bit (as in '4GB').
Hi Philipp, just to let you know that this is still not working on the veyron jerry chromebook with 4GB I have here (RK3288). The boot stops at:
U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100) Trying to boot from SPI
U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
Model: Google Jerry DRAM: 0 Bytes
I'm still investigating why but for sure there are several points to fix to have a proper debug, see [0].
I reread ‘rockchip_sdram_size’ and it looks as if I forgot to mark the value shifted to create chipsize_mb as ULL. When looking at this code I had an uneasy feeling that this might run into the C type rules (i.e. 1 will be a 32bit integer and shifting it by 32 would result in 0), but I didn’t think that this would ever be the case and that any 4GB DRAM setup would be made up of multiple channels or of multiple ranks.
Also I was wondering if we should also fix get_effective_memsize() and gd->bd->bi_dram[].size
[0]https://gist.github.com/carlocaione/b93cfd9ee71e07fcf68d5c02256ff0fa
Cheers,
-- Carlo Caione carlo@endlessm.com
On Fri, 2018-07-27 at 00:54 +0200, Dr. Philipp Tomsich wrote:
On 26 Jul 2018, at 22:05, Carlo Caione carlo@endlessm.com wrote:
On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
The calculation in `rockchip_sdram_size` would overflow for 4GB on 32bit systems (i.e. when PHYS_64BIT is not defined).
This makes the internal variables and the signature prototype use a u64 to ensure that we can represent the 33rd bit (as in '4GB').
Hi Philipp, just to let you know that this is still not working on the veyron jerry chromebook with 4GB I have here (RK3288). The boot stops at:
U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100) Trying to boot from SPI
U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
Model: Google Jerry DRAM: 0 Bytes
I'm still investigating why but for sure there are several points to fix to have a proper debug, see [0].
I reread ‘rockchip_sdram_size’ and it looks as if I forgot to mark the value shifted to create chipsize_mb as ULL. When looking at this code I had an uneasy feeling that this might run into the C type rules (i.e. 1 will be a 32bit integer and shifting it by 32 would result in 0), but I didn’t think that this would ever be the case and that any 4GB DRAM setup would be made up of multiple channels or of multiple ranks.
It doesn't hurt but rockchip_sdram_size() is returning the correct value of 0x100000000 in my tests.
On 27 Jul 2018, at 09:50, Carlo Caione carlo.caione@gmail.com wrote:
On Fri, 2018-07-27 at 00:54 +0200, Dr. Philipp Tomsich wrote:
On 26 Jul 2018, at 22:05, Carlo Caione carlo@endlessm.com wrote:
On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
The calculation in `rockchip_sdram_size` would overflow for 4GB on 32bit systems (i.e. when PHYS_64BIT is not defined).
This makes the internal variables and the signature prototype use a u64 to ensure that we can represent the 33rd bit (as in '4GB').
Hi Philipp, just to let you know that this is still not working on the veyron jerry chromebook with 4GB I have here (RK3288). The boot stops at:
U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100) Trying to boot from SPI
U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
Model: Google Jerry DRAM: 0 Bytes
I'm still investigating why but for sure there are several points to fix to have a proper debug, see [0].
I reread ‘rockchip_sdram_size’ and it looks as if I forgot to mark the value shifted to create chipsize_mb as ULL. When looking at this code I had an uneasy feeling that this might run into the C type rules (i.e. 1 will be a 32bit integer and shifting it by 32 would result in 0), but I didn’t think that this would ever be the case and that any 4GB DRAM setup would be made up of multiple channels or of multiple ranks.
It doesn't hurt but rockchip_sdram_size() is returning the correct value of 0x100000000 in my tests.
The 'gd->bd->bi_dram[i].size’ path will also be involved and truncate this later on, but I am not convinced that it’s worth fixing the dram banks info for the general case.
Generally, typing for these bi_dram structures seems a mess: they are often cast to 32bit and sometimes to 64bit in common code (i.e. fdtdec.c uses unsigned long long). I hope I can avoid touching this code.
Btw, here’s a gem from board_f.c (these two lines are after each other):
gd->ram_top = gd->ram_base + get_effective_memsize(); gd->ram_top = board_get_usable_ram_top(gd->mon_len);
As the first line is clearly deal (except the fact that the compiler can’t tell that get_effective_memsize() should be side-effect free), we can drop it. I’ll send a separate patch for this to be picked up by Tom…
—Phil.
With the ram-size variable changed to u64, we'll need appropriate macros for printing u64 values correctly either in 32bit builds (i.e. ILP32 models) or in 64bit builds (i.e. LP64 models). Best to make the PRIx64 macro available everywhere.
This include inttypes.h from common.h to make the various macros for formatted printing available to everyone.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
include/common.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/common.h b/include/common.h index 940161f..9de9145 100644 --- a/include/common.h +++ b/include/common.h @@ -30,6 +30,7 @@ typedef volatile unsigned char vu_char; #include <linux/stringify.h> #include <asm/ptrace.h> #include <stdarg.h> +#include <inttypes.h> #include <stdio.h> #include <linux/kernel.h>
2018-07-26 22:59 GMT+09:00 Philipp Tomsich philipp.tomsich@theobroma-systems.com:
With the ram-size variable changed to u64, we'll need appropriate macros for printing u64 values correctly either in 32bit builds (i.e. ILP32 models) or in 64bit builds (i.e. LP64 models). Best to make the PRIx64 macro available everywhere.
This include inttypes.h from common.h to make the various macros for formatted printing available to everyone.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
NACK.
PRIx64 is evil crap. I would make the code super ugly. Do not use it.
The right thing to do is use the same typedefs for all architectures.
typedef unsigned char u8; typedef unsigned short u16; typedef unsigned int u32; typedef unsigned long long u64;
This works for both ILP32 and LP64.
Use '%llx' for printing u64 variables _always_.
This is what Linux exactly does.
In fact, Linux defined fixed-width types differently for different architectures in old days.
After long time effort, Linux unified the fixed-width types for the kernel space.
https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/int-l...
See Linux commit:
commit 0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf Author: Geert Uytterhoeven geert@linux-m68k.org Date: Thu Jan 23 15:53:43 2014 -0800
asm/types.h: Remove include/asm-generic/int-l64.h
And, I fixed ARM Trusted Firmware in the same way:
https://github.com/ARM-software/arm-trusted-firmware/commit/0a2d5b43c81ed613...
U-Boot is still doing wrong, and core developers in U-Boot do not understand this, unfortunately.
include/common.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/common.h b/include/common.h index 940161f..9de9145 100644 --- a/include/common.h +++ b/include/common.h @@ -30,6 +30,7 @@ typedef volatile unsigned char vu_char; #include <linux/stringify.h> #include <asm/ptrace.h> #include <stdarg.h> +#include <inttypes.h> #include <stdio.h> #include <linux/kernel.h>
-- 2.1.4
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
+Tom
Hi Masahiro,
On 30 July 2018 at 19:41, Masahiro Yamada yamada.masahiro@socionext.com wrote:
2018-07-26 22:59 GMT+09:00 Philipp Tomsich philipp.tomsich@theobroma-systems.com:
With the ram-size variable changed to u64, we'll need appropriate macros for printing u64 values correctly either in 32bit builds (i.e. ILP32 models) or in 64bit builds (i.e. LP64 models). Best to make the PRIx64 macro available everywhere.
This include inttypes.h from common.h to make the various macros for formatted printing available to everyone.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
NACK.
PRIx64 is evil crap. I would make the code super ugly. Do not use it.
The right thing to do is use the same typedefs for all architectures.
typedef unsigned char u8; typedef unsigned short u16; typedef unsigned int u32; typedef unsigned long long u64;
This works for both ILP32 and LP64.
Use '%llx' for printing u64 variables _always_.
This is what Linux exactly does.
In fact, Linux defined fixed-width types differently for different architectures in old days.
After long time effort, Linux unified the fixed-width types for the kernel space.
https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/int-l...
See Linux commit:
commit 0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf Author: Geert Uytterhoeven geert@linux-m68k.org Date: Thu Jan 23 15:53:43 2014 -0800
asm/types.h: Remove include/asm-generic/int-l64.hAnd, I fixed ARM Trusted Firmware in the same way:
https://github.com/ARM-software/arm-trusted-firmware/commit/0a2d5b43c81ed613...
U-Boot is still doing wrong, and core developers in U-Boot do not understand this, unfortunately.
While this works in many cases we do seem to have problems with some toolchains. Perhaps things are better now as my problems were a a few years back. Things like size_t with %z caused problems too. I remember m68k producing warnings when I tried this.
I am certainly interested in converting over to this other approach. I am also OK with the PRi stuff, since it only affects a relatively small number of cases.
Regards, Simon
On Thu, Aug 02, 2018 at 10:56:11AM -0600, Simon Glass wrote:
+Tom
Hi Masahiro,
On 30 July 2018 at 19:41, Masahiro Yamada yamada.masahiro@socionext.com wrote:
2018-07-26 22:59 GMT+09:00 Philipp Tomsich philipp.tomsich@theobroma-systems.com:
With the ram-size variable changed to u64, we'll need appropriate macros for printing u64 values correctly either in 32bit builds (i.e. ILP32 models) or in 64bit builds (i.e. LP64 models). Best to make the PRIx64 macro available everywhere.
This include inttypes.h from common.h to make the various macros for formatted printing available to everyone.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
NACK.
PRIx64 is evil crap. I would make the code super ugly. Do not use it.
The right thing to do is use the same typedefs for all architectures.
typedef unsigned char u8; typedef unsigned short u16; typedef unsigned int u32; typedef unsigned long long u64;
This works for both ILP32 and LP64.
Use '%llx' for printing u64 variables _always_.
This is what Linux exactly does.
In fact, Linux defined fixed-width types differently for different architectures in old days.
After long time effort, Linux unified the fixed-width types for the kernel space.
https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/int-l...
See Linux commit:
commit 0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf Author: Geert Uytterhoeven geert@linux-m68k.org Date: Thu Jan 23 15:53:43 2014 -0800
asm/types.h: Remove include/asm-generic/int-l64.hAnd, I fixed ARM Trusted Firmware in the same way:
https://github.com/ARM-software/arm-trusted-firmware/commit/0a2d5b43c81ed613...
U-Boot is still doing wrong, and core developers in U-Boot do not understand this, unfortunately.
While this works in many cases we do seem to have problems with some toolchains. Perhaps things are better now as my problems were a a few years back. Things like size_t with %z caused problems too. I remember m68k producing warnings when I tried this.
I am certainly interested in converting over to this other approach. I am also OK with the PRi stuff, since it only affects a relatively small number of cases.
It would certainly be worth giving things another try with current compilers.
2018-08-04 0:01 GMT+09:00 Tom Rini trini@konsulko.com:
On Thu, Aug 02, 2018 at 10:56:11AM -0600, Simon Glass wrote:
+Tom
Hi Masahiro,
On 30 July 2018 at 19:41, Masahiro Yamada yamada.masahiro@socionext.com wrote:
2018-07-26 22:59 GMT+09:00 Philipp Tomsich philipp.tomsich@theobroma-systems.com:
With the ram-size variable changed to u64, we'll need appropriate macros for printing u64 values correctly either in 32bit builds (i.e. ILP32 models) or in 64bit builds (i.e. LP64 models). Best to make the PRIx64 macro available everywhere.
This include inttypes.h from common.h to make the various macros for formatted printing available to everyone.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
NACK.
PRIx64 is evil crap. I would make the code super ugly. Do not use it.
The right thing to do is use the same typedefs for all architectures.
typedef unsigned char u8; typedef unsigned short u16; typedef unsigned int u32; typedef unsigned long long u64;
This works for both ILP32 and LP64.
Use '%llx' for printing u64 variables _always_.
This is what Linux exactly does.
In fact, Linux defined fixed-width types differently for different architectures in old days.
After long time effort, Linux unified the fixed-width types for the kernel space.
https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/int-l...
See Linux commit:
commit 0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf Author: Geert Uytterhoeven geert@linux-m68k.org Date: Thu Jan 23 15:53:43 2014 -0800
asm/types.h: Remove include/asm-generic/int-l64.hAnd, I fixed ARM Trusted Firmware in the same way:
https://github.com/ARM-software/arm-trusted-firmware/commit/0a2d5b43c81ed613...
U-Boot is still doing wrong, and core developers in U-Boot do not understand this, unfortunately.
While this works in many cases we do seem to have problems with some toolchains. Perhaps things are better now as my problems were a a few years back.
Please pin-point the pre-built toolchains with which you had problems.
In other words, please show a compiler that has:
sizeof(int) != 4
or
sizeof(long long) != 8
Things like size_t with %z caused problems too. I remember m68k producing warnings when I tried this.
This is true, but the typedefs of uint{8,16,32,64}_t and the typedef of size_t are _different_ problems.
Please do not mix them up to exaggerate things.
Most 32 bit architectures use "unsigned int" size_t, and all 64 bit architectures use "unsigned long" size_t as stated in https://github.com/torvalds/linux/blob/v4.17/include/uapi/asm-generic/posix_...
Linux kernel hard-codes size_t and ssize_t, thus people are supposed to use a compiler with proper size_t define.
Such compilers are provided in https://mirrors.edge.kernel.org/pub/tools/crosstool/
If you are not happy about hard-coding size_t, you can do like this:
typedef __SIZE_TYPE__ size_t;
typedef __typeof(__builtin_choose_expr(__builtin_types_compatible_p(size_t, unsigned long), (long)0, (int)0)) ssize_t;
I am certainly interested in converting over to this other approach. I am also OK with the PRi stuff, since it only affects a relatively small number of cases.
Whether the code is right or not should not be judged by the number of use cases.
It would certainly be worth giving things another try with current compilers.
-- Tom
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hi Philipp
-----Original Message----- From: Philipp Tomsich philipp.tomsich@theobroma-systems.com Sent: jeudi 26 juillet 2018 16:00
With the ram-size variable changed to u64, we'll need appropriate macros for printing u64 values correctly either in 32bit builds (i.e. ILP32 models) or in 64bit builds (i.e. LP64 models). Best to make the PRIx64 macro available everywhere.
This include inttypes.h from common.h to make the various macros for formatted printing available to everyone.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
include/common.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/common.h b/include/common.h index 940161f..9de9145 100644 --- a/include/common.h +++ b/include/common.h @@ -30,6 +30,7 @@ typedef volatile unsigned char vu_char; #include <linux/stringify.h> #include <asm/ptrace.h> #include <stdarg.h> +#include <inttypes.h> #include <stdio.h> #include <linux/kernel.h>
I think the include should be moved after the line 40 and __STDC_FORMAT_MACROS definition :
-/* Bring in printf format macros if inttypes.h is included */ +/* Bring in printf format macros defined in inttypes.h */ #define __STDC_FORMAT_MACROS +#include <inttypes.h>
Regards Patrick
With the recent changes of the underlying types for the ram size, we need to adjust the formatting. Use PRIx64 to print the (now) u64 field.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
arch/mips/mach-bmips/dram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/mach-bmips/dram.c b/arch/mips/mach-bmips/dram.c index 87ced7c..0884bce 100644 --- a/arch/mips/mach-bmips/dram.c +++ b/arch/mips/mach-bmips/dram.c @@ -28,7 +28,7 @@ int dram_init(void) return 0; }
- debug("SDRAM base=%zx, size=%x\n", ram.base, ram.size); + debug("SDRAM base=%zx, size=%" PRIx64 "\n", ram.base, ram.size);
gd->ram_size = ram.size;
On 26 July 2018 at 07:59, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
With the recent changes of the underlying types for the ram size, we need to adjust the formatting. Use PRIx64 to print the (now) u64 field.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
arch/mips/mach-bmips/dram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
2018-07-26 15:59 GMT+02:00 Philipp Tomsich philipp.tomsich@theobroma-systems.com:
With the recent changes of the underlying types for the ram size, we need to adjust the formatting. Use PRIx64 to print the (now) u64 field.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
arch/mips/mach-bmips/dram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Daniel Schwierzeck daniel.schwierzeck@gmail.com
With the recent changes of the underlying types for the ram size, we need to cast the constant used in clamping to u64 instead of size_t.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
drivers/ram/rockchip/dmc-rk3368.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ram/rockchip/dmc-rk3368.c b/drivers/ram/rockchip/dmc-rk3368.c index 8d1b9fa..35d7605 100644 --- a/drivers/ram/rockchip/dmc-rk3368.c +++ b/drivers/ram/rockchip/dmc-rk3368.c @@ -960,7 +960,7 @@ static int rk3368_dmc_probe(struct udevice *dev) * is SoC register space (i.e. reserved), and 0xfe000000~0xfeffffff is * inaccessible for some IP controller. */ - priv->info.size = min(priv->info.size, (size_t)0xfe000000); + priv->info.size = min(priv->info.size, (u64)0xfe000000);
return 0; }
Hi Phiipp,
On 26 July 2018 at 07:59, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
With the recent changes of the underlying types for the ram size, we need to cast the constant used in clamping to u64 instead of size_t.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
drivers/ram/rockchip/dmc-rk3368.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/drivers/ram/rockchip/dmc-rk3368.c b/drivers/ram/rockchip/dmc-rk3368.c index 8d1b9fa..35d7605 100644 --- a/drivers/ram/rockchip/dmc-rk3368.c +++ b/drivers/ram/rockchip/dmc-rk3368.c @@ -960,7 +960,7 @@ static int rk3368_dmc_probe(struct udevice *dev) * is SoC register space (i.e. reserved), and 0xfe000000~0xfeffffff is * inaccessible for some IP controller. */
priv->info.size = min(priv->info.size, (size_t)0xfe000000);
priv->info.size = min(priv->info.size, (u64)0xfe000000);
Why case at all? Is it because min() is a macro?
return 0;}
2.1.4
Regards, Simon
With the recent changes of the underlying types for the ram size, we need to adjust the formatting. Use PRIx64 to print the (now) u64 field.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
arch/arm/mach-stm32mp/dram_init.c | 2 +- drivers/ram/stm32mp1/stm32mp1_ram.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-stm32mp/dram_init.c b/arch/arm/mach-stm32mp/dram_init.c index 7688b3e..e4c6302 100644 --- a/arch/arm/mach-stm32mp/dram_init.c +++ b/arch/arm/mach-stm32mp/dram_init.c @@ -25,7 +25,7 @@ int dram_init(void) debug("Cannot get RAM size: %d\n", ret); return ret; } - debug("RAM init base=%lx, size=%x\n", ram.base, ram.size); + debug("RAM init base=%lx, size=%" PRIx64 "\n", ram.base, ram.size);
gd->ram_size = ram.size;
diff --git a/drivers/ram/stm32mp1/stm32mp1_ram.c b/drivers/ram/stm32mp1/stm32mp1_ram.c index bd497a3..faf78b2 100644 --- a/drivers/ram/stm32mp1/stm32mp1_ram.c +++ b/drivers/ram/stm32mp1/stm32mp1_ram.c @@ -130,7 +130,7 @@ static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev)
/* check memory access for all memory */ if (config.info.size != priv->info.size) { - printf("DDR invalid size : 0x%x, expected 0x%x\n", + printf("DDR invalid size : 0x%" PRIx64 ", expected 0x%x\n", priv->info.size, config.info.size); return -EINVAL; }
Hi Philipp
-----Original Message----- From: Philipp Tomsich philipp.tomsich@theobroma-systems.com Sent: jeudi 26 juillet 2018 16:00
With the recent changes of the underlying types for the ram size, we need to adjust the formatting. Use PRIx64 to print the (now) u64 field.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
arch/arm/mach-stm32mp/dram_init.c | 2 +- drivers/ram/stm32mp1/stm32mp1_ram.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-stm32mp/dram_init.c b/arch/arm/mach- stm32mp/dram_init.c index 7688b3e..e4c6302 100644 --- a/arch/arm/mach-stm32mp/dram_init.c +++ b/arch/arm/mach-stm32mp/dram_init.c @@ -25,7 +25,7 @@ int dram_init(void) debug("Cannot get RAM size: %d\n", ret); return ret; }
- debug("RAM init base=%lx, size=%x\n", ram.base, ram.size);
debug("RAM init base=%lx, size=%" PRIx64 "\n", ram.base, ram.size);
gd->ram_size = ram.size;
diff --git a/drivers/ram/stm32mp1/stm32mp1_ram.c b/drivers/ram/stm32mp1/stm32mp1_ram.c index bd497a3..faf78b2 100644 --- a/drivers/ram/stm32mp1/stm32mp1_ram.c +++ b/drivers/ram/stm32mp1/stm32mp1_ram.c @@ -130,7 +130,7 @@ static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev)
/* check memory access for all memory */ if (config.info.size != priv->info.size) {
printf("DDR invalid size : 0x%x, expected 0x%x\n",
printf("DDR invalid size : 0x%" PRIx64 ", expected 0x%x\n", priv->info.size, config.info.size);-- 2.1.4
Tested on STM32MP157c EV1 board with debug activated for RAM driver. The traces are displayed correctly.
Reviewed-by: Patrick Delaunay patrick.delaunay@st.com Tested-by: Patrick Delaunay patrick.delaunay@st.com
Regard Patrick
On 26 July 2018 at 07:59, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
With the recent changes of the underlying types for the ram size, we need to adjust the formatting. Use PRIx64 to print the (now) u64 field.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
arch/arm/mach-stm32mp/dram_init.c | 2 +- drivers/ram/stm32mp1/stm32mp1_ram.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
With the change to a 64bit ram size field for 32bit architectures, we need to add an explicit cast to truncate when converting to a pointer. This casts to 'void*' through 'uintptr_t'.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
board/keymile/km_arm/km_arm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/keymile/km_arm/km_arm.c b/board/keymile/km_arm/km_arm.c index ea03be9..13304cf 100644 --- a/board/keymile/km_arm/km_arm.c +++ b/board/keymile/km_arm/km_arm.c @@ -521,13 +521,15 @@ int post_hotkeys_pressed(void)
ulong post_word_load(void) { - void* addr = (void *) (gd->ram_size - BOOTCOUNT_ADDR + POST_WORD_OFF); + void *addr = (void *)((uintptr_t)gd->ram_size - + BOOTCOUNT_ADDR + POST_WORD_OFF); return in_le32(addr);
} void post_word_store(ulong value) { - void* addr = (void *) (gd->ram_size - BOOTCOUNT_ADDR + POST_WORD_OFF); + void *addr = (void *)((uintptr_t)gd->ram_size - + BOOTCOUNT_ADDR + POST_WORD_OFF); out_le32(addr, value); }
On 26 July 2018 at 07:59, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
With the change to a 64bit ram size field for 32bit architectures, we need to add an explicit cast to truncate when converting to a pointer. This casts to 'void*' through 'uintptr_t'.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
board/keymile/km_arm/km_arm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
With the recent changes of the underlying types for the ram size, we need to adjust the formatting. Use PRIx64 to print the (now) u64 field.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
board/compulab/cm_fx6/cm_fx6.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c index c114cdc..b163abf 100644 --- a/board/compulab/cm_fx6/cm_fx6.c +++ b/board/compulab/cm_fx6/cm_fx6.c @@ -704,7 +704,8 @@ int dram_init(void) gd->ram_size -= 0x100000; break; default: - printf("ERROR: Unsupported DRAM size 0x%lx\n", gd->ram_size); + printf("ERROR: Unsupported DRAM size 0x%" PRIx64 "\n", + gd->ram_size); return -1; }
2018-07-26 22:59 GMT+09:00 Philipp Tomsich philipp.tomsich@theobroma-systems.com:
With the recent changes of the underlying types for the ram size, we need to adjust the formatting. Use PRIx64 to print the (now) u64 field.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
As I commented in 3/9, using PRIx64 is wrong.
board/compulab/cm_fx6/cm_fx6.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c index c114cdc..b163abf 100644 --- a/board/compulab/cm_fx6/cm_fx6.c +++ b/board/compulab/cm_fx6/cm_fx6.c @@ -704,7 +704,8 @@ int dram_init(void) gd->ram_size -= 0x100000; break; default:
printf("ERROR: Unsupported DRAM size 0x%lx\n", gd->ram_size);
printf("ERROR: Unsupported DRAM size 0x%" PRIx64 "\n",gd->ram_size); return -1; }-- 2.1.4
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On 30 July 2018 at 19:42, Masahiro Yamada yamada.masahiro@socionext.com wrote:
2018-07-26 22:59 GMT+09:00 Philipp Tomsich philipp.tomsich@theobroma-systems.com:
With the recent changes of the underlying types for the ram size, we need to adjust the formatting. Use PRIx64 to print the (now) u64 field.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
As I commented in 3/9, using PRIx64 is wrong.
I'm happy enough if we can more to using fixed types (long int = 32-bit, long long = 64-bit). But is anyone actually working on this?
For now this is standard C and seems good enough to me.
Reviewed-by: Simon Glass sjg@chromium.org
With the change of the ram size fields to u64, we need to use explicit casts to ensure that min()-implementation sees the same type for both its arguments.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
arch/powerpc/cpu/mpc85xx/cpu.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/cpu.c b/arch/powerpc/cpu/mpc85xx/cpu.c index bf48836..5664872 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu.c +++ b/arch/powerpc/cpu/mpc85xx/cpu.c @@ -604,7 +604,8 @@ static int reset_tlb(phys_addr_t p_addr, u32 size, phys_addr_t *phys_offset) int arch_memory_test_advance(u32 *vstart, u32 *size, phys_addr_t *phys_offset) { phys_addr_t test_cap, p_addr; - phys_size_t p_size = min(gd->ram_size, CONFIG_MAX_MEM_MAPPED); + phys_size_t p_size = min((phys_size_t)gd->ram_size, + (phys_size_t)CONFIG_MAX_MEM_MAPPED);
#if !defined(CONFIG_PHYS_64BIT) || \ !defined(CONFIG_SYS_INIT_RAM_ADDR_PHYS) || \ @@ -632,7 +633,8 @@ int arch_memory_test_advance(u32 *vstart, u32 *size, phys_addr_t *phys_offset) /* initialization for testing area */ int arch_memory_test_prepare(u32 *vstart, u32 *size, phys_addr_t *phys_offset) { - phys_size_t p_size = min(gd->ram_size, CONFIG_MAX_MEM_MAPPED); + phys_size_t p_size = min((phys_size_t)gd->ram_size, + (phys_size_t)CONFIG_MAX_MEM_MAPPED);
*vstart = CONFIG_SYS_DDR_SDRAM_BASE; *size = (u32) p_size; /* CONFIG_MAX_MEM_MAPPED < 4G */
On 07/26/2018 07:00 AM, Philipp Tomsich wrote:
With the change of the ram size fields to u64, we need to use explicit casts to ensure that min()-implementation sees the same type for both its arguments.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
arch/powerpc/cpu/mpc85xx/cpu.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/cpu.c b/arch/powerpc/cpu/mpc85xx/cpu.c index bf48836..5664872 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu.c +++ b/arch/powerpc/cpu/mpc85xx/cpu.c @@ -604,7 +604,8 @@ static int reset_tlb(phys_addr_t p_addr, u32 size, phys_addr_t *phys_offset) int arch_memory_test_advance(u32 *vstart, u32 *size, phys_addr_t *phys_offset) { phys_addr_t test_cap, p_addr;
- phys_size_t p_size = min(gd->ram_size, CONFIG_MAX_MEM_MAPPED);
- phys_size_t p_size = min((phys_size_t)gd->ram_size,
(phys_size_t)CONFIG_MAX_MEM_MAPPED);#if !defined(CONFIG_PHYS_64BIT) || \ !defined(CONFIG_SYS_INIT_RAM_ADDR_PHYS) || \ @@ -632,7 +633,8 @@ int arch_memory_test_advance(u32 *vstart, u32 *size, phys_addr_t *phys_offset) /* initialization for testing area */ int arch_memory_test_prepare(u32 *vstart, u32 *size, phys_addr_t *phys_offset) {
- phys_size_t p_size = min(gd->ram_size, CONFIG_MAX_MEM_MAPPED);
phys_size_t p_size = min((phys_size_t)gd->ram_size,
(phys_size_t)CONFIG_MAX_MEM_MAPPED);*vstart = CONFIG_SYS_DDR_SDRAM_BASE; *size = (u32) p_size; /* CONFIG_MAX_MEM_MAPPED < 4G */
We didn't have any issue with 36-bit physical address on mpc85xx. I see this change is needed after you change gd->ram_size.
Reviewed-by: York Sun york.sun@nxp.com
On Thu, Jul 26, 2018 at 03:59:42PM +0200, Philipp Tomsich 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.
This increases the fields holding the RAM size to have at least 33bits (i.e. we use a u64) and fixes the fallout from the change: some casts, the usage of min() and a few printf formats have to be adjusted.
Philipp Tomsich (9): dm: allow 4GB of DRAM on 32bit systems rockchip: support 4GB DRAM on 32bit systems common: include <inttypes.h> always MIPS: use PRIx64 macros for printing ram size rockchip: rk3368: change type of ram-size field for a min()-calculation ram: stm32mp1: use PRIx64 macros for printing ram size board: keymile: add explicit cast to truncate the 64bit ram size field board: cm_fx6: use PRIx64 macros for printing ram size mpc85xx: add casts for ram size in min() calculation.
arch/arm/include/asm/arch-rockchip/sdram_common.h | 2 +- arch/arm/mach-rockchip/sdram_common.c | 8 ++++---- arch/arm/mach-stm32mp/dram_init.c | 2 +- arch/mips/mach-bmips/dram.c | 2 +- arch/powerpc/cpu/mpc85xx/cpu.c | 6 ++++-- board/compulab/cm_fx6/cm_fx6.c | 3 ++- board/keymile/km_arm/km_arm.c | 6 ++++-- drivers/ram/rockchip/dmc-rk3368.c | 2 +- drivers/ram/stm32mp1/stm32mp1_ram.c | 2 +- include/asm-generic/global_data.h | 2 +- include/common.h | 1 + include/ram.h | 9 ++++++++- 12 files changed, 29 insertions(+), 16 deletions(-)
Please rework this on top of https://patchwork.ozlabs.org/user/todo/uboot/?series=59452 thanks!
participants (14)
-
Alexander Graf -
Carlo Caione -
Carlo Caione -
Daniel Schwierzeck -
Dr. Philipp Tomsich -
Lokesh Vutla -
Masahiro Yamada -
Patrick DELAUNAY -
Philipp Tomsich
-
Rask Ingemann Lambertsen -
Simon Glass -
Tom Rini -
Vagrant Cascadian -
York Sun