[U-Boot] [PATCH] arm: imx6: configure NoC on i.MX6DQP

[Stefano Babic <sbabic@denx.de>]

Hi Filip,

On 14/09/2016 13:36, Filip Brozovic wrote:
> The i.MX6DP and i.MX6QP incorporate NoC interconnect logic
> which needs to be configured in order to use external DDR memory.
> 
> This patch enables the SPL to configure the necessary registers
> in accordance with the NXP engineering bulletin EB828.
> 
> Signed-off-by: Filip Brozovic <fbrozovic@gmail.com>
> ---
>  arch/arm/cpu/armv7/mx6/ddr.c            | 69 +++++++++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-mx6/mx6-ddr.h | 19 +++++++++
>  2 files changed, 88 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/mx6/ddr.c b/arch/arm/cpu/armv7/mx6/ddr.c
> index 7beb7ea..60bfd08 100644
> --- a/arch/arm/cpu/armv7/mx6/ddr.c
> +++ b/arch/arm/cpu/armv7/mx6/ddr.c
> @@ -1181,6 +1181,8 @@ void mx6_ddr3_cfg(const struct mx6_ddr_sysinfo *sysinfo,
>  {
>  	volatile struct mmdc_p_regs *mmdc0;
>  	volatile struct mmdc_p_regs *mmdc1;
> +	struct src *src_regs = (struct src *)SRC_BASE_ADDR;
> +	u8 soc_boot_cfg3 = ((readl(&src_regs->sbmr1) & 0xff0000) >> 16) & 0xff;
>  	u32 val;
>  	u8 tcke, tcksrx, tcksre, txpdll, taofpd, taonpd, trrd;
>  	u8 todtlon, taxpd, tanpd, tcwl, txp, tfaw, tcl;
> @@ -1473,6 +1475,73 @@ void mx6_ddr3_cfg(const struct mx6_ddr_sysinfo *sysinfo,
>  	/* Step 12: Configure and activate periodic refresh */
>  	mmdc0->mdref = (sysinfo->refsel << 14) | (sysinfo->refr << 11);
>  
> +	/*
> +	 * i.MX6DQP only: If the NoC scheduler is enabled,
> +	 * configure it and disable MMDC arbitration/reordering (see EB828)
> +	 */
> +	if (is_mx6dqp() && ((soc_boot_cfg3 & 0x30) == 0x00 ||
> +			    (soc_boot_cfg3 & 0x33) == 0x31))

I would suggest you add defines for the value of the "DDR Memory Map
default config" bit, because this is not very readable. And you add in
the comment a reference to the manual, i.e. a reference to the table
(5.5 ?) where BOOT_CFG3 is explained.

 {
> +		struct mx6dqp_noc_sched_regs *noc_sched =
> +			(struct mx6dqp_noc_sched_regs *)MX6DQP_NOC_SCHED_BASE;
> +
> +		/* These values are fixed based on integration parameters and
> +		 * should not be modified */

Codestyle: Multi-line comment must be fixed

> +		noc_sched->rlat = 0x00000040;
> +		noc_sched->ipu1 = 0x00000020;
> +		noc_sched->ipu2 = 0x00000020;
> +
> +		noc_sched->activate = (1 << 10) |	/* FawBank */
> +				      (tfaw << 4) |
> +				      (trrd << 0);
> +		noc_sched->ddrtiming = (((sysinfo->dsize == 1) ? 1 : 0) << 31) |
> +				       ((tcwl + twtr) << 26) |
> +				       ((tcl - tcwl + 2) << 21) |
> +				       (4 << 18) |	/* Burst Length = 8 */
> +				       ((tcwl + twr + trp + trcd) << 12) |
> +				       ((trtp + trp + trcd - 4) << 6) |
> +				       (trc << 0);
> +

Same here: what about to add defines for shifting and seting into the
register ? This is not very nice to read it.


> +		if (sysinfo->dsize == 2) {
> +			if (ddr3_cfg->coladdr == 10) {
> +				if (ddr3_cfg->rowaddr == 15 &&
> +				    sysinfo->ncs == 2)
> +					noc_sched->ddrconf = 4;
> +				else
> +					noc_sched->ddrconf = 0;
> +			} else if (ddr3_cfg->coladdr == 11) {
> +				noc_sched->ddrconf = 1;
> +			}
> +		} else {
> +			if (ddr3_cfg->coladdr == 9) {
> +				if (ddr3_cfg->rowaddr == 13)
> +					noc_sched->ddrconf = 2;
> +				else if (ddr3_cfg->rowaddr == 14)
> +					noc_sched->ddrconf = 15;
> +			} else if (ddr3_cfg->coladdr == 10) {
> +				if (ddr3_cfg->rowaddr == 14 &&
> +				    sysinfo->ncs == 2)
> +					noc_sched->ddrconf = 14;
> +				else if (ddr3_cfg->rowaddr == 15 &&
> +					 sysinfo->ncs == 2)
> +					noc_sched->ddrconf = 9;
> +				else
> +					noc_sched->ddrconf = 3;
> +			} else if (ddr3_cfg->coladdr == 11) {
> +				if (ddr3_cfg->rowaddr == 15 &&
> +				    sysinfo->ncs == 2)
> +					noc_sched->ddrconf = 4;
> +				else
> +					noc_sched->ddrconf = 0;
> +			} else if (ddr3_cfg->coladdr == 12) {
> +				if (ddr3_cfg->rowaddr == 14)
> +					noc_sched->ddrconf = 1;
> +			}
> +		}
> +
> +		/* Disable MMDC arbitration/reordering */
> +		mmdc0->maarcr = 0x14420000;
> +	}
> +
>  	/* Step 13: Deassert config request - init complete */
>  	mmdc0->mdscr = 0x00000000;
>  
> diff --git a/arch/arm/include/asm/arch-mx6/mx6-ddr.h b/arch/arm/include/asm/arch-mx6/mx6-ddr.h
> index 9922409..c881782 100644
> --- a/arch/arm/include/asm/arch-mx6/mx6-ddr.h
> +++ b/arch/arm/include/asm/arch-mx6/mx6-ddr.h
> @@ -307,6 +307,25 @@ struct mx6dq_iomux_grp_regs {
>  	u32 grp_b6ds;
>  };
>  
> +/*
> + * NoC scheduler registers - only on IMX6DQP
> + */
> +#define MX6DQP_NOC_SCHED_BASE	0x00bb0000
> +struct mx6dqp_noc_sched_regs {
> +	u32 coreid;
> +	u32 revid;
> +	u32 ddrconf;
> +	u32 ddrtiming;
> +	u32 ddrmode;
> +	u32 rlat;
> +	u32 res1[4];
> +	u32 ipu1;
> +	u32 ipu2;
> +	u32 res2[2];
> +	u32 activate;
> +	u32 res3[16];
> +};
> +
>  #define MX6SDL_IOM_DDR_BASE     0x020e0400
>  struct mx6sdl_iomux_ddr_regs {
>  	u32 res1[25];
> 

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================