| Message ID | 1473852987-9720-1-git-send-email-fbrozovic@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
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
Hi Filip Is there any update on this patch? I am looking into imx6qp bring-up and this looks like a nice patch. 2016-10-04 10:51 GMT+02:00 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@denx.de > ===================================================================== > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
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)) { + 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 */ + 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); + + 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];
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(+)