[U-Boot] [PATCH] rockchip: rk3288: correct sdram setting
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com ---
arch/arm/include/asm/arch-rockchip/ddr_rk3288.h | 42 ++++++++--------- arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 61 +++++++++++-------------- 2 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h index fccabcd..f2e3130 100644 --- a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h +++ b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h @@ -459,26 +459,26 @@ enum { * [3:2] bw_ch0 * [1:0] dbw_ch0 */ -#define SYS_REG_DDRTYPE_SHIFT 13 -#define SYS_REG_DDRTYPE_MASK 7 -#define SYS_REG_NUM_CH_SHIFT 12 -#define SYS_REG_NUM_CH_MASK 1 -#define SYS_REG_ROW_3_4_SHIFT(ch) (30 + (ch)) -#define SYS_REG_ROW_3_4_MASK 1 -#define SYS_REG_CHINFO_SHIFT(ch) (28 + (ch)) -#define SYS_REG_RANK_SHIFT(ch) (11 + (ch) * 16) -#define SYS_REG_RANK_MASK 1 -#define SYS_REG_COL_SHIFT(ch) (9 + (ch) * 16) -#define SYS_REG_COL_MASK 3 -#define SYS_REG_BK_SHIFT(ch) (8 + (ch) * 16) -#define SYS_REG_BK_MASK 1 -#define SYS_REG_CS0_ROW_SHIFT(ch) (6 + (ch) * 16) -#define SYS_REG_CS0_ROW_MASK 3 -#define SYS_REG_CS1_ROW_SHIFT(ch) (4 + (ch) * 16) -#define SYS_REG_CS1_ROW_MASK 3 -#define SYS_REG_BW_SHIFT(ch) (2 + (ch) * 16) -#define SYS_REG_BW_MASK 3 -#define SYS_REG_DBW_SHIFT(ch) ((ch) * 16) -#define SYS_REG_DBW_MASK 3 +#define SYS_REG_ENC_ROW_3_4(n, ch) ((n) << (30 + (ch))) +#define SYS_REG_DEC_ROW_3_4(n, ch) ((n >> (30 + ch)) & 0x1) +#define SYS_REG_ENC_CHINFO(ch) (1 << (28 + (ch))) +#define SYS_REG_ENC_DDRTYPE(n) ((n) << 13) +#define SYS_REG_ENC_NUM_CH(n) (((n) - 1) << 12) +#define SYS_REG_DEC_NUM_CH(n) (1 + ((n >> 12) & 0x1)) +#define SYS_REG_ENC_RANK(n, ch) (((n) - 1) << (11 + ((ch) * 16))) +#define SYS_REG_DEC_RANK(n, ch) (1 + ((n >> (11 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_COL(n, ch) (((n) - 9) << (9 + ((ch) * 16))) +#define SYS_REG_DEC_COL(n, ch) (9 + ((n >> (9 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BK(n, ch) (((n) == 3 ? 0 : 1) \ + << (8 + ((ch) * 16))) +#define SYS_REG_DEC_BK(n, ch) (3 - ((n >> (8 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_CS0_ROW(n, ch) (((n) - 13) << (6 + ((ch) * 16))) +#define SYS_REG_DEC_CS0_ROW(n, ch) (13 + ((n >> (6 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_CS1_ROW(n, ch) (((n) - 13) << (4 + ((ch) * 16))) +#define SYS_REG_DEC_CS1_ROW(n, ch) (13 + ((n >> (4 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BW(n, ch) ((2 >> (n)) << (2 + ((ch) * 16))) +#define SYS_REG_DEC_BW(n, ch) (2 >> ((n >> (2 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_DBW(n, ch) ((2 >> (n)) << (0 + ((ch) * 16))) +#define SYS_REG_DEC_DBW(n, ch) (2 >> ((n >> (0 + 16 * ch)) & 0x3))
#endif diff --git a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c index e9e2211..4db39ec 100644 --- a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c @@ -551,27 +551,27 @@ static void dram_cfg_rbc(const struct chan_info *chan, u32 chnum, static void dram_all_config(const struct dram_info *dram, const struct rk3288_sdram_params *sdram_params) { - unsigned int chan; + unsigned int channel; u32 sys_reg = 0;
- sys_reg |= sdram_params->base.dramtype << SYS_REG_DDRTYPE_SHIFT; - sys_reg |= (sdram_params->num_channels - 1) << SYS_REG_NUM_CH_SHIFT; - for (chan = 0; chan < sdram_params->num_channels; chan++) { + sys_reg |= SYS_REG_ENC_DDRTYPE(sdram_params->base.dramtype); + sys_reg |= SYS_REG_ENC_NUM_CH(sdram_params->num_channels); + for (channel = 0; channel < sdram_params->num_channels; channel++) { const struct rk3288_sdram_channel *info = - &sdram_params->ch[chan]; - - sys_reg |= info->row_3_4 << SYS_REG_ROW_3_4_SHIFT(chan); - sys_reg |= chan << SYS_REG_CHINFO_SHIFT(chan); - sys_reg |= (info->rank - 1) << SYS_REG_RANK_SHIFT(chan); - sys_reg |= (info->col - 9) << SYS_REG_COL_SHIFT(chan); - sys_reg |= info->bk == 3 ? 1 << SYS_REG_BK_SHIFT(chan) : 0; - sys_reg |= (info->cs0_row - 13) << SYS_REG_CS0_ROW_SHIFT(chan); - sys_reg |= (info->cs1_row - 13) << SYS_REG_CS1_ROW_SHIFT(chan); - sys_reg |= info->bw << SYS_REG_BW_SHIFT(chan); - sys_reg |= info->dbw << SYS_REG_DBW_SHIFT(chan); - - dram_cfg_rbc(&dram->chan[chan], chan, sdram_params); + &(sdram_params->ch[channel]); + sys_reg |= SYS_REG_ENC_ROW_3_4(info->row_3_4, channel); + sys_reg |= SYS_REG_ENC_CHINFO(channel); + sys_reg |= SYS_REG_ENC_RANK(info->rank, channel); + sys_reg |= SYS_REG_ENC_COL(info->col, channel); + sys_reg |= SYS_REG_ENC_BK(info->bk, channel); + sys_reg |= SYS_REG_ENC_CS0_ROW(info->cs0_row, channel); + sys_reg |= SYS_REG_ENC_CS1_ROW(info->cs1_row, channel); + sys_reg |= SYS_REG_ENC_BW(info->bw, channel); + sys_reg |= SYS_REG_ENC_DBW(info->dbw, channel); + + dram_cfg_rbc(&dram->chan[channel], channel, sdram_params); } + writel(sys_reg, &dram->pmu->sys_reg[2]); rk_clrsetreg(&dram->sgrf->soc_con2, 0x1f, sdram_params->base.stride); } @@ -712,23 +712,16 @@ size_t sdram_size_mb(struct rk3288_pmu *pmu) size_t size_mb = 0; u32 ch; u32 sys_reg = readl(&pmu->sys_reg[2]); - u32 chans; - - chans = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT) & SYS_REG_NUM_CH_MASK); - - for (ch = 0; ch < chans; ch++) { - rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) & - SYS_REG_RANK_MASK); - col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK); - bk = sys_reg & (1 << SYS_REG_BK_SHIFT(ch)) ? 3 : 0; - cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) & - SYS_REG_CS0_ROW_MASK); - cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) & - SYS_REG_CS1_ROW_MASK); - bw = (sys_reg >> SYS_REG_BW_SHIFT(ch)) & - SYS_REG_BW_MASK; - row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) & - SYS_REG_ROW_3_4_MASK; + u32 ch_num = SYS_REG_DEC_NUM_CH(sys_reg); + + for (ch = 0; ch < ch_num; ch++) { + rank = SYS_REG_DEC_RANK(sys_reg, ch); + col = SYS_REG_DEC_COL(sys_reg, ch); + bk = SYS_REG_DEC_BK(sys_reg, ch); + cs0_row = SYS_REG_DEC_CS0_ROW(sys_reg, ch); + cs1_row = SYS_REG_DEC_CS1_ROW(sys_reg, ch); + bw = SYS_REG_DEC_BW(sys_reg, ch); + row_3_4 = SYS_REG_DEC_ROW_3_4(sys_reg, ch);
chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
Hi Chris,
On 29 February 2016 at 05:16, Chris Zhong zyw@rock-chips.com wrote:
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com
arch/arm/include/asm/arch-rockchip/ddr_rk3288.h | 42 ++++++++--------- arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 61 +++++++++++-------------- 2 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h index fccabcd..f2e3130 100644 --- a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h +++ b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h @@ -459,26 +459,26 @@ enum {
- [3:2] bw_ch0
- [1:0] dbw_ch0
*/ -#define SYS_REG_DDRTYPE_SHIFT 13 -#define SYS_REG_DDRTYPE_MASK 7 -#define SYS_REG_NUM_CH_SHIFT 12 -#define SYS_REG_NUM_CH_MASK 1 -#define SYS_REG_ROW_3_4_SHIFT(ch) (30 + (ch)) -#define SYS_REG_ROW_3_4_MASK 1 -#define SYS_REG_CHINFO_SHIFT(ch) (28 + (ch)) -#define SYS_REG_RANK_SHIFT(ch) (11 + (ch) * 16) -#define SYS_REG_RANK_MASK 1 -#define SYS_REG_COL_SHIFT(ch) (9 + (ch) * 16) -#define SYS_REG_COL_MASK 3 -#define SYS_REG_BK_SHIFT(ch) (8 + (ch) * 16) -#define SYS_REG_BK_MASK 1 -#define SYS_REG_CS0_ROW_SHIFT(ch) (6 + (ch) * 16) -#define SYS_REG_CS0_ROW_MASK 3 -#define SYS_REG_CS1_ROW_SHIFT(ch) (4 + (ch) * 16) -#define SYS_REG_CS1_ROW_MASK 3 -#define SYS_REG_BW_SHIFT(ch) (2 + (ch) * 16) -#define SYS_REG_BW_MASK 3 -#define SYS_REG_DBW_SHIFT(ch) ((ch) * 16) -#define SYS_REG_DBW_MASK 3 +#define SYS_REG_ENC_ROW_3_4(n, ch) ((n) << (30 + (ch))) +#define SYS_REG_DEC_ROW_3_4(n, ch) ((n >> (30 + ch)) & 0x1) +#define SYS_REG_ENC_CHINFO(ch) (1 << (28 + (ch))) +#define SYS_REG_ENC_DDRTYPE(n) ((n) << 13) +#define SYS_REG_ENC_NUM_CH(n) (((n) - 1) << 12) +#define SYS_REG_DEC_NUM_CH(n) (1 + ((n >> 12) & 0x1)) +#define SYS_REG_ENC_RANK(n, ch) (((n) - 1) << (11 + ((ch) * 16))) +#define SYS_REG_DEC_RANK(n, ch) (1 + ((n >> (11 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_COL(n, ch) (((n) - 9) << (9 + ((ch) * 16))) +#define SYS_REG_DEC_COL(n, ch) (9 + ((n >> (9 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BK(n, ch) (((n) == 3 ? 0 : 1) \
<< (8 + ((ch) * 16)))+#define SYS_REG_DEC_BK(n, ch) (3 - ((n >> (8 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_CS0_ROW(n, ch) (((n) - 13) << (6 + ((ch) * 16))) +#define SYS_REG_DEC_CS0_ROW(n, ch) (13 + ((n >> (6 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_CS1_ROW(n, ch) (((n) - 13) << (4 + ((ch) * 16))) +#define SYS_REG_DEC_CS1_ROW(n, ch) (13 + ((n >> (4 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BW(n, ch) ((2 >> (n)) << (2 + ((ch) * 16))) +#define SYS_REG_DEC_BW(n, ch) (2 >> ((n >> (2 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_DBW(n, ch) ((2 >> (n)) << (0 + ((ch) * 16))) +#define SYS_REG_DEC_DBW(n, ch) (2 >> ((n >> (0 + 16 * ch)) & 0x3))
Are the above shift/masks actually wrong? I'm not keen on this style of packing and unpacking registers since it is really hard to read and it's not obvious that pack and unpack work the same way.
#endif diff --git a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c index e9e2211..4db39ec 100644 --- a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c @@ -551,27 +551,27 @@ static void dram_cfg_rbc(const struct chan_info *chan, u32 chnum, static void dram_all_config(const struct dram_info *dram, const struct rk3288_sdram_params *sdram_params) {
unsigned int chan;
unsigned int channel; u32 sys_reg = 0;
sys_reg |= sdram_params->base.dramtype << SYS_REG_DDRTYPE_SHIFT;sys_reg |= (sdram_params->num_channels - 1) << SYS_REG_NUM_CH_SHIFT;for (chan = 0; chan < sdram_params->num_channels; chan++) {
sys_reg |= SYS_REG_ENC_DDRTYPE(sdram_params->base.dramtype);sys_reg |= SYS_REG_ENC_NUM_CH(sdram_params->num_channels);for (channel = 0; channel < sdram_params->num_channels; channel++) { const struct rk3288_sdram_channel *info =
&sdram_params->ch[chan];sys_reg |= info->row_3_4 << SYS_REG_ROW_3_4_SHIFT(chan);sys_reg |= chan << SYS_REG_CHINFO_SHIFT(chan);sys_reg |= (info->rank - 1) << SYS_REG_RANK_SHIFT(chan);sys_reg |= (info->col - 9) << SYS_REG_COL_SHIFT(chan);sys_reg |= info->bk == 3 ? 1 << SYS_REG_BK_SHIFT(chan) : 0;sys_reg |= (info->cs0_row - 13) << SYS_REG_CS0_ROW_SHIFT(chan);sys_reg |= (info->cs1_row - 13) << SYS_REG_CS1_ROW_SHIFT(chan);sys_reg |= info->bw << SYS_REG_BW_SHIFT(chan);sys_reg |= info->dbw << SYS_REG_DBW_SHIFT(chan);dram_cfg_rbc(&dram->chan[chan], chan, sdram_params);
&(sdram_params->ch[channel]);sys_reg |= SYS_REG_ENC_ROW_3_4(info->row_3_4, channel);sys_reg |= SYS_REG_ENC_CHINFO(channel);sys_reg |= SYS_REG_ENC_RANK(info->rank, channel);sys_reg |= SYS_REG_ENC_COL(info->col, channel);sys_reg |= SYS_REG_ENC_BK(info->bk, channel);sys_reg |= SYS_REG_ENC_CS0_ROW(info->cs0_row, channel);sys_reg |= SYS_REG_ENC_CS1_ROW(info->cs1_row, channel);sys_reg |= SYS_REG_ENC_BW(info->bw, channel);sys_reg |= SYS_REG_ENC_DBW(info->dbw, channel);
Can you fix this bug while still using the original #defines?
dram_cfg_rbc(&dram->chan[channel], channel, sdram_params); }writel(sys_reg, &dram->pmu->sys_reg[2]); rk_clrsetreg(&dram->sgrf->soc_con2, 0x1f, sdram_params->base.stride);} @@ -712,23 +712,16 @@ size_t sdram_size_mb(struct rk3288_pmu *pmu) size_t size_mb = 0; u32 ch; u32 sys_reg = readl(&pmu->sys_reg[2]);
u32 chans;chans = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT) & SYS_REG_NUM_CH_MASK);for (ch = 0; ch < chans; ch++) {rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &SYS_REG_RANK_MASK);col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);bk = sys_reg & (1 << SYS_REG_BK_SHIFT(ch)) ? 3 : 0;cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &SYS_REG_CS0_ROW_MASK);cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &SYS_REG_CS1_ROW_MASK);bw = (sys_reg >> SYS_REG_BW_SHIFT(ch)) &SYS_REG_BW_MASK;row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &SYS_REG_ROW_3_4_MASK;
u32 ch_num = SYS_REG_DEC_NUM_CH(sys_reg);for (ch = 0; ch < ch_num; ch++) {rank = SYS_REG_DEC_RANK(sys_reg, ch);col = SYS_REG_DEC_COL(sys_reg, ch);bk = SYS_REG_DEC_BK(sys_reg, ch);cs0_row = SYS_REG_DEC_CS0_ROW(sys_reg, ch);cs1_row = SYS_REG_DEC_CS1_ROW(sys_reg, ch);bw = SYS_REG_DEC_BW(sys_reg, ch);row_3_4 = SYS_REG_DEC_ROW_3_4(sys_reg, ch); chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));-- 2.6.3
Regards, Simon
Hi Simon
On 03/01/2016 10:04 AM, Simon Glass wrote:
Hi Chris,
On 29 February 2016 at 05:16, Chris Zhong zyw@rock-chips.com wrote:
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com
arch/arm/include/asm/arch-rockchip/ddr_rk3288.h | 42 ++++++++--------- arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 61 +++++++++++-------------- 2 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h index fccabcd..f2e3130 100644 --- a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h +++ b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h @@ -459,26 +459,26 @@ enum {
- [3:2] bw_ch0
- [1:0] dbw_ch0
*/ -#define SYS_REG_DDRTYPE_SHIFT 13 -#define SYS_REG_DDRTYPE_MASK 7 -#define SYS_REG_NUM_CH_SHIFT 12 -#define SYS_REG_NUM_CH_MASK 1 -#define SYS_REG_ROW_3_4_SHIFT(ch) (30 + (ch)) -#define SYS_REG_ROW_3_4_MASK 1 -#define SYS_REG_CHINFO_SHIFT(ch) (28 + (ch)) -#define SYS_REG_RANK_SHIFT(ch) (11 + (ch) * 16) -#define SYS_REG_RANK_MASK 1 -#define SYS_REG_COL_SHIFT(ch) (9 + (ch) * 16) -#define SYS_REG_COL_MASK 3 -#define SYS_REG_BK_SHIFT(ch) (8 + (ch) * 16) -#define SYS_REG_BK_MASK 1 -#define SYS_REG_CS0_ROW_SHIFT(ch) (6 + (ch) * 16) -#define SYS_REG_CS0_ROW_MASK 3 -#define SYS_REG_CS1_ROW_SHIFT(ch) (4 + (ch) * 16) -#define SYS_REG_CS1_ROW_MASK 3 -#define SYS_REG_BW_SHIFT(ch) (2 + (ch) * 16) -#define SYS_REG_BW_MASK 3 -#define SYS_REG_DBW_SHIFT(ch) ((ch) * 16) -#define SYS_REG_DBW_MASK 3 +#define SYS_REG_ENC_ROW_3_4(n, ch) ((n) << (30 + (ch))) +#define SYS_REG_DEC_ROW_3_4(n, ch) ((n >> (30 + ch)) & 0x1) +#define SYS_REG_ENC_CHINFO(ch) (1 << (28 + (ch))) +#define SYS_REG_ENC_DDRTYPE(n) ((n) << 13) +#define SYS_REG_ENC_NUM_CH(n) (((n) - 1) << 12) +#define SYS_REG_DEC_NUM_CH(n) (1 + ((n >> 12) & 0x1)) +#define SYS_REG_ENC_RANK(n, ch) (((n) - 1) << (11 + ((ch) * 16))) +#define SYS_REG_DEC_RANK(n, ch) (1 + ((n >> (11 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_COL(n, ch) (((n) - 9) << (9 + ((ch) * 16))) +#define SYS_REG_DEC_COL(n, ch) (9 + ((n >> (9 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BK(n, ch) (((n) == 3 ? 0 : 1) \
<< (8 + ((ch) * 16)))+#define SYS_REG_DEC_BK(n, ch) (3 - ((n >> (8 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_CS0_ROW(n, ch) (((n) - 13) << (6 + ((ch) * 16))) +#define SYS_REG_DEC_CS0_ROW(n, ch) (13 + ((n >> (6 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_CS1_ROW(n, ch) (((n) - 13) << (4 + ((ch) * 16))) +#define SYS_REG_DEC_CS1_ROW(n, ch) (13 + ((n >> (4 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BW(n, ch) ((2 >> (n)) << (2 + ((ch) * 16))) +#define SYS_REG_DEC_BW(n, ch) (2 >> ((n >> (2 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_DBW(n, ch) ((2 >> (n)) << (0 + ((ch) * 16))) +#define SYS_REG_DEC_DBW(n, ch) (2 >> ((n >> (0 + 16 * ch)) & 0x3))
Are the above shift/masks actually wrong? I'm not keen on this style of packing and unpacking registers since it is really hard to read and it's not obvious that pack and unpack work the same way.
Actually, I copy these code from coreboot[0], can we just keep the code consistent with it?
[0]https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/ firmware-veyron-6588.B/src/soc/rockchip/rk3288/sdram.c
#endif diff --git a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c index e9e2211..4db39ec 100644 --- a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c @@ -551,27 +551,27 @@ static void dram_cfg_rbc(const struct chan_info *chan, u32 chnum, static void dram_all_config(const struct dram_info *dram, const struct rk3288_sdram_params *sdram_params) {
unsigned int chan;
unsigned int channel; u32 sys_reg = 0;
sys_reg |= sdram_params->base.dramtype << SYS_REG_DDRTYPE_SHIFT;sys_reg |= (sdram_params->num_channels - 1) << SYS_REG_NUM_CH_SHIFT;for (chan = 0; chan < sdram_params->num_channels; chan++) {
sys_reg |= SYS_REG_ENC_DDRTYPE(sdram_params->base.dramtype);sys_reg |= SYS_REG_ENC_NUM_CH(sdram_params->num_channels);for (channel = 0; channel < sdram_params->num_channels; channel++) { const struct rk3288_sdram_channel *info =
&sdram_params->ch[chan];sys_reg |= info->row_3_4 << SYS_REG_ROW_3_4_SHIFT(chan);sys_reg |= chan << SYS_REG_CHINFO_SHIFT(chan);sys_reg |= (info->rank - 1) << SYS_REG_RANK_SHIFT(chan);sys_reg |= (info->col - 9) << SYS_REG_COL_SHIFT(chan);sys_reg |= info->bk == 3 ? 1 << SYS_REG_BK_SHIFT(chan) : 0;sys_reg |= (info->cs0_row - 13) << SYS_REG_CS0_ROW_SHIFT(chan);sys_reg |= (info->cs1_row - 13) << SYS_REG_CS1_ROW_SHIFT(chan);sys_reg |= info->bw << SYS_REG_BW_SHIFT(chan);sys_reg |= info->dbw << SYS_REG_DBW_SHIFT(chan);dram_cfg_rbc(&dram->chan[chan], chan, sdram_params);
&(sdram_params->ch[channel]);sys_reg |= SYS_REG_ENC_ROW_3_4(info->row_3_4, channel);sys_reg |= SYS_REG_ENC_CHINFO(channel);sys_reg |= SYS_REG_ENC_RANK(info->rank, channel);sys_reg |= SYS_REG_ENC_COL(info->col, channel);sys_reg |= SYS_REG_ENC_BK(info->bk, channel);sys_reg |= SYS_REG_ENC_CS0_ROW(info->cs0_row, channel);sys_reg |= SYS_REG_ENC_CS1_ROW(info->cs1_row, channel);sys_reg |= SYS_REG_ENC_BW(info->bw, channel);sys_reg |= SYS_REG_ENC_DBW(info->dbw, channel);Can you fix this bug while still using the original #defines?
dram_cfg_rbc(&dram->chan[channel], channel, sdram_params); }writel(sys_reg, &dram->pmu->sys_reg[2]); rk_clrsetreg(&dram->sgrf->soc_con2, 0x1f, sdram_params->base.stride);@@ -712,23 +712,16 @@ size_t sdram_size_mb(struct rk3288_pmu *pmu) size_t size_mb = 0; u32 ch; u32 sys_reg = readl(&pmu->sys_reg[2]);
u32 chans;chans = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT) & SYS_REG_NUM_CH_MASK);for (ch = 0; ch < chans; ch++) {rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &SYS_REG_RANK_MASK);col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);bk = sys_reg & (1 << SYS_REG_BK_SHIFT(ch)) ? 3 : 0;cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &SYS_REG_CS0_ROW_MASK);cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &SYS_REG_CS1_ROW_MASK);bw = (sys_reg >> SYS_REG_BW_SHIFT(ch)) &SYS_REG_BW_MASK;row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &SYS_REG_ROW_3_4_MASK;
u32 ch_num = SYS_REG_DEC_NUM_CH(sys_reg);for (ch = 0; ch < ch_num; ch++) {rank = SYS_REG_DEC_RANK(sys_reg, ch);col = SYS_REG_DEC_COL(sys_reg, ch);bk = SYS_REG_DEC_BK(sys_reg, ch);cs0_row = SYS_REG_DEC_CS0_ROW(sys_reg, ch);cs1_row = SYS_REG_DEC_CS1_ROW(sys_reg, ch);bw = SYS_REG_DEC_BW(sys_reg, ch);row_3_4 = SYS_REG_DEC_ROW_3_4(sys_reg, ch); chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));-- 2.6.3
Regards, Simon
Hi Chris,
On 29 February 2016 at 19:29, Chris Zhong zyw@rock-chips.com wrote:
Hi Simon
On 03/01/2016 10:04 AM, Simon Glass wrote:
Hi Chris,
On 29 February 2016 at 05:16, Chris Zhong zyw@rock-chips.com wrote:
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com
arch/arm/include/asm/arch-rockchip/ddr_rk3288.h | 42 ++++++++--------- arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 61 +++++++++++-------------- 2 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h index fccabcd..f2e3130 100644 --- a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h +++ b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h @@ -459,26 +459,26 @@ enum {
- [3:2] bw_ch0
- [1:0] dbw_ch0
*/ -#define SYS_REG_DDRTYPE_SHIFT 13 -#define SYS_REG_DDRTYPE_MASK 7 -#define SYS_REG_NUM_CH_SHIFT 12 -#define SYS_REG_NUM_CH_MASK 1 -#define SYS_REG_ROW_3_4_SHIFT(ch) (30 + (ch)) -#define SYS_REG_ROW_3_4_MASK 1 -#define SYS_REG_CHINFO_SHIFT(ch) (28 + (ch)) -#define SYS_REG_RANK_SHIFT(ch) (11 + (ch) * 16) -#define SYS_REG_RANK_MASK 1 -#define SYS_REG_COL_SHIFT(ch) (9 + (ch) * 16) -#define SYS_REG_COL_MASK 3 -#define SYS_REG_BK_SHIFT(ch) (8 + (ch) * 16) -#define SYS_REG_BK_MASK 1 -#define SYS_REG_CS0_ROW_SHIFT(ch) (6 + (ch) * 16) -#define SYS_REG_CS0_ROW_MASK 3 -#define SYS_REG_CS1_ROW_SHIFT(ch) (4 + (ch) * 16) -#define SYS_REG_CS1_ROW_MASK 3 -#define SYS_REG_BW_SHIFT(ch) (2 + (ch) * 16) -#define SYS_REG_BW_MASK 3 -#define SYS_REG_DBW_SHIFT(ch) ((ch) * 16) -#define SYS_REG_DBW_MASK 3 +#define SYS_REG_ENC_ROW_3_4(n, ch) ((n) << (30 + (ch))) +#define SYS_REG_DEC_ROW_3_4(n, ch) ((n >> (30 + ch)) & 0x1) +#define SYS_REG_ENC_CHINFO(ch) (1 << (28 + (ch))) +#define SYS_REG_ENC_DDRTYPE(n) ((n) << 13) +#define SYS_REG_ENC_NUM_CH(n) (((n) - 1) << 12) +#define SYS_REG_DEC_NUM_CH(n) (1 + ((n >> 12) & 0x1)) +#define SYS_REG_ENC_RANK(n, ch) (((n) - 1) << (11 + ((ch)
- 16)))
+#define SYS_REG_DEC_RANK(n, ch) (1 + ((n >> (11 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_COL(n, ch) (((n) - 9) << (9 + ((ch) * 16))) +#define SYS_REG_DEC_COL(n, ch) (9 + ((n >> (9 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BK(n, ch) (((n) == 3 ? 0 : 1) \
<< (8 + ((ch) * 16)))+#define SYS_REG_DEC_BK(n, ch) (3 - ((n >> (8 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_CS0_ROW(n, ch) (((n) - 13) << (6 + ((ch) * 16))) +#define SYS_REG_DEC_CS0_ROW(n, ch) (13 + ((n >> (6 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_CS1_ROW(n, ch) (((n) - 13) << (4 + ((ch) * 16))) +#define SYS_REG_DEC_CS1_ROW(n, ch) (13 + ((n >> (4 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BW(n, ch) ((2 >> (n)) << (2 + ((ch) * 16))) +#define SYS_REG_DEC_BW(n, ch) (2 >> ((n >> (2 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_DBW(n, ch) ((2 >> (n)) << (0 + ((ch) * 16))) +#define SYS_REG_DEC_DBW(n, ch) (2 >> ((n >> (0 + 16 * ch)) & 0x3))
Are the above shift/masks actually wrong? I'm not keen on this style of packing and unpacking registers since it is really hard to read and it's not obvious that pack and unpack work the same way.
Actually, I copy these code from coreboot[0], can we just keep the code consistent with it?
I'm really not keen on that style. It is really hard to read.
So please can you fix the bug without changing the code style?
Actually the mask I set up for rockchip is wrong also. It should be:
#define SYS_REG_DDRTYPE_SHIFT 13 #define SYS_REG_DDRTYPE_MASK (7 << SYS_REG_DDRTYPE_SHIFT)
I'll take a look at fixing these.
[0]https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/ firmware-veyron-6588.B/src/soc/rockchip/rk3288/sdram.c
#endif diff --git a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c index e9e2211..4db39ec 100644 --- a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c @@ -551,27 +551,27 @@ static void dram_cfg_rbc(const struct chan_info *chan, u32 chnum, static void dram_all_config(const struct dram_info *dram, const struct rk3288_sdram_params *sdram_params) {
unsigned int chan;
unsigned int channel; u32 sys_reg = 0;
sys_reg |= sdram_params->base.dramtype << SYS_REG_DDRTYPE_SHIFT;sys_reg |= (sdram_params->num_channels - 1) <<SYS_REG_NUM_CH_SHIFT;
for (chan = 0; chan < sdram_params->num_channels; chan++) {
sys_reg |= SYS_REG_ENC_DDRTYPE(sdram_params->base.dramtype);sys_reg |= SYS_REG_ENC_NUM_CH(sdram_params->num_channels);for (channel = 0; channel < sdram_params->num_channels;channel++) { const struct rk3288_sdram_channel *info =
&sdram_params->ch[chan];sys_reg |= info->row_3_4 << SYS_REG_ROW_3_4_SHIFT(chan);sys_reg |= chan << SYS_REG_CHINFO_SHIFT(chan);sys_reg |= (info->rank - 1) << SYS_REG_RANK_SHIFT(chan);sys_reg |= (info->col - 9) << SYS_REG_COL_SHIFT(chan);sys_reg |= info->bk == 3 ? 1 << SYS_REG_BK_SHIFT(chan) :0;
sys_reg |= (info->cs0_row - 13) <<SYS_REG_CS0_ROW_SHIFT(chan);
sys_reg |= (info->cs1_row - 13) <<SYS_REG_CS1_ROW_SHIFT(chan);
sys_reg |= info->bw << SYS_REG_BW_SHIFT(chan);sys_reg |= info->dbw << SYS_REG_DBW_SHIFT(chan);dram_cfg_rbc(&dram->chan[chan], chan, sdram_params);
&(sdram_params->ch[channel]);sys_reg |= SYS_REG_ENC_ROW_3_4(info->row_3_4, channel);sys_reg |= SYS_REG_ENC_CHINFO(channel);sys_reg |= SYS_REG_ENC_RANK(info->rank, channel);sys_reg |= SYS_REG_ENC_COL(info->col, channel);sys_reg |= SYS_REG_ENC_BK(info->bk, channel);sys_reg |= SYS_REG_ENC_CS0_ROW(info->cs0_row, channel);sys_reg |= SYS_REG_ENC_CS1_ROW(info->cs1_row, channel);sys_reg |= SYS_REG_ENC_BW(info->bw, channel);sys_reg |= SYS_REG_ENC_DBW(info->dbw, channel);Can you fix this bug while still using the original #defines?
dram_cfg_rbc(&dram->chan[channel], channel,sdram_params); }
writel(sys_reg, &dram->pmu->sys_reg[2]); rk_clrsetreg(&dram->sgrf->soc_con2, 0x1f,sdram_params->base.stride); } @@ -712,23 +712,16 @@ size_t sdram_size_mb(struct rk3288_pmu *pmu) size_t size_mb = 0; u32 ch; u32 sys_reg = readl(&pmu->sys_reg[2]);
u32 chans;chans = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT) &SYS_REG_NUM_CH_MASK);
for (ch = 0; ch < chans; ch++) {rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &SYS_REG_RANK_MASK);col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) &SYS_REG_COL_MASK);
bk = sys_reg & (1 << SYS_REG_BK_SHIFT(ch)) ? 3 : 0;cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &SYS_REG_CS0_ROW_MASK);cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &SYS_REG_CS1_ROW_MASK);bw = (sys_reg >> SYS_REG_BW_SHIFT(ch)) &SYS_REG_BW_MASK;row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &SYS_REG_ROW_3_4_MASK;
u32 ch_num = SYS_REG_DEC_NUM_CH(sys_reg);for (ch = 0; ch < ch_num; ch++) {rank = SYS_REG_DEC_RANK(sys_reg, ch);col = SYS_REG_DEC_COL(sys_reg, ch);bk = SYS_REG_DEC_BK(sys_reg, ch);cs0_row = SYS_REG_DEC_CS0_ROW(sys_reg, ch);cs1_row = SYS_REG_DEC_CS1_ROW(sys_reg, ch);bw = SYS_REG_DEC_BW(sys_reg, ch);row_3_4 = SYS_REG_DEC_ROW_3_4(sys_reg, ch); chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));-- 2.6.3
Regards, Simon
Regards, Simon
Hi Simon
On 2016年03月07日 10:39, Simon Glass wrote:
Hi Chris,
On 29 February 2016 at 19:29, Chris Zhong zyw@rock-chips.com wrote:
Hi Simon
On 03/01/2016 10:04 AM, Simon Glass wrote:
Hi Chris,
On 29 February 2016 at 05:16, Chris Zhong zyw@rock-chips.com wrote:
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com
arch/arm/include/asm/arch-rockchip/ddr_rk3288.h | 42 ++++++++--------- arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 61 +++++++++++-------------- 2 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h index fccabcd..f2e3130 100644 --- a/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h +++ b/arch/arm/include/asm/arch-rockchip/ddr_rk3288.h @@ -459,26 +459,26 @@ enum { * [3:2] bw_ch0 * [1:0] dbw_ch0 */ -#define SYS_REG_DDRTYPE_SHIFT 13 -#define SYS_REG_DDRTYPE_MASK 7 -#define SYS_REG_NUM_CH_SHIFT 12 -#define SYS_REG_NUM_CH_MASK 1 -#define SYS_REG_ROW_3_4_SHIFT(ch) (30 + (ch)) -#define SYS_REG_ROW_3_4_MASK 1 -#define SYS_REG_CHINFO_SHIFT(ch) (28 + (ch)) -#define SYS_REG_RANK_SHIFT(ch) (11 + (ch) * 16) -#define SYS_REG_RANK_MASK 1 -#define SYS_REG_COL_SHIFT(ch) (9 + (ch) * 16) -#define SYS_REG_COL_MASK 3 -#define SYS_REG_BK_SHIFT(ch) (8 + (ch) * 16) -#define SYS_REG_BK_MASK 1 -#define SYS_REG_CS0_ROW_SHIFT(ch) (6 + (ch) * 16) -#define SYS_REG_CS0_ROW_MASK 3 -#define SYS_REG_CS1_ROW_SHIFT(ch) (4 + (ch) * 16) -#define SYS_REG_CS1_ROW_MASK 3 -#define SYS_REG_BW_SHIFT(ch) (2 + (ch) * 16) -#define SYS_REG_BW_MASK 3 -#define SYS_REG_DBW_SHIFT(ch) ((ch) * 16) -#define SYS_REG_DBW_MASK 3 +#define SYS_REG_ENC_ROW_3_4(n, ch) ((n) << (30 + (ch))) +#define SYS_REG_DEC_ROW_3_4(n, ch) ((n >> (30 + ch)) & 0x1) +#define SYS_REG_ENC_CHINFO(ch) (1 << (28 + (ch))) +#define SYS_REG_ENC_DDRTYPE(n) ((n) << 13) +#define SYS_REG_ENC_NUM_CH(n) (((n) - 1) << 12) +#define SYS_REG_DEC_NUM_CH(n) (1 + ((n >> 12) & 0x1)) +#define SYS_REG_ENC_RANK(n, ch) (((n) - 1) << (11 + ((ch)
- 16)))
+#define SYS_REG_DEC_RANK(n, ch) (1 + ((n >> (11 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_COL(n, ch) (((n) - 9) << (9 + ((ch) * 16))) +#define SYS_REG_DEC_COL(n, ch) (9 + ((n >> (9 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BK(n, ch) (((n) == 3 ? 0 : 1) \
<< (8 + ((ch) * 16)))+#define SYS_REG_DEC_BK(n, ch) (3 - ((n >> (8 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_CS0_ROW(n, ch) (((n) - 13) << (6 + ((ch) * 16))) +#define SYS_REG_DEC_CS0_ROW(n, ch) (13 + ((n >> (6 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_CS1_ROW(n, ch) (((n) - 13) << (4 + ((ch) * 16))) +#define SYS_REG_DEC_CS1_ROW(n, ch) (13 + ((n >> (4 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BW(n, ch) ((2 >> (n)) << (2 + ((ch) * 16))) +#define SYS_REG_DEC_BW(n, ch) (2 >> ((n >> (2 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_DBW(n, ch) ((2 >> (n)) << (0 + ((ch) * 16))) +#define SYS_REG_DEC_DBW(n, ch) (2 >> ((n >> (0 + 16 * ch)) & 0x3))
Are the above shift/masks actually wrong? I'm not keen on this style of packing and unpacking registers since it is really hard to read and it's not obvious that pack and unpack work the same way.
Actually, I copy these code from coreboot[0], can we just keep the code consistent with it?
I'm really not keen on that style. It is really hard to read.
So please can you fix the bug without changing the code style?
Actually the mask I set up for rockchip is wrong also. It should be:
#define SYS_REG_DDRTYPE_SHIFT 13 #define SYS_REG_DDRTYPE_MASK (7 << SYS_REG_DDRTYPE_SHIFT)
I'll take a look at fixing these.
Okay, I'll send a new patch today using a better style, read the definition is not easy.
[0]https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/ firmware-veyron-6588.B/src/soc/rockchip/rk3288/sdram.c
#endif diff --git a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c index e9e2211..4db39ec 100644 --- a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c @@ -551,27 +551,27 @@ static void dram_cfg_rbc(const struct chan_info *chan, u32 chnum, static void dram_all_config(const struct dram_info *dram, const struct rk3288_sdram_params *sdram_params) {
unsigned int chan;
unsigned int channel; u32 sys_reg = 0;
sys_reg |= sdram_params->base.dramtype << SYS_REG_DDRTYPE_SHIFT;sys_reg |= (sdram_params->num_channels - 1) <<SYS_REG_NUM_CH_SHIFT;
for (chan = 0; chan < sdram_params->num_channels; chan++) {
sys_reg |= SYS_REG_ENC_DDRTYPE(sdram_params->base.dramtype);sys_reg |= SYS_REG_ENC_NUM_CH(sdram_params->num_channels);for (channel = 0; channel < sdram_params->num_channels;channel++) { const struct rk3288_sdram_channel *info =
&sdram_params->ch[chan];sys_reg |= info->row_3_4 << SYS_REG_ROW_3_4_SHIFT(chan);sys_reg |= chan << SYS_REG_CHINFO_SHIFT(chan);sys_reg |= (info->rank - 1) << SYS_REG_RANK_SHIFT(chan);sys_reg |= (info->col - 9) << SYS_REG_COL_SHIFT(chan);sys_reg |= info->bk == 3 ? 1 << SYS_REG_BK_SHIFT(chan) :0;
sys_reg |= (info->cs0_row - 13) <<SYS_REG_CS0_ROW_SHIFT(chan);
sys_reg |= (info->cs1_row - 13) <<SYS_REG_CS1_ROW_SHIFT(chan);
sys_reg |= info->bw << SYS_REG_BW_SHIFT(chan);sys_reg |= info->dbw << SYS_REG_DBW_SHIFT(chan);dram_cfg_rbc(&dram->chan[chan], chan, sdram_params);
&(sdram_params->ch[channel]);sys_reg |= SYS_REG_ENC_ROW_3_4(info->row_3_4, channel);sys_reg |= SYS_REG_ENC_CHINFO(channel);sys_reg |= SYS_REG_ENC_RANK(info->rank, channel);sys_reg |= SYS_REG_ENC_COL(info->col, channel);sys_reg |= SYS_REG_ENC_BK(info->bk, channel);sys_reg |= SYS_REG_ENC_CS0_ROW(info->cs0_row, channel);sys_reg |= SYS_REG_ENC_CS1_ROW(info->cs1_row, channel);sys_reg |= SYS_REG_ENC_BW(info->bw, channel);sys_reg |= SYS_REG_ENC_DBW(info->dbw, channel);Can you fix this bug while still using the original #defines?
dram_cfg_rbc(&dram->chan[channel], channel,sdram_params); }
writel(sys_reg, &dram->pmu->sys_reg[2]); rk_clrsetreg(&dram->sgrf->soc_con2, 0x1f,sdram_params->base.stride); } @@ -712,23 +712,16 @@ size_t sdram_size_mb(struct rk3288_pmu *pmu) size_t size_mb = 0; u32 ch; u32 sys_reg = readl(&pmu->sys_reg[2]);
u32 chans;chans = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT) &SYS_REG_NUM_CH_MASK);
for (ch = 0; ch < chans; ch++) {rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &SYS_REG_RANK_MASK);col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) &SYS_REG_COL_MASK);
bk = sys_reg & (1 << SYS_REG_BK_SHIFT(ch)) ? 3 : 0;cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &SYS_REG_CS0_ROW_MASK);cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &SYS_REG_CS1_ROW_MASK);bw = (sys_reg >> SYS_REG_BW_SHIFT(ch)) &SYS_REG_BW_MASK;row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &SYS_REG_ROW_3_4_MASK;
u32 ch_num = SYS_REG_DEC_NUM_CH(sys_reg);for (ch = 0; ch < ch_num; ch++) {rank = SYS_REG_DEC_RANK(sys_reg, ch);col = SYS_REG_DEC_COL(sys_reg, ch);bk = SYS_REG_DEC_BK(sys_reg, ch);cs0_row = SYS_REG_DEC_CS0_ROW(sys_reg, ch);cs1_row = SYS_REG_DEC_CS1_ROW(sys_reg, ch);bw = SYS_REG_DEC_BW(sys_reg, ch);row_3_4 = SYS_REG_DEC_ROW_3_4(sys_reg, ch); chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));-- 2.6.3
Regards, Simon
Regards, Simon
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com ---
Changes in v2: Modified into a more readable code style(Simon Glass)
arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c index e9e2211..17daeca 100644 --- a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c @@ -561,14 +561,14 @@ static void dram_all_config(const struct dram_info *dram, &sdram_params->ch[chan];
sys_reg |= info->row_3_4 << SYS_REG_ROW_3_4_SHIFT(chan); - sys_reg |= chan << SYS_REG_CHINFO_SHIFT(chan); + sys_reg |= 1 << SYS_REG_CHINFO_SHIFT(chan); sys_reg |= (info->rank - 1) << SYS_REG_RANK_SHIFT(chan); sys_reg |= (info->col - 9) << SYS_REG_COL_SHIFT(chan); - sys_reg |= info->bk == 3 ? 1 << SYS_REG_BK_SHIFT(chan) : 0; + sys_reg |= info->bk == 3 ? 0 : 1 << SYS_REG_BK_SHIFT(chan); sys_reg |= (info->cs0_row - 13) << SYS_REG_CS0_ROW_SHIFT(chan); sys_reg |= (info->cs1_row - 13) << SYS_REG_CS1_ROW_SHIFT(chan); - sys_reg |= info->bw << SYS_REG_BW_SHIFT(chan); - sys_reg |= info->dbw << SYS_REG_DBW_SHIFT(chan); + sys_reg |= (2 >> info->bw) << SYS_REG_BW_SHIFT(chan); + sys_reg |= (2 >>info->dbw) << SYS_REG_DBW_SHIFT(chan);
dram_cfg_rbc(&dram->chan[chan], chan, sdram_params); } @@ -720,13 +720,13 @@ size_t sdram_size_mb(struct rk3288_pmu *pmu) rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) & SYS_REG_RANK_MASK); col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK); - bk = sys_reg & (1 << SYS_REG_BK_SHIFT(ch)) ? 3 : 0; + bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK) ; cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) & SYS_REG_CS0_ROW_MASK); cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) & SYS_REG_CS1_ROW_MASK); - bw = (sys_reg >> SYS_REG_BW_SHIFT(ch)) & - SYS_REG_BW_MASK; + bw = (2 >> (sys_reg >> SYS_REG_BW_SHIFT(ch)) & + SYS_REG_BW_MASK); row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) & SYS_REG_ROW_3_4_MASK;
Hi Chris,
On 6 March 2016 at 23:51, Chris Zhong zyw@rock-chips.com wrote:
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com
Changes in v2: Modified into a more readable code style(Simon Glass)
arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Longer term, might it be possible to write this info into the device tree and pass it to Linux that way?
diff --git a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c index e9e2211..17daeca 100644 --- a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c @@ -561,14 +561,14 @@ static void dram_all_config(const struct dram_info *dram, &sdram_params->ch[chan];
sys_reg |= info->row_3_4 << SYS_REG_ROW_3_4_SHIFT(chan);
sys_reg |= chan << SYS_REG_CHINFO_SHIFT(chan);
sys_reg |= 1 << SYS_REG_CHINFO_SHIFT(chan); sys_reg |= (info->rank - 1) << SYS_REG_RANK_SHIFT(chan); sys_reg |= (info->col - 9) << SYS_REG_COL_SHIFT(chan);
sys_reg |= info->bk == 3 ? 1 << SYS_REG_BK_SHIFT(chan) : 0;
sys_reg |= info->bk == 3 ? 0 : 1 << SYS_REG_BK_SHIFT(chan); sys_reg |= (info->cs0_row - 13) << SYS_REG_CS0_ROW_SHIFT(chan); sys_reg |= (info->cs1_row - 13) << SYS_REG_CS1_ROW_SHIFT(chan);
sys_reg |= info->bw << SYS_REG_BW_SHIFT(chan);sys_reg |= info->dbw << SYS_REG_DBW_SHIFT(chan);
sys_reg |= (2 >> info->bw) << SYS_REG_BW_SHIFT(chan);sys_reg |= (2 >>info->dbw) << SYS_REG_DBW_SHIFT(chan); dram_cfg_rbc(&dram->chan[chan], chan, sdram_params); }@@ -720,13 +720,13 @@ size_t sdram_size_mb(struct rk3288_pmu *pmu) rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) & SYS_REG_RANK_MASK); col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
bk = sys_reg & (1 << SYS_REG_BK_SHIFT(ch)) ? 3 : 0;
bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK) ; cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) & SYS_REG_CS0_ROW_MASK); cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) & SYS_REG_CS1_ROW_MASK);
bw = (sys_reg >> SYS_REG_BW_SHIFT(ch)) &SYS_REG_BW_MASK;
bw = (2 >> (sys_reg >> SYS_REG_BW_SHIFT(ch)) &SYS_REG_BW_MASK); row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) & SYS_REG_ROW_3_4_MASK;-- 1.9.1
Regards, Simon
On 2016年03月08日 02:30, Simon Glass wrote:
Hi Chris,
On 6 March 2016 at 23:51, Chris Zhong zyw@rock-chips.com wrote:
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com
Changes in v2: Modified into a more readable code style(Simon Glass)
arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Longer term, might it be possible to write this info into the device tree and pass it to Linux that way?
Yes, agree, that would be better. At least not need so many complicated calculations.
diff --git a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c index e9e2211..17daeca 100644 --- a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c @@ -561,14 +561,14 @@ static void dram_all_config(const struct dram_info *dram, &sdram_params->ch[chan];
sys_reg |= info->row_3_4 << SYS_REG_ROW_3_4_SHIFT(chan);
sys_reg |= chan << SYS_REG_CHINFO_SHIFT(chan);
sys_reg |= 1 << SYS_REG_CHINFO_SHIFT(chan); sys_reg |= (info->rank - 1) << SYS_REG_RANK_SHIFT(chan); sys_reg |= (info->col - 9) << SYS_REG_COL_SHIFT(chan);
sys_reg |= info->bk == 3 ? 1 << SYS_REG_BK_SHIFT(chan) : 0;
sys_reg |= info->bk == 3 ? 0 : 1 << SYS_REG_BK_SHIFT(chan); sys_reg |= (info->cs0_row - 13) << SYS_REG_CS0_ROW_SHIFT(chan); sys_reg |= (info->cs1_row - 13) << SYS_REG_CS1_ROW_SHIFT(chan);
sys_reg |= info->bw << SYS_REG_BW_SHIFT(chan);sys_reg |= info->dbw << SYS_REG_DBW_SHIFT(chan);
sys_reg |= (2 >> info->bw) << SYS_REG_BW_SHIFT(chan);sys_reg |= (2 >>info->dbw) << SYS_REG_DBW_SHIFT(chan); dram_cfg_rbc(&dram->chan[chan], chan, sdram_params); }@@ -720,13 +720,13 @@ size_t sdram_size_mb(struct rk3288_pmu *pmu) rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) & SYS_REG_RANK_MASK); col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
bk = sys_reg & (1 << SYS_REG_BK_SHIFT(ch)) ? 3 : 0;
bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK) ; cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) & SYS_REG_CS0_ROW_MASK); cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) & SYS_REG_CS1_ROW_MASK);
bw = (sys_reg >> SYS_REG_BW_SHIFT(ch)) &SYS_REG_BW_MASK;
bw = (2 >> (sys_reg >> SYS_REG_BW_SHIFT(ch)) &SYS_REG_BW_MASK); row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) & SYS_REG_ROW_3_4_MASK;-- 1.9.1
Regards, Simon
On 6 March 2016 at 23:51, Chris Zhong zyw@rock-chips.com wrote:
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com
Changes in v2: Modified into a more readable code style(Simon Glass)
arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
On 10 March 2016 at 08:20, Simon Glass sjg@chromium.org wrote:
On 6 March 2016 at 23:51, Chris Zhong zyw@rock-chips.com wrote:
The DMC driver in v3.14 kernel[0] get the ddr setting from PMU_SYS_REG2, and it expects uboot to store the value using a same protocol. But now the ddr setting value is different with DMC, so if you enable the DMC, system would crash in kernel. Correct the sdram setting here, according to the requirements of kernel.
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ chromeos-3.14/drivers/clk/rockchip/clk-rk3288-dmc.c
Signed-off-by: Chris Zhong zyw@rock-chips.com
Changes in v2: Modified into a more readable code style(Simon Glass)
arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-rockchip, thanks!
participants (2)
-
Chris Zhong -
Simon Glass