[U-Boot] [PATCH 14/15] drivers: ddr: introduce DDR driver for i.MX8M

[Peng Fan]

> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky at boundarydevices.com]
> Sent: 2018年11月10日 2:53
> To: Peng Fan <peng.fan at nxp.com>; sbabic at denx.de; Fabio Estevam
> <fabio.estevam at nxp.com>
> Cc: u-boot at lists.denx.de; dl-linux-imx <linux-imx at nxp.com>
> Subject: Re: [U-Boot] [PATCH 14/15] drivers: ddr: introduce DDR driver for
> i.MX8M
> 
> On 11/9/2018 1:17 AM, Peng Fan wrote:
> > Introduce DDR driver for i.MX8M. The driver will be used by SPL to
> > initialze DDR PHY and DDR Controller.
> >
> > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > ---
> >  arch/arm/include/asm/arch-imx8m/ddr.h | 575
> ++++++++++++++++++++++++++++++++++
> >  drivers/Makefile                      |   1 +
> >  drivers/ddr/Kconfig                   |   1 +
> >  drivers/ddr/imx/Kconfig               |   1 +
> >  drivers/ddr/imx/imx8m/Kconfig         |  22 ++
> >  drivers/ddr/imx/imx8m/Makefile        |  11 +
> >  drivers/ddr/imx/imx8m/ddr4_init.c     | 113 +++++++
> >  drivers/ddr/imx/imx8m/ddrphy_train.c  |  87 +++++
> > drivers/ddr/imx/imx8m/ddrphy_utils.c  | 186 +++++++++++
> >  drivers/ddr/imx/imx8m/helper.c        | 171 ++++++++++
> >  drivers/ddr/imx/imx8m/lpddr4_init.c   | 188 +++++++++++
> >  11 files changed, 1356 insertions(+)
> >  create mode 100644 drivers/ddr/imx/Kconfig  create mode 100644
> > drivers/ddr/imx/imx8m/Kconfig  create mode 100644
> > drivers/ddr/imx/imx8m/Makefile  create mode 100644
> > drivers/ddr/imx/imx8m/ddr4_init.c  create mode 100644
> > drivers/ddr/imx/imx8m/ddrphy_train.c
> >  create mode 100644 drivers/ddr/imx/imx8m/ddrphy_utils.c
> >  create mode 100644 drivers/ddr/imx/imx8m/helper.c  create mode
> 100644
> > drivers/ddr/imx/imx8m/lpddr4_init.c
> >
> > diff --git a/arch/arm/include/asm/arch-imx8m/ddr.h
> > b/arch/arm/include/asm/arch-imx8m/ddr.h
> > index 1a5cbabdaf..c7da3017ba 100644
> > --- a/arch/arm/include/asm/arch-imx8m/ddr.h
> > +++ b/arch/arm/include/asm/arch-imx8m/ddr.h
> > @@ -6,6 +6,10 @@
> >  #ifndef __ASM_ARCH_IMX8M_DDR_H
> >  #define __ASM_ARCH_IMX8M_DDR_H
> >
> > +#include <asm/io.h>
> > +#include <asm/types.h>
> > +#include <asm/arch/ddr.h>
> > +
> >  #define DDRC_DDR_SS_GPR0		0x3d000000
> >  #define DDRC_IPS_BASE_ADDR_0		0x3f400000
> >  #define IP2APB_DDRPHY_IPS_BASE_ADDR(X)	(0x3c000000 + (X *
> 0x2000000))
> > @@ -352,4 +356,575 @@ enum msg_response {
> >  	TRAIN_FAIL = 0xff,
> >  };
> >
> > +#define DDRC_MSTR_0             0x3d400000
> > +#define DDRC_STAT_0             0x3d400004
> > +#define DDRC_MSTR1_0            0x3d400008
> 
> Looks like this should be a structure

I tried structure before, see https://patchwork.ozlabs.org/patch/857974/
But that still not good. There are too many registers, I would like
to keep current code. The ddr code are used in NXP vendor tree and are used in
GA release, and the ddr tool will auto generated code, manually converting the current code
to structure will be also easy to introduce errors.

Stefano, Fabio, what do you think?

> 
> > +/**********************/
> > +#define DDRC_MSTR(X)             (DDRC_IPS_BASE_ADDR(X) + 0x00)
> > +#define DDRC_STAT(X)             (DDRC_IPS_BASE_ADDR(X) + 0x04)
> > +#define DDRC_MSTR1(X)            (DDRC_IPS_BASE_ADDR(X) + 0x08)
> 
> Same structure
> 
> > +#define DDRC_ADVECCINDEX(X)      (DDRC_IPS_BASE_ADDR(X) + 0x3)
> > +#define DDRC_ADVECCSTAT(X)       (DDRC_IPS_BASE_ADDR(X) + 0x3)
> > +#define DDRC_ECCPOISONPAT0(X)    (DDRC_IPS_BASE_ADDR(X) + 0x3)
> > +#define DDRC_ECCPOISONPAT1(X)    (DDRC_IPS_BASE_ADDR(X) + 0x3)
> > +#define DDRC_ECCPOISONPAT2(X)    (DDRC_IPS_BASE_ADDR(X) + 0x3)
> > +#define DDRC_HIFCTL(X)           (DDRC_IPS_BASE_ADDR(X) + 0x3)
> 
> Are they really the same address ???

This could be dropped.

Thanks,
Peng.

> 
> > +#define DRC_PERF_MON_BASE_ADDR(X)            (0x3d800000 + ((X) *
> 0x2000000))
> > +#define DRC_PERF_MON_CNT0_CTL(X)
> (DRC_PERF_MON_BASE_ADDR(X) + 0x0)
> > +#define DRC_PERF_MON_CNT1_CTL(X)
> (DRC_PERF_MON_BASE_ADDR(X) + 0x4)
> > +#define DRC_PERF_MON_CNT2_CTL(X)
> (DRC_PERF_MON_BASE_ADDR(X) + 0x8)
> > +#define DRC_PERF_MON_CNT3_CTL(X)
> (DRC_PERF_MON_BASE_ADDR(X) + 0xC)
> 
> Structure
> 
> > +void ddrphy_init_set_dfi_clk(unsigned int drate) {
> > +	switch (drate) {
> > +	case 3200:
> > +		dram_pll_init(DRAM_PLL_OUT_800M);
> > +		dram_disable_bypass();
> > +		break;
> 
> 
> Maybe drate can be changed to a Hz freq instead of MHZ
> 	if (drate > 400M) {
> 		dram_pll_init(drate/4);
> 		dram_disable_bypass();
> 	} else {
> 		dram_enable_bypass(drate);
> 	}