All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andre Przywara <andre.przywara@arm.com>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: jagan@amarulasolutions.com, u-boot@lists.denx.de
Subject: Re: [PATCH 3/8] sunxi: parameterize H616 DRAM ODT values
Date: Wed, 4 Jan 2023 00:36:41 +0000	[thread overview]
Message-ID: <20230104003641.0683c235@slackpad.lan> (raw)
In-Reply-To: <20221211163213.98540-4-jernej.skrabec@gmail.com>

On Sun, 11 Dec 2022 17:32:08 +0100
Jernej Skrabec <jernej.skrabec@gmail.com> wrote:

Hi Jernej,

> While ODT values for same memory type are similar, they are not
> necessary the same. Let's parameterize them and make parameter same as
> in vendor DRAM settings. That way it will be easy to introduce new board
> support.

checked that this results in the same parameters to writel().
One small thing below:

> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  .../include/asm/arch-sunxi/dram_sun50i_h616.h |  3 +
>  arch/arm/mach-sunxi/Kconfig                   | 15 +++++
>  arch/arm/mach-sunxi/dram_sun50i_h616.c        | 59 ++++++++++++-------
>  configs/orangepi_zero2_defconfig              |  3 +
>  4 files changed, 58 insertions(+), 22 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 134679d55205..c9e1f84bfcdd 100644
> --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
> +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h
> @@ -144,6 +144,9 @@ struct dram_para {
>  	u8 rows;
>  	u8 ranks;
>  	u8 bus_full_width;
> +	u32 dx_odt;
> +	u32 dx_dri;
> +	u32 ca_dri;
>  };
>  
>  
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index dbe6005daab1..cad53f19912c 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -83,6 +83,21 @@ config DRAM_SUN50I_H616_UNKNOWN_FEATURE
>  	---help---
>  	  Select this when DRAM on your H616 board needs this unknown
>  	  feature.
> +
> +config DRAM_SUN50I_H616_DX_ODT
> +	hex "H616 DRAM DX ODT parameter"
> +	help
> +	  DX ODT value from vendor DRAM settings.
> +
> +config DRAM_SUN50I_H616_DX_DRI
> +	hex "H616 DRAM DX DRI parameter"
> +	help
> +	  DX DRI value from vendor DRAM settings.
> +
> +config DRAM_SUN50I_H616_CA_DRI
> +	hex "H616 DRAM CA DRI parameter"
> +	help
> +	  CA DRI 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 49983bf7a1b8..06a07dfbf9cc 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> @@ -234,37 +234,49 @@ static const u8 phy_init[] = {
>  	0x09, 0x05, 0x18
>  };
>  
> -static void mctl_phy_configure_odt(void)
> +static void mctl_phy_configure_odt(struct dram_para *para)
>  {
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x388);
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x38c);
> +	unsigned int val;
>  
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x3c8);
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x3cc);
> +	val = para->dx_dri & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x388);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x38c);
>  
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x408);
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x40c);
> +	val = (para->dx_dri >> 8) & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3c8);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3cc);
>  
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x448);
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x44c);
> +	val = (para->dx_dri >> 16) & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x408);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x40c);
>  
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x340);
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x344);
> +	val = (para->dx_dri >> 24) & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x448);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x44c);
>  
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x348);
> -	writel_relaxed(0xe, SUNXI_DRAM_PHY0_BASE + 0x34c);
> +	val = para->ca_dri & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x340);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x344);
>  
> -	writel_relaxed(0x8, SUNXI_DRAM_PHY0_BASE + 0x380);
> -	writel_relaxed(0x8, SUNXI_DRAM_PHY0_BASE + 0x384);
> +	val = (para->ca_dri >> 8) & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x348);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x34c);
>  
> -	writel_relaxed(0x8, SUNXI_DRAM_PHY0_BASE + 0x3c0);
> -	writel_relaxed(0x8, SUNXI_DRAM_PHY0_BASE + 0x3c4);
> +	val = para->dx_odt & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x380);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x384);
>  
> -	writel_relaxed(0x8, SUNXI_DRAM_PHY0_BASE + 0x400);
> -	writel_relaxed(0x8, SUNXI_DRAM_PHY0_BASE + 0x404);
> +	val = (para->dx_odt >> 8) & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3c0);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3c4);
>  
> -	writel_relaxed(0x8, SUNXI_DRAM_PHY0_BASE + 0x440);
> -	writel_relaxed(0x8, SUNXI_DRAM_PHY0_BASE + 0x444);
> +	val = (para->dx_odt >> 16) & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x400);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x404);
> +
> +	val = (para->dx_odt >> 24) & 0x1f;
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x440);
> +	writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x444);
>  
>  	dmb();
>  }
> @@ -722,7 +734,7 @@ static bool mctl_phy_init(struct dram_para *para)
>  	writel(0x80, SUNXI_DRAM_PHY0_BASE + 0x45c);
>  
>  	if (IS_ENABLED(CONFIG_DRAM_ODT_EN))
> -		mctl_phy_configure_odt();
> +		mctl_phy_configure_odt(para);
>  
>  	clrsetbits_le32(SUNXI_DRAM_PHY0_BASE + 4, 7, 0xa);
>  
> @@ -1007,6 +1019,9 @@ unsigned long sunxi_dram_init(void)
>  	struct dram_para para = {
>  		.clk = CONFIG_DRAM_CLK,
>  		.type = SUNXI_DRAM_TYPE_DDR3,
> +		.dx_odt = CONFIG_DRAM_SUN50I_H616_DX_ODT,
> +		.dx_dri = CONFIG_DRAM_SUN50I_H616_DX_DRI,
> +		.ca_dri = CONFIG_DRAM_SUN50I_H616_CA_DRI,
>  	};
>  	unsigned long size;
>  
> diff --git a/configs/orangepi_zero2_defconfig b/configs/orangepi_zero2_defconfig
> index 877eccf31bd6..ca398faef1d3 100644
> --- a/configs/orangepi_zero2_defconfig
> +++ b/configs/orangepi_zero2_defconfig
> @@ -6,6 +6,9 @@ CONFIG_DRAM_SUN50I_H616_WRITE_LEVELING=y
>  CONFIG_DRAM_SUN50I_H616_READ_CALIBRATION=y
>  CONFIG_DRAM_SUN50I_H616_READ_TRAINING=y
>  CONFIG_DRAM_SUN50I_H616_WRITE_TRAINING=y
> +CONFIG_DRAM_SUN50I_H616_DX_ODT=0x8080808
> +CONFIG_DRAM_SUN50I_H616_DX_DRI=0xe0e0e0e
> +CONFIG_DRAM_SUN50I_H616_CA_DRI=0xe0e

Can you make those values 8 nibbles and 4 nibbles long (so starting
with a 0)? It's somewhat confusing to read like this, it *looks* like a
32-bit value, but it's actually only 28 bits wide. This gets confusing
when matching up values against dumps or a hex editor.

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

Cheers,
Andre

>  CONFIG_MACH_SUN50I_H616=y
>  CONFIG_MMC0_CD_PIN="PF6"
>  CONFIG_R_I2C_ENABLE=y


  reply	other threads:[~2023-01-04  0:41 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-11 16:32 [PATCH 0/8] sunxi: Update H616 DRAM driver Jernej Skrabec
2022-12-11 16:32 ` [PATCH 1/8] sunxi: Fix write to H616 DRAM CR register Jernej Skrabec
2023-01-04  0:35   ` Andre Przywara
2022-12-11 16:32 ` [PATCH 2/8] sunxi: cosmetic: Fix H616 DRAM driver code style Jernej Skrabec
2023-01-04  0:36   ` Andre Przywara
2022-12-11 16:32 ` [PATCH 3/8] sunxi: parameterize H616 DRAM ODT values Jernej Skrabec
2023-01-04  0:36   ` Andre Przywara [this message]
2022-12-11 16:32 ` [PATCH 4/8] sunxi: Convert H616 DRAM options to single setting Jernej Skrabec
2022-12-12 17:50   ` Andre Przywara
2022-12-13 16:23     ` Jernej Škrabec
2022-12-13 16:51       ` Andre Przywara
2022-12-13 17:08         ` Jernej Škrabec
2022-12-11 16:32 ` [PATCH 5/8] sunxi: Always configure ODT on H616 DRAM Jernej Skrabec
2023-01-04  0:37   ` Andre Przywara
2023-01-04 21:12     ` Jernej Škrabec
2022-12-11 16:32 ` [PATCH 6/8] sunxi: Make bit delay function in H616 DRAM code void Jernej Skrabec
2023-01-04  0:37   ` Andre Przywara
2022-12-11 16:32 ` [PATCH 7/8] sunxi: Parameterize bit delay code in H616 DRAM driver Jernej Skrabec
2023-01-04  0:37   ` Andre Przywara
2023-01-04 21:28     ` Jernej Škrabec
2022-12-11 16:32 ` [PATCH 8/8] sunxi: Parameterize H616 DRAM code some more Jernej Skrabec
2022-12-11 18:33   ` Jernej Škrabec
2023-01-04  0:38   ` Andre Przywara
2023-01-04 21:30     ` Jernej Škrabec
2022-12-12  1:04 ` [PATCH 0/8] sunxi: Update H616 DRAM driver Andre Przywara
2022-12-12 16:14   ` Jernej Škrabec
2023-01-04  0:47 ` Andre Przywara
2023-01-04 21:02   ` Jernej Škrabec
2023-01-04 23:21     ` Andre Przywara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230104003641.0683c235@slackpad.lan \
    --to=andre.przywara@arm.com \
    --cc=jagan@amarulasolutions.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.