[U-Boot] [PATCH v2 13/23] sunxi: H3: add and rename some DRAM contoller registers
Simon Glass
sjg at chromium.org
Mon Dec 5 07:26:04 CET 2016
Hi Andre,
On 4 December 2016 at 18:52, Andre Przywara <andre.przywara at arm.com> wrote:
> From: Jens Kuske <jenskuske at gmail.com>
>
> The IOCR registers got renamed to BDLR to match the public
> documentation of similar controllers.
>
> Signed-off-by: Jens Kuske <jenskuske at gmail.com>
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> ---
> 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 <sjg at chromium.org>
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
More information about the U-Boot
mailing list