Re: [PATCH 5/8] sunxi: Always configure ODT on H616 DRAM

[Andre Przywara <andre.przywara@arm.com>]

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

> Vendor H616 DRAM code always configure part which we call ODT
> configuration. Let's reflect that here too.

I wonder if we need this patch at all. "depends on !H616" looks
counter-intuitive, since this suggests it's always off.
As it stands, it doesn't hurt. "default y" does the right thing, and if
people want to shoot themselves in the foot: fine by me.

At least I would like to keep the Kconfig part. We could change the
condition in the code into an explaining comment, if you still want to
force this on.

And coming back from patch 7/8: how does this correspond to
DRAM_SUN50I_H616_ODT_EN?

Cheers,
Andre

> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  arch/arm/mach-sunxi/Kconfig            | 2 +-
>  arch/arm/mach-sunxi/dram_sun50i_h616.c | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index abcbd0fb9061..778304b77e26 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -488,12 +488,12 @@ config DRAM_ZQ
>  
>  config DRAM_ODT_EN
>  	bool "sunxi dram odt enable"
> +	depends on !MACH_SUN50I_H616
>  	default y if MACH_SUN8I_A23
>  	default y if MACH_SUNXI_H3_H5
>  	default y if MACH_SUN8I_R40
>  	default y if MACH_SUN50I
>  	default y if MACH_SUN50I_H6
> -	default y if MACH_SUN50I_H616
>  	---help---
>  	Select this to enable dram odt (on die termination).
>  
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> index 14a01a3c4e54..bf5b4ddfb5c2 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> @@ -736,8 +736,7 @@ static bool mctl_phy_init(struct dram_para *para)
>  	writel(0x80, SUNXI_DRAM_PHY0_BASE + 0x3dc);
>  	writel(0x80, SUNXI_DRAM_PHY0_BASE + 0x45c);
>  
> -	if (IS_ENABLED(CONFIG_DRAM_ODT_EN))
> -		mctl_phy_configure_odt(para);
> +	mctl_phy_configure_odt(para);
>  
>  	clrsetbits_le32(SUNXI_DRAM_PHY0_BASE + 4, 7, 0xa);
>