[U-Boot] [PATCH 1/2] imx: mx7: fix potential overflow in imx_ddr_size()
From: Marcel Ziswiler marcel.ziswiler@toradex.com
The imx_ddr_size() function may overflow as it is possible to kind of over provision the DDR controller. Fix this by capping it to 2 GB which is the maximum allowed size as per reference manual.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
--
arch/arm/mach-imx/mx7/ddr.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-imx/mx7/ddr.c b/arch/arm/mach-imx/mx7/ddr.c index f19aeb8042..9713835bf2 100644 --- a/arch/arm/mach-imx/mx7/ddr.c +++ b/arch/arm/mach-imx/mx7/ddr.c @@ -196,5 +196,9 @@ unsigned int imx_ddr_size(void) if (field_val <= 29) bits++;
+ /* cap to max 2 GB */ + if (bits > 31) + bits = 31; + return 1 << bits; }
From: Fabio Estevam festevam@gmail.com
Rather than passing a hardcoded maxsize to the generic get_ram_size() function use the i.MX 7 specific imx_ddr_size() function, which extracts the memory size at runtime by reading the DDR controller registers.
This is a purely cosmetic change as the generic get_ram_size() function already took care of properly automatically detecting 256MB, 512MB or 1GB modules.
Signed-off-by: Fabio Estevam festevam@gmail.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
---
board/toradex/colibri_imx7/colibri_imx7.c | 2 +- include/configs/colibri_imx7.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/board/toradex/colibri_imx7/colibri_imx7.c b/board/toradex/colibri_imx7/colibri_imx7.c index 2b7591eb00..a4c99626b4 100644 --- a/board/toradex/colibri_imx7/colibri_imx7.c +++ b/board/toradex/colibri_imx7/colibri_imx7.c @@ -52,7 +52,7 @@ DECLARE_GLOBAL_DATA_PTR;
int dram_init(void) { - gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); + gd->ram_size = get_ram_size((void *)PHYS_SDRAM, imx_ddr_size());
return 0; } diff --git a/include/configs/colibri_imx7.h b/include/configs/colibri_imx7.h index ff6bd678cf..02849ba35f 100644 --- a/include/configs/colibri_imx7.h +++ b/include/configs/colibri_imx7.h @@ -14,7 +14,6 @@ #include "mx7_common.h"
/*#define CONFIG_DBG_MONITOR*/ -#define PHYS_SDRAM_SIZE SZ_1G
/* Size of malloc() pool */ #define CONFIG_SYS_MALLOC_LEN (32 * SZ_1M)
Hi Marcel,
On Wed, Sep 19, 2018 at 8:01 AM, Marcel Ziswiler marcel@ziswiler.com wrote:
From: Fabio Estevam festevam@gmail.com
Rather than passing a hardcoded maxsize to the generic get_ram_size() function use the i.MX 7 specific imx_ddr_size() function, which extracts the memory size at runtime by reading the DDR controller registers.
This is a purely cosmetic change as the generic get_ram_size() function already took care of properly automatically detecting 256MB, 512MB or 1GB modules.
Signed-off-by: Fabio Estevam festevam@gmail.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
Thanks for the respin:
Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
Hi,
On 9/19/18 4:01 AM, Marcel Ziswiler wrote:
From: Fabio Estevam festevam@gmail.com
Rather than passing a hardcoded maxsize to the generic get_ram_size() function use the i.MX 7 specific imx_ddr_size() function, which extracts the memory size at runtime by reading the DDR controller registers.
This is a purely cosmetic change as the generic get_ram_size() function already took care of properly automatically detecting 256MB, 512MB or 1GB modules.
Signed-off-by: Fabio Estevam festevam@gmail.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
Hm, with that we take the MMDC register information as the upper bound, and use regular U-Boot get_ram_size() to determine size by poking memory addresses. Seems sensible.
Acked-by: Stefan Agner stefan.agner@toradex.com
Fabio, I guess other boards use SPL to use different MMDC configuration for different memory size? Is there a downside doing this size over-provisioning?
-- Stefan
board/toradex/colibri_imx7/colibri_imx7.c | 2 +- include/configs/colibri_imx7.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/board/toradex/colibri_imx7/colibri_imx7.c b/board/toradex/colibri_imx7/colibri_imx7.c index 2b7591eb00..a4c99626b4 100644 --- a/board/toradex/colibri_imx7/colibri_imx7.c +++ b/board/toradex/colibri_imx7/colibri_imx7.c @@ -52,7 +52,7 @@ DECLARE_GLOBAL_DATA_PTR;
int dram_init(void) {
- gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
gd->ram_size = get_ram_size((void *)PHYS_SDRAM, imx_ddr_size());
return 0;
} diff --git a/include/configs/colibri_imx7.h b/include/configs/colibri_imx7.h index ff6bd678cf..02849ba35f 100644 --- a/include/configs/colibri_imx7.h +++ b/include/configs/colibri_imx7.h @@ -14,7 +14,6 @@ #include "mx7_common.h"
/*#define CONFIG_DBG_MONITOR*/ -#define PHYS_SDRAM_SIZE SZ_1G
/* Size of malloc() pool */ #define CONFIG_SYS_MALLOC_LEN (32 * SZ_1M)
Hi Stefan,
On Wed, Sep 19, 2018 at 12:48 PM, Stefan Agner stefan.agner@toradex.com wrote:
Hm, with that we take the MMDC register information as the upper bound, and use regular U-Boot get_ram_size() to determine size by poking memory addresses. Seems sensible.
Acked-by: Stefan Agner stefan.agner@toradex.com
Fabio, I guess other boards use SPL to use different MMDC configuration for different memory size? Is there a downside doing this size over-provisioning?
That's correct: the imx7 boards that use imx_ddr_size() are cl-som-imx7 and pico-imx7d, which uses SPL and provide different MMDC configuration depending on the memory density.
I don't see a downside in doing this over-provisioning.
Regards,
Fabio Estevam
Hi Marcel,
On Wed, Sep 19, 2018 at 8:01 AM, Marcel Ziswiler marcel@ziswiler.com wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
The imx_ddr_size() function may overflow as it is possible to kind of over provision the DDR controller. Fix this by capping it to 2 GB which is the maximum allowed size as per reference manual.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
Looks good, thanks:
Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
participants (3)
-
Fabio Estevam -
Marcel Ziswiler -
Stefan Agner