[PATCH 0/5] sunxi: h6/h616: consolidate DRAM code

[PATCH 0/5] sunxi: h6/h616: consolidate DRAM code

[Jernej Skrabec]

While working on A523 support, it became obvious that newer sunxi DRAM
drivers are similar in many ways, so it makes sense to share some code
between them. Let's start with DRAM size and ranks detection. There were
many fixes for it in H616 driver, so make sense to unify code interface
and split out h616 part into separate file and reuse it for h6 too. This
will probably solve some DRAM size detection issues on h6 reported
through the years.

Patch 1 adds more error reporting in case rows or column size is not
correctly detected.

Patches 2-4 bring h6 code close enough to h616 so code sharing is
possible.

Patch 5 splits out h616 code and reuse it on h6.

Note I'm currently not able test the code, so if anyone else could do it
I would appreciate very much.

There are further possibilities regarding code sharing in the future,
like:
- memory controller is pretty similar for H6, H616 and A523
- unify config and parameters structures

Best regards,
Jernej

Jernej Skrabec (5):
  sunxi: h616: Panic if DRAM size is not detected
  sunxi: H6: Remove useless DRAM timings parameter
  sunxi: H6: DRAM: Constify function parameters
  sunxi: h6: dram: split dram_para struct
  sunxi: h6/h616: Reuse common DRAM infrastructure

 .../include/asm/arch-sunxi/dram_dw_helpers.h  |  22 +++
 .../include/asm/arch-sunxi/dram_sun50i_h6.h   |   9 +-
 arch/arm/mach-sunxi/Makefile                  |   4 +-
 arch/arm/mach-sunxi/dram_dw_helpers.c         | 160 ++++++++++++++++
 arch/arm/mach-sunxi/dram_sun50i_h6.c          | 180 +++++-------------
 arch/arm/mach-sunxi/dram_sun50i_h616.c        | 145 +-------------
 .../mach-sunxi/dram_timings/h6_ddr3_1333.c    |   2 +-
 arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c  |   2 +-
 8 files changed, 247 insertions(+), 277 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
 create mode 100644 arch/arm/mach-sunxi/dram_dw_helpers.c

-- 
2.49.0

[PATCH 1/5] sunxi: h616: Panic if DRAM size is not detected

[Jernej Skrabec]

If colum or row size is not detected, panic instead of continuing. It
won't work anyway and it's better to inform user directly what's wrong
instead of failing later down the road for random reason.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 arch/arm/mach-sunxi/dram_sun50i_h616.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
index cd9d321a0185..d1768a7e7d3a 100644
--- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
+++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
@@ -1396,7 +1396,7 @@ static bool mctl_check_pattern(ulong offset)
 static void mctl_auto_detect_dram_size(const struct dram_para *para,
 				       struct dram_config *config)
 {
-	unsigned int shift, cols, rows;
+	unsigned int shift, cols, rows, found;
 	u32 buffer[16];
 
 	/* max. config for columns, but not rows */
@@ -1416,10 +1416,15 @@ static void mctl_auto_detect_dram_size(const struct dram_para *para,
 	shift = config->bus_full_width + 1;
 
 	/* detect column address bits */
+	found = 0;
 	for (cols = 8; cols < 11; cols++) {
-		if (mctl_check_pattern(1ULL << (cols + shift)))
+		if (mctl_check_pattern(1ULL << (cols + shift))) {
+			found = 1;
 			break;
+		}
 	}
+	if (!found)
+		panic("DRAM init failed: Can't detect number of columns!");
 	debug("detected %u columns\n", cols);
 
 	/* restore data */
@@ -1437,10 +1442,15 @@ static void mctl_auto_detect_dram_size(const struct dram_para *para,
 
 	/* detect row address bits */
 	shift = config->bus_full_width + 4 + config->cols;
+	found = 0;
 	for (rows = 13; rows < 17; rows++) {
-		if (mctl_check_pattern(1ULL << (rows + shift)))
+		if (mctl_check_pattern(1ULL << (rows + shift))) {
+			found = 1;
 			break;
+		}
 	}
+	if (!found)
+		panic("DRAM init failed: Can't detect number of rows!");
 	debug("detected %u rows\n", rows);
 
 	/* restore data again */
-- 
2.49.0

[PATCH 2/5] sunxi: H6: Remove useless DRAM timings parameter

[Jernej Skrabec]

This is just cosmetic fix for later easier rework.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h | 2 +-
 arch/arm/mach-sunxi/dram_sun50i_h6.c             | 2 +-
 arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c  | 2 +-
 arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c     | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
index f0caecc807dd..f05a1845b32b 100644
--- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
+++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
@@ -330,6 +330,6 @@ static inline int ns_to_t(int nanoseconds)
 	return DIV_ROUND_UP(ctrl_freq * nanoseconds, 1000);
 }
 
-void mctl_set_timing_params(struct dram_para *para);
+void mctl_set_timing_params(void);
 
 #endif /* _SUNXI_DRAM_SUN50I_H6_H */
diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
index e7862bd06ea3..0adbda756639 100644
--- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
+++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
@@ -45,7 +45,7 @@ static bool mctl_core_init(struct dram_para *para)
 	switch (para->type) {
 	case SUNXI_DRAM_TYPE_LPDDR3:
 	case SUNXI_DRAM_TYPE_DDR3:
-		mctl_set_timing_params(para);
+		mctl_set_timing_params();
 		break;
 	default:
 		panic("Unsupported DRAM type!");
diff --git a/arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c b/arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c
index afe8e25c7f58..1ed46fed411f 100644
--- a/arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c
+++ b/arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c
@@ -37,7 +37,7 @@ static u32 mr_ddr3[7] = {
 };
 
 /* TODO: flexible timing */
-void mctl_set_timing_params(struct dram_para *para)
+void mctl_set_timing_params(void)
 {
 	struct sunxi_mctl_ctl_reg * const mctl_ctl =
 			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
diff --git a/arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c b/arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c
index c243b574406d..c02f542c989f 100644
--- a/arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c
+++ b/arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c
@@ -16,7 +16,7 @@ static u32 mr_lpddr3[12] = {
 };
 
 /* TODO: flexible timing */
-void mctl_set_timing_params(struct dram_para *para)
+void mctl_set_timing_params(void)
 {
 	struct sunxi_mctl_ctl_reg * const mctl_ctl =
 			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
-- 
2.49.0

[PATCH 3/5] sunxi: H6: DRAM: Constify function parameters

[Jernej Skrabec]

Constify parameters for two reasons:
- Allow more compile time optimizations
- It will allow later sharing of common code with H616 (when it will be
  rearranged some more)

Commit does same kind of changes as 457e2cd665bd ("sunxi: H616: dram:
const-ify DRAM function parameters")

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 arch/arm/mach-sunxi/dram_sun50i_h6.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
index 0adbda756639..24b2cb1579f4 100644
--- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
+++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
@@ -34,13 +34,13 @@
  * similar PHY is ZynqMP.
  */
 
-static void mctl_sys_init(struct dram_para *para);
-static void mctl_com_init(struct dram_para *para);
-static bool mctl_channel_init(struct dram_para *para);
+static void mctl_sys_init(u32 clk_rate);
+static void mctl_com_init(const struct dram_para *para);
+static bool mctl_channel_init(const struct dram_para *para);
 
 static bool mctl_core_init(struct dram_para *para)
 {
-	mctl_sys_init(para);
+	mctl_sys_init(para->clk);
 	mctl_com_init(para);
 	switch (para->type) {
 	case SUNXI_DRAM_TYPE_LPDDR3:
@@ -150,7 +150,7 @@ static void mctl_set_master_priority(void)
 	MBUS_CONF(HDCP2,  true,    HIGH, 2,  100,   64,   32);
 }
 
-static void mctl_sys_init(struct dram_para *para)
+static void mctl_sys_init(u32 clk_rate)
 {
 	struct sunxi_ccm_reg * const ccm =
 			(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
@@ -171,7 +171,7 @@ static void mctl_sys_init(struct dram_para *para)
 
 	/* Set PLL5 rate to doubled DRAM clock rate */
 	writel(CCM_PLL5_CTRL_EN | CCM_PLL5_LOCK_EN |
-	       CCM_PLL5_CTRL_N(para->clk * 2 / 24), &ccm->pll5_cfg);
+	       CCM_PLL5_CTRL_N(clk_rate * 2 / 24), &ccm->pll5_cfg);
 	mctl_await_completion(&ccm->pll5_cfg, CCM_PLL5_LOCK, CCM_PLL5_LOCK);
 
 	/* Configure DRAM mod clock */
@@ -196,7 +196,7 @@ static void mctl_sys_init(struct dram_para *para)
 	writel(0x8000, &mctl_ctl->unk_0x00c);
 }
 
-static void mctl_set_addrmap(struct dram_para *para)
+static void mctl_set_addrmap(const struct dram_para *para)
 {
 	struct sunxi_mctl_ctl_reg * const mctl_ctl =
 			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
@@ -282,7 +282,7 @@ static void mctl_set_addrmap(struct dram_para *para)
 	mctl_ctl->addrmap[8] = 0x3F3F;
 }
 
-static void mctl_com_init(struct dram_para *para)
+static void mctl_com_init(const struct dram_para *para)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
@@ -352,7 +352,7 @@ static void mctl_com_init(struct dram_para *para)
 	}
 }
 
-static void mctl_bit_delay_set(struct dram_para *para)
+static void mctl_bit_delay_set(const struct dram_para *para)
 {
 	struct sunxi_mctl_phy_reg * const mctl_phy =
 			(struct sunxi_mctl_phy_reg *)SUNXI_DRAM_PHY0_BASE;
@@ -411,7 +411,7 @@ static void mctl_bit_delay_set(struct dram_para *para)
 	}
 }
 
-static bool mctl_channel_init(struct dram_para *para)
+static bool mctl_channel_init(const struct dram_para *para)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
-- 
2.49.0

[PATCH 4/5] sunxi: h6: dram: split dram_para struct

[Jernej Skrabec]

This change is same as in 78aa00c38e86 ("sunxi: H616: dram: split struct
dram_para"), but for H6. This is needed in order to extract common code
between H6 and H616 later.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 .../include/asm/arch-sunxi/dram_sun50i_h6.h   |   7 +-
 arch/arm/mach-sunxi/dram_sun50i_h6.c          | 145 ++++++++++--------
 2 files changed, 82 insertions(+), 70 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
index f05a1845b32b..af6cd337d7e0 100644
--- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
+++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
@@ -315,12 +315,15 @@ check_member(sunxi_mctl_phy_reg, dx[3].reserved_0xf0, 0xaf0);
 struct dram_para {
 	u32 clk;
 	enum sunxi_dram_type type;
+	const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
+	const u8 dx_write_delays[NR_OF_BYTE_LANES][WR_LINES_PER_BYTE_LANE];
+};
+
+struct dram_config {
 	u8 cols;
 	u8 rows;
 	u8 ranks;
 	u8 bus_full_width;
-	const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
-	const u8 dx_write_delays[NR_OF_BYTE_LANES][WR_LINES_PER_BYTE_LANE];
 };
 
 static inline int ns_to_t(int nanoseconds)
diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
index 24b2cb1579f4..6a9e53f965eb 100644
--- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
+++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
@@ -35,13 +35,16 @@
  */
 
 static void mctl_sys_init(u32 clk_rate);
-static void mctl_com_init(const struct dram_para *para);
-static bool mctl_channel_init(const struct dram_para *para);
+static void mctl_com_init(const struct dram_para *para,
+			  const struct dram_config *config);
+static bool mctl_channel_init(const struct dram_para *para,
+			      const struct dram_config *config);
 
-static bool mctl_core_init(struct dram_para *para)
+static bool mctl_core_init(const struct dram_para *para,
+			   const struct dram_config *config)
 {
 	mctl_sys_init(para->clk);
-	mctl_com_init(para);
+	mctl_com_init(para, config);
 	switch (para->type) {
 	case SUNXI_DRAM_TYPE_LPDDR3:
 	case SUNXI_DRAM_TYPE_DDR3:
@@ -50,7 +53,7 @@ static bool mctl_core_init(struct dram_para *para)
 	default:
 		panic("Unsupported DRAM type!");
 	};
-	return mctl_channel_init(para);
+	return mctl_channel_init(para, config);
 }
 
 /* PHY initialisation */
@@ -196,15 +199,15 @@ static void mctl_sys_init(u32 clk_rate)
 	writel(0x8000, &mctl_ctl->unk_0x00c);
 }
 
-static void mctl_set_addrmap(const struct dram_para *para)
+static void mctl_set_addrmap(const struct dram_config *config)
 {
 	struct sunxi_mctl_ctl_reg * const mctl_ctl =
 			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
-	u8 cols = para->cols;
-	u8 rows = para->rows;
-	u8 ranks = para->ranks;
+	u8 cols = config->cols;
+	u8 rows = config->rows;
+	u8 ranks = config->ranks;
 
-	if (!para->bus_full_width)
+	if (!config->bus_full_width)
 		cols -= 1;
 
 	/* Ranks */
@@ -282,7 +285,8 @@ static void mctl_set_addrmap(const struct dram_para *para)
 	mctl_ctl->addrmap[8] = 0x3F3F;
 }
 
-static void mctl_com_init(const struct dram_para *para)
+static void mctl_com_init(const struct dram_para *para,
+			  const struct dram_config *config)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
@@ -292,7 +296,7 @@ static void mctl_com_init(const struct dram_para *para)
 			(struct sunxi_mctl_phy_reg *)SUNXI_DRAM_PHY0_BASE;
 	u32 reg_val, tmp;
 
-	mctl_set_addrmap(para);
+	mctl_set_addrmap(config);
 
 	setbits_le32(&mctl_com->cr, BIT(31));
 
@@ -311,12 +315,12 @@ static void mctl_com_init(const struct dram_para *para)
 	clrsetbits_le32(&mctl_com->unk_0x008, 0x3f00, reg_val);
 
 	/* TODO: DDR4 */
-	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(para->ranks);
+	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(config->ranks);
 	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
 		reg_val |= MSTR_DEVICETYPE_LPDDR3;
 	if (para->type == SUNXI_DRAM_TYPE_DDR3)
 		reg_val |= MSTR_DEVICETYPE_DDR3 | MSTR_2TMODE;
-	if (para->bus_full_width)
+	if (config->bus_full_width)
 		reg_val |= MSTR_BUSWIDTH_FULL;
 	else
 		reg_val |= MSTR_BUSWIDTH_HALF;
@@ -328,7 +332,7 @@ static void mctl_com_init(const struct dram_para *para)
 		reg_val = DCR_DDR3 | DCR_DDR8BANK | DCR_DDR2T;
 	writel(reg_val | 0x400, &mctl_phy->dcr);
 
-	if (para->ranks == 2)
+	if (config->ranks == 2)
 		writel(0x0303, &mctl_ctl->odtmap);
 	else
 		writel(0x0201, &mctl_ctl->odtmap);
@@ -346,7 +350,7 @@ static void mctl_com_init(const struct dram_para *para)
 	}
 	writel(reg_val, &mctl_ctl->odtcfg);
 
-	if (!para->bus_full_width) {
+	if (!config->bus_full_width) {
 		writel(0x0, &mctl_phy->dx[2].gcr[0]);
 		writel(0x0, &mctl_phy->dx[3].gcr[0]);
 	}
@@ -411,7 +415,8 @@ static void mctl_bit_delay_set(const struct dram_para *para)
 	}
 }
 
-static bool mctl_channel_init(const struct dram_para *para)
+static bool mctl_channel_init(const struct dram_para *para,
+			      const struct dram_config *config)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
@@ -446,14 +451,14 @@ static bool mctl_channel_init(const struct dram_para *para)
 
 	udelay(100);
 
-	if (para->ranks == 2)
+	if (config->ranks == 2)
 		setbits_le32(&mctl_phy->dtcr[1], 0x30000);
 	else
 		clrsetbits_le32(&mctl_phy->dtcr[1], 0x30000, 0x10000);
 
 	if (sunxi_dram_is_lpddr(para->type))
 		clrbits_le32(&mctl_phy->dtcr[1], BIT(1));
-	if (para->ranks == 2) {
+	if (config->ranks == 2) {
 		writel(0x00010001, &mctl_phy->rankidr);
 		writel(0x20000, &mctl_phy->odtcr);
 	} else {
@@ -555,11 +560,12 @@ static bool mctl_channel_init(const struct dram_para *para)
 	return true;
 }
 
-static void mctl_auto_detect_rank_width(struct dram_para *para)
+static void mctl_auto_detect_rank_width(const struct dram_para *para,
+					struct dram_config *config)
 {
 	/* this is minimum size that it's supported */
-	para->cols = 8;
-	para->rows = 13;
+	config->cols = 8;
+	config->rows = 13;
 
 	/*
 	 * Previous versions of this driver tried to auto detect the rank
@@ -575,68 +581,69 @@ static void mctl_auto_detect_rank_width(struct dram_para *para)
 	 */
 
 	debug("testing 32-bit width, rank = 2\n");
-	para->bus_full_width = 1;
-	para->ranks = 2;
-	if (mctl_core_init(para))
+	config->bus_full_width = 1;
+	config->ranks = 2;
+	if (mctl_core_init(para, config))
 		return;
 
 	debug("testing 32-bit width, rank = 1\n");
-	para->bus_full_width = 1;
-	para->ranks = 1;
-	if (mctl_core_init(para))
+	config->bus_full_width = 1;
+	config->ranks = 1;
+	if (mctl_core_init(para, config))
 		return;
 
 	debug("testing 16-bit width, rank = 2\n");
-	para->bus_full_width = 0;
-	para->ranks = 2;
-	if (mctl_core_init(para))
+	config->bus_full_width = 0;
+	config->ranks = 2;
+	if (mctl_core_init(para, config))
 		return;
 
 	debug("testing 16-bit width, rank = 1\n");
-	para->bus_full_width = 0;
-	para->ranks = 1;
-	if (mctl_core_init(para))
+	config->bus_full_width = 0;
+	config->ranks = 1;
+	if (mctl_core_init(para, config))
 		return;
 
 	panic("This DRAM setup is currently not supported.\n");
 }
 
-static void mctl_auto_detect_dram_size(struct dram_para *para)
+static void mctl_auto_detect_dram_size(const struct dram_para *para,
+				       struct dram_config *config)
 {
 	/* TODO: non-(LP)DDR3 */
 
 	/* detect row address bits */
-	para->cols = 8;
-	para->rows = 18;
-	mctl_core_init(para);
+	config->cols = 8;
+	config->rows = 18;
+	mctl_core_init(para, config);
 
-	for (para->rows = 13; para->rows < 18; para->rows++) {
+	for (config->rows = 13; config->rows < 18; config->rows++) {
 		/* 8 banks, 8 bit per byte and 16/32 bit width */
-		if (mctl_mem_matches((1 << (para->rows + para->cols +
-					    4 + para->bus_full_width))))
+		if (mctl_mem_matches((1 << (config->rows + config->cols +
+					    4 + config->bus_full_width))))
 			break;
 	}
 
 	/* detect column address bits */
-	para->cols = 11;
-	mctl_core_init(para);
+	config->cols = 11;
+	mctl_core_init(para, config);
 
-	for (para->cols = 8; para->cols < 11; para->cols++) {
+	for (config->cols = 8; config->cols < 11; config->cols++) {
 		/* 8 bits per byte and 16/32 bit width */
-		if (mctl_mem_matches(1 << (para->cols + 1 +
-					   para->bus_full_width)))
+		if (mctl_mem_matches(1 << (config->cols + 1 +
+					   config->bus_full_width)))
 			break;
 	}
 }
 
-unsigned long mctl_calc_size(struct dram_para *para)
+unsigned long mctl_calc_size(const struct dram_config *config)
 {
-	u8 width = para->bus_full_width ? 4 : 2;
+	u8 width = config->bus_full_width ? 4 : 2;
 
 	/* TODO: non-(LP)DDR3 */
 
 	/* 8 banks */
-	return (1ULL << (para->cols + para->rows + 3)) * width * para->ranks;
+	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
 }
 
 #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
@@ -661,36 +668,38 @@ unsigned long mctl_calc_size(struct dram_para *para)
 	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
 	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 }}
 
-unsigned long sunxi_dram_init(void)
-{
-	struct sunxi_mctl_com_reg * const mctl_com =
-			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
-	struct sunxi_prcm_reg *const prcm =
-		(struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
-	struct dram_para para = {
-		.clk = CONFIG_DRAM_CLK,
+static const struct dram_para para = {
+	.clk = CONFIG_DRAM_CLK,
 #ifdef CONFIG_SUNXI_DRAM_H6_LPDDR3
-		.type = SUNXI_DRAM_TYPE_LPDDR3,
-		.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,
-		.dx_write_delays = SUN50I_H6_LPDDR3_DX_WRITE_DELAYS,
+	.type = SUNXI_DRAM_TYPE_LPDDR3,
+	.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,
+	.dx_write_delays = SUN50I_H6_LPDDR3_DX_WRITE_DELAYS,
 #elif defined(CONFIG_SUNXI_DRAM_H6_DDR3_1333)
-		.type = SUNXI_DRAM_TYPE_DDR3,
-		.dx_read_delays  = SUN50I_H6_DDR3_DX_READ_DELAYS,
-		.dx_write_delays = SUN50I_H6_DDR3_DX_WRITE_DELAYS,
+	.type = SUNXI_DRAM_TYPE_DDR3,
+	.dx_read_delays  = SUN50I_H6_DDR3_DX_READ_DELAYS,
+	.dx_write_delays = SUN50I_H6_DDR3_DX_WRITE_DELAYS,
 #endif
-	};
+};
+
+unsigned long sunxi_dram_init(void)
+{
+	struct sunxi_mctl_com_reg * const mctl_com =
+			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
+	struct sunxi_prcm_reg *const prcm =
+		(struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
+	struct dram_config config;
 
 	unsigned long size;
 
 	setbits_le32(&prcm->res_cal_ctrl, BIT(8));
 	clrbits_le32(&prcm->ohms240, 0x3f);
 
-	mctl_auto_detect_rank_width(&para);
-	mctl_auto_detect_dram_size(&para);
+	mctl_auto_detect_rank_width(&para, &config);
+	mctl_auto_detect_dram_size(&para, &config);
 
-	mctl_core_init(&para);
+	mctl_core_init(&para, &config);
 
-	size = mctl_calc_size(&para);
+	size = mctl_calc_size(&config);
 
 	clrsetbits_le32(&mctl_com->cr, 0xf0, (size >> (10 + 10 + 4)) & 0xf0);
 
-- 
2.49.0

[PATCH 5/5] sunxi: h6/h616: Reuse common DRAM infrastructure

[Jernej Skrabec]

H616 rank and size detection code is superior to the H6. Nevertheless,
they are structurally the same. Split functions from H616 into new file
and reuse them in H6 DRAM driver too. This should also fix some bugs for
H6 too, like incorrect DRAM size detection.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 .../include/asm/arch-sunxi/dram_dw_helpers.h  |  22 +++
 arch/arm/mach-sunxi/Makefile                  |   4 +-
 arch/arm/mach-sunxi/dram_dw_helpers.c         | 160 ++++++++++++++++++
 arch/arm/mach-sunxi/dram_sun50i_h6.c          |  91 +---------
 arch/arm/mach-sunxi/dram_sun50i_h616.c        | 155 +----------------
 5 files changed, 190 insertions(+), 242 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
 create mode 100644 arch/arm/mach-sunxi/dram_dw_helpers.c

diff --git a/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
new file mode 100644
index 000000000000..bc9e0d868c55
--- /dev/null
+++ b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Helpers that are commonly used with DW memory controller.
+ *
+ * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
+ *
+ */
+
+#ifndef _DRAM_DW_HELPERS_H
+#define _DRAM_DW_HELPERS_H
+
+#include <asm/arch/dram.h>
+
+bool mctl_core_init(const struct dram_para *para,
+		    const struct dram_config *config);
+void mctl_auto_detect_rank_width(const struct dram_para *para,
+				 struct dram_config *config);
+void mctl_auto_detect_dram_size(const struct dram_para *para,
+				struct dram_config *config);
+unsigned long mctl_calc_size(const struct dram_config *config);
+
+#endif
diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
index eb6a49119a13..a33cd5b0f07a 100644
--- a/arch/arm/mach-sunxi/Makefile
+++ b/arch/arm/mach-sunxi/Makefile
@@ -41,8 +41,8 @@ obj-$(CONFIG_DRAM_SUN9I)	+= dram_sun9i.o
 obj-$(CONFIG_SPL_SPI_SUNXI)	+= spl_spi_sunxi.o
 obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_sunxi_dw.o
 obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_timings/
-obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o
+obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o dram_dw_helpers.o
 obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_timings/
-obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o
+obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o dram_dw_helpers.o
 obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_timings/
 endif
diff --git a/arch/arm/mach-sunxi/dram_dw_helpers.c b/arch/arm/mach-sunxi/dram_dw_helpers.c
new file mode 100644
index 000000000000..885d1f2c0b12
--- /dev/null
+++ b/arch/arm/mach-sunxi/dram_dw_helpers.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Helpers that are commonly used with DW memory controller.
+ *
+ * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
+ *
+ */
+
+#include <init.h>
+#include <asm/arch/dram_dw_helpers.h>
+
+void mctl_auto_detect_rank_width(const struct dram_para *para,
+				 struct dram_config *config)
+{
+	/* this is minimum size that it's supported */
+	config->cols = 8;
+	config->rows = 13;
+
+	/*
+	 * Strategy here is to test most demanding combination first and least
+	 * demanding last, otherwise HW might not be fully utilized. For
+	 * example, half bus width and rank = 1 combination would also work
+	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
+	 * visible.
+	 */
+
+	debug("testing 32-bit width, rank = 2\n");
+	config->bus_full_width = 1;
+	config->ranks = 2;
+	if (mctl_core_init(para, config))
+		return;
+
+	debug("testing 32-bit width, rank = 1\n");
+	config->bus_full_width = 1;
+	config->ranks = 1;
+	if (mctl_core_init(para, config))
+		return;
+
+	debug("testing 16-bit width, rank = 2\n");
+	config->bus_full_width = 0;
+	config->ranks = 2;
+	if (mctl_core_init(para, config))
+		return;
+
+	debug("testing 16-bit width, rank = 1\n");
+	config->bus_full_width = 0;
+	config->ranks = 1;
+	if (mctl_core_init(para, config))
+		return;
+
+	panic("This DRAM setup is currently not supported.\n");
+}
+
+static void mctl_write_pattern(void)
+{
+	unsigned int i;
+	u32 *ptr, val;
+
+	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
+	for (i = 0; i < 16; ptr++, i++) {
+		if (i & 1)
+			val = ~(ulong)ptr;
+		else
+			val = (ulong)ptr;
+		writel(val, ptr);
+	}
+}
+
+static bool mctl_check_pattern(ulong offset)
+{
+	unsigned int i;
+	u32 *ptr, val;
+
+	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
+	for (i = 0; i < 16; ptr++, i++) {
+		if (i & 1)
+			val = ~(ulong)ptr;
+		else
+			val = (ulong)ptr;
+		if (val != *(ptr + offset / 4))
+			return false;
+	}
+
+	return true;
+}
+
+void mctl_auto_detect_dram_size(const struct dram_para *para,
+				struct dram_config *config)
+{
+	unsigned int shift, cols, rows, found;
+	u32 buffer[16];
+
+	/* max. config for columns, but not rows */
+	config->cols = 11;
+	config->rows = 13;
+	mctl_core_init(para, config);
+
+	/*
+	 * Store content so it can be restored later. This is important
+	 * if controller was already initialized and holds any data
+	 * which is important for restoring system.
+	 */
+	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
+
+	mctl_write_pattern();
+
+	shift = config->bus_full_width + 1;
+
+	/* detect column address bits */
+	found = 0;
+	for (cols = 8; cols < 11; cols++) {
+		if (mctl_check_pattern(1ULL << (cols + shift))) {
+			found = 1;
+			break;
+		}
+	}
+	if (!found)
+		panic("DRAM init failed: Can't detect number of columns!");
+	debug("detected %u columns\n", cols);
+
+	/* restore data */
+	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
+
+	/* reconfigure to make sure that all active rows are accessible */
+	config->cols = 8;
+	config->rows = 17;
+	mctl_core_init(para, config);
+
+	/* store data again as it might be moved */
+	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
+
+	mctl_write_pattern();
+
+	/* detect row address bits */
+	shift = config->bus_full_width + 4 + config->cols;
+	found = 0;
+	for (rows = 13; rows < 17; rows++) {
+		if (mctl_check_pattern(1ULL << (rows + shift))) {
+			found = 1;
+			break;
+		}
+	}
+	if (!found)
+		panic("DRAM init failed: Can't detect number of rows!");
+	debug("detected %u rows\n", rows);
+
+	/* restore data again */
+	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
+
+	config->cols = cols;
+	config->rows = rows;
+}
+
+unsigned long mctl_calc_size(const struct dram_config *config)
+{
+	u8 width = config->bus_full_width ? 4 : 2;
+
+	/* 8 banks */
+	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
+}
diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
index 6a9e53f965eb..fbb865131e08 100644
--- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
+++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
@@ -10,6 +10,7 @@
 #include <asm/io.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/dram.h>
+#include <asm/arch/dram_dw_helpers.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/prcm.h>
 #include <linux/bitops.h>
@@ -40,8 +41,8 @@ static void mctl_com_init(const struct dram_para *para,
 static bool mctl_channel_init(const struct dram_para *para,
 			      const struct dram_config *config);
 
-static bool mctl_core_init(const struct dram_para *para,
-			   const struct dram_config *config)
+bool mctl_core_init(const struct dram_para *para,
+		    const struct dram_config *config)
 {
 	mctl_sys_init(para->clk);
 	mctl_com_init(para, config);
@@ -560,92 +561,6 @@ static bool mctl_channel_init(const struct dram_para *para,
 	return true;
 }
 
-static void mctl_auto_detect_rank_width(const struct dram_para *para,
-					struct dram_config *config)
-{
-	/* this is minimum size that it's supported */
-	config->cols = 8;
-	config->rows = 13;
-
-	/*
-	 * Previous versions of this driver tried to auto detect the rank
-	 * and width by looking at controller registers. However this proved
-	 * to be not reliable, so this approach here is the more robust
-	 * solution. Check the git history for details.
-	 *
-	 * Strategy here is to test most demanding combination first and least
-	 * demanding last, otherwise HW might not be fully utilized. For
-	 * example, half bus width and rank = 1 combination would also work
-	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
-	 * visible.
-	 */
-
-	debug("testing 32-bit width, rank = 2\n");
-	config->bus_full_width = 1;
-	config->ranks = 2;
-	if (mctl_core_init(para, config))
-		return;
-
-	debug("testing 32-bit width, rank = 1\n");
-	config->bus_full_width = 1;
-	config->ranks = 1;
-	if (mctl_core_init(para, config))
-		return;
-
-	debug("testing 16-bit width, rank = 2\n");
-	config->bus_full_width = 0;
-	config->ranks = 2;
-	if (mctl_core_init(para, config))
-		return;
-
-	debug("testing 16-bit width, rank = 1\n");
-	config->bus_full_width = 0;
-	config->ranks = 1;
-	if (mctl_core_init(para, config))
-		return;
-
-	panic("This DRAM setup is currently not supported.\n");
-}
-
-static void mctl_auto_detect_dram_size(const struct dram_para *para,
-				       struct dram_config *config)
-{
-	/* TODO: non-(LP)DDR3 */
-
-	/* detect row address bits */
-	config->cols = 8;
-	config->rows = 18;
-	mctl_core_init(para, config);
-
-	for (config->rows = 13; config->rows < 18; config->rows++) {
-		/* 8 banks, 8 bit per byte and 16/32 bit width */
-		if (mctl_mem_matches((1 << (config->rows + config->cols +
-					    4 + config->bus_full_width))))
-			break;
-	}
-
-	/* detect column address bits */
-	config->cols = 11;
-	mctl_core_init(para, config);
-
-	for (config->cols = 8; config->cols < 11; config->cols++) {
-		/* 8 bits per byte and 16/32 bit width */
-		if (mctl_mem_matches(1 << (config->cols + 1 +
-					   config->bus_full_width)))
-			break;
-	}
-}
-
-unsigned long mctl_calc_size(const struct dram_config *config)
-{
-	u8 width = config->bus_full_width ? 4 : 2;
-
-	/* TODO: non-(LP)DDR3 */
-
-	/* 8 banks */
-	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
-}
-
 #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
 	{{  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
 	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
index d1768a7e7d3a..80d9de557876 100644
--- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
+++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
@@ -17,6 +17,7 @@
 #include <asm/io.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/dram.h>
+#include <asm/arch/dram_dw_helpers.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/prcm.h>
 #include <linux/bitops.h>
@@ -1310,164 +1311,14 @@ static bool mctl_ctrl_init(const struct dram_para *para,
 	return true;
 }
 
-static bool mctl_core_init(const struct dram_para *para,
-			   const struct dram_config *config)
+bool mctl_core_init(const struct dram_para *para,
+		    const struct dram_config *config)
 {
 	mctl_sys_init(para->clk);
 
 	return mctl_ctrl_init(para, config);
 }
 
-static void mctl_auto_detect_rank_width(const struct dram_para *para,
-					struct dram_config *config)
-{
-	/* this is minimum size that it's supported */
-	config->cols = 8;
-	config->rows = 13;
-
-	/*
-	 * Strategy here is to test most demanding combination first and least
-	 * demanding last, otherwise HW might not be fully utilized. For
-	 * example, half bus width and rank = 1 combination would also work
-	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
-	 * visible.
-	 */
-
-	debug("testing 32-bit width, rank = 2\n");
-	config->bus_full_width = 1;
-	config->ranks = 2;
-	if (mctl_core_init(para, config))
-		return;
-
-	debug("testing 32-bit width, rank = 1\n");
-	config->bus_full_width = 1;
-	config->ranks = 1;
-	if (mctl_core_init(para, config))
-		return;
-
-	debug("testing 16-bit width, rank = 2\n");
-	config->bus_full_width = 0;
-	config->ranks = 2;
-	if (mctl_core_init(para, config))
-		return;
-
-	debug("testing 16-bit width, rank = 1\n");
-	config->bus_full_width = 0;
-	config->ranks = 1;
-	if (mctl_core_init(para, config))
-		return;
-
-	panic("This DRAM setup is currently not supported.\n");
-}
-
-static void mctl_write_pattern(void)
-{
-	unsigned int i;
-	u32 *ptr, val;
-
-	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
-	for (i = 0; i < 16; ptr++, i++) {
-		if (i & 1)
-			val = ~(ulong)ptr;
-		else
-			val = (ulong)ptr;
-		writel(val, ptr);
-	}
-}
-
-static bool mctl_check_pattern(ulong offset)
-{
-	unsigned int i;
-	u32 *ptr, val;
-
-	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
-	for (i = 0; i < 16; ptr++, i++) {
-		if (i & 1)
-			val = ~(ulong)ptr;
-		else
-			val = (ulong)ptr;
-		if (val != *(ptr + offset / 4))
-			return false;
-	}
-
-	return true;
-}
-
-static void mctl_auto_detect_dram_size(const struct dram_para *para,
-				       struct dram_config *config)
-{
-	unsigned int shift, cols, rows, found;
-	u32 buffer[16];
-
-	/* max. config for columns, but not rows */
-	config->cols = 11;
-	config->rows = 13;
-	mctl_core_init(para, config);
-
-	/*
-	 * Store content so it can be restored later. This is important
-	 * if controller was already initialized and holds any data
-	 * which is important for restoring system.
-	 */
-	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
-
-	mctl_write_pattern();
-
-	shift = config->bus_full_width + 1;
-
-	/* detect column address bits */
-	found = 0;
-	for (cols = 8; cols < 11; cols++) {
-		if (mctl_check_pattern(1ULL << (cols + shift))) {
-			found = 1;
-			break;
-		}
-	}
-	if (!found)
-		panic("DRAM init failed: Can't detect number of columns!");
-	debug("detected %u columns\n", cols);
-
-	/* restore data */
-	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
-
-	/* reconfigure to make sure that all active rows are accessible */
-	config->cols = 8;
-	config->rows = 17;
-	mctl_core_init(para, config);
-
-	/* store data again as it might be moved */
-	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
-
-	mctl_write_pattern();
-
-	/* detect row address bits */
-	shift = config->bus_full_width + 4 + config->cols;
-	found = 0;
-	for (rows = 13; rows < 17; rows++) {
-		if (mctl_check_pattern(1ULL << (rows + shift))) {
-			found = 1;
-			break;
-		}
-	}
-	if (!found)
-		panic("DRAM init failed: Can't detect number of rows!");
-	debug("detected %u rows\n", rows);
-
-	/* restore data again */
-	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
-
-	config->cols = cols;
-	config->rows = rows;
-}
-
-static unsigned long mctl_calc_size(const struct dram_config *config)
-{
-	u8 width = config->bus_full_width ? 4 : 2;
-
-	/* 8 banks */
-	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
-}
-
 static const struct dram_para para = {
 	.clk = CONFIG_DRAM_CLK,
 #ifdef CONFIG_SUNXI_DRAM_H616_DDR3_1333
-- 
2.49.0

Re: [PATCH 2/5] sunxi: H6: Remove useless DRAM timings parameter

[Andre Przywara]

On Fri, 11 Apr 2025 18:14:36 +0200
Jernej Skrabec <jernej.skrabec@gmail.com> wrote:

> This is just cosmetic fix for later easier rework.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h | 2 +-
>  arch/arm/mach-sunxi/dram_sun50i_h6.c             | 2 +-
>  arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c  | 2 +-
>  arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c     | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> index f0caecc807dd..f05a1845b32b 100644
> --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> @@ -330,6 +330,6 @@ static inline int ns_to_t(int nanoseconds)
>  	return DIV_ROUND_UP(ctrl_freq * nanoseconds, 1000);
>  }
>  
> -void mctl_set_timing_params(struct dram_para *para);
> +void mctl_set_timing_params(void);
>  
>  #endif /* _SUNXI_DRAM_SUN50I_H6_H */
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> index e7862bd06ea3..0adbda756639 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> @@ -45,7 +45,7 @@ static bool mctl_core_init(struct dram_para *para)
>  	switch (para->type) {
>  	case SUNXI_DRAM_TYPE_LPDDR3:
>  	case SUNXI_DRAM_TYPE_DDR3:
> -		mctl_set_timing_params(para);
> +		mctl_set_timing_params();
>  		break;
>  	default:
>  		panic("Unsupported DRAM type!");
> diff --git a/arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c b/arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c
> index afe8e25c7f58..1ed46fed411f 100644
> --- a/arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c
> +++ b/arch/arm/mach-sunxi/dram_timings/h6_ddr3_1333.c
> @@ -37,7 +37,7 @@ static u32 mr_ddr3[7] = {
>  };
>  
>  /* TODO: flexible timing */
> -void mctl_set_timing_params(struct dram_para *para)
> +void mctl_set_timing_params(void)
>  {
>  	struct sunxi_mctl_ctl_reg * const mctl_ctl =
>  			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
> diff --git a/arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c b/arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c
> index c243b574406d..c02f542c989f 100644
> --- a/arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c
> +++ b/arch/arm/mach-sunxi/dram_timings/h6_lpddr3.c
> @@ -16,7 +16,7 @@ static u32 mr_lpddr3[12] = {
>  };
>  
>  /* TODO: flexible timing */
> -void mctl_set_timing_params(struct dram_para *para)
> +void mctl_set_timing_params(void)
>  {
>  	struct sunxi_mctl_ctl_reg * const mctl_ctl =
>  			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;

Re: [PATCH 3/5] sunxi: H6: DRAM: Constify function parameters

[Andre Przywara]

On Fri, 11 Apr 2025 18:14:37 +0200
Jernej Skrabec <jernej.skrabec@gmail.com> wrote:

Hi,

> Constify parameters for two reasons:
> - Allow more compile time optimizations
> - It will allow later sharing of common code with H616 (when it will be
>   rearranged some more)
> 
> Commit does same kind of changes as 457e2cd665bd ("sunxi: H616: dram:
> const-ify DRAM function parameters")

Makes sense, and indeed all those functions only read from para:

> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arch/arm/mach-sunxi/dram_sun50i_h6.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> index 0adbda756639..24b2cb1579f4 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> @@ -34,13 +34,13 @@
>   * similar PHY is ZynqMP.
>   */
>  
> -static void mctl_sys_init(struct dram_para *para);
> -static void mctl_com_init(struct dram_para *para);
> -static bool mctl_channel_init(struct dram_para *para);
> +static void mctl_sys_init(u32 clk_rate);
> +static void mctl_com_init(const struct dram_para *para);
> +static bool mctl_channel_init(const struct dram_para *para);
>  
>  static bool mctl_core_init(struct dram_para *para)
>  {
> -	mctl_sys_init(para);
> +	mctl_sys_init(para->clk);
>  	mctl_com_init(para);
>  	switch (para->type) {
>  	case SUNXI_DRAM_TYPE_LPDDR3:
> @@ -150,7 +150,7 @@ static void mctl_set_master_priority(void)
>  	MBUS_CONF(HDCP2,  true,    HIGH, 2,  100,   64,   32);
>  }
>  
> -static void mctl_sys_init(struct dram_para *para)
> +static void mctl_sys_init(u32 clk_rate)
>  {
>  	struct sunxi_ccm_reg * const ccm =
>  			(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> @@ -171,7 +171,7 @@ static void mctl_sys_init(struct dram_para *para)
>  
>  	/* Set PLL5 rate to doubled DRAM clock rate */
>  	writel(CCM_PLL5_CTRL_EN | CCM_PLL5_LOCK_EN |
> -	       CCM_PLL5_CTRL_N(para->clk * 2 / 24), &ccm->pll5_cfg);
> +	       CCM_PLL5_CTRL_N(clk_rate * 2 / 24), &ccm->pll5_cfg);
>  	mctl_await_completion(&ccm->pll5_cfg, CCM_PLL5_LOCK, CCM_PLL5_LOCK);
>  
>  	/* Configure DRAM mod clock */
> @@ -196,7 +196,7 @@ static void mctl_sys_init(struct dram_para *para)
>  	writel(0x8000, &mctl_ctl->unk_0x00c);
>  }
>  
> -static void mctl_set_addrmap(struct dram_para *para)
> +static void mctl_set_addrmap(const struct dram_para *para)
>  {
>  	struct sunxi_mctl_ctl_reg * const mctl_ctl =
>  			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
> @@ -282,7 +282,7 @@ static void mctl_set_addrmap(struct dram_para *para)
>  	mctl_ctl->addrmap[8] = 0x3F3F;
>  }
>  
> -static void mctl_com_init(struct dram_para *para)
> +static void mctl_com_init(const struct dram_para *para)
>  {
>  	struct sunxi_mctl_com_reg * const mctl_com =
>  			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
> @@ -352,7 +352,7 @@ static void mctl_com_init(struct dram_para *para)
>  	}
>  }
>  
> -static void mctl_bit_delay_set(struct dram_para *para)
> +static void mctl_bit_delay_set(const struct dram_para *para)
>  {
>  	struct sunxi_mctl_phy_reg * const mctl_phy =
>  			(struct sunxi_mctl_phy_reg *)SUNXI_DRAM_PHY0_BASE;
> @@ -411,7 +411,7 @@ static void mctl_bit_delay_set(struct dram_para *para)
>  	}
>  }
>  
> -static bool mctl_channel_init(struct dram_para *para)
> +static bool mctl_channel_init(const struct dram_para *para)
>  {
>  	struct sunxi_mctl_com_reg * const mctl_com =
>  			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;

Re: [PATCH 5/5] sunxi: h6/h616: Reuse common DRAM infrastructure

[Andre Przywara]

On Fri, 11 Apr 2025 18:14:39 +0200
Jernej Skrabec <jernej.skrabec@gmail.com> wrote:

Hi Jernej,

> H616 rank and size detection code is superior to the H6. Nevertheless,
> they are structurally the same. Split functions from H616 into new file
> and reuse them in H6 DRAM driver too. This should also fix some bugs for
> H6 too, like incorrect DRAM size detection.

that's a nice patch, but actually breaks on my Pine H64:

U-Boot SPL 2025.04-01089-g14694431269f (Apr 15 2025 - 22:55:07 +0100)
DRAM:DRAM init failed: Can't detect number of columns!
resetting ...

It's none of the previous H6 reworks that causes the problem, but it's
actually already in the current code somehow: If I apply the equivalent
of [PATCH 1/5] to the H6 code, it already complains. Replacing the
panic with a printf() gives me (on mainline):

U-Boot SPL 2025.04-01085-g7a43fe97dc86-dirty (Apr 15 2025 - 23:48:42 +0100)
DRAM:detected 14 rows (found=1)
detected 11 columns (found=0)
 2048 MiB
Trying to boot from FEL
NOTICE:  BL31: v2.12.0(debug):v2.12.0-724-gf745e004a
   < continues booting >

So it seems like the match failed, but apparently 11 columns are correct anyway.
And since the same happens after this patch, I assume that even the
newer and better matching routine doesn't change that, unfortunately.

The config on this board is: 2 ranks, full width, 11 cols, 14 rows => 2GB.
And that seems to work (in Linux).
It's a single chip Elpida FA232A2MA JD-F, listed by Micron as
EDFA232A2MA-JD-F-D, though I couldn't find a datasheet quickly.

Cheers,
Andre

> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  .../include/asm/arch-sunxi/dram_dw_helpers.h  |  22 +++
>  arch/arm/mach-sunxi/Makefile                  |   4 +-
>  arch/arm/mach-sunxi/dram_dw_helpers.c         | 160 ++++++++++++++++++
>  arch/arm/mach-sunxi/dram_sun50i_h6.c          |  91 +---------
>  arch/arm/mach-sunxi/dram_sun50i_h616.c        | 155 +----------------
>  5 files changed, 190 insertions(+), 242 deletions(-)
>  create mode 100644 arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
>  create mode 100644 arch/arm/mach-sunxi/dram_dw_helpers.c
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> new file mode 100644
> index 000000000000..bc9e0d868c55
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Helpers that are commonly used with DW memory controller.
> + *
> + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> + *
> + */
> +
> +#ifndef _DRAM_DW_HELPERS_H
> +#define _DRAM_DW_HELPERS_H
> +
> +#include <asm/arch/dram.h>
> +
> +bool mctl_core_init(const struct dram_para *para,
> +		    const struct dram_config *config);
> +void mctl_auto_detect_rank_width(const struct dram_para *para,
> +				 struct dram_config *config);
> +void mctl_auto_detect_dram_size(const struct dram_para *para,
> +				struct dram_config *config);
> +unsigned long mctl_calc_size(const struct dram_config *config);
> +
> +#endif
> diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
> index eb6a49119a13..a33cd5b0f07a 100644
> --- a/arch/arm/mach-sunxi/Makefile
> +++ b/arch/arm/mach-sunxi/Makefile
> @@ -41,8 +41,8 @@ obj-$(CONFIG_DRAM_SUN9I)	+= dram_sun9i.o
>  obj-$(CONFIG_SPL_SPI_SUNXI)	+= spl_spi_sunxi.o
>  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_sunxi_dw.o
>  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_timings/
> -obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o
> +obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o dram_dw_helpers.o
>  obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_timings/
> -obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o
> +obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o dram_dw_helpers.o
>  obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_timings/
>  endif
> diff --git a/arch/arm/mach-sunxi/dram_dw_helpers.c b/arch/arm/mach-sunxi/dram_dw_helpers.c
> new file mode 100644
> index 000000000000..885d1f2c0b12
> --- /dev/null
> +++ b/arch/arm/mach-sunxi/dram_dw_helpers.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Helpers that are commonly used with DW memory controller.
> + *
> + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> + *
> + */
> +
> +#include <init.h>
> +#include <asm/arch/dram_dw_helpers.h>
> +
> +void mctl_auto_detect_rank_width(const struct dram_para *para,
> +				 struct dram_config *config)
> +{
> +	/* this is minimum size that it's supported */
> +	config->cols = 8;
> +	config->rows = 13;
> +
> +	/*
> +	 * Strategy here is to test most demanding combination first and least
> +	 * demanding last, otherwise HW might not be fully utilized. For
> +	 * example, half bus width and rank = 1 combination would also work
> +	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> +	 * visible.
> +	 */
> +
> +	debug("testing 32-bit width, rank = 2\n");
> +	config->bus_full_width = 1;
> +	config->ranks = 2;
> +	if (mctl_core_init(para, config))
> +		return;
> +
> +	debug("testing 32-bit width, rank = 1\n");
> +	config->bus_full_width = 1;
> +	config->ranks = 1;
> +	if (mctl_core_init(para, config))
> +		return;
> +
> +	debug("testing 16-bit width, rank = 2\n");
> +	config->bus_full_width = 0;
> +	config->ranks = 2;
> +	if (mctl_core_init(para, config))
> +		return;
> +
> +	debug("testing 16-bit width, rank = 1\n");
> +	config->bus_full_width = 0;
> +	config->ranks = 1;
> +	if (mctl_core_init(para, config))
> +		return;
> +
> +	panic("This DRAM setup is currently not supported.\n");
> +}
> +
> +static void mctl_write_pattern(void)
> +{
> +	unsigned int i;
> +	u32 *ptr, val;
> +
> +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> +	for (i = 0; i < 16; ptr++, i++) {
> +		if (i & 1)
> +			val = ~(ulong)ptr;
> +		else
> +			val = (ulong)ptr;
> +		writel(val, ptr);
> +	}
> +}
> +
> +static bool mctl_check_pattern(ulong offset)
> +{
> +	unsigned int i;
> +	u32 *ptr, val;
> +
> +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> +	for (i = 0; i < 16; ptr++, i++) {
> +		if (i & 1)
> +			val = ~(ulong)ptr;
> +		else
> +			val = (ulong)ptr;
> +		if (val != *(ptr + offset / 4))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +void mctl_auto_detect_dram_size(const struct dram_para *para,
> +				struct dram_config *config)
> +{
> +	unsigned int shift, cols, rows, found;
> +	u32 buffer[16];
> +
> +	/* max. config for columns, but not rows */
> +	config->cols = 11;
> +	config->rows = 13;
> +	mctl_core_init(para, config);
> +
> +	/*
> +	 * Store content so it can be restored later. This is important
> +	 * if controller was already initialized and holds any data
> +	 * which is important for restoring system.
> +	 */
> +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> +
> +	mctl_write_pattern();
> +
> +	shift = config->bus_full_width + 1;
> +
> +	/* detect column address bits */
> +	found = 0;
> +	for (cols = 8; cols < 11; cols++) {
> +		if (mctl_check_pattern(1ULL << (cols + shift))) {
> +			found = 1;
> +			break;
> +		}
> +	}
> +	if (!found)
> +		panic("DRAM init failed: Can't detect number of columns!");
> +	debug("detected %u columns\n", cols);
> +
> +	/* restore data */
> +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> +
> +	/* reconfigure to make sure that all active rows are accessible */
> +	config->cols = 8;
> +	config->rows = 17;
> +	mctl_core_init(para, config);
> +
> +	/* store data again as it might be moved */
> +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> +
> +	mctl_write_pattern();
> +
> +	/* detect row address bits */
> +	shift = config->bus_full_width + 4 + config->cols;
> +	found = 0;
> +	for (rows = 13; rows < 17; rows++) {
> +		if (mctl_check_pattern(1ULL << (rows + shift))) {
> +			found = 1;
> +			break;
> +		}
> +	}
> +	if (!found)
> +		panic("DRAM init failed: Can't detect number of rows!");
> +	debug("detected %u rows\n", rows);
> +
> +	/* restore data again */
> +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> +
> +	config->cols = cols;
> +	config->rows = rows;
> +}
> +
> +unsigned long mctl_calc_size(const struct dram_config *config)
> +{
> +	u8 width = config->bus_full_width ? 4 : 2;
> +
> +	/* 8 banks */
> +	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> +}
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> index 6a9e53f965eb..fbb865131e08 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> @@ -10,6 +10,7 @@
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
>  #include <asm/arch/dram.h>
> +#include <asm/arch/dram_dw_helpers.h>
>  #include <asm/arch/cpu.h>
>  #include <asm/arch/prcm.h>
>  #include <linux/bitops.h>
> @@ -40,8 +41,8 @@ static void mctl_com_init(const struct dram_para *para,
>  static bool mctl_channel_init(const struct dram_para *para,
>  			      const struct dram_config *config);
>  
> -static bool mctl_core_init(const struct dram_para *para,
> -			   const struct dram_config *config)
> +bool mctl_core_init(const struct dram_para *para,
> +		    const struct dram_config *config)
>  {
>  	mctl_sys_init(para->clk);
>  	mctl_com_init(para, config);
> @@ -560,92 +561,6 @@ static bool mctl_channel_init(const struct dram_para *para,
>  	return true;
>  }
>  
> -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> -					struct dram_config *config)
> -{
> -	/* this is minimum size that it's supported */
> -	config->cols = 8;
> -	config->rows = 13;
> -
> -	/*
> -	 * Previous versions of this driver tried to auto detect the rank
> -	 * and width by looking at controller registers. However this proved
> -	 * to be not reliable, so this approach here is the more robust
> -	 * solution. Check the git history for details.
> -	 *
> -	 * Strategy here is to test most demanding combination first and least
> -	 * demanding last, otherwise HW might not be fully utilized. For
> -	 * example, half bus width and rank = 1 combination would also work
> -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> -	 * visible.
> -	 */
> -
> -	debug("testing 32-bit width, rank = 2\n");
> -	config->bus_full_width = 1;
> -	config->ranks = 2;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 32-bit width, rank = 1\n");
> -	config->bus_full_width = 1;
> -	config->ranks = 1;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 16-bit width, rank = 2\n");
> -	config->bus_full_width = 0;
> -	config->ranks = 2;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 16-bit width, rank = 1\n");
> -	config->bus_full_width = 0;
> -	config->ranks = 1;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	panic("This DRAM setup is currently not supported.\n");
> -}
> -
> -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> -				       struct dram_config *config)
> -{
> -	/* TODO: non-(LP)DDR3 */
> -
> -	/* detect row address bits */
> -	config->cols = 8;
> -	config->rows = 18;
> -	mctl_core_init(para, config);
> -
> -	for (config->rows = 13; config->rows < 18; config->rows++) {
> -		/* 8 banks, 8 bit per byte and 16/32 bit width */
> -		if (mctl_mem_matches((1 << (config->rows + config->cols +
> -					    4 + config->bus_full_width))))
> -			break;
> -	}
> -
> -	/* detect column address bits */
> -	config->cols = 11;
> -	mctl_core_init(para, config);
> -
> -	for (config->cols = 8; config->cols < 11; config->cols++) {
> -		/* 8 bits per byte and 16/32 bit width */
> -		if (mctl_mem_matches(1 << (config->cols + 1 +
> -					   config->bus_full_width)))
> -			break;
> -	}
> -}
> -
> -unsigned long mctl_calc_size(const struct dram_config *config)
> -{
> -	u8 width = config->bus_full_width ? 4 : 2;
> -
> -	/* TODO: non-(LP)DDR3 */
> -
> -	/* 8 banks */
> -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> -}
> -
>  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
>  	{{  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
>  	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> index d1768a7e7d3a..80d9de557876 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> @@ -17,6 +17,7 @@
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
>  #include <asm/arch/dram.h>
> +#include <asm/arch/dram_dw_helpers.h>
>  #include <asm/arch/cpu.h>
>  #include <asm/arch/prcm.h>
>  #include <linux/bitops.h>
> @@ -1310,164 +1311,14 @@ static bool mctl_ctrl_init(const struct dram_para *para,
>  	return true;
>  }
>  
> -static bool mctl_core_init(const struct dram_para *para,
> -			   const struct dram_config *config)
> +bool mctl_core_init(const struct dram_para *para,
> +		    const struct dram_config *config)
>  {
>  	mctl_sys_init(para->clk);
>  
>  	return mctl_ctrl_init(para, config);
>  }
>  
> -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> -					struct dram_config *config)
> -{
> -	/* this is minimum size that it's supported */
> -	config->cols = 8;
> -	config->rows = 13;
> -
> -	/*
> -	 * Strategy here is to test most demanding combination first and least
> -	 * demanding last, otherwise HW might not be fully utilized. For
> -	 * example, half bus width and rank = 1 combination would also work
> -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> -	 * visible.
> -	 */
> -
> -	debug("testing 32-bit width, rank = 2\n");
> -	config->bus_full_width = 1;
> -	config->ranks = 2;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 32-bit width, rank = 1\n");
> -	config->bus_full_width = 1;
> -	config->ranks = 1;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 16-bit width, rank = 2\n");
> -	config->bus_full_width = 0;
> -	config->ranks = 2;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 16-bit width, rank = 1\n");
> -	config->bus_full_width = 0;
> -	config->ranks = 1;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	panic("This DRAM setup is currently not supported.\n");
> -}
> -
> -static void mctl_write_pattern(void)
> -{
> -	unsigned int i;
> -	u32 *ptr, val;
> -
> -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> -	for (i = 0; i < 16; ptr++, i++) {
> -		if (i & 1)
> -			val = ~(ulong)ptr;
> -		else
> -			val = (ulong)ptr;
> -		writel(val, ptr);
> -	}
> -}
> -
> -static bool mctl_check_pattern(ulong offset)
> -{
> -	unsigned int i;
> -	u32 *ptr, val;
> -
> -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> -	for (i = 0; i < 16; ptr++, i++) {
> -		if (i & 1)
> -			val = ~(ulong)ptr;
> -		else
> -			val = (ulong)ptr;
> -		if (val != *(ptr + offset / 4))
> -			return false;
> -	}
> -
> -	return true;
> -}
> -
> -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> -				       struct dram_config *config)
> -{
> -	unsigned int shift, cols, rows, found;
> -	u32 buffer[16];
> -
> -	/* max. config for columns, but not rows */
> -	config->cols = 11;
> -	config->rows = 13;
> -	mctl_core_init(para, config);
> -
> -	/*
> -	 * Store content so it can be restored later. This is important
> -	 * if controller was already initialized and holds any data
> -	 * which is important for restoring system.
> -	 */
> -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> -
> -	mctl_write_pattern();
> -
> -	shift = config->bus_full_width + 1;
> -
> -	/* detect column address bits */
> -	found = 0;
> -	for (cols = 8; cols < 11; cols++) {
> -		if (mctl_check_pattern(1ULL << (cols + shift))) {
> -			found = 1;
> -			break;
> -		}
> -	}
> -	if (!found)
> -		panic("DRAM init failed: Can't detect number of columns!");
> -	debug("detected %u columns\n", cols);
> -
> -	/* restore data */
> -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> -
> -	/* reconfigure to make sure that all active rows are accessible */
> -	config->cols = 8;
> -	config->rows = 17;
> -	mctl_core_init(para, config);
> -
> -	/* store data again as it might be moved */
> -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> -
> -	mctl_write_pattern();
> -
> -	/* detect row address bits */
> -	shift = config->bus_full_width + 4 + config->cols;
> -	found = 0;
> -	for (rows = 13; rows < 17; rows++) {
> -		if (mctl_check_pattern(1ULL << (rows + shift))) {
> -			found = 1;
> -			break;
> -		}
> -	}
> -	if (!found)
> -		panic("DRAM init failed: Can't detect number of rows!");
> -	debug("detected %u rows\n", rows);
> -
> -	/* restore data again */
> -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> -
> -	config->cols = cols;
> -	config->rows = rows;
> -}
> -
> -static unsigned long mctl_calc_size(const struct dram_config *config)
> -{
> -	u8 width = config->bus_full_width ? 4 : 2;
> -
> -	/* 8 banks */
> -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> -}
> -
>  static const struct dram_para para = {
>  	.clk = CONFIG_DRAM_CLK,
>  #ifdef CONFIG_SUNXI_DRAM_H616_DDR3_1333

Re: [PATCH 5/5] sunxi: h6/h616: Reuse common DRAM infrastructure

[Jernej Škrabec]

Dne sreda, 16. april 2025 ob 01:40:04 Srednjeevropski poletni čas je Andre Przywara napisal(a):
> On Fri, 11 Apr 2025 18:14:39 +0200
> Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> 
> Hi Jernej,
> 
> > H616 rank and size detection code is superior to the H6. Nevertheless,
> > they are structurally the same. Split functions from H616 into new file
> > and reuse them in H6 DRAM driver too. This should also fix some bugs for
> > H6 too, like incorrect DRAM size detection.
> 
> that's a nice patch, but actually breaks on my Pine H64:
> 
> U-Boot SPL 2025.04-01089-g14694431269f (Apr 15 2025 - 22:55:07 +0100)
> DRAM:DRAM init failed: Can't detect number of columns!
> resetting ...
> 
> It's none of the previous H6 reworks that causes the problem, but it's
> actually already in the current code somehow: If I apply the equivalent
> of [PATCH 1/5] to the H6 code, it already complains. Replacing the
> panic with a printf() gives me (on mainline):
> 
> U-Boot SPL 2025.04-01085-g7a43fe97dc86-dirty (Apr 15 2025 - 23:48:42 +0100)
> DRAM:detected 14 rows (found=1)
> detected 11 columns (found=0)
>  2048 MiB
> Trying to boot from FEL
> NOTICE:  BL31: v2.12.0(debug):v2.12.0-724-gf745e004a
>    < continues booting >
> 
> So it seems like the match failed, but apparently 11 columns are correct anyway.
> And since the same happens after this patch, I assume that even the
> newer and better matching routine doesn't change that, unfortunately.
> 
> The config on this board is: 2 ranks, full width, 11 cols, 14 rows => 2GB.
> And that seems to work (in Linux).
> It's a single chip Elpida FA232A2MA JD-F, listed by Micron as
> EDFA232A2MA-JD-F-D, though I couldn't find a datasheet quickly.

Thanks for testing! It looks like it's best just to drop first patch in the
series. 11 columns is for sure valid. Not that sure about 17 rows, but
better leave it as is.

What do you think?

Best regards,
Jernej

> 
> Cheers,
> Andre
> 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > ---
> >  .../include/asm/arch-sunxi/dram_dw_helpers.h  |  22 +++
> >  arch/arm/mach-sunxi/Makefile                  |   4 +-
> >  arch/arm/mach-sunxi/dram_dw_helpers.c         | 160 ++++++++++++++++++
> >  arch/arm/mach-sunxi/dram_sun50i_h6.c          |  91 +---------
> >  arch/arm/mach-sunxi/dram_sun50i_h616.c        | 155 +----------------
> >  5 files changed, 190 insertions(+), 242 deletions(-)
> >  create mode 100644 arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> >  create mode 100644 arch/arm/mach-sunxi/dram_dw_helpers.c
> > 
> > diff --git a/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> > new file mode 100644
> > index 000000000000..bc9e0d868c55
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Helpers that are commonly used with DW memory controller.
> > + *
> > + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> > + *
> > + */
> > +
> > +#ifndef _DRAM_DW_HELPERS_H
> > +#define _DRAM_DW_HELPERS_H
> > +
> > +#include <asm/arch/dram.h>
> > +
> > +bool mctl_core_init(const struct dram_para *para,
> > +		    const struct dram_config *config);
> > +void mctl_auto_detect_rank_width(const struct dram_para *para,
> > +				 struct dram_config *config);
> > +void mctl_auto_detect_dram_size(const struct dram_para *para,
> > +				struct dram_config *config);
> > +unsigned long mctl_calc_size(const struct dram_config *config);
> > +
> > +#endif
> > diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
> > index eb6a49119a13..a33cd5b0f07a 100644
> > --- a/arch/arm/mach-sunxi/Makefile
> > +++ b/arch/arm/mach-sunxi/Makefile
> > @@ -41,8 +41,8 @@ obj-$(CONFIG_DRAM_SUN9I)	+= dram_sun9i.o
> >  obj-$(CONFIG_SPL_SPI_SUNXI)	+= spl_spi_sunxi.o
> >  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_sunxi_dw.o
> >  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_timings/
> > -obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o
> > +obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o dram_dw_helpers.o
> >  obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_timings/
> > -obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o
> > +obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o dram_dw_helpers.o
> >  obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_timings/
> >  endif
> > diff --git a/arch/arm/mach-sunxi/dram_dw_helpers.c b/arch/arm/mach-sunxi/dram_dw_helpers.c
> > new file mode 100644
> > index 000000000000..885d1f2c0b12
> > --- /dev/null
> > +++ b/arch/arm/mach-sunxi/dram_dw_helpers.c
> > @@ -0,0 +1,160 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Helpers that are commonly used with DW memory controller.
> > + *
> > + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> > + *
> > + */
> > +
> > +#include <init.h>
> > +#include <asm/arch/dram_dw_helpers.h>
> > +
> > +void mctl_auto_detect_rank_width(const struct dram_para *para,
> > +				 struct dram_config *config)
> > +{
> > +	/* this is minimum size that it's supported */
> > +	config->cols = 8;
> > +	config->rows = 13;
> > +
> > +	/*
> > +	 * Strategy here is to test most demanding combination first and least
> > +	 * demanding last, otherwise HW might not be fully utilized. For
> > +	 * example, half bus width and rank = 1 combination would also work
> > +	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> > +	 * visible.
> > +	 */
> > +
> > +	debug("testing 32-bit width, rank = 2\n");
> > +	config->bus_full_width = 1;
> > +	config->ranks = 2;
> > +	if (mctl_core_init(para, config))
> > +		return;
> > +
> > +	debug("testing 32-bit width, rank = 1\n");
> > +	config->bus_full_width = 1;
> > +	config->ranks = 1;
> > +	if (mctl_core_init(para, config))
> > +		return;
> > +
> > +	debug("testing 16-bit width, rank = 2\n");
> > +	config->bus_full_width = 0;
> > +	config->ranks = 2;
> > +	if (mctl_core_init(para, config))
> > +		return;
> > +
> > +	debug("testing 16-bit width, rank = 1\n");
> > +	config->bus_full_width = 0;
> > +	config->ranks = 1;
> > +	if (mctl_core_init(para, config))
> > +		return;
> > +
> > +	panic("This DRAM setup is currently not supported.\n");
> > +}
> > +
> > +static void mctl_write_pattern(void)
> > +{
> > +	unsigned int i;
> > +	u32 *ptr, val;
> > +
> > +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > +	for (i = 0; i < 16; ptr++, i++) {
> > +		if (i & 1)
> > +			val = ~(ulong)ptr;
> > +		else
> > +			val = (ulong)ptr;
> > +		writel(val, ptr);
> > +	}
> > +}
> > +
> > +static bool mctl_check_pattern(ulong offset)
> > +{
> > +	unsigned int i;
> > +	u32 *ptr, val;
> > +
> > +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > +	for (i = 0; i < 16; ptr++, i++) {
> > +		if (i & 1)
> > +			val = ~(ulong)ptr;
> > +		else
> > +			val = (ulong)ptr;
> > +		if (val != *(ptr + offset / 4))
> > +			return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +void mctl_auto_detect_dram_size(const struct dram_para *para,
> > +				struct dram_config *config)
> > +{
> > +	unsigned int shift, cols, rows, found;
> > +	u32 buffer[16];
> > +
> > +	/* max. config for columns, but not rows */
> > +	config->cols = 11;
> > +	config->rows = 13;
> > +	mctl_core_init(para, config);
> > +
> > +	/*
> > +	 * Store content so it can be restored later. This is important
> > +	 * if controller was already initialized and holds any data
> > +	 * which is important for restoring system.
> > +	 */
> > +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > +
> > +	mctl_write_pattern();
> > +
> > +	shift = config->bus_full_width + 1;
> > +
> > +	/* detect column address bits */
> > +	found = 0;
> > +	for (cols = 8; cols < 11; cols++) {
> > +		if (mctl_check_pattern(1ULL << (cols + shift))) {
> > +			found = 1;
> > +			break;
> > +		}
> > +	}
> > +	if (!found)
> > +		panic("DRAM init failed: Can't detect number of columns!");
> > +	debug("detected %u columns\n", cols);
> > +
> > +	/* restore data */
> > +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > +
> > +	/* reconfigure to make sure that all active rows are accessible */
> > +	config->cols = 8;
> > +	config->rows = 17;
> > +	mctl_core_init(para, config);
> > +
> > +	/* store data again as it might be moved */
> > +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > +
> > +	mctl_write_pattern();
> > +
> > +	/* detect row address bits */
> > +	shift = config->bus_full_width + 4 + config->cols;
> > +	found = 0;
> > +	for (rows = 13; rows < 17; rows++) {
> > +		if (mctl_check_pattern(1ULL << (rows + shift))) {
> > +			found = 1;
> > +			break;
> > +		}
> > +	}
> > +	if (!found)
> > +		panic("DRAM init failed: Can't detect number of rows!");
> > +	debug("detected %u rows\n", rows);
> > +
> > +	/* restore data again */
> > +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > +
> > +	config->cols = cols;
> > +	config->rows = rows;
> > +}
> > +
> > +unsigned long mctl_calc_size(const struct dram_config *config)
> > +{
> > +	u8 width = config->bus_full_width ? 4 : 2;
> > +
> > +	/* 8 banks */
> > +	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> > +}
> > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > index 6a9e53f965eb..fbb865131e08 100644
> > --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > @@ -10,6 +10,7 @@
> >  #include <asm/io.h>
> >  #include <asm/arch/clock.h>
> >  #include <asm/arch/dram.h>
> > +#include <asm/arch/dram_dw_helpers.h>
> >  #include <asm/arch/cpu.h>
> >  #include <asm/arch/prcm.h>
> >  #include <linux/bitops.h>
> > @@ -40,8 +41,8 @@ static void mctl_com_init(const struct dram_para *para,
> >  static bool mctl_channel_init(const struct dram_para *para,
> >  			      const struct dram_config *config);
> >  
> > -static bool mctl_core_init(const struct dram_para *para,
> > -			   const struct dram_config *config)
> > +bool mctl_core_init(const struct dram_para *para,
> > +		    const struct dram_config *config)
> >  {
> >  	mctl_sys_init(para->clk);
> >  	mctl_com_init(para, config);
> > @@ -560,92 +561,6 @@ static bool mctl_channel_init(const struct dram_para *para,
> >  	return true;
> >  }
> >  
> > -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> > -					struct dram_config *config)
> > -{
> > -	/* this is minimum size that it's supported */
> > -	config->cols = 8;
> > -	config->rows = 13;
> > -
> > -	/*
> > -	 * Previous versions of this driver tried to auto detect the rank
> > -	 * and width by looking at controller registers. However this proved
> > -	 * to be not reliable, so this approach here is the more robust
> > -	 * solution. Check the git history for details.
> > -	 *
> > -	 * Strategy here is to test most demanding combination first and least
> > -	 * demanding last, otherwise HW might not be fully utilized. For
> > -	 * example, half bus width and rank = 1 combination would also work
> > -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> > -	 * visible.
> > -	 */
> > -
> > -	debug("testing 32-bit width, rank = 2\n");
> > -	config->bus_full_width = 1;
> > -	config->ranks = 2;
> > -	if (mctl_core_init(para, config))
> > -		return;
> > -
> > -	debug("testing 32-bit width, rank = 1\n");
> > -	config->bus_full_width = 1;
> > -	config->ranks = 1;
> > -	if (mctl_core_init(para, config))
> > -		return;
> > -
> > -	debug("testing 16-bit width, rank = 2\n");
> > -	config->bus_full_width = 0;
> > -	config->ranks = 2;
> > -	if (mctl_core_init(para, config))
> > -		return;
> > -
> > -	debug("testing 16-bit width, rank = 1\n");
> > -	config->bus_full_width = 0;
> > -	config->ranks = 1;
> > -	if (mctl_core_init(para, config))
> > -		return;
> > -
> > -	panic("This DRAM setup is currently not supported.\n");
> > -}
> > -
> > -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> > -				       struct dram_config *config)
> > -{
> > -	/* TODO: non-(LP)DDR3 */
> > -
> > -	/* detect row address bits */
> > -	config->cols = 8;
> > -	config->rows = 18;
> > -	mctl_core_init(para, config);
> > -
> > -	for (config->rows = 13; config->rows < 18; config->rows++) {
> > -		/* 8 banks, 8 bit per byte and 16/32 bit width */
> > -		if (mctl_mem_matches((1 << (config->rows + config->cols +
> > -					    4 + config->bus_full_width))))
> > -			break;
> > -	}
> > -
> > -	/* detect column address bits */
> > -	config->cols = 11;
> > -	mctl_core_init(para, config);
> > -
> > -	for (config->cols = 8; config->cols < 11; config->cols++) {
> > -		/* 8 bits per byte and 16/32 bit width */
> > -		if (mctl_mem_matches(1 << (config->cols + 1 +
> > -					   config->bus_full_width)))
> > -			break;
> > -	}
> > -}
> > -
> > -unsigned long mctl_calc_size(const struct dram_config *config)
> > -{
> > -	u8 width = config->bus_full_width ? 4 : 2;
> > -
> > -	/* TODO: non-(LP)DDR3 */
> > -
> > -	/* 8 banks */
> > -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> > -}
> > -
> >  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
> >  	{{  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
> >  	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
> > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > index d1768a7e7d3a..80d9de557876 100644
> > --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > @@ -17,6 +17,7 @@
> >  #include <asm/io.h>
> >  #include <asm/arch/clock.h>
> >  #include <asm/arch/dram.h>
> > +#include <asm/arch/dram_dw_helpers.h>
> >  #include <asm/arch/cpu.h>
> >  #include <asm/arch/prcm.h>
> >  #include <linux/bitops.h>
> > @@ -1310,164 +1311,14 @@ static bool mctl_ctrl_init(const struct dram_para *para,
> >  	return true;
> >  }
> >  
> > -static bool mctl_core_init(const struct dram_para *para,
> > -			   const struct dram_config *config)
> > +bool mctl_core_init(const struct dram_para *para,
> > +		    const struct dram_config *config)
> >  {
> >  	mctl_sys_init(para->clk);
> >  
> >  	return mctl_ctrl_init(para, config);
> >  }
> >  
> > -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> > -					struct dram_config *config)
> > -{
> > -	/* this is minimum size that it's supported */
> > -	config->cols = 8;
> > -	config->rows = 13;
> > -
> > -	/*
> > -	 * Strategy here is to test most demanding combination first and least
> > -	 * demanding last, otherwise HW might not be fully utilized. For
> > -	 * example, half bus width and rank = 1 combination would also work
> > -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> > -	 * visible.
> > -	 */
> > -
> > -	debug("testing 32-bit width, rank = 2\n");
> > -	config->bus_full_width = 1;
> > -	config->ranks = 2;
> > -	if (mctl_core_init(para, config))
> > -		return;
> > -
> > -	debug("testing 32-bit width, rank = 1\n");
> > -	config->bus_full_width = 1;
> > -	config->ranks = 1;
> > -	if (mctl_core_init(para, config))
> > -		return;
> > -
> > -	debug("testing 16-bit width, rank = 2\n");
> > -	config->bus_full_width = 0;
> > -	config->ranks = 2;
> > -	if (mctl_core_init(para, config))
> > -		return;
> > -
> > -	debug("testing 16-bit width, rank = 1\n");
> > -	config->bus_full_width = 0;
> > -	config->ranks = 1;
> > -	if (mctl_core_init(para, config))
> > -		return;
> > -
> > -	panic("This DRAM setup is currently not supported.\n");
> > -}
> > -
> > -static void mctl_write_pattern(void)
> > -{
> > -	unsigned int i;
> > -	u32 *ptr, val;
> > -
> > -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > -	for (i = 0; i < 16; ptr++, i++) {
> > -		if (i & 1)
> > -			val = ~(ulong)ptr;
> > -		else
> > -			val = (ulong)ptr;
> > -		writel(val, ptr);
> > -	}
> > -}
> > -
> > -static bool mctl_check_pattern(ulong offset)
> > -{
> > -	unsigned int i;
> > -	u32 *ptr, val;
> > -
> > -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > -	for (i = 0; i < 16; ptr++, i++) {
> > -		if (i & 1)
> > -			val = ~(ulong)ptr;
> > -		else
> > -			val = (ulong)ptr;
> > -		if (val != *(ptr + offset / 4))
> > -			return false;
> > -	}
> > -
> > -	return true;
> > -}
> > -
> > -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> > -				       struct dram_config *config)
> > -{
> > -	unsigned int shift, cols, rows, found;
> > -	u32 buffer[16];
> > -
> > -	/* max. config for columns, but not rows */
> > -	config->cols = 11;
> > -	config->rows = 13;
> > -	mctl_core_init(para, config);
> > -
> > -	/*
> > -	 * Store content so it can be restored later. This is important
> > -	 * if controller was already initialized and holds any data
> > -	 * which is important for restoring system.
> > -	 */
> > -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > -
> > -	mctl_write_pattern();
> > -
> > -	shift = config->bus_full_width + 1;
> > -
> > -	/* detect column address bits */
> > -	found = 0;
> > -	for (cols = 8; cols < 11; cols++) {
> > -		if (mctl_check_pattern(1ULL << (cols + shift))) {
> > -			found = 1;
> > -			break;
> > -		}
> > -	}
> > -	if (!found)
> > -		panic("DRAM init failed: Can't detect number of columns!");
> > -	debug("detected %u columns\n", cols);
> > -
> > -	/* restore data */
> > -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > -
> > -	/* reconfigure to make sure that all active rows are accessible */
> > -	config->cols = 8;
> > -	config->rows = 17;
> > -	mctl_core_init(para, config);
> > -
> > -	/* store data again as it might be moved */
> > -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > -
> > -	mctl_write_pattern();
> > -
> > -	/* detect row address bits */
> > -	shift = config->bus_full_width + 4 + config->cols;
> > -	found = 0;
> > -	for (rows = 13; rows < 17; rows++) {
> > -		if (mctl_check_pattern(1ULL << (rows + shift))) {
> > -			found = 1;
> > -			break;
> > -		}
> > -	}
> > -	if (!found)
> > -		panic("DRAM init failed: Can't detect number of rows!");
> > -	debug("detected %u rows\n", rows);
> > -
> > -	/* restore data again */
> > -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > -
> > -	config->cols = cols;
> > -	config->rows = rows;
> > -}
> > -
> > -static unsigned long mctl_calc_size(const struct dram_config *config)
> > -{
> > -	u8 width = config->bus_full_width ? 4 : 2;
> > -
> > -	/* 8 banks */
> > -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> > -}
> > -
> >  static const struct dram_para para = {
> >  	.clk = CONFIG_DRAM_CLK,
> >  #ifdef CONFIG_SUNXI_DRAM_H616_DDR3_1333
> 
> 

Re: [PATCH 5/5] sunxi: h6/h616: Reuse common DRAM infrastructure

[Andre Przywara]

On Wed, 16 Apr 2025 18:30:43 +0200
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

> Dne sreda, 16. april 2025 ob 01:40:04 Srednjeevropski poletni čas je Andre Przywara napisal(a):
> > On Fri, 11 Apr 2025 18:14:39 +0200
> > Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> > 
> > Hi Jernej,
> >   
> > > H616 rank and size detection code is superior to the H6. Nevertheless,
> > > they are structurally the same. Split functions from H616 into new file
> > > and reuse them in H6 DRAM driver too. This should also fix some bugs for
> > > H6 too, like incorrect DRAM size detection.  
> > 
> > that's a nice patch, but actually breaks on my Pine H64:
> > 
> > U-Boot SPL 2025.04-01089-g14694431269f (Apr 15 2025 - 22:55:07 +0100)
> > DRAM:DRAM init failed: Can't detect number of columns!
> > resetting ...
> > 
> > It's none of the previous H6 reworks that causes the problem, but it's
> > actually already in the current code somehow: If I apply the equivalent
> > of [PATCH 1/5] to the H6 code, it already complains. Replacing the
> > panic with a printf() gives me (on mainline):
> > 
> > U-Boot SPL 2025.04-01085-g7a43fe97dc86-dirty (Apr 15 2025 - 23:48:42 +0100)
> > DRAM:detected 14 rows (found=1)
> > detected 11 columns (found=0)
> >  2048 MiB
> > Trying to boot from FEL
> > NOTICE:  BL31: v2.12.0(debug):v2.12.0-724-gf745e004a
> >    < continues booting >
> > 
> > So it seems like the match failed, but apparently 11 columns are correct anyway.
> > And since the same happens after this patch, I assume that even the
> > newer and better matching routine doesn't change that, unfortunately.
> > 
> > The config on this board is: 2 ranks, full width, 11 cols, 14 rows => 2GB.
> > And that seems to work (in Linux).
> > It's a single chip Elpida FA232A2MA JD-F, listed by Micron as
> > EDFA232A2MA-JD-F-D, though I couldn't find a datasheet quickly.  
> 
> Thanks for testing! It looks like it's best just to drop first patch in the
> series. 11 columns is for sure valid. Not that sure about 17 rows, but
> better leave it as is.
> What do you think?

Well, that certainly works, and I will probably take it, as the series
itself is a nice cleanup and is in the right direction. However this
leaves this problem still being around: the mctl_check_pattern()
function fails, even if this is apparently the right configuration. So
we should figure out what's going on, even though with 1/5 dropped and
5/5 changed accordingly it works (TM) the same as before.

Cheers,
Andre

> 
> 
> Best regards,
> Jernej
> 
> > 
> > Cheers,
> > Andre
> >   
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > > ---
> > >  .../include/asm/arch-sunxi/dram_dw_helpers.h  |  22 +++
> > >  arch/arm/mach-sunxi/Makefile                  |   4 +-
> > >  arch/arm/mach-sunxi/dram_dw_helpers.c         | 160 ++++++++++++++++++
> > >  arch/arm/mach-sunxi/dram_sun50i_h6.c          |  91 +---------
> > >  arch/arm/mach-sunxi/dram_sun50i_h616.c        | 155 +----------------
> > >  5 files changed, 190 insertions(+), 242 deletions(-)
> > >  create mode 100644 arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> > >  create mode 100644 arch/arm/mach-sunxi/dram_dw_helpers.c
> > > 
> > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> > > new file mode 100644
> > > index 000000000000..bc9e0d868c55
> > > --- /dev/null
> > > +++ b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> > > @@ -0,0 +1,22 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * Helpers that are commonly used with DW memory controller.
> > > + *
> > > + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> > > + *
> > > + */
> > > +
> > > +#ifndef _DRAM_DW_HELPERS_H
> > > +#define _DRAM_DW_HELPERS_H
> > > +
> > > +#include <asm/arch/dram.h>
> > > +
> > > +bool mctl_core_init(const struct dram_para *para,
> > > +		    const struct dram_config *config);
> > > +void mctl_auto_detect_rank_width(const struct dram_para *para,
> > > +				 struct dram_config *config);
> > > +void mctl_auto_detect_dram_size(const struct dram_para *para,
> > > +				struct dram_config *config);
> > > +unsigned long mctl_calc_size(const struct dram_config *config);
> > > +
> > > +#endif
> > > diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
> > > index eb6a49119a13..a33cd5b0f07a 100644
> > > --- a/arch/arm/mach-sunxi/Makefile
> > > +++ b/arch/arm/mach-sunxi/Makefile
> > > @@ -41,8 +41,8 @@ obj-$(CONFIG_DRAM_SUN9I)	+= dram_sun9i.o
> > >  obj-$(CONFIG_SPL_SPI_SUNXI)	+= spl_spi_sunxi.o
> > >  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_sunxi_dw.o
> > >  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_timings/
> > > -obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o
> > > +obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o dram_dw_helpers.o
> > >  obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_timings/
> > > -obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o
> > > +obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o dram_dw_helpers.o
> > >  obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_timings/
> > >  endif
> > > diff --git a/arch/arm/mach-sunxi/dram_dw_helpers.c b/arch/arm/mach-sunxi/dram_dw_helpers.c
> > > new file mode 100644
> > > index 000000000000..885d1f2c0b12
> > > --- /dev/null
> > > +++ b/arch/arm/mach-sunxi/dram_dw_helpers.c
> > > @@ -0,0 +1,160 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Helpers that are commonly used with DW memory controller.
> > > + *
> > > + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> > > + *
> > > + */
> > > +
> > > +#include <init.h>
> > > +#include <asm/arch/dram_dw_helpers.h>
> > > +
> > > +void mctl_auto_detect_rank_width(const struct dram_para *para,
> > > +				 struct dram_config *config)
> > > +{
> > > +	/* this is minimum size that it's supported */
> > > +	config->cols = 8;
> > > +	config->rows = 13;
> > > +
> > > +	/*
> > > +	 * Strategy here is to test most demanding combination first and least
> > > +	 * demanding last, otherwise HW might not be fully utilized. For
> > > +	 * example, half bus width and rank = 1 combination would also work
> > > +	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> > > +	 * visible.
> > > +	 */
> > > +
> > > +	debug("testing 32-bit width, rank = 2\n");
> > > +	config->bus_full_width = 1;
> > > +	config->ranks = 2;
> > > +	if (mctl_core_init(para, config))
> > > +		return;
> > > +
> > > +	debug("testing 32-bit width, rank = 1\n");
> > > +	config->bus_full_width = 1;
> > > +	config->ranks = 1;
> > > +	if (mctl_core_init(para, config))
> > > +		return;
> > > +
> > > +	debug("testing 16-bit width, rank = 2\n");
> > > +	config->bus_full_width = 0;
> > > +	config->ranks = 2;
> > > +	if (mctl_core_init(para, config))
> > > +		return;
> > > +
> > > +	debug("testing 16-bit width, rank = 1\n");
> > > +	config->bus_full_width = 0;
> > > +	config->ranks = 1;
> > > +	if (mctl_core_init(para, config))
> > > +		return;
> > > +
> > > +	panic("This DRAM setup is currently not supported.\n");
> > > +}
> > > +
> > > +static void mctl_write_pattern(void)
> > > +{
> > > +	unsigned int i;
> > > +	u32 *ptr, val;
> > > +
> > > +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > > +	for (i = 0; i < 16; ptr++, i++) {
> > > +		if (i & 1)
> > > +			val = ~(ulong)ptr;
> > > +		else
> > > +			val = (ulong)ptr;
> > > +		writel(val, ptr);
> > > +	}
> > > +}
> > > +
> > > +static bool mctl_check_pattern(ulong offset)
> > > +{
> > > +	unsigned int i;
> > > +	u32 *ptr, val;
> > > +
> > > +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > > +	for (i = 0; i < 16; ptr++, i++) {
> > > +		if (i & 1)
> > > +			val = ~(ulong)ptr;
> > > +		else
> > > +			val = (ulong)ptr;
> > > +		if (val != *(ptr + offset / 4))
> > > +			return false;
> > > +	}
> > > +
> > > +	return true;
> > > +}
> > > +
> > > +void mctl_auto_detect_dram_size(const struct dram_para *para,
> > > +				struct dram_config *config)
> > > +{
> > > +	unsigned int shift, cols, rows, found;
> > > +	u32 buffer[16];
> > > +
> > > +	/* max. config for columns, but not rows */
> > > +	config->cols = 11;
> > > +	config->rows = 13;
> > > +	mctl_core_init(para, config);
> > > +
> > > +	/*
> > > +	 * Store content so it can be restored later. This is important
> > > +	 * if controller was already initialized and holds any data
> > > +	 * which is important for restoring system.
> > > +	 */
> > > +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > > +
> > > +	mctl_write_pattern();
> > > +
> > > +	shift = config->bus_full_width + 1;
> > > +
> > > +	/* detect column address bits */
> > > +	found = 0;
> > > +	for (cols = 8; cols < 11; cols++) {
> > > +		if (mctl_check_pattern(1ULL << (cols + shift))) {
> > > +			found = 1;
> > > +			break;
> > > +		}
> > > +	}
> > > +	if (!found)
> > > +		panic("DRAM init failed: Can't detect number of columns!");
> > > +	debug("detected %u columns\n", cols);
> > > +
> > > +	/* restore data */
> > > +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > > +
> > > +	/* reconfigure to make sure that all active rows are accessible */
> > > +	config->cols = 8;
> > > +	config->rows = 17;
> > > +	mctl_core_init(para, config);
> > > +
> > > +	/* store data again as it might be moved */
> > > +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > > +
> > > +	mctl_write_pattern();
> > > +
> > > +	/* detect row address bits */
> > > +	shift = config->bus_full_width + 4 + config->cols;
> > > +	found = 0;
> > > +	for (rows = 13; rows < 17; rows++) {
> > > +		if (mctl_check_pattern(1ULL << (rows + shift))) {
> > > +			found = 1;
> > > +			break;
> > > +		}
> > > +	}
> > > +	if (!found)
> > > +		panic("DRAM init failed: Can't detect number of rows!");
> > > +	debug("detected %u rows\n", rows);
> > > +
> > > +	/* restore data again */
> > > +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > > +
> > > +	config->cols = cols;
> > > +	config->rows = rows;
> > > +}
> > > +
> > > +unsigned long mctl_calc_size(const struct dram_config *config)
> > > +{
> > > +	u8 width = config->bus_full_width ? 4 : 2;
> > > +
> > > +	/* 8 banks */
> > > +	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> > > +}
> > > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > index 6a9e53f965eb..fbb865131e08 100644
> > > --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > @@ -10,6 +10,7 @@
> > >  #include <asm/io.h>
> > >  #include <asm/arch/clock.h>
> > >  #include <asm/arch/dram.h>
> > > +#include <asm/arch/dram_dw_helpers.h>
> > >  #include <asm/arch/cpu.h>
> > >  #include <asm/arch/prcm.h>
> > >  #include <linux/bitops.h>
> > > @@ -40,8 +41,8 @@ static void mctl_com_init(const struct dram_para *para,
> > >  static bool mctl_channel_init(const struct dram_para *para,
> > >  			      const struct dram_config *config);
> > >  
> > > -static bool mctl_core_init(const struct dram_para *para,
> > > -			   const struct dram_config *config)
> > > +bool mctl_core_init(const struct dram_para *para,
> > > +		    const struct dram_config *config)
> > >  {
> > >  	mctl_sys_init(para->clk);
> > >  	mctl_com_init(para, config);
> > > @@ -560,92 +561,6 @@ static bool mctl_channel_init(const struct dram_para *para,
> > >  	return true;
> > >  }
> > >  
> > > -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> > > -					struct dram_config *config)
> > > -{
> > > -	/* this is minimum size that it's supported */
> > > -	config->cols = 8;
> > > -	config->rows = 13;
> > > -
> > > -	/*
> > > -	 * Previous versions of this driver tried to auto detect the rank
> > > -	 * and width by looking at controller registers. However this proved
> > > -	 * to be not reliable, so this approach here is the more robust
> > > -	 * solution. Check the git history for details.
> > > -	 *
> > > -	 * Strategy here is to test most demanding combination first and least
> > > -	 * demanding last, otherwise HW might not be fully utilized. For
> > > -	 * example, half bus width and rank = 1 combination would also work
> > > -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> > > -	 * visible.
> > > -	 */
> > > -
> > > -	debug("testing 32-bit width, rank = 2\n");
> > > -	config->bus_full_width = 1;
> > > -	config->ranks = 2;
> > > -	if (mctl_core_init(para, config))
> > > -		return;
> > > -
> > > -	debug("testing 32-bit width, rank = 1\n");
> > > -	config->bus_full_width = 1;
> > > -	config->ranks = 1;
> > > -	if (mctl_core_init(para, config))
> > > -		return;
> > > -
> > > -	debug("testing 16-bit width, rank = 2\n");
> > > -	config->bus_full_width = 0;
> > > -	config->ranks = 2;
> > > -	if (mctl_core_init(para, config))
> > > -		return;
> > > -
> > > -	debug("testing 16-bit width, rank = 1\n");
> > > -	config->bus_full_width = 0;
> > > -	config->ranks = 1;
> > > -	if (mctl_core_init(para, config))
> > > -		return;
> > > -
> > > -	panic("This DRAM setup is currently not supported.\n");
> > > -}
> > > -
> > > -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> > > -				       struct dram_config *config)
> > > -{
> > > -	/* TODO: non-(LP)DDR3 */
> > > -
> > > -	/* detect row address bits */
> > > -	config->cols = 8;
> > > -	config->rows = 18;
> > > -	mctl_core_init(para, config);
> > > -
> > > -	for (config->rows = 13; config->rows < 18; config->rows++) {
> > > -		/* 8 banks, 8 bit per byte and 16/32 bit width */
> > > -		if (mctl_mem_matches((1 << (config->rows + config->cols +
> > > -					    4 + config->bus_full_width))))
> > > -			break;
> > > -	}
> > > -
> > > -	/* detect column address bits */
> > > -	config->cols = 11;
> > > -	mctl_core_init(para, config);
> > > -
> > > -	for (config->cols = 8; config->cols < 11; config->cols++) {
> > > -		/* 8 bits per byte and 16/32 bit width */
> > > -		if (mctl_mem_matches(1 << (config->cols + 1 +
> > > -					   config->bus_full_width)))
> > > -			break;
> > > -	}
> > > -}
> > > -
> > > -unsigned long mctl_calc_size(const struct dram_config *config)
> > > -{
> > > -	u8 width = config->bus_full_width ? 4 : 2;
> > > -
> > > -	/* TODO: non-(LP)DDR3 */
> > > -
> > > -	/* 8 banks */
> > > -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> > > -}
> > > -
> > >  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
> > >  	{{  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
> > >  	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
> > > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > > index d1768a7e7d3a..80d9de557876 100644
> > > --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > > +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > > @@ -17,6 +17,7 @@
> > >  #include <asm/io.h>
> > >  #include <asm/arch/clock.h>
> > >  #include <asm/arch/dram.h>
> > > +#include <asm/arch/dram_dw_helpers.h>
> > >  #include <asm/arch/cpu.h>
> > >  #include <asm/arch/prcm.h>
> > >  #include <linux/bitops.h>
> > > @@ -1310,164 +1311,14 @@ static bool mctl_ctrl_init(const struct dram_para *para,
> > >  	return true;
> > >  }
> > >  
> > > -static bool mctl_core_init(const struct dram_para *para,
> > > -			   const struct dram_config *config)
> > > +bool mctl_core_init(const struct dram_para *para,
> > > +		    const struct dram_config *config)
> > >  {
> > >  	mctl_sys_init(para->clk);
> > >  
> > >  	return mctl_ctrl_init(para, config);
> > >  }
> > >  
> > > -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> > > -					struct dram_config *config)
> > > -{
> > > -	/* this is minimum size that it's supported */
> > > -	config->cols = 8;
> > > -	config->rows = 13;
> > > -
> > > -	/*
> > > -	 * Strategy here is to test most demanding combination first and least
> > > -	 * demanding last, otherwise HW might not be fully utilized. For
> > > -	 * example, half bus width and rank = 1 combination would also work
> > > -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> > > -	 * visible.
> > > -	 */
> > > -
> > > -	debug("testing 32-bit width, rank = 2\n");
> > > -	config->bus_full_width = 1;
> > > -	config->ranks = 2;
> > > -	if (mctl_core_init(para, config))
> > > -		return;
> > > -
> > > -	debug("testing 32-bit width, rank = 1\n");
> > > -	config->bus_full_width = 1;
> > > -	config->ranks = 1;
> > > -	if (mctl_core_init(para, config))
> > > -		return;
> > > -
> > > -	debug("testing 16-bit width, rank = 2\n");
> > > -	config->bus_full_width = 0;
> > > -	config->ranks = 2;
> > > -	if (mctl_core_init(para, config))
> > > -		return;
> > > -
> > > -	debug("testing 16-bit width, rank = 1\n");
> > > -	config->bus_full_width = 0;
> > > -	config->ranks = 1;
> > > -	if (mctl_core_init(para, config))
> > > -		return;
> > > -
> > > -	panic("This DRAM setup is currently not supported.\n");
> > > -}
> > > -
> > > -static void mctl_write_pattern(void)
> > > -{
> > > -	unsigned int i;
> > > -	u32 *ptr, val;
> > > -
> > > -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > > -	for (i = 0; i < 16; ptr++, i++) {
> > > -		if (i & 1)
> > > -			val = ~(ulong)ptr;
> > > -		else
> > > -			val = (ulong)ptr;
> > > -		writel(val, ptr);
> > > -	}
> > > -}
> > > -
> > > -static bool mctl_check_pattern(ulong offset)
> > > -{
> > > -	unsigned int i;
> > > -	u32 *ptr, val;
> > > -
> > > -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > > -	for (i = 0; i < 16; ptr++, i++) {
> > > -		if (i & 1)
> > > -			val = ~(ulong)ptr;
> > > -		else
> > > -			val = (ulong)ptr;
> > > -		if (val != *(ptr + offset / 4))
> > > -			return false;
> > > -	}
> > > -
> > > -	return true;
> > > -}
> > > -
> > > -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> > > -				       struct dram_config *config)
> > > -{
> > > -	unsigned int shift, cols, rows, found;
> > > -	u32 buffer[16];
> > > -
> > > -	/* max. config for columns, but not rows */
> > > -	config->cols = 11;
> > > -	config->rows = 13;
> > > -	mctl_core_init(para, config);
> > > -
> > > -	/*
> > > -	 * Store content so it can be restored later. This is important
> > > -	 * if controller was already initialized and holds any data
> > > -	 * which is important for restoring system.
> > > -	 */
> > > -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > > -
> > > -	mctl_write_pattern();
> > > -
> > > -	shift = config->bus_full_width + 1;
> > > -
> > > -	/* detect column address bits */
> > > -	found = 0;
> > > -	for (cols = 8; cols < 11; cols++) {
> > > -		if (mctl_check_pattern(1ULL << (cols + shift))) {
> > > -			found = 1;
> > > -			break;
> > > -		}
> > > -	}
> > > -	if (!found)
> > > -		panic("DRAM init failed: Can't detect number of columns!");
> > > -	debug("detected %u columns\n", cols);
> > > -
> > > -	/* restore data */
> > > -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > > -
> > > -	/* reconfigure to make sure that all active rows are accessible */
> > > -	config->cols = 8;
> > > -	config->rows = 17;
> > > -	mctl_core_init(para, config);
> > > -
> > > -	/* store data again as it might be moved */
> > > -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > > -
> > > -	mctl_write_pattern();
> > > -
> > > -	/* detect row address bits */
> > > -	shift = config->bus_full_width + 4 + config->cols;
> > > -	found = 0;
> > > -	for (rows = 13; rows < 17; rows++) {
> > > -		if (mctl_check_pattern(1ULL << (rows + shift))) {
> > > -			found = 1;
> > > -			break;
> > > -		}
> > > -	}
> > > -	if (!found)
> > > -		panic("DRAM init failed: Can't detect number of rows!");
> > > -	debug("detected %u rows\n", rows);
> > > -
> > > -	/* restore data again */
> > > -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > > -
> > > -	config->cols = cols;
> > > -	config->rows = rows;
> > > -}
> > > -
> > > -static unsigned long mctl_calc_size(const struct dram_config *config)
> > > -{
> > > -	u8 width = config->bus_full_width ? 4 : 2;
> > > -
> > > -	/* 8 banks */
> > > -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> > > -}
> > > -
> > >  static const struct dram_para para = {
> > >  	.clk = CONFIG_DRAM_CLK,
> > >  #ifdef CONFIG_SUNXI_DRAM_H616_DDR3_1333  
> > 
> >   
> 
> 
> 
> 
> 

Re: [PATCH 5/5] sunxi: h6/h616: Reuse common DRAM infrastructure

[Jernej Škrabec]

Dne četrtek, 17. april 2025 ob 02:18:25 Srednjeevropski poletni čas je Andre Przywara napisal(a):
> On Wed, 16 Apr 2025 18:30:43 +0200
> Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> 
> > Dne sreda, 16. april 2025 ob 01:40:04 Srednjeevropski poletni čas je Andre Przywara napisal(a):
> > > On Fri, 11 Apr 2025 18:14:39 +0200
> > > Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> > > 
> > > Hi Jernej,
> > >   
> > > > H616 rank and size detection code is superior to the H6. Nevertheless,
> > > > they are structurally the same. Split functions from H616 into new file
> > > > and reuse them in H6 DRAM driver too. This should also fix some bugs for
> > > > H6 too, like incorrect DRAM size detection.  
> > > 
> > > that's a nice patch, but actually breaks on my Pine H64:
> > > 
> > > U-Boot SPL 2025.04-01089-g14694431269f (Apr 15 2025 - 22:55:07 +0100)
> > > DRAM:DRAM init failed: Can't detect number of columns!
> > > resetting ...
> > > 
> > > It's none of the previous H6 reworks that causes the problem, but it's
> > > actually already in the current code somehow: If I apply the equivalent
> > > of [PATCH 1/5] to the H6 code, it already complains. Replacing the
> > > panic with a printf() gives me (on mainline):
> > > 
> > > U-Boot SPL 2025.04-01085-g7a43fe97dc86-dirty (Apr 15 2025 - 23:48:42 +0100)
> > > DRAM:detected 14 rows (found=1)
> > > detected 11 columns (found=0)
> > >  2048 MiB
> > > Trying to boot from FEL
> > > NOTICE:  BL31: v2.12.0(debug):v2.12.0-724-gf745e004a
> > >    < continues booting >
> > > 
> > > So it seems like the match failed, but apparently 11 columns are correct anyway.
> > > And since the same happens after this patch, I assume that even the
> > > newer and better matching routine doesn't change that, unfortunately.
> > > 
> > > The config on this board is: 2 ranks, full width, 11 cols, 14 rows => 2GB.
> > > And that seems to work (in Linux).
> > > It's a single chip Elpida FA232A2MA JD-F, listed by Micron as
> > > EDFA232A2MA-JD-F-D, though I couldn't find a datasheet quickly.  
> > 
> > Thanks for testing! It looks like it's best just to drop first patch in the
> > series. 11 columns is for sure valid. Not that sure about 17 rows, but
> > better leave it as is.
> > What do you think?
> 
> Well, that certainly works, and I will probably take it, as the series
> itself is a nice cleanup and is in the right direction. However this
> leaves this problem still being around: the mctl_check_pattern()
> function fails, even if this is apparently the right configuration. So
> we should figure out what's going on, even though with 1/5 dropped and
> 5/5 changed accordingly it works (TM) the same as before.

I just checked JEDEC LPDDR3 standard and 11 columns is max. for full bus
(32 lines) configuration. It's used for 2 GB chips, which it is in this
case, so everything is fine.

Code just scans if there is less than 11 columns. Not matching anything
means there are 11 columns.

I would say everything is in order.

Best regards,
Jernej

> 
> Cheers,
> Andre
> 
> > 
> > 
> > Best regards,
> > Jernej
> > 
> > > 
> > > Cheers,
> > > Andre
> > >   
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > > > ---
> > > >  .../include/asm/arch-sunxi/dram_dw_helpers.h  |  22 +++
> > > >  arch/arm/mach-sunxi/Makefile                  |   4 +-
> > > >  arch/arm/mach-sunxi/dram_dw_helpers.c         | 160 ++++++++++++++++++
> > > >  arch/arm/mach-sunxi/dram_sun50i_h6.c          |  91 +---------
> > > >  arch/arm/mach-sunxi/dram_sun50i_h616.c        | 155 +----------------
> > > >  5 files changed, 190 insertions(+), 242 deletions(-)
> > > >  create mode 100644 arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> > > >  create mode 100644 arch/arm/mach-sunxi/dram_dw_helpers.c
> > > > 
> > > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> > > > new file mode 100644
> > > > index 000000000000..bc9e0d868c55
> > > > --- /dev/null
> > > > +++ b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> > > > @@ -0,0 +1,22 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > +/*
> > > > + * Helpers that are commonly used with DW memory controller.
> > > > + *
> > > > + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> > > > + *
> > > > + */
> > > > +
> > > > +#ifndef _DRAM_DW_HELPERS_H
> > > > +#define _DRAM_DW_HELPERS_H
> > > > +
> > > > +#include <asm/arch/dram.h>
> > > > +
> > > > +bool mctl_core_init(const struct dram_para *para,
> > > > +		    const struct dram_config *config);
> > > > +void mctl_auto_detect_rank_width(const struct dram_para *para,
> > > > +				 struct dram_config *config);
> > > > +void mctl_auto_detect_dram_size(const struct dram_para *para,
> > > > +				struct dram_config *config);
> > > > +unsigned long mctl_calc_size(const struct dram_config *config);
> > > > +
> > > > +#endif
> > > > diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
> > > > index eb6a49119a13..a33cd5b0f07a 100644
> > > > --- a/arch/arm/mach-sunxi/Makefile
> > > > +++ b/arch/arm/mach-sunxi/Makefile
> > > > @@ -41,8 +41,8 @@ obj-$(CONFIG_DRAM_SUN9I)	+= dram_sun9i.o
> > > >  obj-$(CONFIG_SPL_SPI_SUNXI)	+= spl_spi_sunxi.o
> > > >  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_sunxi_dw.o
> > > >  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_timings/
> > > > -obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o
> > > > +obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o dram_dw_helpers.o
> > > >  obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_timings/
> > > > -obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o
> > > > +obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o dram_dw_helpers.o
> > > >  obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_timings/
> > > >  endif
> > > > diff --git a/arch/arm/mach-sunxi/dram_dw_helpers.c b/arch/arm/mach-sunxi/dram_dw_helpers.c
> > > > new file mode 100644
> > > > index 000000000000..885d1f2c0b12
> > > > --- /dev/null
> > > > +++ b/arch/arm/mach-sunxi/dram_dw_helpers.c
> > > > @@ -0,0 +1,160 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Helpers that are commonly used with DW memory controller.
> > > > + *
> > > > + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> > > > + *
> > > > + */
> > > > +
> > > > +#include <init.h>
> > > > +#include <asm/arch/dram_dw_helpers.h>
> > > > +
> > > > +void mctl_auto_detect_rank_width(const struct dram_para *para,
> > > > +				 struct dram_config *config)
> > > > +{
> > > > +	/* this is minimum size that it's supported */
> > > > +	config->cols = 8;
> > > > +	config->rows = 13;
> > > > +
> > > > +	/*
> > > > +	 * Strategy here is to test most demanding combination first and least
> > > > +	 * demanding last, otherwise HW might not be fully utilized. For
> > > > +	 * example, half bus width and rank = 1 combination would also work
> > > > +	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> > > > +	 * visible.
> > > > +	 */
> > > > +
> > > > +	debug("testing 32-bit width, rank = 2\n");
> > > > +	config->bus_full_width = 1;
> > > > +	config->ranks = 2;
> > > > +	if (mctl_core_init(para, config))
> > > > +		return;
> > > > +
> > > > +	debug("testing 32-bit width, rank = 1\n");
> > > > +	config->bus_full_width = 1;
> > > > +	config->ranks = 1;
> > > > +	if (mctl_core_init(para, config))
> > > > +		return;
> > > > +
> > > > +	debug("testing 16-bit width, rank = 2\n");
> > > > +	config->bus_full_width = 0;
> > > > +	config->ranks = 2;
> > > > +	if (mctl_core_init(para, config))
> > > > +		return;
> > > > +
> > > > +	debug("testing 16-bit width, rank = 1\n");
> > > > +	config->bus_full_width = 0;
> > > > +	config->ranks = 1;
> > > > +	if (mctl_core_init(para, config))
> > > > +		return;
> > > > +
> > > > +	panic("This DRAM setup is currently not supported.\n");
> > > > +}
> > > > +
> > > > +static void mctl_write_pattern(void)
> > > > +{
> > > > +	unsigned int i;
> > > > +	u32 *ptr, val;
> > > > +
> > > > +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > > > +	for (i = 0; i < 16; ptr++, i++) {
> > > > +		if (i & 1)
> > > > +			val = ~(ulong)ptr;
> > > > +		else
> > > > +			val = (ulong)ptr;
> > > > +		writel(val, ptr);
> > > > +	}
> > > > +}
> > > > +
> > > > +static bool mctl_check_pattern(ulong offset)
> > > > +{
> > > > +	unsigned int i;
> > > > +	u32 *ptr, val;
> > > > +
> > > > +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > > > +	for (i = 0; i < 16; ptr++, i++) {
> > > > +		if (i & 1)
> > > > +			val = ~(ulong)ptr;
> > > > +		else
> > > > +			val = (ulong)ptr;
> > > > +		if (val != *(ptr + offset / 4))
> > > > +			return false;
> > > > +	}
> > > > +
> > > > +	return true;
> > > > +}
> > > > +
> > > > +void mctl_auto_detect_dram_size(const struct dram_para *para,
> > > > +				struct dram_config *config)
> > > > +{
> > > > +	unsigned int shift, cols, rows, found;
> > > > +	u32 buffer[16];
> > > > +
> > > > +	/* max. config for columns, but not rows */
> > > > +	config->cols = 11;
> > > > +	config->rows = 13;
> > > > +	mctl_core_init(para, config);
> > > > +
> > > > +	/*
> > > > +	 * Store content so it can be restored later. This is important
> > > > +	 * if controller was already initialized and holds any data
> > > > +	 * which is important for restoring system.
> > > > +	 */
> > > > +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > > > +
> > > > +	mctl_write_pattern();
> > > > +
> > > > +	shift = config->bus_full_width + 1;
> > > > +
> > > > +	/* detect column address bits */
> > > > +	found = 0;
> > > > +	for (cols = 8; cols < 11; cols++) {
> > > > +		if (mctl_check_pattern(1ULL << (cols + shift))) {
> > > > +			found = 1;
> > > > +			break;
> > > > +		}
> > > > +	}
> > > > +	if (!found)
> > > > +		panic("DRAM init failed: Can't detect number of columns!");
> > > > +	debug("detected %u columns\n", cols);
> > > > +
> > > > +	/* restore data */
> > > > +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > > > +
> > > > +	/* reconfigure to make sure that all active rows are accessible */
> > > > +	config->cols = 8;
> > > > +	config->rows = 17;
> > > > +	mctl_core_init(para, config);
> > > > +
> > > > +	/* store data again as it might be moved */
> > > > +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > > > +
> > > > +	mctl_write_pattern();
> > > > +
> > > > +	/* detect row address bits */
> > > > +	shift = config->bus_full_width + 4 + config->cols;
> > > > +	found = 0;
> > > > +	for (rows = 13; rows < 17; rows++) {
> > > > +		if (mctl_check_pattern(1ULL << (rows + shift))) {
> > > > +			found = 1;
> > > > +			break;
> > > > +		}
> > > > +	}
> > > > +	if (!found)
> > > > +		panic("DRAM init failed: Can't detect number of rows!");
> > > > +	debug("detected %u rows\n", rows);
> > > > +
> > > > +	/* restore data again */
> > > > +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > > > +
> > > > +	config->cols = cols;
> > > > +	config->rows = rows;
> > > > +}
> > > > +
> > > > +unsigned long mctl_calc_size(const struct dram_config *config)
> > > > +{
> > > > +	u8 width = config->bus_full_width ? 4 : 2;
> > > > +
> > > > +	/* 8 banks */
> > > > +	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> > > > +}
> > > > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > > index 6a9e53f965eb..fbb865131e08 100644
> > > > --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > > +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > > @@ -10,6 +10,7 @@
> > > >  #include <asm/io.h>
> > > >  #include <asm/arch/clock.h>
> > > >  #include <asm/arch/dram.h>
> > > > +#include <asm/arch/dram_dw_helpers.h>
> > > >  #include <asm/arch/cpu.h>
> > > >  #include <asm/arch/prcm.h>
> > > >  #include <linux/bitops.h>
> > > > @@ -40,8 +41,8 @@ static void mctl_com_init(const struct dram_para *para,
> > > >  static bool mctl_channel_init(const struct dram_para *para,
> > > >  			      const struct dram_config *config);
> > > >  
> > > > -static bool mctl_core_init(const struct dram_para *para,
> > > > -			   const struct dram_config *config)
> > > > +bool mctl_core_init(const struct dram_para *para,
> > > > +		    const struct dram_config *config)
> > > >  {
> > > >  	mctl_sys_init(para->clk);
> > > >  	mctl_com_init(para, config);
> > > > @@ -560,92 +561,6 @@ static bool mctl_channel_init(const struct dram_para *para,
> > > >  	return true;
> > > >  }
> > > >  
> > > > -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> > > > -					struct dram_config *config)
> > > > -{
> > > > -	/* this is minimum size that it's supported */
> > > > -	config->cols = 8;
> > > > -	config->rows = 13;
> > > > -
> > > > -	/*
> > > > -	 * Previous versions of this driver tried to auto detect the rank
> > > > -	 * and width by looking at controller registers. However this proved
> > > > -	 * to be not reliable, so this approach here is the more robust
> > > > -	 * solution. Check the git history for details.
> > > > -	 *
> > > > -	 * Strategy here is to test most demanding combination first and least
> > > > -	 * demanding last, otherwise HW might not be fully utilized. For
> > > > -	 * example, half bus width and rank = 1 combination would also work
> > > > -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> > > > -	 * visible.
> > > > -	 */
> > > > -
> > > > -	debug("testing 32-bit width, rank = 2\n");
> > > > -	config->bus_full_width = 1;
> > > > -	config->ranks = 2;
> > > > -	if (mctl_core_init(para, config))
> > > > -		return;
> > > > -
> > > > -	debug("testing 32-bit width, rank = 1\n");
> > > > -	config->bus_full_width = 1;
> > > > -	config->ranks = 1;
> > > > -	if (mctl_core_init(para, config))
> > > > -		return;
> > > > -
> > > > -	debug("testing 16-bit width, rank = 2\n");
> > > > -	config->bus_full_width = 0;
> > > > -	config->ranks = 2;
> > > > -	if (mctl_core_init(para, config))
> > > > -		return;
> > > > -
> > > > -	debug("testing 16-bit width, rank = 1\n");
> > > > -	config->bus_full_width = 0;
> > > > -	config->ranks = 1;
> > > > -	if (mctl_core_init(para, config))
> > > > -		return;
> > > > -
> > > > -	panic("This DRAM setup is currently not supported.\n");
> > > > -}
> > > > -
> > > > -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> > > > -				       struct dram_config *config)
> > > > -{
> > > > -	/* TODO: non-(LP)DDR3 */
> > > > -
> > > > -	/* detect row address bits */
> > > > -	config->cols = 8;
> > > > -	config->rows = 18;
> > > > -	mctl_core_init(para, config);
> > > > -
> > > > -	for (config->rows = 13; config->rows < 18; config->rows++) {
> > > > -		/* 8 banks, 8 bit per byte and 16/32 bit width */
> > > > -		if (mctl_mem_matches((1 << (config->rows + config->cols +
> > > > -					    4 + config->bus_full_width))))
> > > > -			break;
> > > > -	}
> > > > -
> > > > -	/* detect column address bits */
> > > > -	config->cols = 11;
> > > > -	mctl_core_init(para, config);
> > > > -
> > > > -	for (config->cols = 8; config->cols < 11; config->cols++) {
> > > > -		/* 8 bits per byte and 16/32 bit width */
> > > > -		if (mctl_mem_matches(1 << (config->cols + 1 +
> > > > -					   config->bus_full_width)))
> > > > -			break;
> > > > -	}
> > > > -}
> > > > -
> > > > -unsigned long mctl_calc_size(const struct dram_config *config)
> > > > -{
> > > > -	u8 width = config->bus_full_width ? 4 : 2;
> > > > -
> > > > -	/* TODO: non-(LP)DDR3 */
> > > > -
> > > > -	/* 8 banks */
> > > > -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> > > > -}
> > > > -
> > > >  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
> > > >  	{{  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
> > > >  	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
> > > > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > > > index d1768a7e7d3a..80d9de557876 100644
> > > > --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > > > +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > > > @@ -17,6 +17,7 @@
> > > >  #include <asm/io.h>
> > > >  #include <asm/arch/clock.h>
> > > >  #include <asm/arch/dram.h>
> > > > +#include <asm/arch/dram_dw_helpers.h>
> > > >  #include <asm/arch/cpu.h>
> > > >  #include <asm/arch/prcm.h>
> > > >  #include <linux/bitops.h>
> > > > @@ -1310,164 +1311,14 @@ static bool mctl_ctrl_init(const struct dram_para *para,
> > > >  	return true;
> > > >  }
> > > >  
> > > > -static bool mctl_core_init(const struct dram_para *para,
> > > > -			   const struct dram_config *config)
> > > > +bool mctl_core_init(const struct dram_para *para,
> > > > +		    const struct dram_config *config)
> > > >  {
> > > >  	mctl_sys_init(para->clk);
> > > >  
> > > >  	return mctl_ctrl_init(para, config);
> > > >  }
> > > >  
> > > > -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> > > > -					struct dram_config *config)
> > > > -{
> > > > -	/* this is minimum size that it's supported */
> > > > -	config->cols = 8;
> > > > -	config->rows = 13;
> > > > -
> > > > -	/*
> > > > -	 * Strategy here is to test most demanding combination first and least
> > > > -	 * demanding last, otherwise HW might not be fully utilized. For
> > > > -	 * example, half bus width and rank = 1 combination would also work
> > > > -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> > > > -	 * visible.
> > > > -	 */
> > > > -
> > > > -	debug("testing 32-bit width, rank = 2\n");
> > > > -	config->bus_full_width = 1;
> > > > -	config->ranks = 2;
> > > > -	if (mctl_core_init(para, config))
> > > > -		return;
> > > > -
> > > > -	debug("testing 32-bit width, rank = 1\n");
> > > > -	config->bus_full_width = 1;
> > > > -	config->ranks = 1;
> > > > -	if (mctl_core_init(para, config))
> > > > -		return;
> > > > -
> > > > -	debug("testing 16-bit width, rank = 2\n");
> > > > -	config->bus_full_width = 0;
> > > > -	config->ranks = 2;
> > > > -	if (mctl_core_init(para, config))
> > > > -		return;
> > > > -
> > > > -	debug("testing 16-bit width, rank = 1\n");
> > > > -	config->bus_full_width = 0;
> > > > -	config->ranks = 1;
> > > > -	if (mctl_core_init(para, config))
> > > > -		return;
> > > > -
> > > > -	panic("This DRAM setup is currently not supported.\n");
> > > > -}
> > > > -
> > > > -static void mctl_write_pattern(void)
> > > > -{
> > > > -	unsigned int i;
> > > > -	u32 *ptr, val;
> > > > -
> > > > -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > > > -	for (i = 0; i < 16; ptr++, i++) {
> > > > -		if (i & 1)
> > > > -			val = ~(ulong)ptr;
> > > > -		else
> > > > -			val = (ulong)ptr;
> > > > -		writel(val, ptr);
> > > > -	}
> > > > -}
> > > > -
> > > > -static bool mctl_check_pattern(ulong offset)
> > > > -{
> > > > -	unsigned int i;
> > > > -	u32 *ptr, val;
> > > > -
> > > > -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> > > > -	for (i = 0; i < 16; ptr++, i++) {
> > > > -		if (i & 1)
> > > > -			val = ~(ulong)ptr;
> > > > -		else
> > > > -			val = (ulong)ptr;
> > > > -		if (val != *(ptr + offset / 4))
> > > > -			return false;
> > > > -	}
> > > > -
> > > > -	return true;
> > > > -}
> > > > -
> > > > -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> > > > -				       struct dram_config *config)
> > > > -{
> > > > -	unsigned int shift, cols, rows, found;
> > > > -	u32 buffer[16];
> > > > -
> > > > -	/* max. config for columns, but not rows */
> > > > -	config->cols = 11;
> > > > -	config->rows = 13;
> > > > -	mctl_core_init(para, config);
> > > > -
> > > > -	/*
> > > > -	 * Store content so it can be restored later. This is important
> > > > -	 * if controller was already initialized and holds any data
> > > > -	 * which is important for restoring system.
> > > > -	 */
> > > > -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > > > -
> > > > -	mctl_write_pattern();
> > > > -
> > > > -	shift = config->bus_full_width + 1;
> > > > -
> > > > -	/* detect column address bits */
> > > > -	found = 0;
> > > > -	for (cols = 8; cols < 11; cols++) {
> > > > -		if (mctl_check_pattern(1ULL << (cols + shift))) {
> > > > -			found = 1;
> > > > -			break;
> > > > -		}
> > > > -	}
> > > > -	if (!found)
> > > > -		panic("DRAM init failed: Can't detect number of columns!");
> > > > -	debug("detected %u columns\n", cols);
> > > > -
> > > > -	/* restore data */
> > > > -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > > > -
> > > > -	/* reconfigure to make sure that all active rows are accessible */
> > > > -	config->cols = 8;
> > > > -	config->rows = 17;
> > > > -	mctl_core_init(para, config);
> > > > -
> > > > -	/* store data again as it might be moved */
> > > > -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> > > > -
> > > > -	mctl_write_pattern();
> > > > -
> > > > -	/* detect row address bits */
> > > > -	shift = config->bus_full_width + 4 + config->cols;
> > > > -	found = 0;
> > > > -	for (rows = 13; rows < 17; rows++) {
> > > > -		if (mctl_check_pattern(1ULL << (rows + shift))) {
> > > > -			found = 1;
> > > > -			break;
> > > > -		}
> > > > -	}
> > > > -	if (!found)
> > > > -		panic("DRAM init failed: Can't detect number of rows!");
> > > > -	debug("detected %u rows\n", rows);
> > > > -
> > > > -	/* restore data again */
> > > > -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> > > > -
> > > > -	config->cols = cols;
> > > > -	config->rows = rows;
> > > > -}
> > > > -
> > > > -static unsigned long mctl_calc_size(const struct dram_config *config)
> > > > -{
> > > > -	u8 width = config->bus_full_width ? 4 : 2;
> > > > -
> > > > -	/* 8 banks */
> > > > -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> > > > -}
> > > > -
> > > >  static const struct dram_para para = {
> > > >  	.clk = CONFIG_DRAM_CLK,
> > > >  #ifdef CONFIG_SUNXI_DRAM_H616_DDR3_1333  
> > > 
> > >   
> > 
> > 
> > 
> > 
> > 
> 
> 

Re: [PATCH 4/5] sunxi: h6: dram: split dram_para struct

[Andre Przywara]

On Fri, 11 Apr 2025 18:14:38 +0200
Jernej Skrabec <jernej.skrabec@gmail.com> wrote:

> This change is same as in 78aa00c38e86 ("sunxi: H616: dram: split struct
> dram_para"), but for H6. This is needed in order to extract common code
> between H6 and H616 later.

Thanks for doing that, and like that other patch, that results in some
code savings: I count more than 450 bytes.

> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  .../include/asm/arch-sunxi/dram_sun50i_h6.h   |   7 +-
>  arch/arm/mach-sunxi/dram_sun50i_h6.c          | 145 ++++++++++--------
>  2 files changed, 82 insertions(+), 70 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> index f05a1845b32b..af6cd337d7e0 100644
> --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> @@ -315,12 +315,15 @@ check_member(sunxi_mctl_phy_reg, dx[3].reserved_0xf0, 0xaf0);
>  struct dram_para {
>  	u32 clk;
>  	enum sunxi_dram_type type;
> +	const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
> +	const u8 dx_write_delays[NR_OF_BYTE_LANES][WR_LINES_PER_BYTE_LANE];
> +};
> +
> +struct dram_config {
>  	u8 cols;
>  	u8 rows;
>  	u8 ranks;
>  	u8 bus_full_width;
> -	const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
> -	const u8 dx_write_delays[NR_OF_BYTE_LANES][WR_LINES_PER_BYTE_LANE];
>  };
>  
>  static inline int ns_to_t(int nanoseconds)
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> index 24b2cb1579f4..6a9e53f965eb 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> @@ -35,13 +35,16 @@
>   */
>  
>  static void mctl_sys_init(u32 clk_rate);
> -static void mctl_com_init(const struct dram_para *para);
> -static bool mctl_channel_init(const struct dram_para *para);
> +static void mctl_com_init(const struct dram_para *para,
> +			  const struct dram_config *config);
> +static bool mctl_channel_init(const struct dram_para *para,
> +			      const struct dram_config *config);
>  
> -static bool mctl_core_init(struct dram_para *para)
> +static bool mctl_core_init(const struct dram_para *para,
> +			   const struct dram_config *config)
>  {
>  	mctl_sys_init(para->clk);
> -	mctl_com_init(para);
> +	mctl_com_init(para, config);
>  	switch (para->type) {
>  	case SUNXI_DRAM_TYPE_LPDDR3:
>  	case SUNXI_DRAM_TYPE_DDR3:
> @@ -50,7 +53,7 @@ static bool mctl_core_init(struct dram_para *para)
>  	default:
>  		panic("Unsupported DRAM type!");
>  	};
> -	return mctl_channel_init(para);
> +	return mctl_channel_init(para, config);
>  }
>  
>  /* PHY initialisation */
> @@ -196,15 +199,15 @@ static void mctl_sys_init(u32 clk_rate)
>  	writel(0x8000, &mctl_ctl->unk_0x00c);
>  }
>  
> -static void mctl_set_addrmap(const struct dram_para *para)
> +static void mctl_set_addrmap(const struct dram_config *config)
>  {
>  	struct sunxi_mctl_ctl_reg * const mctl_ctl =
>  			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
> -	u8 cols = para->cols;
> -	u8 rows = para->rows;
> -	u8 ranks = para->ranks;
> +	u8 cols = config->cols;
> +	u8 rows = config->rows;
> +	u8 ranks = config->ranks;
>  
> -	if (!para->bus_full_width)
> +	if (!config->bus_full_width)
>  		cols -= 1;
>  
>  	/* Ranks */
> @@ -282,7 +285,8 @@ static void mctl_set_addrmap(const struct dram_para *para)
>  	mctl_ctl->addrmap[8] = 0x3F3F;
>  }
>  
> -static void mctl_com_init(const struct dram_para *para)
> +static void mctl_com_init(const struct dram_para *para,
> +			  const struct dram_config *config)
>  {
>  	struct sunxi_mctl_com_reg * const mctl_com =
>  			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
> @@ -292,7 +296,7 @@ static void mctl_com_init(const struct dram_para *para)
>  			(struct sunxi_mctl_phy_reg *)SUNXI_DRAM_PHY0_BASE;
>  	u32 reg_val, tmp;
>  
> -	mctl_set_addrmap(para);
> +	mctl_set_addrmap(config);
>  
>  	setbits_le32(&mctl_com->cr, BIT(31));
>  
> @@ -311,12 +315,12 @@ static void mctl_com_init(const struct dram_para *para)
>  	clrsetbits_le32(&mctl_com->unk_0x008, 0x3f00, reg_val);
>  
>  	/* TODO: DDR4 */
> -	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(para->ranks);
> +	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(config->ranks);
>  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
>  		reg_val |= MSTR_DEVICETYPE_LPDDR3;
>  	if (para->type == SUNXI_DRAM_TYPE_DDR3)
>  		reg_val |= MSTR_DEVICETYPE_DDR3 | MSTR_2TMODE;
> -	if (para->bus_full_width)
> +	if (config->bus_full_width)
>  		reg_val |= MSTR_BUSWIDTH_FULL;
>  	else
>  		reg_val |= MSTR_BUSWIDTH_HALF;
> @@ -328,7 +332,7 @@ static void mctl_com_init(const struct dram_para *para)
>  		reg_val = DCR_DDR3 | DCR_DDR8BANK | DCR_DDR2T;
>  	writel(reg_val | 0x400, &mctl_phy->dcr);
>  
> -	if (para->ranks == 2)
> +	if (config->ranks == 2)
>  		writel(0x0303, &mctl_ctl->odtmap);
>  	else
>  		writel(0x0201, &mctl_ctl->odtmap);
> @@ -346,7 +350,7 @@ static void mctl_com_init(const struct dram_para *para)
>  	}
>  	writel(reg_val, &mctl_ctl->odtcfg);
>  
> -	if (!para->bus_full_width) {
> +	if (!config->bus_full_width) {
>  		writel(0x0, &mctl_phy->dx[2].gcr[0]);
>  		writel(0x0, &mctl_phy->dx[3].gcr[0]);
>  	}
> @@ -411,7 +415,8 @@ static void mctl_bit_delay_set(const struct dram_para *para)
>  	}
>  }
>  
> -static bool mctl_channel_init(const struct dram_para *para)
> +static bool mctl_channel_init(const struct dram_para *para,
> +			      const struct dram_config *config)
>  {
>  	struct sunxi_mctl_com_reg * const mctl_com =
>  			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
> @@ -446,14 +451,14 @@ static bool mctl_channel_init(const struct dram_para *para)
>  
>  	udelay(100);
>  
> -	if (para->ranks == 2)
> +	if (config->ranks == 2)
>  		setbits_le32(&mctl_phy->dtcr[1], 0x30000);
>  	else
>  		clrsetbits_le32(&mctl_phy->dtcr[1], 0x30000, 0x10000);
>  
>  	if (sunxi_dram_is_lpddr(para->type))
>  		clrbits_le32(&mctl_phy->dtcr[1], BIT(1));
> -	if (para->ranks == 2) {
> +	if (config->ranks == 2) {
>  		writel(0x00010001, &mctl_phy->rankidr);
>  		writel(0x20000, &mctl_phy->odtcr);
>  	} else {
> @@ -555,11 +560,12 @@ static bool mctl_channel_init(const struct dram_para *para)
>  	return true;
>  }
>  
> -static void mctl_auto_detect_rank_width(struct dram_para *para)
> +static void mctl_auto_detect_rank_width(const struct dram_para *para,
> +					struct dram_config *config)
>  {
>  	/* this is minimum size that it's supported */
> -	para->cols = 8;
> -	para->rows = 13;
> +	config->cols = 8;
> +	config->rows = 13;
>  
>  	/*
>  	 * Previous versions of this driver tried to auto detect the rank
> @@ -575,68 +581,69 @@ static void mctl_auto_detect_rank_width(struct dram_para *para)
>  	 */
>  
>  	debug("testing 32-bit width, rank = 2\n");
> -	para->bus_full_width = 1;
> -	para->ranks = 2;
> -	if (mctl_core_init(para))
> +	config->bus_full_width = 1;
> +	config->ranks = 2;
> +	if (mctl_core_init(para, config))
>  		return;
>  
>  	debug("testing 32-bit width, rank = 1\n");
> -	para->bus_full_width = 1;
> -	para->ranks = 1;
> -	if (mctl_core_init(para))
> +	config->bus_full_width = 1;
> +	config->ranks = 1;
> +	if (mctl_core_init(para, config))
>  		return;
>  
>  	debug("testing 16-bit width, rank = 2\n");
> -	para->bus_full_width = 0;
> -	para->ranks = 2;
> -	if (mctl_core_init(para))
> +	config->bus_full_width = 0;
> +	config->ranks = 2;
> +	if (mctl_core_init(para, config))
>  		return;
>  
>  	debug("testing 16-bit width, rank = 1\n");
> -	para->bus_full_width = 0;
> -	para->ranks = 1;
> -	if (mctl_core_init(para))
> +	config->bus_full_width = 0;
> +	config->ranks = 1;
> +	if (mctl_core_init(para, config))
>  		return;
>  
>  	panic("This DRAM setup is currently not supported.\n");
>  }
>  
> -static void mctl_auto_detect_dram_size(struct dram_para *para)
> +static void mctl_auto_detect_dram_size(const struct dram_para *para,
> +				       struct dram_config *config)
>  {
>  	/* TODO: non-(LP)DDR3 */
>  
>  	/* detect row address bits */
> -	para->cols = 8;
> -	para->rows = 18;
> -	mctl_core_init(para);
> +	config->cols = 8;
> +	config->rows = 18;
> +	mctl_core_init(para, config);
>  
> -	for (para->rows = 13; para->rows < 18; para->rows++) {
> +	for (config->rows = 13; config->rows < 18; config->rows++) {
>  		/* 8 banks, 8 bit per byte and 16/32 bit width */
> -		if (mctl_mem_matches((1 << (para->rows + para->cols +
> -					    4 + para->bus_full_width))))
> +		if (mctl_mem_matches((1 << (config->rows + config->cols +
> +					    4 + config->bus_full_width))))
>  			break;
>  	}
>  
>  	/* detect column address bits */
> -	para->cols = 11;
> -	mctl_core_init(para);
> +	config->cols = 11;
> +	mctl_core_init(para, config);
>  
> -	for (para->cols = 8; para->cols < 11; para->cols++) {
> +	for (config->cols = 8; config->cols < 11; config->cols++) {
>  		/* 8 bits per byte and 16/32 bit width */
> -		if (mctl_mem_matches(1 << (para->cols + 1 +
> -					   para->bus_full_width)))
> +		if (mctl_mem_matches(1 << (config->cols + 1 +
> +					   config->bus_full_width)))
>  			break;
>  	}
>  }
>  
> -unsigned long mctl_calc_size(struct dram_para *para)
> +unsigned long mctl_calc_size(const struct dram_config *config)
>  {
> -	u8 width = para->bus_full_width ? 4 : 2;
> +	u8 width = config->bus_full_width ? 4 : 2;
>  
>  	/* TODO: non-(LP)DDR3 */
>  
>  	/* 8 banks */
> -	return (1ULL << (para->cols + para->rows + 3)) * width * para->ranks;
> +	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
>  }
>  
>  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
> @@ -661,36 +668,38 @@ unsigned long mctl_calc_size(struct dram_para *para)
>  	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
>  	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 }}
>  
> -unsigned long sunxi_dram_init(void)
> -{
> -	struct sunxi_mctl_com_reg * const mctl_com =
> -			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
> -	struct sunxi_prcm_reg *const prcm =
> -		(struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
> -	struct dram_para para = {
> -		.clk = CONFIG_DRAM_CLK,
> +static const struct dram_para para = {
> +	.clk = CONFIG_DRAM_CLK,
>  #ifdef CONFIG_SUNXI_DRAM_H6_LPDDR3
> -		.type = SUNXI_DRAM_TYPE_LPDDR3,
> -		.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,
> -		.dx_write_delays = SUN50I_H6_LPDDR3_DX_WRITE_DELAYS,
> +	.type = SUNXI_DRAM_TYPE_LPDDR3,
> +	.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,
> +	.dx_write_delays = SUN50I_H6_LPDDR3_DX_WRITE_DELAYS,
>  #elif defined(CONFIG_SUNXI_DRAM_H6_DDR3_1333)
> -		.type = SUNXI_DRAM_TYPE_DDR3,
> -		.dx_read_delays  = SUN50I_H6_DDR3_DX_READ_DELAYS,
> -		.dx_write_delays = SUN50I_H6_DDR3_DX_WRITE_DELAYS,
> +	.type = SUNXI_DRAM_TYPE_DDR3,
> +	.dx_read_delays  = SUN50I_H6_DDR3_DX_READ_DELAYS,
> +	.dx_write_delays = SUN50I_H6_DDR3_DX_WRITE_DELAYS,
>  #endif
> -	};
> +};
> +
> +unsigned long sunxi_dram_init(void)
> +{
> +	struct sunxi_mctl_com_reg * const mctl_com =
> +			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
> +	struct sunxi_prcm_reg *const prcm =
> +		(struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
> +	struct dram_config config;
>  
>  	unsigned long size;
>  
>  	setbits_le32(&prcm->res_cal_ctrl, BIT(8));
>  	clrbits_le32(&prcm->ohms240, 0x3f);
>  
> -	mctl_auto_detect_rank_width(&para);
> -	mctl_auto_detect_dram_size(&para);
> +	mctl_auto_detect_rank_width(&para, &config);
> +	mctl_auto_detect_dram_size(&para, &config);
>  
> -	mctl_core_init(&para);
> +	mctl_core_init(&para, &config);
>  
> -	size = mctl_calc_size(&para);
> +	size = mctl_calc_size(&config);
>  
>  	clrsetbits_le32(&mctl_com->cr, 0xf0, (size >> (10 + 10 + 4)) & 0xf0);
>  

Re: [PATCH 5/5] sunxi: h6/h616: Reuse common DRAM infrastructure

[Andre Przywara]

On Fri, 11 Apr 2025 18:14:39 +0200
Jernej Skrabec <jernej.skrabec@gmail.com> wrote:

> H616 rank and size detection code is superior to the H6. Nevertheless,
> they are structurally the same. Split functions from H616 into new file
> and reuse them in H6 DRAM driver too. This should also fix some bugs for
> H6 too, like incorrect DRAM size detection.

So as I dropped patch 1/5 for now, I also backed out the respective
changes from this patch.
The H616 part is basically identical before and after, but of course the
H6 sees code changes. I tested this on my two H6 boards, and they still
worked (TM), so I am going to take it.

> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  .../include/asm/arch-sunxi/dram_dw_helpers.h  |  22 +++
>  arch/arm/mach-sunxi/Makefile                  |   4 +-
>  arch/arm/mach-sunxi/dram_dw_helpers.c         | 160 ++++++++++++++++++
>  arch/arm/mach-sunxi/dram_sun50i_h6.c          |  91 +---------
>  arch/arm/mach-sunxi/dram_sun50i_h616.c        | 155 +----------------
>  5 files changed, 190 insertions(+), 242 deletions(-)
>  create mode 100644 arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
>  create mode 100644 arch/arm/mach-sunxi/dram_dw_helpers.c
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> new file mode 100644
> index 000000000000..bc9e0d868c55
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-sunxi/dram_dw_helpers.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Helpers that are commonly used with DW memory controller.
> + *
> + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> + *
> + */
> +
> +#ifndef _DRAM_DW_HELPERS_H
> +#define _DRAM_DW_HELPERS_H
> +
> +#include <asm/arch/dram.h>
> +
> +bool mctl_core_init(const struct dram_para *para,
> +		    const struct dram_config *config);
> +void mctl_auto_detect_rank_width(const struct dram_para *para,
> +				 struct dram_config *config);
> +void mctl_auto_detect_dram_size(const struct dram_para *para,
> +				struct dram_config *config);
> +unsigned long mctl_calc_size(const struct dram_config *config);
> +
> +#endif
> diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
> index eb6a49119a13..a33cd5b0f07a 100644
> --- a/arch/arm/mach-sunxi/Makefile
> +++ b/arch/arm/mach-sunxi/Makefile
> @@ -41,8 +41,8 @@ obj-$(CONFIG_DRAM_SUN9I)	+= dram_sun9i.o
>  obj-$(CONFIG_SPL_SPI_SUNXI)	+= spl_spi_sunxi.o
>  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_sunxi_dw.o
>  obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_timings/
> -obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o
> +obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_sun50i_h6.o dram_dw_helpers.o
>  obj-$(CONFIG_DRAM_SUN50I_H6)	+= dram_timings/
> -obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o
> +obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_sun50i_h616.o dram_dw_helpers.o
>  obj-$(CONFIG_DRAM_SUN50I_H616)	+= dram_timings/
>  endif
> diff --git a/arch/arm/mach-sunxi/dram_dw_helpers.c b/arch/arm/mach-sunxi/dram_dw_helpers.c
> new file mode 100644
> index 000000000000..885d1f2c0b12
> --- /dev/null
> +++ b/arch/arm/mach-sunxi/dram_dw_helpers.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Helpers that are commonly used with DW memory controller.
> + *
> + * (C) Copyright 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> + *
> + */
> +
> +#include <init.h>
> +#include <asm/arch/dram_dw_helpers.h>
> +
> +void mctl_auto_detect_rank_width(const struct dram_para *para,
> +				 struct dram_config *config)
> +{
> +	/* this is minimum size that it's supported */
> +	config->cols = 8;
> +	config->rows = 13;
> +
> +	/*
> +	 * Strategy here is to test most demanding combination first and least
> +	 * demanding last, otherwise HW might not be fully utilized. For
> +	 * example, half bus width and rank = 1 combination would also work
> +	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> +	 * visible.
> +	 */
> +
> +	debug("testing 32-bit width, rank = 2\n");
> +	config->bus_full_width = 1;
> +	config->ranks = 2;
> +	if (mctl_core_init(para, config))
> +		return;
> +
> +	debug("testing 32-bit width, rank = 1\n");
> +	config->bus_full_width = 1;
> +	config->ranks = 1;
> +	if (mctl_core_init(para, config))
> +		return;
> +
> +	debug("testing 16-bit width, rank = 2\n");
> +	config->bus_full_width = 0;
> +	config->ranks = 2;
> +	if (mctl_core_init(para, config))
> +		return;
> +
> +	debug("testing 16-bit width, rank = 1\n");
> +	config->bus_full_width = 0;
> +	config->ranks = 1;
> +	if (mctl_core_init(para, config))
> +		return;
> +
> +	panic("This DRAM setup is currently not supported.\n");
> +}
> +
> +static void mctl_write_pattern(void)
> +{
> +	unsigned int i;
> +	u32 *ptr, val;
> +
> +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> +	for (i = 0; i < 16; ptr++, i++) {
> +		if (i & 1)
> +			val = ~(ulong)ptr;
> +		else
> +			val = (ulong)ptr;
> +		writel(val, ptr);
> +	}
> +}
> +
> +static bool mctl_check_pattern(ulong offset)
> +{
> +	unsigned int i;
> +	u32 *ptr, val;
> +
> +	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> +	for (i = 0; i < 16; ptr++, i++) {
> +		if (i & 1)
> +			val = ~(ulong)ptr;
> +		else
> +			val = (ulong)ptr;
> +		if (val != *(ptr + offset / 4))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +void mctl_auto_detect_dram_size(const struct dram_para *para,
> +				struct dram_config *config)
> +{
> +	unsigned int shift, cols, rows, found;
> +	u32 buffer[16];
> +
> +	/* max. config for columns, but not rows */
> +	config->cols = 11;
> +	config->rows = 13;
> +	mctl_core_init(para, config);
> +
> +	/*
> +	 * Store content so it can be restored later. This is important
> +	 * if controller was already initialized and holds any data
> +	 * which is important for restoring system.
> +	 */
> +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> +
> +	mctl_write_pattern();
> +
> +	shift = config->bus_full_width + 1;
> +
> +	/* detect column address bits */
> +	found = 0;
> +	for (cols = 8; cols < 11; cols++) {
> +		if (mctl_check_pattern(1ULL << (cols + shift))) {
> +			found = 1;
> +			break;
> +		}
> +	}
> +	if (!found)
> +		panic("DRAM init failed: Can't detect number of columns!");
> +	debug("detected %u columns\n", cols);
> +
> +	/* restore data */
> +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> +
> +	/* reconfigure to make sure that all active rows are accessible */
> +	config->cols = 8;
> +	config->rows = 17;
> +	mctl_core_init(para, config);
> +
> +	/* store data again as it might be moved */
> +	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> +
> +	mctl_write_pattern();
> +
> +	/* detect row address bits */
> +	shift = config->bus_full_width + 4 + config->cols;
> +	found = 0;
> +	for (rows = 13; rows < 17; rows++) {
> +		if (mctl_check_pattern(1ULL << (rows + shift))) {
> +			found = 1;
> +			break;
> +		}
> +	}
> +	if (!found)
> +		panic("DRAM init failed: Can't detect number of rows!");
> +	debug("detected %u rows\n", rows);
> +
> +	/* restore data again */
> +	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> +
> +	config->cols = cols;
> +	config->rows = rows;
> +}
> +
> +unsigned long mctl_calc_size(const struct dram_config *config)
> +{
> +	u8 width = config->bus_full_width ? 4 : 2;
> +
> +	/* 8 banks */
> +	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> +}
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> index 6a9e53f965eb..fbb865131e08 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> @@ -10,6 +10,7 @@
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
>  #include <asm/arch/dram.h>
> +#include <asm/arch/dram_dw_helpers.h>
>  #include <asm/arch/cpu.h>
>  #include <asm/arch/prcm.h>
>  #include <linux/bitops.h>
> @@ -40,8 +41,8 @@ static void mctl_com_init(const struct dram_para *para,
>  static bool mctl_channel_init(const struct dram_para *para,
>  			      const struct dram_config *config);
>  
> -static bool mctl_core_init(const struct dram_para *para,
> -			   const struct dram_config *config)
> +bool mctl_core_init(const struct dram_para *para,
> +		    const struct dram_config *config)
>  {
>  	mctl_sys_init(para->clk);
>  	mctl_com_init(para, config);
> @@ -560,92 +561,6 @@ static bool mctl_channel_init(const struct dram_para *para,
>  	return true;
>  }
>  
> -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> -					struct dram_config *config)
> -{
> -	/* this is minimum size that it's supported */
> -	config->cols = 8;
> -	config->rows = 13;
> -
> -	/*
> -	 * Previous versions of this driver tried to auto detect the rank
> -	 * and width by looking at controller registers. However this proved
> -	 * to be not reliable, so this approach here is the more robust
> -	 * solution. Check the git history for details.
> -	 *
> -	 * Strategy here is to test most demanding combination first and least
> -	 * demanding last, otherwise HW might not be fully utilized. For
> -	 * example, half bus width and rank = 1 combination would also work
> -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> -	 * visible.
> -	 */
> -
> -	debug("testing 32-bit width, rank = 2\n");
> -	config->bus_full_width = 1;
> -	config->ranks = 2;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 32-bit width, rank = 1\n");
> -	config->bus_full_width = 1;
> -	config->ranks = 1;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 16-bit width, rank = 2\n");
> -	config->bus_full_width = 0;
> -	config->ranks = 2;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 16-bit width, rank = 1\n");
> -	config->bus_full_width = 0;
> -	config->ranks = 1;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	panic("This DRAM setup is currently not supported.\n");
> -}
> -
> -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> -				       struct dram_config *config)
> -{
> -	/* TODO: non-(LP)DDR3 */
> -
> -	/* detect row address bits */
> -	config->cols = 8;
> -	config->rows = 18;
> -	mctl_core_init(para, config);
> -
> -	for (config->rows = 13; config->rows < 18; config->rows++) {
> -		/* 8 banks, 8 bit per byte and 16/32 bit width */
> -		if (mctl_mem_matches((1 << (config->rows + config->cols +
> -					    4 + config->bus_full_width))))
> -			break;
> -	}
> -
> -	/* detect column address bits */
> -	config->cols = 11;
> -	mctl_core_init(para, config);
> -
> -	for (config->cols = 8; config->cols < 11; config->cols++) {
> -		/* 8 bits per byte and 16/32 bit width */
> -		if (mctl_mem_matches(1 << (config->cols + 1 +
> -					   config->bus_full_width)))
> -			break;
> -	}
> -}
> -
> -unsigned long mctl_calc_size(const struct dram_config *config)
> -{
> -	u8 width = config->bus_full_width ? 4 : 2;
> -
> -	/* TODO: non-(LP)DDR3 */
> -
> -	/* 8 banks */
> -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> -}
> -
>  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
>  	{{  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
>  	 {  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0 },	\
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> index d1768a7e7d3a..80d9de557876 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> @@ -17,6 +17,7 @@
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
>  #include <asm/arch/dram.h>
> +#include <asm/arch/dram_dw_helpers.h>
>  #include <asm/arch/cpu.h>
>  #include <asm/arch/prcm.h>
>  #include <linux/bitops.h>
> @@ -1310,164 +1311,14 @@ static bool mctl_ctrl_init(const struct dram_para *para,
>  	return true;
>  }
>  
> -static bool mctl_core_init(const struct dram_para *para,
> -			   const struct dram_config *config)
> +bool mctl_core_init(const struct dram_para *para,
> +		    const struct dram_config *config)
>  {
>  	mctl_sys_init(para->clk);
>  
>  	return mctl_ctrl_init(para, config);
>  }
>  
> -static void mctl_auto_detect_rank_width(const struct dram_para *para,
> -					struct dram_config *config)
> -{
> -	/* this is minimum size that it's supported */
> -	config->cols = 8;
> -	config->rows = 13;
> -
> -	/*
> -	 * Strategy here is to test most demanding combination first and least
> -	 * demanding last, otherwise HW might not be fully utilized. For
> -	 * example, half bus width and rank = 1 combination would also work
> -	 * on HW with full bus width and rank = 2, but only 1/4 RAM would be
> -	 * visible.
> -	 */
> -
> -	debug("testing 32-bit width, rank = 2\n");
> -	config->bus_full_width = 1;
> -	config->ranks = 2;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 32-bit width, rank = 1\n");
> -	config->bus_full_width = 1;
> -	config->ranks = 1;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 16-bit width, rank = 2\n");
> -	config->bus_full_width = 0;
> -	config->ranks = 2;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	debug("testing 16-bit width, rank = 1\n");
> -	config->bus_full_width = 0;
> -	config->ranks = 1;
> -	if (mctl_core_init(para, config))
> -		return;
> -
> -	panic("This DRAM setup is currently not supported.\n");
> -}
> -
> -static void mctl_write_pattern(void)
> -{
> -	unsigned int i;
> -	u32 *ptr, val;
> -
> -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> -	for (i = 0; i < 16; ptr++, i++) {
> -		if (i & 1)
> -			val = ~(ulong)ptr;
> -		else
> -			val = (ulong)ptr;
> -		writel(val, ptr);
> -	}
> -}
> -
> -static bool mctl_check_pattern(ulong offset)
> -{
> -	unsigned int i;
> -	u32 *ptr, val;
> -
> -	ptr = (u32 *)CFG_SYS_SDRAM_BASE;
> -	for (i = 0; i < 16; ptr++, i++) {
> -		if (i & 1)
> -			val = ~(ulong)ptr;
> -		else
> -			val = (ulong)ptr;
> -		if (val != *(ptr + offset / 4))
> -			return false;
> -	}
> -
> -	return true;
> -}
> -
> -static void mctl_auto_detect_dram_size(const struct dram_para *para,
> -				       struct dram_config *config)
> -{
> -	unsigned int shift, cols, rows, found;
> -	u32 buffer[16];
> -
> -	/* max. config for columns, but not rows */
> -	config->cols = 11;
> -	config->rows = 13;
> -	mctl_core_init(para, config);
> -
> -	/*
> -	 * Store content so it can be restored later. This is important
> -	 * if controller was already initialized and holds any data
> -	 * which is important for restoring system.
> -	 */
> -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> -
> -	mctl_write_pattern();
> -
> -	shift = config->bus_full_width + 1;
> -
> -	/* detect column address bits */
> -	found = 0;
> -	for (cols = 8; cols < 11; cols++) {
> -		if (mctl_check_pattern(1ULL << (cols + shift))) {
> -			found = 1;
> -			break;
> -		}
> -	}
> -	if (!found)
> -		panic("DRAM init failed: Can't detect number of columns!");
> -	debug("detected %u columns\n", cols);
> -
> -	/* restore data */
> -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> -
> -	/* reconfigure to make sure that all active rows are accessible */
> -	config->cols = 8;
> -	config->rows = 17;
> -	mctl_core_init(para, config);
> -
> -	/* store data again as it might be moved */
> -	memcpy(buffer, (u32 *)CFG_SYS_SDRAM_BASE, sizeof(buffer));
> -
> -	mctl_write_pattern();
> -
> -	/* detect row address bits */
> -	shift = config->bus_full_width + 4 + config->cols;
> -	found = 0;
> -	for (rows = 13; rows < 17; rows++) {
> -		if (mctl_check_pattern(1ULL << (rows + shift))) {
> -			found = 1;
> -			break;
> -		}
> -	}
> -	if (!found)
> -		panic("DRAM init failed: Can't detect number of rows!");
> -	debug("detected %u rows\n", rows);
> -
> -	/* restore data again */
> -	memcpy((u32 *)CFG_SYS_SDRAM_BASE, buffer, sizeof(buffer));
> -
> -	config->cols = cols;
> -	config->rows = rows;
> -}
> -
> -static unsigned long mctl_calc_size(const struct dram_config *config)
> -{
> -	u8 width = config->bus_full_width ? 4 : 2;
> -
> -	/* 8 banks */
> -	return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks;
> -}
> -
>  static const struct dram_para para = {
>  	.clk = CONFIG_DRAM_CLK,
>  #ifdef CONFIG_SUNXI_DRAM_H616_DDR3_1333