The original imx_ddr_size() implementation had some issues reported by checkpatch like this:

CHECK: Prefer kernel type 'u32' over 'uint32_t' #127: FILE: arch/arm/mach-imx/mmdc_size.c:16: + uint32_t ctl;

WARNING: Prefer 'unsigned int' to bare use of 'unsigned' #151: FILE: arch/arm/mach-imx/mmdc_size.c:40: + unsigned ctl = readl(&mem->ctl);

Fix all of them.

Signed-off-by: Fabio Estevam festevam@gmail.com --- arch/arm/mach-imx/mmdc_size.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-imx/mmdc_size.c b/arch/arm/mach-imx/mmdc_size.c index 8a3c6bdea6..13f587c38a 100644 --- a/arch/arm/mach-imx/mmdc_size.c +++ b/arch/arm/mach-imx/mmdc_size.c @@ -13,13 +13,13 @@ static const unsigned char bank_lookup[] = {3, 2};

/* these MMDC registers are common to the IMX53 and IMX6 */ struct esd_mmdc_regs { - uint32_t ctl; - uint32_t pdc; - uint32_t otc; - uint32_t cfg0; - uint32_t cfg1; - uint32_t cfg2; - uint32_t misc; + u32 ctl; + u32 pdc; + u32 otc; + u32 cfg0; + u32 cfg1; + u32 cfg2; + u32 misc; };

#define ESD_MMDC_CTL_GET_ROW(mdctl) ((ctl >> 24) & 7) @@ -34,11 +34,11 @@ struct esd_mmdc_regs { * width and the MMDC MDMISC register holds the number of banks. Combine * all these bits to determine the meme size the MMDC has been configured for */ -unsigned imx_ddr_size(void) +unsigned int imx_ddr_size(void) { struct esd_mmdc_regs *mem = (struct esd_mmdc_regs *)MEMCTL_BASE; - unsigned ctl = readl(&mem->ctl); - unsigned misc = readl(&mem->misc); + unsigned int ctl = readl(&mem->ctl); + unsigned int misc = readl(&mem->misc); int bits = 11 + 0 + 0 + 1; /* row + col + bank + width */

bits += ESD_MMDC_CTL_GET_ROW(ctl);

i.MX7ULP uses the same MMDC controller IP as found on i.MX53 and i.MX6, so build mmdc_size.c for i.MX7ULP as well.

Signed-off-by: Fabio Estevam festevam@gmail.com --- arch/arm/mach-imx/Makefile | 2 +- arch/arm/mach-imx/mmdc_size.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile index 0c8519be94..b29f90b024 100644 --- a/arch/arm/mach-imx/Makefile +++ b/arch/arm/mach-imx/Makefile @@ -48,7 +48,7 @@ obj-$(CONFIG_SECURE_BOOT) += hab.o obj-$(CONFIG_SYSCOUNTER_TIMER) += syscounter.o endif ifeq ($(SOC),$(filter $(SOC),mx7ulp)) -obj-y += cache.o +obj-y += cache.o mmdc_size.o obj-$(CONFIG_SECURE_BOOT) += hab.o endif ifeq ($(SOC),$(filter $(SOC),vf610)) diff --git a/arch/arm/mach-imx/mmdc_size.c b/arch/arm/mach-imx/mmdc_size.c index 13f587c38a..1a094726aa 100644 --- a/arch/arm/mach-imx/mmdc_size.c +++ b/arch/arm/mach-imx/mmdc_size.c @@ -5,8 +5,10 @@

#if defined(CONFIG_MX53) #define MEMCTL_BASE ESDCTL_BASE_ADDR -#else +#elif defined(CONFIG_MX6) #define MEMCTL_BASE MMDC_P0_BASE_ADDR +#elif defined(CONFIG_MX7ULP) +#define MEMCTL_BASE MMDC0_RBASE #endif static const unsigned char col_lookup[] = {9, 10, 11, 8, 12, 9, 9, 9}; static const unsigned char bank_lookup[] = {3, 2};

Use imx_ddr_size() to retrieve the total DDR size instead of passing a hardcoded memory size.

imx_ddr_size() calculates the memory size based on the actual MMDC registers values and is useful to detect misconfigurations, so switch to this more robust approach.

Signed-off-by: Fabio Estevam festevam@gmail.com --- board/freescale/mx7ulp_evk/mx7ulp_evk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/freescale/mx7ulp_evk/mx7ulp_evk.c b/board/freescale/mx7ulp_evk/mx7ulp_evk.c index 7527263577..c939514a5f 100644 --- a/board/freescale/mx7ulp_evk/mx7ulp_evk.c +++ b/board/freescale/mx7ulp_evk/mx7ulp_evk.c @@ -17,7 +17,7 @@ DECLARE_GLOBAL_DATA_PTR;

int dram_init(void) { - gd->ram_size = PHYS_SDRAM_SIZE; + gd->ram_size = imx_ddr_size();

return 0; }

Hi Fabio,

get a lost patch, I pick it up:

On 18/07/19 20:04, Fabio Estevam wrote:

Place imx_ddr_size() into a separate file.

The motivation for doing this is to be able to easily reuse imx_ddr_size() on i.MX7ULP.

Currently imx_ddr_size() is inside arch/arm/mach-imx/cpu.c, which is not built for i.MX7ULP.

Changing the logic to allow building cpu.c for i.MX7UP would require adding several ifdef's, leading to a not a very elegant solution.

To allow better reuse, just place imx_ddr_size() into a common mmdc_size.c file.

Signed-off-by: Fabio Estevam festevam@gmail.com

arch/arm/mach-imx/Makefile | 2 +- arch/arm/mach-imx/cpu.c | 53 --------------------------------- arch/arm/mach-imx/mmdc_size.c | 55 +++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 54 deletions(-) create mode 100644 arch/arm/mach-imx/mmdc_size.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile index 898478fc4a..0c8519be94 100644 --- a/arch/arm/mach-imx/Makefile +++ b/arch/arm/mach-imx/Makefile @@ -20,7 +20,7 @@ obj-y += cpu.o endif

ifeq ($(SOC),$(filter $(SOC),mx5 mx6)) -obj-y += cpu.o speed.o +obj-y += cpu.o speed.o mmdc_size.o

This does not filter MX51, code is not common for it. What do you mind if I pick it up with :

ifeq ($(SOC),$(filter $(SOC),mx5 mx6)) obj-y += cpu.o speed.o ifneq ($(CONFIG_MX51),y) obj-y += mmdc_size.o endif

Regards, Stefano

obj-$(CONFIG_GPT_TIMER) += timer.o obj-$(CONFIG_SYS_I2C_MXC) += i2c-mxv7.o endif diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c index 3a8cf30c06..0b30ff40cd 100644 --- a/arch/arm/mach-imx/cpu.c +++ b/arch/arm/mach-imx/cpu.c @@ -87,59 +87,6 @@ static char *get_reset_cause(void) } #endif

-#if defined(CONFIG_MX53) || defined(CONFIG_MX6) -#if defined(CONFIG_MX53) -#define MEMCTL_BASE ESDCTL_BASE_ADDR -#else -#define MEMCTL_BASE MMDC_P0_BASE_ADDR -#endif -static const unsigned char col_lookup[] = {9, 10, 11, 8, 12, 9, 9, 9}; -static const unsigned char bank_lookup[] = {3, 2};

-/* these MMDC registers are common to the IMX53 and IMX6 */ -struct esd_mmdc_regs {

• uint32_t ctl;
• uint32_t pdc;
• uint32_t otc;
• uint32_t cfg0;
• uint32_t cfg1;
• uint32_t cfg2;
• uint32_t misc;

-};

-#define ESD_MMDC_CTL_GET_ROW(mdctl) ((ctl >> 24) & 7) -#define ESD_MMDC_CTL_GET_COLUMN(mdctl) ((ctl >> 20) & 7) -#define ESD_MMDC_CTL_GET_WIDTH(mdctl) ((ctl >> 16) & 3) -#define ESD_MMDC_CTL_GET_CS1(mdctl) ((ctl >> 30) & 1) -#define ESD_MMDC_MISC_GET_BANK(mdmisc) ((misc >> 5) & 1)

-/*

• • imx_ddr_size - return size in bytes of DRAM according MMDC config
• • The MMDC MDCTL register holds the number of bits for row, col, and data
• • width and the MMDC MDMISC register holds the number of banks. Combine
• • all these bits to determine the meme size the MMDC has been configured for
• */

-unsigned imx_ddr_size(void) -{

• struct esd_mmdc_regs *mem = (struct esd_mmdc_regs *)MEMCTL_BASE;
• unsigned ctl = readl(&mem->ctl);
• unsigned misc = readl(&mem->misc);
• int bits = 11 + 0 + 0 + 1; /* row + col + bank + width */
• bits += ESD_MMDC_CTL_GET_ROW(ctl);
• bits += col_lookup[ESD_MMDC_CTL_GET_COLUMN(ctl)];
• bits += bank_lookup[ESD_MMDC_MISC_GET_BANK(misc)];
• bits += ESD_MMDC_CTL_GET_WIDTH(ctl);
• bits += ESD_MMDC_CTL_GET_CS1(ctl);
• /* The MX6 can do only 3840 MiB of DRAM */
• if (bits == 32)

• return 0xf0000000;
  

• return 1 << bits;

-} -#endif

#if defined(CONFIG_DISPLAY_CPUINFO) && !defined(CONFIG_SPL_BUILD)

const char *get_imx_type(u32 imxtype) diff --git a/arch/arm/mach-imx/mmdc_size.c b/arch/arm/mach-imx/mmdc_size.c new file mode 100644 index 0000000000..8a3c6bdea6 --- /dev/null +++ b/arch/arm/mach-imx/mmdc_size.c @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: GPL-2.0+

+#include <common.h> +#include <asm/io.h>

+#if defined(CONFIG_MX53) +#define MEMCTL_BASE ESDCTL_BASE_ADDR +#else +#define MEMCTL_BASE MMDC_P0_BASE_ADDR +#endif +static const unsigned char col_lookup[] = {9, 10, 11, 8, 12, 9, 9, 9}; +static const unsigned char bank_lookup[] = {3, 2};

+/* these MMDC registers are common to the IMX53 and IMX6 */ +struct esd_mmdc_regs {

• uint32_t ctl;
• uint32_t pdc;
• uint32_t otc;
• uint32_t cfg0;
• uint32_t cfg1;
• uint32_t cfg2;
• uint32_t misc;

+};

+#define ESD_MMDC_CTL_GET_ROW(mdctl) ((ctl >> 24) & 7) +#define ESD_MMDC_CTL_GET_COLUMN(mdctl) ((ctl >> 20) & 7) +#define ESD_MMDC_CTL_GET_WIDTH(mdctl) ((ctl >> 16) & 3) +#define ESD_MMDC_CTL_GET_CS1(mdctl) ((ctl >> 30) & 1) +#define ESD_MMDC_MISC_GET_BANK(mdmisc) ((misc >> 5) & 1)

+/*

• • imx_ddr_size - return size in bytes of DRAM according MMDC config
• • The MMDC MDCTL register holds the number of bits for row, col, and data
• • width and the MMDC MDMISC register holds the number of banks. Combine
• • all these bits to determine the meme size the MMDC has been configured for
• */

+unsigned imx_ddr_size(void) +{

• struct esd_mmdc_regs *mem = (struct esd_mmdc_regs *)MEMCTL_BASE;
• unsigned ctl = readl(&mem->ctl);
• unsigned misc = readl(&mem->misc);
• int bits = 11 + 0 + 0 + 1; /* row + col + bank + width */
• bits += ESD_MMDC_CTL_GET_ROW(ctl);
• bits += col_lookup[ESD_MMDC_CTL_GET_COLUMN(ctl)];
• bits += bank_lookup[ESD_MMDC_MISC_GET_BANK(misc)];
• bits += ESD_MMDC_CTL_GET_WIDTH(ctl);
• bits += ESD_MMDC_CTL_GET_CS1(ctl);
• /* The MX6 can do only 3840 MiB of DRAM */
• if (bits == 32)

• return 0xf0000000;
  

• return 1 << bits;

+}