| Message ID | 20170211150843.47865-2-icenowy@aosc.xyz |
|---|---|
| State | RFC |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi, renaming is not quite enough, see the comments below. On 11.02.2017 16:08, Icenowy Zheng wrote: > The DesignWare DRAM controller used by H3 and newer SoCs use a bit to > identify whether the DRAM is half-width. > > As H3 itself come with 32-bit DRAM, the two modes of the bit used to be > named "MCTL_CR_32BIT" and "MCTL_CR_16BIT", but for SoCs with 16-bit DRAM > they're really 8-bit and 16-bit. > > Rename the bit's macro, and also rename the variable name in > dram_sun8i_h3.c. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> > --- > arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h | 6 +++--- > arch/arm/mach-sunxi/dram_sunxi_dw.c | 11 ++++++----- > 2 files changed, 9 insertions(+), 8 deletions(-) > > 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 25d07d9863..48bd6f7c0f 100644 > --- a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h > @@ -52,9 +52,9 @@ struct sunxi_mctl_com_reg { > #define MCTL_CR_SEQUENTIAL (0x1 << 15) > #define MCTL_CR_INTERLEAVED (0x0 << 15) > > -#define MCTL_CR_32BIT (0x1 << 12) > -#define MCTL_CR_16BIT (0x0 << 12) > -#define MCTL_CR_BUS_WIDTH(x) ((x) == 32 ? MCTL_CR_32BIT : MCTL_CR_16BIT) > +#define MCTL_CR_FULL_WIDTH (0x1 << 12) > +#define MCTL_CR_HALF_WIDTH (0x0 << 12) > +#define MCTL_CR_BUS_FULL_WIDTH(x) ((x) << 12) > > #define MCTL_CR_PAGE_SIZE(x) ((fls(x) - 4) << 8) > #define MCTL_CR_ROW_BITS(x) (((x) - 1) << 4) > diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c b/arch/arm/mach-sunxi/dram_sunxi_dw.c > index 9f7cc7fd4c..0c73a43075 100644 > --- a/arch/arm/mach-sunxi/dram_sunxi_dw.c > +++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c > @@ -28,7 +28,7 @@ > #define LINES_PER_BYTE_LANE (BITS_PER_BYTE + 3) > struct dram_para { > u16 page_size; > - u8 bus_width; > + u8 bus_full_width; > u8 dual_rank; > u8 row_bits; > const u8 dx_read_delays[NR_OF_BYTE_LANES][LINES_PER_BYTE_LANE]; > @@ -358,7 +358,8 @@ static void mctl_set_cr(struct dram_para *para) > (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE; > > writel(MCTL_CR_BL8 | MCTL_CR_2T | MCTL_CR_DDR3 | MCTL_CR_INTERLEAVED | > - MCTL_CR_EIGHT_BANKS | MCTL_CR_BUS_WIDTH(para->bus_width) | > + MCTL_CR_EIGHT_BANKS | > + MCTL_CR_BUS_FULL_WIDTH(para->bus_full_width) | > (para->dual_rank ? MCTL_CR_DUAL_RANK : MCTL_CR_SINGLE_RANK) | > MCTL_CR_PAGE_SIZE(para->page_size) | > MCTL_CR_ROW_BITS(para->row_bits), &mctl_com->cr); > @@ -471,7 +472,7 @@ static int mctl_channel_init(uint16_t socid, struct dram_para *para) > } > > /* set half DQ */ > - if (para->bus_width != 32) { > + if (!para->bus_full_width) { > writel(0x0, &mctl_ctl->dx[2].gcr); > writel(0x0, &mctl_ctl->dx[3].gcr); This is not correct, it still disables byte 2 and 3, which don't even exist on 16bit bus devices. On a 16bit device dx[1] would have do be disabled for half width. > } > @@ -509,7 +510,7 @@ static int mctl_channel_init(uint16_t socid, struct dram_para *para) > ((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; > + para->bus_full_width = 0; Same here, it only detects that byte 2 and 3 are missing, to detect half width on 16bit devices byte 1 would have to be checked in the if above. Also, the rank detection above must not check byte 1 on 16bit devices. > } > > mctl_set_cr(para); > @@ -613,7 +614,7 @@ unsigned long sunxi_dram_init(void) > > struct dram_para para = { > .dual_rank = 0, > - .bus_width = 32, > + .bus_full_width = 1, > .row_bits = 15, > .page_size = 4096, > >
12.02.2017, 01:00, "Jens Kuske" <jenskuske@gmail.com>: > Hi, > > renaming is not quite enough, see the comments below. > > On 11.02.2017 16:08, Icenowy Zheng wrote: >> The DesignWare DRAM controller used by H3 and newer SoCs use a bit to >> identify whether the DRAM is half-width. >> >> As H3 itself come with 32-bit DRAM, the two modes of the bit used to be >> named "MCTL_CR_32BIT" and "MCTL_CR_16BIT", but for SoCs with 16-bit DRAM >> they're really 8-bit and 16-bit. >> >> Rename the bit's macro, and also rename the variable name in >> dram_sun8i_h3.c. >> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> >> --- >> arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h | 6 +++--- >> arch/arm/mach-sunxi/dram_sunxi_dw.c | 11 ++++++----- >> 2 files changed, 9 insertions(+), 8 deletions(-) >> >> 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 25d07d9863..48bd6f7c0f 100644 >> --- a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h >> +++ b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h >> @@ -52,9 +52,9 @@ struct sunxi_mctl_com_reg { >> #define MCTL_CR_SEQUENTIAL (0x1 << 15) >> #define MCTL_CR_INTERLEAVED (0x0 << 15) >> >> -#define MCTL_CR_32BIT (0x1 << 12) >> -#define MCTL_CR_16BIT (0x0 << 12) >> -#define MCTL_CR_BUS_WIDTH(x) ((x) == 32 ? MCTL_CR_32BIT : MCTL_CR_16BIT) >> +#define MCTL_CR_FULL_WIDTH (0x1 << 12) >> +#define MCTL_CR_HALF_WIDTH (0x0 << 12) >> +#define MCTL_CR_BUS_FULL_WIDTH(x) ((x) << 12) >> >> #define MCTL_CR_PAGE_SIZE(x) ((fls(x) - 4) << 8) >> #define MCTL_CR_ROW_BITS(x) (((x) - 1) << 4) >> diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c b/arch/arm/mach-sunxi/dram_sunxi_dw.c >> index 9f7cc7fd4c..0c73a43075 100644 >> --- a/arch/arm/mach-sunxi/dram_sunxi_dw.c >> +++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c >> @@ -28,7 +28,7 @@ >> #define LINES_PER_BYTE_LANE (BITS_PER_BYTE + 3) >> struct dram_para { >> u16 page_size; >> - u8 bus_width; >> + u8 bus_full_width; >> u8 dual_rank; >> u8 row_bits; >> const u8 dx_read_delays[NR_OF_BYTE_LANES][LINES_PER_BYTE_LANE]; >> @@ -358,7 +358,8 @@ static void mctl_set_cr(struct dram_para *para) >> (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE; >> >> writel(MCTL_CR_BL8 | MCTL_CR_2T | MCTL_CR_DDR3 | MCTL_CR_INTERLEAVED | >> - MCTL_CR_EIGHT_BANKS | MCTL_CR_BUS_WIDTH(para->bus_width) | >> + MCTL_CR_EIGHT_BANKS | >> + MCTL_CR_BUS_FULL_WIDTH(para->bus_full_width) | >> (para->dual_rank ? MCTL_CR_DUAL_RANK : MCTL_CR_SINGLE_RANK) | >> MCTL_CR_PAGE_SIZE(para->page_size) | >> MCTL_CR_ROW_BITS(para->row_bits), &mctl_com->cr); >> @@ -471,7 +472,7 @@ static int mctl_channel_init(uint16_t socid, struct dram_para *para) >> } >> >> /* set half DQ */ >> - if (para->bus_width != 32) { >> + if (!para->bus_full_width) { >> writel(0x0, &mctl_ctl->dx[2].gcr); >> writel(0x0, &mctl_ctl->dx[3].gcr); > > This is not correct, it still disables byte 2 and 3, which don't even > exist on 16bit bus devices. On a 16bit device dx[1] would have do be > disabled for half width. Yes, verified on dram_sun8i_a33.c . > >> } >> @@ -509,7 +510,7 @@ static int mctl_channel_init(uint16_t socid, struct dram_para *para) >> ((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; >> + para->bus_full_width = 0; > > Same here, it only detects that byte 2 and 3 are missing, to detect half > width on 16bit devices byte 1 would have to be checked in the if above. > Also, the rank detection above must not check byte 1 on 16bit devices. > >> } >> >> mctl_set_cr(para); >> @@ -613,7 +614,7 @@ unsigned long sunxi_dram_init(void) >> >> struct dram_para para = { >> .dual_rank = 0, >> - .bus_width = 32, >> + .bus_full_width = 1, >> .row_bits = 15, >> .page_size = 4096, > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
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 25d07d9863..48bd6f7c0f 100644 --- a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h +++ b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h @@ -52,9 +52,9 @@ struct sunxi_mctl_com_reg { #define MCTL_CR_SEQUENTIAL (0x1 << 15) #define MCTL_CR_INTERLEAVED (0x0 << 15) -#define MCTL_CR_32BIT (0x1 << 12) -#define MCTL_CR_16BIT (0x0 << 12) -#define MCTL_CR_BUS_WIDTH(x) ((x) == 32 ? MCTL_CR_32BIT : MCTL_CR_16BIT) +#define MCTL_CR_FULL_WIDTH (0x1 << 12) +#define MCTL_CR_HALF_WIDTH (0x0 << 12) +#define MCTL_CR_BUS_FULL_WIDTH(x) ((x) << 12) #define MCTL_CR_PAGE_SIZE(x) ((fls(x) - 4) << 8) #define MCTL_CR_ROW_BITS(x) (((x) - 1) << 4) diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c b/arch/arm/mach-sunxi/dram_sunxi_dw.c index 9f7cc7fd4c..0c73a43075 100644 --- a/arch/arm/mach-sunxi/dram_sunxi_dw.c +++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c @@ -28,7 +28,7 @@ #define LINES_PER_BYTE_LANE (BITS_PER_BYTE + 3) struct dram_para { u16 page_size; - u8 bus_width; + u8 bus_full_width; u8 dual_rank; u8 row_bits; const u8 dx_read_delays[NR_OF_BYTE_LANES][LINES_PER_BYTE_LANE]; @@ -358,7 +358,8 @@ static void mctl_set_cr(struct dram_para *para) (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE; writel(MCTL_CR_BL8 | MCTL_CR_2T | MCTL_CR_DDR3 | MCTL_CR_INTERLEAVED | - MCTL_CR_EIGHT_BANKS | MCTL_CR_BUS_WIDTH(para->bus_width) | + MCTL_CR_EIGHT_BANKS | + MCTL_CR_BUS_FULL_WIDTH(para->bus_full_width) | (para->dual_rank ? MCTL_CR_DUAL_RANK : MCTL_CR_SINGLE_RANK) | MCTL_CR_PAGE_SIZE(para->page_size) | MCTL_CR_ROW_BITS(para->row_bits), &mctl_com->cr); @@ -471,7 +472,7 @@ static int mctl_channel_init(uint16_t socid, struct dram_para *para) } /* set half DQ */ - if (para->bus_width != 32) { + if (!para->bus_full_width) { writel(0x0, &mctl_ctl->dx[2].gcr); writel(0x0, &mctl_ctl->dx[3].gcr); } @@ -509,7 +510,7 @@ static int mctl_channel_init(uint16_t socid, struct dram_para *para) ((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; + para->bus_full_width = 0; } mctl_set_cr(para); @@ -613,7 +614,7 @@ unsigned long sunxi_dram_init(void) struct dram_para para = { .dual_rank = 0, - .bus_width = 32, + .bus_full_width = 1, .row_bits = 15, .page_size = 4096,
The DesignWare DRAM controller used by H3 and newer SoCs use a bit to identify whether the DRAM is half-width. As H3 itself come with 32-bit DRAM, the two modes of the bit used to be named "MCTL_CR_32BIT" and "MCTL_CR_16BIT", but for SoCs with 16-bit DRAM they're really 8-bit and 16-bit. Rename the bit's macro, and also rename the variable name in dram_sun8i_h3.c. Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> --- arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h | 6 +++--- arch/arm/mach-sunxi/dram_sunxi_dw.c | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-)