[PATCH] sunxi: dram: h6: fix the unreliability related to the DDR3 sequence

Andre Przywara andre.przywara at arm.com
Thu Feb 1 19:17:35 CET 2024


On Mon, 22 Jan 2024 22:15:30 +0100
patrick9876 at free.fr wrote:

Hi Patrick,

> From: Patrick Lerda <patrick9876 at free.fr>
> 
> Indeed, the DDR3 has a non-zero probability to not be properly
> initialized. This could be the PLL that is not locked or anything else.
> When this happens and the code tests the correct board configuration,
> the proper board configuration is not set. The board could end with
> half the expected memory size, or u-boot could stall.
> 
> This change adds a loop to execute the DDR3 sequence again until the
> stable state is reached.
> 
> My H6 TX6 board was prone to this issue. Once fixed with this change,
> the same board can now handle 10000+ consecutive reboots properly.

That's certainly an interesting approach, though I feel like it papers
over something. So for instance if the PLL is not locked, we should rather
fix that than going the Windows way (retry, reboot, reinstall) ;-)

But indeed there are some reports about instability of the DRAM init,
reporting twice the size being a common issue.
For a start, can you try to apply this series?
https://patchwork.ozlabs.org/project/uboot/list/?series=378679

Also there is this patch:
https://lore.kernel.org/u-boot/20231001161336.31140-2-viraniac@gmail.com/
And while I am still a bit sceptical about this solution, this looks
better than just retrying to me.

I would be grateful if you could test these patches and report back.

Cheers,
Andre

> Fixes: ec9cdaaa13d ("sunxi: dram: h6: Improve DDR3 config detection")
> Signed-off-by: Patrick Lerda <patrick9876 at free.fr>
> ---
>  arch/arm/mach-sunxi/dram_sun50i_h6.c | 207 ++++++++++++++-------------
>  1 file changed, 111 insertions(+), 96 deletions(-)
> 
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> index 62bc2a0231..462adb1c9e 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> @@ -420,116 +420,131 @@ static bool mctl_channel_init(struct dram_para *para)
>  			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
>  	struct sunxi_mctl_phy_reg * const mctl_phy =
>  			(struct sunxi_mctl_phy_reg *)SUNXI_DRAM_PHY0_BASE;
> -	int i;
> +	int i, j = 0;
>  	u32 val;
>  
> -	setbits_le32(&mctl_ctl->dfiupd[0], BIT(31) | BIT(30));
> -	setbits_le32(&mctl_ctl->zqctl[0], BIT(31) | BIT(30));
> -	writel(0x2f05, &mctl_ctl->sched[0]);
> -	setbits_le32(&mctl_ctl->rfshctl3, BIT(0));
> -	setbits_le32(&mctl_ctl->dfimisc, BIT(0));
> -	setbits_le32(&mctl_ctl->unk_0x00c, BIT(8));
> -	clrsetbits_le32(&mctl_phy->pgcr[1], 0x180, 0xc0);
> -	/* TODO: non-LPDDR3 types */
> -	clrsetbits_le32(&mctl_phy->pgcr[2], GENMASK(17, 0), ns_to_t(7800));
> -	clrbits_le32(&mctl_phy->pgcr[6], BIT(0));
> -	clrsetbits_le32(&mctl_phy->dxccr, 0xee0, 0x220);
> -	/* TODO: VT compensation */
> -	clrsetbits_le32(&mctl_phy->dsgcr, BIT(0), 0x440060);
> -	clrbits_le32(&mctl_phy->vtcr[1], BIT(1));
> -
> -	for (i = 0; i < 4; i++)
> -		clrsetbits_le32(&mctl_phy->dx[i].gcr[0], 0xe00, 0x800);
> -	for (i = 0; i < 4; i++)
> -		clrsetbits_le32(&mctl_phy->dx[i].gcr[2], 0xffff, 0x5555);
> -	for (i = 0; i < 4; i++)
> -		clrsetbits_le32(&mctl_phy->dx[i].gcr[3], 0x3030, 0x1010);
> -
> -	udelay(100);
> +	do {
> +		setbits_le32(&mctl_ctl->dfiupd[0], BIT(31) | BIT(30));
> +		setbits_le32(&mctl_ctl->zqctl[0], BIT(31) | BIT(30));
> +		writel(0x2f05, &mctl_ctl->sched[0]);
> +		setbits_le32(&mctl_ctl->rfshctl3, BIT(0));
> +		setbits_le32(&mctl_ctl->dfimisc, BIT(0));
> +		setbits_le32(&mctl_ctl->unk_0x00c, BIT(8));
> +		clrsetbits_le32(&mctl_phy->pgcr[1], 0x180, 0xc0);
> +		/* TODO: non-LPDDR3 types */
> +		clrsetbits_le32(&mctl_phy->pgcr[2], GENMASK(17, 0),
> +				ns_to_t(7800));
> +		clrbits_le32(&mctl_phy->pgcr[6], BIT(0));
> +		clrsetbits_le32(&mctl_phy->dxccr, 0xee0, 0x220);
> +		/* TODO: VT compensation */
> +		clrsetbits_le32(&mctl_phy->dsgcr, BIT(0), 0x440060);
> +		clrbits_le32(&mctl_phy->vtcr[1], BIT(1));
>  
> -	if (para->ranks == 2)
> -		setbits_le32(&mctl_phy->dtcr[1], 0x30000);
> -	else
> -		clrsetbits_le32(&mctl_phy->dtcr[1], 0x30000, 0x10000);
> +		for (i = 0; i < 4; i++)
> +			clrsetbits_le32(&mctl_phy->dx[i].gcr[0], 0xe00, 0x800);
> +		for (i = 0; i < 4; i++)
> +			clrsetbits_le32(&mctl_phy->dx[i].gcr[2], 0xffff,
> +					0x5555);
> +		for (i = 0; i < 4; i++)
> +			clrsetbits_le32(&mctl_phy->dx[i].gcr[3], 0x3030,
> +					0x1010);
>  
> -	if (sunxi_dram_is_lpddr(para->type))
> -		clrbits_le32(&mctl_phy->dtcr[1], BIT(1));
> -	if (para->ranks == 2) {
> -		writel(0x00010001, &mctl_phy->rankidr);
> -		writel(0x20000, &mctl_phy->odtcr);
> -	} else {
> -		writel(0x0, &mctl_phy->rankidr);
> -		writel(0x10000, &mctl_phy->odtcr);
> -	}
> +		udelay(100);
>  
> -	/* set bits [3:0] to 1? 0 not valid in ZynqMP d/s */
> -	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> -		clrsetbits_le32(&mctl_phy->dtcr[0], 0xF0000000, 0x10000040);
> -	else
> -		clrsetbits_le32(&mctl_phy->dtcr[0], 0xF0000000, 0x10000000);
> -	if (para->clk <= 792) {
> -		if (para->clk <= 672) {
> -			if (para->clk <= 600)
> -				val = 0x300;
> -			else
> -				val = 0x400;
> +		if (para->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) {
> +			writel(0x00010001, &mctl_phy->rankidr);
> +			writel(0x20000, &mctl_phy->odtcr);
>  		} else {
> -			val = 0x500;
> +			writel(0x0, &mctl_phy->rankidr);
> +			writel(0x10000, &mctl_phy->odtcr);
>  		}
> -	} else {
> -		val = 0x600;
> -	}
> -	/* FIXME: NOT REVIEWED YET */
> -	clrsetbits_le32(&mctl_phy->zq[0].zqcr, 0x700, val);
> -	clrsetbits_le32(&mctl_phy->zq[0].zqpr[0], 0xff,
> -			CONFIG_DRAM_ZQ & 0xff);
> -	clrbits_le32(&mctl_phy->zq[0].zqor[0], 0xfffff);
> -	setbits_le32(&mctl_phy->zq[0].zqor[0], (CONFIG_DRAM_ZQ >> 8) & 0xff);
> -	setbits_le32(&mctl_phy->zq[0].zqor[0], (CONFIG_DRAM_ZQ & 0xf00) - 0x100);
> -	setbits_le32(&mctl_phy->zq[0].zqor[0], (CONFIG_DRAM_ZQ & 0xff00) << 4);
> -	clrbits_le32(&mctl_phy->zq[1].zqpr[0], 0xfffff);
> -	setbits_le32(&mctl_phy->zq[1].zqpr[0], (CONFIG_DRAM_ZQ >> 16) & 0xff);
> -	setbits_le32(&mctl_phy->zq[1].zqpr[0], ((CONFIG_DRAM_ZQ >> 8) & 0xf00) - 0x100);
> -	setbits_le32(&mctl_phy->zq[1].zqpr[0], (CONFIG_DRAM_ZQ & 0xff0000) >> 4);
> -	if (para->type == SUNXI_DRAM_TYPE_LPDDR3) {
> -		for (i = 1; i < 14; i++)
> -			writel(0x06060606, &mctl_phy->acbdlr[i]);
> -	}
>  
> -	val = PIR_ZCAL | PIR_DCAL | PIR_PHYRST | PIR_DRAMINIT | PIR_QSGATE |
> -	      PIR_RDDSKW | PIR_WRDSKW | PIR_RDEYE | PIR_WREYE;
> -	if (para->type == SUNXI_DRAM_TYPE_DDR3)
> -		val |= PIR_DRAMRST | PIR_WL;
> -	mctl_phy_pir_init(val);
> +		/* set bits [3:0] to 1? 0 not valid in ZynqMP d/s */
> +		if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> +			clrsetbits_le32(&mctl_phy->dtcr[0], 0xF0000000,
> +					0x10000040);
> +		else
> +			clrsetbits_le32(&mctl_phy->dtcr[0], 0xF0000000,
> +					0x10000000);
> +		if (para->clk <= 792) {
> +			if (para->clk <= 672) {
> +				if (para->clk <= 600)
> +					val = 0x300;
> +				else
> +					val = 0x400;
> +			} else {
> +				val = 0x500;
> +			}
> +		} else {
> +			val = 0x600;
> +		}
> +		/* FIXME: NOT REVIEWED YET */
> +		clrsetbits_le32(&mctl_phy->zq[0].zqcr, 0x700, val);
> +		clrsetbits_le32(&mctl_phy->zq[0].zqpr[0], 0xff,
> +				CONFIG_DRAM_ZQ & 0xff);
> +		clrbits_le32(&mctl_phy->zq[0].zqor[0], 0xfffff);
> +		setbits_le32(&mctl_phy->zq[0].zqor[0],
> +			     (CONFIG_DRAM_ZQ >> 8) & 0xff);
> +		setbits_le32(&mctl_phy->zq[0].zqor[0],
> +			     (CONFIG_DRAM_ZQ & 0xf00) - 0x100);
> +		setbits_le32(&mctl_phy->zq[0].zqor[0], (CONFIG_DRAM_ZQ & 0xff00)
> +							       << 4);
> +		clrbits_le32(&mctl_phy->zq[1].zqpr[0], 0xfffff);
> +		setbits_le32(&mctl_phy->zq[1].zqpr[0],
> +			     (CONFIG_DRAM_ZQ >> 16) & 0xff);
> +		setbits_le32(&mctl_phy->zq[1].zqpr[0],
> +			     ((CONFIG_DRAM_ZQ >> 8) & 0xf00) - 0x100);
> +		setbits_le32(&mctl_phy->zq[1].zqpr[0],
> +			     (CONFIG_DRAM_ZQ & 0xff0000) >> 4);
> +		if (para->type == SUNXI_DRAM_TYPE_LPDDR3) {
> +			for (i = 1; i < 14; i++)
> +				writel(0x06060606, &mctl_phy->acbdlr[i]);
> +		}
>  
> -	/* TODO: DDR4 types ? */
> -	for (i = 0; i < 4; i++)
> -		writel(0x00000909, &mctl_phy->dx[i].gcr[5]);
> +		val = PIR_ZCAL | PIR_DCAL | PIR_PHYRST | PIR_DRAMINIT |
> +		      PIR_QSGATE | PIR_RDDSKW | PIR_WRDSKW | PIR_RDEYE |
> +		      PIR_WREYE;
> +		if (para->type == SUNXI_DRAM_TYPE_DDR3)
> +			val |= PIR_DRAMRST | PIR_WL;
> +		mctl_phy_pir_init(val);
>  
> -	for (i = 0; i < 4; i++) {
> -		if (IS_ENABLED(CONFIG_DRAM_ODT_EN))
> -			val = 0x0;
> -		else
> -			val = 0xaaaa;
> -		clrsetbits_le32(&mctl_phy->dx[i].gcr[2], 0xffff, val);
> +		/* TODO: DDR4 types ? */
> +		for (i = 0; i < 4; i++)
> +			writel(0x00000909, &mctl_phy->dx[i].gcr[5]);
>  
> -		if (IS_ENABLED(CONFIG_DRAM_ODT_EN))
> -			val = 0x0;
> -		else
> -			val = 0x2020;
> -		clrsetbits_le32(&mctl_phy->dx[i].gcr[3], 0x3030, val);
> -	}
> +		for (i = 0; i < 4; i++) {
> +			if (IS_ENABLED(CONFIG_DRAM_ODT_EN))
> +				val = 0x0;
> +			else
> +				val = 0xaaaa;
> +			clrsetbits_le32(&mctl_phy->dx[i].gcr[2], 0xffff, val);
>  
> -	mctl_bit_delay_set(para);
> -	udelay(1);
> +			if (IS_ENABLED(CONFIG_DRAM_ODT_EN))
> +				val = 0x0;
> +			else
> +				val = 0x2020;
> +			clrsetbits_le32(&mctl_phy->dx[i].gcr[3], 0x3030, val);
> +		}
>  
> -	setbits_le32(&mctl_phy->pgcr[6], BIT(0));
> -	clrbits_le32(&mctl_phy->pgcr[6], 0xfff8);
> -	for (i = 0; i < 4; i++)
> -		clrbits_le32(&mctl_phy->dx[i].gcr[3], ~0x3ffff);
> -	udelay(10);
> +		mctl_bit_delay_set(para);
> +		udelay(1);
> +
> +		setbits_le32(&mctl_phy->pgcr[6], BIT(0));
> +		clrbits_le32(&mctl_phy->pgcr[6], 0xfff8);
> +		for (i = 0; i < 4; i++)
> +			clrbits_le32(&mctl_phy->dx[i].gcr[3], ~0x3ffff);
> +		udelay(10);
> +		val = readl(&mctl_phy->pgsr[0]) & 0xff00000;
> +	} while (val && j++ < 64);
>  
> -	if (readl(&mctl_phy->pgsr[0]) & 0xff00000) {
> +	if (val) {
>  		/* Oops! There's something wrong! */
>  		debug("PLL = %x\n", readl(0x3001010));
>  		debug("DRAM PHY PGSR0 = %x\n", readl(&mctl_phy->pgsr[0]));



More information about the U-Boot mailing list