Message ID | feea04ed1050c137c6867d66e4f06655ab692281.1576873692.git.tommyhebb@gmail.com |
---|---|
State | Accepted |
Commit | 95052b4b4001647ac0edd3216f92737cc06b1622 |
Delegated to: | Kever Yang |
Headers | show |
Series | ram: rk3399: don't assume phy_io_config() uses real regs | expand |
On 2019/12/21 上午4:28, Thomas Hebb wrote: > In the RK3399 DRAM driver, the function set_ds_odt() supports operating > in two different modes, selected by the ctl_phy_reg argument: when true, > the function reads and writes directly from the DRAM registers, accessed > through "chan->pctl->denali_*"; when false, the function reads and > writes from an array, accessed through "params->pctl_regs.denali_*", > which is written to DRAM registers at a later time. > > However, phy_config_io(), which is called by set_ds_odt() to do a subset > of its register operations, operates directly on DRAM registers at all > times. This means that it reads incorrect values (and writes new values > prematurely) when ctl_phy_reg in set_ds_odt() is false. Fix this by > passing in the address of the registers to work with. > > This prevents an "Invalid DRV value" error in the SPL debug log and > (presumably) results in a more correct end state. See the following logs > from a RK3399 NanoPi M4 board (4GB LPDDR3): > > Before: > > sdram_init() Starting SDRAM initialization... > phy_io_config() Invalid DRV value. > phy_io_config() Invalid DRV value. > sdram_init() sdram_init: data trained for rank 2, ch 0 > phy_io_config() Invalid DRV value. > phy_io_config() Invalid DRV value. > sdram_init() sdram_init: data trained for rank 2, ch 1 > Channel 0: LPDDR3, 933MHz > BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB > Channel 1: LPDDR3, 933MHz > BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB > 256B stride > 256B stride > sdram_init() Finish SDRAM initialization... > > After: > > sdram_init() Starting SDRAM initialization... > sdram_init() sdram_init: data trained for rank 2, ch 0 > sdram_init() sdram_init: data trained for rank 2, ch 1 > Channel 0: LPDDR3, 933MHz > BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB > Channel 1: LPDDR3, 933MHz > BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB > 256B stride > 256B stride > sdram_init() Finish SDRAM initialization... > > Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> Thanks, - Kever > --- > > drivers/ram/rockchip/sdram_rk3399.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c > index aa2ea920ba..9e61641511 100644 > --- a/drivers/ram/rockchip/sdram_rk3399.c > +++ b/drivers/ram/rockchip/sdram_rk3399.c > @@ -337,11 +337,9 @@ static void set_memory_map(const struct chan_info *chan, u32 channel, > writel(0x2EC7FFFF, &denali_pi[34]); > } > > -static int phy_io_config(const struct chan_info *chan, > +static int phy_io_config(u32 *denali_phy, u32 *denali_ctl, > const struct rk3399_sdram_params *params, u32 mr5) > { > - u32 *denali_phy = chan->publ->denali_phy; > - u32 *denali_ctl = chan->pctl->denali_ctl; > u32 vref_mode_dq, vref_value_dq, vref_mode_ac, vref_value_ac; > u32 mode_sel; > u32 speed; > @@ -780,7 +778,7 @@ static void set_ds_odt(const struct chan_info *chan, > /* phy_pad_fdbk_term 1bit DENALI_PHY_930 offset_17 */ > clrsetbits_le32(&denali_phy[930], 0x1 << 17, reg_value); > > - phy_io_config(chan, params, mr5); > + phy_io_config(denali_phy, denali_ctl, params, mr5); > } > > static void pctl_start(struct dram_info *dram,
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index aa2ea920ba..9e61641511 100644 --- a/drivers/ram/rockchip/sdram_rk3399.c +++ b/drivers/ram/rockchip/sdram_rk3399.c @@ -337,11 +337,9 @@ static void set_memory_map(const struct chan_info *chan, u32 channel, writel(0x2EC7FFFF, &denali_pi[34]); } -static int phy_io_config(const struct chan_info *chan, +static int phy_io_config(u32 *denali_phy, u32 *denali_ctl, const struct rk3399_sdram_params *params, u32 mr5) { - u32 *denali_phy = chan->publ->denali_phy; - u32 *denali_ctl = chan->pctl->denali_ctl; u32 vref_mode_dq, vref_value_dq, vref_mode_ac, vref_value_ac; u32 mode_sel; u32 speed; @@ -780,7 +778,7 @@ static void set_ds_odt(const struct chan_info *chan, /* phy_pad_fdbk_term 1bit DENALI_PHY_930 offset_17 */ clrsetbits_le32(&denali_phy[930], 0x1 << 17, reg_value); - phy_io_config(chan, params, mr5); + phy_io_config(denali_phy, denali_ctl, params, mr5); } static void pctl_start(struct dram_info *dram,
In the RK3399 DRAM driver, the function set_ds_odt() supports operating in two different modes, selected by the ctl_phy_reg argument: when true, the function reads and writes directly from the DRAM registers, accessed through "chan->pctl->denali_*"; when false, the function reads and writes from an array, accessed through "params->pctl_regs.denali_*", which is written to DRAM registers at a later time. However, phy_config_io(), which is called by set_ds_odt() to do a subset of its register operations, operates directly on DRAM registers at all times. This means that it reads incorrect values (and writes new values prematurely) when ctl_phy_reg in set_ds_odt() is false. Fix this by passing in the address of the registers to work with. This prevents an "Invalid DRV value" error in the SPL debug log and (presumably) results in a more correct end state. See the following logs from a RK3399 NanoPi M4 board (4GB LPDDR3): Before: sdram_init() Starting SDRAM initialization... phy_io_config() Invalid DRV value. phy_io_config() Invalid DRV value. sdram_init() sdram_init: data trained for rank 2, ch 0 phy_io_config() Invalid DRV value. phy_io_config() Invalid DRV value. sdram_init() sdram_init: data trained for rank 2, ch 1 Channel 0: LPDDR3, 933MHz BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB Channel 1: LPDDR3, 933MHz BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB 256B stride 256B stride sdram_init() Finish SDRAM initialization... After: sdram_init() Starting SDRAM initialization... sdram_init() sdram_init: data trained for rank 2, ch 0 sdram_init() sdram_init: data trained for rank 2, ch 1 Channel 0: LPDDR3, 933MHz BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB Channel 1: LPDDR3, 933MHz BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB 256B stride 256B stride sdram_init() Finish SDRAM initialization... Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> --- drivers/ram/rockchip/sdram_rk3399.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)