diff mbox series

ram: rk3399: don't assume phy_io_config() uses real regs

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

Commit Message

Tom Hebb Dec. 20, 2019, 8:28 p.m. UTC
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(-)

Comments

Kever Yang Jan. 6, 2020, 9:44 a.m. UTC | #1
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 mbox series

Patch

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,