From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Sun, 4 Dec 2016 23:26:04 -0700 Subject: [U-Boot] [PATCH v2 13/23] sunxi: H3: add and rename some DRAM contoller registers In-Reply-To: <1480902750-839-14-git-send-email-andre.przywara@arm.com> References: <1480902750-839-1-git-send-email-andre.przywara@arm.com> <1480902750-839-14-git-send-email-andre.przywara@arm.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Andre, On 4 December 2016 at 18:52, Andre Przywara wrote: > From: Jens Kuske > > The IOCR registers got renamed to BDLR to match the public > documentation of similar controllers. > > Signed-off-by: Jens Kuske > Signed-off-by: Andre Przywara > --- > arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h | 43 ++++++++++++++----------- > arch/arm/mach-sunxi/dram_sun8i_h3.c | 34 +++++++++---------- > 2 files changed, 41 insertions(+), 36 deletions(-) Reviewed-by: Simon Glass Some suggestions below if you have the energy. > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h > index d0f2b8a..867fd12 100644 > --- a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h > @@ -81,7 +81,7 @@ struct sunxi_mctl_ctl_reg { > u32 rfshtmg; /* 0x90 refresh timing */ > u32 rfshctl1; /* 0x94 */ > u32 pwrtmg; /* 0x98 */ > - u8 res3[0x20]; /* 0x9c */ > + u8 res3[0x20]; /* 0x9c */ > u32 dqsgmr; /* 0xbc */ > u32 dtcr; /* 0xc0 */ > u32 dtar[4]; /* 0xc4 */ > @@ -106,20 +106,23 @@ struct sunxi_mctl_ctl_reg { > u32 perfhpr[2]; /* 0x1c4 */ > u32 perflpr[2]; /* 0x1cc */ > u32 perfwr[2]; /* 0x1d4 */ > - u8 res8[0x2c]; /* 0x1dc */ > - u32 aciocr; /* 0x208 */ > - u8 res9[0xf4]; /* 0x20c */ > + u8 res8[0x24]; /* 0x1dc */ > + u32 acmdlr; /* 0x200 AC master delay line register */ > + u32 aclcdlr; /* 0x204 AC local calibrated delay line register */ > + u32 aciocr; /* 0x208 AC I/O configuration register */ > + u8 res9[0x4]; /* 0x20c */ > + u32 acbdlr[31]; /* 0x210 AC bit delay line registers */ > + u8 res10[0x74]; /* 0x28c */ > struct { /* 0x300 DATX8 modules*/ > - u32 mdlr; /* 0x00 */ > - u32 lcdlr[3]; /* 0x04 */ > - u32 iocr[11]; /* 0x10 IO configuration register */ > - u32 bdlr6; /* 0x3c */ > - u32 gtr; /* 0x40 */ > - u32 gcr; /* 0x44 */ > - u32 gsr[3]; /* 0x48 */ > + u32 mdlr; /* 0x00 master delay line register */ > + u32 lcdlr[3]; /* 0x04 local calibrated delay line registers */ > + u32 bdlr[12]; /* 0x10 bit delay line registers */ > + u32 gtr; /* 0x40 general timing register */ > + u32 gcr; /* 0x44 general configuration register */ > + u32 gsr[3]; /* 0x48 general status registers */ > u8 res0[0x2c]; /* 0x54 */ > - } datx[4]; > - u8 res10[0x388]; /* 0x500 */ > + } dx[4]; > + u8 res11[0x388]; /* 0x500 */ > u32 upd2; /* 0x888 */ > }; > > @@ -174,12 +177,14 @@ struct sunxi_mctl_ctl_reg { > > #define ZQCR_PWRDOWN (0x1 << 31) /* ZQ power down */ 1U << 31 > > -#define DATX_IOCR_DQ(x) (x) /* DQ0-7 IOCR index */ > -#define DATX_IOCR_DM (8) /* DM IOCR index */ > -#define DATX_IOCR_DQS (9) /* DQS IOCR index */ > -#define DATX_IOCR_DQSN (10) /* DQSN IOCR index */ > +#define ACBDLR_WRITE_DELAY(x) ((x) << 8) Better to have #define ACBDLR_WRITE_DELAY_SHIFT 8 #define ACBDLR_WRITE_DELAY_MASK (0xff << ACBDLR_WRITE_DELAY_SHIFT) and then use that in the code. Similarly with other accessors. > > -#define DATX_IOCR_WRITE_DELAY(x) ((x) << 8) > -#define DATX_IOCR_READ_DELAY(x) ((x) << 0) > +#define DXBDLR_DQ(x) (x) /* DQ0-7 BDLR index */ > +#define DXBDLR_DM (8) /* DM BDLR index */ Can we drop the unnecessary brackets around constants? > +#define DXBDLR_DQS (9) /* DQS BDLR index */ > +#define DXBDLR_DQSN (10) /* DQSN BDLR index */ > + > +#define DXBDLR_WRITE_DELAY(x) ((x) << 8) > +#define DXBDLR_READ_DELAY(x) ((x) << 0) > > #endif /* _SUNXI_DRAM_SUN8I_H3_H */ > diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_sun8i_h3.c > index b08b8e6..3dd6803 100644 > --- a/arch/arm/mach-sunxi/dram_sun8i_h3.c > +++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c > @@ -72,21 +72,21 @@ static void mctl_dq_delay(u32 read, u32 write) > u32 val; > > for (i = 0; i < 4; i++) { > - val = DATX_IOCR_WRITE_DELAY((write >> (i * 4)) & 0xf) | > - DATX_IOCR_READ_DELAY(((read >> (i * 4)) & 0xf) * 2); > + val = DXBDLR_WRITE_DELAY((write >> (i * 4)) & 0xf) | > + DXBDLR_READ_DELAY(((read >> (i * 4)) & 0xf) * 2); > > - for (j = DATX_IOCR_DQ(0); j <= DATX_IOCR_DM; j++) > - writel(val, &mctl_ctl->datx[i].iocr[j]); > + for (j = DXBDLR_DQ(0); j <= DXBDLR_DM; j++) > + writel(val, &mctl_ctl->dx[i].bdlr[j]); > } > > clrbits_le32(&mctl_ctl->pgcr[0], 1 << 26); > > for (i = 0; i < 4; i++) { > - val = DATX_IOCR_WRITE_DELAY((write >> (16 + i * 4)) & 0xf) | > - DATX_IOCR_READ_DELAY((read >> (16 + i * 4)) & 0xf); > + val = DXBDLR_WRITE_DELAY((write >> (16 + i * 4)) & 0xf) | > + DXBDLR_READ_DELAY((read >> (16 + i * 4)) & 0xf); > > - writel(val, &mctl_ctl->datx[i].iocr[DATX_IOCR_DQS]); > - writel(val, &mctl_ctl->datx[i].iocr[DATX_IOCR_DQSN]); > + writel(val, &mctl_ctl->dx[i].bdlr[DXBDLR_DQS]); > + writel(val, &mctl_ctl->dx[i].bdlr[DXBDLR_DQSN]); > } > > setbits_le32(&mctl_ctl->pgcr[0], 1 << 26); > @@ -344,7 +344,7 @@ static int mctl_channel_init(struct dram_para *para) > > /* set dramc odt */ > for (i = 0; i < 4; i++) > - clrsetbits_le32(&mctl_ctl->datx[i].gcr, (0x3 << 4) | > + clrsetbits_le32(&mctl_ctl->dx[i].gcr, (0x3 << 4) | > (0x1 << 1) | (0x3 << 2) | (0x3 << 12) | > (0x3 << 14), > IS_ENABLED(CONFIG_DRAM_ODT_EN) ? 0x0 : 0x2); > @@ -364,8 +364,8 @@ static int mctl_channel_init(struct dram_para *para) > > /* set half DQ */ > if (para->bus_width != 32) { > - writel(0x0, &mctl_ctl->datx[2].gcr); > - writel(0x0, &mctl_ctl->datx[3].gcr); > + writel(0x0, &mctl_ctl->dx[2].gcr); > + writel(0x0, &mctl_ctl->dx[3].gcr); > } > > /* data training configuration */ > @@ -386,17 +386,17 @@ static int mctl_channel_init(struct dram_para *para) > /* detect ranks and bus width */ > if (readl(&mctl_ctl->pgsr[0]) & (0xfe << 20)) { > /* only one rank */ > - if (((readl(&mctl_ctl->datx[0].gsr[0]) >> 24) & 0x2) || > - ((readl(&mctl_ctl->datx[1].gsr[0]) >> 24) & 0x2)) { > + if (((readl(&mctl_ctl->dx[0].gsr[0]) >> 24) & 0x2) || Looks like these fields should have #defines also. > + ((readl(&mctl_ctl->dx[1].gsr[0]) >> 24) & 0x2)) { > clrsetbits_le32(&mctl_ctl->dtcr, 0xf << 24, 0x1 << 24); > para->dual_rank = 0; > } > > /* only half DQ width */ > - if (((readl(&mctl_ctl->datx[2].gsr[0]) >> 24) & 0x1) || > - ((readl(&mctl_ctl->datx[3].gsr[0]) >> 24) & 0x1)) { > - writel(0x0, &mctl_ctl->datx[2].gcr); > - writel(0x0, &mctl_ctl->datx[3].gcr); > + if (((readl(&mctl_ctl->dx[2].gsr[0]) >> 24) & 0x1) || > + ((readl(&mctl_ctl->dx[3].gsr[0]) >> 24) & 0x1)) { > + writel(0x0, &mctl_ctl->dx[2].gcr); > + writel(0x0, &mctl_ctl->dx[3].gcr); > para->bus_width = 16; > } > > -- > 2.8.2 > Regards, Simon