diff mbox series

[7/8] sunxi: Parameterize bit delay code in H616 DRAM driver

Message ID 20221211163213.98540-8-jernej.skrabec@gmail.com
State Superseded
Delegated to: Andre Przywara
Headers show
Series sunxi: Update H616 DRAM driver | expand

Commit Message

Jernej Škrabec Dec. 11, 2022, 4:32 p.m. UTC
These values are highly board specific and thus make sense to add
parameter for them. To ease adding support for new boards, let's make
them same as in vendor DRAM settings.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 .../include/asm/arch-sunxi/dram_sun50i_h616.h |   4 +
 arch/arm/mach-sunxi/Kconfig                   |  18 ++
 arch/arm/mach-sunxi/dram_sun50i_h616.c        | 189 +++++++++++++-----
 3 files changed, 162 insertions(+), 49 deletions(-)

Comments

Andre Przywara Jan. 4, 2023, 12:37 a.m. UTC | #1
On Sun, 11 Dec 2022 17:32:12 +0100
Jernej Skrabec <jernej.skrabec@gmail.com> wrote:

Hi Jernej,

> These values are highly board specific and thus make sense to add
> parameter for them. To ease adding support for new boards, let's make
> them same as in vendor DRAM settings.

So scrolling up and down: does this patch miss the TPR11 and TPR12
values in the OPi-Zero2 defconfig? And should we not default to 0 in
Kconfig to help spotting this omission more easily for new boards?
If I pieced the bits together correctly, we end up with the same values
in the register with TPR11=0xfffedddb and TPR12=0xeddca998, and ODT_EN
being irrelevant.

> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  .../include/asm/arch-sunxi/dram_sun50i_h616.h |   4 +
>  arch/arm/mach-sunxi/Kconfig                   |  18 ++
>  arch/arm/mach-sunxi/dram_sun50i_h616.c        | 189 +++++++++++++-----
>  3 files changed, 162 insertions(+), 49 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
> index b5140c79b70e..c7890c83391f 100644
> --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
> +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
> @@ -145,6 +145,7 @@ check_member(sunxi_mctl_ctl_reg, unk_0x4240, 0x4240);
>  #define TPR10_READ_CALIBRATION	BIT(21)
>  #define TPR10_READ_TRAINING	BIT(22)
>  #define TPR10_WRITE_TRAINING	BIT(23)
> +#define TPR10_UNKNOWN_FEAT3	BIT(30)

As mentioned in the other patch: if we don't know the meaning of this
bit, I'd prefer using BIT(30) directly, or at least encode BIT30
in the name.

>  
>  struct dram_para {
>  	u32 clk;
> @@ -156,7 +157,10 @@ struct dram_para {
>  	u32 dx_odt;
>  	u32 dx_dri;
>  	u32 ca_dri;
> +	u32 odt_en;
>  	u32 tpr10;
> +	u32 tpr11;
> +	u32 tpr12;
>  };
>  
>  
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 778304b77e26..b050f0a56971 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -67,11 +67,29 @@ config DRAM_SUN50I_H616_CA_DRI
>  	help
>  	  CA DRI value from vendor DRAM settings.
>  
> +config DRAM_SUN50I_H616_ODT_EN
> +	hex "H616 DRAM ODT EN parameter"
> +	default 0x1
> +	help
> +	  ODT EN value from vendor DRAM settings.
> +
>  config DRAM_SUN50I_H616_TPR10
>  	hex "H616 DRAM TPR10 parameter"
>  	help
>  	  TPR10 value from vendor DRAM settings. It tells which features
>  	  should be configured, like write leveling, read calibration, etc.
> +
> +config DRAM_SUN50I_H616_TPR11
> +	hex "H616 DRAM TPR11 parameter"
> +	default 0x0
> +	help
> +	  TPR11 value from vendor DRAM settings.
> +
> +config DRAM_SUN50I_H616_TPR12
> +	hex "H616 DRAM TPR12 parameter"
> +	default 0x0
> +	help
> +	  TPR12 value from vendor DRAM settings.
>  endif
>  
>  config SUN6I_PRCM
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> index 3b2ba168498c..df06cea42464 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> @@ -574,7 +574,7 @@ static bool mctl_phy_write_training(struct dram_para *para)
>  
>  static void mctl_phy_bit_delay_compensation(struct dram_para *para)
>  {
> -	u32 *ptr;
> +	u32 *ptr, val;
>  	int i;
>  
>  	if (para->tpr10 & TPR10_UNKNOWN_FEAT2) {
> @@ -582,49 +582,93 @@ static void mctl_phy_bit_delay_compensation(struct dram_para *para)
>  		setbits_le32(SUNXI_DRAM_PHY0_BASE + 8, 8);
>  		clrbits_le32(SUNXI_DRAM_PHY0_BASE + 0x190, 0x10);
>  
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = para->tpr11 & 0x3f;
> +		else
> +			val = (para->tpr11 & 0xf) << 1;
> +
>  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x484);
>  		for (i = 0; i < 9; i++) {
> -			writel_relaxed(0x16, ptr);
> -			writel_relaxed(0x16, ptr + 0x30);
> +			writel_relaxed(val, ptr);
> +			writel_relaxed(val, ptr + 0x30);
>  			ptr += 2;
>  		}
> -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x4d0);
> -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x590);
> -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x4cc);
> -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x58c);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->odt_en >> 15) & 0x1e;

So I guess odt_en stands for "ODT enable". Looking at the D1 DRAM
driver, they have a boot0 parameter odt_en which is either 0x0 or 0x1,
so it looks like a boolean value. This seems to be also the case here?
In the D1 driver, this seems to gate the ZQ value being used, which
provides the actual timing values.
So is TPR10_UNKNOWN_FEAT3 actually this ODT_EN switch, and the variable
containing the timing bits should be zq_value or CONFIG_DRAM_ZQ?

> +		else
> +			val = (para->tpr11 >> 15) & 0x1e;

So I am wondering if it's less confusing to mask first, then shift?
Granted, this here is shift from [19:16] into [5:1], so it's not easy
either way, but I think it's easier to match the mask with the provided
TPR11 value.

> +
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x4d0);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x590);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x4cc);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x58c);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->tpr11 >> 8) & 0x3f;
> +		else
> +			val = (para->tpr11 >> 3) & 0x1e;
>  
>  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x4d8);
>  		for (i = 0; i < 9; i++) {
> -			writel_relaxed(0x1a, ptr);
> -			writel_relaxed(0x1a, ptr + 0x30);
> +			writel_relaxed(val, ptr);
> +			writel_relaxed(val, ptr + 0x30);
>  			ptr += 2;
>  		}
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x524);
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x5e4);
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x520);
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x5e0);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->odt_en >> 19) & 0x1e;

Is this really odt_en as a base, while *most* of the other bits use
TPR11 as well? Just asking, since it doesn't fit into my naive pattern
matching ;-)

The rest looks correct (minus the repeat of the above mentioned
issues), I compared the register offsets between - and +.

Cheers,
Andre

> +		else
> +			val = (para->tpr11 >> 19) & 0x1e;
> +
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x524);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5e4);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x520);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5e0);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->tpr11 >> 16) & 0x3f;
> +		else
> +			val = (para->tpr11 >> 7) & 0x1e;
>  
>  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x604);
>  		for (i = 0; i < 9; i++) {
> -			writel_relaxed(0x1a, ptr);
> -			writel_relaxed(0x1a, ptr + 0x30);
> +			writel_relaxed(val, ptr);
> +			writel_relaxed(val, ptr + 0x30);
>  			ptr += 2;
>  		}
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x650);
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x710);
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x64c);
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x70c);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->odt_en >> 23) & 0x1e;
> +		else
> +			val = (para->tpr11 >> 23) & 0x1e;
> +
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x650);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x710);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x64c);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x70c);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->tpr11 >> 24) & 0x3f;
> +		else
> +			val = (para->tpr11 >> 11) & 0x1e;
>  
>  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x658);
>  		for (i = 0; i < 9; i++) {
> -			writel_relaxed(0x1a, ptr);
> -			writel_relaxed(0x1a, ptr + 0x30);
> +			writel_relaxed(val, ptr);
> +			writel_relaxed(val, ptr + 0x30);
>  			ptr += 2;
>  		}
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x6a4);
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x764);
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x6a0);
> -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x760);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->odt_en >> 27) & 0x1e;
> +		else
> +			val = (para->tpr11 >> 27) & 0x1e;
> +
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6a4);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x764);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6a0);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x760);
>  
>  		dmb();
>  
> @@ -635,49 +679,93 @@ static void mctl_phy_bit_delay_compensation(struct dram_para *para)
>  		clrbits_le32(SUNXI_DRAM_PHY0_BASE + 0x54, 0x80);
>  		clrbits_le32(SUNXI_DRAM_PHY0_BASE + 0x190, 4);
>  
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = para->tpr12 & 0x3f;
> +		else
> +			val = (para->tpr12 & 0xf) << 1;
> +
>  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x480);
>  		for (i = 0; i < 9; i++) {
> -			writel_relaxed(0x10, ptr);
> -			writel_relaxed(0x10, ptr + 0x30);
> +			writel_relaxed(val, ptr);
> +			writel_relaxed(val, ptr + 0x30);
>  			ptr += 2;
>  		}
> -		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x528);
> -		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x5e8);
> -		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x4c8);
> -		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x588);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->odt_en << 1) & 0x1e;
> +		else
> +			val = (para->tpr12 >> 15) & 0x1e;
> +
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x528);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5e8);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x4c8);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x588);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->tpr12 >> 8) & 0x3f;
> +		else
> +			val = (para->tpr12 >> 3) & 0x1e;
>  
>  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x4d4);
>  		for (i = 0; i < 9; i++) {
> -			writel_relaxed(0x12, ptr);
> -			writel_relaxed(0x12, ptr + 0x30);
> +			writel_relaxed(val, ptr);
> +			writel_relaxed(val, ptr + 0x30);
>  			ptr += 2;
>  		}
> -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x52c);
> -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x5ec);
> -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x51c);
> -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x5dc);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->odt_en >> 3) & 0x1e;
> +		else
> +			val = (para->tpr12 >> 19) & 0x1e;
> +
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x52c);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5ec);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x51c);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5dc);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->tpr12 >> 16) & 0x3f;
> +		else
> +			val = (para->tpr12 >> 7) & 0x1e;
>  
>  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x600);
>  		for (i = 0; i < 9; i++) {
> -			writel_relaxed(0x12, ptr);
> -			writel_relaxed(0x12, ptr + 0x30);
> +			writel_relaxed(val, ptr);
> +			writel_relaxed(val, ptr + 0x30);
>  			ptr += 2;
>  		}
> -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x6a8);
> -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x768);
> -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x648);
> -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x708);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->odt_en >> 7) & 0x1e;
> +		else
> +			val = (para->tpr12 >> 23) & 0x1e;
> +
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6a8);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x768);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x648);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x708);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->tpr12 >> 24) & 0x3f;
> +		else
> +			val = (para->tpr12 >> 11) & 0x1e;
>  
>  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x654);
>  		for (i = 0; i < 9; i++) {
> -			writel_relaxed(0x14, ptr);
> -			writel_relaxed(0x14, ptr + 0x30);
> +			writel_relaxed(val, ptr);
> +			writel_relaxed(val, ptr + 0x30);
>  			ptr += 2;
>  		}
> -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x6ac);
> -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x76c);
> -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x69c);
> -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x75c);
> +
> +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> +			val = (para->odt_en >> 11) & 0x1e;
> +		else
> +			val = (para->tpr12 >> 27) & 0x1e;
> +
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6ac);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x76c);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x69c);
> +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x75c);
>  
>  		dmb();
>  
> @@ -1021,7 +1109,10 @@ unsigned long sunxi_dram_init(void)
>  		.dx_odt = CONFIG_DRAM_SUN50I_H616_DX_ODT,
>  		.dx_dri = CONFIG_DRAM_SUN50I_H616_DX_DRI,
>  		.ca_dri = CONFIG_DRAM_SUN50I_H616_CA_DRI,
> +		.odt_en = CONFIG_DRAM_SUN50I_H616_ODT_EN,
>  		.tpr10 = CONFIG_DRAM_SUN50I_H616_TPR10,
> +		.tpr11 = CONFIG_DRAM_SUN50I_H616_TPR11,
> +		.tpr12 = CONFIG_DRAM_SUN50I_H616_TPR12,
>  	};
>  	unsigned long size;
>
Jernej Škrabec Jan. 4, 2023, 9:28 p.m. UTC | #2
Dne sreda, 04. januar 2023 ob 01:37:47 CET je Andre Przywara napisal(a):
> On Sun, 11 Dec 2022 17:32:12 +0100
> Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> 
> Hi Jernej,
> 
> > These values are highly board specific and thus make sense to add
> > parameter for them. To ease adding support for new boards, let's make
> > them same as in vendor DRAM settings.
> 
> So scrolling up and down: does this patch miss the TPR11 and TPR12
> values in the OPi-Zero2 defconfig? 

No, because 0 (which is default) is correct here.

> And should we not default to 0 in
> Kconfig to help spotting this omission more easily for new boards?

Not all boards need to set all the values. I set default values for symbols 
which seem to have same value for multiple boards.

> If I pieced the bits together correctly, we end up with the same values
> in the register with TPR11=0xfffedddb and TPR12=0xeddca998, and ODT_EN
> being irrelevant.
> 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > ---
> > 
> >  .../include/asm/arch-sunxi/dram_sun50i_h616.h |   4 +
> >  arch/arm/mach-sunxi/Kconfig                   |  18 ++
> >  arch/arm/mach-sunxi/dram_sun50i_h616.c        | 189 +++++++++++++-----
> >  3 files changed, 162 insertions(+), 49 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
> > b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h index
> > b5140c79b70e..c7890c83391f 100644
> > --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
> > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
> > @@ -145,6 +145,7 @@ check_member(sunxi_mctl_ctl_reg, unk_0x4240, 0x4240);
> > 
> >  #define TPR10_READ_CALIBRATION	BIT(21)
> >  #define TPR10_READ_TRAINING	BIT(22)
> >  #define TPR10_WRITE_TRAINING	BIT(23)
> > 
> > +#define TPR10_UNKNOWN_FEAT3	BIT(30)
> 
> As mentioned in the other patch: if we don't know the meaning of this
> bit, I'd prefer using BIT(30) directly, or at least encode BIT30
> in the name.
> 
> >  struct dram_para {
> >  
> >  	u32 clk;
> > 
> > @@ -156,7 +157,10 @@ struct dram_para {
> > 
> >  	u32 dx_odt;
> >  	u32 dx_dri;
> >  	u32 ca_dri;
> > 
> > +	u32 odt_en;
> > 
> >  	u32 tpr10;
> > 
> > +	u32 tpr11;
> > +	u32 tpr12;
> > 
> >  };
> > 
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index 778304b77e26..b050f0a56971 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -67,11 +67,29 @@ config DRAM_SUN50I_H616_CA_DRI
> > 
> >  	help
> >  	
> >  	  CA DRI value from vendor DRAM settings.
> > 
> > +config DRAM_SUN50I_H616_ODT_EN
> > +	hex "H616 DRAM ODT EN parameter"
> > +	default 0x1
> > +	help
> > +	  ODT EN value from vendor DRAM settings.
> > +
> > 
> >  config DRAM_SUN50I_H616_TPR10
> >  
> >  	hex "H616 DRAM TPR10 parameter"
> >  	help
> >  	
> >  	  TPR10 value from vendor DRAM settings. It tells which features
> >  	  should be configured, like write leveling, read calibration, 
etc.
> > 
> > +
> > +config DRAM_SUN50I_H616_TPR11
> > +	hex "H616 DRAM TPR11 parameter"
> > +	default 0x0
> > +	help
> > +	  TPR11 value from vendor DRAM settings.
> > +
> > +config DRAM_SUN50I_H616_TPR12
> > +	hex "H616 DRAM TPR12 parameter"
> > +	default 0x0
> > +	help
> > +	  TPR12 value from vendor DRAM settings.
> > 
> >  endif
> >  
> >  config SUN6I_PRCM
> > 
> > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > b/arch/arm/mach-sunxi/dram_sun50i_h616.c index 3b2ba168498c..df06cea42464
> > 100644
> > --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > @@ -574,7 +574,7 @@ static bool mctl_phy_write_training(struct dram_para
> > *para)> 
> >  static void mctl_phy_bit_delay_compensation(struct dram_para *para)
> >  {
> > 
> > -	u32 *ptr;
> > +	u32 *ptr, val;
> > 
> >  	int i;
> >  	
> >  	if (para->tpr10 & TPR10_UNKNOWN_FEAT2) {
> > 
> > @@ -582,49 +582,93 @@ static void mctl_phy_bit_delay_compensation(struct
> > dram_para *para)> 
> >  		setbits_le32(SUNXI_DRAM_PHY0_BASE + 8, 8);
> >  		clrbits_le32(SUNXI_DRAM_PHY0_BASE + 0x190, 0x10);
> > 
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = para->tpr11 & 0x3f;
> > +		else
> > +			val = (para->tpr11 & 0xf) << 1;
> > +
> > 
> >  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x484);
> >  		for (i = 0; i < 9; i++) {
> > 
> > -			writel_relaxed(0x16, ptr);
> > -			writel_relaxed(0x16, ptr + 0x30);
> > +			writel_relaxed(val, ptr);
> > +			writel_relaxed(val, ptr + 0x30);
> > 
> >  			ptr += 2;
> >  		
> >  		}
> > 
> > -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x4d0);
> > -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x590);
> > -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x4cc);
> > -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x58c);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->odt_en >> 15) & 0x1e;
> 
> So I guess odt_en stands for "ODT enable". Looking at the D1 DRAM
> driver, they have a boot0 parameter odt_en which is either 0x0 or 0x1,
> so it looks like a boolean value. This seems to be also the case here?

Nope. While I only ever see odt_en being 1, based on vendor code it can have 
additional bits set, as it can be seen from above code.

> In the D1 driver, this seems to gate the ZQ value being used, which
> provides the actual timing values.
> So is TPR10_UNKNOWN_FEAT3 actually this ODT_EN switch, and the variable
> containing the timing bits should be zq_value or CONFIG_DRAM_ZQ?

No idea.

> 
> > +		else
> > +			val = (para->tpr11 >> 15) & 0x1e;
> 
> So I am wondering if it's less confusing to mask first, then shift?
> Granted, this here is shift from [19:16] into [5:1], so it's not easy
> either way, but I think it's easier to match the mask with the provided
> TPR11 value.

I did it this way because mask is much smaller and easier to read, but neither 
are straightforward to understand. I guess some macro could be crafted for 
such purpose.

> 
> > +
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x4d0);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x590);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x4cc);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x58c);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->tpr11 >> 8) & 0x3f;
> > +		else
> > +			val = (para->tpr11 >> 3) & 0x1e;
> > 
> >  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x4d8);
> >  		for (i = 0; i < 9; i++) {
> > 
> > -			writel_relaxed(0x1a, ptr);
> > -			writel_relaxed(0x1a, ptr + 0x30);
> > +			writel_relaxed(val, ptr);
> > +			writel_relaxed(val, ptr + 0x30);
> > 
> >  			ptr += 2;
> >  		
> >  		}
> > 
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x524);
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x5e4);
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x520);
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x5e0);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->odt_en >> 19) & 0x1e;
> 
> Is this really odt_en as a base, while *most* of the other bits use
> TPR11 as well? Just asking, since it doesn't fit into my naive pattern
> matching ;-)

odt_en is name from device tree. It doesn't seem to match actual purpose, but 
there is not much we can do without documentation.

Best regards,
Jernej

> 
> The rest looks correct (minus the repeat of the above mentioned
> issues), I compared the register offsets between - and +.
> 
> Cheers,
> Andre
> 
> > +		else
> > +			val = (para->tpr11 >> 19) & 0x1e;
> > +
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x524);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5e4);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x520);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5e0);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->tpr11 >> 16) & 0x3f;
> > +		else
> > +			val = (para->tpr11 >> 7) & 0x1e;
> > 
> >  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x604);
> >  		for (i = 0; i < 9; i++) {
> > 
> > -			writel_relaxed(0x1a, ptr);
> > -			writel_relaxed(0x1a, ptr + 0x30);
> > +			writel_relaxed(val, ptr);
> > +			writel_relaxed(val, ptr + 0x30);
> > 
> >  			ptr += 2;
> >  		
> >  		}
> > 
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x650);
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x710);
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x64c);
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x70c);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->odt_en >> 23) & 0x1e;
> > +		else
> > +			val = (para->tpr11 >> 23) & 0x1e;
> > +
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x650);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x710);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x64c);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x70c);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->tpr11 >> 24) & 0x3f;
> > +		else
> > +			val = (para->tpr11 >> 11) & 0x1e;
> > 
> >  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x658);
> >  		for (i = 0; i < 9; i++) {
> > 
> > -			writel_relaxed(0x1a, ptr);
> > -			writel_relaxed(0x1a, ptr + 0x30);
> > +			writel_relaxed(val, ptr);
> > +			writel_relaxed(val, ptr + 0x30);
> > 
> >  			ptr += 2;
> >  		
> >  		}
> > 
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x6a4);
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x764);
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x6a0);
> > -		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x760);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->odt_en >> 27) & 0x1e;
> > +		else
> > +			val = (para->tpr11 >> 27) & 0x1e;
> > +
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6a4);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x764);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6a0);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x760);
> > 
> >  		dmb();
> > 
> > @@ -635,49 +679,93 @@ static void mctl_phy_bit_delay_compensation(struct
> > dram_para *para)> 
> >  		clrbits_le32(SUNXI_DRAM_PHY0_BASE + 0x54, 0x80);
> >  		clrbits_le32(SUNXI_DRAM_PHY0_BASE + 0x190, 4);
> > 
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = para->tpr12 & 0x3f;
> > +		else
> > +			val = (para->tpr12 & 0xf) << 1;
> > +
> > 
> >  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x480);
> >  		for (i = 0; i < 9; i++) {
> > 
> > -			writel_relaxed(0x10, ptr);
> > -			writel_relaxed(0x10, ptr + 0x30);
> > +			writel_relaxed(val, ptr);
> > +			writel_relaxed(val, ptr + 0x30);
> > 
> >  			ptr += 2;
> >  		
> >  		}
> > 
> > -		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x528);
> > -		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x5e8);
> > -		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x4c8);
> > -		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x588);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->odt_en << 1) & 0x1e;
> > +		else
> > +			val = (para->tpr12 >> 15) & 0x1e;
> > +
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x528);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5e8);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x4c8);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x588);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->tpr12 >> 8) & 0x3f;
> > +		else
> > +			val = (para->tpr12 >> 3) & 0x1e;
> > 
> >  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x4d4);
> >  		for (i = 0; i < 9; i++) {
> > 
> > -			writel_relaxed(0x12, ptr);
> > -			writel_relaxed(0x12, ptr + 0x30);
> > +			writel_relaxed(val, ptr);
> > +			writel_relaxed(val, ptr + 0x30);
> > 
> >  			ptr += 2;
> >  		
> >  		}
> > 
> > -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x52c);
> > -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x5ec);
> > -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x51c);
> > -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x5dc);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->odt_en >> 3) & 0x1e;
> > +		else
> > +			val = (para->tpr12 >> 19) & 0x1e;
> > +
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x52c);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5ec);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x51c);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5dc);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->tpr12 >> 16) & 0x3f;
> > +		else
> > +			val = (para->tpr12 >> 7) & 0x1e;
> > 
> >  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x600);
> >  		for (i = 0; i < 9; i++) {
> > 
> > -			writel_relaxed(0x12, ptr);
> > -			writel_relaxed(0x12, ptr + 0x30);
> > +			writel_relaxed(val, ptr);
> > +			writel_relaxed(val, ptr + 0x30);
> > 
> >  			ptr += 2;
> >  		
> >  		}
> > 
> > -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x6a8);
> > -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x768);
> > -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x648);
> > -		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x708);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->odt_en >> 7) & 0x1e;
> > +		else
> > +			val = (para->tpr12 >> 23) & 0x1e;
> > +
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6a8);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x768);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x648);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x708);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->tpr12 >> 24) & 0x3f;
> > +		else
> > +			val = (para->tpr12 >> 11) & 0x1e;
> > 
> >  		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x654);
> >  		for (i = 0; i < 9; i++) {
> > 
> > -			writel_relaxed(0x14, ptr);
> > -			writel_relaxed(0x14, ptr + 0x30);
> > +			writel_relaxed(val, ptr);
> > +			writel_relaxed(val, ptr + 0x30);
> > 
> >  			ptr += 2;
> >  		
> >  		}
> > 
> > -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x6ac);
> > -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x76c);
> > -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x69c);
> > -		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x75c);
> > +
> > +		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
> > +			val = (para->odt_en >> 11) & 0x1e;
> > +		else
> > +			val = (para->tpr12 >> 27) & 0x1e;
> > +
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6ac);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x76c);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x69c);
> > +		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x75c);
> > 
> >  		dmb();
> > 
> > @@ -1021,7 +1109,10 @@ unsigned long sunxi_dram_init(void)
> > 
> >  		.dx_odt = CONFIG_DRAM_SUN50I_H616_DX_ODT,
> >  		.dx_dri = CONFIG_DRAM_SUN50I_H616_DX_DRI,
> >  		.ca_dri = CONFIG_DRAM_SUN50I_H616_CA_DRI,
> > 
> > +		.odt_en = CONFIG_DRAM_SUN50I_H616_ODT_EN,
> > 
> >  		.tpr10 = CONFIG_DRAM_SUN50I_H616_TPR10,
> > 
> > +		.tpr11 = CONFIG_DRAM_SUN50I_H616_TPR11,
> > +		.tpr12 = CONFIG_DRAM_SUN50I_H616_TPR12,
> > 
> >  	};
> >  	unsigned long size;
diff mbox series

Patch

diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
index b5140c79b70e..c7890c83391f 100644
--- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
+++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
@@ -145,6 +145,7 @@  check_member(sunxi_mctl_ctl_reg, unk_0x4240, 0x4240);
 #define TPR10_READ_CALIBRATION	BIT(21)
 #define TPR10_READ_TRAINING	BIT(22)
 #define TPR10_WRITE_TRAINING	BIT(23)
+#define TPR10_UNKNOWN_FEAT3	BIT(30)
 
 struct dram_para {
 	u32 clk;
@@ -156,7 +157,10 @@  struct dram_para {
 	u32 dx_odt;
 	u32 dx_dri;
 	u32 ca_dri;
+	u32 odt_en;
 	u32 tpr10;
+	u32 tpr11;
+	u32 tpr12;
 };
 
 
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 778304b77e26..b050f0a56971 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -67,11 +67,29 @@  config DRAM_SUN50I_H616_CA_DRI
 	help
 	  CA DRI value from vendor DRAM settings.
 
+config DRAM_SUN50I_H616_ODT_EN
+	hex "H616 DRAM ODT EN parameter"
+	default 0x1
+	help
+	  ODT EN value from vendor DRAM settings.
+
 config DRAM_SUN50I_H616_TPR10
 	hex "H616 DRAM TPR10 parameter"
 	help
 	  TPR10 value from vendor DRAM settings. It tells which features
 	  should be configured, like write leveling, read calibration, etc.
+
+config DRAM_SUN50I_H616_TPR11
+	hex "H616 DRAM TPR11 parameter"
+	default 0x0
+	help
+	  TPR11 value from vendor DRAM settings.
+
+config DRAM_SUN50I_H616_TPR12
+	hex "H616 DRAM TPR12 parameter"
+	default 0x0
+	help
+	  TPR12 value from vendor DRAM settings.
 endif
 
 config SUN6I_PRCM
diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
index 3b2ba168498c..df06cea42464 100644
--- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
+++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
@@ -574,7 +574,7 @@  static bool mctl_phy_write_training(struct dram_para *para)
 
 static void mctl_phy_bit_delay_compensation(struct dram_para *para)
 {
-	u32 *ptr;
+	u32 *ptr, val;
 	int i;
 
 	if (para->tpr10 & TPR10_UNKNOWN_FEAT2) {
@@ -582,49 +582,93 @@  static void mctl_phy_bit_delay_compensation(struct dram_para *para)
 		setbits_le32(SUNXI_DRAM_PHY0_BASE + 8, 8);
 		clrbits_le32(SUNXI_DRAM_PHY0_BASE + 0x190, 0x10);
 
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = para->tpr11 & 0x3f;
+		else
+			val = (para->tpr11 & 0xf) << 1;
+
 		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x484);
 		for (i = 0; i < 9; i++) {
-			writel_relaxed(0x16, ptr);
-			writel_relaxed(0x16, ptr + 0x30);
+			writel_relaxed(val, ptr);
+			writel_relaxed(val, ptr + 0x30);
 			ptr += 2;
 		}
-		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x4d0);
-		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x590);
-		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x4cc);
-		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x58c);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->odt_en >> 15) & 0x1e;
+		else
+			val = (para->tpr11 >> 15) & 0x1e;
+
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x4d0);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x590);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x4cc);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x58c);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->tpr11 >> 8) & 0x3f;
+		else
+			val = (para->tpr11 >> 3) & 0x1e;
 
 		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x4d8);
 		for (i = 0; i < 9; i++) {
-			writel_relaxed(0x1a, ptr);
-			writel_relaxed(0x1a, ptr + 0x30);
+			writel_relaxed(val, ptr);
+			writel_relaxed(val, ptr + 0x30);
 			ptr += 2;
 		}
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x524);
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x5e4);
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x520);
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x5e0);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->odt_en >> 19) & 0x1e;
+		else
+			val = (para->tpr11 >> 19) & 0x1e;
+
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x524);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5e4);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x520);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5e0);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->tpr11 >> 16) & 0x3f;
+		else
+			val = (para->tpr11 >> 7) & 0x1e;
 
 		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x604);
 		for (i = 0; i < 9; i++) {
-			writel_relaxed(0x1a, ptr);
-			writel_relaxed(0x1a, ptr + 0x30);
+			writel_relaxed(val, ptr);
+			writel_relaxed(val, ptr + 0x30);
 			ptr += 2;
 		}
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x650);
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x710);
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x64c);
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x70c);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->odt_en >> 23) & 0x1e;
+		else
+			val = (para->tpr11 >> 23) & 0x1e;
+
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x650);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x710);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x64c);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x70c);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->tpr11 >> 24) & 0x3f;
+		else
+			val = (para->tpr11 >> 11) & 0x1e;
 
 		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x658);
 		for (i = 0; i < 9; i++) {
-			writel_relaxed(0x1a, ptr);
-			writel_relaxed(0x1a, ptr + 0x30);
+			writel_relaxed(val, ptr);
+			writel_relaxed(val, ptr + 0x30);
 			ptr += 2;
 		}
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x6a4);
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x764);
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x6a0);
-		writel_relaxed(0x1e, SUNXI_DRAM_PHY0_BASE + 0x760);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->odt_en >> 27) & 0x1e;
+		else
+			val = (para->tpr11 >> 27) & 0x1e;
+
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6a4);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x764);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6a0);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x760);
 
 		dmb();
 
@@ -635,49 +679,93 @@  static void mctl_phy_bit_delay_compensation(struct dram_para *para)
 		clrbits_le32(SUNXI_DRAM_PHY0_BASE + 0x54, 0x80);
 		clrbits_le32(SUNXI_DRAM_PHY0_BASE + 0x190, 4);
 
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = para->tpr12 & 0x3f;
+		else
+			val = (para->tpr12 & 0xf) << 1;
+
 		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x480);
 		for (i = 0; i < 9; i++) {
-			writel_relaxed(0x10, ptr);
-			writel_relaxed(0x10, ptr + 0x30);
+			writel_relaxed(val, ptr);
+			writel_relaxed(val, ptr + 0x30);
 			ptr += 2;
 		}
-		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x528);
-		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x5e8);
-		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x4c8);
-		writel_relaxed(0x18, SUNXI_DRAM_PHY0_BASE + 0x588);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->odt_en << 1) & 0x1e;
+		else
+			val = (para->tpr12 >> 15) & 0x1e;
+
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x528);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5e8);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x4c8);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x588);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->tpr12 >> 8) & 0x3f;
+		else
+			val = (para->tpr12 >> 3) & 0x1e;
 
 		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x4d4);
 		for (i = 0; i < 9; i++) {
-			writel_relaxed(0x12, ptr);
-			writel_relaxed(0x12, ptr + 0x30);
+			writel_relaxed(val, ptr);
+			writel_relaxed(val, ptr + 0x30);
 			ptr += 2;
 		}
-		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x52c);
-		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x5ec);
-		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x51c);
-		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x5dc);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->odt_en >> 3) & 0x1e;
+		else
+			val = (para->tpr12 >> 19) & 0x1e;
+
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x52c);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5ec);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x51c);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x5dc);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->tpr12 >> 16) & 0x3f;
+		else
+			val = (para->tpr12 >> 7) & 0x1e;
 
 		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x600);
 		for (i = 0; i < 9; i++) {
-			writel_relaxed(0x12, ptr);
-			writel_relaxed(0x12, ptr + 0x30);
+			writel_relaxed(val, ptr);
+			writel_relaxed(val, ptr + 0x30);
 			ptr += 2;
 		}
-		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x6a8);
-		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x768);
-		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x648);
-		writel_relaxed(0x1a, SUNXI_DRAM_PHY0_BASE + 0x708);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->odt_en >> 7) & 0x1e;
+		else
+			val = (para->tpr12 >> 23) & 0x1e;
+
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6a8);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x768);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x648);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x708);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->tpr12 >> 24) & 0x3f;
+		else
+			val = (para->tpr12 >> 11) & 0x1e;
 
 		ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x654);
 		for (i = 0; i < 9; i++) {
-			writel_relaxed(0x14, ptr);
-			writel_relaxed(0x14, ptr + 0x30);
+			writel_relaxed(val, ptr);
+			writel_relaxed(val, ptr + 0x30);
 			ptr += 2;
 		}
-		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x6ac);
-		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x76c);
-		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x69c);
-		writel_relaxed(0x1c, SUNXI_DRAM_PHY0_BASE + 0x75c);
+
+		if (para->tpr10 & TPR10_UNKNOWN_FEAT3)
+			val = (para->odt_en >> 11) & 0x1e;
+		else
+			val = (para->tpr12 >> 27) & 0x1e;
+
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x6ac);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x76c);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x69c);
+		writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x75c);
 
 		dmb();
 
@@ -1021,7 +1109,10 @@  unsigned long sunxi_dram_init(void)
 		.dx_odt = CONFIG_DRAM_SUN50I_H616_DX_ODT,
 		.dx_dri = CONFIG_DRAM_SUN50I_H616_DX_DRI,
 		.ca_dri = CONFIG_DRAM_SUN50I_H616_CA_DRI,
+		.odt_en = CONFIG_DRAM_SUN50I_H616_ODT_EN,
 		.tpr10 = CONFIG_DRAM_SUN50I_H616_TPR10,
+		.tpr11 = CONFIG_DRAM_SUN50I_H616_TPR11,
+		.tpr12 = CONFIG_DRAM_SUN50I_H616_TPR12,
 	};
 	unsigned long size;