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

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Dne sreda, 04. januar 2023 ob 01:37:17 CET je Andre Przywara napisal(a):
> 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.

I don't see it that way. It just means it's unused for H616.

> 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?

The thing is that I can't give you good explanation for anything above due to 
nature of reverse engineering. It's just how vendor code is done and I would 
argue that my original assumption when I was writing this driver was wrong and 
I shouldn't use this symbol at all in first place. I'm not even sure if I named 
mctl_phy_configure_odt() completely correct. It has to do something with ODT, 
but I don't know if it gets enabled here or not. To me, it looks more like 
that just some parameters are set here.

Best regards,
Jernej

> 
> 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);