| Message ID | 20221211163213.98540-6-jernej.skrabec@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Andre Przywara |
| Headers | show |
| Series | sunxi: Update H616 DRAM driver | expand |
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); >
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);
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);
Vendor H616 DRAM code always configure part which we call ODT configuration. Let's reflect that here too. 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(-)