On Fri, 09 Jun 2023 22:38:38 +0200
Jernej Škrabec wrote:
Hi,
> Dne sreda, 07. junij 2023 ob 02:07:45 CEST je Andre Przywara napisal(a):
> > From: iuncuim
> >
> > The H616 SoC has support for several types of DRAM: DDR3, LPDDR3,
> > DDR4 and LPDDR4.
> > At the moment, the driver only supports DDR3 memory.
> > Let's extend the driver to support the LPDDR3 memory. All "magic"
> > values obtained from the boot0.
> > ---
> > arch/arm/mach-sunxi/Kconfig | 8 +
> > arch/arm/mach-sunxi/dram_sun50i_h616.c| 193 +-
> > arch/arm/mach-sunxi/dram_timings/Makefile | 1 +
> > .../arm/mach-sunxi/dram_timings/h616_lpddr3.c | 95 +
> > 4 files changed, 242 insertions(+), 55 deletions(-)
> > create mode 100644 arch/arm/mach-sunxi/dram_timings/h616_lpddr3.c
> >
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index 197d77ea658..5ce82a955c6 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -487,6 +487,14 @@ config SUNXI_DRAM_H6_DDR3_1333
> > This option is the DDR3 timing used by the boot0 on H6 TV boxes
> > which use a DDR3-1333 timing.
> >
> > +config SUNXI_DRAM_H616_LPDDR3
> > + bool "LPDDR3 DRAM chips on the H616 DRAM controller"
> > + select SUNXI_DRAM_LPDDR3
> > + depends on DRAM_SUN50I_H616
> > + ---help---
> > + This option is the LPDDR3 timing used by the stock boot0 by
> > + Allwinner.
> > +
> > config SUNXI_DRAM_H616_DDR3_1333
> > bool "DDR3-1333 boot0 timings on the H616 DRAM controller"
> > select SUNXI_DRAM_DDR3
> > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > index 4e988cebf59..082746ea7f3 100644
> > --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> > @@ -228,10 +228,17 @@ static void mctl_set_addrmap(const struct dram_config
> > *config)
> > }
> >
> > static const u8 phy_init[] = {
> > +#ifdef CONFIG_SUNXI_DRAM_H616_DDR3_1333
> > 0x07, 0x0b, 0x02, 0x16, 0x0d, 0x0e, 0x14, 0x19,
> > 0x0a, 0x15, 0x03, 0x13, 0x04, 0x0c, 0x10, 0x06,
> > 0x0f, 0x11, 0x1a, 0x01, 0x12, 0x17, 0x00, 0x08,
> > 0x09, 0x05, 0x18
> > +#elif defined(CONFIG_SUNXI_DRAM_H616_LPDDR3)
> > + 0x18, 0x06, 0x00, 0x05, 0x04, 0x03, 0x09, 0x02,
> > + 0x08, 0x01, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
> > + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x07,
> > + 0x17, 0x19, 0x1a
> > +#endif
> > };
> >
> > static void mctl_phy_configure_odt(const struct dram_para *para)
> > @@ -263,19 +270,31 @@ static void mctl_phy_configure_odt(const struct
> > dram_para *para)
> > writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x34c);
> >
> > val = para->dx_odt & 0x1f;
> > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x380);
> > + if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > + writel_relaxed(0, SUNXI_DRAM_PHY0_BASE + 0x380);
> > + else
> > + writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x380);
> > writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x384);
> >
> > val = (para->dx_odt >> 8) & 0x1f;
> > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3c0);
> > + if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > + writel_relaxed(0, SUNXI_DRAM_PHY0_BASE + 0x3c0);
> > + else
> > + writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3c0);
> > writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3c4);
> >
> > val = (para->dx_odt >> 16) & 0x1f;
> > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x400);
> > + if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > + writel_relaxed(0, SUNXI_DRAM_PHY0_BASE + 0x400);
> > + else
> > + 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);
> > + if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > + writel_relaxed(0, SUNXI_DRAM_PHY0_BASE + 0x440);
> > + else
> > + writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x440);
> > writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x444);
>
> Above dx_odt oriented changes are not needed. They depend on Kconfig symbol,
> which can simply be set to 0.
Ha, I thought so as well, but if you look closely, it's only *one* of
the two writes which turn to zero, the other register stays at the
Kconfig value. Annoying, I know ;-)
I couldn't think of a shorter or even cleaner way to express this,
really, apart from a macro maybe, so I guess we have to live with it.
Cheers,
Andre
> I didn't do detailed check, but if it works, it should be ok. Were calculated
> values here compared to vendor driver?
>
> Best regards,
> Jernej
>
> >
> > dmb();
> > @@ -794,31 +813,47 @@ static void mctl_phy_ca_bit_delay_compensation(const
> > struct dram_para *para,
> > writel(val, SUNXI_DRAM_PHY0_BASE + 0x7e0);
> > writel(val, SUNXI_DRAM_PHY0_BASE + 0x7f4);
> >
> > - /* fo